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

["Kefu Chai" <k.chai@proxmox.com>]

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',