fix(dhcp): stop handing an installed node a boot file on either backend

A node that has finished installing has chain.currstate "boot", and from
that point the server must stop naming it a boot file: a node given a
netboot script every time it powers on reinstalls itself forever, and does
so silently, because each individual boot looks like a successful one.

Neither backend did that. The wire suite found both.

ISC gated the whole boot-from-disk branch on $doiscsi, so the rule only
fired for a node with an iscsi table row. An ordinary installed node fell
through to the netboot branches below and its xNBA second stage was handed
its own install script. Both the xnba and the pxe branch had it; the pxe
one handed out pxelinux.0.

Kea reads "boot-file-name": "" as *unspecified*, not as "no boot file", so
the empty boot file on the node's reservation did not outrank the classes
every client shares. The always-evaluated xcat-bios class supplied
xcat/xnba.kpxe, the loader came back announcing user class xNBA, and the
per-network class then supplied the network's boot script -- so withholding
only the per-node script withheld nothing. The fix is the same mechanism the
DROP and NOIP classes already use: a xcat-localboot class holding the MACs,
and a "not member('xcat-localboot')" guard on every class that names a boot
file. Kea requires a class to be defined before it is referenced, so
xcat-localboot is written first.

iSCSI is the exception on both backends. Its root disk is on the network and
gPXE is what attaches it, so it still gets a loader: ISC keeps its $doiscsi
branch, and Kea leaves those MACs out of the class.

spec.md S-31 is rewritten to say what correct is -- no boot file at all, on
either request of an xNBA boot, whatever the netboot method -- rather than
the weaker "not its own script", which the second stage above shows is not
the same statement.

conf/localboot.conf asserts the stronger form and gains a netboot=pxe node,
since that branch is written separately and the xnba node passing says
nothing about it. dhcpfixture.sh defines it.

An assertion with != or not-in now holds against a target that is absent
from the reply: a reply naming no boot file has certainly not named the
node's install script. Every other operator still fails on absence, and
"absent" remains the way to assert absence itself.

perl-xCAT/xCAT/DHCP/BootPolicy.pm

@@ -648,6 +648,52 @@ sub kea_drop_client_class {
     };
 }
 
