[pve-devel] [PATCH cluster 3/5] Add verification and fallback to cluster join/addnode

[Stefan Reiter]

Verify that the config of the new node is valid and compatible with the
cluster (i.e. that the links for the new node match the currently
configured nodes).

Additionally, fallback is provided via a new parameter to addnode,
'new_node_ip'. Previously, fallback was handled on the joining node, by
setting it's local IP as 'link0', however, a cluster with only one link,
but numbered 1-7 is still valid, and a fallback is possible, but the old
code would now fail.

Instead, pass the locally resolved IP via a seperate parameter
(resolving the IP on the cluster side is impractical, as IP resolution
could fail or provide a wrong IP for Corosync).

Fallback fails in case the cluster has more than one link, in this case
only the user can know which NIC/IP corresponds to which cluster link.

Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
---
 data/PVE/API2/ClusterConfig.pm | 47 +++++++++++++++++++++++++++++++++-
 data/PVE/CLI/pvecm.pm          |  6 +++++
 data/PVE/Cluster/Setup.pm      |  6 +++++
 3 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/data/PVE/API2/ClusterConfig.pm b/data/PVE/API2/ClusterConfig.pm
index 241c14e..2d15f97 100644
--- a/data/PVE/API2/ClusterConfig.pm
+++ b/data/PVE/API2/ClusterConfig.pm
@@ -207,6 +207,12 @@ __PACKAGE__->register_method ({
 		description => "Do not throw error if node already exists.",
 		optional => 1,
 	    },
+	    new_node_ip => {
+		type => 'string',
+		description => "IP Address of node to add. Used as fallback if no links are given.",
+		format => 'ip',
+		optional => 1,
+	    },
 	}),
     },
     returns => {
@@ -268,6 +274,43 @@ __PACKAGE__->register_method ({
 		$check_duplicate_addr->($link);
 	    }
 
+	    # Make sure that the newly added node has links compatible with the
+	    # rest of the cluster. To start, extract all links that currently
+	    # exist. Check any node, all have the same links here (because of
+	    # verify_conf above).
+	    my $node_options = (values %$nodelist)[0];
+	    my $cluster_links = {};
+	    foreach my $opt (keys %$node_options) {
+		next if $opt !~ $PVE::Corosync::link_addr_re;
+		$cluster_links->{$2} = $node_options->{$opt};
+	    }
+
+	    if (%$links) {
+		# verify specified links exist and none are missing
+		for my $linknum (0..$PVE::Corosync::max_link_iter) {
+		    my $have_cluster = defined($cluster_links->{$linknum});
+		    my $have_new = defined($links->{$linknum});
+
+		    die "corosync: link $linknum exists in cluster config but wasn't specified for new node\n"
+			if $have_cluster && !$have_new;
+		    die "corosync: link $linknum specified for new node doesn't exist in cluster config\n"
+			if !$have_cluster && $have_new;
+		}
+	    } else {
+		# when called without any link parameters, fall back to
+		# new_node_ip, if all existing nodes only have a single link too
+		die "no links and no fallback ip (new_node_ip) given, cannot join cluster\n"
+		    if !$param->{new_node_ip};
+
+		if (%$cluster_links == 1) {
+		    my $linknum = (keys %$cluster_links)[0];
+		    $links->{$linknum} = { address => $param->{new_node_ip} };
+		} else {
+		    die "no links given but multiple links found on other nodes,"
+		      . " fallback only supported on single-link clusters\n";
+		}
+	    }
+
 	    if (defined(my $res = $nodelist->{$name})) {
 		$param->{nodeid} = $res->{nodeid} if !$param->{nodeid};
 		$param->{votes} = $res->{quorum_votes} if !defined($param->{votes});
@@ -481,7 +524,9 @@ __PACKAGE__->register_method ({
     path => 'join',
     method => 'POST',
     protected => 1,
-    description => "Joins this node into an existing cluster.",
+    description => "Joins this node into an existing cluster. If no links are"
+		 . " given, default to IP resolved by node's hostname on single"
+		 . " link (fallback fails for clusters with multiple links).",
     parameters => {
 	additionalProperties => 0,
 	properties => PVE::Corosync::add_corosync_link_properties({
diff --git a/data/PVE/CLI/pvecm.pm b/data/PVE/CLI/pvecm.pm
index 5856b56..bdae416 100755
--- a/data/PVE/CLI/pvecm.pm
+++ b/data/PVE/CLI/pvecm.pm
@@ -402,6 +402,12 @@ __PACKAGE__->register_method ({
 		    $links->{$link}, get_standard_option('corosync-link'));
 	    }
 
+	    # this will be used as fallback if no links are specified
+	    push @$cmd, '--new_node_ip', $local_ip_address;
+
+	    print "No local links given, will attempt fallback to $local_ip_address\n"
+		if !%$links;
+
 	    if (system (@$cmd) != 0) {
 		my $cmdtxt = join (' ', @$cmd);
 		die "unable to add node: command failed ($cmdtxt)\n";
diff --git a/data/PVE/Cluster/Setup.pm b/data/PVE/Cluster/Setup.pm
index e8c5eca..9850299 100644
--- a/data/PVE/Cluster/Setup.pm
+++ b/data/PVE/Cluster/Setup.pm
@@ -668,6 +668,12 @@ sub join {
 	$args->{"link$link"} = PVE::Corosync::print_corosync_link($links->{$link});
     }
 
+    # this will be used as fallback if no links are specified
+    $args->{new_node_ip} = $local_ip_address;
+
+    print "No local links given, will attempt fallback to $local_ip_address\n"
+	if !%$links;
+
     print "Request addition of this node\n";
     my $res = eval { $conn->post("/cluster/config/nodes/$nodename", $args); };
     if ($@) {
-- 
2.20.1