[pve-devel] [RFC manager] fix #2422: allow multiple Ceph public networks

[pve-devel] [RFC manager] fix #2422: allow multiple Ceph public networks

[Fabian Ebner]

picked up from an old patch by Alwin[0].

The first four patches are cleanups/preparation.

The last patch is the big one which introduces the handling of multiple IP
addresses. Quickly tested with a dual IPv4/IPv6 setup and an external client
and didn't see any issues (altough I might've missed something in my struggle
to get the network configuration right). It is a bit messy (not sure that is
fully possible to avoid) and I'd like to test it some more, so sending it as an
RFC. Would be great if somebody else could test it too.

Note that you also need a dual stack cluster network even if it's separate from
the public network, so the OSDs will start up.

[0]: https://lists.proxmox.com/pipermail/pve-devel/2020-March/042304.html

Fabian Ebner (5):
  api: ceph: mon: split up arguments for run_command
  api: ceph: create mon: handle ms_bind_ipv* options more generally
  api: ceph: create mon: factor out monmaptool command
  api: ceph: create mon: explicitly add subsequent monitors to the
    monmap
  fix #2422: allow multiple Ceph public networks

 PVE/API2/Ceph/MON.pm | 228 +++++++++++++++++++++++++++++++------------
 1 file changed, 163 insertions(+), 65 deletions(-)

-- 
2.20.1

[pve-devel] [RFC manager 1/5] api: ceph: mon: split up arguments for run_command

[Fabian Ebner]

no functional change is intended.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/API2/Ceph/MON.pm | 46 +++++++++++++++++++++++++++++++++++++-------
 1 file changed, 39 insertions(+), 7 deletions(-)

