[pve-devel] [PATCH cluster] allow also deleting a node by name when ringX_addr is a IP

[Thomas Lamprecht]

On 09/20/2016 10:42 AM, Fabian Grünbichler wrote:
> On Tue, Sep 20, 2016 at 10:17:44AM +0200, Thomas Lamprecht wrote:
>> This is an additional (convenience) fix for the delnode param.
>> As we always have the node name in our config - either the 'name'
>> (preferred) or the 'ring0_addr' property of a node entry in the
>> corosync.conf holds it - allow also deleting by it if the ringX_addr
>> is set to an IP, else this may be confusing as the user uses
>> normally the node name only to do things.
>>
>> Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
>> ---
>>
>> This is a follow up patch for the recently pushed commits
>> 7aed82482c7a0eef13582f61ed6bfd69cb7ff33c
>> d09373b46d6ffdc260e842dd3946e732ec0233ee
>> which patch set I overlooked on the mailing list
>>
>>   data/PVE/CLI/pvecm.pm | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/data/PVE/CLI/pvecm.pm b/data/PVE/CLI/pvecm.pm
>> index b26a1ec..b72fb00 100755
>> --- a/data/PVE/CLI/pvecm.pm
>> +++ b/data/PVE/CLI/pvecm.pm
>> @@ -414,9 +414,11 @@ __PACKAGE__->register_method ({
>>   
>>   	foreach my $tmp_node (keys %$nodelist) {
>>   	    my $d = $nodelist->{$tmp_node};
>> +	    my $name = $d->{name};
>>   	    my $ring0_addr = $d->{ring0_addr};
>>   	    my $ring1_addr = $d->{ring1_addr};
>>   	    if (($tmp_node eq $param->{node}) ||
>> +		(defined($name) && ($name eq $param->{node})) ||
>>   		(defined($ring0_addr) && ($ring0_addr eq $param->{node})) ||
>>   		(defined($ring1_addr) && ($ring1_addr eq $param->{node}))) {
>>   		$node = $tmp_node;
>> -- 
>> 2.1.4
> but the name information is stored redundantely in the nodelist - once
> as key of the node element, once as sub element with the key name:
>
> perl -e 'use strict; use warnings; use Data::Dumper; use PVE::CLI::pvecm; use PVE::Cluster; print Dumper(PVE::CLI::pvecm::corosync_nodelist(PVE::Cluster::cfs_read_file("corosync.conf")));'
>
> $VAR1 = {
>            'nodename2' => {
>                            'nodeid' => '2',
>                            'ring0_addr' => '10.0.1.12',
>                            'name' => 'nodename2',
>                            'quorum_votes' => '1'
>                          },
>            'nodename3' => {
>                            'name' => 'nodename3',
>                            'nodeid' => '3',
>                            'ring0_addr' => '10.0.1.13',
>                            'quorum_votes' => '1'
>                          },
>            'nodename1' => {
>                            'quorum_votes' => '1',
>                            'ring0_addr' => '10.0.1.11',
>                            'nodeid' => '1',
>                            'name' => 'nodename1'
>                          }
>          };
>
> pve-cluster/data/PVE/CLI/pvecm.pm:736
>
> 		foreach my $child (@{$ne->{children}}) {
> 		    next if !defined($child->{key});
> 		    $node->{$child->{key}} = $child->{value};
> 		    # use 'name' over 'ring0_addr' if set
> 		    if ($child->{key} eq 'name') {
> 			delete $nodelist->{$name} if $name;
> 			$name = $child->{value};
> 			$nodelist->{$name} = $node;
> 		    } elsif(!$name && $child->{key} eq 'ring0_addr') {
> 			$name = $child->{value};
> 			$nodelist->{$name} = $node;
> 		    }
> 		}
>
> since we are iterating over the nodelist's keys, $tmp_node is always
> equal to $d->{name}, so this patch does not make any sense? or am I
> missing something here?

Nope, you're quite right, sorry for the noise. :)