[PATCH manager v2 3/4] migrations: cephx: rotate the monitor key with monitors we do not manage

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

Rotating the 'mon.' key writes each monitor's keyring and restarts the
monitors, which needs a shell on their hosts. A monitor outside this
cluster made the rotation fail.

Such a monitor does not need either step. It gets the new key from the
auth database like every other monitor. Only the keyring in its data
directory keeps the old key, and a monitor uses that keyring only if a
peer rejects it.

So write and restart only the monitors we manage. For the others, print
the commands to refresh their keyring and check that they are still in
the quorum. Skip the keyring checks for them in the preflight.

Signed-off-by: Kefu Chai <k.chai@proxmox.com>
---
 bin/pve-cephx-rotate-service-keys   |  76 +++++++++++++--
 test/CephKeyMigrationScript_test.pl | 142 +++++++++++++++++++++++++++-
 test/CephKeyMigration_test.pl       |  16 ++++
 3 files changed, 225 insertions(+), 9 deletions(-)

diff --git a/bin/pve-cephx-rotate-service-keys b/bin/pve-cephx-rotate-service-keys
index 8048944b..c7565e9e 100755
--- a/bin/pve-cephx-rotate-service-keys
+++ b/bin/pve-cephx-rotate-service-keys
@@ -189,6 +189,14 @@ my sub daemon_location($type, $id, $host) {
     };
 }
 
