Killing the command is not killing the build. sh() ran the command through /bin/sh,
and cancellation signalled that shell alone -- but dpkg-buildpackage starts workers
of its own, and those survive their shell. The lock was then released while they
were still writing debian/changelog and debian/control, which is the state the lock
exists to prevent: the next build takes the checkout and the two rewrite it
together.
The command now runs in its own process group, so cancellation can take all of it.
Both sides call setpgid, so neither depends on which runs first, and INT and TERM
are blocked across the fork so cancellation cannot land in the window before the
group exists.
Cancellation escalates from the caught signal to KILL, and then CHECKS: a shell that
has exited is not a build that has stopped, so it waits for the whole group to
disappear rather than for the leader to be reaped. If the group is still there after
that, the locks are RETAINED and the process exits non-zero. Releasing a lock while
a worker may still be writing is worse than leaving a lock behind for a person to
clear -- the first corrupts a build, the second stops one.
cancel_build ignores INT and TERM while it runs, so a second Ctrl-C cannot interrupt
the cleanup half way and release the lock early.
sh() also reports a signalled command as 128+signal instead of 0. $? >> 8 is zero
for a child killed by a signal, so a build stopped mid-way looked to its caller like
one that had succeeded.
Two cases added to builddebs_lock_cancellation.t: a build whose worker is a
grandchild, and a command killed by a signal. Verified by signalling the pid instead
of the group, which leaves the worker running and turns the first red.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
The build lock is released in DESTROY, and perl does not run DESTROY when a signal
ends the process. A build stopped with SIGTERM or SIGINT therefore left its lock
directory behind, and the next build of that checkout died on
FATAL: another build of <path> already holds <dir> (held by [pid=NNNN])
naming a pid that had already exited. Nothing clears it but a person. One such
directory blocked an openSUSE target across three consecutive CI runs before anyone
looked at what the lock actually said.
buildrpms.pl has released its lock on cancellation for some time, through an END
block and an abort handler. This is the Debian builder catching up.
The order matters, and is the reason this is not simply an END block. The command in
flight is stopped BEFORE the lock is released: handing the checkout to a second build
while dpkg-buildpackage is still rewriting debian/changelog and debian/control in it
is worse than holding the lock a moment longer. The wait for that command is bounded,
so a subprocess that ignores the signal cannot hold the lock for ever either.
sh() now forks and execs rather than calling system(), because system() gives no pid
and a handler cannot stop what it cannot name. The child _exits rather than exits, so
it never runs the parent's END block and releases a lock the parent still holds.
The handler is installed by XCAT::BuildUtils::install_build_cancellation rather than
written inline in the builder, so a test can use the same wiring the builder uses. A
test that installs an equivalent handler of its own proves the helper works while
saying nothing about whether anything calls it -- the first version of this test did
exactly that, and passed with the wiring removed.
Release is idempotent: a signal handler and then DESTROY both reach it, and the
second must not remove a directory a LATER build has since taken.
builddebs_lock_cancellation.t terminates the holder, then takes the lock again, and
checks no build subprocess was orphaned. Verified by removing the wiring: assertions
9 through 12 fail, naming the leaked lock, the refused build and the stray process.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
A build tree can live on an NFS re-export. The kernel refuses locks on one --
"Clients are not allowed to get file locks or delegations from a reexport
server" -- so every flock() there answers errno 524, and a build that takes one
dies before it starts.
buildrpms.pl's per-target lock and BuildUtils.pm's take_build_lock, which
builddebs.pl calls for the Ubuntu core build, are both atomic mkdir claims now.
Each records its owner and names it when it refuses.
A directory is not released by a filehandle closing, which is how both locks
were freed before. buildrpms.pl releases from END, and again in abort_builds
because that handler re-raises the signal with DEFAULT and END blocks do not run
then -- a killed build would otherwise strand the lock for every later one.
BuildUtils returns a small object whose DESTROY releases it, preserving the
caller's "hold the returned value" contract.
Both releases are guarded by owning pid: both scripts fork, and the flock they
replace could not be released by a child.
builddebs_lock.t closed the returned value to prove the lock is released, which
is "Not a GLOB reference" against the new contract. It now lets the value go out
of scope. What it asserts is unchanged: a second build of the same checkout is
refused, and the next one succeeds once the first releases.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
deb_belongs_to_dist read any trailing ~word in a package version as the
codename the deb was built for. Debian uses ~ for a prerelease and --release
takes whatever the caller gives it, so `--release 1~rc1` left every package
excluded from every suite, with --genesis nowhere in the command and the run
still reporting what it published. Only xcat-genesis-base carries a codename,
because only it carries a kernel, so only it is asked.
genesis_dists() names the releases a Genesis image can be built on. A plain
--genesis run took the release list the rest of the build uses, and focal is on
it: focal ships debhelper 12.10 and the package declares debhelper-compat
(= 13), so sbuild stopped on the build dependencies before dracut ran and the
run ended on its first release. The default plan now leaves focal out and says
so. A release named with --genesis-dist is still built as asked.
genesis_deb_per_codename.t covers both. Applying the codename rule to every
package again fails five assertions; a genesis_dists that filters nothing, or
an image that loses its own rule, fails two each.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
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>
builddebs.pl publishes every Architecture:all deb into every release, because
every other xcat-core deb is the same file for all of them. The Genesis image is
not: it carries the kernel of the root that built it. All three images therefore
landed in all three suites, and apt serves the newest, which belongs to another
release.
deb_belongs_to_dist reads the codename back from the version and keeps an image
out of any other suite. A deb with no codename in its version is unaffected.
The jammy build also stopped on /usr/share/terminfo/l/linux and v/vt100:
ncurses-base installs terminfo under /lib, and the module asks for the
/usr/share copies, which come from ncurses-term. noble happened to have it.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
dracut copies the kernel, the kernel modules and every command out of the root
it runs in. The Ubuntu Genesis deb was built once, on the build host, so every
Ubuntu release got the build host's kernel. Nothing called the builder at all:
the pipelines converted the EL rpm with alien instead.
builddebs.pl gains --genesis, --genesis-only and --genesis-dist. The Genesis
deb is now built once per codename inside that codename's <codename>-<arch>-sbuild
chroot, the chroots xcat-dep's sbuild-all.pl already creates on the Ubuntu build
host. builddeb-genesis-base takes --expect-codename and stops when the root it
woke up in is a different release, so a build on the build host cannot produce a
codename's image.
builddebs.pl reads the build log through XCAT::BuildUtils::genesis_log_errors and
fails the build on FAILED:, a package apt cannot find and four more lines that a
zero exit status hides. The extracted payload goes through verify-genesis-payload
with the command list read back from the dracut module, plus the DHCP client, the
97xcat hooks and a /lib/modules that holds this chroot's kernel and no other.
The build root gains isc-dhcp-client, ifenslave and util-linux-extra, which
supply dhclient, ifenslave and hwclock.
genesis_deb_per_codename.t, genesis_payload_verification.t and
genesis_ubuntu_build_root.t fail without this change.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
The xcat and xcatsn packages declared only amd64 and ppc64el, and the
builder built and indexed only those two architectures, so a riscv64
management node had no package to install. Both packages are now built for
riscv64, every release the repository serves declares the architecture,
and the generated mklocalrepo.sh maps a riscv64 host to its own repository
instead of amd64. Each package now carries its own architecture list:
xCAT-genesis-scripts keeps the two it has control files for, because
riscv64 Genesis ships as an OpenEmbedded package instead.
Signed-off-by: Vinícius Ferrão <2031761+viniciusferrao@users.noreply.github.com>
The rebase onto master left the repository with two modules named
BuildUtils.pm: the shared build helpers at the root, package BuildUtils, and
the target architecture parser at build-utils/lib/XCAT/BuildUtils.pm, package
XCAT::BuildUtils. buildrpms.pl loaded both, one through `@INC` and one through a
path require. A reader cannot tell which module a BuildUtils reference names,
and the test sandbox staged the wrong one.
Move the shared helpers into build-utils/lib/XCAT/BuildUtils.pm as
XCAT::BuildUtils, and export targetarch_from_target beside them. Both builders
and the four tests now put build-utils/lib on `@INC` and import from the one
module. targetarch_from_target keeps its behaviour: it returns the same
architecture as before for suffixed targets, empty and undefined input, mixed
case and every architecture token.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Daniel Hilst
Vinícius Ferrão