[PATCH manager 1/3] migrations: cephx: collect the run's cluster information in one place

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

A run collects the cluster information again after anything that
changes the auth database: resuming a journalled lockbox key rotation,
aborting a staged client key, or retiring an aborted one. Each place
did it by hand, and they drifted apart. After the lockbox resume, the
RADOS handle and the monitor key were not set again, so the node
preflight called daemon_is_running() on an undefined handle. A later
collection overwrote the preferred cipher, read on its own, with the
monitor map's reading, which names an unsupported value 'unknown'. A
client key change dropped the daemons recover_left_behind() had
restored from the saved plan, so the plan left out a daemon an earlier
run had stopped.

Collect it in one helper that every place uses, which sets these fields
and keeps the restored daemons.

Signed-off-by: Kefu Chai <k.chai@proxmox.com>
---
 bin/pve-cephx-rotate-service-keys   | 40 ++++++++++++++++++-----------
 test/CephKeyMigrationScript_test.pl | 14 ++++++++++
 2 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/bin/pve-cephx-rotate-service-keys b/bin/pve-cephx-rotate-service-keys
index ad425f96..1cdb2a45 100755
--- a/bin/pve-cephx-rotate-service-keys
+++ b/bin/pve-cephx-rotate-service-keys
@@ -5687,6 +5687,26 @@ my sub staging_progress($plan, $state) {
         . ".\nKeep the journal and check unfinished key copies before resuming.";
 }
 
+# the cluster information a run plans with, collected again after each change to the auth database;
+# Ceph still does not list the daemons recover_left_behind() restored, so keep them
+my sub collect_run_info($rados, $opts, $state, $previous = undef) {
+    my $info = collect_cluster_info($rados, $opts, $state);
+    $info->{rados} = $rados;
+    $info->{mon_entry} = eval { auth_entry($rados, 'mon.') } // {};
+    $info->{pve_mon_key} = pve_mon_keyring_key();
+    # read on its own, as the monitor map names an unsupported value 'unknown'
+    $info->{preferred_cipher} = current_preferred_cipher($rados);
+
+    for my $type (keys %{ ($previous // {})->{daemons} // {} }) {
+        my $listed = { map { $_->{entity} => 1 } @{ $info->{daemons}->{$type} // [] } };
+        push $info->{daemons}->{$type}->@*,
+            grep { $_->{recovered} && !$listed->{ $_->{entity} } }
+            $previous->{daemons}->{$type}->@*;
+    }
+
+    return $info;
+}
+
 my sub run_migration($opts, $confirm = \&confirm_apply_run) {
     local $storage_copy_warnings_seen = {};
     $monmap_changed = 0;
@@ -5756,15 +5776,12 @@ my sub run_migration($opts, $confirm = \&confirm_apply_run) {
         return 1;
     }
 
-    $info = collect_cluster_info($rados, $opts, $state);
+    $info = collect_run_info($rados, $opts, $state);
     check_client_authentication($info, $opts, $state, $client_files, $auth_cache);
     # Before any recovery of earlier runs: a selection that would have to stage new keys while a
     # monitor cannot keep two client keys valid stops here. The check repeats once the staged
     # records are settled, as settling can leave a key that needs staging anew.
     assert_client_staging_supported($info, $opts, $state, client_key_files());
-    $info->{rados} = $rados;
-    $info->{mon_entry} = eval { auth_entry($rados, 'mon.') } // {};
-    $info->{pve_mon_key} = pve_mon_keyring_key();
 
     if ($opts->{apply}) {
         # Remove state fields written by older versions but never consumed.
@@ -5881,10 +5898,8 @@ my sub run_migration($opts, $confirm = \&confirm_apply_run) {
         );
     }
 
-    $info->{preferred_cipher} = current_preferred_cipher($rados);
-
     # Resume mutates the auth database and tags, so anything collected before it is stale.
-    $info = collect_cluster_info($rados, $opts, $state) if $lockbox_changed;
+    $info = collect_run_info($rados, $opts, $state) if $lockbox_changed;
 
     my $recovered = recover_left_behind($info, $state);
     for my $daemon (@$recovered) {
@@ -5930,20 +5945,14 @@ my sub run_migration($opts, $confirm = \&confirm_apply_run) {
             );
         }
         # the managed copies and mount-refresh state changed, so recollect before planning
-        $info = collect_cluster_info($rados, $opts, $state);
-        $info->{rados} = $rados;
-        $info->{mon_entry} = eval { auth_entry($rados, 'mon.') } // {};
-        $info->{pve_mon_key} = pve_mon_keyring_key();
+        $info = collect_run_info($rados, $opts, $state, $info);
     }
 
     if (my @confirmed = @{ $opts->{'confirm-abort-clients-refreshed'} // [] }) {
         retire_aborted_keys($rados, $state, \@confirmed);
         release_manual_promotion($rados, $state);
         # pending keys were retired, so every auth-derived input is stale
-        $info = collect_cluster_info($rados, $opts, $state);
-        $info->{rados} = $rados;
-        $info->{mon_entry} = eval { auth_entry($rados, 'mon.') } // {};
-        $info->{pve_mon_key} = pve_mon_keyring_key();
+        $info = collect_run_info($rados, $opts, $state, $info);
     }
 
     my $plan = build_plan($info, $state, $opts, $client_files);
@@ -6209,6 +6218,7 @@ sub key_migration_test_hooks {
         migrate_daemon => \&migrate_daemon,
         recover_left_behind => \&recover_left_behind,
         collect_cluster_info => \&collect_cluster_info,
+        collect_run_info => \&collect_run_info,
         collect_monitor_state => \&collect_current_monitor_state,
         restriction_snapshot => \&collect_restriction_snapshot,
         summary_snapshot => \&collect_summary_snapshot,
diff --git a/test/CephKeyMigrationScript_test.pl b/test/CephKeyMigrationScript_test.pl
index ef13d8dd..1392b514 100755
--- a/test/CephKeyMigrationScript_test.pl
+++ b/test/CephKeyMigrationScript_test.pl
@@ -6535,6 +6535,20 @@ for my $case ([0, 0], [1, 0], [0, 1], [1, 1]) {
     };
     my $rados = HostInventoryRados->new();
     my $info = $HOOKS->{collect_cluster_info}->($rados, {}, {});
+
+    # collected again after a client key change, the daemons restored from the saved plan stay
+    my $recovered =
+        { type => 'mgr', id => 'b', entity => 'mgr.b', node => 'node-a', recovered => 1 };
+    my $again = $HOOKS->{collect_run_info}->(
+        $rados, {}, {}, { daemons => { mgr => [$info->{daemons}->{mgr}->@*, $recovered] } },
+    );
+    is_deeply(
+        [map { $_->{entity} } $again->{daemons}->{mgr}->@*],
+        ['mgr.a', 'mgr.b'],
+        'collecting again keeps a daemon restored from the saved plan',
+    );
+    is($again->{rados}, $rados, 'and the RADOS handle the plan needs');
+
     my @daemons = map { $info->{daemons}->{$_}->@* } qw(mon mgr mds osd);
     is_deeply([map { $_->{node} } @daemons], [('node-a') x 4], 'all collectors map daemon FQDNs');
     my $plan = { daemons => [grep { $_->{type} ne 'mon' } @daemons], mon_key => 1 };
-- 
2.47.3