[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