+# The name of the class holding every MAC that is to be handed no boot file.
+sub kea_localboot_class_name { return 'xcat-localboot'; }
+
+# The test fragment that keeps a class from matching one of those MACs.
+sub kea_localboot_guard {
+    return "not member('" . kea_localboot_class_name() . "')";
+}
+
+# The MACs of nodes that have an operating system and must be left to start it.
+#
+# A node whose chain.currstate is boot or iscsiboot is given an empty
+# boot-file-name in its reservation, and that is not enough on its own: Kea
+# reads an empty string as "not specified" and falls through to the classes,
+# which match on architecture and hand the machine a loader anyway. It is the
+# same trap the DROP class above exists to avoid, and it is worse here, because
+# the loader then asks again as an xNBA second stage and is answered with the
+# network's boot script -- so an installed node netboots on every power cycle
+# instead of starting its disk. spec.md S-31.
+#
+# So the MACs are collected into one class and every class that names a boot
+# file is written to exclude members of it -- see kea_apply_localboot_guard.
+# One class shared by the whole cluster, with the user-context recording whose
+# MACs they are, so a later makedhcp for one node can rebuild it without losing
+# the rest: exactly how DROP is kept.
+#
+# A node booting from an iSCSI target is deliberately not in here. Its root
+# disk is on the network and gPXE is what attaches it, so it does still want a
+# loader; ISC draws the same line with $doiscsi.
+sub kea_localboot_client_class {
+    my ( $class, %opts ) = @_;
+
+    my @macs = grep { $_->{node} && $_->{mac} } @{ $opts{macs} || [] };
+    return unless @macs;
+
+    my @sorted = sort { $a->{node} cmp $b->{node} or $a->{mac} cmp $b->{mac} } @macs;
+
+    return {
+        name           => kea_localboot_class_name(),
+        test           => join( ' or ', map { _mac_test( $_->{mac} ) } @sorted ),
+        'user-context' => {
+            'xcat-purpose' => 'localboot-suppress',
+            'xcat-macs'    => [ map { { node => $_->{node}, mac => lc( $_->{mac} ) } } @sorted ],
+        },
+    };
+}
+
 #: The encapsulated space ISC declares as "option space isan" -- option 43
 #: carrying the initiator name in 203 and the root path in 201.
 sub kea_isan_option_defs {

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

@@ -1136,8 +1136,29 @@ sub addnode
 
         if ($nrent and $nrent->{netboot} and $nrent->{netboot} eq 'xnba' and $lstatements !~ /filename/) {
             if (-f "$tftpdir/xcat/xnba.kpxe") {
-                if ($doiscsi and $chainent and $chainent->{currstate} and ($chainent->{currstate} eq 'iscsiboot' or $chainent->{currstate} eq 'boot')) {
-                    $lstatements = 'if option client-architecture = 00:00 and not exists gpxe.bus-id { filename = \"xcat/xnba.kpxe\"; } else { filename = \"\"; } ' . $lstatements;
+                if ($chainent and $chainent->{currstate} and ($chainent->{currstate} eq 'iscsiboot' or $chainent->{currstate} eq 'boot')) {
+
+                    # A node in state boot or iscsiboot has an operating system
+                    # and must be left to start it -- spec.md S-31. iSCSI is
+                    # the one case that still needs a loader, because the root
+                    # disk is on the network and gPXE is what attaches it: BIOS
+                    # firmware is given xnba.kpxe, and the second stage, which
+                    # announces gpxe.bus-id, is given nothing. Without an iSCSI
+                    # target there is nothing to attach and the node is given
+                    # no boot file at all, which is what Kea does for either
+                    # state (kea_node_boot_intent).
+                    #
+                    # This used to be gated on $doiscsi, so an ordinary
+                    # installed node fell through to the netboot branches
+                    # below and its xNBA second stage was handed the node's
+                    # install script -- silently reinstalling the machine on
+                    # every power cycle, since each such boot looks like a
+                    # successful boot.
+                    if ($doiscsi) {
+                        $lstatements = 'if option client-architecture = 00:00 and not exists gpxe.bus-id { filename = \"xcat/xnba.kpxe\"; } else { filename = \"\"; } ' . $lstatements;
+                    } else {
+                        $lstatements = 'filename = \"\";' . $lstatements;
+                    }
                 } else {
 
                     # If proxydhcp daemon is enabled for windows deployment, do vendor-class-identifier of "PXEClient" to bump it over to proxydhcp.c
@@ -1161,8 +1182,15 @@ sub addnode
             }    #TODO: warn when windows
         } elsif ($nrent and $nrent->{netboot} and $nrent->{netboot} eq 'pxe' and $lstatements !~ /filename/) {
             if (-f "$tftpdir/xcat/xnba.kpxe") {
-                if ($doiscsi and $chainent and $chainent->{currstate} and ($chainent->{currstate} eq 'iscsiboot' or $chainent->{currstate} eq 'boot')) {
-                    $lstatements = 'if exists gpxe.bus-id { filename = \"\"; } else if exists client-architecture { filename = \"xcat/xnba.kpxe\"; } ' . $lstatements;
+                if ($chainent and $chainent->{currstate} and ($chainent->{currstate} eq 'iscsiboot' or $chainent->{currstate} eq 'boot')) {
+
+                    # S-31 again, and the same $doiscsi gate: without it an
+                    # installed pxe node was handed pxelinux.0 on every boot.
+                    if ($doiscsi) {
+                        $lstatements = 'if exists gpxe.bus-id { filename = \"\"; } else if exists client-architecture { filename = \"xcat/xnba.kpxe\"; } ' . $lstatements;
+                    } else {
+                        $lstatements = 'filename = \"\";' . $lstatements;
+                    }
                 } else {
                     $lstatements = 'if option vendor-class-identifier = \"ScaleMP\" { filename = \"vsmp/pxelinux.0\"; } else { filename = \"pxelinux.0\"; }' . $lstatements;
                 }
@@ -3704,14 +3732,98 @@ sub kea_sync_node_client_classes
         $changed = 1;
     }
 
+    # Same bookkeeping for the nodes that are to be handed no boot file: the
+    # remove above took this node range's MACs out of the class, and these are
+    # the ones it is to have from now on.
+    if (@{ $generated->{localboot} }) {
+        kea_set_localboot_client_class(
+            $config,
+            [ @{ kea_localboot_client_class_macs($config) }, @{ $generated->{localboot} } ]
+        );
+        $changed = 1;
+    }
+
     my $classes = $generated->{classes};
-    return $changed unless @$classes;
+    if (@$classes) {
+        $config->{Dhcp4} ||= {};
+        my @existing = @{ $config->{Dhcp4}{'client-classes'} || [] };
+        $config->{Dhcp4}{'client-classes'} = [ @$classes, @existing ];
+        $changed = 1;
+    }
+
+    kea_apply_localboot_guard($config) if $changed;
+
+    return $changed;
+}
+
+# Keep every class that names a boot file from matching a node that is to boot
+# from its disk, and keep the class those nodes are named in ahead of them:
+# Kea rejects a member() test naming a class that is not defined above it.
+#
+# This runs over the whole config rather than over the classes one generator
+# produced, because the leak is not confined to one of them -- the global
+# architecture classes, the per-network xNBA classes and the per-node classes
+# all name a boot file, and any one of them matching is a node reinstalling
+# itself. Applying the guard where the config is assembled means a class added
+# later is covered without anyone remembering to.
+#
+# It is also its own inverse: with no such node left the class is gone and the
+# guard comes back off, because a member() test naming a class that no longer
+# exists is a configuration Kea refuses to load.
+sub kea_apply_localboot_guard
+{
+    my ($config) = @_;
+
+    return unless $config && $config->{Dhcp4};
+    my $classes = $config->{Dhcp4}{'client-classes'} || [];
+    my $name    = xCAT::DHCP::BootPolicy->kea_localboot_class_name();
+    my $guard   = xCAT::DHCP::BootPolicy->kea_localboot_guard();
+
+    my ($localboot) = grep { ( $_->{name} || '' ) eq $name } @$classes;
+    my @rest = grep { ( $_->{name} || '' ) ne $name } @$classes;
+
+    foreach my $class (@rest) {
+        next unless defined $class->{test} and defined $class->{'boot-file-name'};
+
+        # Take off the guard this sub put on last time before putting it back,
+        # brackets and all, so regenerating does not wrap the test one layer
+        # deeper every time.
+        my $test = $class->{test};
+        $test =~ s/^\((.*)\) and \Q$guard\E$/$1/s;
+
+        $class->{test} = $localboot ? "($test) and $guard" : $test;
+    }
+
+    $config->{Dhcp4}{'client-classes'} = $localboot ? [ $localboot, @rest ] : \@rest;
+
+    return;
+}
+
+sub kea_localboot_client_class_macs
+{
+    my ($config) = @_;
+
+    my $name = xCAT::DHCP::BootPolicy->kea_localboot_class_name();
+    foreach my $class ( @{ ( $config->{Dhcp4} || {} )->{'client-classes'} || [] } ) {
+        next unless ( $class->{name} || '' ) eq $name;
+        return ( $class->{'user-context'} || {} )->{'xcat-macs'} || [];
+    }
+    return [];
+}
+
+sub kea_set_localboot_client_class
+{
+    my ( $config, $macs ) = @_;
 
     $config->{Dhcp4} ||= {};
-    my @existing = @{ $config->{Dhcp4}{'client-classes'} || [] };
-    $config->{Dhcp4}{'client-classes'} = [ @$classes, @existing ];
+    my $name = xCAT::DHCP::BootPolicy->kea_localboot_class_name();
+    my @classes = grep { ( $_->{name} || '' ) ne $name }
+      @{ $config->{Dhcp4}{'client-classes'} || [] };
+    my $localboot = xCAT::DHCP::BootPolicy->kea_localboot_client_class( macs => $macs );
+    unshift @classes, $localboot if $localboot;
+    $config->{Dhcp4}{'client-classes'} = \@classes;
 
-    return 1;
+    return;
 }
 
 sub kea_drop_client_class_macs
@@ -3778,6 +3890,19 @@ sub kea_remove_node_client_classes
         $changed = 1;
     }
 
+    # And out of the class of nodes that are to be handed no boot file, which
+    # is shared the same way. A node that has just been reinstalled leaves it
+    # here and is put back by the sync that follows if it is still in state
+    # boot.
+    my $lb_macs = kea_localboot_client_class_macs($config);
+    my @kept_lb = grep { !$nodes{ $_->{node} || '' } } @$lb_macs;
+    if ( scalar(@kept_lb) != scalar(@$lb_macs) ) {
+        kea_set_localboot_client_class( $config, \@kept_lb );
+        $changed = 1;
+    }
+
+    kea_apply_localboot_guard($config) if $changed;
+
     return $changed;
 }
 
@@ -3808,6 +3933,7 @@ sub kea_node_client_classes_for_nodes
     my @iscsi;
     my @noip;
     my @proxydhcp;
+    my @localboot;
     foreach my $node (@$nodes) {
         my $nrent = $nrents && $nrents->{$node} ? $nrents->{$node}->[0] : undef;
         my $netboot = $nrent ? $nrent->{netboot} : undef;
@@ -3848,8 +3974,13 @@ sub kea_node_client_classes_for_nodes
                 push @proxydhcp, {%record};
             } elsif ( $intent eq 'disk' ) {
 
-                # Nothing: the reservation names an empty boot file and that
-                # outranks anything a class could say.
+                # No boot classes of its own, and the reservation names an
+                # empty boot file -- but neither of those stops the classes
+                # everybody shares from naming one, so the MAC goes into the
+                # class those classes are written to exclude. Unless the node
+                # boots from an iSCSI target, which still needs a loader to
+                # attach the disk.
+                push @localboot, {%record} unless $ient and $ient->{server} and $ient->{target};
             } elsif ($netboot and $netboot eq 'xnba' and $nxtsrv) {
                 push @xnba, { %record, next_server => $nxtsrv, httpport => $httpport };
             } elsif ($netboot and $netboot eq 'pxe') {
@@ -3873,7 +4004,8 @@ sub kea_node_client_classes_for_nodes
             @{ xCAT::DHCP::BootPolicy->kea_proxydhcp_node_classes( nodes => \@proxydhcp ) },
             @{ xCAT::DHCP::BootPolicy->kea_iscsi_node_classes( nodes => \@iscsi ) },
         ],
-        noip => \@noip,
+        noip      => \@noip,
+        localboot => \@localboot,
     };
 }
 

