From 698bccf493e81e7c2e1cc61a3175bf34c29f4263 Mon Sep 17 00:00:00 2001 From: Daniel Hilst <392820+dhilst@users.noreply.github.com> Date: Fri, 9 Oct 2026 13:30:47 -0300 Subject: [PATCH] fix(xcat-core): the netboot restore test cannot tell which chdef failed The bats stub returned one exit code for every "chdef netboot=", and the helper calls chdef twice: once to set the method to ipxe, and once to put the saved method back. With the stub failing both, "a netboot method that cannot be put back fails the case" passed on the FORWARD failure, so removing restore_rc=1 from the restoring branch left all 18 tests green. The test named a defect it could not detect. The stub now counts its netboot calls and fails the one the test names, so the forward chdef succeeds. The test asserts that forward call is in the call log, and matches the message only the restoring branch prints. Removing restore_rc=1 now fails test 8 and no other. docs/source/advanced/ipxe/index.rst said both methods run the same boot scripts, which stopped being true when the shim line became conditional on the method. It now says what differs, and states that Secure Boot reaches netboot=ipxe only: xnba.efi is unsigned, and grub2 nodes are out of scope. Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com> --- docs/source/advanced/ipxe/index.rst | 9 +++++++- .../bats/ipxe_secureboot_shim_restore.bats | 21 +++++++++++++++---- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/docs/source/advanced/ipxe/index.rst b/docs/source/advanced/ipxe/index.rst index f605eabaa..40df83bcb 100644 --- a/docs/source/advanced/ipxe/index.rst +++ b/docs/source/advanced/ipxe/index.rst @@ -6,7 +6,14 @@ xCAT boots x86 nodes with iPXE through two ``netboot`` methods: * ``ipxe``: the unmodified iPXE release in the ``ipxe-xcat`` package. * ``xnba``: the xCAT Network Boot Agent in the ``xnba-undi`` package, a patched iPXE. This method is deprecated, and a later release removes it. -Both methods run the same boot scripts. ``nodeset`` writes them under ``xcat/ipxe/nodes`` for ``ipxe`` and under ``xcat/xnba/nodes`` for ``xnba``. The xCAT packages install both ``ipxe-xcat`` and ``xnba-undi``. +The two methods share the script writer, so the scripts are the same except where the method +decides otherwise. ``nodeset`` writes them under ``xcat/ipxe/nodes`` for ``ipxe`` and under +``xcat/xnba/nodes`` for ``xnba``. The xCAT packages install both ``ipxe-xcat`` and ``xnba-undi``. + +**Secure Boot applies to ``netboot=ipxe`` only.** A node with ``netboot=ipxe`` gets a ``shim`` +line before the kernel, so the firmware verifies what it loads. A node with ``netboot=xnba`` +does not: ``xnba.efi`` is unsigned, and no shim can chain to it. ``grub2`` and ``grub2-http`` +nodes, which is every ppc64le, aarch64 and riscv64 node, are outside this chapter. New x86 nodes get ``netboot=ipxe``: node discovery, profile-based node definitions and the x86 node templates of ``mkdef --template`` set it. diff --git a/xCAT-test/bats/ipxe_secureboot_shim_restore.bats b/xCAT-test/bats/ipxe_secureboot_shim_restore.bats index a5b7d617e..614dcdf6e 100644 --- a/xCAT-test/bats/ipxe_secureboot_shim_restore.bats +++ b/xCAT-test/bats/ipxe_secureboot_shim_restore.bats @@ -37,7 +37,9 @@ setup() export STUB_LSDEF_FAIL_ATTR= export STUB_NODESET_RESTORE_RC=0 export STUB_CHDEF_RC=0 - export STUB_CHDEF_NETBOOT_RC=0 + # Which chdef netboot= call to fail, counted from 1. The helper sets the method and later + # puts it back, so one return code for both cannot say which call failed. + export STUB_CHDEF_NETBOOT_FAIL_NTH=0 # The netboot the node is declared with. Four of the six confs that carry this case declare # their compute node netboot=xnba, so this is not a hypothetical value. export STUB_NETBOOT=ipxe @@ -46,6 +48,7 @@ setup() export STUB_UEFI="$TFTPDIR/xcat/ipxe/nodes/${NODE}.uefi" export STUB_SHIM_LINE="shim http://\${next-server}/install/$OS/x86_64/EFI/BOOT/BOOTX64.EFI" export STUB_CALLS="$CALLS" + export STUB_NETBOOT_COUNT="$BATS_TEST_TMPDIR/netboot-count" export STUB_TFTPDIR="$TFTPDIR" export STUB_INSTALLDIR="$INSTALLDIR" export STUB_OS="$OS" @@ -113,7 +116,12 @@ STUB #!/bin/bash echo "chdef $*" >>"$STUB_CALLS" case "$*" in - *netboot=*) exit "$STUB_CHDEF_NETBOOT_RC" ;; + *netboot=*) + n=$(( $(cat "$STUB_NETBOOT_COUNT" 2>/dev/null || echo 0) + 1 )) + echo "$n" >"$STUB_NETBOOT_COUNT" + [ "$n" = "$STUB_CHDEF_NETBOOT_FAIL_NTH" ] && exit 1 + exit 0 + ;; esac exit "$STUB_CHDEF_RC" STUB @@ -198,11 +206,16 @@ STUB # which is the same defect the provmethod and nodeset restorations already report. @test "a netboot method that cannot be put back fails the case" { STUB_NETBOOT=xnba - STUB_CHDEF_NETBOOT_RC=1 + # Fail the SECOND netboot chdef, which is the restoring one. Failing both cannot tell a + # broken restoration from a forward chdef that never ran. + STUB_CHDEF_NETBOOT_FAIL_NTH=2 run "$SCRIPT" "$NODE" [ "$status" -ne 0 ] [[ "$output" != *IPXE_SHIM_OK* ]] - [[ "$output" == *IPXE_SHIM_FAIL* ]] + # The forward chdef must have succeeded, or this measures the wrong call. + grep -q "^chdef -t node -o $NODE netboot=ipxe$" "$CALLS" + # Only the restoring branch prints this; the forward one does not. + [[ "$output" == *"keeps the netboot method this case set"* ]] } # The case file now requires the OK token, so an exit 0 without it is a pass the run cannot