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.rei...@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 | 20 ++++----
data/PVE/Cluster/Setup.pm | 22 +++++----
data/PVE/Corosync.pm | 89 +++++++++++++++++++++++-----------
4 files changed, 100 insertions(+), 82 deletions(-)
diff --git a/data/PVE/API2/ClusterConfig.pm b/data/PVE/API2/ClusterConfig.pm
index 3d778b8..241c14e 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_iter) {
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@pam', "adding node $name to
cluster");
PVE::Corosync::update_nodelist($conf, $nodelist);
@@ -495,7 +484,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."
@@ -512,17 +501,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 172ef8c..5856b56 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' },
@@ -355,8 +353,7 @@ __PACKAGE__->register_method ({
my $host = $param->{hostname};
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);
my $worker = sub {
@@ -381,7 +378,7 @@ __PACKAGE__->register_method ({
# allow fallback to old ssh only join if wished or needed
- 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 +396,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 5569d6c..e8c5eca 100644
--- a/data/PVE/Cluster/Setup.pm
+++ b/data/PVE/Cluster/Setup.pm
@@ -561,7 +561,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"; };
@@ -604,8 +604,12 @@ 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);
+
+ if ($links) {
+ foreach my $link (keys %$links) {
+ $check_ip->($links->{$link}->{address}, "link$link");
+ }
+ }
if ($errors) {
warn "detected the following error(s):\n$errors";
@@ -619,11 +623,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();
@@ -661,10 +664,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 5973aaf..69927ff 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,39 @@ 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_COUNT => 8;
+our $max_link_iter = MAX_LINK_COUNT - 1;
+
+sub add_corosync_link_properties {
+ my ($prop) = @_;
+
+ for my $lnum (0..$max_link_iter) {
+ $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_iter) {
+ $links->{$lnum} = parse_corosync_link($args->{"link$lnum"})
+ if $args->{"link$lnum"};
+ }
+
+ return $links;
+}
+
# a very simply parser ...
sub parse_conf {
my ($filename, $raw) = @_;
@@ -234,16 +270,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 = PVE::Cluster::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 => {
@@ -252,11 +291,8 @@ sub create_conf {
cluster_name => $clustername,
config_version => 0,
ip_version => 'ipv4-6',
- interface => {
- 0 => {
- linknumber => 0,
- },
- },
+ link_mode => 'passive',
+ interface => {},
},
nodelist => {
node => {
@@ -264,7 +300,6 @@ sub create_conf {
name => $nodename,
nodeid => $nodeid,
quorum_votes => $votes,
- ring0_addr => $link0->{address},
},
},
},
@@ -277,19 +312,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 = PVE::Cluster::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 +387,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_iter) {
my $have_interface = defined($interfaces->{$link});
foreach my $node (@node_names) {
my ($optname, $addr) = $find_option->($nodelist->{$node},
$link);
@@ -372,7 +405,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_iter) {
my $nodes_with_link = {};
foreach my $node (@node_names) {
my ($optname, $addr) = $find_option->($nodelist->{$node},
$link);
--
2.20.1
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
