* [PATCH manager 0/3] migrations: cephx: catch daemons whose key on disk is stale
@ 2026-10-12 14:37 Kefu Chai
2026-10-12 14:38 ` [PATCH manager 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-12 14:37 UTC (permalink / raw)
To: pve-devel
A user ran into the following after the upgrade to Ceph 20: an admin
rotated the keys of all MDS daemons by hand with
'ceph auth rotate --key-type=aes256k', then ran the migration helper.
The helper only looked at the cipher of each key in the auth database,
saw aes256k, and reported PASS. The keyrings on disk still held the old
keys, so when the MDS daemons restarted they failed with
handle_auth_bad_method and CephFS went down. This series is my attempt
to catch that case.
Patch 1 moves the places that collect a run's cluster information into
a single helper. While doing so I found some older bugs it fixes: the
RADOS handle got lost after resuming a lockbox key, the preferred
cipher was overwritten by a later read from the monitor map, which
reports an unsupported value as 'unknown', and daemons recovered from a
saved plan were dropped again after the client key steps. It does not
depend on the other patches, so it could go in on its own.
Patch 2 compares the key each manager, metadata server and OSD reads at
its next start with the key the monitors hold. That is the key in its
keyring and, for an OSD, also the one in its bluestore label. It warns
about a daemon whose copy differs, and if a copy differs or cannot be
compared, the verdicts that report the cluster as migrated become a
warning naming that daemon instead of a PASS. The cost is one more SSH
session per node on every run, dry runs included.
Patch 3 then plans such a daemon for rotation like one still on the old
cipher, so the helper fixes the copies itself. I split it out in case
you would rather take only the detection, or want to discuss the
rotation part separately.
Besides the unit and script tests, I tried it on a small Proxmox VE 9.2
cluster with Ceph 20.2.4. With the final version:
- an MDS rotated by hand and stopped, with '--restart-daemons': the
plan named its start command, its key was written, and it started
fine when I started it by hand afterwards
- a running MDS rotated by hand while another node was unreachable: its
key was swapped live, and the closing verdict was a warning naming
the daemons on the unreachable node
With an earlier version, which differed in messages and checks but not
in how keys are rotated:
- a running MDS rotated by hand: its key was swapped live, and it
restarted cleanly afterwards
- an OSD rotated by hand: its label and keyring were both rewritten,
and it restarted cleanly
- a '--restart-daemons' run interrupted with SIGINT halfway: the next
run planned only the daemon it had not reached
One thing I was unsure about: with a stale key on disk, a dry run still
prints "Preflight checks passed for the displayed plan", as for any
other plan. If that reads too much like a PASS for the cluster, I am
happy to change it.
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 | 68 ++++-
bin/pve-cephx-rotate-service-keys | 230 +++++++++++++----
test/CephKeyMigrationScript_test.pl | 380 ++++++++++++++++++++++++++++
test/CephKeyMigration_test.pl | 138 ++++++++++
4 files changed, 766 insertions(+), 50 deletions(-)
--
2.47.3
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH manager 1/3] migrations: cephx: collect the run's cluster information in one place
2026-10-12 14:37 [PATCH manager 0/3] migrations: cephx: catch daemons whose key on disk is stale Kefu Chai
@ 2026-10-12 14:38 ` Kefu Chai
2026-10-12 14:38 ` [PATCH manager 2/3] migrations: cephx: report daemons whose key on disk differs Kefu Chai
2026-10-12 14:38 ` [PATCH manager 3/3] migrations: cephx: rotate " Kefu Chai
2 siblings, 0 replies; 4+ messages in thread
From: Kefu Chai @ 2026-10-12 14:38 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 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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH manager 2/3] migrations: cephx: report daemons whose key on disk differs
2026-10-12 14:37 [PATCH manager 0/3] migrations: cephx: catch daemons whose key on disk is stale Kefu Chai
2026-10-12 14:38 ` [PATCH manager 1/3] migrations: cephx: collect the run's cluster information in one place Kefu Chai
@ 2026-10-12 14:38 ` Kefu Chai
2026-10-12 14:38 ` [PATCH manager 3/3] migrations: cephx: rotate " Kefu Chai
2 siblings, 0 replies; 4+ messages in thread
From: Kefu Chai @ 2026-10-12 14:38 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 on an unreachable node or with a pending key
staged, 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 | 63 +++++-
bin/pve-cephx-rotate-service-keys | 167 +++++++++++++--
test/CephKeyMigrationScript_test.pl | 318 ++++++++++++++++++++++++++++
test/CephKeyMigration_test.pl | 117 ++++++++++
4 files changed, 639 insertions(+), 26 deletions(-)
diff --git a/PVE/Ceph/KeyMigration.pm b/PVE/Ceph/KeyMigration.pm
index d0613e8b..1abf1eee 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,57 @@ 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;
+
+ # writing the active key would undo a rotation someone staged and not yet committed
+ my $pending = $info->{exported}->{ $daemon->{entity} }->{pending_key};
+ return { verdict => 'unknown', why => 'pending' }
+ if length($pending // '')
+ && grep { ($copies->{$_} // '') eq key_fingerprint($pending) } @wrong;
+
+ return { verdict => 'stale', copies => \@wrong };
+}
+
sub needs_rotation($info, $entity) {
my $entry = $info->{exported}->{$entity};
@@ -1747,8 +1802,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}->@*];
@@ -1756,7 +1809,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 1cdb2a45..647251cf 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
@@ -404,6 +405,7 @@ my $PROBE_SCRIPT = <<'PERL_EOF';
use strict;
use warnings;
+use Digest::SHA qw(sha256_hex);
use JSON;
use File::Temp;
use POSIX ();
@@ -447,10 +449,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 eq 'mon' ? 'mon.' : "$type.$id";
if (-e "$dir/block") {
my ($label, $err, $details) =
@@ -460,10 +479,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";
}
@@ -1585,25 +1609,37 @@ 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_member($daemon->{node}, "$daemon->{type}.$daemon->{id}");
- 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_member($_->{node}, "$_->{type}.$_->{id}") for @touched;
+ my $by_node = probe_daemons($opts, @touched);
+ 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
@@ -1611,8 +1647,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)) {
- my $probe = $by_node->{ $daemon->{node} }->{"$daemon->{type}:$daemon->{id}"} // {};
+ for my $daemon (@touched) {
+ 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'};
@@ -1625,6 +1662,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',
+ 'pending' => 'it holds the pending key staged for it; commit or clear that key first',
+ '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 (!defined($node) || ref($node) || !length($node)) {
+ push @doubts, "$entity: no valid host name was reported";
+ } elsif (!cluster_nodes()->{$node}) {
+ push @doubts, "$entity on node '$node': not a node of this Proxmox VE cluster";
+ } else {
+ push @probed, $daemon;
+ next;
+ }
+ push @$disk_key_doubts, $entity;
+ }
+ my $by_node = probe_daemons($opts, @probed);
+
+ $info->{stale_keys} = {};
+ for my $daemon (@probed) {
+ my $node = $by_node->{ $daemon->{node} };
+ my $probe =
+ $node->{error}
+ ? { store => 'probe-error', error => $node->{error} }
+ : $node->{probes}->{"$daemon->{type}:$daemon->{id}"} // {};
+ 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;
+ push @$disk_key_doubts, $daemon->{entity};
+ }
+ }
+ 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(\@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
@@ -2379,7 +2499,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;
}
@@ -4942,7 +5062,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};
@@ -5709,6 +5829,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();
@@ -5907,6 +6028,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();
@@ -5999,9 +6123,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(
@@ -6170,12 +6294,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);
@@ -6260,6 +6384,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 1392b514..2cfc75f0 100755
--- a/test/CephKeyMigrationScript_test.pl
+++ b/test/CephKeyMigrationScript_test.pl
@@ -7991,4 +7991,322 @@ 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();
+ my $write_keyring = sub {
+ my ($type, $id, $text) = @_;
+ my $dir = "$root/$type/ceph-$id";
+ mkdir("$root/$type");
+ mkdir($dir);
+ open(my $fh, '>', "$dir/keyring") or die $!;
+ print {$fh} $text;
+ close($fh);
+ };
+ my $ring = sub {
+ my ($entity, $key) = @_;
+ return "[$entity]\n\tkey = $key\n\tcaps mon = \"allow profile mds\"\n";
+ };
+ # a stand-in for ceph-bluestore-tool, printing a label for osd.7 that holds the active key
+ my $bin = "$root/bin";
+ mkdir($bin);
+ my $whoami = 7;
+ my $write_tool = sub {
+ open(my $tool, '>', "$bin/ceph-bluestore-tool") or die $!;
+ print {$tool} "#!/bin/sh\necho '{\"$root/osd/ceph-7/block\": {\"whoami\": \"$whoami\","
+ . " \"ceph_fsid\": \"cluster-fsid\", \"osd_uuid\": \"u7\", \"osd_key\": \"$NEW\"}}'\n";
+ close($tool);
+ chmod(0755, "$bin/ceph-bluestore-tool");
+ };
+ $write_tool->();
+ local $ENV{PATH} = "$bin:$ENV{PATH}";
+
+ my $mds = { type => 'mds', id => 'a', entity => 'mds.a', node => $node };
+ my $info = {
+ fsid => 'cluster-fsid',
+ daemons => { mon => [], mgr => [], mds => [$mds], osd => [] },
+ exported => { 'mds.a' => { key => $NEW }, 'osd.7' => { key => $NEW } },
+ };
+ my @probed;
+ my $check = sub {
+ my ($opts, $fail) = @_;
+ my $output;
+ @probed = ();
+ 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) = @_;
+ my ($perl, undef, undef, @specs) = @$cmd;
+ push @probed, @specs;
+ die(($fail eq '1' ? "ssh: connect to host $node: No route to host" : $fail) . "\n")
+ if $fail;
+ 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, {}, $opts // {});
+ return ($info->{stale_keys}, $output // '');
+ };
+
+ $write_keyring->('mds', 'a', $ring->('mds.a', $OLD));
+ my ($stale, $output) = $check->();
+ is_deeply(
+ $stale,
+ { 'mds.a' => ['keyring'] },
+ 'a keyring holding another key than the monitors 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->('mds', 'a', $ring->('client.admin', $OLD) . $ring->('mds.a', $NEW));
+ ($stale) = $check->();
+ is_deeply($stale, {}, 'a keyring is judged by the key under the daemon\'s own section');
+ $write_keyring->('mds', 'a', $ring->('mds.a', $OLD) . $ring->('mds.b', $NEW));
+ ($stale) = $check->();
+ is_deeply([keys %$stale], ['mds.a'], 'so a matching key under another section does not count');
+ $write_keyring->('mds', 'a', "[mds.a]\n");
+ ($stale) = $check->();
+ is_deeply($stale, { 'mds.a' => ['keyring'] }, 'and a keyring without a key is stale');
+
+ $write_keyring->('mds', 'a', $ring->('mds.a', $NEW));
+
+ # an OSD restart reads its data-directory keyring, which can disagree with its label
+ my $osd = { type => 'osd', id => '7', entity => 'osd.7', node => $node };
+ $info->{daemons}->{osd} = [$osd];
+ $write_keyring->('osd', '7', $ring->('osd.7', $OLD));
+ open(my $block, '>', "$root/osd/ceph-7/block") or die $!;
+ close($block);
+ ($stale, $output) = $check->();
+ is_deeply($stale, { 'osd.7' => ['keyring'] }, 'an OSD keyring disagreeing is found');
+ $whoami = 9;
+ $write_tool->();
+ ($stale, $output) = $check->();
+ is_deeply($stale, {}, 'a label left behind by another OSD is not judged');
+ like(
+ $output,
+ qr/^WARN: Could not compare the key on disk.*osd\.7 on node '\Q$node\E': its bluestore label does not identify it as this OSD/ms,
+ 'but reported, so a key changed by hand there would not go unnoticed',
+ );
+ $info->{daemons}->{osd} = [{ %$osd, id => '8', entity => 'osd.8' }];
+ $info->{exported}->{'osd.8'} = { key => $NEW };
+ ($stale, $output) = $check->();
+ like(
+ $output,
+ qr/osd\.8 on node '\Q$node\E': no key it reads was found under /,
+ 'and so is a daemon without a data directory',
+ );
+ $info->{daemons}->{osd} = [
+ { %$osd, id => '8', entity => 'osd.8', node => undef },
+ { %$osd, id => '9', entity => 'osd.9', node => 'elsewhere' },
+ ];
+ $info->{exported}->{'osd.9'} = { key => $NEW };
+ ($stale, $output) = $check->();
+ is_deeply([grep { m/^osd:/ } @probed], [], 'a daemon without a cluster node is not probed');
+ like($output, qr/osd\.8: no valid host name was reported$/m, 'one without a host is reported');
+ like(
+ $output,
+ qr/osd\.9 on node 'elsewhere': not a node of this Proxmox VE cluster$/m,
+ 'and one on a foreign host, in the same form as the others',
+ );
+ $info->{daemons}->{osd} = [];
+
+ ($stale, $output) = $check->({}, 1);
+ 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',
+ );
+ {
+ no warnings qw(once redefine);
+ local *PVE::Ceph::Services::get_ceph_versions = sub { return {} };
+ my $asked = 0;
+ local *main::run_command = sub { $asked++; die "ssh: No route to host\n" };
+ eval { $HOOKS->{probe_nodes}->($info, { daemons => [$mds], lockbox_keys => [] }) };
+ like($@, qr/could not reach node '\Q$node\E'/, 'a plan touching one of them refuses');
+ is($asked, 1, 'after asking that node again');
+ local *main::run_command = sub {
+ my ($cmd, %param) = @_;
+ $param{outfunc}->("keyring mds:a [mds.a]");
+ return 0;
+ };
+ $HOOKS->{probe_nodes}->($info, { daemons => [$mds], lockbox_keys => [] });
+ is($mds->{store}, 'file', 'so a node that answers by then is no longer refused');
+ }
+
+ 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');
+
+ ($stale) = $check->({ only => { osd => 1 } });
+ is_deeply(\@probed, [], "a daemon outside the '--only' scope is not probed");
+}
+
+# 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]) {
+ my ($mode, $on_disk) = @$case;
+ 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 MigrationRunTest::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");
+ {
+ # each case loads the script again into the same package
+ local $SIG{__WARN__} = sub { warn @_ if $_[0] !~ m/^Subroutine \S+ redefined/ };
+ eval "package MigrationRunTest; $source" or die $@;
+ }
+ my $run = MigrationRunTest::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 = {
+ 'mon.' => { entity => 'mon.', key => $NEW },
+ 'mds.a' => { entity => 'mds.a', key => $NEW, caps => { mon => 'allow profile mds' } },
+ };
+ 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 (@probes, @written, @told, @units);
+ local *MigrationRunTest::file_set_contents = sub { };
+ local *MigrationRunTest::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 $mode eq '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 ($result, $error, $out) = (undef, '', '');
+ {
+ open(my $stdout, '>', \$out) or die $!;
+ local *STDOUT = $stdout;
+ $result = eval {
+ $run->(
+ {
+ $mode =~ m/^apply/ ? (apply => 1) : (),
+ $mode eq 'apply restart' ? ('restart-daemons' => 1) : (),
+ },
+ sub { return 1 },
+ );
+ };
+ $error = $@;
+ }
+ is($error, '', "the $mode itself does not die");
+ is(scalar(@probes), 1, 'the keyring of the metadata server is inspected once');
+ if ($mode eq 'unreachable') {
+ 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');
+ } 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',
+ );
+ 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');
+ } 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');
+ }
+}
+
done_testing();
diff --git a/test/CephKeyMigration_test.pl b/test/CephKeyMigration_test.pl
index 79fccedf..c0d9384a 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,122 @@ 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');
}
+# --- whether the key a daemon reads at its next start is the one the monitors accept -----------
+{
+ my $old = key_fingerprint($OLD);
+ my $new = key_fingerprint($NEW);
+ my $probes = parse_probe_output(
+ "keyring mds:a [mds.a]\nkeyfp mds:a $old\n"
+ . "keyring mds:b [mds.b]\nkeyfp mds:b\n"
+ . "label osd:2 $LABEL\nkeyfp osd:2 $new\n"
+ . "label osd:3 $LABEL\n"
+ . "store osd:4 missing\n",
+ );
+ is($probes->{'mds:a'}->{'keyring-fingerprint'}, $old, 'a keyring reports its key fingerprint');
+ is($probes->{'mds:a'}->{store}, 'file', 'which does not change what kind of store it is');
+ is($probes->{'mds:b'}->{'keyring-fingerprint'}, '', 'one without a key reports none');
+ ok(
+ !exists($probes->{'osd:3'}->{'keyring-fingerprint'}),
+ 'an OSD without a keyring reports none',
+ );
+
+ my $info = {
+ fsid => 'e3bcf831-39e8-4db3-a458-bbfa643a139d',
+ exported => { map { $_ => { key => $NEW } } qw(mds.a mds.b mds.c osd.2 osd.3 osd.4) },
+ };
+ my $verdict = sub {
+ my ($entity, $probe, %daemon) = @_;
+ my ($type, $id) = split(/\./, $entity);
+ return disk_key_verdict(
+ $info,
+ { type => $type, id => $id, entity => $entity, %daemon },
+ $probe // $probes->{"$type:$id"},
+ );
+ };
+
+ is_deeply(
+ $verdict->('mds.a'),
+ { verdict => 'stale', copies => ['keyring'] },
+ 'a keyring holding another key than the monitors is stale',
+ );
+ is_deeply(
+ $verdict->('mds.b'),
+ { verdict => 'stale', copies => ['keyring'] },
+ 'and so is one holding no key',
+ );
+ is_deeply($verdict->('osd.2'), { verdict => 'match' }, 'an OSD whose label and keyring match');
+ is_deeply(
+ $verdict->('osd.3', { %{ $probes->{'osd:3'} }, 'label-whoami' => '3' }),
+ { verdict => 'match' },
+ 'and one not activated, by its label',
+ );
+ $info->{exported}->{'osd.2'}->{key} = $OLD;
+ is_deeply(
+ $verdict->('osd.2'),
+ { verdict => 'stale', copies => ['keyring', 'label'] },
+ 'an OSD is compared by its label and its keyring',
+ );
+ $info->{exported}->{'osd.2'}->{key} = $NEW;
+ is_deeply(
+ $verdict->('osd.2', { %{ $probes->{'osd:2'} }, 'keyring-fingerprint' => $old }),
+ { verdict => 'stale', copies => ['keyring'] },
+ 'as a restart reads the keyring, not the label',
+ );
+ is_deeply(
+ $verdict->('osd.2', { %{ $probes->{'osd:2'} }, 'label-whoami' => '9' }),
+ { verdict => 'unknown', why => 'identity' },
+ 'a label left behind by another OSD says nothing about this one',
+ );
+ is_deeply(
+ $verdict->('osd.4'),
+ { verdict => 'unknown', why => 'missing' },
+ 'a daemon without a key on disk is unknown',
+ );
+ is_deeply(
+ $verdict->('mds.c', {}),
+ { verdict => 'unknown', why => 'missing' },
+ 'as is one not probed',
+ );
+
+ $info->{exported}->{'mds.a'}->{pending_key} = $OLD;
+ is_deeply(
+ $verdict->('mds.a'),
+ { verdict => 'unknown', why => 'pending' },
+ 'a copy holding a staged pending key is left to whoever staged it',
+ );
+ delete $info->{exported}->{'mds.a'};
+ is_deeply($verdict->('mds.a'), { verdict => 'match' }, 'without an auth entry nothing differs');
+
+ ok(daemon_in_scope(undef, 'mds', 'mds.a'), 'without --only every daemon is in scope');
+ ok(daemon_in_scope({ mds => 1 }, 'mds', 'mds.a'), 'a type selects its daemons');
+ ok(daemon_in_scope({ 'mds.a' => 1 }, 'mds', 'mds.a'), 'and so does a name');
+ ok(!daemon_in_scope({ osd => 1 }, 'mds', 'mds.a'), 'others are out of scope');
+
+ my $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({ daemons => $daemons }, {})],
+ ['mgr.x', 'osd.2'],
+ 'a daemon resumed from the saved plan is not compared, as it is rewritten anyway',
+ );
+ is_deeply(
+ [
+ map { $_->{entity} }
+ disk_key_candidates({ daemons => $daemons }, { rotated => { 'osd.2' => 1 } })
+ ],
+ ['mgr.x'],
+ 'nor is one whose rotation is unfinished',
+ );
+ is_deeply(
+ [map { $_->{entity} } disk_key_candidates({ daemons => $daemons }, {}, { osd => 1 })],
+ ['osd.2'],
+ "and '--only' narrows the comparison",
+ );
+}
+
# --- 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 3/3] migrations: cephx: rotate daemons whose key on disk differs
2026-10-12 14:37 [PATCH manager 0/3] migrations: cephx: catch daemons whose key on disk is stale Kefu Chai
2026-10-12 14:38 ` [PATCH manager 1/3] migrations: cephx: collect the run's cluster information in one place Kefu Chai
2026-10-12 14:38 ` [PATCH manager 2/3] migrations: cephx: report daemons whose key on disk differs Kefu Chai
@ 2026-10-12 14:38 ` Kefu Chai
2 siblings, 0 replies; 4+ messages in thread
From: Kefu Chai @ 2026-10-12 14:38 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.
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 <k.chai@proxmox.com>
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-10-12 14:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-12 14:37 [PATCH manager 0/3] migrations: cephx: catch daemons whose key on disk is stale Kefu Chai
2026-10-12 14:38 ` [PATCH manager 1/3] migrations: cephx: collect the run's cluster information in one place Kefu Chai
2026-10-12 14:38 ` [PATCH manager 2/3] migrations: cephx: report daemons whose key on disk differs Kefu Chai
2026-10-12 14:38 ` [PATCH manager 3/3] migrations: cephx: rotate " Kefu Chai
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.