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

[Stefan Reiter]

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      | 40 ++++++++++------------------------
 data/PVE/Corosync.pm           | 29 ++++++++++++++++++++++++
 3 files changed, 45 insertions(+), 34 deletions(-)

diff --git a/data/PVE/API2/ClusterConfig.pm b/data/PVE/API2/ClusterConfig.pm
index bff0c48..0454947 100644
--- a/data/PVE/API2/ClusterConfig.pm
+++ b/data/PVE/API2/ClusterConfig.pm
@@ -115,6 +115,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();
@@ -124,11 +129,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 c6f6c9e..77d3664 100644
--- a/data/PVE/Cluster/Setup.pm
+++ b/data/PVE/Cluster/Setup.pm
@@ -566,55 +566,37 @@ sub assert_joinable {
     my ($local_addr, $links, $force) = @_;
 
     my $errors = '';
-    my $error = sub { $errors .= "* $_[0]\n"; };
+    my $error = sub { $errors .= "* $_[0]"; };
 
     if (-f $authfile) {
-	$error->("authentication key '$authfile' already exists");
+	$error->("authentication key '$authfile' already exists\n");
     }
 
     if (-f $clusterconf)  {
-	$error->("cluster config '$clusterconf' already exists");
+	$error->("cluster config '$clusterconf' already exists\n");
     }
 
     my $vmlist = PVE::Cluster::get_vmlist();
     if ($vmlist && $vmlist->{ids} && scalar(keys %{$vmlist->{ids}})) {
-	$error->("this host already contains virtual guests");
+	$error->("this host already contains virtual guests\n");
     }
 
     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', it must be configured exactly once on local node!\n")
-	    if (scalar(@$configured_ips) != 1);
-    };
+	$error->("corosync is currently running, is this node already in a cluster?\n");
+    }
 
-    $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 69927ff..19c8976 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);
 
@@ -267,6 +268,32 @@ 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', it is not configured on local node (must exist exactly once)!\n"
+	if (scalar(@$configured_ips) < 1);
+
+    die "$logid: cannot use IP '$ip', it is configured multiple times on local node (must exist exactly once)!\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 {
@@ -317,6 +344,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};
-- 
2.20.1