[pve-devel] [PATCH v3 cluster 2/4] Enable support for up to 8 corosync links

Stefan Reiter s.reiter at proxmox.com
Thu Jan 9 16:31:34 CET 2020


add_corosync_link_properties/extract_corosync_link_args are introduced
as helpers to avoid hardcoding links in parameters=>properties on
several occasions, while still providing autocompletion with pvecm by
being seperate parameters instead of an array.

Maximum number of links is given as constant MAX_LINK_COUNT, should it
change in the future.

All necessary functions have been updated to
use the new $links array format instead of seperate $link0/$link1
parameters, and call sites changed accordingly.

Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
---

This patch intentionally removes some verification and fallback code to be
reintroduced in the next one.

 data/PVE/API2/ClusterConfig.pm | 51 +++++++-------------
 data/PVE/CLI/pvecm.pm          | 21 ++++----
 data/PVE/Cluster/Setup.pm      | 20 ++++----
 data/PVE/Corosync.pm           | 88 +++++++++++++++++++++++-----------
 4 files changed, 97 insertions(+), 83 deletions(-)

diff --git a/data/PVE/API2/ClusterConfig.pm b/data/PVE/API2/ClusterConfig.pm
index e05fd55..2c5bb03 100644
--- a/data/PVE/API2/ClusterConfig.pm
+++ b/data/PVE/API2/ClusterConfig.pm
@@ -68,10 +68,11 @@ __PACKAGE__->register_method ({
     path => '',
     method => 'POST',
     protected => 1,
-    description => "Generate new cluster configuration.",
+    description => "Generate new cluster configuration. If no links given,"
+		 . " default to local IP address as link0.",
     parameters => {
 	additionalProperties => 0,
-	properties => {
+	properties => PVE::Corosync::add_corosync_link_properties({
 	    clustername => {
 		description => "The name of the cluster.",
 		type => 'string', format => 'pve-node',
@@ -84,9 +85,7 @@ __PACKAGE__->register_method ({
 		minimum => 1,
 		optional => 1,
 	    },
-	    link0 => get_standard_option('corosync-link'),
-	    link1 => get_standard_option('corosync-link'),
-	},
+	}),
     },
     returns => { type => 'string' },
     code => sub {
@@ -110,7 +109,7 @@ __PACKAGE__->register_method ({
 	    my $nodename = PVE::INotify::nodename();
 
 	    # get the corosync basis config for the new cluster
-	    my $config = PVE::Corosync::create_conf($nodename, %$param);
+	    my $config = PVE::Corosync::create_conf($nodename, $param);
 
 	    print "Writing corosync config to /etc/pve/corosync.conf\n";
 	    PVE::Corosync::atomic_write_conf($config);
@@ -194,7 +193,7 @@ __PACKAGE__->register_method ({
     description => "Adds a node to the cluster configuration. This call is for internal use.",
     parameters => {
 	additionalProperties => 0,
-	properties => {
+	properties => PVE::Corosync::add_corosync_link_properties({
 	    node => get_standard_option('pve-node'),
 	    nodeid => get_standard_option('corosync-nodeid'),
 	    votes => {
@@ -208,9 +207,7 @@ __PACKAGE__->register_method ({
 		description => "Do not throw error if node already exists.",
 		optional => 1,
 	    },
-	    link0 => get_standard_option('corosync-link'),
-	    link1 => get_standard_option('corosync-link'),
-	},
+	}),
     },
     returns => {
 	type => "object",
@@ -256,7 +253,7 @@ __PACKAGE__->register_method ({
 		while (my ($k, $v) = each %$nodelist) {
 		    next if $k eq $name; # allows re-adding a node if force is set
 
-		    for my $linknumber (0..1) {
+		    for my $linknumber (0..PVE::Corosync::MAX_LINK_INDEX) {
 			my $id = "ring${linknumber}_addr";
 			next if !defined($v->{$id});
 
@@ -266,20 +263,10 @@ __PACKAGE__->register_method ({
 		}
 	    };
 
-	    my $link0 = PVE::Corosync::parse_corosync_link($param->{link0});
-	    my $link1 = PVE::Corosync::parse_corosync_link($param->{link1});
-
-	    $check_duplicate_addr->($link0);
-	    $check_duplicate_addr->($link1);
-
-	    # FIXME: handle all links (0-7), they're all independent now
-	    $link0->{address} //= $name if exists($totem_cfg->{interface}->{0});
-
-	    die "corosync: using 'link1' parameter needs a interface with linknumber '1' configured!\n"
-		if $link1 && !defined($totem_cfg->{interface}->{1});
-
-	    die "corosync: totem interface with linknumber 1 configured but 'link1' parameter not defined!\n"
-		if defined($totem_cfg->{interface}->{1}) && !defined($link1);
+	    my $links = PVE::Corosync::extract_corosync_link_args($param);
+	    foreach my $link (values %$links) {
+		$check_duplicate_addr->($link);
+	    }
 
 	    if (defined(my $res = $nodelist->{$name})) {
 		$param->{nodeid} = $res->{nodeid} if !$param->{nodeid};
@@ -317,13 +304,15 @@ __PACKAGE__->register_method ({
 	    warn $@ if $@;
 
 	    $nodelist->{$name} = {
-		ring0_addr => $link0->{address},
 		nodeid => $param->{nodeid},
 		name => $name,
 	    };
-	    $nodelist->{$name}->{ring1_addr} = $link1->{address} if defined($link1);
 	    $nodelist->{$name}->{quorum_votes} = $param->{votes} if $param->{votes};
 
+	    foreach my $link (keys %$links) {
+		$nodelist->{$name}->{"ring${link}_addr"} = $links->{$link}->{address};
+	    }
+
 	    PVE::Cluster::log_msg('notice', 'root at pam', "adding node $name to cluster");
 
 	    PVE::Corosync::update_nodelist($conf, $nodelist);
@@ -508,7 +497,7 @@ __PACKAGE__->register_method ({
     description => "Joins this node into an existing cluster.",
     parameters => {
 	additionalProperties => 0,
-	properties => {
+	properties => PVE::Corosync::add_corosync_link_properties({
 	    hostname => {
 		type => 'string',
 		description => "Hostname (or IP) of an existing cluster member."
@@ -525,17 +514,13 @@ __PACKAGE__->register_method ({
 		description => "Do not throw error if node already exists.",
 		optional => 1,
 	    },
-	    link0 => get_standard_option('corosync-link', {
-		default => "IP resolved by node's hostname",
-	    }),
-	    link1 => get_standard_option('corosync-link'),
 	    fingerprint => get_standard_option('fingerprint-sha256'),
 	    password => {
 		description => "Superuser (root) password of peer node.",
 		type => 'string',
 		maxLength => 128,
 	    },
-	},
+	}),
     },
     returns => { type => 'string' },
     code => sub {
diff --git a/data/PVE/CLI/pvecm.pm b/data/PVE/CLI/pvecm.pm
index c634c9b..df0b1b9 100755
--- a/data/PVE/CLI/pvecm.pm
+++ b/data/PVE/CLI/pvecm.pm
@@ -316,7 +316,7 @@ __PACKAGE__->register_method ({
     description => "Adds the current node to an existing cluster.",
     parameters => {
     	additionalProperties => 0,
-	properties => {
+	properties => PVE::Corosync::add_corosync_link_properties({
 	    hostname => {
 		type => 'string',
 		description => "Hostname (or IP) of an existing cluster member."
@@ -333,8 +333,6 @@ __PACKAGE__->register_method ({
 		description => "Do not throw error if node already exists.",
 		optional => 1,
 	    },
-	    link0 => get_standard_option('corosync-link'),
-	    link1 => get_standard_option('corosync-link'),
 	    fingerprint => get_standard_option('fingerprint-sha256', {
 		optional => 1,
 	    }),
@@ -343,7 +341,7 @@ __PACKAGE__->register_method ({
 		description => "Always use SSH to join, even if peer may do it over API.",
 		optional => 1,
 	    },
-	},
+	}),
     },
     returns => { type => 'null' },
 
@@ -377,11 +375,9 @@ __PACKAGE__->register_method ({
 	    # allow fallback to old ssh only join if wished or needed
 
 	    my $local_ip_address = PVE::Cluster::remote_node_ip($nodename);
+	    my $links = PVE::Corosync::extract_corosync_link_args($param);
 
-	    my $link0 = PVE::Corosync::parse_corosync_link($param->{link0});
-	    my $link1 = PVE::Corosync::parse_corosync_link($param->{link1});
-
-	    PVE::Cluster::Setup::assert_joinable($local_ip_address, $link0, $link1, $param->{force});
+	    PVE::Cluster::Setup::assert_joinable($local_ip_address, $links, $param->{force});
 
 	    PVE::Cluster::Setup::setup_sshd_config();
 	    PVE::Cluster::Setup::setup_rootsshconfig();
@@ -399,10 +395,11 @@ __PACKAGE__->register_method ({
 
 	    push @$cmd, '--nodeid', $param->{nodeid} if $param->{nodeid};
 	    push @$cmd, '--votes', $param->{votes} if defined($param->{votes});
-	    # just pass the un-parsed string through, or as we've address as
-	    # the default_key, we can just pass the fallback directly too
-	    push @$cmd, '--link0', $param->{link0} // $local_ip_address;
-	    push @$cmd, '--link1', $param->{link1} if defined($param->{link1});
+
+	    foreach my $link (keys %$links) {
+		push @$cmd, "--link$link", PVE::JSONSchema::print_property_string(
+		    $links->{$link}, get_standard_option('corosync-link'));
+	    }
 
 	    if (system (@$cmd) != 0) {
 		my $cmdtxt = join (' ', @$cmd);
diff --git a/data/PVE/Cluster/Setup.pm b/data/PVE/Cluster/Setup.pm
index b115d45..88e1680 100644
--- a/data/PVE/Cluster/Setup.pm
+++ b/data/PVE/Cluster/Setup.pm
@@ -579,7 +579,7 @@ sub gen_pve_vzdump_files {
 # join helpers
 
 sub assert_joinable {
-    my ($local_addr, $link0, $link1, $force) = @_;
+    my ($local_addr, $links, $force) = @_;
 
     my $errors = '';
     my $error = sub { $errors .= "* $_[0]\n"; };
@@ -622,8 +622,10 @@ sub assert_joinable {
     };
 
     $check_ip->($local_addr, 'local node address');
-    $check_ip->($link0->{address}, 'ring0') if defined($link0);
-    $check_ip->($link1->{address}, 'ring1') if defined($link1);
+
+    foreach my $link (keys %$links) {
+	$check_ip->($links->{$link}->{address}, "link$link");
+    }
 
     if ($errors) {
 	warn "detected the following error(s):\n$errors";
@@ -637,11 +639,10 @@ sub join {
     my $nodename = PVE::INotify::nodename();
     my $local_ip_address = PVE::Cluster::remote_node_ip($nodename);
 
-    my $link0 = PVE::Corosync::parse_corosync_link($param->{link0});
-    my $link1 = PVE::Corosync::parse_corosync_link($param->{link1});
+    my $links = PVE::Corosync::extract_corosync_link_args($param);
 
     # check if we can join with the given parameters and current node state
-    assert_joinable($local_ip_address, $link0, $link1, $param->{force});
+    assert_joinable($local_ip_address, $links, $param->{force});
 
     setup_sshd_config();
     setup_rootsshconfig();
@@ -679,10 +680,9 @@ sub join {
     $args->{force} = $param->{force} if defined($param->{force});
     $args->{nodeid} = $param->{nodeid} if $param->{nodeid};
     $args->{votes} = $param->{votes} if defined($param->{votes});
-    # just pass the un-parsed string through, or as we've address as the
-    # default_key, we can just pass the fallback directly too
-    $args->{link0} = $param->{link0} // $local_ip_address;
-    $args->{link1} = $param->{link1} if defined($param->{link1});
+    foreach my $link (keys %$links) {
+	$args->{"link$link"} = PVE::Corosync::print_corosync_link($links->{$link});
+    }
 
     print "Request addition of this node\n";
     my $res = eval { $conn->post("/cluster/config/nodes/$nodename", $args); };
diff --git a/data/PVE/Corosync.pm b/data/PVE/Corosync.pm
index c69c8d8..2eb38b8 100644
--- a/data/PVE/Corosync.pm
+++ b/data/PVE/Corosync.pm
@@ -35,12 +35,15 @@ my $corosync_link_format = {
 	minimum => 0,
 	maximum => 255,
 	default => 0,
-	description => "The priority for the link when knet is used in 'passive' mode. Lower value means higher priority.",
+	description => "The priority for the link when knet is used in 'passive'"
+		     . " mode (default). Lower value means higher priority. Only"
+		     . " valid for cluster create, ignored on node add.",
     },
 };
 my $corosync_link_desc = {
     type => 'string', format => $corosync_link_format,
-    description => "Address and priority information of a single corosync link.",
+    description => "Address and priority information of a single corosync link."
+		 . " (up to 8 links supported; link0..link7)",
     optional => 1,
 };
 PVE::JSONSchema::register_standard_option("corosync-link", $corosync_link_desc);
@@ -53,6 +56,38 @@ sub parse_corosync_link {
     return PVE::JSONSchema::parse_property_string($corosync_link_format, $value);
 }
 
+sub print_corosync_link {
+    my ($link) = @_;
+
+    return undef if !defined($link);
+
+    return PVE::JSONSchema::print_property_string($link, $corosync_link_format);
+}
+
+use constant MAX_LINK_INDEX => 7;
+
+sub add_corosync_link_properties {
+    my ($prop) = @_;
+
+    for my $lnum (0..MAX_LINK_INDEX) {
+	$prop->{"link$lnum"} = PVE::JSONSchema::get_standard_option("corosync-link");
+    }
+
+    return $prop;
+}
+
+sub extract_corosync_link_args {
+    my ($args) = @_;
+
+    my $links = {};
+    for my $lnum (0..MAX_LINK_INDEX) {
+	$links->{$lnum} = parse_corosync_link($args->{"link$lnum"})
+	    if $args->{"link$lnum"};
+    }
+
+    return $links;
+}
+
 # a very simply parser ...
 sub parse_conf {
     my ($filename, $raw) = @_;
@@ -232,16 +267,19 @@ sub atomic_write_conf {
 # for creating a new cluster with the current node
 # params are those from the API/CLI cluster create call
 sub create_conf {
-    my ($nodename, %param) = @_;
+    my ($nodename, $param) = @_;
 
-    my $clustername = $param{clustername};
-    my $nodeid = $param{nodeid} || 1;
-    my $votes = $param{votes} || 1;
+    my $clustername = $param->{clustername};
+    my $nodeid = $param->{nodeid} || 1;
+    my $votes = $param->{votes} || 1;
 
     my $local_ip_address = PVE::Cluster::remote_node_ip($nodename);
 
-    my $link0 = parse_corosync_link($param{link0});
-    $link0->{address} //= $local_ip_address;
+    my $links = extract_corosync_link_args($param);
+
+    # if no links given, fall back to local IP as link0
+    $links->{0} = { address => $local_ip_address }
+	if !%$links;
 
     my $conf = {
 	totem => {
@@ -250,11 +288,8 @@ sub create_conf {
 	    cluster_name => $clustername,
 	    config_version => 0,
 	    ip_version => 'ipv4-6',
-	    interface => {
-		0 => {
-		    linknumber => 0,
-		},
-	    },
+	    link_mode => 'passive',
+	    interface => {},
 	},
 	nodelist => {
 	    node => {
@@ -262,7 +297,6 @@ sub create_conf {
 		    name => $nodename,
 		    nodeid => $nodeid,
 		    quorum_votes => $votes,
-		    ring0_addr => $link0->{address},
 		},
 	    },
 	},
@@ -275,19 +309,17 @@ sub create_conf {
 	},
     };
     my $totem = $conf->{totem};
+    my $node = $conf->{nodelist}->{node}->{$nodename};
+
+    foreach my $lnum (keys %$links) {
+	my $link = $links->{$lnum};
+
+	$totem->{interface}->{$lnum} = { linknumber => $lnum };
+
+	my $prio = $link->{priority};
+	$totem->{interface}->{$lnum}->{knet_link_priority} = $prio if $prio;
 
-    $totem->{interface}->{0}->{knet_link_priority} = $link0->{priority}
-	if defined($link0->{priority});
-
-    my $link1 = parse_corosync_link($param{link1});
-    if ($link1->{address}) {
-	$conf->{totem}->{interface}->{1} = {
-	    linknumber => 1,
-	};
-	$totem->{link_mode} = 'passive';
-	$totem->{interface}->{1}->{knet_link_priority} = $link1->{priority}
-	    if defined($link1->{priority});
-	$conf->{nodelist}->{node}->{$nodename}->{ring1_addr} = $link1->{address};
+	$node->{"ring${lnum}_addr"} = $link->{address};
     }
 
     return { main => $conf };
@@ -354,7 +386,7 @@ sub verify_conf {
     if (%$interfaces) {
 	# if interfaces are defined, *all* links must have a matching interface
 	# definition, and vice versa
-	for my $link (0..1) {
+	for my $link (0..MAX_LINK_INDEX) {
 	    my $have_interface = defined($interfaces->{$link});
 	    foreach my $node (@node_names) {
 		my $linkdef = $node_links->{$node}->{$link};
@@ -374,7 +406,7 @@ sub verify_conf {
 	}
     } else {
 	# without interfaces, only check that links are consistent among nodes
-	for my $link (0..1) {
+	for my $link (0..MAX_LINK_INDEX) {
 	    my $nodes_with_link = {};
 	    foreach my $node (@node_names) {
 		my $linkdef = $node_links->{$node}->{$link};
-- 
2.20.1




More information about the pve-devel mailing list