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.lampre...@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@pve.proxmox.com >> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel