[PATCH manager 1/4] migrations: cephx: ask a monitor about itself through one route

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

The helper asks a monitor three things about itself: which auth methods
it requires, whether it can hold two valid client keys, and which
clients have a session with it. All three go through the admin socket,
which needs no cephx and works even while auth is broken, but does need
a shell on the monitor's host.

A daemon can run on a host that is not a node of this Proxmox VE
cluster, such as the tiebreaker monitor of a stretch cluster. Nothing of
ours reaches such a host: no SSH trust, no pmxcfs, no pvestatd. The
probes fail, so the helper treats the monitor as unreachable, refuses to
stage a client key, and reports an incomplete session view.

Record whether a daemon's node is one we manage, and route the three
probes through that. 'ceph tell' reaches the same admin socket over the
network, but needs cephx and a quorum, so it is the fallback, not the
default. It runs as 'mon.' so it keeps working during a 'client.admin'
rotation, and carries a connect timeout so a monitor across a WAN link
cannot stall a run.

Signed-off-by: Kefu Chai <k.chai@proxmox.com>
---
 PVE/Ceph/KeyMigration.pm            | 19 +++----
 bin/pve-cephx-rotate-service-keys   | 78 ++++++++++++++++-------------
 test/CephKeyMigrationScript_test.pl | 62 +++++++++++++++++++++--
 3 files changed, 110 insertions(+), 49 deletions(-)

diff --git a/PVE/Ceph/KeyMigration.pm b/PVE/Ceph/KeyMigration.pm
index d0613e8b..1ad73826 100644
--- a/PVE/Ceph/KeyMigration.pm
+++ b/PVE/Ceph/KeyMigration.pm
@@ -1101,15 +1101,16 @@ sub merge_configured_daemons($daemons, $type, $configured, $existing = undef) {
             push @$ghosts, { type => $type, id => "$id", node => $configured->{$id} };
             next;
         }
-        push @$daemons,
-            {
-                type => $type,
-                id => "$id",
-                entity => $type eq 'mon' ? 'mon.' : "$type.$id",
-                node => $configured->{$id},
-                down => 1,
-                $type eq 'osd' && $existing ? ('osd-uuid' => $existing->{$id}) : (),
-            };
+        push @$daemons, {
+            type => $type,
+            id => "$id",
+            entity => $type eq 'mon' ? 'mon.' : "$type.$id",
+            node => $configured->{$id},
+            # pvestatd publishes this inventory, so the node is one of this cluster's own
+            managed => 1,
+            down => 1,
+            $type eq 'osd' && $existing ? ('osd-uuid' => $existing->{$id}) : (),
+        };
     }
 
     return wantarray ? ($daemons, $ghosts) : $daemons;
diff --git a/bin/pve-cephx-rotate-service-keys b/bin/pve-cephx-rotate-service-keys
index b60f3619..95d10266 100755
--- a/bin/pve-cephx-rotate-service-keys
+++ b/bin/pve-cephx-rotate-service-keys
@@ -175,11 +175,20 @@ my sub cluster_nodes() {
     return { map { $_ => 1 } PVE::Cluster::get_nodelist()->@* };
 }
 