+my sub mon_keyring_path($id) {
+    return "/var/lib/ceph/mon/$ccname-$id/keyring";
+}
+
+my sub unmanaged_monitors($info) {
+    return grep { !$_->{managed} } $info->{daemons}->{mon}->@*;
+}
+
 my sub assert_located($node, $entity) {
     die "Cannot manage '$entity': no valid host name was reported\n"
         if !defined($node) || ref($node) || !length($node);
@@ -1589,7 +1597,9 @@ my sub probe_nodes($info, $plan, $opts = {}) {
     my $installed = installed_versions();
     my $specs = {};
     for my $daemon (touched_daemons($info, $plan)) {
-        assert_member($daemon->{node}, "$daemon->{type}.$daemon->{id}");
+        assert_located($daemon->{node}, "$daemon->{type}.$daemon->{id}");
+        # a host we do not manage has nothing to probe, so its running version is checked instead
+        next if !$daemon->{managed};
         push $specs->{ $daemon->{node} }->@*, "$daemon->{type}:$daemon->{id}";
     }
 
@@ -1612,6 +1622,7 @@ my sub probe_nodes($info, $plan, $opts = {}) {
             for $info->{daemons}->{$type}->@*;
     }
     for my $daemon (touched_daemons($info, $plan)) {
+        next if !$daemon->{managed};
         my $probe = $by_node->{ $daemon->{node} }->{"$daemon->{type}:$daemon->{id}"} // {};
         $daemon->{store} = $probe->{store};
         $daemon->{error} = $probe->{error};
@@ -2598,6 +2609,10 @@ my sub preflight_nodes($info, $plan, $opts) {
 
     my @unusable;
     for my $daemon (@touched) {
+        # a monitor gets the 'mon.' key from the auth database, so it needs no keyring on its host.
+        # Other daemons read their key from a file there.
+        next if !$daemon->{managed} && $daemon->{type} eq 'mon';
+
         eval { assert_daemon_location($info->{rados}, $daemon, $info->{fsid}, $daemon) };
         push @unusable, $@ if $@;
         my $type = $daemon->{type};
@@ -2893,14 +2908,25 @@ my sub print_plan($info, $plan, $state, $opts, $storage_entities) {
         $step++;
         log_text("Step $step: rotate the shared 'mon.' key. Every keyring is written first, then"
             . " the monitors restart one at a time.");
+        my @unmanaged = unmanaged_monitors($info);
+        log_step("Not managed by this cluster, so their emergency keyring keeps the old key: "
+                . join(', ', map { "mon.$_->{id} on '$_->{node}'" } @unmanaged)
+                . ". The commands for their admin follow at the end of the run.")
+            if @unmanaged;
         if ($opts->{verbose}) {
             log_step(
                 $info->{mon_key_in_auth_db}
                 ? "Ceph lists the key in its health checks."
                 : "Ceph's health checks cannot see the key."
             );
-            log_step("monitors, restarted one at a time: "
-                . join(', ', map { "$_->{id} (node $_->{node})" } $info->{daemons}->{mon}->@*));
+            log_step(
+                "monitors, restarted one at a time: "
+                    . join(
+                        ', ',
+                        map { "$_->{id} (node $_->{node})" }
+                        grep { $_->{managed} } $info->{daemons}->{mon}->@*,
+                    ),
+            );
         }
     }
 
@@ -3253,6 +3279,37 @@ my sub merge_pve_mon_keyring($entry) {
     return;
 }
 
+# Such a monitor already uses the new key from the auth database. Only the keyring in its data
+# directory, which it uses if a peer rejects it, keeps the old key, and only its admin can fix that.
+my sub report_unmanaged_monitors($rados, $unmanaged) {
+    log_warn("These monitors run on hosts outside this Proxmox VE cluster. They use the new key"
+        . " already, while their emergency keyring still holds the old one. To bring that copy up"
+        . " to date, their admin can run:");
+    for my $mon (@$unmanaged) {
+        my $path = mon_keyring_path($mon->{id});
+        log_step("on '$mon->{node}', as root:");
+        log_step("  ceph auth get mon. -o $path");
+        log_step("  chown ceph:ceph $path && chmod 600 $path");
+        log_step("  systemctl restart ceph-mon\@$mon->{id}");
+    }
+    log_text("The first command needs an admin keyring on that host. Without one, run it on a node"
+        . " of this cluster and copy the file over. Nothing is broken until then, but such a"
+        . " monitor cannot authenticate from its keyring alone, which is what it falls back on"
+        . " after losing its store.");
+
+    my @lost = map { "mon.$_->{id} on '$_->{node}'" }
+        grep { !daemon_is_running($rados, 'mon', $_->{id}) } @$unmanaged;
+    if (@lost) {
+        log_warn("These are not in the quorum right now, so check them before anything else: "
+            . join(', ', @lost));
+        return;
+    }
+
+    log_pass("every monitor outside this cluster is still in the quorum");
+
+    return;
+}
+
 my sub migrate_mon_key($rados, $state, $info, $opts, $plan) {
     log_heading(
         $plan->{mon_repair_only}
@@ -3263,8 +3320,9 @@ my sub migrate_mon_key($rados, $state, $info, $opts, $plan) {
     # a stale-copy repair is not gated behind the opt-in, so it must not rotate and restart the
     # quorum unasked. Finishing a started rotation is the exception
     my $rotate = !$plan->{mon_repair_only};
-    assert_daemon_location($rados, $_, $info->{fsid})
-        for $rotate ? $info->{daemons}->{mon}->@* : ();
+    my @managed = grep { $_->{managed} } $info->{daemons}->{mon}->@*;
+    my @unmanaged = unmanaged_monitors($info);
+    assert_daemon_location($rados, $_, $info->{fsid}) for $rotate ? @managed : ();
     my $entry = $rotate ? rotate_entity($rados, $state, 'mon.') : auth_entry($rados, 'mon.');
     my $keyring = keyring_text($entry);
     my $target = key_fingerprint($entry->{key});
@@ -3272,11 +3330,11 @@ my sub migrate_mon_key($rados, $state, $info, $opts, $plan) {
     merge_pve_mon_keyring($entry) if ($info->{pve_mon_key} // '') ne $entry->{key};
 
     # all keyrings first, so a monitor going down in between still finds the new key locally
-    for my $mon ($info->{daemons}->{mon}->@*) {
+    for my $mon (@managed) {
         next if $plan->{mon_repair_only};
         next if ($state->{mon_keyring}->{ $mon->{id} } // '') eq $target;
 
-        my $path = "/var/lib/ceph/mon/$ccname-$mon->{id}/keyring";
+        my $path = mon_keyring_path($mon->{id});
         log_info("writing the new key to '$path' on node '$mon->{node}'");
         write_node_file($mon->{node}, $path, $keyring);
 
@@ -3285,7 +3343,7 @@ my sub migrate_mon_key($rados, $state, $info, $opts, $plan) {
     }
 
     # only monitors holding the superseded key restart, so a keyring repair leaves the quorum alone
-    for my $mon ($info->{daemons}->{mon}->@*) {
+    for my $mon (@managed) {
         next if $plan->{mon_repair_only};
         next if ($state->{mon_restarted}->{ $mon->{id} } // '') eq $target;
 
@@ -3321,6 +3379,8 @@ my sub migrate_mon_key($rados, $state, $info, $opts, $plan) {
 
     log_pass("the shared monitor key now uses the '$CIPHER' cipher");
 
+    report_unmanaged_monitors($rados, \@unmanaged) if @unmanaged;
+
     return;
 }
 
diff --git a/test/CephKeyMigrationScript_test.pl b/test/CephKeyMigrationScript_test.pl
index 6ccadbc4..5d251596 100755
--- a/test/CephKeyMigrationScript_test.pl
+++ b/test/CephKeyMigrationScript_test.pl
@@ -6193,6 +6193,49 @@ for my $case ([0, 0], [1, 0], [0, 1], [1, 1]) {
         return ($verdict, $output // '');
     };
 
+    # an unmanaged monitor gets the key from the auth database, so no keyring is required there
+    {
+        my $mons = [
+            (
+                map { {
+                    type => 'mon',
+                    id => "$_",
+                    entity => 'mon.',
+                    node => "node$_",
+                    managed => 1,
+                    store => 'file',
+                    version => '20.2.4',
+                    binary => '20.2.4',
+                } } 1 .. 2
+            ),
+            {
+                type => 'mon',
+                id => 'tiebreaker',
+                entity => 'mon.',
+                node => 'witness.example',
+                managed => 0,
+                version => '20.2.4',
+            },
+        ];
+        my $info = {
+            daemons => { mon => $mons, mgr => [], mds => [], osd => [] },
+            monmap_mons => [map { $_->{id} } @$mons],
+            fsid => 'cluster-fsid',
+            rados => MonCountRados->new(),
+        };
+        my ($output, $verdict) = ('');
+        {
+            local *STDOUT;
+            open(STDOUT, '>', \$output) or die $!;
+            $verdict = $HOOKS->{preflight_nodes}->(
+                $info, { daemons => [], lockbox_keys => [], mon_key => 1 }, {},
+            );
+        }
+        is($verdict, 1, 'a monitor we do not manage does not block the monitor key rotation');
+        unlike($output, qr/neither a keyring file nor a bluestore device/, 'nothing is read there');
+        unlike($output, qr/Cannot manage 'mon\.tiebreaker'/, 'and nothing is verified there');
+    }
+
     my ($verdict, $output) = $preflight->(2, { mon_key => 1 });
     is($verdict, -1, 'a two-monitor cluster refuses the shared monitor key rotation');
     like($output, qr/monitor map holds 2 monitors/, 'naming what it found');
@@ -7827,7 +7870,10 @@ for my $case ([0, 0], [1, 0], [0, 1], [1, 1]) {
         fsid => 'cluster-fsid',
         pve_mon_key => $OLD,
         exported => { 'mon.' => { key => $OLD } },
-        daemons => { mon => [map { { type => 'mon', id => $_, node => "node-$_" } } qw(a b)] },
+        daemons => {
+            mon =>
+                [map { { type => 'mon', id => $_, node => "node-$_", managed => 1 } } qw(a b)],
+        },
     };
     my $plan = { mon_key => 1 };
     my $state = {};
@@ -7896,6 +7942,100 @@ for my $case ([0, 0], [1, 0], [0, 1], [1, 1]) {
     );
 }
 
+# A monitor outside this cluster gets the rotated key from the auth database. Its keyring and
+# restart are left to its admin.
+{
+    no warnings qw(once redefine);
+    local *PVE::Ceph::Services::get_blocking_health_errors = sub { return [] };
+    local *PVE::Ceph::Services::wait_for_safe_to_stop = sub { return (1, '') };
+    local *PVE::Ceph::Services::wait_for_daemon_up = sub { };
+    local *PVE::SSHInfo::get_ssh_info = sub { return { node => $_[0] } };
+    local *PVE::SSHInfo::ssh_info_to_command = sub { return ['ssh', $_[0]->{node}, '--'] };
+    local *main::file_set_contents = sub { };
+
+    my @commands;
+    local *main::run_command = sub { push @commands, join(' ', $_[0]->@*) };
+
+    my $run = sub {
+        my ($quorum) = @_;
+        my $rados = ClientRotationRados->new($OLD);
+        my $mon_command = ClientRotationRados->can('mon_command');
+        local *ClientRotationRados::mon_command = sub {
+            my ($self, $args) = @_;
+            die "the cluster cannot be read\n"
+                if !defined($quorum) && $args->{prefix} =~ m/^(?:quorum_status|health)$/;
+            return $quorum if $args->{prefix} eq 'quorum_status';
+            return {} if $args->{prefix} eq 'health';
+            return $mon_command->(@_);
+        };
+        my $info = {
+            fsid => 'cluster-fsid',
+            pve_mon_key => $OLD,
+            exported => { 'mon.' => { key => $OLD } },
+            daemons => {
+                mon => [
+                    (
+                        map { { type => 'mon', id => $_, node => "node-$_", managed => 1 } }
+                            qw(a b)
+                    ),
+                    {
+                        type => 'mon',
+                        id => 'tiebreaker',
+                        node => 'witness.example',
+                        managed => 0,
+                    },
+                ],
+            },
+        };
+        my $state = {};
+        my $out = '';
+        {
+            open(my $stdout, '>', \$out) or die $!;
+            local *STDOUT = $stdout;
+            eval {
+                $HOOKS->{migrate_mon_key}
+                    ->($rados, $state, $info, { timeout => 30 }, { mon_key => 1 });
+            };
+            main::log_fail($@) if $@;
+        }
+        return ($state, $out, $rados);
+    };
+
+    @commands = ();
+    my ($state, $out, $rados) = $run->({ quorum_names => [qw(a b tiebreaker)] });
+    is($rados->{key}, $NEW, 'the shared key is rotated, not refused over the outside monitor');
+    is_deeply(
+        $state->{mon_restarted},
+        { map { $_ => key_fingerprint($NEW) } qw(a b) },
+        'the monitors this cluster manages are restarted',
+    );
+    is(
+        scalar(grep { /ssh witness\.example/ } @commands),
+        0,
+        'and nothing is sent to the host outside it',
+    );
+    like(
+        $out,
+        qr{ceph auth get mon\. -o /var/lib/ceph/mon/\S+-tiebreaker/keyring},
+        'the admin is handed the keyring command for that monitor',
+    );
+    like($out, qr/systemctl restart ceph-mon\@tiebreaker/, 'and the restart to run there');
+    like($out, qr/chmod 600/, 'with the mode that keeps the key private');
+    like($out, qr/needs an admin keyring on that host/, 'and what to do without admin access');
+    like($out, qr/PASS.*outside this cluster is still in the quorum/, 'the quorum is confirmed');
+
+    (undef, $out) = $run->({ quorum_names => [qw(a b)] });
+    like(
+        $out,
+        qr/WARN.*not in the quorum right now.*mon\.tiebreaker on 'witness\.example'/,
+        'a monitor that left the quorum is named instead',
+    );
+
+    # an unreadable cluster says nothing about that monitor, and the rotation is done by then
+    (undef, $out) = $run->(undef);
+    unlike($out, qr/not in the quorum right now/, 'an unreadable cluster accuses nobody');
+}
+
 {
     my $out = '';
     {
diff --git a/test/CephKeyMigration_test.pl b/test/CephKeyMigration_test.pl
index d22ac24b..d087c76a 100755
--- a/test/CephKeyMigration_test.pl
+++ b/test/CephKeyMigration_test.pl
@@ -566,6 +566,22 @@ is(
         1,
         'repairing only the stored copy writes to none of them, so none is validated either',
     );
+
+    # its key is rotated too, so its version is checked. Only the work on its node is skipped.
+    my $stretch = {
+        daemons => {
+            mon => [
+                { entity => 'mon.', id => 'a', managed => 1 },
+                { entity => 'mon.', id => 'b', managed => 1 },
+                { entity => 'mon.', id => 'tiebreaker', managed => 0 },
+            ],
+        },
+    };
+    is(
+        scalar(touched_daemons($stretch, { daemons => [$osd], mon_key => 1 })),
+        4,
+        'a monitor outside the cluster counts as changed as well',
+    );
 }
 
 # --- daemons ceph cannot see ------------------------------------------------------------------
-- 
2.47.3