xCAT-test/autotest/testcase/dhcptest/cases0

@@ -142,15 +142,19 @@ check:rc==0
 end
 
 start:dhcptest_boot_file_conditions
-description:A loader is named only when it is fetchable, and its URL names the right port
+description:A loader is named only when it is fetchable, when it is wanted, and its URL names the right port
 label:mn_only,dhcp,dhcp_wire
 os:Linux
-# Both of these change what the management node looks like before the
+# The first two change what the management node looks like before the
 # configuration is generated -- one takes a loader off disk, the other moves
 # the web server's port -- because both backends decide what to write by
 # reading those. Each puts the machine back and regenerates before returning,
 # so the cases that follow are not run against a cluster half way through an
 # experiment.
+#
+# The third asks the opposite question: a node that has an operating system
+# already must not be named a netboot script, however fetchable that script
+# is, or it reinstalls itself on every power cycle.
 cmd:#!/bin/bash
     set -u
     FIX=/opt/xcat/share/xcat/tools/autotest/testcase/dhcptest/dhcpfixture.sh
@@ -160,6 +164,7 @@ cmd:#!/bin/bash
     rc=0
     "$FIX" run-loader-absent || rc=1
     "$FIX" run-httpport || rc=1
+    "$FIX" run-localboot || rc=1
     exit $rc
 check:rc==0
 end

