[pve-devel] [RFC cluster 5/6] node join: use new corosync link parameters
Fabian Grünbichler
f.gruenbichler at proxmox.com
Wed May 29 10:50:20 CEST 2019
On Tue, May 28, 2019 at 06:33:12PM +0200, Thomas Lamprecht wrote:
> Similar to the change to cluster creation use now also the
> corosync-link definition for the rest of the cluster join/add calls.
>
> What could be a bit strange is that bindnet shows here up too in the
> schema definition, while it's documented that it's only really used
> for cluster created we may want to "hide" it here, but that's
> details, and as long as we adapt the docs clearly and users can use
> the WebUI I think this can stay as is..
>
> As join and addnode is quite connected, do all in one patch.
>
> Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
> ---
> data/PVE/API2/ClusterConfig.pm | 45 ++++++++++++++++++++--------------
> data/PVE/CLI/pvecm.pm | 15 ++++++++----
> data/PVE/Cluster.pm | 18 ++++++++------
> 3 files changed, 48 insertions(+), 30 deletions(-)
>
> diff --git a/data/PVE/API2/ClusterConfig.pm b/data/PVE/API2/ClusterConfig.pm
> index 8b1fdbe..366f914 100644
> --- a/data/PVE/API2/ClusterConfig.pm
> +++ b/data/PVE/API2/ClusterConfig.pm
> @@ -214,8 +214,8 @@ __PACKAGE__->register_method ({
> description => "Do not throw error if node already exists.",
> optional => 1,
> },
> - ring0_addr => get_standard_option('corosync-ring0-addr'),
> - ring1_addr => get_standard_option('corosync-ring1-addr'),
> + link0 => get_standard_option('corosync-link'),
> + link1 => get_standard_option('corosync-link'),
> },
> },
> returns => {
> @@ -243,27 +243,36 @@ __PACKAGE__->register_method ({
>
> # ensure we do not reuse an address, that can crash the whole cluster!
> my $check_duplicate_addr = sub {
> - my $addr = shift;
> - return if !defined($addr);
> + my $link = shift;
> + return if !defined($link);
> + my $addr = $link->{address} // return;
>
> while (my ($k, $v) = each %$nodelist) {
> next if $k eq $name; # allows re-adding a node if force is set
> - if ($v->{ring0_addr} eq $addr || ($v->{ring1_addr} && $v->{ring1_addr} eq $addr)) {
> - die "corosync: address '$addr' already defined by node '$k'\n";
> +
> + for my $linknumber (0..1) {
> + my $id = "ring${linknumber}_addr";
> + next if !defined($v->{$id});
> +
> + die "corosync $id: address '$addr' already defined by node '$k'\n"
> + if $v->{$id} eq $addr;
> }
> }
> };
>
> - &$check_duplicate_addr($param->{ring0_addr});
> - &$check_duplicate_addr($param->{ring1_addr});
> + my $link0 = PVE::Cluster::parse_corosync_link($param->{link0});
> + my $link1 = PVE::Cluster::parse_corosync_link($param->{link1});
>
> - $param->{ring0_addr} = $name if !$param->{ring0_addr};
> + $check_duplicate_addr->($link0);
> + $check_duplicate_addr->($link1);
>
> - die "corosync: using 'ring1_addr' parameter needs a configured ring 1 interface!\n"
> - if $param->{ring1_addr} && !defined($totem_cfg->{interface}->{1});
> + $link0->{address} //= $name;
I wonder whether we should support clusters without link0 as well? might
happen easily:
cluster only has link0 configured
network topology change, add new link1
deconfigure link0
cluster joining no longer possible without specifying a bogus link0
address and manually cleaning up corosync.conf afterwards?
in general, we probably want to remove any special handling of link0/1,
and just set fallback defaults on link0 if no other link is specified..
> - die "corosync: ring 1 interface configured but 'ring1_addr' parameter not defined!\n"
> - if defined($totem_cfg->{interface}->{1}) && !defined($param->{ring1_addr});
> + die "corosync: using 'link1' parameter needs a interface with linknumber '1' configured!\n"
> + if $link1 && !defined($totem_cfg->{interface}->{1});
for completeness sake: this is no longer true (at least if you don't care about setting
priorities ;)).
it still might make sense to keep this semantics/check since it enables
the reverse safeguard below:
> +
> + die "corosync: totem interface with linknumber 1 configured but 'link1' parameter not defined!\n"
> + if defined($totem_cfg->{interface}->{1}) && !defined($link1);
^^^
that is to say - no additional links without additional interfaces, and
no nodes without additional links for all configured additional
interfaces.
>
> if (defined(my $res = $nodelist->{$name})) {
> $param->{nodeid} = $res->{nodeid} if !$param->{nodeid};
> @@ -301,11 +310,11 @@ __PACKAGE__->register_method ({
> warn $@ if $@;
>
> $nodelist->{$name} = {
> - ring0_addr => $param->{ring0_addr},
> + ring0_addr => $link0->{address},
> nodeid => $param->{nodeid},
> name => $name,
> };
> - $nodelist->{$name}->{ring1_addr} = $param->{ring1_addr} if $param->{ring1_addr};
> + $nodelist->{$name}->{ring1_addr} = $link1->{address} if defined($link1);
> $nodelist->{$name}->{quorum_votes} = $param->{votes} if $param->{votes};
>
> PVE::Cluster::log_msg('notice', 'root at pam', "adding node $name to cluster");
> @@ -414,7 +423,7 @@ __PACKAGE__->register_method ({
> properties => {
> name => get_standard_option('pve-node'),
> nodeid => get_standard_option('corosync-nodeid'),
> - ring0_addr => get_standard_option('corosync-ring0-addr'),
> + link0 => get_standard_option('corosync-link'),
> quorum_votes => { type => 'integer', minimum => 0 },
> pve_addr => { type => 'string', format => 'ip' },
> pve_fp => get_standard_option('fingerprint-sha256'),
> @@ -484,10 +493,10 @@ __PACKAGE__->register_method ({
> description => "Do not throw error if node already exists.",
> optional => 1,
> },
> - ring0_addr => get_standard_option('corosync-ring0-addr', {
> + link0 => get_standard_option('corosync-link', {
> default => "IP resolved by node's hostname",
> }),
> - ring1_addr => get_standard_option('corosync-ring1-addr'),
> + link1 => get_standard_option('corosync-link'),
> fingerprint => get_standard_option('fingerprint-sha256'),
> password => {
> description => "Superuser (root) password of peer node.",
> diff --git a/data/PVE/CLI/pvecm.pm b/data/PVE/CLI/pvecm.pm
> index e9055d4..823130a 100755
> --- a/data/PVE/CLI/pvecm.pm
> +++ b/data/PVE/CLI/pvecm.pm
> @@ -334,8 +334,8 @@ __PACKAGE__->register_method ({
> description => "Do not throw error if node already exists.",
> optional => 1,
> },
> - ring0_addr => get_standard_option('corosync-ring0-addr'),
> - ring1_addr => get_standard_option('corosync-ring1-addr'),
> + link0 => get_standard_option('corosync-link'),
> + link1 => get_standard_option('corosync-link'),
> fingerprint => get_standard_option('fingerprint-sha256', {
> optional => 1,
> }),
> @@ -356,7 +356,10 @@ __PACKAGE__->register_method ({
> my $host = $param->{hostname};
> my $local_ip_address = PVE::Cluster::remote_node_ip($nodename);
>
> - PVE::Cluster::assert_joinable($local_ip_address, $param->{ring0_addr}, $param->{ring1_addr}, $param->{force});
> + my $link0 = PVE::Cluster::parse_corosync_link($param->{link0});
> + my $link1 = PVE::Cluster::parse_corosync_link($param->{link1});
> +
> + PVE::Cluster::assert_joinable($local_ip_address, $link0, $link1, $param->{force});
>
> my $worker = sub {
>
> @@ -398,8 +401,10 @@ __PACKAGE__->register_method ({
>
> push @$cmd, '--nodeid', $param->{nodeid} if $param->{nodeid};
> push @$cmd, '--votes', $param->{votes} if defined($param->{votes});
> - push @$cmd, '--ring0_addr', $param->{ring0_addr} // $local_ip_address;
> - push @$cmd, '--ring1_addr', $param->{ring1_addr} if defined($param->{ring1_addr});
> + # 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});
>
> if (system (@$cmd) != 0) {
> my $cmdtxt = join (' ', @$cmd);
> diff --git a/data/PVE/Cluster.pm b/data/PVE/Cluster.pm
> index 8e611d9..085d2e6 100644
> --- a/data/PVE/Cluster.pm
> +++ b/data/PVE/Cluster.pm
> @@ -1877,7 +1877,7 @@ sub parse_corosync_link {
> }
>
> sub assert_joinable {
> - my ($local_addr, $ring0_addr, $ring1_addr, $force) = @_;
> + my ($local_addr, $link0, $link1, $force) = @_;
>
> my $errors = '';
> my $error = sub { $errors .= "* $_[0]\n"; };
> @@ -1920,8 +1920,8 @@ sub assert_joinable {
> };
>
> $check_ip->($local_addr, 'local node address');
> - $check_ip->($ring0_addr, 'ring0');
> - $check_ip->($ring1_addr, 'ring1');
> + $check_ip->($link0->{address}, 'ring0') if defined($link0);
> + $check_ip->($link1->{address}, 'ring1') if defined($link1);
>
> if ($errors) {
> warn "detected the following error(s):\n$errors";
> @@ -1961,9 +1961,11 @@ sub join {
> my $nodename = PVE::INotify::nodename();
> my $local_ip_address = remote_node_ip($nodename);
>
> - my ($ring0_addr, $ring1_addr) = $param->@{'ring0_addr', 'ring1_addr'};
> + my $link0 = parse_corosync_link($param->{link0});
> + my $link1 = parse_corosync_link($param->{link1});
> +
> # check if we can join with the given parameters and current node state
> - assert_joinable($local_ip_address, $ring0_addr, $ring1_addr, $param->{force});
> + assert_joinable($local_ip_address, $link0, $link1, $param->{force});
>
> setup_sshd_config();
> setup_rootsshconfig();
> @@ -2001,8 +2003,10 @@ 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});
> - $args->{ring0_addr} = $ring0_addr // $local_ip_address;
> - $args->{ring1_addr} = $ring1_addr if defined($ring1_addr);
> + # 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});
>
> print "Request addition of this node\n";
> my $res = $conn->post("/cluster/config/nodes/$nodename", $args);
> --
> 2.20.1
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
More information about the pve-devel
mailing list