Merge pull request #7717 from VersatusHPC/fix/makedhcp-restrict-sn-dispatch

fix(dhcp): only dispatch makedhcp to the service nodes serving the nodes

xCAT-server/lib/xcat/plugins/dhcp.pm

@@ -1458,10 +1458,22 @@ sub preprocess_request
                 push @requests, $reqcopy;
             }
 
+            # A request naming nodes only concerns the service nodes that serve
+            # them. Regenerating the networks still reaches every dhcp server,
+            # since a dynamic range is not tied to a node. If none of the named
+            # nodes can be mapped to a service node, keep sending to all of them
+            # rather than risk leaving one out.
+            my %servingsn = ();
+            unless ($opt{n}) {
+                my $sn_hash = xCAT::ServiceNodeUtils->getSNformattedhash(\@nodes, "xcat", "MN");
+                foreach my $sn (keys %$sn_hash) { $servingsn{$sn} = 1; }
+            }
+
             foreach my $s (@snlist)
             {
                 if (scalar @nodes == 1 and $nodes[0] eq $s) { next; }
                 next if ($issn && exists($iphash{$s}));
+                next if (keys(%servingsn) && !exists($servingsn{$s}));
 
                 my $reqcopy = {%$req};
                 $reqcopy->{'_xcatdest'} = $s;

xCAT-test/unit/dhcp_sn_dispatch_scope.t

@@ -0,0 +1,58 @@
+#!/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/dhcp.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);
+
+# The dispatch loop that fans a node-targeted makedhcp out to service nodes.
+my ($loop) = $source =~ m{
+    ( my \s+ %servingsn .*? foreach \s+ my \s+ \$s \s+ \(\@snlist\) .*? \n \s* \} \n )
+}sx;
+
+ok( $loop, 'the service node dispatch loop was located' )
+  or BAIL_OUT('dhcp.pm no longer matches the expected dispatch shape');
+
+like(
+    $loop,
+    qr/getSNformattedhash\(\\\@nodes/,
+    'the named nodes are mapped to their service nodes'
+);
+like(
+    $loop,
+    qr/next \s+ if \s+ \(keys\(%servingsn\) \s* && \s* !exists\(\$servingsn\{\$s\}\)\)/x,
+    'a service node serving none of the named nodes is skipped'
+);
+
+# Regenerating the networks must still reach every dhcp server, because a
+# dynamic range does not belong to any node.
+like(
+    $loop,
+    qr/unless \s* \(\$opt\{n\}\)/x,
+    'network regeneration is exempt from the node based restriction'
+);
+
+# Empty mapping must fall back to the previous fan-out rather than silently
+# dispatching to nothing.
+my ($guard) = $loop =~ /(next\s+if\s+\(keys\(%servingsn\)[^\n]*)/;
+like(
+    $guard,
+    qr/keys\(%servingsn\)\s*&&/,
+    'an unmapped noderange still reaches every service node'
+);
+
+# The pre-existing conditions in the loop must survive.
+like( $loop, qr/scalar \@nodes == 1 and \$nodes\[0\] eq \$s/, 'the self dispatch guard is retained' );
+like( $loop, qr/\$issn && exists\(\$iphash\{\$s\}\)/,          'the service node self skip is retained' );
+
+done_testing();