buildrpms_source_only.t's CLI half failed in CI: the runner has no
Parallel::ForkManager, so buildrpms.pl aborted at compile time and never
reached the option check the test is about. The module is needed only by
the test suite -- buildrpms.pl is not a runtime dependency of any package
-- so it goes in the workflow apt list.
The same half also escaped its scratch tree. Before buildrpms.pl looks at
@ARGV it rewrites the tracked Gitinfo in its working directory and creates
$HOME/rpmbuild, so running it from the checkout left the tree dirty and
reached into the developer's home to exercise argument parsing. It now
runs from a staged copy with HOME pointed at the sandbox.
Exit 2 is pinned rather than "non-zero", though perl also exits 2 on a
compile abort -- which is exactly how this assertion stayed green in CI
while the program could not load. The message assertion is what separates
the two, and the comment now says so.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Four fixes from @viniciusferrao's review plus the CI break his review predates.
orig tarball version. dpkg looks for <source>_<upstream>.orig.tar.gz with no
Debian revision, and the call site passed the full Version-Release. The rule now
lives in BuildUtils::upstream_version and orig_tarball_name applies it, so the
call site cannot get it wrong whichever string it is handed. Currently dormant --
every package is Format: 1.0, so the quilt branch does not run, which is why the
differential build did not catch it.
--dest could write to the filesystem root. Cwd::abs_path returns undef when a
PARENT component is missing (a missing leaf is fine), and the caller interpolated
that, so `--dest /no/such/parent/out` became `/debs` and `/xcat-core` at /.
Replaced with BuildUtils::resolve_dest, which is rel2abs and purely lexical --
correct for an output directory that does not exist yet.
Generated debian/control left behind. xCAT-genesis-scripts has no debian/control
of its own; it is generated from control-<arch>. The cleanup restored only files
that already existed, so the generated one stayed. Worse than dirty: ppc64el ran
last, so the restore put back the amd64 BACKUP and the leftover was the wrong
architecture's control, which a later single-arch build would have started from.
with_prepared_tree now records created files and removes them. Verified by a real
build: the checkout is byte-clean afterwards, matching the oracle.
CI install step. build-ubunturepo wrote its repo to $curdir/../../xcat-core,
which under GitHub's work/<repo>/<repo> layout IS $RUNNER_WORKSPACE, so
install_xcat's `./mklocalrepo.sh` happened to be in the directory it chdir'd to.
builddebs.pl writes inside the checkout instead -- that outside-the-checkout path
is what used to rm -rf the tree -- so install_xcat now names the script by its
real location and fails with a clear message if the build produced no repository.
This is what reddened xcat_pr_test at 2m13s; the builder itself was fine (the
exact CI invocation, `./builddebs.pl --force` with no --dest, returns 0 with all
14 packages).
The executable bit was already fixed before the review landed.
Both new helpers are tested and mutation-verified: not stripping the revision
reddens 3 assertions, swapping rel2abs back to abs_path reddens 2. Equivalence
re-measured after these changes -- all 14 packages identical to build-ubunturepo
in control and in every non-changelog file by md5.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
builddebs.pl was written against build-ubunturepo but never run beside it. Keeping
the old script in the tree for one more cycle makes the comparison repeatable, and
running the two on xcat-master-ub found four defects that no unit test would have.
* builddebs.pl was not executable. github_action_xcat_test.pl runs
`sudo ./builddebs.pl`, so CI would have failed with Permission denied.
* .deb files were collected after every ARCHITECTURE. dpkg-genbuildinfo reads
the sibling artifacts of the source package it is building, so moving the
amd64 .deb away before ppc64el ran killed the second build with
dpkg-genbuildinfo: error: cannot fstat file ../xcat_..._amd64.deb
Collection now happens once a package's last architecture is done.
* A checkout dirtied by an aborted run poisoned the next one: a leftover
.changes made dpkg-genbuildinfo name an architecture the run had not reached.
The dpkg output is now cleared at start.
* The maintainer was xcat@xcat.org where build-ubunturepo uses
xcat-build@xcat.org, and a Release file in the tree was ignored.
buildrpms.pl WRITES Release, so a pipeline building both would have stamped
its debs with a different release from its rpms. Both corrected.
Equivalence, measured rather than assumed. Both builders were run on
xcat-master-ub from the same tree:
package set identical, 14/14, same names
control metadata identical, 14/14
payload contents identical, 14/14
changelog.Debian differs, 2 packages -- deliberately, see below
Byte-identity is NOT the criterion, because neither implementation has it: two
consecutive runs of build-ubunturepo on the same tree agree on 0 of 14 packages.
A .deb records the build wall-clock time in its ar member mtimes and
SOURCE_DATE_EPOCH does not reach dpkg-deb here, so every run differs from every
other. That is a pre-existing property of the Debian build, not a regression, and
worth fixing separately.
The changelog difference is build-ubunturepo being wrong. Its
sed -i "s/^ -- .*/ -- $DEBFULLNAME <$DEBEMAIL> $deterministic_date/"
carries no line address, so it rewrites EVERY trailer in debian/changelog:
"OCF xCAT <xcat@ocf.co.uk> Mon, 25 Oct 2010" ships as "xCAT Build
<xcat-build@xcat.org> Tue, 01 Sep 2026". It falsifies the authorship and dates of
the 2008 and 2010 releases. builddebs.pl rewrites only the top stanza. Matching
byte-for-byte would mean reproducing the defect, so this difference stays.
(xcat-vlan appeared to differ under `diff -r`; that was diff reporting dangling
symlinks in both trees. Its member listing is identical.)
build-ubunturepo is documented as retained-for-comparison and not to be extended.
It goes once the Ubuntu CD pipelines call builddebs.pl.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
build-ubunturepo was 710 lines of shell doing the Debian half of what
buildrpms.pl does for rpms, with no code in common and a different CLI. It also
carried paths that are dead: GSA uploads, the PROMOTE/PREGA release flows, and a
-d mode that built an xcat-dep repository from a different project's packages.
builddebs.pl replaces it and mirrors buildrpms.pl -- Getopt::Long options, one
package list, build then index then sign -- so the two builders read the same way
and share BuildUtils.pm.
The design rests on one fact: xcat-core debs are Perl. They are byte-identical
for every Ubuntu release, so they are built ONCE and the same files are published
into every codename. Only xCAT, xCATsn and xCAT-genesis-scripts carry an
architecture, and there the difference is packaging metadata, not compiled
output. That is why this needs no sbuild and no per-codename chroot -- unlike
xcat-dep, whose packages are compiled and genuinely differ per release.
BuildUtils.pm holds what both builders need and what was worth making testable:
the Version-Release derivation from the commit time, the xCAT-probe helper
staging, the deb arch and dist tables, the debian/control version pinning, the
changelog rewrite, the reprepro conf generation, and the build lock. Every
function is pure or takes its side effect as an argument, so build_utils.t (45
assertions) drives each one rather than grepping a builder for evidence that it
is called. Verified by mutation: shrinking the arch table reddens 1, dropping
the /g from the control pin reddens 2.
The env-var CLI maps to options: BUILDALL=1 -> --force, GPGSIGN=1 -> --gpg-sign,
GPG_HOME -> --gpg-home, DEST -> --dest, DISTS -> --dist (repeatable). UP=0 has no
equivalent because uploading is gone -- the CD pipeline's deploy step publishes.
Callers updated: github_action_xcat_test.pl and travis.pl. The comment in
github_action_xcat_test.pl explaining why CI copies the tree before building is
corrected -- build-ubunturepo rm -rf'd $curdir/../../xcat-core, which under
GitHub's work/<repo>/<repo> layout is the checkout's own parent; builddebs.pl
writes under dist/debs inside the checkout and restores every file it edits, so
the copy is now only isolating the tests from build residue.
Two tests moved with it. build_ubunturepo_lock.t extracted the lock out of the
shell with a regex and ran that; the lock is now a function, so builddebs_lock.t
calls it -- and asserts what actually matters, that two builds of one checkout
fail fast while two builds of different checkouts run concurrently.
ubuntu_2604_pkglist.t asserted that resolute appeared in a shell fragment of
build-ubunturepo's source; it now asks BuildUtils for the release list and checks
a resolute stanza reaches conf/distributions. That assertion would have passed on
any file containing the fragment and broken on a reflow that changed nothing.
Verified: prove -r xCAT-test/unit fails on 6 files here against 7 on
upstream/master, the difference being apache_config_sources.t, fixed by the
preceding commit. The remaining 6 are missing DB modules on the machine that ran
it and are identical on both.
NOT done here, and required before this can merge: the Ubuntu core CD pipelines
still invoke ./build-ubunturepo (ci/ubuntu/Jenkinsfile.core-ubuntu-{devel,stable}
in VersatusHPC/xcat-core-ci-cd, and the inline script in each live Jenkins job).
Those must be switched to builddebs.pl in the same change window.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
buildrpms.pl is what actually builds xcat-core now: every core CD pipeline calls
it (ci/{el,suse,ubuntu}/Jenkinsfile.core-*), and it reaches none of these
scripts. What is left of them is a second, diverging build path that nothing
runs -- buildcore.sh has no caller at all (its one mention in
Jenkinsfile.core-el-stable is a comment), makerpm's only caller is
buildlocal.sh, and buildlocal.sh has no caller.
That divergence broke master. #7774 added a source-only mode to these scripts
rather than to buildrpms.pl, giving makerpm an unguarded
. "$(dirname "$0")/build-utils/source-only.sh"
at line 13, used on every build path (xcat_configure_rpm_build_mode,
xcat_announce_build), not only under SRCONLY. apache_config_sources.t stages
makerpm into a fixture that creates build-utils/ but copies only
sync-xcat-apache-configs into it, so makerpm died at line 13 and nothing reached
SOURCES/. Because prove -r xCAT-test/unit runs the whole suite, that reddened
xcat_pr_test on every open PR. It merged on a green run from 2026-08-27;
apache_config_sources.t landed 2026-08-30, and nothing rebuilt it in between.
Removed: makerpm, buildcore.sh, buildlocal.sh, build-utils/source-only.sh,
build-utils/buildcore-source-only.sh, and the tests that covered only them
(makerpm_source_only.t, buildcore_source_only.t). The makerpm half of
apache_config_sources.t goes with makerpm; its sync-xcat-apache-configs half
stays, because buildrpms.pl invokes that helper directly. The source-only
capability is preserved as buildrpms.pl --source-only, in the preceding commit.
build-ubunturepo is deliberately NOT touched. It is not a legacy script: both
Ubuntu core CD pipelines invoke it, github_action_xcat_test.pl:325 invokes it on
every PR, and it was last fixed this week. buildrpms.pl is RPM-only -- mock and
createrepo_c, no dpkg path -- so there is nothing to migrate it to. Giving
Debian builds an equivalent modern builder is separate work.
Verified: prove -r xCAT-test/unit before and after differs by exactly one file
-- apache_config_sources.t fails on upstream/master (4a0d9e0bb, tests 64-65) and
passes here. The other six failures are identical on both and are missing DB
modules on the machine that ran it. No remaining reference to any removed file
except the one comment in buildrpms.pl naming where its templates came from.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
buildrpms.pl already produces a source rpm for every package on every run --
buildall() is createmockconfig -> buildsources -> buildspkgs (mock --buildsrpm)
-> buildpkgs (mock --rebuild). Source-only is that sequence without the last
step, so it belongs here rather than in a parallel implementation: the spec, the
staged sources and the mock root are identical either way, and anything built
beside them can drift from what a real build does.
--source-only stops after buildspkgs. Two things downstream had to learn about
it, and both are about not publishing something untrue:
- index_repo no longer re-indexes the binary directory. Running createrepo_c
over a directory with no binaries in it would replace working metadata with
metadata for an empty repository -- a repo that resolves nothing. The srpm
index is still regenerated.
- write_repo_metadata_dir emits nothing. The .repo file and buildinfo describe
an installable binary repository, which this mode does not produce.
--source-only with --merge-core-repos is refused: one builds packages, the other
assembles per-arch trees that are already built.
buildrpms.pl cannot be loaded by a test -- it runs mkdir, git and read_text at
file scope -- so the two routines whose behaviour changed are lifted out with a
regex and eval'd into a scratch package with their collaborators stubbed, per
the code standard, with BAIL_OUT if the extraction stops matching. The CLI
contract is exercised by running the real program. Verified by mutation:
removing the index_repo guard reddens 2 of the 7 assertions.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Scan the z/VM, bmcconfig, energy, PPC configuration, CIM and z/VM
utility sources. Fail when a printSyslog line or a checkSSH_Rc command
string carries a password variable, when a directory query output logs
without the redactor, when the masked log lines lose their mask or
their position built operand, when the executed commands lose their
real values, when the CIM verbose dump returns to the raw request, or
when the attribute and access messages hold a password value again.
Drive the real redact_directory_entry with sentinel USER, IDENTITY,
IDENT, MDISK range, DEVNO and V-DISK, APPCPASS and keyword records in
both spellings, with and without one or more comment stars, cover the
COMMAND statement whole, keep the statements beside the passwords, and
prove that a passwordless record never masks across the line into the
next record. Assert that both clone loops redact the query output at the
source, that the directory helpers hand a redacted copy to the failure
checker, and that every error branch behind the output check, the
directory fetch, the mini disk fetch and the four disk list callers,
echoes the redacted text.
Six modules wrote passwords to their own log and diagnostic messages,
outside the daemon redaction pipeline. The z/VM plugin logged each
smcli command line through printSyslog, with the disk read, write and
multi passwords, the image password, the provision root password and
the page volume parm disk password, passed the real disk passwords to
checkSSH_Rc, which echoes the command to syslog and to the client on
failure, and logged raw directory entries whose USER and MDISK
statements carry the logon and disk passwords. The bmcconfig plugin
logged the BMC password in its attribute report, in syslog and in the
command response. The energy plugin logged the HCP password in a
verbose message, and the CIM utilities dumped the whole HTTP request,
with its basic authorization header, to the verbose callback. The PPC
configuration module logged the HMC, FSP and BPA passwords in its
verbose credential reports.
Mask the passwords in the logged text. The executed commands keep the
real values. The page volume log string is built by operand position,
so a decoy value in another operand cannot divert the mask. The
checkSSH_Rc calls receive the masked command string, as the routine
documentation asks. Add redact_directory_entry to the z/VM utilities.
The routine masks the USER, IDENTITY and IDENT logon password, the
MDISK passwords after the access mode in the range form and in the
DEVNO, V-DISK and T-DISK forms, the APPCPASS statement, and the
keyword password assignments in the short and the full spelling. The
match separators stay on one line, so a record without passwords never
masks the record below it, and one or more comment stars do not hide a
credential record from the rules. The COMMAND statement masks whole,
because it can start any CP command with an inline password. Every directory query sink logs
through it,
and the clone loops redact the query output at the source, because the
failure checker and the retained disk list reuse the text. The
directory helpers keep their raw return value for the callers and hand
a redacted copy to the failure checker. Every error branch that echoes
a fetched record after the output check does so through the redactor,
because a password can spell an error word and trip the check: the
directory fetch, the mini disk keyword fetch, and the four disk list
callers. The CIM dump masks
the authorization header. The bmcconfig report now names the password
state, set or missing, which the report needs for diagnosis.
configeth's netplan arm took the interface down on every reconfigure, but the
`netplan_apply` that brings it back is gated on reboot_nic_bool. During the
diskful provision postscripts stage reboot_nic_bool is 0, so the link went down
and stayed down until the node rebooted -- and it is the install NIC the
postscripts are still talking over.
Gate the down on reboot_nic_bool as well, so the two halves are paired the way
the redhat arm already pairs them. Nothing is lost by skipping it: the drop-in
is written either way and takes effect at boot, which is the same reasoning the
existing comment gives for skipping the apply.
The ifupdown arm is deliberately left alone -- it answers its `ifdown` with an
unconditional `ifup`, so it never leaves the link down.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
On a netplan-rendered node the reconfigure path takes the interface down
unconditionally, while the `netplan apply` that brings it back is gated on
reboot_nic_bool. In the diskful provision postscripts stage reboot_nic_bool is
0, so configeth downs the NIC and nothing brings it back until the node
reboots -- over the very interface the postscripts are talking on.
The other two arms pair their halves: the ifupdown arm answers `ifdown` with an
unconditional `ifup`, and the redhat arm gates both the down and the up. Only
the netplan arm takes one half.
Nothing in the suite covers it. This drives the two blocks configeth actually
executes -- the down-selection inside the modify branch and the restart block
underneath it -- with `ip`, `ifdown`, `ifup` and `netplan` shadowed by shell
functions, which bash resolves ahead of $PATH, so every call is recorded rather
than run and the host's network is never touched.
Six of the eight assertions pass already and are there to pin the arms that are
correct: redhat gating both halves, ifupdown pairing its own, and the
reboot_nic_bool=1 path doing the down before the apply. The two that fail are
the bug.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Cover the order that names the install device: installnic, then primarynic,
then mac.mac. Either attribute may name an interface or carry an address, and
the keyword mac returns to mac.mac.
Cover the device the kickstart names for each of those inputs. A node that
sets neither attribute keeps the address it has today. Cover the defect the
change closes, where a mac.mac entry that holds several untagged addresses
resolves to the last of them.
Cover that Ubuntu keeps its own pair of a name and an address over the same
resolution, and that the unique local address still comes from the hardware
address.
noderes.installnic names the adapter that deploys the operating system. The
kickstart network line ignored it and named the adapter from mac.mac alone, so
a node that sets installnic got a kernel command line that obeys installnic and
a kickstart that configures a different adapter. On a node with more than one
adapter the installer then brings up the wrong one and cannot reach the
repository.
Name the device from the shared resolution, which gives the interface name when
installnic or primarynic names one, and the address otherwise. A node that sets
neither attribute keeps the address it has today.
The unique local address still comes from the hardware address, because
autoulaaddress builds the address from it.
The order that names the install device is noderes.installnic, then
noderes.primarynic, then mac.mac. gen_net_boot_params owns that order for the
netboot kernel parameters. Only the Ubuntu template reused it. Every other
install template reads mac.mac on its own.
Move the reuse into install_device_params so that any install template can
share it. subiquity_install_netcfg keeps its own name and its own return
value, because netplan needs the pair of a name and an address. Behaviour
does not change.
Five comments in the discovery path explained a consequence chain or
justified the code to a reviewer rather than stating what is not visible
locally: the payload protocol the code below already shows, a rationale
for the missing boot loader message, the two Genesis compression
variants, the HTTP and TFTP entry pair, and the :noboot divergence from
the PXELINUX files. Say the non-obvious part once.
Comments only; no executable line changes.
Firmware configured for UEFI HTTP boot sends client architecture 28
(0x001c) and only accepts an offer whose boot file is a URL and whose
reply carries the HTTPClient vendor class; it ignores the PXE offer that
answers architecture 27. riscv64 nodes therefore could not boot at all
from firmware set up that way.
Answer them with the same grub2 image over HTTP. The URL has to name the
management node address of the network the request came in on, so the
class belongs to the subnet, like the POWER OPAL and xNBA network classes
next to it, and it is only offered while the boot loader is actually
published. The ISC backend renders the matching subnet branch.
This covers every client without a reservation, which is the discovery
case. A node that nodeset has configured keeps its per-node boot loader
over TFTP, as on the other architectures.
Daniel Hilst
Vinícius Ferrão
Vinícius Ferrão