[PATCH manager 1/1] api: ceph: pool: createpool: refuse EC layouts the cluster cannot place

[Kefu Chai <k.chai@proxmox.com>]

Each of the k + m chunks of an erasure coded pool needs its own failure
domain, and Ceph serves I/O on a placement group only once min_size of
them are placed, which is k + 1, or k if m is 1. createpool does not
check this, so on three hosts

  # pveceph pool create test-ec --erasure-coding k=4,m=2 --pg_num 64

succeeds, but every placement group of the data pool stays incomplete,
and creating an image on it hangs until pvesm alloc times out:

  rbd create 'vm-999-disk-0' error: 'storage-test-ec'-locked command
  timed out - aborting

So count the failure domains below the CRUSH root that hold a weighted
OSD, of the device class if given, and refuse the pool if there are
fewer than min_size. If there are fewer than k + m, just note in the
task log that the pool stays degraded. Out OSDs only lead to such a
note, as they are usually back in soon.

Check an existing profile the same way, with its own failure domain,
device class and root, if it places k + m chunks, one per failure
domain: the jerasure, isa, clay and shec plugins do, lrc does not.

Signed-off-by: Kefu Chai <k.chai@proxmox.com>
---
 PVE/API2/Ceph/Pool.pm |  52 ++++++++++
 PVE/Ceph/Tools.pm     |  48 +++++++++
 test/CephPool_test.pl | 232 +++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 329 insertions(+), 3 deletions(-)

diff --git a/PVE/API2/Ceph/Pool.pm b/PVE/API2/Ceph/Pool.pm
index aac94996..a27568ec 100644
--- a/PVE/API2/Ceph/Pool.pm
+++ b/PVE/API2/Ceph/Pool.pm
@@ -416,6 +416,35 @@ my $ec_format = {
     },
 };
 
