[pve-devel] [RFC cluster v2 01/10] move addnode/delnode from CLI to cluster config API

Thomas Lamprecht t.lamprecht at proxmox.com
Wed Dec 6 16:28:35 CET 2017


On 12/06/2017 04:01 PM, Fabian Grünbichler wrote:
> 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 ;)
> 

Yes, totally agree. This was merely copied for now and I oversaw this in
the sea of changes :)


> 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