superseded by https://lore.proxmox.com/pve-devel/[email protected]/T/#t
On Thu, Mar 12, 2026 at 11:35 AM Kefu Chai <[email protected]> wrote: > On Thu Mar 12, 2026 at 1:01 AM CST, Thomas Lamprecht wrote: > > Am 11.03.26 um 14:29 schrieb Kefu Chai: > >> The condition guarding bootstrap-osd keyring creation checks for > >> `auth_client_required eq 'cephx'` by reading ceph.conf directly. When > >> this setting is absent from ceph.conf (relying on the Ceph default, or > >> configured via the mon config database instead), the check evaluates as > >> `undef eq 'cephx'` which is false, causing PVE to skip creating the > >> bootstrap keyring. ceph-volume then fails because it cannot find > >> /var/lib/ceph/bootstrap-osd/ceph.keyring. > >> > >> This can happen when: > >> - ceph.conf [global] was created before `pveceph init` wrote the auth > >> settings (pveceph init skips writing them if [global] already exists) > >> - auth settings were moved from ceph.conf to the mon config database > >> - an upgrade or migration left ceph.conf without the auth lines > >> > >> Fix by defaulting to 'cephx' when the setting is absent (matching > >> Ceph's own default) and inverting the check to only skip keyring > >> creation when auth is explicitly set to 'none'. > >> > >> Signed-off-by: Kefu Chai <[email protected]> > >> Signed-off-by: Kefu Chai <[email protected]> > >> --- > >> PVE/API2/Ceph/OSD.pm | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/PVE/API2/Ceph/OSD.pm b/PVE/API2/Ceph/OSD.pm > >> index a952c952..062729ae 100644 > >> --- a/PVE/API2/Ceph/OSD.pm > >> +++ b/PVE/API2/Ceph/OSD.pm > >> @@ -407,7 +407,7 @@ __PACKAGE__->register_method({ > >> > >> if ( > >> !-f $ceph_bootstrap_osd_keyring > >> - && $ceph_conf->{global}->{auth_client_required} eq 'cephx' > >> + && ($ceph_conf->{global}->{auth_client_required} // > 'cephx') ne 'none' > > > > As the same variable is already accessed in the line above without a > > fallback, it either has to be already always defined and this // 'cephx' > > is superfluous, or we should pull that out upfront and use it for both, > > like: > > > > my $auth_client_required = $ceph_conf->{global}->{auth_client_required} > // 'cephx'; > > > > (disclaimer, I did not review within the full code context, only found > > above pattern slightly odd) > > Thank you for the feedback! Your suggestion to extract > $auth_client_required to a named variable is a good style improvement: > it makes the intent clearer and is more readable. I've updated the patch > to do exactly that. > > Regarding "accessed in the line above without a fallback": in this > function, auth_client_required is only read in one place (the if > condition), so there isn't a second use to consolidate. The diff context > does show other $ceph_conf->{global}->{...} accesses nearby, which may > be what caught your eye. Totally understandable given you noted you > didn't have the full context. > > The updated version now reads: > > my $auth_client_required = $ceph_conf->{global}->{auth_client_required} // > 'cephx'; > if (!-f $ceph_bootstrap_osd_keyring && $auth_client_required ne 'none') { > > This is cleaner regardless, since the extracted variable avoids the long > hash dereference inside the condition. Thanks again for taking a look! > > > > >> ) { > >> my $bindata = $rados->mon_command({ > >> prefix => 'auth get-or-create', > > > -- Regards Kefu Chai