xCAT-test/autotest/testcase/dhcptest/dhcpfixture.sh

@@ -121,6 +121,14 @@ LB_NODE=dhcptestboot
 LB_IP=10.99.0.81
 LB_MAC=02:00:dc:11:00:61
 
+# The same state on the other netboot method that has a boot-from-disk rule of
+# its own. An installed node must be left alone whichever way it was
+# provisioned, and the two methods are written as separate branches in the ISC
+# generator, so one of them being right proves nothing about the other.
+LB_PXE_NODE=dhcptestbootpxe
+LB_PXE_IP=10.99.0.82
+LB_PXE_MAC=02:00:dc:11:00:62
+
 # A machine that speaks BOOTP and not DHCP, and the web port a cluster that is
 # not serving on 80 would use.
 BOOTP_MAC=02:00:de:ad:b0:07
@@ -839,12 +847,16 @@ do_run_localboot() {
         arch=x86_64 netboot=xnba tftpserver="$SRV_IP" xcatmaster="$SRV_IP"
     chtab node="$LB_NODE" chain.currstate=boot \
         || die "cannot set chain.currstate for $LB_NODE"
-    echo done > "$STATE/localboot"
-    makedhcp "$LB_NODE" || die "makedhcp $LB_NODE failed"
+    extra_define "$LB_PXE_NODE" groups=dhcptest ip="$LB_PXE_IP" mac="$LB_PXE_MAC" \
+        arch=x86_64 netboot=pxe tftpserver="$SRV_IP" xcatmaster="$SRV_IP"
+    chtab node="$LB_PXE_NODE" chain.currstate=boot \
+        || die "cannot set chain.currstate for $LB_PXE_NODE"
+    makedhcp "$LB_NODE,$LB_PXE_NODE" || die "makedhcp for the installed nodes failed"
 
     dhcptest_run \
         --set booted_mac="$LB_MAC" --set booted_ip="$LB_IP" \
         --set booted_script="http://$SRV_IP/tftpboot/xcat/xnba/nodes/$LB_NODE" \
+        --set booted_pxe_mac="$LB_PXE_MAC" --set booted_pxe_ip="$LB_PXE_IP" \
         conf/localboot.conf || rc=1
     return $rc
 }

