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

[Thomas Lamprecht]

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