From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 054B61FF09B for ; Mon, 12 Oct 2026 16:38:24 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id F2BF2216BB; Mon, 12 Oct 2026 16:38:13 +0200 (CEST) From: Kefu Chai To: pve-devel@lists.proxmox.com Subject: [PATCH manager 1/3] migrations: cephx: collect the run's cluster information in one place Date: Mon, 12 Oct 2026 22:38:00 +0800 Message-ID: <20261012143802.1193012-2-k.chai@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20261012143802.1193012-1-k.chai@proxmox.com> References: <20261012143802.1193012-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: 1791815888693 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.354 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: TVNXYKRJNIXYX2SCN3PGTIKFQGDY5UG7 X-Message-ID-Hash: TVNXYKRJNIXYX2SCN3PGTIKFQGDY5UG7 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 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