xCAT-test/dhcptest/README.md

@@ -144,6 +144,13 @@ One per line under a single multi-line `assert` key, as `target op value`.
 a server that names no boot file sends an empty `file` header and no option 67,
 which is an absent value, and `matches ^$` can never hold against it.
 
+The negative operators `!=` and `not-in` hold against an absent target, since a
+reply that says nothing on the subject has certainly not said the wrong thing.
+So `bootfile != http://.../nodes/node01` passes both for a server that names
+some other loader and for one that names none, which is what "must not be sent
+its install script" means. Every other operator fails on an absent target,
+because there is nothing to compare against.
+
 Assert on `bootfile`, not on `file`, unless the header itself is the point.
 Servers genuinely differ — ISC dhcpd fills the BOOTP header, dnsmasq answers in
 option 67 once the client has asked for it — and firmware reads whichever

xCAT-test/dhcptest/conf/localboot.conf

@@ -9,21 +9,26 @@
 # The address is still owned by the node and must still be offered -- this is a
 # statement about the boot file, not about the reservation.
 #
-# The two steps are the two halves of an xNBA boot. Firmware asks first with no
-# user class and is answered with the loader binary; the loader then asks again
-# announcing user class xNBA, and *that* is the request that would be answered
-# with the node's script. A test that only sends the first request cannot see
-# the difference, because the script never appears in the first reply.
+# Two of the steps are the two halves of an xNBA boot. Firmware asks first with
+# no user class; the loader it would be given asks again announcing user class
+# xNBA, and *that* is the request the node's own script is keyed on. Both are
+# sent because they are answered by different rules and a test that sends only
+# the first cannot see the second break.
 #
