Re: [PATCH manager 1/1] ceph: osd: fix bootstrap keyring creation when auth_client_required is not in ceph.conf

[kefu chai <tchaikov@gmail.com>]

superseded by
https://lore.proxmox.com/pve-devel/20260312050901.1054941-1-k.chai@proxmox.com/T/#t

On Thu, Mar 12, 2026 at 11:35 AM Kefu Chai <k.chai@proxmox.com> 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 <tchaikov@gmail.com>
> >> Signed-off-by: Kefu Chai <k.chai@proxmox.com>
> >> ---
> >>  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