[pve-devel] applied: [PATCH manager v2] ceph: create mon: fix & improve check if IP is in public net

[Wolfgang Bumiller]

applied

On Tue, Nov 28, 2017 at 04:32:44PM +0100, Thomas Lamprecht wrote:
> If a CIDR gets passed to Net::IP it is expected to not be from the
> middle of  an subnet, i.e., 192.168.1.12/24 is *not* OK but
> 192.168.1.0/24 would be OK.
> 
> As the Network/interfaces files also accepts CIDR notation for the
> 'address' param (now also for IPv4) this let to problems in our node
> monitor IP detection code, which used the interface file and Net::IP to
> find any address from the ceph public network.
> 
> So change to our newer helper PVE::Network::get_local_ip_from_cidr to
> get all configured and ready (=up) IPs from this network.
> 
> Also handle the case where multiple networks where returned, add a
> parameter to allow specifying one of those and ask the user to do so.
> 
> If no public network is configured and no mon-address parameter was
> passed, we fall back to the remote node IP of the node, as was done
> previously. We expect that the user only overwrites the mon-address
> if he knows what he do and omit checks here.
> ---
> 
> changes v1 -> v2:
> * the regex in grep had an unquoted variable, instead of quoting just
>   use 'eq' in grep, makes it even more clear what we do
> * don't use post-fix if on return but only on dies, make code a bit more
>   readable
> 
>  PVE/API2/Ceph.pm | 52 +++++++++++++++++++++++++++++++---------------------
>  1 file changed, 31 insertions(+), 21 deletions(-)
> 
> diff --git a/PVE/API2/Ceph.pm b/PVE/API2/Ceph.pm
> index 0fd8e388..be193a46 100644
> --- a/PVE/API2/Ceph.pm
> +++ b/PVE/API2/Ceph.pm
> @@ -918,24 +918,32 @@ __PACKAGE__->register_method ({
>  	return undef;
>      }});
>  
> -my $find_node_ip = sub {
> -    my ($cidr) = @_;
> +my $find_mon_ip = sub {
> +    my ($pubnet, $node, $overwrite_ip) = @_;
>  
> -    my $net = Net::IP->new($cidr) || die Net::IP::Error() . "\n";
> -    my $id = $net->version == 6 ? 'address6' : 'address';
> -
> -    my $config = PVE::INotify::read_file('interfaces');
> -    my $ifaces = $config->{ifaces};
> -
> -    foreach my $iface (keys %$ifaces) {
> -	my $d = $ifaces->{$iface};
> -	next if !$d->{$id};
> -	my $a = Net::IP->new($d->{$id});
> -	next if !$a;
> -	return $d->{$id} if $net->overlaps($a);
> +    if (!$pubnet) {
> +	return $overwrite_ip // PVE::Cluster::remote_node_ip($node);
>      }
>  
> -    die "unable to find local address within network '$cidr'\n";
> +    my $allowed_ips = PVE::Network::get_local_ip_from_cidr($pubnet);
> +    die "No IP configured and up from ceph public network '$pubnet'\n"
> +	if scalar(@$allowed_ips) < 1;
> +
> +    if (!$overwrite_ip) {
> +	if (scalar(@$allowed_ips) == 1) {
> +	    return $allowed_ips->[0];
> +	}
> +	die "Multiple IPs for ceph public network '$pubnet' detected on $node:\n".
> +	    join("\n", @$allowed_ips) ."\nuse 'mon-address' to specify one of them.\n";
> +    } else {
> +	if (grep { $_ eq $overwrite_ip } @$allowed_ips) {
> +	    return $overwrite_ip;
> +	}
> +	die "Monitor IP '$overwrite_ip' not in ceph public network '$pubnet'\n"
> +	    if !PVE::Network::is_ip_in_cidr($overwrite_ip, $pubnet);
> +
> +	die "Specified monitor IP '$overwrite_ip' not configured or up on $node!\n";
> +    }
>  };
>  
>  my $create_mgr = sub {
> @@ -1016,6 +1024,12 @@ __PACKAGE__->register_method ({
>  		default => 0,
>  		description => "When set, only a monitor will be created.",
>  	    },
> +	    'mon-address' => {
> +		description => 'Overwrites autodetected monitor IP address. ' .
> +		               'Must be in the public network of ceph.',
> +		type => 'string', format => 'ip',
> +		optional => 1,
> +	    },
>  	},
>      },
>      returns => { type => 'string' },
> @@ -1057,12 +1071,8 @@ __PACKAGE__->register_method ({
>  	my $monid = $param->{id} // $param->{node};
>  
>  	my $monsection = "mon.$monid";
> -	my $ip;
> -	if (my $pubnet = $cfg->{global}->{'public network'}) {
> -	    $ip = &$find_node_ip($pubnet);
> -	} else {
> -	    $ip = PVE::Cluster::remote_node_ip($param->{node});
> -	}
> +	my $pubnet = $cfg->{global}->{'public network'};
> +	my $ip = $find_mon_ip->($pubnet, $param->{node}, $param->{'mon-address'});
>  
>  	my $monaddr = Net::IP::ip_is_ipv6($ip) ? "[$ip]:6789" : "$ip:6789";
>  	my $monname = $param->{node};
> -- 
> 2.11.0