* [PATCH manager v2 0/3] migrations: cephx: catch daemons whose key on disk is stale
@ 2026-10-25 9:57 Kefu Chai
2026-10-25 9:57 ` [PATCH manager v2 1/3] migrations: cephx: collect the run's cluster information in one place Kefu Chai
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Kefu Chai @ 2026-10-25 9:57 UTC (permalink / raw)
To: pve-devel
Hi,
after upgrading to Ceph 20, a user rotated their MDS keys by hand with
'ceph auth rotate --key-type=aes256k' and then ran the migration
helper. It saw aes256k in the auth database and happily said PASS, but
the keyrings on disk still had the old keys. So the MDS daemons
couldn't authenticate at their next restart, and CephFS went down.
Since v1 we've also seen this with OSDs, and there fixing the keyring
isn't even enough, because ceph-volume rebuilds it from the bluestore
label at every boot.
With this series the helper also looks at the keys the daemons will
actually read next time, i.e. the keyrings and the OSD labels. Any
daemon with a stale copy gets planned for rotation, which rewrites all
its copies, and if a copy can't be checked, the helper won't call the
cluster migrated. The cost is one more SSH session per node per run.
Patch 1 is a cleanup that fixes a few older bugs on the way, so feel
free to pick it up on its own.
I gave it a spin on a three node cluster with Ceph 20.2.4, with a stale
OSD label: with the OSD running, with '--restart-daemons', and with it
down at its start limit. Each time the OSD came back fine after a
'ceph-volume lvm activate', which is what a reboot does.
One thing I'm unsure about: a dry run with a stale key still prints
"Preflight checks passed for the displayed plan". If that reads too
much like a PASS to you, I'm happy to change it.
Changes since v1:
- rebased; only daemons on cluster nodes are probed now, the rest are
listed as not compared
- dropped the special case for a staged pending key on disk
- stale daemons are counted in needs_rotation()
- the start hint for a stopped daemon runs 'systemctl reset-failed'
first
- the "already on the new cipher" message no longer credits an earlier
run
- reworked the tests to match the existing ones
v1: https://lore.proxmox.com/pve-devel/20261012143802.1193012-1-k.chai@proxmox.com/
Thanks for having a look!
Kefu Chai (3):
migrations: cephx: collect the run's cluster information in one place
migrations: cephx: report daemons whose key on disk differs
migrations: cephx: rotate daemons whose key on disk differs
PVE/Ceph/KeyMigration.pm | 60 +++++-
bin/pve-cephx-rotate-service-keys | 249 +++++++++++++++++++------
test/CephKeyMigrationScript_test.pl | 278 ++++++++++++++++++++++++++++
test/CephKeyMigration_test.pl | 91 +++++++++
4 files changed, 619 insertions(+), 59 deletions(-)
--
2.47.3
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH manager v2 1/3] migrations: cephx: collect the run's cluster information in one place
2026-10-25 9:57 [PATCH manager v2 0/3] migrations: cephx: catch daemons whose key on disk is stale Kefu Chai
@ 2026-10-25 9:57 ` Kefu Chai
2026-10-25 9:57 ` [PATCH manager v2 2/3] migrations: cephx: report daemons whose key on disk differs Kefu Chai
2026-10-25 9:57 ` [PATCH manager v2 3/3] migrations: cephx: rotate " Kefu Chai
2 siblings, 0 replies; 4+ messages in thread
From: Kefu Chai @ 2026-10-25 9:57 UTC (permalink / raw)
To: pve-devel
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 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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH manager v2 2/3] migrations: cephx: report daemons whose key on disk differs
2026-10-25 9:57 [PATCH manager v2 0/3] migrations: cephx: catch daemons whose key on disk is stale Kefu Chai
2026-10-25 9:57 ` [PATCH manager v2 1/3] migrations: cephx: collect the run's cluster information in one place Kefu Chai
@ 2026-10-25 9:57 ` Kefu Chai
2026-10-25 9:57 ` [PATCH manager v2 3/3] migrations: cephx: rotate " Kefu Chai
2 siblings, 0 replies; 4+ messages in thread
From: Kefu Chai @ 2026-10-25 9:57 UTC (permalink / raw)
To: pve-devel
The helper judged a daemon migrated by the cipher of its key in the
auth database alone. After a 'ceph auth rotate' by hand, the dry run
reported PASS, while the daemon kept the old key on disk and failed
with handle_auth_bad_method at its next restart. A user lost CephFS
this way when all its MDS daemons restarted.
Before any verdict, have the node probe report a fingerprint of the
key each manager, metadata server and OSD in scope reads at its next
start: the one in its keyring and, for an OSD, also the one in its
bluestore label. Warn about every daemon whose copy differs from the
key the monitors hold, naming the copy. List a daemon whose copy cannot
be compared, for example one on an unreachable node, and in both cases
turn the verdicts that report the cluster as migrated into a warning
instead of a PASS.
This probes every daemon in scope on every run, dry runs included, with
one SSH session per node and one 'ceph-bluestore-tool show-label' per
OSD, where only the daemons of a plan were probed before.
Signed-off-by: Kefu Chai <k.chai@proxmox.com>
---
PVE/Ceph/KeyMigration.pm | 57 ++++++-
bin/pve-cephx-rotate-service-keys | 172 ++++++++++++++++++---
test/CephKeyMigrationScript_test.pl | 227 ++++++++++++++++++++++++++++
test/CephKeyMigration_test.pl | 75 +++++++++
4 files changed, 502 insertions(+), 29 deletions(-)
diff --git a/PVE/Ceph/KeyMigration.pm b/PVE/Ceph/KeyMigration.pm
index c24c1c3b..0cf2fc0e 100644
--- a/PVE/Ceph/KeyMigration.pm
+++ b/PVE/Ceph/KeyMigration.pm
@@ -21,7 +21,8 @@ our @EXPORT_OK = qw(
key_cipher key_fingerprint keyring_text short_version version_has_cipher
parse_storage_key storage_config_problem storage_key_copy_problems storage_key_copy_warnings
parse_auth_methods summarize_monitor_authentication
- parse_probe_output osd_label_identity needs_rotation mon_key_needs_rotation mon_keyring_stale
+ parse_probe_output osd_label_identity daemon_in_scope disk_key_candidates disk_key_verdict
+ needs_rotation mon_key_needs_rotation mon_keyring_stale
mon_key_rotation_wanted client_keys_requested migration_unfinished unfinished_entities
bulk_storage_staging_needed client_staging_needed touched_daemons
plan_client_keys plan_lockbox_keys build_plan configured_daemon_locations
@@ -262,7 +263,7 @@ sub version_has_cipher($version) {
sub parse_probe_output($output) {
my $probes = {};
for my $line (split(/\n/, $output)) {
- next if $line !~ m/^(label|keyring|store|error) (\S+)(?: (.*))?$/;
+ next if $line !~ m/^(label|keyring|keyfp|store|error) (\S+)(?: (.*))?$/;
my ($kind, $spec, $payload) = ($1, $2, $3 // '');
my $probe = $probes->{$spec} //= {};
@@ -279,6 +280,9 @@ sub parse_probe_output($output) {
} elsif ($kind eq 'keyring') {
$probe->{store} = 'file';
$probe->{sections} = [$payload =~ m/\[([^\]]+)\]/g];
+ } elsif ($kind eq 'keyfp') {
+ # empty for a keyring without a key under the daemon's own section
+ $probe->{'keyring-fingerprint'} = $payload;
} elsif ($kind eq 'store') {
$probe->{store} = $payload;
} elsif ($kind eq 'label') {
@@ -326,6 +330,51 @@ sub osd_label_identity($daemon, $cluster_fsid) {
return 'ok';
}
+# '--only' names daemon types or single daemons
+sub daemon_in_scope($only, $type, $entity) {
+ return 1 if !$only || $only->{$type};
+ return $only->{$entity} ? 1 : 0;
+}
+
+# daemons in scope whose key on disk is compared; the plan rewrites the keys of resumed ones anyway
+sub disk_key_candidates($info, $state, $only = undef) {
+ return grep {
+ daemon_in_scope($only, $_->{type}, $_->{entity})
+ && !$_->{recovered}
+ && !migration_unfinished($state, $_->{entity})
+ } map {
+ $info->{daemons}->{$_}->@*
+ } @$DAEMON_TYPES;
+}
+
+# Compare the key a daemon reads at its next start with the monitors'. An OSD reads its keyring at a
+# restart, which ceph-volume rebuilds from the bluestore label at activation, so both count.
+sub disk_key_verdict($info, $daemon, $probe) {
+ my $active = ($info->{exported}->{ $daemon->{entity} } // {})->{key};
+ return { verdict => 'match' } if !length($active // '');
+
+ my $store = ($probe // {})->{store} // 'missing';
+ my $copies;
+ if ($store eq 'file') {
+ $copies = { keyring => $probe->{'keyring-fingerprint'} };
+ } elsif ($store eq 'block') {
+ return { verdict => 'unknown', why => 'identity' }
+ if osd_label_identity({ %$daemon, %$probe }, $info->{fsid}) ne 'ok';
+ $copies = { label => key_fingerprint($probe->{'label-key'}) };
+ # only an activated OSD has a keyring
+ $copies->{keyring} = $probe->{'keyring-fingerprint'}
+ if exists($probe->{'keyring-fingerprint'});
+ } else {
+ return { verdict => 'unknown', why => $store };
+ }
+
+ my $want = key_fingerprint($active);
+ my @wrong = sort grep { ($copies->{$_} // '') ne $want } keys %$copies;
+ return { verdict => 'match' } if !@wrong;
+
+ return { verdict => 'stale', copies => \@wrong };
+}
+
sub needs_rotation($info, $entity) {
my $entry = $info->{exported}->{$entity};
@@ -1750,8 +1799,6 @@ sub build_plan($info, $state, $opts, $files) {
}
for my $type (@$DAEMON_TYPES) {
- next if $only && !$only->{$type} && !grep { m/^\Q$type\E\./ } keys %$only;
-
# Restarting a standby costs no failover, so the active manager goes last: a fallback
# restart there fails over onto one already migrated.
my $daemons = [$info->{daemons}->{$type}->@*];
@@ -1759,7 +1806,7 @@ sub build_plan($info, $state, $opts, $files) {
if $type eq 'mgr';
for my $daemon (@$daemons) {
- next if $only && !$only->{$type} && !$only->{ $daemon->{entity} };
+ next if !daemon_in_scope($only, $type, $daemon->{entity});
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 b0202313..7b6bd536 100755
--- a/bin/pve-cephx-rotate-service-keys
+++ b/bin/pve-cephx-rotate-service-keys
@@ -31,7 +31,8 @@ use PVE::Ceph::KeyMigration qw(
key_cipher key_fingerprint keyring_text short_version version_has_cipher
parse_storage_key storage_config_problem storage_key_copy_problems storage_key_copy_warnings
parse_auth_methods summarize_monitor_authentication
- parse_probe_output osd_label_identity needs_rotation mon_key_needs_rotation mon_keyring_stale
+ parse_probe_output osd_label_identity disk_key_candidates disk_key_verdict
+ needs_rotation mon_key_needs_rotation mon_keyring_stale
mon_key_rotation_wanted client_keys_requested migration_unfinished unfinished_entities
client_staging_needed touched_daemons
plan_client_keys plan_lockbox_keys build_plan configured_daemon_locations
@@ -471,6 +472,7 @@ my $PROBE_SCRIPT = <<'PERL_EOF';
use strict;
use warnings;
+use Digest::SHA qw(sha256_hex);
use JSON;
use File::Temp;
use POSIX ();
@@ -514,10 +516,27 @@ sub command_output {
return ($out, undef);
}
+# the sections of a keyring, and the fingerprint of the key under the daemon's own section
+sub read_keyring {
+ my ($path, $entity) = @_;
+ open(my $fh, '<', $path) or return;
+ my ($sections, $section, $key) = ('', '', undef);
+ for my $line (<$fh>) {
+ if ($line =~ m/^(\[([^\]]*)\])/) {
+ $sections .= $1;
+ $section = $2;
+ } elsif ($section eq $entity && $line =~ m/^\s*key\s*=\s*(\S+)/) {
+ $key //= $1;
+ }
+ }
+ return ($sections, defined($key) ? substr(sha256_hex($key), 0, 16) : '');
+}
+
my ($cluster, @specs) = @ARGV;
for my $spec (@specs) {
my ($type, $id) = split(/:/, $spec, 2);
my $dir = "/var/lib/ceph/$type/$cluster-$id";
+ my $entity = "$type.$id";
if (-e "$dir/block") {
my ($label, $err, $details) =
@@ -527,10 +546,15 @@ for my $spec (@specs) {
} else {
$label =~ s/\n//g;
print "label $spec $label\n";
+ # what a restart reads, until the next activation rebuilds it from the label
+ my (undef, $fingerprint) = read_keyring("$dir/keyring", $entity);
+ print "keyfp $spec $fingerprint\n" if defined($fingerprint);
}
- } elsif (-f "$dir/keyring" && open(my $fh, '<', "$dir/keyring")) {
- my $sections = join('', map { m/^(\[[^\]]*\])/ ? $1 : () } <$fh>);
+ } elsif (-f "$dir/keyring"
+ && (my ($sections, $fingerprint) = read_keyring("$dir/keyring", $entity))
+ ) {
print "keyring $spec $sections\n";
+ print "keyfp $spec $fingerprint\n";
} else {
print "store $spec missing\n";
}
@@ -1641,27 +1665,39 @@ my sub installed_versions() {
return { map { $_ => $broadcast->{$_}->{version}->{str} } keys %$broadcast };
}
-my sub probe_nodes($info, $plan, $opts = {}) {
- my $installed = installed_versions();
+# one SSH session per node; a node that cannot be reached gets its error instead of probes
+my sub probe_daemons($opts, @daemons) {
my $specs = {};
- for my $daemon (touched_daemons($info, $plan)) {
- assert_located($daemon->{node}, "$daemon->{type}.$daemon->{id}");
- # a host we do not manage has nothing to probe, so its running version is checked instead
- next if !$daemon->{managed};
- push $specs->{ $daemon->{node} }->@*, "$daemon->{type}:$daemon->{id}";
- }
+ push $specs->{ $_->{node} }->@*, "$_->{type}:$_->{id}" for @daemons;
my $by_node = {};
log_info(
"collecting daemon keyrings and bluestore labels from " . scalar(keys %$specs) . " node(s)")
if scalar(keys %$specs) && ($opts->{apply} || $opts->{verbose});
for my $node (sort keys %$specs) {
-
my $output =
eval { node_perl($node, $PROBE_SCRIPT, args => [$ccname, sort $specs->{$node}->@*]); };
- die "could not reach node '$node': $@" if $@;
+ if (my $err = $@) {
+ # an abort has to travel, not read as a node that cannot be reached
+ die $err if $err =~ m/aborting (?:on signal|bulk-restart)/;
+ $by_node->{$node} = { error => "could not reach node '$node': $err" };
+ next;
+ }
+ $by_node->{$node} = { probes => parse_probe_output($output) };
+ }
- $by_node->{$node} = parse_probe_output($output);
+ return $by_node;
+}
+
+my sub probe_nodes($info, $plan, $opts = {}) {
+ my $installed = installed_versions();
+ my @touched = touched_daemons($info, $plan);
+ assert_located($_->{node}, "$_->{type}.$_->{id}") for @touched;
+ # a host we do not manage has nothing to probe, so its running version is checked instead
+ my @managed = grep { $_->{managed} } @touched;
+ my $by_node = probe_daemons($opts, @managed);
+ for my $node (sort keys %$by_node) {
+ die $by_node->{$node}->{error} if $by_node->{$node}->{error};
}
# every daemon is judged on its Ceph version, only the touched ones on their data directory
@@ -1669,9 +1705,9 @@ my sub probe_nodes($info, $plan, $opts = {}) {
$_->{binary} = defined($_->{node}) ? $installed->{ $_->{node} } : undef
for $info->{daemons}->{$type}->@*;
}
- for my $daemon (touched_daemons($info, $plan)) {
- next if !$daemon->{managed};
- my $probe = $by_node->{ $daemon->{node} }->{"$daemon->{type}:$daemon->{id}"} // {};
+ for my $daemon (@managed) {
+ my $probe = $by_node->{ $daemon->{node} }->{probes}->{"$daemon->{type}:$daemon->{id}"}
+ // {};
$daemon->{store} = $probe->{store};
$daemon->{error} = $probe->{error};
$daemon->{'error-details'} = $probe->{'error-details'};
@@ -1684,6 +1720,89 @@ my sub probe_nodes($info, $plan, $opts = {}) {
return;
}
+my sub describe_disk_key_doubt($daemon, $why) {
+ my $text = {
+ 'probe-error' => $daemon->{error} // 'its inspection failed',
+ 'identity' => 'its bluestore label does not identify it as this OSD',
+ }->{$why}
+ // "no key it reads was found under /var/lib/ceph/$daemon->{type}/$ccname-$daemon->{id}";
+ return "$daemon->{entity} on node '$daemon->{node}': $text";
+}
+
+# daemons whose key on disk this run could not compare with the monitors'
+our $disk_key_doubts = [];
+
+# A key changed by hand reads as migrated in the auth database, while the daemon keeps the old one
+# on disk and fails at its next start. Record such daemons in $info->{stale_keys}.
+my sub check_disk_keys($info, $state, $opts) {
+ my (@probed, @doubts);
+ for my $daemon (disk_key_candidates($info, $state, $opts->{only})) {
+ my ($entity, $node) = ($daemon->{entity}, $daemon->{node});
+ if ($daemon->{managed}) {
+ push @probed, $daemon;
+ } elsif (defined($node)) {
+ push @doubts,
+ [$entity, "$entity on node '$node': not a node of this Proxmox VE cluster"];
+ } else {
+ push @doubts, [$entity, "$entity: no valid host name was reported"];
+ }
+ }
+ my $by_node = probe_daemons($opts, @probed);
+
+ $info->{stale_keys} = {};
+ for my $daemon (@probed) {
+ my ($entity, $node) = ($daemon->{entity}, $daemon->{node});
+ if (my $err = $by_node->{$node}->{error}) {
+ chomp($err);
+ push @doubts, [$entity, "$entity on node '$node': $err"];
+ next;
+ }
+ my $probe = $by_node->{$node}->{probes}->{"$daemon->{type}:$daemon->{id}"} // {};
+ my $verdict = disk_key_verdict($info, $daemon, $probe);
+ if ($verdict->{verdict} eq 'stale') {
+ $info->{stale_keys}->{$entity} = $verdict->{copies};
+ } elsif ($verdict->{verdict} eq 'unknown') {
+ chomp(my $text = describe_disk_key_doubt({ %$daemon, %$probe }, $verdict->{why}));
+ push @doubts, [$entity, $text];
+ }
+ }
+ # a stale copy is not planned for repair yet, so it also keeps the verdict from passing
+ $disk_key_doubts = [(map { $_->[0] } @doubts), sort keys $info->{stale_keys}->%*];
+ if (@doubts) {
+ log_warn("Could not compare the key on disk with the monitors' for these daemons, so a"
+ . " key changed outside this script would go unnoticed there:");
+ log_steps([map { $_->[1] } @doubts]);
+ }
+ # a keyring is read at the next start, an OSD's label at its next activation
+ my $stale = $info->{stale_keys};
+ if (%$stale) {
+ log_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: "
+ . join(
+ ', ',
+ map { "$_ (" . join(' and ', $stale->{$_}->@*) . ")" } sort keys %$stale,
+ )
+ . "."
+ );
+ }
+
+ return;
+}
+
+# a daemon whose key on disk could not be compared may still fail at its next start
+my sub log_verdict($message) {
+ if (@$disk_key_doubts) {
+ log_warn("$message But the key on disk could not be confirmed for "
+ . join(', ', @$disk_key_doubts)
+ . ", so a daemon listed may still fail at its next start.");
+ return;
+ }
+ log_pass($message);
+
+ return;
+}
+
# A client key rotated outside this script leaves clients behind the same way as one this
# script rotates, and only the key fingerprints recorded at earlier apply runs make that
# visible. Shared by the preflight and by the destructive actions, which reconcile again right
@@ -2442,7 +2561,7 @@ my sub preflight_cluster(
&& !$recovered_count
&& !@unfinished
) {
- log_pass("Every service key uses '$CIPHER', and so do the service tickets.");
+ log_verdict("Every service key uses '$CIPHER', and so do the service tickets.");
mon_key_hint($info, $opts);
return 0;
}
@@ -5113,7 +5232,7 @@ my sub print_open_options(
}
if (!$planned && !$opts->{only} && $open->{complete}) {
- log_pass("Cephx migration is complete. Authentication and new keys use only '$CIPHER'.");
+ log_verdict("Cephx migration is complete. Authentication and new keys use only '$CIPHER'.");
log_text("AUTH_INSECURE_ROTATING_SERVICE_KEY_TYPE clears on its own within a few hours."
. " You do not need to wait for it.")
if ($snapshot->{health_checks} // {})->{AUTH_INSECURE_ROTATING_SERVICE_KEY_TYPE};
@@ -5880,6 +5999,7 @@ my sub collect_run_info($rados, $opts, $state, $previous = undef) {
my sub run_migration($opts, $confirm = \&confirm_apply_run) {
local $storage_copy_warnings_seen = {};
+ local $disk_key_doubts = [];
$monmap_changed = 0;
PVE::RPCEnvironment->setup_default_cli_env();
PVE::Ceph::Tools::check_ceph_inited();
@@ -6078,6 +6198,9 @@ my sub run_migration($opts, $confirm = \&confirm_apply_run) {
. " saved plan because Ceph no longer lists it");
}
+ # before any verdict
+ check_disk_keys($info, $state, $opts);
+
# Refresh storage mappings after any lock wait, then make sure they still agree with the
# options before settling a staged key.
$client_files = client_key_files();
@@ -6170,9 +6293,9 @@ my sub run_migration($opts, $confirm = \&confirm_apply_run) {
}
if ($opts->{only}) {
- log_pass("There is nothing left to migrate in the scope given with '--only'.");
+ log_verdict("There is nothing left to migrate in the scope given with '--only'.");
} else {
- log_pass("Every service key uses '$CIPHER', and so do the service tickets.");
+ log_verdict("Every service key uses '$CIPHER', and so do the service tickets.");
}
mon_key_hint($info, $opts);
print_open_options(
@@ -6338,12 +6461,12 @@ my sub run_migration($opts, $confirm = \&confirm_apply_run) {
log_heading("Done");
if ($plan->{scoped}) {
- log_pass("The keys covered by '--only' now use the '$CIPHER' cipher. Run this without"
+ log_verdict("The keys covered by '--only' now use the '$CIPHER' cipher. Run this without"
. " '--only' to migrate the rest and to switch the service cipher over.");
mon_key_hint($info, $opts);
} elsif (scalar($plan->{daemons}->@*)) {
- log_pass("The manager, metadata server, and OSD keys of this cluster now use the '$CIPHER'"
- . " cipher.");
+ log_verdict("The manager, metadata server, and OSD keys of this cluster now use the"
+ . " '$CIPHER' cipher.");
mon_key_hint($info, $opts);
} else {
mon_key_hint($info, $opts);
@@ -6428,6 +6551,7 @@ sub key_migration_test_hooks {
print_closing_notes => \&print_closing_notes,
resume_live_swap => \&resume_live_swap,
probe_nodes => \&probe_nodes,
+ check_disk_keys => \&check_disk_keys,
probe_script => $PROBE_SCRIPT,
preflight_nodes => \&preflight_nodes,
preflight_mon_key => \&preflight_mon_key,
diff --git a/test/CephKeyMigrationScript_test.pl b/test/CephKeyMigrationScript_test.pl
index b03c2e29..1d473dd6 100755
--- a/test/CephKeyMigrationScript_test.pl
+++ b/test/CephKeyMigrationScript_test.pl
@@ -8411,4 +8411,231 @@ for my $case ([0, 0], [1, 0], [0, 1], [1, 1]) {
}
+# a key changed in the auth database alone must not read as a finished migration
+{
+ my $root = tempdir(CLEANUP => 1);
+ my $node = PVE::INotify::nodename();
+ mkdir("$root/mds");
+ mkdir("$root/mds/ceph-a");
+ my $write_keyring = sub {
+ open(my $fh, '>', "$root/mds/ceph-a/keyring") or die $!;
+ print {$fh} join('', map { "[$_->[0]]\n\tkey = $_->[1]\n" } @_);
+ close($fh);
+ };
+
+ my $mds = { type => 'mds', id => 'a', entity => 'mds.a', node => $node, managed => 1 };
+ my $info = {
+ fsid => 'cluster-fsid',
+ daemons => { mon => [], mgr => [], mds => [$mds], osd => [] },
+ exported => { 'mds.a' => { key => $NEW } },
+ };
+ my $check = sub {
+ my ($fail) = @_;
+ my $output;
+ no warnings qw(once redefine);
+ # the real probe, reading a data directory under $root instead of /var/lib/ceph. Through
+ # a pipe, as run_command() loses a child's output while STDOUT is captured in memory.
+ local *main::run_command = sub {
+ my ($cmd, %param) = @_;
+ die "$fail\n" if $fail;
+ my ($perl, undef, undef, @specs) = @$cmd;
+ my $script = File::Temp->new();
+ print {$script} $param{input} =~ s{/var/lib/ceph/}{$root/}gr;
+ close($script);
+ open(my $pipe, '-|', $perl, $script->filename, 'ceph', @specs) or die $!;
+ while (my $line = <$pipe>) {
+ chomp $line;
+ $param{outfunc}->($line);
+ }
+ close($pipe) or die "probe failed\n";
+ };
+ local *STDOUT;
+ open(STDOUT, '>', \$output) or die $!;
+ $HOOKS->{check_disk_keys}->($info, {}, {});
+ return ($info->{stale_keys}, $output // '');
+ };
+
+ $write_keyring->(['mds.a', $OLD]);
+ my ($stale, $output) = $check->();
+ is_deeply($stale, { 'mds.a' => ['keyring'] }, 'a keyring holding an old key is found');
+ like(
+ $output,
+ qr/^WARN: These keep a key on disk that the monitors do not accept.*: mds\.a \(keyring\)\.$/m,
+ 'and reported, naming the copy that differs',
+ );
+ $write_keyring->(['client.admin', $OLD], ['mds.a', $NEW]);
+ ($stale) = $check->();
+ is_deeply($stale, {}, 'a keyring is judged by the key under the daemon\'s own section');
+
+ ($stale, $output) = $check->("ssh: connect to host $node: No route to host");
+ is_deeply($stale, {}, 'an unreachable node judges nothing');
+ like(
+ $output,
+ qr/Could not compare the key on disk.*mds\.a on node '\Q$node\E': could not reach node .*No route to host/s,
+ 'but says which daemons it could not compare',
+ );
+ eval { $check->('aborting on signal, run this again to resume') };
+ like($@, qr/aborting on signal/, 'an abort while probing ends the check, not as a dead node');
+
+ $info->{daemons}->{mds} = [{ %$mds, node => 'elsewhere', managed => 0 }];
+ (undef, $output) = $check->();
+ like(
+ $output,
+ qr/mds\.a on node 'elsewhere': not a node of this Proxmox VE cluster$/m,
+ 'a daemon outside the cluster is reported too',
+ );
+}
+
+# The real orchestration, on a cluster that reads as migrated in the auth database while a
+# daemon kept an old key on disk.
+{
+ no warnings qw(once redefine);
+ my $dir = tempdir(CLEANUP => 1);
+ my $source = file_get_contents($SCRIPT);
+ $source =~ s{/etc/pve/priv/cephx-key-migration.json}{$dir/state.json}g;
+ $source =~ s{/var/lock/pve-ceph-bulk-restart.lck}{$dir/operation.lock}g;
+ $source =~
+ s/package PVE::Ceph::KeyMigration::ClusterLock;/package MigrationDiskKeyTest::ClusterLock;/;
+ local *PVE::Ceph::Tools::get_config = sub { return $_[0] eq 'ccname' ? 'ceph' : "$dir/$_[0]" };
+ file_set_contents("$dir/pve_mon_key_path", "[mon.]\n key = $NEW\n");
+ # a package of its own, as the run above loaded the script into MigrationRunTest already
+ eval "package MigrationDiskKeyTest; $source" or die $@;
+ my $migrate = MigrationDiskKeyTest::key_migration_test_hooks()->{run_migration};
+
+ local *PVE::RPCEnvironment::setup_default_cli_env = sub { };
+ local *PVE::Ceph::Tools::check_ceph_inited = sub { };
+ local *PVE::Ceph::KeyMigration::ClusterLock::take = sub { return {} };
+ local *PVE::Ceph::Services::get_cluster_service = sub { return {} };
+ local *PVE::Ceph::Services::get_ceph_versions = sub {
+ return { 'node-a' => { version => { str => '20.2.4' } } };
+ };
+ # 'ceph tell' runs on this node
+ local *PVE::Cluster::get_nodelist = sub { return ['node-a', PVE::INotify::nodename()] };
+ local *PVE::SSHInfo::get_ssh_info = sub { return { node => $_[0] } };
+ local *PVE::SSHInfo::ssh_info_to_command = sub { return ['ssh', $_[0]->{node}, '--'] };
+ local *PVE::Storage::config = sub { return { ids => {} } };
+ local *PVE::Ceph::Services::with_cluster_bulk_restart_lock = sub { $_[3]->() };
+ local *PVE::Ceph::Services::acquire_cluster_bulk_restart_lock = sub { };
+ local *PVE::Ceph::Services::get_blocking_health_errors = sub { return [] };
+ local *PVE::Ceph::Services::daemon_is_up = sub { return 1 };
+ local *PVE::Ceph::Services::wait_for_safe_to_stop = sub { return (1, '') };
+ local *PVE::Ceph::Services::wait_for_daemon_up = sub { };
+
+ my $entries;
+ my $fresh = 'AgCk941qku/sDSAAIjO5RhRv/ogXhuxccNS4DZxlXS1LUgzEGFIiY/U7ImI=';
+ my $rados = CurrentMonitorRados->new(
+ mons => ['a'],
+ quorum => ['a'],
+ osds => [],
+ metadata => [{ name => 'a', hostname => 'node-a', ceph_version_short => '20.2.4' }],
+ );
+ local *PVE::Ceph::Services::ResilientRados::new = sub { return $rados };
+ my $mon_command = CurrentMonitorRados->can('mon_command');
+ my @auth;
+ local *CurrentMonitorRados::mon_command = sub {
+ my ($self, $args) = @_;
+ my $prefix = $args->{prefix};
+ return 'cluster-fsid' if $prefix eq 'fsid';
+ return { quorum_names => ['a'], features => { quorum_mon => ['cephx_auth_aes256k'] } }
+ if $prefix eq 'quorum_status';
+ return {} if $prefix eq 'mgr dump';
+ return [{ name => 'a', hostname => 'node-a', ceph_version => 'ceph version 20.2.4' }]
+ if $prefix eq 'mds metadata';
+ return [] if $prefix =~ /^(?:mgr|osd) metadata$/;
+ return [
+ map {
+ { %$_ }
+ } values %$entries
+ ]
+ if $prefix eq 'auth export';
+ return [{ %{ $entries->{ $args->{entity} } } }] if $prefix eq 'auth get';
+ if ($prefix =~ m/^auth (rotate|get-or-create-pending|commit-pending)$/) {
+ push @auth, "$1 $args->{entity}";
+ my $entry = $entries->{ $args->{entity} };
+ $entry->{pending_key} = $fresh if $1 eq 'get-or-create-pending';
+ $entry->{key} = delete($entry->{pending_key}) if $1 eq 'commit-pending';
+ return [{%$entry}];
+ }
+ return $mon_command->(@_);
+ };
+ my ($on_disk, $unreachable, @probes, @written, @told, @units);
+ local *MigrationDiskKeyTest::file_set_contents = sub { };
+ local *MigrationDiskKeyTest::run_command = sub {
+ my ($cmd, %args) = @_;
+ return 0 if $cmd->[0] eq 'dpkg' && $cmd->[1] eq '--compare-versions';
+ my $out = '';
+ if (grep { $_ eq "'mds:a'" || $_ eq 'mds:a' } @$cmd) {
+ push @probes, join(' ', @$cmd);
+ die "ssh: connect to host node-a: No route to host\n" if $unreachable;
+ $out = "keyring mds:a [mds.a]\nkeyfp mds:a " . key_fingerprint($on_disk);
+ } elsif ($cmd->[-1] =~ m{/var/lib/ceph/mds/ceph-a/keyring'?$}) {
+ my ($key) = $args{input} =~ m/__END__\n\[mds\.a\]\n\tkey = (\S+)\n/;
+ push @written, $key;
+ $on_disk = $key;
+ } elsif (grep { $_ eq 'tell' } @$cmd) {
+ push @told,
+ join(' ', grep { $_ ne '--cluster' && $_ ne 'ceph' } @$cmd[1 .. $#$cmd]);
+ } elsif (grep { m/^'?systemctl'?$/ } @$cmd) {
+ push @units, join(' ', map { s/'//gr } @$cmd[-2, -1]);
+ } elsif ($cmd->[-1] eq 'sessions') {
+ $out = '[]';
+ } elsif ($cmd->[-1] eq 'auth_service_required') {
+ $out = encode_json({ auth_service_required => 'cephx' });
+ } elsif ($cmd->[-1] eq 'mon_auth_client_pending_key_auto_promote') {
+ $out = encode_json({ mon_auth_client_pending_key_auto_promote => 'false' });
+ } else {
+ die 'unexpected node command: ' . join(' ', @$cmd) . "\n";
+ }
+ $args{outfunc}->($_) for split(/\n/, $out);
+ return 0;
+ };
+ my $run = sub {
+ my ($disk_key, $opts, $node_down) = @_;
+ ($on_disk, $unreachable) = ($disk_key, $node_down);
+ (@probes, @written, @told, @units, @auth) = ();
+ $entries = {
+ 'mon.' => { entity => 'mon.', key => $NEW },
+ 'mds.a' =>
+ { entity => 'mds.a', key => $NEW, caps => { mon => 'allow profile mds' } },
+ };
+ my ($result, $out) = (undef, '');
+ open(my $stdout, '>', \$out) or die $!;
+ local *STDOUT = $stdout;
+ $result = eval {
+ $migrate->($opts, sub { return 1 });
+ };
+ is($@, '', 'the run itself does not die');
+ return ($result, $out);
+ };
+
+ my ($result, $out) = $run->($NEW, {});
+ 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');
+ is(scalar(@probes), 1, 'after inspecting the keyring of the metadata server once');
+
+ ($result, $out) = $run->($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',
+ );
+ 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',
+ );
+ 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');
+
+ ($result, $out) = $run->($NEW, {}, 1);
+ is($result, 0, 'a cluster whose daemon could not be checked still reads as migrated');
+ 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,
+ 'but the verdict says which daemon could not be checked',
+ );
+ unlike($out, qr/^PASS:/m, 'and passes nothing');
+}
+
done_testing();
diff --git a/test/CephKeyMigration_test.pl b/test/CephKeyMigration_test.pl
index d087c76a..dfa3ec91 100755
--- a/test/CephKeyMigration_test.pl
+++ b/test/CephKeyMigration_test.pl
@@ -15,6 +15,7 @@ use PVE::Ceph::KeyMigration qw(
parse_auth_methods summarize_monitor_authentication
$CIPHER $LEGACY_CIPHER
key_cipher key_fingerprint parse_probe_output osd_label_identity needs_rotation mon_keyring_stale
+ daemon_in_scope disk_key_candidates disk_key_verdict
mon_key_rotation_wanted migration_unfinished unfinished_entities touched_daemons
plan_client_keys build_plan bulk_storage_staging_needed client_staging_needed
client_keys_requested
@@ -427,6 +428,80 @@ isnt(key_fingerprint($NEW), key_fingerprint($OLD), 'different keys get different
is($broken->{'osd:5'}->{store}, 'block-without-key', 'so is one that does not decode');
}
+{
+ # the fingerprint of the key under the daemon's own section, empty if that section has none
+ my $old = key_fingerprint($OLD);
+ my $probes = parse_probe_output(
+ "keyring mds:a [mds.a]\nkeyfp mds:a $old\nkeyring mds:b [mds.b]\nkeyfp mds:b\n"
+ . "label osd:3 $LABEL\n",
+ );
+ is($probes->{'mds:a'}->{'keyring-fingerprint'}, $old, 'a keyring reports its key fingerprint');
+ is($probes->{'mds:b'}->{'keyring-fingerprint'}, '', 'one without a key reports none');
+ ok(!exists($probes->{'osd:3'}->{'keyring-fingerprint'}), 'an OSD not activated has no keyring');
+}
+
+# --- whether the key a daemon reads at its next start is one the monitors accept -------------
+{
+ my ($old, $new) = (key_fingerprint($OLD), key_fingerprint($NEW));
+ my $info = {
+ fsid => 'e3bcf831-39e8-4db3-a458-bbfa643a139d',
+ exported => { map { $_ => { key => $NEW } } qw(mds.a osd.2) },
+ };
+ my $mds = { type => 'mds', id => 'a', entity => 'mds.a' };
+ my $osd = { type => 'osd', id => '2', entity => 'osd.2' };
+ my $label = parse_probe_output("label osd:2 $LABEL\n")->{'osd:2'};
+
+ is_deeply(
+ disk_key_verdict($info, $mds, { store => 'file', 'keyring-fingerprint' => $old }),
+ { verdict => 'stale', copies => ['keyring'] },
+ 'a keyring holding another key than the monitors is stale',
+ );
+ is_deeply(
+ disk_key_verdict($info, $osd, { %$label, 'keyring-fingerprint' => $new }),
+ { verdict => 'match' },
+ 'an OSD whose label and keyring hold the active key matches',
+ );
+ is_deeply(
+ disk_key_verdict($info, $osd, { %$label, 'keyring-fingerprint' => $old }),
+ { verdict => 'stale', copies => ['keyring'] },
+ 'a restart reads the keyring, so it counts apart from the label',
+ );
+ is_deeply(
+ disk_key_verdict($info, $osd, { %$label, 'label-whoami' => '9' }),
+ { verdict => 'unknown', why => 'identity' },
+ 'a label left behind by another OSD says nothing about this one',
+ );
+ is_deeply(
+ disk_key_verdict($info, $mds, { store => 'missing' }),
+ { verdict => 'unknown', why => 'missing' },
+ 'a daemon without a key on disk cannot be judged',
+ );
+ is_deeply(
+ disk_key_verdict({ %$info, exported => {} }, $mds, { store => 'file' }),
+ { verdict => 'match' },
+ 'without an auth entry there is nothing to differ from',
+ );
+}
+{
+ ok(daemon_in_scope(undef, 'mds', 'mds.a'), 'without --only every daemon is in scope');
+ ok(daemon_in_scope({ 'mds.a' => 1 }, 'mds', 'mds.a'), 'a name selects its daemon');
+ ok(!daemon_in_scope({ osd => 1 }, 'mds', 'mds.a'), 'and others are out of scope');
+
+ # the plan rewrites the keys of a resumed daemon anyway, so its copies are not compared
+ my $info = {
+ daemons => {
+ mgr => [{ type => 'mgr', id => 'x', entity => 'mgr.x' }],
+ mds => [{ type => 'mds', id => 'a', entity => 'mds.a', recovered => 1 }],
+ osd => [{ type => 'osd', id => '2', entity => 'osd.2' }],
+ },
+ };
+ is_deeply(
+ [map { $_->{entity} } disk_key_candidates($info, { rotated => { 'osd.2' => 1 } })],
+ ['mgr.x'],
+ 'a daemon resumed from the saved plan or with an unfinished rotation is not compared',
+ );
+}
+
# --- which keys still need work -------------------------------------------------------------
{
my $info = { exported => { 'osd.1' => { key => $OLD }, 'osd.2' => { key => $NEW } } };
--
2.47.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH manager v2 3/3] migrations: cephx: rotate daemons whose key on disk differs
2026-10-25 9:57 [PATCH manager v2 0/3] migrations: cephx: catch daemons whose key on disk is stale Kefu Chai
2026-10-25 9:57 ` [PATCH manager v2 1/3] migrations: cephx: collect the run's cluster information in one place Kefu Chai
2026-10-25 9:57 ` [PATCH manager v2 2/3] migrations: cephx: report daemons whose key on disk differs Kefu Chai
@ 2026-10-25 9:57 ` Kefu Chai
2 siblings, 0 replies; 4+ messages in thread
From: Kefu Chai @ 2026-10-25 9:57 UTC (permalink / raw)
To: pve-devel
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.
Have needs_rotation() count such a daemon, so the plan takes it like
one still on the old cipher and migrating it writes a key the monitors
accept to every copy. A daemon Ceph does not list stays stopped as
before, and as it may have stopped because of that very key, the plan
and the run now name the commands that start it, with or without
'--restart-daemons'. They clear the unit's start limit first, since
failed starts can use it up, and 'systemctl start' alone is then
refused.
A daemon an earlier run rotated can now reach the branch that keeps its
current key after that key was changed by hand. Its message claimed the
earlier run produced the key, so fold it into the branch that keeps any
key already on the new cipher, which says only that.
Signed-off-by: Kefu Chai <k.chai@proxmox.com>
---
PVE/Ceph/KeyMigration.pm | 3 ++
bin/pve-cephx-rotate-service-keys | 41 +++++++++++++---------
test/CephKeyMigrationScript_test.pl | 53 ++++++++++++++++++++++++-----
test/CephKeyMigration_test.pl | 16 +++++++++
4 files changed, 88 insertions(+), 25 deletions(-)
diff --git a/PVE/Ceph/KeyMigration.pm b/PVE/Ceph/KeyMigration.pm
index 0cf2fc0e..45d64dc6 100644
--- a/PVE/Ceph/KeyMigration.pm
+++ b/PVE/Ceph/KeyMigration.pm
@@ -376,6 +376,9 @@ sub disk_key_verdict($info, $daemon, $probe) {
}
sub needs_rotation($info, $entity) {
+ # a key changed by hand reads as migrated in the auth database
+ return 1 if ($info->{stale_keys} // {})->{$entity};
+
my $entry = $info->{exported}->{$entity};
# 'ceph-mon --mkfs' moves 'mon.' out of the auth database into the monitor keyrings, so judge
diff --git a/bin/pve-cephx-rotate-service-keys b/bin/pve-cephx-rotate-service-keys
index 7b6bd536..f3af6cfa 100755
--- a/bin/pve-cephx-rotate-service-keys
+++ b/bin/pve-cephx-rotate-service-keys
@@ -1766,8 +1766,7 @@ my sub check_disk_keys($info, $state, $opts) {
push @doubts, [$entity, $text];
}
}
- # a stale copy is not planned for repair yet, so it also keeps the verdict from passing
- $disk_key_doubts = [(map { $_->[0] } @doubts), sort keys $info->{stale_keys}->%*];
+ $disk_key_doubts = [map { $_->[0] } @doubts];
if (@doubts) {
log_warn("Could not compare the key on disk with the monitors' for these daemons, so a"
. " key changed outside this script would go unnoticed there:");
@@ -2560,6 +2559,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);
@@ -2964,6 +2964,12 @@ my sub recover_left_behind($info, $state) {
return \@recovered;
}
+# failed starts may have used up the unit's start limit, and a plain start is refused then
+my sub start_command($type, $id) {
+ my $unit = "ceph-$type\@$id";
+ return "systemctl reset-failed $unit; systemctl start $unit";
+}
+
my sub print_plan($info, $plan, $state, $opts, $storage_entities) {
$storage_entities //= {};
my $records = staged_records($info, $state);
@@ -3136,16 +3142,17 @@ 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 { start_command($_->{type}, $_->{id}) . " (node '$_->{node}')" } @down]);
+ }
if ($opts->{verbose}) {
log_step("in this order:");
log_steps([map { "$_->{entity} on $_->{node}" } $plan->{daemons}->@*]);
@@ -3383,9 +3390,9 @@ my sub health_gate($rados, $type, $what) {
my sub rotate_entity($rados, $state, $entity) {
my $before = auth_entry($rados, $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");
+ # the key may come from an earlier run or from someone else, so judge it, not the marker
+ if ((key_cipher($before->{key}) // -1) == $CIPHER_ID) {
+ log_info("the key of '$entity' already uses the '$CIPHER' cipher, keeping it");
return $before;
}
if ($state->{rotated}->{$entity}) {
@@ -3393,10 +3400,6 @@ my sub rotate_entity($rados, $state, $entity) {
. ($CIPHER_NAMES->{ key_cipher($before->{key}) // -1 } // 'unreadable')
. "' cipher now, so it is rotated again");
}
- if ((key_cipher($before->{key}) // -1) == $CIPHER_ID) {
- log_info("the key of '$entity' already uses the '$CIPHER' cipher, leaving it alone");
- return $before;
- }
# markers of an older rotation must not hide this one on resume
delete $state->{done}->{$entity};
@@ -4948,7 +4951,9 @@ 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 '"
+ . start_command($type, $id)
+ . "'.");
$state->{done}->{$entity} = time();
delete $state->{live_swap}->{$entity};
save_state($state);
@@ -5987,6 +5992,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 1d473dd6..aadeb702 100755
--- a/test/CephKeyMigrationScript_test.pl
+++ b/test/CephKeyMigrationScript_test.pl
@@ -4448,6 +4448,27 @@ 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,
+ };
+ my $output = '';
+ open(my $stdout, '>', \$output) or die $!;
+ local *STDOUT = $stdout;
+ $HOOKS->{print_plan}->($info, $plan, {}, {}, $stores);
+ like(
+ $output,
+ qr/left stopped: mds\.a\. Start one that should run afterwards, on its node:\n systemctl reset-failed ceph-mds\@a; systemctl start ceph-mds\@a \(node 'pve1'\)$/m,
+ 'a daemon left stopped comes with the command to start it',
+ );
+ }
+
my $mixed = $render->(
[$tool, $resume], { 'rotate-all-storage-keys' => 1, 'rotate-client-keys' => 1 },
);
@@ -8614,20 +8635,36 @@ for my $case ([0, 0], [1, 0], [0, 1], [1, 1]) {
is(scalar(@probes), 1, 'after inspecting the keyring of the metadata server once');
($result, $out) = $run->($OLD, {});
- is($result, 0, 'a daemon holding another key than the monitors does not stop the run');
+ is($result, 0, 'a daemon holding another key than the monitors is planned');
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',
- );
- 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');
+ ($result, $out) = $run->($OLD, { apply => 1 });
+ 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');
+
+ ($result, $out) = $run->($OLD, { apply => 1, 'restart-daemons' => 1 });
+ 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',
+ );
+
($result, $out) = $run->($NEW, {}, 1);
is($result, 0, 'a cluster whose daemon could not be checked still reads as migrated');
like(
diff --git a/test/CephKeyMigration_test.pl b/test/CephKeyMigration_test.pl
index dfa3ec91..6cbf51af 100755
--- a/test/CephKeyMigration_test.pl
+++ b/test/CephKeyMigration_test.pl
@@ -512,6 +512,9 @@ isnt(key_fingerprint($NEW), key_fingerprint($OLD), 'different keys get different
0,
'an entity with no auth entry is not this step\'s business',
);
+ # a key changed by hand reads as migrated in the auth database
+ $info->{stale_keys} = { 'osd.2' => ['label'] };
+ is(needs_rotation($info, 'osd.2'), 1, 'unless a copy on disk holds another key');
}
{
# 'ceph-mon --mkfs' moves 'mon.' out of the auth database and 'ceph-authtool --gen-key' has
@@ -1652,6 +1655,19 @@ 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',
+ );
+
my $switched = cluster();
$switched->{service_cipher} = $CIPHER;
is(
--
2.47.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-10-25 9:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-25 9:57 [PATCH manager v2 0/3] migrations: cephx: catch daemons whose key on disk is stale Kefu Chai
2026-10-25 9:57 ` [PATCH manager v2 1/3] migrations: cephx: collect the run's cluster information in one place Kefu Chai
2026-10-25 9:57 ` [PATCH manager v2 2/3] migrations: cephx: report daemons whose key on disk differs Kefu Chai
2026-10-25 9:57 ` [PATCH manager v2 3/3] migrations: cephx: rotate " Kefu Chai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox