[pve-devel] [PATCH manager 07/14] ceph: mon create: refactor mon assertions

[Thomas Lamprecht]

On 6/18/19 3:42 PM, Dominik Csapak wrote:
> by using our new 'get_services_info'
> 
> this already checks for nautilus+ style 'mon_host' key in the ceph.conf
> for the ip address
> 
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
>  PVE/API2/Ceph/MON.pm | 40 +++++++++++++++++++++++-----------------
>  1 file changed, 23 insertions(+), 17 deletions(-)
> 
> diff --git a/PVE/API2/Ceph/MON.pm b/PVE/API2/Ceph/MON.pm
> index 44110bd5..75c28039 100644
> --- a/PVE/API2/Ceph/MON.pm
> +++ b/PVE/API2/Ceph/MON.pm
> @@ -56,6 +56,23 @@ my $find_mon_ip = sub {
>      }
>  };
>  
> +my $assert_mon_prerequisites = sub {
> +    my ($cfg, $monhash, $monid, $monip) = @_;
> +
> +    my $ip_regex = '(?:^|[^\d])' . # start or nondigit
> +	       "\Q$monip\E" .  # the ip not interpreted as regex
> +	       '(?:[^\d]|$)';  # end or nondigit
> +
> +    if (($cfg->{global}->{mon_host} && $cfg->{global}->{mon_host} =~ m/$ip_regex/) ||
> +	grep { $_->{addr} && $_->{addr}  =~ m/$ip_regex/ } values %$monhash) {
> +	die "monitor address '$monip' already in use\n";
> +    }
> +
> +    if (defined($monhash->{$monid})) {
> +	die "monitor '$monid' already exists\n";
> +    }
> +};
> +
>  __PACKAGE__->register_method ({
>      name => 'listmon',
>      path => '',
> @@ -163,32 +180,21 @@ __PACKAGE__->register_method ({
>  
>  	my $cfg = cfs_read_file('ceph.conf');
>  	my $rados = eval { PVE::RADOS->new() }; # try a rados connection, fails for first monitor
> +	my $monhash = PVE::Ceph::Services::get_services_info('mon', $cfg, $rados);
> +	my $firstmon = !defined($rados);

above adds really to confusion and is a bit strange, IMO!

Please keep using !$rados or !defined($rados) for below checks and
re-open with longer timeout logic
>  
> -	my $moncount = 0;
> -	my $monaddrhash = {};
> -	foreach my $section (keys %$cfg) {
> -	    next if $section eq 'global';
> -
> -	    if ($section =~ m/^mon\./) {
> -		my $d = $cfg->{$section};
> -		$moncount++;
> -		if ($d->{'mon addr'}) {
> -		    $monaddrhash->{$d->{'mon addr'}} = $section;
> -		}
> -	    }
> +	if ($firstmon && (scalar(keys %$monhash) || $cfg->{global}->{mon_host})) {

s/!$firstmon/!$rados/

> +	    die "Could not connect to ceph cluster despite configured monitors\n";
>  	}
>  
>  	my $monid = $param->{monid} // $param->{node};
> -
>  	my $monsection = "mon.$monid";
>  	my $ip = $find_mon_ip->($cfg, $rados, $param->{node}, $param->{'mon-address'});
>  
>  	my $monaddr = Net::IP::ip_is_ipv6($ip) ? "[$ip]:6789" : "$ip:6789";
>  	my $monname = $param->{node};
>  
> -	die "monitor '$monsection' already exists\n" if $cfg->{$monsection};
> -	die "monitor address '$monaddr' already in use by '$monaddrhash->{$monaddr}'\n"
> -	    if $monaddrhash->{$monaddr};
> +	$assert_mon_prerequisites->($cfg, $monhash, $monid, $ip);
>  
>  	my $worker = sub  {
>  	    my $upid = shift;
> @@ -225,7 +231,7 @@ __PACKAGE__->register_method ({
>  
>  		run_command("chown ceph:ceph $mondir");
>  
> -		if ($moncount > 0) {
> +		if (!$firstmon) {

if (!defined($rados)) { # cannot have a rados connection when creating first mon..

>  		    my $rados = PVE::RADOS->new(timeout => PVE::Ceph::Tools::get_config('long_rados_timeout'));
>  		    my $mapdata = $rados->mon_command({ prefix => 'mon getmap', format => 'plain' });
>  		    file_set_contents($monmap, $mapdata);
>