-# What is asserted is exactly spec.md S-31: not "no boot file", which would
-# forbid a server from handing back the stage-1 binary, but "not the node's
-# xNBA script". The script URL is supplied on the command line so this file
-# stays a statement about the wire:
+# What is asserted is spec.md S-31: no boot file at all, on either request.
+# Allowing the stage-1 binary through would be a weaker statement that looks
+# equivalent and is not -- a node handed the stage-1 loader comes back as an
+# xNBA second stage and is answered with the *network's* script, so a server
+# that withholds only the per-node script still reinstalls the machine. The
+# script URL is still supplied on the command line, for the failure output to
+# name what was wrongly sent, so this file stays a statement about the wire:
 #
 #   dhcptest run -i eth1 \
 #       --set booted_mac=02:00:dc:11:00:61 \
 #       --set booted_ip=10.0.0.61 \
 #       --set booted_script=http://10.0.0.1/tftpboot/xcat/xnba/nodes/dhcptestboot \
+#       --set booted_pxe_mac=02:00:dc:11:00:62 \
+#       --set booted_pxe_ip=10.0.0.62 \
 #       conf/localboot.conf
 
 [defaults]
@@ -44,6 +49,24 @@ assert =
     msgtype  == OFFER
     yiaddr   == %(booted_ip)s
     bootfile != %(booted_script)s
+    bootfile absent
+
+[scenario booted-pxe-node-is-not-sent-a-loader]
+description = S-31: An installed node provisioned with netboot=pxe is not handed pxelinux either
+
+# A different node in the same state, provisioned the other way. netboot=pxe
+# has a boot-from-disk rule of its own, written as its own branch, so the xNBA
+# node above passing says nothing about this one. There is no second stage to
+# ask for here: pxelinux is what the firmware itself would be given.
+[step pxe-firmware-discover]
+type         = discover
+mac          = %(booted_pxe_mac)s
+client_arch  = 0x0000
+vendor_class = PXEClient:Arch:00000:UNDI:002001
+assert =
+    msgtype  == OFFER
+    yiaddr   == %(booted_pxe_ip)s
+    bootfile absent
 
 [scenario booted-node-is-not-sent-a-script-on-the-second-stage]
 description = S-31: And is not sent one when the loader asks again as xNBA
@@ -62,3 +85,4 @@ assert =
     msgtype  == OFFER
     yiaddr   == %(booted_ip)s
     bootfile != %(booted_script)s
+    bootfile absent

xCAT-test/dhcptest/spec.md

@@ -356,10 +356,20 @@ Scenario: A ScaleMP client is given the ScaleMP loader
 @S-31
 Scenario: A node told to boot from disk is not handed a loader
   Given a node whose chain.currstate is "boot" or "iscsiboot"
-  When it discovers
+  And it has no iSCSI target
+  When it discovers, with or without the xNBA user class
   Then it is offered its address
-  And it is not handed an xNBA script
-  # dhcp.pm:1171 -- otherwise a booted node would netboot forever.
+  And it is named no boot file at all, whatever its netboot method
+  # dhcp.pm:1139 -- otherwise a booted node netboots forever. "No boot file at
+  # all" rather than "not its own script": a node handed the stage-1 loader
+  # asks again as an xNBA second stage and is answered with the network's
+  # script, so stopping only the per-node script stops nothing. Both netboot
+  # methods that have a boot-from-disk rule, xnba and pxe, are covered by the
+  # same sentence for the same reason.
+  #
+  # An iSCSI node is the exception and keeps its loader: its root disk is on
+  # the network and gPXE is what attaches it. ISC gates that on $doiscsi; Kea
+  # leaves those MACs out of the xcat-localboot class.
 
 @S-32
 Scenario: A Windows UEFI install defers to the proxyDHCP daemon

xCAT-test/dhcptest/src/dhcptest_lib/assertions.py

@@ -28,6 +28,10 @@ DERIVED_TARGETS = frozenset(["bootfile"])
 #: Operators that take no value.
 NULLARY_OPS = frozenset(["present", "absent"])
 
