[pve-devel] [RFC cluster v2 01/10] move addnode/delnode from CLI to cluster config API
Fabian Grünbichler
f.gruenbichler at proxmox.com
Wed Dec 6 16:01:46 CET 2017
high-level: not sure if those operations should really run sync? maybe
forking a worker just to make sure is a good idea.. also would create a
task log entry and record potential warnings outside of the journal ;)
On Mon, Dec 04, 2017 at 12:11:08PM +0100, Thomas Lamprecht wrote:
> Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
> ---
> data/PVE/API2/ClusterConfig.pm | 210 ++++++++++++++++++++++++++++++++++++++
> data/PVE/CLI/pvecm.pm | 223 +----------------------------------------
> 2 files changed, 213 insertions(+), 220 deletions(-)
>
> diff --git a/data/PVE/API2/ClusterConfig.pm b/data/PVE/API2/ClusterConfig.pm
> index 46db3da..fa01022 100644
> --- a/data/PVE/API2/ClusterConfig.pm
> +++ b/data/PVE/API2/ClusterConfig.pm
> @@ -76,6 +76,216 @@ __PACKAGE__->register_method({
> return PVE::RESTHandler::hash_to_array($nodelist, 'node');
> }});
>
> +# lock method to ensure local and cluster wide atomicity
> +# if we're a single node cluster just lock locally, we have no other cluster
> +# node which we could contend with, else also acquire a cluster wide lock
> +my $config_change_lock = sub {
> + my ($code) = @_;
> +
> + my $local_lock_fn = "/var/lock/pvecm.lock";
> + PVE::Tools::lock_file($local_lock_fn, 10, sub {
> + PVE::Cluster::cfs_update(1);
> + my $members = PVE::Cluster::get_members();
> + if (scalar(keys %$members) > 1) {
> + return PVE::Cluster::cfs_lock_file('corosync.conf', 10, $code);
> + } else {
> + return $code->();
> + }
> + });
> +};
> +
> +
> +__PACKAGE__->register_method ({
> + name => 'addnode',
> + path => 'nodes',
> + method => 'POST',
> + protected => 1,
> + description => "Adds a node to the cluster configuration.",
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + node => PVE::JSONSchema::get_standard_option('pve-node'),
> + nodeid => {
> + type => 'integer',
> + description => "Node id for this node.",
> + minimum => 1,
> + optional => 1,
> + },
> + votes => {
> + type => 'integer',
> + description => "Number of votes for this node",
> + minimum => 0,
> + optional => 1,
> + },
> + force => {
> + type => 'boolean',
> + description => "Do not throw error if node already exists.",
> + optional => 1,
> + },
> + ring0_addr => {
> + type => 'string', format => 'address',
> + description => "Hostname (or IP) of the corosync ring0 address of this node.".
> + " Defaults to nodes hostname.",
> + optional => 1,
> + },
> + ring1_addr => {
> + type => 'string', format => 'address',
> + description => "Hostname (or IP) of the corosync ring1 address, this".
> + " needs an valid bindnet1_addr.",
> + optional => 1,
> + },
> + },
> + },
> + returns => { type => 'null' },
> + code => sub {
> + my ($param) = @_;
> +
> + PVE::Cluster::check_cfs_quorum();
> +
> + my $code = sub {
> + my $conf = PVE::Cluster::cfs_read_file("corosync.conf");
> + my $nodelist = PVE::Corosync::nodelist($conf);
> + my $totem_cfg = PVE::Corosync::totem_config($conf);
> +
> + my $name = $param->{node};
> +
> + # 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);
> +
> + 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";
> + }
> + }
> + };
> +
> + &$check_duplicate_addr($param->{ring0_addr});
> + &$check_duplicate_addr($param->{ring1_addr});
> +
> + $param->{ring0_addr} = $name if !$param->{ring0_addr};
> +
> + die "corosync: using 'ring1_addr' parameter needs a configured ring 1 interface!\n"
> + if $param->{ring1_addr} && !defined($totem_cfg->{interface}->{1});
> +
> + die "corosync: ring 1 interface configured but 'ring1_addr' parameter not defined!\n"
> + if defined($totem_cfg->{interface}->{1}) && !defined($param->{ring1_addr});
> +
> + if (defined(my $res = $nodelist->{$name})) {
> + $param->{nodeid} = $res->{nodeid} if !$param->{nodeid};
> + $param->{votes} = $res->{quorum_votes} if !defined($param->{votes});
> +
> + if ($res->{quorum_votes} == $param->{votes} &&
> + $res->{nodeid} == $param->{nodeid} && $param->{force}) {
> + print "forcing overwrite of configured node '$name'\n";
> + } else {
> + die "can't add existing node '$name'\n";
> + }
> + } elsif (!$param->{nodeid}) {
> + my $nodeid = 1;
> +
> + while(1) {
> + my $found = 0;
> + foreach my $v (values %$nodelist) {
> + if ($v->{nodeid} eq $nodeid) {
> + $found = 1;
> + $nodeid++;
> + last;
> + }
> + }
> + last if !$found;
> + };
> +
> + $param->{nodeid} = $nodeid;
> + }
> +
> + $param->{votes} = 1 if !defined($param->{votes});
> +
> + PVE::Cluster::gen_local_dirs($name);
> +
> + eval { PVE::Cluster::ssh_merge_keys(); };
> + warn $@ if $@;
> +
> + $nodelist->{$name} = {
> + ring0_addr => $param->{ring0_addr},
> + nodeid => $param->{nodeid},
> + name => $name,
> + };
> + $nodelist->{$name}->{ring1_addr} = $param->{ring1_addr} if $param->{ring1_addr};
> + $nodelist->{$name}->{quorum_votes} = $param->{votes} if $param->{votes};
> +
> + PVE::Corosync::update_nodelist($conf, $nodelist);
> + };
> +
> + $config_change_lock->($code);
> + die $@ if $@;
> +
> + return undef;
> + }});
> +
> +
> +__PACKAGE__->register_method ({
> + name => 'delnode',
> + path => 'nodes',
> + method => 'DELETE',
> + protected => 1,
> + description => "Removes a node from the cluster configuration.",
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + node => {
> + type => 'string',
> + description => "Hostname or IP of the corosync ring0 address of this node.",
> + },
> + },
> + },
> + returns => { type => 'null' },
> + code => sub {
> + my ($param) = @_;
> +
> + my $local_node = PVE::INotify::nodename();
> + die "Cannot delete myself from cluster!\n" if $param->{node} eq $local_node;
> +
> + PVE::Cluster::check_cfs_quorum();
> +
> + my $code = sub {
> + my $conf = PVE::Cluster::cfs_read_file("corosync.conf");
> + my $nodelist = PVE::Corosync::nodelist($conf);
> +
> + my $node;
> + my $nodeid;
> +
> + foreach my $tmp_node (keys %$nodelist) {
> + my $d = $nodelist->{$tmp_node};
> + my $ring0_addr = $d->{ring0_addr};
> + my $ring1_addr = $d->{ring1_addr};
> + if (($tmp_node eq $param->{node}) ||
> + (defined($ring0_addr) && ($ring0_addr eq $param->{node})) ||
> + (defined($ring1_addr) && ($ring1_addr eq $param->{node}))) {
> + $node = $tmp_node;
> + $nodeid = $d->{nodeid};
> + last;
> + }
> + }
> +
> + die "Node/IP: $param->{node} is not a known host of the cluster.\n"
> + if !defined($node);
> +
> + delete $nodelist->{$node};
> +
> + PVE::Corosync::update_nodelist($conf, $nodelist);
> +
> + PVE::Tools::run_command(['corosync-cfgtool','-k', $nodeid]) if defined($nodeid);
> + };
> +
> + $config_change_lock->($code);
> + die $@ if $@;
> +
> + return undef;
> + }});
> +
> __PACKAGE__->register_method({
> name => 'totem',
> path => 'totem',
> diff --git a/data/PVE/CLI/pvecm.pm b/data/PVE/CLI/pvecm.pm
> index 6d0d704..a92f831 100755
> --- a/data/PVE/CLI/pvecm.pm
> +++ b/data/PVE/CLI/pvecm.pm
> @@ -11,6 +11,7 @@ use PVE::Cluster;
> use PVE::INotify;
> use PVE::JSONSchema;
> use PVE::CLIHandler;
> +use PVE::API2::ClusterConfig;
> use PVE::Corosync;
>
> use base qw(PVE::CLIHandler);
> @@ -58,23 +59,6 @@ sub backup_database {
> }
> }
>
> -# lock method to ensure local and cluster wide atomicity
> -# if we're a single node cluster just lock locally, we have no other cluster
> -# node which we could contend with, else also acquire a cluster wide lock
> -my $config_change_lock = sub {
> - my ($code) = @_;
> -
> - my $local_lock_fn = "/var/lock/pvecm.lock";
> - PVE::Tools::lock_file($local_lock_fn, 10, sub {
> - PVE::Cluster::cfs_update(1);
> - my $members = PVE::Cluster::get_members();
> - if (scalar(keys %$members) > 1) {
> - return PVE::Cluster::cfs_lock_file('corosync.conf', 10, $code);
> - } else {
> - return $code->();
> - }
> - });
> -};
>
> __PACKAGE__->register_method ({
> name => 'keygen',
> @@ -263,207 +247,6 @@ _EOD
> }});
>
> __PACKAGE__->register_method ({
> - name => 'addnode',
> - path => 'addnode',
> - method => 'PUT',
> - description => "Adds a node to the cluster configuration.",
> - parameters => {
> - additionalProperties => 0,
> - properties => {
> - node => PVE::JSONSchema::get_standard_option('pve-node'),
> - nodeid => {
> - type => 'integer',
> - description => "Node id for this node.",
> - minimum => 1,
> - optional => 1,
> - },
> - votes => {
> - type => 'integer',
> - description => "Number of votes for this node",
> - minimum => 0,
> - optional => 1,
> - },
> - force => {
> - type => 'boolean',
> - description => "Do not throw error if node already exists.",
> - optional => 1,
> - },
> - ring0_addr => {
> - type => 'string', format => 'address',
> - description => "Hostname (or IP) of the corosync ring0 address of this node.".
> - " Defaults to nodes hostname.",
> - optional => 1,
> - },
> - ring1_addr => {
> - type => 'string', format => 'address',
> - description => "Hostname (or IP) of the corosync ring1 address, this".
> - " needs an valid bindnet1_addr.",
> - optional => 1,
> - },
> - },
> - },
> - returns => { type => 'null' },
> -
> - code => sub {
> - my ($param) = @_;
> -
> - if (!$param->{force} && (-t STDIN || -t STDOUT)) {
> - die "error: `addnode` should not get called interactively!\nUse ".
> - "`pvecm add <cluster-node>` to add a node to a cluster!\n";
> - }
> -
> - PVE::Cluster::check_cfs_quorum();
> -
> - my $code = sub {
> - my $conf = PVE::Cluster::cfs_read_file("corosync.conf");
> - my $nodelist = PVE::Corosync::nodelist($conf);
> - my $totem_cfg = PVE::Corosync::totem_config($conf);
> -
> - my $name = $param->{node};
> -
> - # 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);
> -
> - 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";
> - }
> - }
> - };
> -
> - &$check_duplicate_addr($param->{ring0_addr});
> - &$check_duplicate_addr($param->{ring1_addr});
> -
> - $param->{ring0_addr} = $name if !$param->{ring0_addr};
> -
> - die "corosync: using 'ring1_addr' parameter needs a configured ring 1 interface!\n"
> - if $param->{ring1_addr} && !defined($totem_cfg->{interface}->{1});
> -
> - die "corosync: ring 1 interface configured but 'ring1_addr' parameter not defined!\n"
> - if defined($totem_cfg->{interface}->{1}) && !defined($param->{ring1_addr});
> -
> - if (defined(my $res = $nodelist->{$name})) {
> - $param->{nodeid} = $res->{nodeid} if !$param->{nodeid};
> - $param->{votes} = $res->{quorum_votes} if !defined($param->{votes});
> -
> - if ($res->{quorum_votes} == $param->{votes} &&
> - $res->{nodeid} == $param->{nodeid}) {
> - print "node $name already defined\n";
> - if ($param->{force}) {
> - exit (0);
> - } else {
> - exit (-1);
> - }
> - } else {
> - die "can't add existing node\n";
> - }
> - } elsif (!$param->{nodeid}) {
> - my $nodeid = 1;
> -
> - while(1) {
> - my $found = 0;
> - foreach my $v (values %$nodelist) {
> - if ($v->{nodeid} eq $nodeid) {
> - $found = 1;
> - $nodeid++;
> - last;
> - }
> - }
> - last if !$found;
> - };
> -
> - $param->{nodeid} = $nodeid;
> - }
> -
> - $param->{votes} = 1 if !defined($param->{votes});
> -
> - PVE::Cluster::gen_local_dirs($name);
> -
> - eval { PVE::Cluster::ssh_merge_keys(); };
> - warn $@ if $@;
> -
> - $nodelist->{$name} = {
> - ring0_addr => $param->{ring0_addr},
> - nodeid => $param->{nodeid},
> - name => $name,
> - };
> - $nodelist->{$name}->{ring1_addr} = $param->{ring1_addr} if $param->{ring1_addr};
> - $nodelist->{$name}->{quorum_votes} = $param->{votes} if $param->{votes};
> -
> - PVE::Corosync::update_nodelist($conf, $nodelist);
> - };
> -
> - $config_change_lock->($code);
> - die $@ if $@;
> -
> - exit (0);
> - }});
> -
> -
> -__PACKAGE__->register_method ({
> - name => 'delnode',
> - path => 'delnode',
> - method => 'PUT',
> - description => "Removes a node to the cluster configuration.",
> - parameters => {
> - additionalProperties => 0,
> - properties => {
> - node => {
> - type => 'string',
> - description => "Hostname or IP of the corosync ring0 address of this node.",
> - },
> - },
> - },
> - returns => { type => 'null' },
> -
> - code => sub {
> - my ($param) = @_;
> -
> - my $local_node = PVE::INotify::nodename();
> - die "Cannot delete myself from cluster!\n" if $param->{node} eq $local_node;
> -
> - PVE::Cluster::check_cfs_quorum();
> -
> - my $code = sub {
> - my $conf = PVE::Cluster::cfs_read_file("corosync.conf");
> - my $nodelist = PVE::Corosync::nodelist($conf);
> -
> - my $node;
> - my $nodeid;
> -
> - foreach my $tmp_node (keys %$nodelist) {
> - my $d = $nodelist->{$tmp_node};
> - my $ring0_addr = $d->{ring0_addr};
> - my $ring1_addr = $d->{ring1_addr};
> - if (($tmp_node eq $param->{node}) ||
> - (defined($ring0_addr) && ($ring0_addr eq $param->{node})) ||
> - (defined($ring1_addr) && ($ring1_addr eq $param->{node}))) {
> - $node = $tmp_node;
> - $nodeid = $d->{nodeid};
> - last;
> - }
> - }
> -
> - die "Node/IP: $param->{node} is not a known host of the cluster.\n"
> - if !defined($node);
> -
> - delete $nodelist->{$node};
> -
> - PVE::Corosync::update_nodelist($conf, $nodelist);
> -
> - run_command(['corosync-cfgtool','-k', $nodeid]) if defined($nodeid);
> - };
> -
> - $config_change_lock->($code);
> - die $@ if $@;
> -
> - return undef;
> - }});
> -
> -__PACKAGE__->register_method ({
> name => 'add',
> path => 'add',
> method => 'PUT',
> @@ -885,8 +668,8 @@ our $cmddef = {
> keygen => [ __PACKAGE__, 'keygen', ['filename']],
> create => [ __PACKAGE__, 'create', ['clustername']],
> add => [ __PACKAGE__, 'add', ['hostname']],
> - addnode => [ __PACKAGE__, 'addnode', ['node']],
> - delnode => [ __PACKAGE__, 'delnode', ['node']],
> + addnode => [ 'PVE::API2::ClusterConfig', 'addnode', ['node']],
> + delnode => [ 'PVE::API2::ClusterConfig', 'delnode', ['node']],
> status => [ __PACKAGE__, 'status' ],
> nodes => [ __PACKAGE__, 'nodes' ],
> expected => [ __PACKAGE__, 'expected', ['expected']],
> --
> 2.11.0
>
>
> _______________________________________________
> 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