ceph: osd_belongs_to_node: only check tree-entries of type host, refactor

We want to check explicitly for type host, so filter for that first
and create a hash map for easier usage afterwards.

Drop the error when there's no tree, as either RADOS error'd on bad
command already, or there really is no tree (but RADOS worked OK), in
which case we simply return that the OSD did not belong to this node.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>

diff --git a/PVE/API2/Ceph/OSD.pm b/PVE/API2/Ceph/OSD.pm
index 6763e95b93e400f7c598697b978c89a5f83903a3..27e0642c4d6938ecb0e62bf89989c39eb116afcc 100644 (file)
--- a/PVE/API2/Ceph/OSD.pm
+++ b/PVE/API2/Ceph/OSD.pm
@@ -482,15 +482,18 @@ __PACKAGE__->register_method ({
 # $tree ... rados osd tree (passing the tree makes it easy to test)
 sub osd_belongs_to_node {
     my ($tree, $nodename, $osdid) = @_;
+    return 0 if !($tree && $tree->{nodes});
 
-    die "No tree nodes found\n" if !($tree && $tree->{nodes});
-    my $allNodes = $tree->{nodes};
+    my $node_map = {};
+    for my $el (grep { defined($_->{type}) && $_->{type} eq 'host' } @{$tree->{nodes}}) {
+       my $name = $el->{name};
+       die "internal error: duplicate host name found '$name'\n" if $node_map->{$name};
+       $node_map->{$name} = $el;
+    }
 
-    my @match = grep($_->{name} eq $nodename, @$allNodes);
-    my $node = shift @match; # contains rados information about $nodename
-    die "There must not be more than one such node in the list" if @match;
+    my $osds = $node_map->{$nodename}->{children};
+    return 0 if !$osds;
 
-    my $osds = $node->{children};
     return grep($_ == $osdid, @$osds);
 }
 

diff --git a/test/OSD_test.pl b/test/OSD_test.pl
index df8c7881cbbafbac8d3abb9e6e39d313cc518329..174097708a1b3bd8ce27af3be0c82ceae4c82eae 100755 (executable)
--- a/test/OSD_test.pl
+++ b/test/OSD_test.pl
@@ -11,20 +11,26 @@ use PVE::API2::Ceph::OSD;
 
 use Data::Dumper;
 
+# NOTE: not exhausive, reduced to actually required fields!
 my $tree = {
     nodes => [
        {
            id => -3,
            name => 'pveA',
            children => [ 0,1,2,3 ],
-       }, {
+           type => 'host',
+       },
+       {
            id => -5,
            name => 'pveB',
            children => [ 4,5,6,7 ],
-       }, {
+           type => 'host',
+       },
+       {
            id => -7,
            name => 'pveC',
            children => [ 8,9,10,11 ],
+           type => 'host',
        },
     ],
 };
@@ -53,20 +59,22 @@ my $double_nodes_tree = {
     nodes => [
        {
            name => 'pveA',
+           type => 'host',
        },
        {
            name => 'pveA',
+           type => 'host',
        }
     ]
 };
 eval { PVE::API2::Ceph::OSD::osd_belongs_to_node($double_nodes_tree, 'pveA') };
-like($@, qr/not be more than one/, "Die if node occurs too often");
+like($@, qr/duplicate host name found/, "Die if node occurs too often");
 
-my $tree_without_nodes = {
-    dummy => 'dummy',
-};
-eval { PVE::API2::Ceph::OSD::osd_belongs_to_node(undef) };
-like($@, qr/No tree nodes/, "Die if tree has no nodes");
+is (
+    PVE::API2::Ceph::OSD::osd_belongs_to_node(undef),
+    0,
+    "Early-return false if there's no/empty node tree",
+);
 
 
-done_testing(@belong_to_B + @not_belong_to_B + 2);
\ No newline at end of file
+done_testing(@belong_to_B + @not_belong_to_B + 2);