+#: Operators asserting that the reply is *not* something, which a reply that
+#: says nothing on the subject satisfies.
+NEGATIVE_OPS = frozenset(["!=", "not-in"])
+
 OPS = frozenset([
     "==", "!=", "in", "not-in", "present", "absent",
     "matches", "contains", "starts-with", "ends-with",
@@ -142,6 +146,15 @@ def evaluate(assertion, reply, context, extras=None):
                       actual="present" if present else "absent")
 
     if not present:
+        # A negative comparison is satisfied by a target that is not there at
+        # all: a reply naming no boot file has certainly not named the node's
+        # install script. Failing it instead would make "is not X" quietly
+        # stronger than it reads, and would fail the one reply that is most
+        # obviously right. `absent` remains the assertion to write when the
+        # absence itself is the point.
+        if assertion.op in NEGATIVE_OPS:
+            return Result(assertion, True, expected=expected, actual=None,
+                          detail="%s is not present in the reply" % (assertion.target,))
         return Result(assertion, False, expected=expected, actual=None,
                       detail="%s is not present in the reply" % (assertion.target,))
 

xCAT-test/dhcptest/tests/test_assertions.py

@@ -83,6 +83,18 @@ class Evaluation(unittest.TestCase):
         self.assertFalse(self.check("bootfile matches ^$", nothing).ok)
         self.assertTrue(self.check("bootfile present").ok)
 
+    def test_a_negative_comparison_holds_against_a_target_that_is_absent(self):
+        # "is not the node's install script" is satisfied by a reply that names
+        # no boot file at all -- which is the strongest way a server can
+        # satisfy it. The same reading applies to `not-in`. Everything else
+        # still fails on an absent target, because there is nothing to compare.
+        nothing = offer(file="", options={54: "10.0.0.1"})
+        self.assertTrue(self.check("bootfile != pxelinux.0", nothing).ok)
+        self.assertTrue(self.check("option:67 != pxelinux.0", nothing).ok)
+        self.assertTrue(self.check("option:66 not-in 10.0.0.0/24", nothing).ok)
+        self.assertFalse(self.check("bootfile == pxelinux.0", nothing).ok)
+        self.assertFalse(self.check("bootfile starts-with http://", nothing).ok)
+
     def test_options_by_number_and_name(self):
         self.assertTrue(self.check("option:54 == 10.0.0.1").ok)
         self.assertTrue(self.check("option:server_id == 10.0.0.1").ok)

xCAT-test/unit/dhcp_kea_plugin_intent.t

@@ -1449,9 +1449,13 @@ foreach my $case (@invalid_mac_cases) {
     # A node that has an operating system now, and a Windows UEFI install
     # waiting on the proxyDHCP daemon, both have to be handed no boot file.
     # ISC writes filename = "" into the node's host block, which outranks the
-    # subnet chain. On Kea only a reservation outranks a class, so if the
-    # reservation stays silent the subnet's architecture classes answer instead
-    # and an installed node netboots forever.
+    # subnet chain.
+    #
+    # The empty boot-file-name below is necessary and not sufficient on Kea:
+    # Kea reads an empty string as "not specified" and falls through to the
+    # classes, so the reservation alone does not stop the architecture classes
+    # answering. What stops them is the xcat-localboot class, asserted further
+    # down.
     no warnings 'redefine';
     local *xCAT_plugin::dhcp::proxydhcp = sub { return 1; };
 
@@ -1542,4 +1546,98 @@ foreach my $case (@invalid_mac_cases) {
         'the node gets no xNBA class that would pre-empt the deferral' );
 }
 
+{
+    # spec.md S-31. Withholding the node's own classes and writing an empty
+    # boot-file-name into its reservation does not stop an installed node being
+    # netbooted: Kea reads the empty string as "not specified" and the classes
+    # everybody shares match on architecture and hand it a loader anyway. The
+    # machine then comes back as an xNBA second stage and is answered with the
+    # network's script -- so it reinstalls itself on every power cycle, and each
+    # such boot looks like a successful one.
+    #
+    # The MACs therefore go into one class that every boot-naming class is
+    # written to exclude.
+    my %tables = (
+        noderes => DHCPKeaResTable->new(
+            {
+                booted => { netboot => 'xnba' },
+                bootpx => { netboot => 'pxe' },
+                san01  => { netboot => 'xnba' },
+            }
+        ),
+        mac => DHCPKeaResTable->new(
+            {
+                booted => { mac => 'aa:bb:cc:dd:ee:05' },
+                bootpx => { mac => 'aa:bb:cc:dd:ee:06' },
+                san01  => { mac => 'aa:bb:cc:dd:ee:07' },
+            }
+        ),
+        chain => DHCPKeaResTable->new(
+            {
+                booted => { currstate => 'boot' },
+                bootpx => { currstate => 'boot' },
+                san01  => { currstate => 'iscsiboot' },
+            }
+        ),
+        iscsi => DHCPKeaResTable->new(
+            { san01 => { server => '192.0.2.9', target => 'iqn.2024-01.test:san01', lun => 0 } }
+        ),
+    );
+
+    no warnings 'redefine';
+    local *xCAT::Table::new = sub {
+        my ( $class, $name ) = @_;
+        return $tables{$name};
+    };
+    local *xCAT_plugin::dhcp::next_server_for_node = sub { return ( '192.0.2.1', '192.0.2.1' ); };
+
+    # A class of the shape the architecture classes have: it names a boot file
+    # and so must learn to exclude these nodes.
+    my $config = {
+        Dhcp4 => {
+            'client-classes' => [
+                { name => 'xcat-bios', test => 'option[93].hex == 0x0000', 'boot-file-name' => 'xcat/xnba.kpxe' },
+                { name => 'xcat-opal', test => "option[93].hex == 0x000e", 'option-data' => [] },
+            ],
+        },
+    };
+
+    ok( xCAT_plugin::dhcp::kea_sync_node_client_classes( $config, [ 'booted', 'bootpx', 'san01' ] ),
+        'a node that is to boot from its disk changes the configuration' );
+
+    my @names = map { $_->{name} } @{ $config->{Dhcp4}{'client-classes'} };
+    my ($localboot) = grep { $_->{name} eq 'xcat-localboot' } @{ $config->{Dhcp4}{'client-classes'} };
+    ok( $localboot, 'the installed nodes land in a class of their own' );
+    is( $localboot->{test},
+        'pkt4.mac == 0xaabbccddee05 or pkt4.mac == 0xaabbccddee06',
+        'both netboot methods are in it, and the iSCSI node is not: its root disk is on the network and gPXE is what attaches it' );
+    is( $names[0], 'xcat-localboot',
+        'and it is defined first, because Kea rejects a member() test naming a class below it' );
+
+    my %by_name = map { $_->{name} => $_ } @{ $config->{Dhcp4}{'client-classes'} };
+    is( $by_name{'xcat-bios'}{test},
+        "(option[93].hex == 0x0000) and not member('xcat-localboot')",
+        'a class that names a boot file stops matching them' );
+    is( $by_name{'xcat-opal'}{test}, 'option[93].hex == 0x000e',
+        'a class that names none is left alone' );
+
+    # Regenerating must not wrap the test one layer deeper every time.
+    xCAT_plugin::dhcp::kea_sync_node_client_classes( $config, ['booted'] );
+    %by_name = map { $_->{name} => $_ } @{ $config->{Dhcp4}{'client-classes'} };
+    is( $by_name{'xcat-bios'}{test},
+        "(option[93].hex == 0x0000) and not member('xcat-localboot')",
+        'and running makedhcp again writes the same test, not a nested one' );
+    is( $by_name{'xcat-localboot'}{test},
+        'pkt4.mac == 0xaabbccddee05 or pkt4.mac == 0xaabbccddee06',
+        'a makedhcp for one node leaves the rest of the cluster in the class' );
+
+    # With the last of them gone the guard has to come off: a member() test
+    # naming a class that no longer exists is a configuration Kea refuses.
+    xCAT_plugin::dhcp::kea_remove_node_client_classes( $config, [ 'booted', 'bootpx' ] );
+    %by_name = map { $_->{name} => $_ } @{ $config->{Dhcp4}{'client-classes'} };
+    ok( !exists $by_name{'xcat-localboot'}, 'with no installed node left the class goes' );
+    is( $by_name{'xcat-bios'}{test}, 'option[93].hex == 0x0000',
+        'and the guard naming it goes with it' );
+}
+
 done_testing();