diff --git a/xCAT-server/lib/xcat/plugins/confluent.pm b/xCAT-server/lib/xcat/plugins/confluent.pm index 85599338c..ec1e435c4 100644 --- a/xCAT-server/lib/xcat/plugins/confluent.pm +++ b/xCAT-server/lib/xcat/plugins/confluent.pm @@ -235,15 +235,18 @@ sub makeconfluentcfg { my $hmtab = xCAT::Table->new('nodehm'); my $nodepostab = xCAT::Table->new('nodepos'); my $mptab = xCAT::Table->new('mp'); + my $switchtab = xCAT::Table->new('switch'); my @cfgents1; # = $hmtab->getAllNodeAttribs(['cons','serialport','mgt','conserver','termserver','termport']); my @cfgents2; my @cfgents3; + my @cfgents4; my $explicitnodes = 0; if (($nodes and @$nodes > 0) or $req->{noderange}->[0]) { $explicitnodes = 1; @cfgents1 = $hmtab->getNodesAttribs($nodes, [ 'node', 'cons', 'mgt', 'conserver', 'termserver', 'termport', 'consoleondemand' ]); @cfgents2 = $nodepostab->getNodesAttribs($nodes, [ 'node', 'rack', 'u', 'chassis', 'slot', 'room' ]); @cfgents3 = $mptab->getNodesAttribs($nodes, [ 'node', 'mpa', 'id' ]); + @cfgents4 = $switchtab->getNodesAttribs($nodes, [ 'node', 'switch', 'port', 'interface' ]); # Adjust the data structure to make the result consistent with the getAllNodeAttribs() call we make if a noderange was not specified my @tmpcfgents1; @@ -279,16 +282,32 @@ sub makeconfluentcfg { } } @cfgents3 = @tmpcfgents1; + @tmpcfgents1 = (); + foreach my $ent (@cfgents4) + { + foreach my $nodeent (keys %$ent) + { + foreach my $row (@{ $ent->{$nodeent} }) + { + next unless ($row); + $row->{node} = $nodeent unless (defined($row->{node})); + push @tmpcfgents1, $row; + } + } + } + @cfgents4 = @tmpcfgents1; } else { @cfgents1 = $hmtab->getAllNodeAttribs([ 'cons', 'serialport', 'mgt', 'conserver', 'termserver', 'termport', 'consoleondemand' ]); @cfgents2 = $nodepostab->getAllNodeAttribs([ 'rack', 'u', 'chassis', 'slot', 'room' ]); - @cfgents3 = $nodepostab->getAllNodeAttribs([ 'mpa', 'id' ]); + @cfgents3 = $mptab->getAllNodeAttribs([ 'mpa', 'id' ]); + @cfgents4 = $switchtab->getAllNodeAttribs([ 'node', 'switch', 'port', 'interface' ]); } #cfgents1 should now have all the nodes, so we can fill in the cfgents array and cfgenthash one at a time. # skip the nodes that do not have 'cons' defined, unless a serialport setting suggests otherwise my %cfgenthash; + my %cfgnichash; foreach (@cfgents1) { if ($_->{cons} or defined($_->{'serialport'})) { unless ($_->{cons}) { $_->{cons} = $_->{mgt}; } #populate with fallback @@ -307,6 +326,19 @@ sub makeconfluentcfg { $cfgenthash{ $nent->{node} }->{$_} = $nent->{$_}; } } + foreach my $nent (@cfgents4) { + next unless (defined $nent->{node}); + if (defined $nent->{interface} and length $nent->{interface}) { + foreach (keys %$nent) { + $cfgnichash{ $nent->{node} }->{ $nent->{interface} }->{$_} = $nent->{$_}; + } + $cfgenthash{ $nent->{node} }->{node} = $nent->{node}; + } else { + foreach (keys %$nent) { + $cfgenthash{ $nent->{node} }->{$_} = $nent->{$_}; + } + } + } my @cfgents = (); foreach (values %cfgenthash) { push @cfgents, $_; @@ -325,7 +357,7 @@ sub makeconfluentcfg { #if (($req->{_allnodes}) && ($req->{_allnodes}->[0]==1)) {} #TODO: identify nodes that will be removed # call donodeent to add all node entries into the file. It will return the 1st node in error. my $node; - if ($node = donodeent(\%cfgenthash, $confluent, $delmode, $cb)) { + if ($node = donodeent(\%cfgenthash, $confluent, $delmode, $cb, \%cfgnichash)) { #$cb->({node=>[{name=>$node,error=>"Bad configuration, check attributes under the nodehm category",errorcode=>1}]}); xCAT::SvrUtils::sendmsg([ 1, "Bad configuration, check attributes under the nodehm category" ], $cb, $node); @@ -366,7 +398,7 @@ sub makeconfluentcfg { # Now add into the file all the node entries that we kept my $node; - if ($node = donodeent(\%cfgenthash, $confluent, undef, $cb)) { + if ($node = donodeent(\%cfgenthash, $confluent, undef, $cb, \%cfgnichash)) { # donodeent will return the 1st node in error #$cb->({node=>[{name=>$node,error=>"Bad configuration, check attributes under the nodehm category",errorcode=>1}]}); @@ -384,6 +416,7 @@ sub donodeent { my $confluent = shift; my $delmode = shift; my $cb = shift; + my $cfgnichash = shift || {}; my $idx = 0; my $toidx = -1; my $skip = 0; @@ -510,8 +543,28 @@ sub donodeent { } elsif (defined $cfgent->{id}) { $parameters{'enclosure.bay'} = $cfgent->{id}; } + if (defined $cfgent->{switch}) { + $parameters{'net.switch'} = $cfgent->{switch}; + } + if (defined $cfgent->{port}) { + $parameters{'net.switchport'} = $cfgent->{port}; + } + foreach my $nic (keys %{ $cfgnichash->{$node} || {} }) { + my $nicent = $cfgnichash->{$node}->{$nic}; + if (defined $nicent->{switch}) { + $parameters{"net.$nic.switch"} = $nicent->{switch}; + } + if (defined $nicent->{port}) { + $parameters{"net.$nic.switchport"} = $nicent->{port}; + } + } $parameters{'groups'} = [ grep { defined && length } split /,/, $groupdata->{$node}->[0]->{'groups'} ]; if (exists $currnodes{$node}) { + foreach my $topo ('net.switch', 'net.switchport') { + $parameters{$topo} = undef unless (exists $parameters{$topo}); + } + $parameters{'net.*.switch'} = undef; + $parameters{'net.*.switchport'} = undef; $confluent->update('/nodes/' . $node . '/attributes/current', parameters => \%parameters); my $rsp = $confluent->next_result(); while ($rsp) { diff --git a/xCAT-test/unit/confluent_switch_retract.t b/xCAT-test/unit/confluent_switch_retract.t new file mode 100644 index 000000000..8fbab7541 --- /dev/null +++ b/xCAT-test/unit/confluent_switch_retract.t @@ -0,0 +1,117 @@ +#!/usr/bin/env perl +use strict; +use warnings; + +use File::Spec; +use FindBin; +use Test::More; + +my $plugin = File::Spec->catfile( $FindBin::Bin, '..', '..', + 'xCAT-server', 'lib', 'xcat', 'plugins', 'confluent.pm' ); +plan skip_all => 'confluent.pm not found' unless -r $plugin; + +open( my $fh, '<', $plugin ) or die "Unable to read $plugin: $!"; +my $source = do { local $/; <$fh> }; +close($fh); + +# The payload the command sends for a node confluent already holds. This +# mirrors the order the command builds it in: the values first, then the names +# that carry no value. +sub update_payload { + my ( $flat, $pernic ) = @_; + my %parameters; + if ( defined $flat->{switch} ) { $parameters{'net.switch'} = $flat->{switch} } + if ( defined $flat->{port} ) { $parameters{'net.switchport'} = $flat->{port} } + foreach my $nic ( keys %$pernic ) { + my $ent = $pernic->{$nic}; + if ( defined $ent->{switch} ) { $parameters{"net.$nic.switch"} = $ent->{switch} } + if ( defined $ent->{port} ) { $parameters{"net.$nic.switchport"} = $ent->{port} } + } + foreach my $topo ( 'net.switch', 'net.switchport' ) { + $parameters{$topo} = undef unless ( exists $parameters{$topo} ); + } + $parameters{'net.*.switch'} = undef; + $parameters{'net.*.switchport'} = undef; + return \%parameters; +} + +# A node xCAT holds no topology for must name every topology attribute with no +# value, so confluent removes what it still holds. +my $p = update_payload( {}, {} ); +ok( exists $p->{'net.switch'}, 'a node with no topology names the plain switch' ); +is( $p->{'net.switch'}, undef, 'the plain switch carries no value' ); +ok( exists $p->{'net.switchport'}, 'a node with no topology names the plain port' ); +is( $p->{'net.switchport'}, undef, 'the plain port carries no value' ); +is( $p->{'net.*.switch'}, undef, 'every interface switch is named with no value' ); +is( $p->{'net.*.switchport'}, undef, 'every interface port is named with no value' ); + +# A value the switch table holds must survive beside the names that clear. +$p = update_payload( { switch => 'sw1', port => '1' }, {} ); +is( $p->{'net.switch'}, 'sw1', 'a held plain switch keeps its value' ); +is( $p->{'net.switchport'}, '1', 'a held plain port keeps its value' ); +is( $p->{'net.*.switch'}, undef, 'the interface wildcard still clears' ); + +# The interface case. Confluent removes what the wildcard matches before it +# sets the rest of the request, so the current interface survives. +$p = update_payload( {}, { ib0 => { switch => 'sw2', port => '9' } } ); +is( $p->{'net.ib0.switch'}, 'sw2', 'the current interface keeps its switch' ); +is( $p->{'net.ib0.switchport'}, '9', 'the current interface keeps its port' ); +is( $p->{'net.*.switch'}, undef, 'the interface wildcard clears the rest' ); +is( $p->{'net.switch'}, undef, 'a node with only an interface clears the plain switch' ); + +# A renamed interface: only the new name carries a value, and the wildcard +# removes the old one. +$p = update_payload( {}, { ens1f0 => { switch => 'sw3', port => '4' } } ); +is( $p->{'net.ens1f0.switch'}, 'sw3', 'the new interface name carries the switch' ); +ok( !exists $p->{'net.eth0.switch'}, 'the old interface name is not named on its own' ); +is( $p->{'net.*.switch'}, undef, 'the old interface name is removed by the wildcard' ); + +# The wildcard cannot stand in for the names that carry no interface, so those +# have to be named separately. This is why both forms are in the payload. +SKIP: { + eval { require File::FnMatch; 1 } + or skip 'File::FnMatch not available', 1; + ok( !File::FnMatch::fnmatch( 'net.*.switch', 'net.switch' ), + 'the interface wildcard does not match the plain name' ); +} +# Same check without the optional module, using the rule the wildcard follows. +ok( 'net.switch' !~ /^net\..+\.switch$/, + 'the plain name needs its own clear because the wildcard cannot match it' ); +ok( 'net.ib0.switch' =~ /^net\..+\.switch$/, + 'an interface name is what the wildcard matches' ); + +# The command has to clear only where there is something to clear. +my ($body) = $source =~ /\nsub donodeent \{(.*?)\n\}\n/s; +ok( defined($body), 'the donodeent body was located' ); +like( $body, qr/\$parameters\{'net\.\*\.switch'\}\s*= undef/, + 'the command names every interface switch with no value' ); +like( $body, qr/\$parameters\{'net\.\*\.switchport'\}\s*= undef/, + 'the command names every interface port with no value' ); +like( $body, qr/\$parameters\{\$topo\} = undef unless \(exists \$parameters\{\$topo\}\)/, + 'a held value is never replaced by a clear' ); + +my ($updatebranch) = + $body =~ /if \(exists \$currnodes\{\$node\}\) \{(.*?)\n \} else \{/s; +ok( defined($updatebranch), 'the update branch was located' ); +like( $updatebranch, qr/net\.\*\.switch/, + 'the clears are on the branch that updates a node confluent holds' ); + +my ($createbranch) = $body =~ /\n \} else \{(.*)$/s; +ok( defined($createbranch), 'the create branch was located' ); +unlike( $createbranch, qr/net\.\*\.switch/, + 'the branch that creates a node sends no clear' ); + +# The transport has to turn no value into a JSON null, which is what confluent +# reads as a request to remove an attribute. +my $tlv = File::Spec->catfile( $FindBin::Bin, '..', '..', + 'xCAT-server', 'lib', 'xcat', 'Confluent', 'TLV.pm' ); +SKIP: { + skip 'TLV.pm not found', 1 unless -r $tlv; + open( my $tf, '<', $tlv ) or die $!; + my $tsrc = do { local $/; <$tf> }; + close($tf); + like( $tsrc, qr/\$self->\{json\}->utf8->encode\(\$data\)/, + 'the payload is encoded as JSON, which carries no value as null' ); +} + +done_testing(); diff --git a/xCAT-test/unit/confluent_switch_topology.t b/xCAT-test/unit/confluent_switch_topology.t new file mode 100644 index 000000000..7afb1ecd9 --- /dev/null +++ b/xCAT-test/unit/confluent_switch_topology.t @@ -0,0 +1,99 @@ +#!/usr/bin/env perl +use strict; +use warnings; + +use File::Spec; +use FindBin; +use Test::More; + +my $plugin = File::Spec->catfile( $FindBin::Bin, '..', '..', + 'xCAT-server', 'lib', 'xcat', 'plugins', 'confluent.pm' ); +plan skip_all => 'confluent.pm not found' unless -r $plugin; + +open( my $fh, '<', $plugin ) or die "Unable to read $plugin: $!"; +my $source = do { local $/; <$fh> }; +close($fh); + +# confluent.pm needs a management node to load, so drive the shaping of the +# switch rows on its own. This mirrors the merge the command performs. +sub shape { + my @rows = @_; + my ( %cfgenthash, %cfgnichash ); + foreach my $nent (@rows) { + next unless ( defined $nent->{node} ); + if ( defined $nent->{interface} and length $nent->{interface} ) { + foreach ( keys %$nent ) { + $cfgnichash{ $nent->{node} }->{ $nent->{interface} }->{$_} = $nent->{$_}; + } + $cfgenthash{ $nent->{node} }->{node} = $nent->{node}; + } else { + foreach ( keys %$nent ) { + $cfgenthash{ $nent->{node} }->{$_} = $nent->{$_}; + } + } + } + return ( \%cfgenthash, \%cfgnichash ); +} + +# A node has one row for each interface. Keeping only one of them loses the +# port of every other interface, which is what this export exists to carry. +my ( $flat, $pernic ) = shape( + { node => 'n1', switch => 'sw1', port => '1', interface => 'eth0' }, + { node => 'n1', switch => 'sw2', port => '9', interface => 'ib0' }, +); +is( scalar keys %{ $pernic->{n1} }, 2, 'a node with two interfaces keeps both' ); +is( $pernic->{n1}{eth0}{switch}, 'sw1', 'the first interface keeps its switch' ); +is( $pernic->{n1}{eth0}{port}, '1', 'the first interface keeps its port' ); +is( $pernic->{n1}{ib0}{switch}, 'sw2', 'the second interface keeps its switch' ); +is( $pernic->{n1}{ib0}{port}, '9', 'the second interface keeps its port' ); +is( $flat->{n1}{switch}, undef, 'a row naming an interface gives no plain switch' ); +is( $flat->{n1}{port}, undef, 'a row naming an interface gives no plain port' ); + +# The node has to reach the configuration for its interfaces to be written. A +# node whose rows all name an interface is only in the per interface data, so +# the node itself must still be recorded. +is( $flat->{n1}{node}, 'n1', 'a node known only by its interfaces is still exported' ); + +# A row that names no interface keeps the plain names. +( $flat, $pernic ) = shape( { node => 'n2', switch => 'sw3', port => '4' } ); +is( $flat->{n2}{switch}, 'sw3', 'a row with no interface gives the plain switch' ); +is( $flat->{n2}{port}, '4', 'a row with no interface gives the plain port' ); +is( $pernic->{n2}, undef, 'a row with no interface adds no interface entry' ); + +# An empty switch table must leave the configuration untouched. +( $flat, $pernic ) = shape(); +is_deeply( $flat, {}, 'an empty switch table adds no node entry' ); +is_deeply( $pernic, {}, 'an empty switch table adds no interface entry' ); + +# A row without a node name cannot be placed. +( $flat, $pernic ) = shape( { switch => 'sw4', port => '2' } ); +is_deeply( $flat, {}, 'a row with no node is skipped' ); +is_deeply( $pernic, {}, 'a row with no node adds no interface entry' ); + +# The command has to read the switch table, and read it in both branches. The +# branch that takes no node range must not read these columns from nodepos, +# which does not have them. +like( $source, qr/my \$switchtab = xCAT::Table->new\('switch'\)/, + 'the command opens the switch table' ); +like( $source, + qr/\@cfgents4 = \$switchtab->getNodesAttribs\(\$nodes, \[ 'node', 'switch', 'port', 'interface' \]\)/, + 'a node range reads the switch table for those nodes' ); +like( $source, + qr/\@cfgents4 = \$switchtab->getAllNodeAttribs\(\[ 'node', 'switch', 'port', 'interface' \]\)/, + 'no node range reads the whole switch table' ); +unlike( $source, qr/\$nodepostab->getAllNodeAttribs\(\[ 'node', 'switch'/, + 'the switch columns are never read from nodepos' ); + +# The exported names are what confluent reads. +like( $source, qr/\$parameters\{'net\.switch'\} = \$cfgent->\{switch\}/, + 'the plain switch is exported' ); +like( $source, qr/\$parameters\{'net\.switchport'\} = \$cfgent->\{port\}/, + 'the plain port is exported' ); +like( $source, qr/\$parameters\{"net\.\$nic\.switch"\}/, + 'the switch of each interface is exported' ); +like( $source, qr/\$parameters\{"net\.\$nic\.switchport"\}/, + 'the port of each interface is exported' ); +like( $source, qr/\$cfgenthash\{ \$nent->\{node\} \}->\{node\} = \$nent->\{node\}/, + 'a node known only by its interfaces is recorded for the export' ); + +done_testing();