+# Refuse an erasure code layout that CRUSH cannot place. Each of the k + m chunks needs its
+# own failure domain, and Ceph serves I/O only once min_size of them are placed, which it
+# sets to k + 1, or to k if m is 1. Returns a warning if not all chunks can be placed. OSDs
+# that are out only lead to a warning, as they are usually back in soon.
+my $check_ec_layout = sub {
+    my ($k, $m, $domain, $class, $root, $rados) = @_;
+
+    my ($have, $in) = PVE::Ceph::Tools::count_failure_domains($domain, $class, $root, $rados);
+    my $with = $class ? " with '$class' OSDs" : '';
+    die "found no CRUSH buckets of type '$domain'$with below root '$root'.\n" if !$have;
+
+    my $size = $k + $m;
+    my $min_size = $m > 1 ? $k + 1 : $k;
+    die "k=$k and m=$m need at least $min_size '$domain' failure domains$with to serve I/O,"
+        . " but there are only $have.\n"
+        if $have < $min_size;
+    return if $in >= $size;
+
+    my $msg =
+        $in < $min_size
+        ? "k=$k and m=$m need at least $min_size '$domain' failure domains$with to serve I/O,"
+        : "k=$k and m=$m need $size '$domain' failure domains$with to place all chunks,";
+    return "$msg but there are only $have. The pool will stay degraded." if $in == $have;
+    $msg .= " but only $in of the $have have OSDs that are in.";
+    return "$msg The pool cannot serve I/O until the out OSDs are back in." if $in < $min_size;
+    return "$msg The pool will stay degraded." if $have < $size;
+    return "$msg The pool will stay degraded until the out OSDs are back in.";
+};
+
 sub ec_parse_and_check {
     my ($property, $rados) = @_;
     return if !$property;
@@ -426,6 +455,15 @@ sub ec_parse_and_check {
     if (!defined($name)) {
         die "'k' and 'm' are required if no 'profile' is given.\n"
             if !defined($ec->{k}) || !defined($ec->{m});
+
+        my $warning = $check_ec_layout->(
+            $ec->@{qw(k m)},
+            $ec->{'failure-domain'} // 'host',
+            $ec->{'device-class'},
+            'default',
+            $rados,
+        );
+        $ec->{warning} = $warning if $warning;
         return $ec;
     }
 
@@ -450,6 +488,19 @@ sub ec_parse_and_check {
         die "'$opt' is '$ec->{$opt}', but erasure code profile '$name' has $has.\n";
     }
 
+    # lrc places more than k + m chunks, and a profile can allow several per failure domain
+    my $plugin = $profile->{plugin};
+    my $per_domain = $profile->{'crush-osds-per-failure-domain'} || 0;
+    if ((grep { $plugin eq $_ } qw(jerasure isa clay shec)) && $per_domain <= 1) {
+        my $warning = $check_ec_layout->(
+            $actual->@{qw(k m failure-domain)},
+            $actual->{'device-class'} || undef,
+            $profile->{'crush-root'} || 'default',
+            $rados,
+        );
+        $ec->{warning} = $warning if $warning;
+    }
+
     return $ec;
 }
 
@@ -529,6 +580,7 @@ __PACKAGE__->register_method({
                 PVE::RADOS->new(timeout => PVE::Ceph::Tools::get_config('long_rados_timeout'));
 
             if ($ec) {
+                print "$ec->{warning}\n" if $ec->{warning};
                 if (!defined($ec->{profile})) {
                     $ec->{profile} = PVE::Ceph::Tools::get_ecprofile_name($pool, $rados);
                     eval {
diff --git a/PVE/Ceph/Tools.pm b/PVE/Ceph/Tools.pm
index 804ae262..1b13c83c 100644
--- a/PVE/Ceph/Tools.pm
+++ b/PVE/Ceph/Tools.pm
@@ -691,6 +691,54 @@ sub destroy_ecprofile {
     return $rados->mon_command($command);
 }
 
+# Count the CRUSH buckets of type $failure_domain below the bucket named $root that hold an
+# OSD with weight, of $device_class if given. Returns that count, and how many of them hold
+# such an OSD that is also in, as CRUSH skips the ones that are out.
+sub count_failure_domains {
+    my ($failure_domain, $device_class, $root, $rados) = @_;
+    $rados = PVE::RADOS->new() if !$rados;
+
+    my $tree = $rados->mon_command({ prefix => 'osd tree' });
+    my $nodes = { map { $_->{id} => $_ } $tree->{nodes}->@* };
+    my $children = sub {
+        return map { $nodes->{$_} // () } ($_[0]->{children} // [])->@*;
+    };
+
+    my $weighted = sub {
+        my ($osd) = @_;
+        return 0 if !$osd->{crush_weight};
+        return !$device_class || ($osd->{device_class} // '') eq $device_class;
+    };
+
+    # a bucket can be linked below several parents, so visit each one once
+    my ($with_weight, $in, %seen) = (0, 0);
+    my @todo = grep { $_->{name} eq $root && $_->{type} ne 'osd' } $tree->{nodes}->@*;
+    while (my $node = shift @todo) {
+        next if $seen{ $node->{id} }++;
+        if ($node->{type} ne $failure_domain) {
+            push @todo, $children->($node);
+            next;
+        }
+        my ($has_weight, $has_in);
+        my @below = ($node);
+        while (my $item = shift @below) {
+            if ($item->{type} ne 'osd') {
+                push @below, $children->($item);
+            } elsif ($weighted->($item)) {
+                $has_weight = 1;
+                if ($item->{reweight}) {
+                    $has_in = 1;
+                    last;
+                }
+            }
+        }
+        $with_weight++ if $has_weight;
+        $in++ if $has_in;
+    }
+
+    return ($with_weight, $in);
+}
+
 sub get_ecprofile_name {
     my ($name) = @_;
     return "pve_ec_${name}";
diff --git a/test/CephPool_test.pl b/test/CephPool_test.pl
index c4385fba..b8bd4943 100755
--- a/test/CephPool_test.pl
+++ b/test/CephPool_test.pl
@@ -53,11 +53,44 @@ use PVE::Ceph::Tools;
 
     sub mon_command {
         my ($self, $cmd) = @_;
+        return { nodes => $self->{nodes} // [], stray => [] } if $cmd->{prefix} eq 'osd tree';
         push $self->{cmds}->@*, $cmd;
         return;
     }
 }
 
+# an 'osd tree' with one host per [name, [device class, crush weight, reweight], ...] below
+# the root 'default', where the reweight defaults to 1 (in)
+sub osd_tree {
+    my (@hosts) = @_;
+    my ($osd_id, $host_id) = (0, -2);
+    my (@nodes, @host_ids);
+    for my $host (@hosts) {
+        my ($name, @osds) = @$host;
+        my @ids;
+        for my $osd (@osds) {
+            my $id = $osd_id++;
+            push @ids, $id;
+            push @nodes,
+                {
+                    id => $id,
+                    name => "osd.$id",
+                    type => 'osd',
+                    device_class => $osd->[0],
+                    crush_weight => $osd->[1],
+                    reweight => $osd->[2] // 1,
+                };
+        }
+        push @nodes, { id => $host_id, name => $name, type => 'host', children => \@ids };
+        push @host_ids, $host_id--;
+    }
+    push @nodes, { id => -1, name => 'default', type => 'root', children => \@host_ids };
+    return \@nodes;
+}
+
+# enough hosts for any layout the other tests use
+my $big_tree = osd_tree(map { ["host$_", ['hdd', 1], ['ssd', 1]] } 1 .. 8);
+
 # call an API method, run its worker inline, and return what it printed
 my $run = sub {
     my ($method, $param) = @_;
@@ -133,15 +166,17 @@ for my $case (@own_cases) {
 my $rados = FakeRados->new(
     default => { k => '2', m => '2', plugin => 'isa', technique => 'reed_sol_van' },
     ssd => {
+        plugin => 'isa',
         k => '4',
         m => '2',
         'crush-failure-domain' => 'osd',
         'crush-device-class' => 'ssd',
     },
-    0 => { k => '3', m => '1' },
-    typed => { k => '02', m => '+1' },
+    0 => { plugin => 'isa', k => '3', m => '1' },
+    typed => { plugin => 'isa', k => '02', m => '+1' },
     broken => -5,
 );
+$rados->{nodes} = $big_tree;
 
 my $parse = sub { PVE::API2::Ceph::Pool::ec_parse_and_check($_[0], $rados) };
 
@@ -237,7 +272,8 @@ for my $case (@parse_cases) {
 {
     no warnings qw(redefine once);
     my (@profiles, @pools);
-    my $rados = FakeRados->new(0 => { k => '2', m => '1' });
+    my $rados = FakeRados->new(0 => { plugin => 'isa', k => '2', m => '1' });
+    $rados->{nodes} = $big_tree;
 
     local *PVE::RPCEnvironment::get = sub { bless {}, 'PoolRPCEnvironment' };
     local *PVE::Cluster::check_cfs_quorum = sub { };
@@ -270,6 +306,196 @@ for my $case (@parse_cases) {
         is(join(', ', @profiles), $profiles, "$name (profiles)");
         is(join(', ', @pools), $pools, "$name (pools)");
     }
+
+    $rados->{nodes} = osd_tree(map { ["pve$_", ['hdd', 1]] } 1 .. 3);
+    my $out = $run->(
+        'createpool',
+        { name => 'foo', node => 'node', 'erasure-coding' => 'k=2,m=2', add_storages => 0 },
+    );
+    like(
+        $out,
+        qr/^k=2 and m=2 need 4 .* will stay degraded\.$/m,
+        'the task log notes a degraded pool',
+    );
+}
+
+# --- where the chunks of a profile can go -------------------------------------------------
+{
+    # three hosts with an HDD each, one with two SSDs, one with an HDD of weight 0, one with
+    # an HDD that is out, an empty host, a host with an HDD below another root, and a
+    # datacenter holding pve1 and pve2
+    my $nodes = osd_tree(
+        ['pve1', ['hdd', 0.5]],
+        ['pve2', ['hdd', 0.5]],
+        ['pve3', ['hdd', 0.5]],
+        ['pve4', ['ssd', 0.5], ['ssd', 0.5]],
+        ['pve5', ['hdd', 0]],
+        ['pve6', ['hdd', 0.5, 0]],
+        ['pve7'],
+    );
+    push $nodes->@*,
+        {
+            id => 100,
+            name => 'osd.100',
+            type => 'osd',
+            device_class => 'hdd',
+            crush_weight => 1,
+            reweight => 1,
+        },
+        { id => -50, name => 'other', type => 'host', children => [100] },
+        { id => -100, name => 'other-root', type => 'root', children => [-50] },
+        { id => -30, name => 'dc1', type => 'datacenter', children => [-2, -3] };
+    # as Ceph stores them, with the plugin always set
+    my $rados = FakeRados->new(
+        wide => { plugin => 'isa', k => '6', m => '2' },
+        clay => { plugin => 'clay', k => '6', m => '2', d => '7' },
+        shec => { plugin => 'shec', k => '6', m => '2', c => '2' },
+        per1 => { plugin => 'isa', k => '6', m => '2', 'crush-osds-per-failure-domain' => '1' },
+        small => { plugin => 'jerasure', k => '2', m => '2', 'crush-device-class' => 'hdd' },
+        lrc => { plugin => 'lrc', k => '4', m => '2', l => '3' },
+        msr => {
+            plugin => 'isa',
+            k => '4',
+            m => '2',
+            'crush-num-failure-domains' => '2',
+            'crush-osds-per-failure-domain' => '2',
+        },
+        rooted => { plugin => 'isa', k => '2', m => '1', 'crush-root' => 'other-root' },
+        dc => { plugin => 'isa', k => '2', m => '1', 'crush-root' => 'dc1' },
+    );
+    $rados->{nodes} = $nodes;
+
+    my @count_cases = (
+        # failure domain, device class, root, with weight, of those with an OSD in, name
+        ['host', undef, 'default', 5, 4, 'only hosts with an OSD that has weight count'],
+        ['host', 'hdd', 'default', 4, 3, 'a device class limits the hosts'],
+        ['host', 'ssd', 'default', 1, 1, 'a host counts once however many OSDs it has'],
+        ['osd', undef, 'default', 6, 5, "the 'osd' failure domain counts OSDs"],
+        ['rack', undef, 'default', 0, 0, 'a failure domain type without buckets counts none'],
+        ['host', undef, 'other-root', 1, 1, 'another root counts its own hosts'],
+        ['host', undef, 'dc1', 2, 2, 'any bucket can be the root, like a datacenter'],
+    );
+    for my $case (@count_cases) {
+        my ($domain, $class, $root, $weighted, $in, $name) = @$case;
+        is_deeply(
+            [PVE::Ceph::Tools::count_failure_domains($domain, $class, $root, $rados)],
+            [$weighted, $in],
+            $name,
+        );
+    }
+
+    # pve3 is linked below both racks
+    my $linked = FakeRados->new();
+    $linked->{nodes} = osd_tree(['pve1', ['hdd', 1]], ['pve2', ['hdd', 1]], ['pve3', ['hdd', 1]]);
+    my ($root) = grep { $_->{type} eq 'root' } $linked->{nodes}->@*;
+    push $linked->{nodes}->@*,
+        { id => -20, name => 'r1', type => 'rack', children => [-2, -3, -4] },
+        { id => -21, name => 'r2', type => 'rack', children => [-4] };
+    $root->{children} = [-20, -21];
+    is_deeply(
+        [PVE::Ceph::Tools::count_failure_domains('host', undef, 'default', $linked)],
+        [3, 3],
+        'a host linked below two racks counts once',
+    );
+
+    my @check_cases = (
+        # property, expected error, expected warning, name
+        [
+            'k=4,m=2,device-class=hdd',
+            qr/need at least 5 'host' failure domains with 'hdd' OSDs .* only 4/,
+            undef,
+            'k=4,m=2 is refused, as an OSD of weight 0 does not count',
+        ],
+        ['k=2,m=1,device-class=hdd', undef, undef, 'k=2,m=1 fits the hosts with an OSD in'],
+        [
+            'k=2,m=2,device-class=hdd',
+            undef,
+            qr/need 4 'host' .* only 3 of the 4 have OSDs that are in\. .* until the out OSDs/,
+            'k=2,m=2 can serve I/O, but stays degraded while an OSD is out',
+        ],
+        [
+            'k=3,m=1,device-class=hdd',
+            undef,
+            qr/stay degraded/,
+            'with m=1 Ceph needs only k chunks, but the missing one is still noted',
+        ],
+        [
+            'k=4,m=1,device-class=hdd',
+            undef,
+            qr/need at least 4 .* only 3 of the 4 .* cannot serve I\/O until the out OSDs/,
+            'an OSD that is out does not refuse the pool, but is noted',
+        ],
+        [
+            'k=4,m=1,device-class=ssd',
+            qr/need at least 4 'host' failure domains with 'ssd' OSDs .* only 1/,
+            undef,
+            'k=4,m=1 on one SSD host is refused',
+        ],
+        ['k=4,m=1,failure-domain=osd', undef, undef, "the 'osd' failure domain has enough"],
+        [
+            'k=2,m=1,failure-domain=hots',
+            qr/^found no CRUSH buckets of type 'hots' below root 'default'/,
+            undef,
+            'a failure domain type without buckets is named as such',
+        ],
+        [
+            'k=2,m=1,device-class=nvme',
+            qr/^found no CRUSH buckets of type 'host' with 'nvme' OSDs/,
+            undef,
+            'so is a device class no OSD has',
+        ],
+        ['profile=wide', qr/need at least 7 'host' failure domains/, undef, 'an isa profile'],
+        ['profile=clay', qr/need at least 7 'host' failure domains/, undef, 'a clay profile'],
+        ['profile=shec', qr/need at least 7 'host' failure domains/, undef, 'a shec profile'],
+        [
+            'profile=per1',
+            qr/need at least 7 'host' failure domains/,
+            undef,
+            'a profile with one chunk per failure domain',
+        ],
+        [
+            'profile=small',
+            undef,
+            qr/stay degraded/,
+            'a jerasure profile, with its device class',
+        ],
+        ['profile=lrc', undef, undef, 'other plugins are not checked'],
+        [
+            'profile=msr',
+            undef,
+            undef,
+            'nor are profiles with several chunks per failure domain',
+        ],
+        [
+            'profile=rooted',
+            qr/need at least 2 'host' failure domains .* only 1/,
+            undef,
+            "a profile's own CRUSH root is counted",
+        ],
+        [
+            'profile=dc',
+            undef,
+            qr/need 3 'host' failure domains .* only 2\. The pool will stay degraded\.$/,
+            'a CRUSH root of another type is counted too',
+        ],
+    );
+    my @warned;
+    local $SIG{__WARN__} = sub { push @warned, @_ };
+    for my $case (@check_cases) {
+        my ($property, $error, $warning, $name) = @$case;
+        my $ec = eval { PVE::API2::Ceph::Pool::ec_parse_and_check($property, $rados) };
+        if (defined($error)) {
+            like($@, $error, $name);
+        } else {
+            is($@, '', $name);
+        }
+        if (defined($warning)) {
+            like($ec->{warning}, $warning, "$name (warning)");
+        } else {
+            is($ec ? $ec->{warning} : undef, undef, "$name (no warning)");
+        }
+    }
+    is(join('', @warned), '', 'the checks leave the warning to the task log');
 }
 
 done_testing();
-- 
2.47.3