test(xcat-core): the Genesis build-root test stops the whole suite, and its comments over-explain

genesis_ubuntu_build_root.t called BAIL_OUT at four places where an
extraction stopped matching. prove stops every remaining file on a
bail-out, so one stale regex in this file hides the results of the tests
that would have run after it. die is just as loud and costs only this
file.

The branch also states one fact in four places. That dracut copies the
kernel out of the root it runs in appears in the builddebs.pl header, in
its Genesis section, in BuildUtils.pm and in the builder, each time with
the incident that produced it. The chroot stage directory is explained
twice, once at its declaration and again at its only use. Each fact now
stands where the reader meets it, without the bug report around it.

Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>

build-utils/lib/XCAT/BuildUtils.pm

@@ -532,8 +532,7 @@ sub genesis_target_arch {
 # genesis_build_plan: one Genesis build per codename, for one architecture.
 #
 # The set of codenames comes from the caller, so a pipeline builds exactly the releases it
-# publishes. Returns the codename, the chroot to build it in and the package the build
-# produces, which is what the caller needs to run and to collect.
+# publishes.
 sub genesis_build_plan {
     my ($dists, $arch) = @_;
     my @dists = @{ $dists || [] };
@@ -556,11 +555,9 @@ sub genesis_build_plan {
 # genesis_log_deny_rules / genesis_log_errors: what a Genesis build log says when the build
 # failed but the exit status did not.
 #
-# dracut reports a command it cannot install with a FAILED: line and returns 0, and the
-# builder never reads dracut's result. That is how an image with no dhclient was packaged,
-# signed and published by a command that reported success. apt has the same shape: a
-# missing package leaves a diagnostic and a zero status behind a `|| true`. So the log is
-# the gate, not the exit status.
+# dracut reports a command it cannot install with a FAILED: line and returns 0. apt has the
+# same shape: a missing package leaves a diagnostic and a zero status behind a `|| true`. So
+# the log is the gate, not the exit status.
 my @GENESIS_LOG_DENY = (
     [ qr/\bFAILED:/                    => 'dracut could not install a command' ],
     [ qr/Cannot find module/           => 'a kernel module the build names is absent' ],

builddebs.pl

@@ -10,10 +10,8 @@
 # every codename. Only xCAT, xCATsn and xCAT-genesis-scripts carry an architecture, and
 # even there the difference is packaging metadata, not compiled output.
 #
-# The Genesis image is the one exception, and it is why --genesis exists. dracut copies the
-# kernel, the kernel modules and every command out of the root it runs in, so that image
-# genuinely differs per release and is built once per codename inside that codename's
-# sbuild chroot -- the same chroots xcat-dep builds its compiled packages in.
+# The Genesis image is the one exception. --genesis builds it once per codename, inside
+# that codename's sbuild chroot.
 use strict;
 use warnings;
 use feature 'say';
@@ -91,9 +89,7 @@ $opts{packages} = @cli_packages ? \@cli_packages : \@PACKAGES;
 $opts{dists}    = @cli_dists    ? \@cli_dists    : \@DISTS;
 $opts{gpg_key_name} //= 'xCAT Signing Key';
 
-# The Genesis step is off unless it is asked for, so every run that exists today keeps its
-# behaviour. It takes its own codename list: the Genesis deb is the one package that is not
-# the same file for every release, so the caller says which releases it wants built.
+# The Genesis step is off unless it is asked for, so today's runs keep their behaviour.
 $opts{genesis} = 1 if $opts{genesis_only};
 $opts{genesis_dists} = @cli_genesis_dists ? \@cli_genesis_dists : $opts{dists};
 die "FATAL: --genesis-dist needs --genesis\n" if @cli_genesis_dists && !$opts{genesis};
@@ -283,15 +279,14 @@ sub collect_debs {
 
 # ----------------------------------------------------------- the Genesis deb --
 #
-# Every other xcat-core deb is Perl and is built once for every release. The Genesis image
-# is not: dracut copies the kernel, the kernel modules and every command out of the root it
-# runs in. Built on the build host, one image serves every codename with the build host's
-# kernel -- which is how Ubuntu management nodes came to install an image built from an EL
-# kernel. So this step builds one image per codename, inside that codename's sbuild chroot.
+# dracut copies the kernel, the kernel modules and every command out of the root it runs in,
+# so the Genesis image belongs to the release that built it. One build on the build host
+# serves every codename with the build host's kernel. This step builds one image per
+# codename, inside that codename's sbuild chroot.
 #
 # The chroots are the ones xcat-dep's sbuild-all.pl creates on the Ubuntu build host
-# (<codename>-<arch>-sbuild). They hand out disposable overlay sessions, so what the build
-# installs is discarded and the next codename starts from the pristine base.
+# (<codename>-<arch>-sbuild). A session is a disposable overlay, so the next codename starts
+# from the pristine base.
 
 # Where the build runs inside the chroot. A directory of its own at the chroot root, because
 # every other candidate is shared: /build and /opt/xcat-ci-shared are bind mounts the sbuild
@@ -326,9 +321,8 @@ sub begin_chroot_session {
 
 # genesis_build_log_problems: what the log says went wrong when the exit status did not.
 #
-# dracut prints FAILED: for a command it cannot install and exits 0. Reporting the first few
-# offending lines rather than all of them keeps a build console readable; the log file has
-# the rest and is named in the message.
+# Report the first few offending lines only, so a build console stays readable. The message
+# names the log file that has the rest.
 sub genesis_build_log_problems {
     my ($logfile) = @_;
     my $text = -f $logfile ? read_text($logfile) : '';
@@ -352,14 +346,9 @@ sub build_one_genesis_deb {
     my $err;
 
     eval {
-        # The builder needs its own directory, and Version and Release beside it. Copy them
-        # in rather than bind-mount the checkout: the build rewrites debian/control and
-        # debian/changelog, and it must not rewrite them in the tree the pipeline builds from.
-        #
-        # NOT under /build: the sbuild chroots bind-mount /var/lib/sbuild/build there, so every
-        # session of every chroot shares one directory. Two builds running at once overwrite each
-        # other's copy of the builder, and the output of an earlier run is still in it. A
-        # directory of its own at the chroot root lives in the session overlay and goes with it.
+        # Copy the builder in rather than bind-mount the checkout: the build rewrites
+        # debian/control and debian/changelog, and it must not rewrite them in the tree the
+        # pipeline builds from.
         my $stage = "$root$GENESIS_STAGE";
         sh_or_die("rm -rf " . sh_quote($stage) . " && mkdir -p "
                 . sh_quote("$stage/xCAT-genesis-builder"),
@@ -379,8 +368,8 @@ sub build_one_genesis_deb {
             '--expect-codename', sh_quote($codename), '--outdir', "$GENESIS_STAGE/out";
         my $rc = sh("$cmd > " . sh_quote($logfile) . " 2>&1");
 
-        # The log is read whether or not the command failed: a build that exits 0 with
-        # FAILED: lines in its log is the failure this gate exists for.
+        # The log is read whether or not the command failed: dracut exits 0 with FAILED:
+        # lines in its log.
         my $problems = genesis_build_log_problems($logfile);
         if ($rc != 0) {
             die "FATAL: the Genesis build for $codename failed (exit $rc); log: $logfile\n"

xCAT-genesis-builder/builddeb-genesis-base

@@ -5,8 +5,7 @@
 # in, so this must run inside a root of the target codename. builddebs.pl --genesis starts
 # it in that codename's <codename>-<arch>-sbuild chroot, one build per codename.
 # --expect-codename is what stops a run on the build host: a single build there gives every
-# Ubuntu release the build host's kernel, which is how the EL-built image reached Ubuntu
-# management nodes in the first place.
+# Ubuntu release the build host's kernel.
 #
 # Parallel to xCAT-genesis-base.spec, which does the same for EL.
 

xCAT-genesis-builder/dracut_105/ubuntu/module-setup.sh

@@ -53,13 +53,12 @@ install() {
     dracut_install mount.nfs sshd vi reboot lspci parted screen mkfs mkfs.ext4 mkfs.btrfs
     #dracut_install libvirtd /usr/share/libvirt/cpu_map.xml /usr/bin/qemu-img /usr/libexec/qemu-kvm
     dracut_install mkswap df ifenslave ssh-keygen scp clear
-    # getdestiny makes its request file with mktemp. Without it the node reports no
-    # destiny, so xcatd never moves nodelist.status past powering-on.
+    # getdestiny makes its request file with mktemp.
     dracut_install mktemp
     dracut_install dhclient lldpad
 
     # OpenSSH 9.8 moved the per-connection work into sshd-session, which sshd execs by
-    # absolute path. Without it every connection to Genesis is refused.
+    # absolute path.
     for _sshd_helper in \
         /usr/libexec/openssh/sshd-session \
         /usr/libexec/openssh/sshd-auth \

xCAT-genesis-builder/verify-genesis-payload

@@ -3,9 +3,7 @@
 # verify-genesis-payload [--commands-from <module-setup.sh>] <payload-root> [required-path ...]
 #
 # dracut_install() reports a missing binary and returns, so the module install function keeps
-# going and the image ships without it. Four such holes reached a release: no dhclient, no
-# openssl, no sshd-session and no UTF-8 locale. Check the extracted payload before it becomes
-# an rpm.
+# going and the image ships without it. Check the extracted payload before it is packaged.
 #
 # Paths given on the command line are relative to <payload-root>. --commands-from reads back
 # what the dracut module installs: a bare command name is looked for in the four binary

xCAT-test/unit/genesis_deb_per_codename.t

@@ -1,12 +1,7 @@
 #!/usr/bin/env perl
-# The Genesis image carries the kernel and the kernel modules of the root that built it.
-# One build on the build host therefore gives every Ubuntu release the build host's kernel.
-# builddebs.pl --genesis builds one image per codename, in that codename's chroot.
-#
-# Three decisions are tested here, each on the value the code returns:
-#   the chroot each codename builds in;
-#   the lines in a build log that mean the build failed although it exited 0;
-#   the refusal of the builder to run in a root of another release.
+# The Genesis image carries the kernel and the kernel modules of the root that built it, so
+# builddebs.pl --genesis builds one image per codename, in that codename's chroot. Each
+# assertion here reads the value the code returns.
 use strict;
 use warnings;
 
@@ -52,7 +47,6 @@ is($ppc[0]{target},  'ppc64', 'ppc64el reads its image from the ppc64 directory'
 is(XCAT::BuildUtils::genesis_target_arch('amd64'), 'x86_64',
     'amd64 reads its image from the x86_64 directory');
 
-# A codename given twice is still one build.
 is(scalar(() = XCAT::BuildUtils::genesis_build_plan([qw(noble noble)], 'amd64')), 1,
     'a repeated codename does not build twice');
 
@@ -79,8 +73,7 @@ if (XCAT::BuildUtils->can('deb_belongs_to_dist')) {
 
 # --- the log guard ---------------------------------------------------------------------
 #
-# dracut prints FAILED: for a command it cannot install and exits 0. This is the log of the
-# build that shipped the image with no dhclient.
+# dracut prints FAILED: for a command it cannot install and exits 0.
 my $dracut_log = <<'LOG';
 Installing build dependencies...
 dracut: Executing: /usr/bin/dracut --compress gzip -m xcat base -N -f /tmp/genesis.rfs 6.8.0-45-generic

xCAT-test/unit/genesis_payload_verification.t

@@ -31,8 +31,7 @@ my $good = build_payload(sshd_execs_session => 1, session_helper => 1, tmux => 1
 my ($rc, $err) = run($good, 'usr/sbin/dhclient');
 is($rc, 0, 'a complete payload passes') or diag($err);
 
-# doxcat calls dhclient with ISC flags. The released el9 image carried dhclient.conf and
-# dhclient-script but no dhclient, so Genesis never acquired an address.
+# doxcat calls dhclient with ISC flags. dhclient.conf and dhclient-script are not enough.
 my $nodhcp = build_payload(sshd_execs_session => 1, session_helper => 1, tmux => 1, locale => 1, dhclient => 0, mktemp => 1);
 ($rc, $err) = run($nodhcp, 'usr/sbin/dhclient');
 isnt($rc, 0, 'a payload without dhclient fails');
@@ -55,16 +54,14 @@ my $nolocale = build_payload(sshd_execs_session => 1, session_helper => 1, tmux
 isnt($rc, 0, 'a payload with tmux and no UTF-8 locale fails');
 like($err, qr{C\.utf8}, 'the missing locale is named');
 
-# getdestiny makes its request file with mktemp. Without it the node never reports its destiny,
-# so xcatd never sets nodelist.status and the node stays at powering-on.
+# getdestiny makes its request file with mktemp.
 my $nomktemp = build_payload(sshd_execs_session => 1, session_helper => 1, tmux => 1, locale => 1, dhclient => 1, mktemp => 0);
 ($rc, $err) = run($nomktemp, 'usr/sbin/dhclient');
 isnt($rc, 0, 'a payload without mktemp fails');
 like($err, qr{usr/bin/mktemp}, 'the missing mktemp is named');
 
 # dracut_install reports a missing binary and returns, so every name the dracut module
-# installs has to be checked against the payload. The el10 image shipped with no openssl and
-# getcert waited on it for the life of the node.
+# installs has to be checked against the payload.
 my $module = write_module_setup([qw(openssl wget tar)]);
 my $full = build_payload(sshd_execs_session => 1, session_helper => 1, tmux => 1, locale => 1,
     dhclient => 1, mktemp => 1, commands => [qw(openssl wget tar)]);

xCAT-test/unit/genesis_ubuntu_build_root.t

@@ -1,12 +1,11 @@
 #!/usr/bin/env perl
 # The Ubuntu Genesis build root must carry every command the Ubuntu dracut module marks
-# mandatory. dracut_install reports a missing command and returns, so a command the build
-# root does not supply leaves a hole in the image and the build still exits 0.
+# mandatory. dracut_install reports a missing command and returns 0, so a hole in the image
+# does not fail the build.
 #
-# The mandatory list is read by RUNNING the module: module-setup.sh is sourced with
-# dracut_install shadowed, _dracut_install_opt neutralised (its callers are optional by
-# construction), and install() is called. The package list is read by extracting the
-# REQUIRED_PACKAGES assignment from builddeb-genesis-base and evaluating it.
+# The mandatory list comes from RUNNING the module: module-setup.sh is sourced with
+# dracut_install shadowed, _dracut_install_opt neutralised, and install() called. The
+# package list comes from evaluating the REQUIRED_PACKAGES assignment in the build script.
 use strict;
 use warnings;
 
@@ -23,11 +22,9 @@ plan skip_all => 'builddeb-genesis-base not found' unless -f $builder;
 plan skip_all => 'ubuntu module-setup.sh not found' unless -f $module;
 plan tests => 9;
 
-# Commands the Ubuntu dracut module marks mandatory that a minimal Ubuntu server root does
-# NOT already provide, and the packages that supply each one. Every command here needs one of
-# its packages in the build root or the image ships without it. hwclock has two names because
-# it left util-linux for util-linux-extra in 23.04, and the build root asks apt which name the
-# release it is building for carries.
+# Mandatory commands a minimal Ubuntu server root does NOT already provide, and the packages
+# that supply each one. hwclock has two names: it left util-linux for util-linux-extra in
+# 23.04, and the build root asks apt which name this release carries.
 my %PACKAGES_FOR = (
     dhclient  => ['isc-dhcp-client'],
     ifenslave => ['ifenslave'],
@@ -45,24 +42,20 @@ for my $command (sort keys %PACKAGES_FOR) {
        "the build root installs @{[ join ' or ', @provider ]}, which provides '$command'");
 }
 
-# doxcat asks dhclient for the provisioning lease. An image without it never gets an address,
-# so the node netboots and never reports in -- the failure this test exists for.
+# doxcat asks dhclient for the provisioning lease.
 ok($mandatory{dhclient} && scalar(grep { $_ eq 'isc-dhcp-client' } @packages),
    'the Genesis image can obtain a DHCP lease');
 
-# dracut_install is silent about a hole, so the payload needs its own gate before it is
-# packaged. This is the EL path's behaviour (xCAT-genesis-base.spec runs the same verifier).
+# dracut_install is silent about a missing command, so the payload needs its own gate.
+# xCAT-genesis-base.spec runs the same verifier on the EL path.
 my $text = do { open my $fh, '<', $builder or die "$builder: $!"; local $/; <$fh> };
 like($text, qr{verify-genesis-payload}, 'builddeb-genesis-base verifies the payload it packages');
 
 # The image belongs to the release whose kernel it carries, so the builder must refuse a
-# root of any other release. Without the refusal one build on the build host serves every
-# codename with the build host's kernel.
+# root of any other release.
 like($text, qr{--expect-codename}, 'builddeb-genesis-base takes the release it is building for');
 
-# mandatory_commands($module): source the dracut module with dracut_install shadowed, call
-# install(), and return the bare command names it installs unconditionally. Absolute paths are
-# data files, not commands, and are left out.
+# An absolute path in the install() output is a data file, not a command.
 sub mandatory_commands {
     my ($path) = @_;
     my $dir = tempdir(CLEANUP => 1);
@@ -75,32 +68,31 @@ inst_multiple() { :; }
 inst() { :; }
 dpkg-architecture() { echo x86_64-linux-gnu; }
 . '$path'
-# Every caller of _dracut_install_opt is optional by construction: it installs only what the
-# build root already has. Neutralise it AFTER sourcing so it cannot add to the mandatory set.
+# _dracut_install_opt installs only what the build root already has. Neutralise it after
+# sourcing, so its commands stay out of the mandatory set.
 _dracut_install_opt() { :; }
 install
 BASH
     close $fh;
     my @out = qx{bash '$driver' 2>/dev/null};
-    BAIL_OUT("running install() from $path produced nothing") unless @out;
+    die("running install() from $path produced nothing") unless @out;
     my %seen;
     my @names = grep { !$seen{$_}++ } grep { length && !m{^/} } map { chomp; $_ } @out;
-    BAIL_OUT("install() from $path named no bare commands") unless @names;
+    die("install() from $path named no bare commands") unless @names;
     return @names;
 }
 
-# required_packages($path): the packages the build root installs. The fixed list is the
-# REQUIRED_PACKAGES assignment, evaluated so the value comes from the script itself; a command
-# whose package name changed between releases is added by add_first_available, whose candidates
-# count too -- the script picks whichever one apt knows.
+# Evaluate the assignment rather than parse it, so the list is the value the script uses.
+# add_first_available names the alternatives for a package that was renamed between releases,
+# so its candidates count too.
 sub required_packages {
     my ($path) = @_;
     my $text = do { open my $fh, '<', $path or die "$path: $!"; local $/; <$fh> };
     my ($block) = $text =~ /^(REQUIRED_PACKAGES="[^"]*")/ms;
-    BAIL_OUT("no REQUIRED_PACKAGES assignment in $path") unless $block;
+    die("no REQUIRED_PACKAGES assignment in $path") unless $block;
     my $out = qx{bash -c 'set -u; $block; printf "%s\\n" \$REQUIRED_PACKAGES' 2>/dev/null};
     my @packages = grep { length } split /\s+/, ($out // '');
-    BAIL_OUT("REQUIRED_PACKAGES in $path evaluated to nothing") unless @packages;
+    die("REQUIRED_PACKAGES in $path evaluated to nothing") unless @packages;
     push @packages, grep { length } split /\s+/, $1
         while $text =~ /^add_first_available\s+(.+)$/mg;
     return @packages;