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

Reply via email to