From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 96AF11FF0B2 for ; Sun, 25 Oct 2026 10:57:42 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id DE0F920BCB; Sun, 25 Oct 2026 10:57:41 +0100 (CET) From: Kefu Chai To: pve-devel@lists.proxmox.com Subject: [PATCH manager v2 1/3] migrations: cephx: collect the run's cluster information in one place Date: Sun, 25 Oct 2026 17:57:28 +0800 Message-ID: <20261025095730.4027539-2-k.chai@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20261025095730.4027539-1-k.chai@proxmox.com> References: <20261025095730.4027539-1-k.chai@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1792922255934 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.278 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: JUD7QETKU2UCW4UNVPR3ICB553DPR5B6 X-Message-ID-Hash: JUD7QETKU2UCW4UNVPR3ICB553DPR5B6 X-MailFrom: k.chai@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 --- 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 914e671d..b0202313 100755 --- a/bin/pve-cephx-rotate-service-keys +++ b/bin/pve-cephx-rotate-service-keys @@ -5858,6 +5858,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; @@ -5927,15 +5947,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. @@ -6052,10 +6069,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) { @@ -6101,20 +6116,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); @@ -6377,6 +6386,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 af429917..b03c2e29 100755 --- a/test/CephKeyMigrationScript_test.pl +++ b/test/CephKeyMigrationScript_test.pl @@ -6629,6 +6629,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