From cc71816b90f32645329ebefb0cfe9ef99c1fc624 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Fri, 7 Aug 2026 22:32:43 -0300 Subject: [PATCH 1/2] fix(confluent): configure nodes named explicitly without a console method preprocess_request skips any node whose nodehm entry has neither cons nor serialport. That is the right default while scanning the whole table, but it also drops nodes the administrator named on the command line, leaving the dispatched node list empty. makeconfluentcfg then falls back to reading every node, so naming one console-less node quietly configures the whole cluster instead of the node that was asked for. A named node with no nodehm row at all is worse: both the selection here and the second lookup in makeconfluentcfg produce an undefined entry, which reaches confluent as an empty node name. Apply the skip only to the full table scan, and carry the node name through both lookups when the nodehm row is missing. Nodes without a console method are already handled downstream, where makeconfluentcfg keeps explicitly named entries and donodeent falls back to hardware management credentials. Co-authored-by: Jarrod Johnson <10814490+jjohnson42@users.noreply.github.com> --- xCAT-server/lib/xcat/plugins/confluent.pm | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/xCAT-server/lib/xcat/plugins/confluent.pm b/xCAT-server/lib/xcat/plugins/confluent.pm index 8c2baded0..85599338c 100644 --- a/xCAT-server/lib/xcat/plugins/confluent.pm +++ b/xCAT-server/lib/xcat/plugins/confluent.pm @@ -114,6 +114,10 @@ sub preprocess_request { my $hmcache = $hmtab->getNodesAttribs($noderange, [ 'node', 'serialport', 'cons', 'conserver' ]); foreach my $node (@$noderange) { my $ent = $hmcache->{$node}->[0]; #$hmtab->getNodeAttribs($node,['node', 'serialport','cons', 'conserver']); + #A node named on the command line may have no nodehm row at all. Carry + #its name through anyway, otherwise the entry below is an undefined + #reference and the node reaches confluent with an empty name. + unless ($ent) { $ent = { node => $node }; } push @items, $ent; } } else { @@ -123,7 +127,11 @@ sub preprocess_request { my @nodes = (); foreach (@items) { - if (((!defined($_->{cons})) || ($_->{cons} eq "")) and !defined($_->{serialport})) { next; } #skip if 'cons' is not defined for this node, unless serialport suggests otherwise + #skip if 'cons' is not defined for this node, unless serialport suggests otherwise. + #This only applies while scanning every node in the table. A node named + #explicitly on the command line was asked for by the administrator, so it is + #configured whether or not it has a console method. + if ($allnodes and ((!defined($_->{cons})) || ($_->{cons} eq "")) and !defined($_->{serialport})) { next; } if (defined($_->{conserver})) { push @{ $cons_hash{ $_->{conserver} }{nodes} }, $_->{node}; } else { push @{ $cons_hash{$master}{nodes} }, $_->{node}; } push @nodes, $_->{node}; @@ -243,7 +251,13 @@ sub makeconfluentcfg { { foreach my $nodeent (keys %$ent) { - push @tmpcfgents1, $ent->{$nodeent}->[0]; + #A named node may have no nodehm row, which leaves an undefined + #entry that reaches confluent as an empty node name. The key here + #is the node, so carry the name through instead. + my $row = $ent->{$nodeent}->[0]; + $row = {} unless ($row); + $row->{node} = $nodeent unless (defined($row->{node})); + push @tmpcfgents1, $row; } } @cfgents1 = @tmpcfgents1; From bce765732442902bf7057a4c3a2887ce54e1ff98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Fri, 7 Aug 2026 22:32:43 -0300 Subject: [PATCH 2/2] test(confluent): cover explicitly named nodes without a console method Exercise the node-selection block from preprocess_request and the lookup adjustment from makeconfluentcfg directly, both extracted from the shipped plugin, with a stand-in nodehm table. Assert that an explicitly named node is configured whether it has no console attributes or no nodehm row at all, that neither lookup can emit an empty node name, and that the full table scan still skips console-less nodes so an entire cluster is not swept into confluent. That last group is what separates this from simply removing the skip, and it fails if the skip is dropped outright. --- xCAT-test/unit/confluent_explicit_nodes.t | 124 ++++++++++++++++++++++ 1 file changed, 124 insertions(+) create mode 100644 xCAT-test/unit/confluent_explicit_nodes.t diff --git a/xCAT-test/unit/confluent_explicit_nodes.t b/xCAT-test/unit/confluent_explicit_nodes.t new file mode 100644 index 000000000..1c69c93e2 --- /dev/null +++ b/xCAT-test/unit/confluent_explicit_nodes.t @@ -0,0 +1,124 @@ +#!/usr/bin/env perl +use strict; +use warnings; + +use FindBin; +use File::Spec; +use Test::More; + +my $repo_root = File::Spec->catdir( $FindBin::Bin, '..', '..' ); +my $plugin = File::Spec->catfile( $repo_root, 'xCAT-server/lib/xcat/plugins/confluent.pm' ); + +plan skip_all => "$plugin not found" unless -r $plugin; + +open( my $fh, '<', $plugin ) or die "Unable to read $plugin: $!"; +my $source = do { local $/; <$fh> }; +close($fh); + +# A stand-in for the nodehm table, so the selection logic can be exercised +# without a database. getNodesAttribs leaves out nodes that have no row, which +# is what the plugin sees for a node that was never given console attributes. +{ + package StubNodehm; + sub new { my ( $class, %rows ) = @_; return bless { rows => {%rows} }, $class; } + sub getNodesAttribs { + my ( $self, $noderange, $attrs ) = @_; + my %out; + foreach my $node (@$noderange) { + next unless exists $self->{rows}{$node}; + $out{$node} = [ { %{ $self->{rows}{$node} } } ]; + } + return \%out; + } + sub getAllNodeAttribs { + my ( $self, $attrs ) = @_; + return map { { %{ $self->{rows}{$_} } } } sort keys %{ $self->{rows} }; + } +} + +# Extract the node-selection block from preprocess_request and run it directly, +# so this covers the shipped logic rather than a copy of it. +my ($block) = $source =~ m{ + ( my \s+ \@items; .*? + push \s+ \@nodes, \s* \$_->\{node\}; \s* \n \s* \} ) +}sx; + +ok( $block, 'the node-selection block was located in preprocess_request()' ) + or BAIL_OUT('confluent.pm no longer matches the expected node-selection shape'); + +sub select_nodes { + my ( $noderange, %rows ) = @_; + my $hmtab = StubNodehm->new(%rows); + my $master = 'mn.example'; + my %cons_hash; + my $code = 'sub { my ($noderange, $hmtab, $master, $cons_ref) = @_; my %cons_hash; ' + . $block + . ' %$cons_ref = %cons_hash; return \@nodes; }'; + my $sub = eval $code; + die "Unable to evaluate the extracted block: $@" if $@; + my $nodes = $sub->( $noderange, $hmtab, $master, \%cons_hash ); + return ( $nodes, \%cons_hash ); +} + +my %rows = ( + withcons => { node => 'withcons', cons => 'ipmi' }, + withserial => { node => 'withserial', serialport => 0 }, + nocons => { node => 'nocons' }, + withserver => { node => 'withserver', cons => 'ipmi', conserver => 'sn1.example' }, +); + +# An explicitly named node is configured even with no console attributes, and +# even with no nodehm row at all. The administrator asked for it by name. +my ( $explicit ) = select_nodes( ['nocons'], %rows ); +is_deeply( $explicit, ['nocons'], 'an explicitly named node with no console attributes is still configured' ); + +my ( $missing ) = select_nodes( ['neverdefined'], %rows ); +is_deeply( $missing, ['neverdefined'], 'an explicitly named node with no nodehm row is configured under its own name' ); + +my ( $mixed ) = select_nodes( [ 'withcons', 'nocons' ], %rows ); +is_deeply( [ sort @$mixed ], [ 'nocons', 'withcons' ], 'an explicit noderange keeps both console-configured and console-less nodes' ); + +# Scanning the whole table must not change: nodes with no console configuration +# are still skipped, so every node in the cluster is not swept into confluent. +my ( $all ) = select_nodes( undef, %rows ); +is_deeply( + [ sort @$all ], + [ 'withcons', 'withserial', 'withserver' ], + 'a full table scan still skips nodes that have no console configuration' +); +ok( !grep( { $_ eq 'nocons' } @$all ), 'a console-less node is not picked up by a full table scan' ); + +# Conserver routing is unaffected. +my ( undef, $cons_hash ) = select_nodes( [ 'withserver', 'withcons' ], %rows ); +is_deeply( $cons_hash->{'sn1.example'}{nodes}, ['withserver'], 'a node keeps its explicit conserver' ); +is_deeply( $cons_hash->{'mn.example'}{nodes}, ['withcons'], 'a node with no conserver falls back to the management node' ); + +# makeconfluentcfg looks the named nodes up a second time and reshapes the +# result. A node with no nodehm row yields an undefined entry there too, and +# without the node name it reaches confluent as an empty name rather than as +# the node that was asked for. +my ($adjust) = $source =~ m{ + ( my \s+ \@tmpcfgents1; \s*\n + \s* foreach \s+ my \s+ \$ent \s+ \(\@cfgents1\) .*? + \n \s* \} \n \s* \} \n ) +}sx; + +ok( $adjust, 'the explicit-node lookup adjustment was located in makeconfluentcfg()' ) + or BAIL_OUT('confluent.pm no longer matches the expected lookup-adjustment shape'); + +my $adjust_sub = eval 'sub { my (@cfgents1) = @_; ' . $adjust . ' return \@tmpcfgents1; }'; +die "Unable to evaluate the extracted adjustment: $@" if $@; + +my $reshaped = $adjust_sub->( + { withcons => [ { node => 'withcons', cons => 'ipmi' } ] }, + { neverdefined => [] }, +); +is( scalar(@$reshaped), 2, 'both named nodes survive the lookup adjustment' ); +my ($carried) = grep { ($_->{node} || '') eq 'neverdefined' } @$reshaped; +ok( $carried, 'a named node with no nodehm row keeps its name through the adjustment' ); +ok( + !grep( { !defined( $_->{node} ) || $_->{node} eq '' } @$reshaped ), + 'no entry reaches confluent with an empty node name' +); + +done_testing();