diff --git a/PVE/API2/Ceph/MON.pm b/PVE/API2/Ceph/MON.pm
index 19ba5e50..8f2d5bc4 100644
--- a/PVE/API2/Ceph/MON.pm
+++ b/PVE/API2/Ceph/MON.pm
@@ -229,9 +229,23 @@ __PACKAGE__->register_method ({
 
 		if (! -f $mon_keyring) {
 		    print "creating new monitor keyring\n";
-		    run_command("ceph-authtool --create-keyring $mon_keyring ".
-			" --gen-key -n mon. --cap mon 'allow *'");
-		    run_command("ceph-authtool $mon_keyring --import-keyring $client_keyring");
+		    run_command([
+			'ceph-authtool',
+			'--create-keyring',
+			$mon_keyring,
+			'--gen-key',
+			'-n',
+			'mon.',
+			'--cap',
+			'mon',
+			'allow *',
+		    ]);
+		    run_command([
+			'ceph-authtool',
+			$mon_keyring,
+			'--import-keyring',
+			$client_keyring,
+		    ]);
 		}
 
 		my $ccname = PVE::Ceph::Tools::get_config('ccname');
@@ -243,7 +257,7 @@ __PACKAGE__->register_method ({
 		eval {
 		    mkdir $mondir;
 
-		    run_command("chown ceph:ceph $mondir");
+		    run_command(['chown', 'ceph:ceph', $mondir]);
 
 		    if (defined($rados)) { # we can only have a RADOS object if we have a monitor
 			my $mapdata = $rados->mon_command({ prefix => 'mon getmap', format => 'plain' });
@@ -255,11 +269,29 @@ __PACKAGE__->register_method ({
 			    $cfg->{global}->{ms_bind_ipv6} = 'true';
 			    $cfg->{global}->{ms_bind_ipv4} = 'false';
 			}
-			run_command("monmaptool --create --clobber --addv $monid '[v2:$monaddr:3300,v1:$monaddr:6789]' --print $monmap");
+			run_command([
+			    'monmaptool',
+			    '--create',
+			    '--clobber',
+			    '--addv',
+			    $monid,
+			    "[v2:$monaddr:3300,v1:$monaddr:6789]",
+			    '--print',
+			    $monmap,
+			]);
 		    }
 
-		    run_command("ceph-mon --mkfs -i $monid --monmap $monmap --keyring $mon_keyring");
-		    run_command("chown ceph:ceph -R $mondir");
+		    run_command([
+			'ceph-mon',
+			'--mkfs',
+			'-i',
+			$monid,
+			'--monmap',
+			$monmap,
+			'--keyring',
+			$mon_keyring,
+		    ]);
+		    run_command(['chown', 'ceph:ceph', '-R', $mondir]);
 		};
 		my $err = $@;
 		unlink $monmap;
-- 
2.20.1

[pve-devel] [RFC manager 2/5] api: ceph: create mon: handle ms_bind_ipv* options more generally

[Fabian Ebner]

mostly relevant to prepare support for IPv4/IPv6 dual stack mode as a special
case of the planned support for mutliple public networks.

As before, only set the false value when we are dealing with the first address,
but also be explicit about the IPv4 case as the defaults might change in the
future.

Then, when an address of a different type comes along later, set the relevant
bind option to true.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/API2/Ceph/MON.pm | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/PVE/API2/Ceph/MON.pm b/PVE/API2/Ceph/MON.pm
index 8f2d5bc4..618d3f06 100644
--- a/PVE/API2/Ceph/MON.pm
+++ b/PVE/API2/Ceph/MON.pm
@@ -259,6 +259,15 @@ __PACKAGE__->register_method ({
 
 		    run_command(['chown', 'ceph:ceph', $mondir]);
 
+		    my $is_first_address = !defined($rados);
+		    if (Net::IP::ip_is_ipv6($ip)) {
+			$cfg->{global}->{ms_bind_ipv6} = 'true';
+			$cfg->{global}->{ms_bind_ipv4} = 'false' if $is_first_address;
+		    } else {
+			$cfg->{global}->{ms_bind_ipv4} = 'true';
+			$cfg->{global}->{ms_bind_ipv6} = 'false' if $is_first_address;
+		    }
+
 		    if (defined($rados)) { # we can only have a RADOS object if we have a monitor
 			my $mapdata = $rados->mon_command({ prefix => 'mon getmap', format => 'plain' });
 			file_set_contents($monmap, $mapdata);
@@ -266,8 +275,6 @@ __PACKAGE__->register_method ({
 			my $monaddr = $ip;
 			if (Net::IP::ip_is_ipv6($ip)) {
 			    $monaddr = "[$ip]";
-			    $cfg->{global}->{ms_bind_ipv6} = 'true';
-			    $cfg->{global}->{ms_bind_ipv4} = 'false';
 			}
 			run_command([
 			    'monmaptool',
-- 
2.20.1

[pve-devel] [RFC manager 3/5] api: ceph: create mon: factor out monmaptool command

[Fabian Ebner]

so it's easier to re-use for a future variant.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/API2/Ceph/MON.pm | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/PVE/API2/Ceph/MON.pm b/PVE/API2/Ceph/MON.pm
index 618d3f06..0f9b30ba 100644
--- a/PVE/API2/Ceph/MON.pm
+++ b/PVE/API2/Ceph/MON.pm
@@ -268,24 +268,24 @@ __PACKAGE__->register_method ({
 			$cfg->{global}->{ms_bind_ipv6} = 'false' if $is_first_address;
 		    }
 
+		    my $monaddr = Net::IP::ip_is_ipv6($ip) ? "[$ip]" : $ip;
+
+		    my $monmaptool_cmd = [
+			'monmaptool',
+			'--create',
+			'--clobber',
+			'--addv',
+			$monid,
+			"[v2:$monaddr:3300,v1:$monaddr:6789]",
+			'--print',
+			$monmap,
+		    ];
+
 		    if (defined($rados)) { # we can only have a RADOS object if we have a monitor
 			my $mapdata = $rados->mon_command({ prefix => 'mon getmap', format => 'plain' });
 			file_set_contents($monmap, $mapdata);
 		    } else { # we need to create a monmap for the first monitor
-			my $monaddr = $ip;
-			if (Net::IP::ip_is_ipv6($ip)) {
-			    $monaddr = "[$ip]";
-			}
-			run_command([
-			    'monmaptool',
-			    '--create',
-			    '--clobber',
-			    '--addv',
-			    $monid,
-			    "[v2:$monaddr:3300,v1:$monaddr:6789]",
-			    '--print',
-			    $monmap,
-			]);
+			run_command($monmaptool_cmd);
 		    }
 
 		    run_command([
-- 
2.20.1

[pve-devel] [RFC manager 4/5] api: ceph: create mon: explicitly add subsequent monitors to the monmap

[Fabian Ebner]

in preparation for supporting multiple addresses. The config section does not
allow more than one public_addr.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 PVE/API2/Ceph/MON.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/PVE/API2/Ceph/MON.pm b/PVE/API2/Ceph/MON.pm
index 0f9b30ba..6f232e34 100644
--- a/PVE/API2/Ceph/MON.pm
+++ b/PVE/API2/Ceph/MON.pm
@@ -272,7 +272,6 @@ __PACKAGE__->register_method ({
 
 		    my $monmaptool_cmd = [
 			'monmaptool',
-			'--create',
 			'--clobber',
 			'--addv',
 			$monid,
@@ -284,7 +283,9 @@ __PACKAGE__->register_method ({
 		    if (defined($rados)) { # we can only have a RADOS object if we have a monitor
 			my $mapdata = $rados->mon_command({ prefix => 'mon getmap', format => 'plain' });
 			file_set_contents($monmap, $mapdata);
+			run_command($monmaptool_cmd);
 		    } else { # we need to create a monmap for the first monitor
+			push @{$monmaptool_cmd}, '--create';
 			run_command($monmaptool_cmd);
 		    }
 
-- 
2.20.1

[pve-devel] [RFC manager 5/5] fix #2422: allow multiple Ceph public networks

[Fabian Ebner]

From: Alwin Antreich <a.antreich@proxmox.com>

Multiple public networks can be defined in the ceph.conf. The networks need to
be routed to each other.

Support handling multiple IPs for a single monitor. By default, one address from
each public network is selected for monitor creation, but, as before, it can be
overwritten with the mon-address parameter, now taking a list of addresses.

On removal, make sure the all addresses are removed from the mon_host entry in
the ceph configuration.

Originally-by: Alwin Antreich <a.antreich@proxmox.com>
[handling of multiple addresses]
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---

Sorry if the patch is a bit messy to review, what it essentially is, is creating
loops over nets/addresses around the old existing logic.

Viewing with a word-based diff might help in some places.

Changes from Alwin's patch as suggested by Fabian G.:
    * split list after truth check for $pubnet
    * avoid long post-if statement
    * style/indentation fixes
    * verify cidr

Other changes:
    * test/select IPs from each public network separately
    * return and handle a list of IPs, so that it's not necessary to create a
      monitor for each public network
    * make mon-address a list of IPs
    * also handle multiple addresses upon removal

What could still be improved (not new in this patch):
    * match addresses semantically everywhere, which is mostly relevant for IPv6

 PVE/API2/Ceph/MON.pm | 180 ++++++++++++++++++++++++++++---------------
 1 file changed, 119 insertions(+), 61 deletions(-)

diff --git a/PVE/API2/Ceph/MON.pm b/PVE/API2/Ceph/MON.pm
index 6f232e34..63d4d93e 100644
--- a/PVE/API2/Ceph/MON.pm
+++ b/PVE/API2/Ceph/MON.pm
@@ -20,8 +20,10 @@ use PVE::API2::Ceph::MGR;
 
 use base qw(PVE::RESTHandler);
 
-my $find_mon_ip = sub {
-    my ($cfg, $rados, $node, $overwrite_ip) = @_;
+my $find_mon_ips = sub {
+    my ($cfg, $rados, $node, $mon_address) = @_;
+
+    my $overwrite_ips = [ PVE::Tools::split_list($mon_address) ];
 
     my $pubnet;
     if ($rados) {
@@ -34,40 +36,70 @@ my $find_mon_ip = sub {
     $pubnet //= $cfg->{global}->{public_network};
 
     if (!$pubnet) {
-	return $overwrite_ip // PVE::Cluster::remote_node_ip($node);
+	if (scalar(@{$overwrite_ips})) {
+	    return $overwrite_ips;
+	} else {
+	   # don't refactor into '[ PVE::Cluster::remote... ]' as it uses wantarray
+	   my $ip = PVE::Cluster::remote_node_ip($node);
+	   return [ $ip ];
+	}
     }
 
-    my $allowed_ips = PVE::Network::get_local_ip_from_cidr($pubnet);
-    die "No active IP found for the requested ceph public network '$pubnet' on node '$node'\n"
-	if scalar(@$allowed_ips) < 1;
+    my $public_nets = [ PVE::Tools::split_list($pubnet) ];
+    if (scalar(@{$public_nets}) > 1) {
+	warn "Multiple Ceph public networks detected on $node: $pubnet\n";
+	warn "Networks must be capable of routing to each other.\n";
+    }
+
+    my $res = [];
+
+    if (!scalar(@{$overwrite_ips})) { # auto-select one address for each public network
+	for my $net (@{$public_nets}) {
+	    $net = PVE::JSONSchema::pve_verify_cidr($net);
 
-    if (!$overwrite_ip) {
-	if (scalar(@$allowed_ips) == 1 || !grep { $_ ne $allowed_ips->[0] } @$allowed_ips) {
-	    return $allowed_ips->[0];
+	    my $allowed_ips = PVE::Network::get_local_ip_from_cidr($net);
+	    die "No active IP found for the requested ceph public network '$net' on node '$node'\n"
+		if scalar(@$allowed_ips) < 1;
+
+	    if (scalar(@$allowed_ips) == 1 || !grep { $_ ne $allowed_ips->[0] } @$allowed_ips) {
+		push @{$res}, $allowed_ips->[0];
+	    } else {
+		die "Multiple IPs for ceph public network '$net' detected on $node:\n".
+		    join("\n", @$allowed_ips) ."\nuse 'mon-address' to specify one of them.\n";
+	    }
+	}
+    } else { # check if overwrite IPs are active and in any of the public networks
+	my $allowed_ips = [];
+
+	for my $net (@{$public_nets}) {
+	    $net = PVE::JSONSchema::pve_verify_cidr($net);
+
+	    push @{$allowed_ips}, @{PVE::Network::get_local_ip_from_cidr($net)};
 	}
-	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;
+
+	for my $overwrite_ip (@{$overwrite_ips}) {
+	    die "Specified monitor IP '$overwrite_ip' not configured or up on $node!\n"
+		if !grep { $_ eq $overwrite_ip } @{$allowed_ips};
 	}
-	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";
+	$res = $overwrite_ips;
     }
+
+    return $res;
 };
 
 my $assert_mon_prerequisites = sub {
-    my ($cfg, $monhash, $monid, $monip) = @_;
+    my ($cfg, $monhash, $monid, $monips) = @_;
 
-    my $ip_regex = '(?:^|[^\d])' . # start or nondigit
-	       "\Q$monip\E" .  # the ip not interpreted as regex
-	       '(?:[^\d]|$)';  # end or nondigit
+    for my $monip (@{$monips}) {
+	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 (($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})) {
@@ -177,9 +209,9 @@ __PACKAGE__->register_method ({
 		description => "The ID for the monitor, when omitted the same as the nodename",
 	    },
 	    'mon-address' => {
-		description => 'Overwrites autodetected monitor IP address. ' .
-		               'Must be in the public network of ceph.',
-		type => 'string', format => 'ip',
+		description => 'Overwrites autodetected monitor IP address(es). ' .
+		               'Must be in the public network(s) of Ceph.',
+		type => 'string', format => 'ip-list',
 		optional => 1,
 	    },
 	},
@@ -207,9 +239,9 @@ __PACKAGE__->register_method ({
 
 	my $monid = $param->{monid} // $param->{node};
 	my $monsection = "mon.$monid";
-	my $ip = $find_mon_ip->($cfg, $rados, $param->{node}, $param->{'mon-address'});
+	my $ips = $find_mon_ips->($cfg, $rados, $param->{node}, $param->{'mon-address'});
 
-	$assert_mon_prerequisites->($cfg, $monhash, $monid, $ip);
+	$assert_mon_prerequisites->($cfg, $monhash, $monid, $ips);
 
 	my $worker = sub  {
 	    my $upid = shift;
@@ -222,7 +254,7 @@ __PACKAGE__->register_method ({
 		    $rados = PVE::RADOS->new(timeout => PVE::Ceph::Tools::get_config('long_rados_timeout'));
 		}
 		$monhash = PVE::Ceph::Services::get_services_info('mon', $cfg, $rados);
-		$assert_mon_prerequisites->($cfg, $monhash, $monid, $ip);
+		$assert_mon_prerequisites->($cfg, $monhash, $monid, $ips);
 
 		my $client_keyring = PVE::Ceph::Tools::get_or_create_admin_keyring();
 		my $mon_keyring = PVE::Ceph::Tools::get_config('pve_mon_key_path');
@@ -260,22 +292,31 @@ __PACKAGE__->register_method ({
 		    run_command(['chown', 'ceph:ceph', $mondir]);
 
 		    my $is_first_address = !defined($rados);
-		    if (Net::IP::ip_is_ipv6($ip)) {
-			$cfg->{global}->{ms_bind_ipv6} = 'true';
-			$cfg->{global}->{ms_bind_ipv4} = 'false' if $is_first_address;
-		    } else {
-			$cfg->{global}->{ms_bind_ipv4} = 'true';
-			$cfg->{global}->{ms_bind_ipv6} = 'false' if $is_first_address;
-		    }
 
-		    my $monaddr = Net::IP::ip_is_ipv6($ip) ? "[$ip]" : $ip;
+		    my $monaddrs = [];
+
+		    for my $ip (@{$ips}) {
+			if (Net::IP::ip_is_ipv6($ip)) {
+			    $cfg->{global}->{ms_bind_ipv6} = 'true';
+			    $cfg->{global}->{ms_bind_ipv4} = 'false' if $is_first_address;
+			} else {
+			    $cfg->{global}->{ms_bind_ipv4} = 'true';
+			    $cfg->{global}->{ms_bind_ipv6} = 'false' if $is_first_address;
+			}
+
+			my $monaddr = Net::IP::ip_is_ipv6($ip) ? "[$ip]" : $ip;
+			push @{$monaddrs}, "v2:$monaddr:3300";
+			push @{$monaddrs}, "v1:$monaddr:6789";
+
+			$is_first_address = 0;
+		    }
 
 		    my $monmaptool_cmd = [
 			'monmaptool',
 			'--clobber',
 			'--addv',
 			$monid,
-			"[v2:$monaddr:3300,v1:$monaddr:6789]",
+			"[" . join(',', @{$monaddrs}) . "]",
 			'--print',
 			$monmap,
 		    ];
@@ -316,10 +357,10 @@ __PACKAGE__->register_method ({
 			$monhost .= " " . $monhash->{$mon}->{addr};
 		    }
 		}
-		$monhost .= " $ip";
+		$monhost .= " " . join(' ', @{$ips});
 		$cfg->{global}->{mon_host} = $monhost;
 		# The IP is needed in the ceph.conf for the first boot
-		$cfg->{$monsection}->{public_addr} = $ip;
+		$cfg->{$monsection}->{public_addr} = $ips->[0];
 
 		cfs_write_file('ceph.conf', $cfg);
 
@@ -414,11 +455,26 @@ __PACKAGE__->register_method ({
 		$monstat = $rados->mon_command({ prefix => 'quorum_status' });
 		$monlist = $monstat->{monmap}->{mons};
 
-		my $addr;
+		my $addrs = [];
+
+		my $add_addr = sub {
+		    my ($addr) = @_;
+
+		    # extract the ip without port and nonce (if present)
+		    ($addr) = $addr =~ m|^(.*):\d+(/\d+)?$|;
+		    push @{$addrs}, $addr;
+		};
+
 		for my $mon (@$monlist) {
 		    if ($mon->{name} eq $monid) {
-			$addr = $mon->{public_addr} // $mon->{addr};
-			($addr) = $addr =~ m|^(.*):\d+/\d+$|; # extract the ip without port/nonce
+			if ($mon->{public_addrs} && $mon->{public_addrs}->{addrvec}) {
+			    my $addrvec = $mon->{public_addrs}->{addrvec};
+			    for my $addr (@{$addrvec}) {
+				$add_addr->($addr->{addr});
+			    }
+			} else {
+			    $add_addr->($mon->{public_addr} // $mon->{addr});
+			}
 			last;
 		    }
 		}
@@ -433,23 +489,25 @@ __PACKAGE__->register_method ({
 
 		# delete from mon_host
 		if (my $monhost = $cfg->{global}->{mon_host}) {
-		    # various replaces to remove the ip
-		    # we always match the beginning or a separator (also at the end)
-		    # so we do not accidentally remove a wrong ip
-		    # e.g. removing 10.0.0.1 should not remove 10.0.0.101 or 110.0.0.1
-
-		    # remove vector containing this ip
-		    # format is [vX:ip:port/nonce,vY:ip:port/nonce]
-		    my $vectorpart_re = "v\\d+:\Q$addr\E:\\d+\\/\\d+";
-		    $monhost =~ s/(^|[ ,;]*)\[$vectorpart_re(?:,$vectorpart_re)*\](?:[ ,;]+|$)/$1/;
-
-		    # ip (+ port)
-		    $monhost =~ s/(^|[ ,;]+)\Q$addr\E(?::\d+)?(?:[ ,;]+|$)/$1/;
-
-		    # ipv6 only without brackets
-		    if ($addr =~ m/^\[?(.*?:.*?)\]?$/) {
-			$addr = $1;
-			$monhost =~ s/(^|[ ,;]+)\Q$addr\E(?:[ ,;]+|$)/$1/;
+		    for my $addr (@{$addrs}) {
+			# various replaces to remove the ip
+			# we always match the beginning or a separator (also at the end)
+			# so we do not accidentally remove a wrong ip
+			# e.g. removing 10.0.0.1 should not remove 10.0.0.101 or 110.0.0.1
+
+			# remove vector containing this ip
+			# format is [vX:ip:port/nonce,vY:ip:port/nonce]
+			my $vectorpart_re = "v\\d+:\Q$addr\E:\\d+\\/\\d+";
+			$monhost =~ s/(^|[ ,;]*)\[$vectorpart_re(?:,$vectorpart_re)*\](?:[ ,;]+|$)/$1/;
+
+			# ip (+ port)
+			$monhost =~ s/(^|[ ,;]+)\Q$addr\E(?::\d+)?(?:[ ,;]+|$)/$1/;
+
+			# ipv6 only without brackets
+			if ($addr =~ m/^\[?(.*?:.*?)\]?$/) {
+			    $addr = $1;
+			    $monhost =~ s/(^|[ ,;]+)\Q$addr\E(?:[ ,;]+|$)/$1/;
+			}
 		    }
 
 		    # remove trailing separators
-- 
2.20.1

Re: [pve-devel] [RFC manager] fix #2422: allow multiple Ceph public networks

[Dominik Csapak]

On 4/30/21 15:54, Fabian Ebner wrote:
> picked up from an old patch by Alwin[0].
> 
> The first four patches are cleanups/preparation.
> 
> The last patch is the big one which introduces the handling of multiple IP
> addresses. Quickly tested with a dual IPv4/IPv6 setup and an external client
> and didn't see any issues (altough I might've missed something in my struggle
> to get the network configuration right). It is a bit messy (not sure that is
> fully possible to avoid) and I'd like to test it some more, so sending it as an
> RFC. Would be great if somebody else could test it too.
> 
> Note that you also need a dual stack cluster network even if it's separate from
> the public network, so the OSDs will start up.
> 
> [0]: https://lists.proxmox.com/pipermail/pve-devel/2020-March/042304.html
> 
> Fabian Ebner (5):
>    api: ceph: mon: split up arguments for run_command
>    api: ceph: create mon: handle ms_bind_ipv* options more generally
>    api: ceph: create mon: factor out monmaptool command
>    api: ceph: create mon: explicitly add subsequent monitors to the
>      monmap
>    fix #2422: allow multiple Ceph public networks
> 
>   PVE/API2/Ceph/MON.pm | 228 +++++++++++++++++++++++++++++++------------
>   1 file changed, 163 insertions(+), 65 deletions(-)
> 

LGTM, and tested ok, only noticed one small thing

it is possible to give the same ip address twice,
this breaks the monitor (i cannot start it, but cannot destroy it either)

e.g.

pveceph mon create --mon-address '10.0.0.10,10.0.0.10'

it adds it with the ip twice to the monmap
this fails to start (because the port is bound already)
but destroying it fails with 'no such monitor id'

Re: [pve-devel] [RFC manager] fix #2422: allow multiple Ceph public networks

[Fabian Ebner]

Am 04.05.21 um 10:24 schrieb Dominik Csapak:
> On 4/30/21 15:54, Fabian Ebner wrote:
>> picked up from an old patch by Alwin[0].
>>
>> The first four patches are cleanups/preparation.
>>
>> The last patch is the big one which introduces the handling of 
>> multiple IP
>> addresses. Quickly tested with a dual IPv4/IPv6 setup and an external 
>> client
>> and didn't see any issues (altough I might've missed something in my 
>> struggle
>> to get the network configuration right). It is a bit messy (not sure 
>> that is
>> fully possible to avoid) and I'd like to test it some more, so sending 
>> it as an
>> RFC. Would be great if somebody else could test it too.
>>
>> Note that you also need a dual stack cluster network even if it's 
>> separate from
>> the public network, so the OSDs will start up.
>>
>> [0]: https://lists.proxmox.com/pipermail/pve-devel/2020-March/042304.html
>>
>> Fabian Ebner (5):
>>    api: ceph: mon: split up arguments for run_command
>>    api: ceph: create mon: handle ms_bind_ipv* options more generally
>>    api: ceph: create mon: factor out monmaptool command
>>    api: ceph: create mon: explicitly add subsequent monitors to the
>>      monmap
>>    fix #2422: allow multiple Ceph public networks
>>
>>   PVE/API2/Ceph/MON.pm | 228 +++++++++++++++++++++++++++++++------------
>>   1 file changed, 163 insertions(+), 65 deletions(-)
>>
> 
> LGTM, and tested ok, only noticed one small thing
> 
> it is possible to give the same ip address twice,
> this breaks the monitor (i cannot start it, but cannot destroy it either)
> 
> e.g.
> 
> pveceph mon create --mon-address '10.0.0.10,10.0.0.10'
> 
> it adds it with the ip twice to the monmap
> this fails to start (because the port is bound already)
> but destroying it fails with 'no such monitor id'
> 
> 

Thanks for testing, and nice find! I'll make sure to handle that in the 
next version.