[pve-devel] [RFC cluster 3/6] cluster create: use new corosync-link format for totem interfaces

[Fabian Grünbichler]

On Tue, May 28, 2019 at 06:33:10PM +0200, Thomas Lamprecht wrote:
> Preperation for enhanced compatibillity with new corosync 3/knet
> transport. Pretty straight forward switch from ringX_addr to links,
> *but*, for configuration backward compatibillity corosync still uses
> ringX_addr as "link address", this will surely add confusion"link
> address", this will surely add confusion ...
> 
> Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
> ---
>  data/PVE/API2/ClusterConfig.pm | 16 ++-------------
>  data/PVE/Corosync.pm           | 37 +++++++++++++++++-----------------
>  2 files changed, 20 insertions(+), 33 deletions(-)
> 
> diff --git a/data/PVE/API2/ClusterConfig.pm b/data/PVE/API2/ClusterConfig.pm
> index e7142b5..8b1fdbe 100644
> --- a/data/PVE/API2/ClusterConfig.pm
> +++ b/data/PVE/API2/ClusterConfig.pm
> @@ -95,20 +95,8 @@ __PACKAGE__->register_method ({
>  		minimum => 1,
>  		optional => 1,
>  	    },
> -	    bindnet0_addr => {
> -		type => 'string', format => 'ip',
> -		description => "This specifies the network address the corosync ring 0".
> -		    " executive should bind to and defaults to the local IP address of the node.",
> -		optional => 1,
> -	    },
> -	    ring0_addr => get_standard_option('corosync-ring0-addr'),
> -	    bindnet1_addr => {
> -		type => 'string', format => 'ip',
> -		description => "This specifies the network address the corosync ring 1".
> -		    " executive should bind to and is optional.",
> -		optional => 1,
> -	    },
> -	    ring1_addr => get_standard_option('corosync-ring1-addr'),
> +	    link0 => get_standard_option('corosync-link'),
> +	    link1 => get_standard_option('corosync-link'),
>  	},
>      },
>      returns => { type => 'string' },
> diff --git a/data/PVE/Corosync.pm b/data/PVE/Corosync.pm
> index fea7258..8752c64 100644
> --- a/data/PVE/Corosync.pm
> +++ b/data/PVE/Corosync.pm
> @@ -202,12 +202,14 @@ sub create_conf {
>      my $votes = $param{votes} || 1;
>  
>      my $local_ip_address = PVE::Cluster::remote_node_ip($nodename);
> -    my $ring0_addr = $param{ring0_addr} // $local_ip_address;
> -    my $bindnet0_addr = $param{bindnet0_addr} // $ring0_addr;
>  
> -    my $use_ipv6 = ip_is_ipv6($ring0_addr);
> -    die "ring 0 addresses must be from same IP family!\n"
> -	if $use_ipv6 != ip_is_ipv6($bindnet0_addr);
> +    my $link0 = PVE::Cluster::parse_corosync_link($param{link0});
> +    $link0->{address} //= $local_ip_address;
> +    $link0->{bindnet} //= $local_ip_address;

as before, bindnet can be skipped IMHO.

> +
> +    my $use_ipv6 = ip_is_ipv6($link0->{address});
> +    die "link 0 addresses must be from same IP family!\n"
> +	if $use_ipv6 != ip_is_ipv6($link0->{bindnet});

same, and I'd argue for dropping $use_ipv6 altogether

>  
>      my $conf = {
>  	totem => {
> @@ -218,7 +220,7 @@ sub create_conf {
>  	    ip_version => $use_ipv6 ? 'ipv6' : 'ipv4',

and instead either set ip_version to 'ipv4-6' (ask DNS, prefer IPv4) or
leave it out altogether (which defaults to 'ipv6-4' for knet).

ip_is_ipv6 only works for actual IPs and not hostnames, so we have the
following scenarios:
$link0->{address} is an IPv4 address, we set ip_version to ipv4, but
there is nothing to resolve
$link0->{address} is an IPv6 address, we set ip_version to ipv6, but
there is nothing to resolve
$link0->{address} is a hostname, we set ip_version to ipv4, but that
hostname might only have an AAAA record set..

>  	    interface => {
>  		0 => {
> -		    bindnetaddr => $bindnet0_addr,
> +		    bindnetaddr => $link0->{bindnet},
>  		    linknumber => 0,
>  		},
>  	    },
> @@ -229,7 +231,7 @@ sub create_conf {
>  		    name => $nodename,
>  		    nodeid => $nodeid,
>  		    quorum_votes => $votes,
> -		    ring0_addr => $ring0_addr,
> +		    ring0_addr => $link0->{address},
>  		},
>  	    },
>  	},
> @@ -242,23 +244,20 @@ sub create_conf {
>  	},
>      };
>  
> -    die "Param bindnet1_addr set but ring1_addr not specified!\n"
> -	if (defined($param{bindnet1_addr}) && !defined($param{ring1_addr}));
> +    my $link1 = PVE::Cluster::parse_corosync_link($param{link1});
> +    $link1->{bindnet} //= $link1->{address};
>  
> -    my $ring1_addr = $param{ring1_addr};
> -    my $bindnet1_addr = $param{bindnet1_addr} // $param{ring1_addr};
> -
> -    if ($bindnet1_addr) {
> -	die "ring 1 addresses must be from same IP family as ring 0!\n"
> -	    if $use_ipv6 != ip_is_ipv6($bindnet1_addr) ||
> -	       $use_ipv6 != ip_is_ipv6($ring1_addr);
> +    if ($link1->{bindnet}) {
> +	die "link 1 addresses must be from same IP family as link 0!\n"
> +	    if $use_ipv6 != ip_is_ipv6($link1->{address}) ||
> +	       $use_ipv6 != ip_is_ipv6($link1->{bindnet});

even if we keep bindnet, this is no longer true for knet - the family
must only match on each link across all nodes, but not across links.

>  	$conf->{totem}->{interface}->{1} = {
> -	    bindnetaddr => $bindnet1_addr,
> +	    bindnetaddr => $link1->{bindnet},
>  	    linknumber => 1,
>  	};
> -	$conf->{totem}->{rrp_mode} = 'passive';
> -	$conf->{nodelist}->{node}->{$nodename}->{ring1_addr} = $ring1_addr;
> +	$conf->{totem}->{link_mode} = 'passive';
> +	$conf->{nodelist}->{node}->{$nodename}->{ring1_addr} = $link1->{address};
>      }
>  
>      return { main => $conf };
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel