From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 87A5A7876C for ; Fri, 30 Apr 2021 15:54:45 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 5EB8F9775 for ; Fri, 30 Apr 2021 15:54:45 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 4525196FE for ; Fri, 30 Apr 2021 15:54:43 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 206C546439 for ; Fri, 30 Apr 2021 15:54:43 +0200 (CEST) From: Fabian Ebner To: pve-devel@lists.proxmox.com Date: Fri, 30 Apr 2021 15:54:37 +0200 Message-Id: <20210430135437.4816-6-f.ebner@proxmox.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20210430135437.4816-1-f.ebner@proxmox.com> References: <20210430135437.4816-1-f.ebner@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.000 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [mon.pm] Subject: [pve-devel] [RFC manager 5/5] fix #2422: allow multiple Ceph public networks X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 30 Apr 2021 13:54:45 -0000 From: Alwin Antreich 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 [handling of multiple addresses] Signed-off-by: Fabian Ebner --- 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