[pve-devel] [PATCH manager 1/2] pve5to6: check ceph config for mon_host line

[Thomas Lamprecht]

On 7/4/19 10:07 AM, Fabian Grünbichler wrote:
> On Thu, Jul 04, 2019 at 09:35:12AM +0200, Dominik Csapak wrote:
>> this already works on luminous, so it does not harm to add it already,
>> and is recommended when both msgr1 and msgr2 is activated in nautilus
>>
>> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> 
> check for global->keyring would be good as well.
> 
>> ---
>>  PVE/CLI/pve5to6.pm | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/PVE/CLI/pve5to6.pm b/PVE/CLI/pve5to6.pm
>> index c167ebca..bb54c735 100644
>> --- a/PVE/CLI/pve5to6.pm
>> +++ b/PVE/CLI/pve5to6.pm
>> @@ -421,6 +421,19 @@ sub check_ceph {
>>  	}
>>      }
>>  
>> +    log_info("checking Ceph config..");
>> +    my $conf = PVE::Cluster::cfs_read_file('ceph.conf');
>> +    if (defined($conf)) {
> 
> should always be defined, as neither cfs_read_file should return undef
> itself, nor can the parser for ceph.conf.

FYI: if the file does not exists you get undef...
https://git.proxmox.com/?p=pve-cluster.git;a=blob;f=data/PVE/Cluster.pm;h=b53dcd726f9d42cd8e94c59fb584818e79c94ad7;hb=HEAD#l873
also it does not harm, it's just a safe guard.

> 
>> +	my $global = $conf->{global};
> 
> but this might be undef ?

which then returns just undef, it is not created by this, thus
the checks on $global->{subkey} does not result in any auto-vivification
so just fine.

> 
>> +	if (!defined($global->{mon_host}) && !defined($global->{"mon host"})) {
>> +	    log_warn("No mon_host entry found in ceph config.\n  It is recommended to add mon_host with all monitor addresses(without ports) to the global section.");
>> +	} else {
>> +	    log_pass("Found mon_host entry.");
>> +	}
>> +    } else {
>> +	log_skip("no ceph config found");
> 
> this else branch should be dead code, since we checked ceph_inited right at the
> start of this sub ;)

that one should be true, though ;)

> 
>> +    }
>> +
>>      my $local_ceph_ver = PVE::Ceph::Tools::get_local_version(1);
>>      if (defined($local_ceph_ver)) {
>>  	if ($local_ceph_ver == 14) {
>> -- 
>> 2.20.1
>>
>>