+# Where a daemon runs decides what this cluster can do with it. 'managed' means its host is a node
+# of this Proxmox VE cluster, so the cluster's SSH trust, pmxcfs and pvestatd cover it: its files,
+# its unit and its installed packages. A daemon without it is still part of the Ceph cluster and
+# still answers over Ceph, the tiebreaker monitor of a stretch cluster for example.
 my sub daemon_location($type, $id, $host) {
-    return { node => undef } if !defined($host) || (!ref($host) && !length($host));
+    return { node => undef, managed => 0 } if !defined($host) || (!ref($host) && !length($host));
     die "Cannot locate '$type.$id': no valid host name was reported\n" if ref($host);
-    my $node = PVE::Ceph::Services::metadata_host_node($host, cluster_nodes());
-    return { node => $node, $node ne $host ? ('metadata-host' => $host) : () };
+    my $nodes = cluster_nodes();
+    my $node = PVE::Ceph::Services::metadata_host_node($host, $nodes);
+    return {
+        node => $node,
+        managed => $nodes->{$node} ? 1 : 0,
+        $node ne $host ? ('metadata-host' => $host) : (),
+    };
 }
 
 my sub assert_member($node, $entity) {
@@ -559,13 +568,14 @@ my sub auth_entry($rados, $entity) {
 }
 
 # the monitor identity keeps working while a client.admin rotation invalidates the default keyring
-my sub monitor_command($args) {
+my sub monitor_command($args, $timeout = undef) {
     return node_run(
         $nodename,
         [
             'ceph', '--cluster', $ccname, '--name', 'mon.', '--keyring', $pve_mon_keyring,
             @$args,
         ],
+        defined($timeout) ? (timeout => $timeout) : (),
     );
 }
 
@@ -579,19 +589,31 @@ my sub monitor_auth_entry($entity) {
     return $res->[0];
 }
 
+# Everything a monitor is asked about itself goes through its own admin socket, which needs no
+# cephx and answers while the authentication layer is in trouble. A monitor on a host outside this
+# cluster has no socket we can reach, and 'ceph tell' hands the command to that same socket over
+# the network, so the answer is identical. That route needs cephx and a quorum, which is why it is
+# the fallback and not the rule. It runs as 'mon.', which keeps answering while a 'client.admin'
+# rotation is in flight.
+my sub mon_admin_run($node, $id, $words, $run_node = undef) {
+    if (defined($node) && cluster_nodes()->{$node}) {
+        my $cmd = ['ceph', 'daemon', "mon.$id", @$words];
+        return $run_node ? $run_node->($node, $cmd) : node_run($node, $cmd, entity => "mon.$id");
+    }
+
+    # a monitor across a WAN link answers in well under this, while one that cannot be reached at
+    # all must not hold the run the way the 300 second default would
+    return monitor_command(['--connect-timeout', '10', 'tell', "mon.$id", @$words], 20);
+}
+
 my sub poll_monitor_authentication($monitors, $nodes, $run_node = undef) {
     $run_node //= sub($node, $command) { node_run($node, $command, entity => $command->[2]) };
     my ($reports, $errors) = ({}, {});
     for my $mon (@$monitors) {
         my $option = 'auth_service_required';
         my $reply = eval {
-            assert_member($nodes->{$mon}, "mon.$mon");
-            decode_json(
-                $run_node->(
-                    $nodes->{$mon},
-                    ['ceph', 'daemon', "mon.$mon", 'config', 'get', $option],
-                ),
-            );
+            decode_json(mon_admin_run(
+                $nodes->{$mon}, $mon, ['config', 'get', $option], $run_node));
         };
         my $error = $@;
         $reports->{$mon}->{$option} = ref($reply) eq 'HASH' ? $reply->{$option} : undef;
@@ -789,20 +811,12 @@ for my $fsid (@fsids) {
 }
 PERL
 
-# Every monitor is asked over its admin socket, which needs no cephx and answers even while the
-# authentication layer itself is in trouble. One that does not answer marks the result incomplete,
-# so the checks built on it stay honest.
+# A monitor that does not answer marks the result incomplete, so the checks built on it stay honest.
 my sub poll_client_sessions($info) {
     my $per_mon = [];
     for my $mon ($info->{daemons}->{mon}->@*) {
         next if $mon->{down};
-        my $sessions = eval {
-            decode_json(node_run(
-                $mon->{node},
-                ['ceph', 'daemon', "mon.$mon->{id}", 'sessions'],
-                entity => "mon.$mon->{id}",
-            ));
-        };
+        my $sessions = eval { decode_json(mon_admin_run($mon->{node}, $mon->{id}, ['sessions'])) };
         push @$per_mon, { mon => $mon->{id}, sessions => $sessions };
     }
     return summarize_sessions($per_mon, $info->{monmap_mons});
@@ -811,15 +825,14 @@ my sub poll_client_sessions($info) {
 # Whether a monitor can keep two client keys valid is told by its admin socket, which needs no
 # cephx. One that answers but does not report the option is old; one that does not answer at all
 # cannot be told apart from a broken connection, and rules staging out just the same.
-my sub probe_manual_promotion($node, $id, $run_node) {
-    my $out =
-        eval { $run_node->($node, ['ceph', 'daemon', "mon.$id", 'config', 'get', $GRACE_OPTION]) };
+my sub probe_manual_promotion($node, $id, $run_node = undef) {
+    my $out = eval { mon_admin_run($node, $id, ['config', 'get', $GRACE_OPTION], $run_node) };
     if (!$@) {
         my $res = eval { decode_json($out) };
         my $value = ref($res) eq 'HASH' ? $res->{$GRACE_OPTION} : undef;
         return { reached => 1, value => defined($value) && !ref($value) ? "$value" : undef };
     }
-    my $reached = eval { $run_node->($node, ['ceph', 'daemon', "mon.$id", 'version']); 1 } ? 1 : 0;
+    my $reached = eval { mon_admin_run($node, $id, ['version'], $run_node); 1 } ? 1 : 0;
     return { reached => $reached, value => undef };
 }
 
@@ -827,11 +840,7 @@ my sub poll_manual_promotion($info) {
     my $reports = {};
     for my $mon ($info->{daemons}->{mon}->@*) {
         next if $mon->{down};
-        $reports->{ $mon->{id} } = probe_manual_promotion(
-            $mon->{node},
-            $mon->{id},
-            sub($node, $cmd) { node_run($node, $cmd, entity => "mon.$mon->{id}") },
-        );
+        $reports->{ $mon->{id} } = probe_manual_promotion($mon->{node}, $mon->{id});
     }
     return manual_promotion_support($reports, $info->{monmap_mons});
 }
@@ -936,13 +945,9 @@ my sub collect_current_monitor_state($rados, $run_node = undef) {
             $errors->{$id} = 'not in quorum';
         } elsif (!$nodes->{$id}) {
             $errors->{$id} = 'no hostname in monitor metadata';
-        } elsif (!eval { assert_member($nodes->{$id}, "mon.$id"); 1 }) {
-            $errors->{$id} = $@;
         } else {
-            $sessions = eval {
-                decode_json($run_node->(
-                    $nodes->{$id}, ['ceph', 'daemon', "mon.$id", 'sessions']));
-            };
+            $sessions =
+                eval { decode_json(mon_admin_run($nodes->{$id}, $id, ['sessions'], $run_node)) };
             my $error = $@;
             if ($error) {
                 $error =~ s/\s+/ /g;
@@ -6258,6 +6263,7 @@ sub key_migration_test_hooks {
         client_key_files => \&client_key_files,
         check_client_kernels => \&check_client_kernels,
         manual_promotion_with_retries => \&manual_promotion_with_retries,
+        mon_admin_run => \&mon_admin_run,
         describe_live => \&describe_live,
     };
 }
diff --git a/test/CephKeyMigrationScript_test.pl b/test/CephKeyMigrationScript_test.pl
index a53b1430..fa1cee4e 100755
--- a/test/CephKeyMigrationScript_test.pl
+++ b/test/CephKeyMigrationScript_test.pl
@@ -6595,7 +6595,16 @@ for my $case ([0, 0], [1, 0], [0, 1], [1, 1]) {
     $rados->{host} = 'foreign.invalid';
     @commands = ();
     $info = $HOOKS->{collect_cluster_info}->($rados, { 'rotate-lockbox-keys' => 1 }, {});
-    is_deeply(\@commands, [], 'non-member gets no commands during pre-preflight collection');
+    is(
+        scalar(grep { /foreign\.invalid/ } map { join(' ', @$_) } @commands),
+        0,
+        'pre-preflight collection sends nothing to the non-member host',
+    );
+    is(
+        scalar(grep { /tell mon\.a config get/ } map { join(' ', @$_) } @commands),
+        1,
+        'the monitor outside the cluster is asked with tell instead of its socket',
+    );
     like(
         $info->{lockbox}->{'client.osd-lockbox.osd-uuid'}->{missing},
         qr/osd\.7.*foreign\.invalid.*not a node/,
@@ -6604,10 +6613,14 @@ for my $case ([0, 0], [1, 0], [0, 1], [1, 1]) {
     @targets = ();
     $monitor = $HOOKS->{collect_monitor_state}->($rados, sub { push @targets, $_[0]; return '[]' });
     is_deeply(\@targets, [], 'fresh monitor collection never queries a non-member');
-    like(
+    is(
         $monitor->{sessions}->{errors}->{a},
-        qr/mon\.a.*not a node/,
-        'monitor diagnostic names membership',
+        undef,
+        'and its sessions come over the network, so nothing is left unanswered',
+    );
+    ok(
+        $monitor->{manual_promotion}->{supported},
+        'so a client key can still be staged with a monitor outside the cluster',
     );
 
     for my $version (1, 2) {
@@ -7837,6 +7850,47 @@ for my $case ([0, 0], [1, 0], [0, 1], [1, 1]) {
     );
 }
 
+# Whatever a monitor is asked about itself, the route depends on whether this cluster has a shell
+# on its host. 'ceph tell' reaches the same admin socket over the network for one that it does not.
+{
+    no warnings qw(once redefine);
+    local *PVE::SSHInfo::get_ssh_info = sub { return { node => $_[0] } };
+    local *PVE::SSHInfo::ssh_info_to_command = sub { return ['ssh', $_[0]->{node}, '--'] };
+
+    my @commands;
+    local *main::run_command = sub {
+        my ($cmd, %args) = @_;
+        push @commands, join(' ', @$cmd);
+        $args{outfunc}->('answer');
+    };
+
+    my $run = sub {
+        @commands = ();
+        return $HOOKS->{mon_admin_run}->(@_);
+    };
+
+    is($run->('node-a', 'a', ['sessions']), "answer\n", 'a member answers over its admin socket');
+    is($commands[0], 'ssh node-a -- ceph daemon mon.a sessions', 'which is asked through ssh');
+
+    is($run->('witness.invalid', 'tiebreaker', ['sessions']), "answer\n", 'so does an outsider');
+    like(
+        $commands[0],
+        qr/^ceph --cluster \S+ --name mon\. --keyring \S+ --connect-timeout 10 tell mon\.tiebreaker sessions$/,
+        'asked locally with tell, as mon., and with a connect timeout instead of a long default',
+    );
+
+    $run->(undef, 'nameless', ['version']);
+    like($commands[0], qr/tell mon\.nameless version/, 'a monitor without a known host too');
+
+    my @routed;
+    $run->('node-a', 'a', ['config', 'get', 'opt'], sub { push @routed, $_[1]; return '{}' });
+    is_deeply(
+        \@routed,
+        [['ceph', 'daemon', 'mon.a', 'config', 'get', 'opt']],
+        'a caller-supplied runner keeps the member route, which the tests rely on',
+    );
+}
+
 {
     my $out = '';
     {
-- 
2.47.3