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 3DB4C1FF09B for ; Mon, 12 Oct 2026 16:38:31 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 176632168F; Mon, 12 Oct 2026 16:38:29 +0200 (CEST) From: Kefu Chai To: pve-devel@lists.proxmox.com Subject: [PATCH manager 3/3] migrations: cephx: rotate daemons whose key on disk differs Date: Mon, 12 Oct 2026 22:38:02 +0800 Message-ID: <20261012143802.1193012-4-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: 1791815894278 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.658 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) KAM_MAILER 2 Automated Mailer Tag Left in Email 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: RXU7JFGJHWGWBXS2SC3E6K3SWD2VLO3X X-Message-ID-Hash: RXU7JFGJHWGWBXS2SC3E6K3SWD2VLO3X 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 daemon whose key on disk differs from the monitors' is now only reported, and fixing it is left to the admin, although the helper already knows how to bring every copy of a daemon key in line. Plan such a daemon like one still on the old cipher, so migrating it writes a key the monitors accept to every copy. A stopped daemon stays stopped as before, and as it has often stopped because of that very key, the plan and the run now name the command that starts it, with or without '--restart-daemons'. A daemon whose key an earlier run rotated can now reach the branch that reuses that key after the key was changed by hand, so its message no longer claims that the earlier run produced the current key. Signed-off-by: Kefu Chai --- PVE/Ceph/KeyMigration.pm | 5 +++ bin/pve-cephx-rotate-service-keys | 25 ++++++----- test/CephKeyMigrationScript_test.pl | 68 ++++++++++++++++++++++++----- test/CephKeyMigration_test.pl | 21 +++++++++ 4 files changed, 99 insertions(+), 20 deletions(-) diff --git a/PVE/Ceph/KeyMigration.pm b/PVE/Ceph/KeyMigration.pm index 1abf1eee..ced9da94 100644 --- a/PVE/Ceph/KeyMigration.pm +++ b/PVE/Ceph/KeyMigration.pm @@ -1810,6 +1810,11 @@ sub build_plan($info, $state, $opts, $files) { for my $daemon (@$daemons) { next if !daemon_in_scope($only, $type, $daemon->{entity}); + # a key changed by hand reads as migrated in the auth database + if (($info->{stale_keys} // {})->{ $daemon->{entity} }) { + push $plan->{daemons}->@*, $daemon; + next; + } my $unfinished = migration_unfinished($state, $daemon->{entity}); # Not the marker alone: a reused OSD id gets a fresh key that needs migrating like any # other. diff --git a/bin/pve-cephx-rotate-service-keys b/bin/pve-cephx-rotate-service-keys index 647251cf..d9078819 100755 --- a/bin/pve-cephx-rotate-service-keys +++ b/bin/pve-cephx-rotate-service-keys @@ -1703,7 +1703,6 @@ my sub check_disk_keys($info, $state, $opts) { my $verdict = disk_key_verdict($info, $daemon, $probe); if ($verdict->{verdict} eq 'stale') { $info->{stale_keys}->{ $daemon->{entity} } = $verdict->{copies}; - push @$disk_key_doubts, $daemon->{entity}; } elsif ($verdict->{verdict} eq 'unknown') { chomp(my $text = describe_disk_key_doubt({ %$daemon, %$probe }, $verdict->{why})); push @doubts, $text; @@ -2498,6 +2497,7 @@ my sub preflight_cluster( && !restrict_wanted($info, $opts) && !$recovered_count && !@unfinished + && !%{ $info->{stale_keys} // {} } ) { log_verdict("Every service key uses '$CIPHER', and so do the service tickets."); mon_key_hint($info, $opts); @@ -3025,16 +3025,18 @@ my sub print_plan($info, $plan, $state, $opts, $storage_entities) { . " with the same checks."); log_step("A standby manager cannot take a key while running and is restarted.") if grep { $_->{type} eq 'mgr' } $plan->{daemons}->@*; - my @down = map { $_->{entity} } grep { $_->{down} } $plan->{daemons}->@*; - if (@down) { - log_text(" Not running right now, so the key is written to disk and the daemon" - . " left stopped: " - . join(', ', @down) - . "."); - } log_step("'noout' is set on this run's OSDs, as that stop can happen at any point.") if grep { $_->{type} eq 'osd' } $plan->{daemons}->@*; } + my @down = grep { $_->{down} } $plan->{daemons}->@*; + if (@down) { + log_text(" Not running right now, so the key is written to disk and the daemon" + . " left stopped: " + . join(', ', map { $_->{entity} } @down) + . ". Start one that should run afterwards, on its node:"); + log_steps( + [map { "systemctl start ceph-$_->{type}\@$_->{id} (node '$_->{node}')" } @down]); + } if ($opts->{verbose}) { log_step("in this order:"); log_steps([map { "$_->{entity} on $_->{node}" } $plan->{daemons}->@*]); @@ -3273,7 +3275,8 @@ my sub rotate_entity($rados, $state, $entity) { # trust the marker only as far as the key backs it: one changed since would read as migrated if ($state->{rotated}->{$entity} && (key_cipher($before->{key}) // -1) == $CIPHER_ID) { - log_info("the key of '$entity' was already rotated by an earlier run, reusing it"); + log_info("an earlier run rotated '$entity' and its key uses the '$CIPHER' cipher now," + . " reusing that key"); return $before; } if ($state->{rotated}->{$entity}) { @@ -4778,7 +4781,7 @@ my sub migrate_daemon($rados, $state, $daemon, $opts) { # left as found: whoever stopped it did not ask for it back if ($daemon->{down} && !$up) { log_pass("'$entity' uses the '$CIPHER' cipher and stays stopped, as it was before this" - . " run"); + . " run. If it should run, start it on node '$node' with 'systemctl start $unit'."); $state->{done}->{$entity} = time(); delete $state->{live_swap}->{$entity}; save_state($state); @@ -5817,6 +5820,8 @@ my sub collect_run_info($rados, $opts, $state, $previous = undef) { # read on its own, as the monitor map names an unsupported value 'unknown' $info->{preferred_cipher} = current_preferred_cipher($rados); + # what the daemons hold on disk does not change with the auth database + $info->{stale_keys} = $previous->{stale_keys} if $previous; for my $type (keys %{ ($previous // {})->{daemons} // {} }) { my $listed = { map { $_->{entity} => 1 } @{ $info->{daemons}->{$type} // [] } }; push $info->{daemons}->{$type}->@*, diff --git a/test/CephKeyMigrationScript_test.pl b/test/CephKeyMigrationScript_test.pl index 2cfc75f0..adb9b77b 100755 --- a/test/CephKeyMigrationScript_test.pl +++ b/test/CephKeyMigrationScript_test.pl @@ -4399,6 +4399,29 @@ sub run_aggregate_confirmation { kernel => 0, }; + # a daemon Ceph does not list is left stopped, and the plan says how to start it + { + my $plan = { + mon_key => 0, + daemons => + [{ type => 'mds', id => 'a', entity => 'mds.a', node => 'pve1', down => 1 }], + client_keys => [], + lockbox_keys => [], + service_cipher => 0, + }; + for my $restart (0, 1) { + my $output = ''; + open(my $stdout, '>', \$output) or die $!; + local *STDOUT = $stdout; + $HOOKS->{print_plan}->($info, $plan, {}, { 'restart-daemons' => $restart }, $stores); + like( + $output, + qr/left stopped: mds\.a\. Start one that should run afterwards, on its node:\n systemctl start ceph-mds\@a \(node 'pve1'\)$/m, + "a daemon left stopped comes with the command to start it (restart: $restart)", + ); + } + } + my $mixed = $render->( [$tool, $resume], { 'rotate-all-storage-keys' => 1, 'rotate-client-keys' => 1 }, ); @@ -8156,7 +8179,13 @@ for my $case ([0, 0], [1, 0], [0, 1], [1, 1]) { } # the real orchestration runs on a migrated-looking cluster whose daemon kept an old key on disk -for my $case (['dry run', $OLD], ['dry run', $NEW], ['unreachable', $NEW]) { +for my $case ( + ['dry run', $OLD], + ['dry run', $NEW], + ['apply', $OLD], + ['apply restart', $OLD], + ['unreachable', $NEW], +) { my ($mode, $on_disk) = @$case; no warnings qw(once redefine); my $dir = tempdir(CLEANUP => 1); @@ -8280,7 +8309,12 @@ for my $case (['dry run', $OLD], ['dry run', $NEW], ['unreachable', $NEW]) { $error = $@; } is($error, '', "the $mode itself does not die"); - is(scalar(@probes), 1, 'the keyring of the metadata server is inspected once'); + # once for the check, and again for the node preflight of a plan + is( + scalar(@probes), + $case->[1] eq $OLD ? 2 : 1, + 'the keyring of the metadata server is inspected for each', + ); if ($mode eq 'unreachable') { is($result, 0, 'a cluster whose daemon could not be checked still reads as migrated'); like( @@ -8290,22 +8324,36 @@ for my $case (['dry run', $OLD], ['dry run', $NEW], ['unreachable', $NEW]) { ); unlike($out, qr/^PASS:/m, 'and passes nothing'); } elsif ($mode eq 'dry run' && $on_disk eq $OLD) { - is($result, 0, 'a daemon holding another key than the monitors does not stop the run'); - like( - $out, - qr/^WARN: These keep a key on disk that the monitors do not accept, as after a 'ceph auth rotate', and would fail to authenticate once they read it: mds\.a \(keyring\)\.$/m, - 'but is reported, naming the copy that differs', - ); + is($result, 0, 'a daemon holding another key than the monitors is planned'); like( $out, - qr/^WARN: Every service key uses 'aes256k', and so do the service tickets\. But the key on disk could not be confirmed for mds\.a/m, - 'and the verdict names it', + qr/^WARN: These keep a key on disk that the monitors do not accept, as after a 'ceph auth rotate', and would fail to authenticate once they read it: mds\.a \(keyring\)\.\n(?s:.*)^Step 1: rotate 1 metadata server key/m, + 'with the warning saying why, and which copy differs', ); unlike($out, qr/^PASS: Every service key/m, 'without reporting the cluster as migrated'); is_deeply([@written, @told, @units, @auth], [], 'and a dry run changes nothing'); } elsif ($mode eq 'dry run') { is($result, 0, 'with the key on disk matching, the migrated cluster passes'); like($out, qr/^PASS: Every service key uses 'aes256k'/m, 'as before'); + } elsif ($mode eq 'apply') { + is($result, 0, 'the apply run fixes the daemon'); + is_deeply( + \@auth, + ['get-or-create-pending mds.a', 'commit-pending mds.a'], + 'with the live swap of any rotation', + ); + is_deeply(\@written, [$fresh], 'which writes the new key to its keyring'); + is_deeply(\@told, ['tell mds.a rotate-key -i -'], 'and hands it to the running daemon'); + is_deeply(\@units, [], 'without a restart'); + } else { + is($result, 0, "'--restart-daemons' fixes it too"); + is_deeply(\@auth, [], 'without rotating the key, which already uses the new cipher'); + is_deeply(\@written, [$NEW], 'by writing the key the monitors accept'); + is_deeply( + \@units, + ['stop ceph-mds@a', 'reset-failed ceph-mds@a', 'start ceph-mds@a'], + 'and restarting the daemon', + ); } } diff --git a/test/CephKeyMigration_test.pl b/test/CephKeyMigration_test.pl index c0d9384a..a9ba6d71 100755 --- a/test/CephKeyMigration_test.pl +++ b/test/CephKeyMigration_test.pl @@ -1666,6 +1666,27 @@ my sub cluster { 'a scope can name a single daemon, which is how one left behind is repaired', ); + # a key changed in the auth database alone reads as migrated + my $stale = cluster(); + $stale->{exported}->{'osd.1'} = { key => $NEW }; + my $state = { done => { 'mgr.a' => 1, 'osd.1' => 1 } }; + is_deeply(build_plan($stale, $state, {}, {})->{daemons}, [], + 'a migrated cluster plans nothing'); + $stale->{stale_keys} = { 'mgr.a' => ['keyring'] }; + is_deeply( + [map { $_->{entity} } build_plan($stale, $state, {}, {})->{daemons}->@*], + ['mgr.a'], + 'unless a daemon keeps a key on disk that the monitors do not accept', + ); + is_deeply( + [ + map { $_->{entity} } + build_plan($stale, $state, { only => { osd => 1 } }, {})->{daemons}->@* + ], + [], + "but not outside the '--only' scope", + ); + my $switched = cluster(); $switched->{service_cipher} = $CIPHER; is( -- 2.47.3