[pve-devel] [PATCH v2 cluster 4/5] corosync: check if IPs are configured on cluster create

[Thomas Lamprecht]

On 12/4/19 3:04 PM, Stefan Reiter wrote:
> Same as we do in assert_joinable, a cluster with unconfigured IPs will
> fail start after creation anyway.
> 
> Make "check_ip" a standalone sub ("check_ip_configured") and improve
> error messages all around.
> 
> Also move call to create_conf up, so if it fails we exit before SSH and
> auth-key setup.
> 
> Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
> ---
>  data/PVE/API2/ClusterConfig.pm | 10 ++++-----
>  data/PVE/Cluster/Setup.pm      | 38 +++++++++++-----------------------
>  data/PVE/Corosync.pm           | 26 +++++++++++++++++++++++
>  3 files changed, 43 insertions(+), 31 deletions(-)

OK, but could be a separate patch, or the first one, as this is a bit out of
scope of the series, and could be already applied (but as positioned now it
conflicts as it depends on the previous context changes)

> 
> diff --git a/data/PVE/API2/ClusterConfig.pm b/data/PVE/API2/ClusterConfig.pm
> index 10dcd6d..64bebfe 100644
> --- a/data/PVE/API2/ClusterConfig.pm
> +++ b/data/PVE/API2/ClusterConfig.pm
> @@ -97,6 +97,11 @@ __PACKAGE__->register_method ({
>  	my $authuser = $rpcenv->get_user();
>  
>  	my $code = sub {
> +	    my $nodename = PVE::INotify::nodename();
> +
> +	    # get the corosync basis config for the new cluster
> +	    my $config = PVE::Corosync::create_conf($nodename, $param);
> +
>  	    STDOUT->autoflush();
>  	    PVE::Cluster::Setup::setup_sshd_config(1);
>  	    PVE::Cluster::Setup::setup_rootsshconfig();
> @@ -106,11 +111,6 @@ __PACKAGE__->register_method ({
>  		if !-f $authfile;
>  	    die "no authentication key available\n" if -f !$authfile;
>  
> -	    my $nodename = PVE::INotify::nodename();
> -
> -	    # get the corosync basis config for the new cluster
> -	    my $config = PVE::Corosync::create_conf($nodename, $param);
> -
>  	    print "Writing corosync config to /etc/pve/corosync.conf\n";
>  	    PVE::Corosync::atomic_write_conf($config);
>  
> diff --git a/data/PVE/Cluster/Setup.pm b/data/PVE/Cluster/Setup.pm
> index 8aa7f4b..6d880dd 100644
> --- a/data/PVE/Cluster/Setup.pm
> +++ b/data/PVE/Cluster/Setup.pm
> @@ -582,7 +582,11 @@ sub assert_joinable {
>      my ($local_addr, $links, $force) = @_;
>  
>      my $errors = '';
> -    my $error = sub { $errors .= "* $_[0]\n"; };
> +    my $error = sub {
> +	my ($msg) = @_;
> +	chomp $msg;
> +	$errors .= "* $msg\n";
> +    };
>  
>      if (-f $authfile) {
>  	$error->("authentication key '$authfile' already exists");
> @@ -598,39 +602,21 @@ sub assert_joinable {
>      }
>  
>      if (PVE::Tools::run_command(['corosync-quorumtool', '-l'], noerr => 1, quiet => 1) == 0) {
> -	$error->("corosync is already running, is this node already in a cluster?!");
> -    }
> -
> -    # check if corosync ring IPs are configured on the current nodes interfaces
> -    my $check_ip = sub {
> -	my $ip = shift // return;
> -	my $logid = shift;
> -	if (!PVE::JSONSchema::pve_verify_ip($ip, 1)) {
> -	    my $host = $ip;
> -	    eval { $ip = PVE::Network::get_ip_from_hostname($host); };
> -	    if ($@) {
> -		$error->("$logid: cannot use '$host': $@\n") ;
> -		return;
> -	    }
> -	}
> -
> -	my $cidr = (Net::IP::ip_is_ipv6($ip)) ? "$ip/128" : "$ip/32";
> -	my $configured_ips = PVE::Network::get_local_ip_from_cidr($cidr);
> -
> -	$error->("$logid: cannot use IP '$ip', not found on local node!\n")
> -	    if scalar(@$configured_ips) < 1;
> -    };
> +	$error->("corosync is currently running, is this node already in a cluster?");
> +    }
>  
> -    $check_ip->($local_addr, 'local node address');
> +    eval { PVE::Corosync::check_ip_configured($local_addr, 'local node address'); };
> +    $error->($@) if $@;
>  
>      if ($links) {
>  	foreach my $link (keys %$links) {
> -	    $check_ip->($links->{$link}->{address}, "link$link");
> +	    eval { PVE::Corosync::check_ip_configured($links->{$link}->{address}, "link$link"); };
> +	    $error->($@) if $@;
>  	}
>      }
>  
>      if ($errors) {
> -	warn "detected the following error(s):\n$errors";
> +	warn "Detected the following error(s):\n$errors";
>  	die "Check if node may join a cluster failed!\n" if !$force;
>      }
>  }
> diff --git a/data/PVE/Corosync.pm b/data/PVE/Corosync.pm
> index caec799..4ed2e5c 100644
> --- a/data/PVE/Corosync.pm
> +++ b/data/PVE/Corosync.pm
> @@ -11,6 +11,7 @@ use Socket qw(AF_INET AF_INET6 inet_ntop);
>  
>  use PVE::Cluster;
>  use PVE::JSONSchema;
> +use PVE::Network;
>  use PVE::Tools;
>  use PVE::Tools qw($IPV4RE $IPV6RE);
>  
> @@ -264,6 +265,29 @@ sub atomic_write_conf {
>  	|| die "activating corosync.conf.new failed - $!\n";
>  }
>  
> +# check if an IP address is configured exactly once on the local node, so that
> +# corosync can safely use it as a link
> +sub check_ip_configured {
> +    my ($ip, $logid) = @_;
> +
> +    return if !defined($ip);
> +    $logid //= "error";
> +
> +    if (!PVE::JSONSchema::pve_verify_ip($ip, 1)) {
> +	my $host = $ip;
> +	eval { $ip = PVE::Network::get_ip_from_hostname($host); };
> +	if ($@) {
> +	    die "$logid: cannot use '$host': $@\n";
> +	}
> +    }
> +
> +    my $cidr = (Net::IP::ip_is_ipv6($ip)) ? "$ip/128" : "$ip/32";
> +    my $configured_ips = PVE::Network::get_local_ip_from_cidr($cidr);
> +
> +    die "$logid: cannot use IP '$ip', not found on local node\n"
> +	if (scalar(@$configured_ips) < 1);
> +};
> +
>  # for creating a new cluster with the current node
>  # params are those from the API/CLI cluster create call
>  sub create_conf {
> @@ -314,6 +338,8 @@ sub create_conf {
>      foreach my $lnum (keys %$links) {
>  	my $link = $links->{$lnum};
>  
> +	check_ip_configured($link->{address}, "link$lnum");
> +
>  	$totem->{interface}->{$lnum} = { linknumber => $lnum };
>  
>  	my $prio = $link->{priority};
>