Pin both directions for every architecture xCAT supports on Ubuntu, the
pass-through for names Debian shares, and the round trip that copycd and
debootstrap depend on agreeing about.
Build each Ubuntu media layout on disk and ask the resolver for its kernel and
initrd: the netboot trees, the flat netboot layout, both casper images, and the
POWER layouts where the kernel and the initrd sit in different directories.
Pin the precedence the installer depends on, a netboot tree over a live image
and a hardware-enablement kernel over the release one, and pin the three ways
media resolve to nothing.
getipaddr answers from %::hostiphash before it resolves. The bypass tests
OnlyV6 and GetAllAddresses and does not test OnlyV4, so a caller that asks for
IPv4 is handed whatever the first lookup cached.
An unrestricted lookup passes AF_UNSPEC to getaddrinfo, so on a dual-stack
management node with an AAAA record it caches the IPv6 address. xcatd is
long-lived and the hash is a global, so one earlier caller poisons every OnlyV4
caller after it.
debian.pm then writes nfsroot=2001:db8::1:/install, which is not a parseable
nfsroot, and the Subiquity install never mounts. dhcp.pm and mknb.pm hold four
more OnlyV4 callers with the same exposure.
The test also pins what the fix must not break: an IPv4 cache entry is still
served to an OnlyV4 caller, and an unrestricted caller still gets its cache hit
whatever family it holds.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
The Subiquity diskful path builds three values from configuration, and nothing
checks that the value it builds is one the consumer can use.
The boot flip addresses port 3002. xcatd's install monitor listens on
site.xcatiport, so a cluster that moves the port loses the flip and every node
PXE-loops. The exchange also counts any reply as an accepted request, so a
different service on that port reads as a flipped node. xcatd greets with
"ready" and answers every request with "done".
subiquity_nfsroot_server resolves the install server without a family. A
dual-stack management node answers with its IPv6 address, and nfsroot=<v6>:<path>
is unparseable, because klibc nfsmount takes everything after the first colon as
the path.
The DNS setup writes the xcatmaster name as a nameserver when getent fails,
which is the case the step exists to prevent.
ubuntu_subiquity_boot_flip.t drives the template's own late-command against a
stand-in monitor. ubuntu_resolvconf_ip.t runs the DNS step and reads the file it
writes. debian_subiquity_boot_params.t drives the helpers with a dual-stack
resolver. All three fail on this commit.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
The static host query reads dhcpd.conf when no configuration is in memory. When the
read fails the query returns nothing, and listnode reports "no DHCP reservation
found" -- the answer for a node that has no reservation. The operator cannot tell
the two apart.
The same query only reads a "hardware ethernet" line. An InfiniBand node declares
"hardware infiniband", so its query answer carries no hardware address. A twin
declaration inside the same markers must not replace the primary one either.
The new assertions drive the writer to build both InfiniBand shapes, set the path of
dhcpd.conf to a file that does not exist, and call listnode. A deletion that names a
hostname is asserted to keep the other declarations of the node.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
makentp reads site.ntpbackend to pick the NTP daemon, so an admin has to set it, but the
attribute appears in no help text. lsdef -t site -h and tabdump -d print the site table
description from xCAT::Schema, which documents site.dhcpbackend and says nothing about
ntpbackend. The makentp man page lists the site attributes the command honors and does not
list it either.
The selector test now reads the site help from the loaded schema and the makentp pod.
Five assertions fail: the attribute name, its three valid values, and the man page entry.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
xCAT::NTP::Backend->available reports chrony as available when chronyd exists. makentp
configures chrony only when systemctl exists as well, and setupntp hands over to ntpd without
it. On a host with chronyd and no systemctl the selector answers chrony, makentp takes the ntpd
path anyway, and the admin is told nothing.
ntp_backend_selection.t drives available and choose with the command probe injected, so a case
can hold chronyd present and systemctl absent without depending on what the test host has
installed.
Six assertions fail: available reports chrony on chronyd alone, and choose neither downgrades
nor asks for an install.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
setupntp treats the requested backend as a preference in one direction only. A node given
--backend chrony without chronyd uses ntpd, but a node given --backend ntpd without ntpd still
hands over to setupntp.traditional, which writes /etc/ntp.conf and starts a daemon that is not
installed.
The selection cases in makentp_ntp_deps.t now also make ntpd absent. The stub for check_executes
takes a list of absent commands, so a case can hide ntpd, or ntpd and chronyd together.
Two cases fail: --backend ntpd and --use-ntpd both keep ntpd where only chrony is present.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
The respawn tests cover the pacing in xCAT::RespawnUtils. Two things the daemon
itself has to do are untested.
A SIGCHLD handler that does not clear $pid_MON leaves xcatd holding a dead pid,
so the service loop never re-forks the monitor. Only ssl_reaper clears it, and
generic_reaper is the handler at startup and again while connections are
throttled.
The respawn block also runs from the middle of the service loop, so the child
inherits the client connections the parent has accepted but not yet dispatched.
xcatd cannot be loaded in a unit test, so the two reapers and the respawn fork
block are lifted out of the program text and run in a scratch package against
stand-in descriptors. generic_reaper fails, and so does the pending-connection
assertion; ssl_reaper passes and guards the path that already works.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
DHCP backend auto-selection fails hard when the backend it prefers is not installed.
The xcat metapackage's "isc-dhcp-server | kea" Depends guarantees isc-dhcp-server,
while auto-selection prefers kea on 22.04+, where kea arrives only through Recommends.
An apt run that skips recommends therefore leaves a node with isc only, auto picks the
absent kea, and makedhcp errors out on a freshly installed management node (#7710).
makedhcp -q re-enters omshell on the very releases whose write paths avoid it. xCAT
already records each reservation as a static host block in dhcpd.conf on Ubuntu's
ISC-limited releases because their ISC DHCP 4.4 omshell can wedge at 100% CPU and
never be reaped, but listnode still called _omshell_query_host unconditionally.
Both host-block scans also match the node name loosely: /\Q$node\E\b.* start$/ matches
at a hyphen, so "compute" matches the "compute-01" marker -- a query can answer with
another node's address and a delete can remove another node's reservation.
Cover all three: the auto-selection fallback, a query answered from dhcpd.conf without
omshell, and exact node-name matching for both the query and the delete.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Replace source-shape extraction with real preprocess and command execution. Model missing table rows with the same return shape as xCAT::Table.
Signed-off-by: Vinícius Ferrão <2031761+viniciusferrao@users.noreply.github.com>
Cover the three shapes a declaration can take: one that names an entity from
element text, one that names it from an attribute value, and one that carries
no entity at all. The parser must refuse each, and nothing may expand.
The attribute case is the one that matters most. The option that stops the
parser expanding an entity leaves an attribute alone, so a test that only used
element text would pass against a parser that still grows a request through an
attribute.
Cover an ordinary request as well, so a refusal that is too wide is visible:
the command, the node range and the argument must still arrive.
Run all of it against the parser of a recent XML::Simple and against the parser
this module builds for an older XML::Simple.
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>
Drive redact_directory_entry with a password on a continued COMMAND
record, in both spellings. Pin the records that do not end the
continuation: a sequence number, a blank record, a comment, and a record
that reads as another directory statement.
buildrpms_source_only.t runs buildrpms.pl from a copied sandbox, and staged
only BuildUtils.pm. Master added a second module with the same basename,
build-utils/lib/XCAT/BuildUtils.pm, which buildrpms.pl also loads. The
sandboxed run then died with "Can't locate .../build-utils/lib/XCAT/
BuildUtils.pm" before it reached the argument parsing the test asserts on.
Stage each needed file at its own relative path and create the parent
directory, so both modules reach the sandbox.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
The two builders wrote the same run-a-command-or-stop step in opposite
polarities. builddebs.pl used sh(...) == 0 or die; buildrpms.pl used
sh(...) and die, and also used the == 0 or die form for its sh_retry calls, so
both directions appeared in one file. The 'and die' spelling reads as though
the die is what happens next rather than what happens on failure, which is a
poor thing to have to re-read at every call site.
Add BuildUtils::sh_or_die and convert the fourteen plain sh() call sites to it.
The failure message now also carries the exit code, which every one of the old
spellings discarded -- a build that failed said only that a command failed, not
what it returned. The sh_retry sites keep their own form: retry is a different
operation and sh_retry is local to buildrpms.pl.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
The two builders read and wrote files three different ways. buildrpms.pl used
File::Slurper, builddebs.pl opened and closed handles inline, and BuildUtils.pm
carried its own read_file/write_file pair to avoid the dependency. The result
was that the same operation was spelled differently in each place, and the
Version stamp was read by hand in both builders: buildrpms.pl chomped it ten
lines after reading it, which is how a stamp keeps a trailing newline nobody
notices until it reaches a package name.
Drop the hand-rolled pair and use File::Slurper throughout, and add
BuildUtils::read_line for the one-line Version and Release stamps -- it strips
the newline at the point of reading and reports an absent file as undef, which
is what builddebs.pl's fallback to snap_release needs.
File::Slurper is already required by buildrpms.pl and is already in the CI
package list, so this adds no new build-host prerequisite. The lock file still
uses a raw open: flock needs a real handle.
Behaviour note: Version is now read as its first line rather than the whole
file with the final newline removed. For the one-line file it is, these are the
same; a malformed multi-line Version now yields its first line instead of an
embedded newline.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
builddebs.pl stages the genesis bmcsetup and getipmi helpers into the xCAT
package by writing the rewritten text and then chmod'ing it, which is the third
copy of the write-then-make-executable pair the previous change collapsed. It
was left out because it uses 0755 while the published repo helper uses 0775.
Give write_script an optional mode, defaulting to the 0775 it already used, and
use it for the postscripts with 0755. The two modes stay exactly as they were
-- the postscripts are deliberately not group-writable -- and the test asserts
that the caller's mode is honoured rather than overwritten by the default.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Both builders ship a mklocalrepo.sh beside the packages they publish, and each
wrote it its own way: builddebs.pl opened, printed and closed the file by hand
and then chmod'ed it, while buildrpms.pl used File::Slurper and a separate
chmod. Writing the text and setting the executable bit are one operation -- a
copy published without 0775 is published broken -- but nothing tied them
together, so each caller had to remember the second step.
Add BuildUtils::write_script, which writes the file and sets the mode, and use
it from both builders. The mode is now asserted in build_utils.t instead of
being left to the callers to repeat.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Both builders wrote the same six fields beside a published repository --
VERSION, RELEASE, BUILD_TIME, BUILD_MACHINE, COMMIT_ID, COMMIT_ID_LONG -- each
assembling them by hand, and each shelling out to hostname to do it.
builddebs.pl also ran git rev-parse a second time to get the commit, although
it had already derived the same value into $GITINFO earlier in the file. It now
uses that.
The filenames and the time formats stay as they were. deploy.sh copies
builddebs.pl's buildinfo verbatim and cluster-test.pl parses buildrpms.pl's
buildinfo.txt, so both are a contract with consumers outside this repository;
the shared helper takes the format from its caller rather than picking one.
Confirmed byte for byte that each builder's stamp is unchanged.
Covered by tests: the field names and their order, the seven-character short
commit, and that the two callers' formats still differ.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
builddebs.pl spelled out the same slurp five times, and twice wrapped it in the
identical read, transform, write-back sequence -- once to pin the dependency
versions in debian/control and once to rewrite the changelog header. The two
blocks differed only in the file and the function applied to its contents.
BuildUtils.pm now provides read_file, write_file and rewrite_file. The last
leaves a file that is not there alone rather than creating it, which is what
both call sites guarded for with -f.
These use plain open and close rather than File::Slurper. buildrpms.pl already
depends on that module, but builddebs.pl does not, and putting it in the shared
module would oblige a deb build to install something it otherwise has no need
of.
Reading a missing file now names the file it could not read; one of the call
sites died with no message at all.
Covered by tests: a round trip, that rewrite_file applies its transform and
reports whether it acted, that it neither runs the transform nor creates the
file when there is nothing there, and that a failed read names the path.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Both builders defined their own sh() and usage(). The usage() copies differed
only in spelling, but the sh() copies disagreed about what they return:
buildrpms.pl shifted system()'s result into an exit code, builddebs.pl returned
the raw wait status. The same command therefore reported 1 from one builder and
256 from the other, and a caller comparing sh() against a particular code was
correct in only one of them. Both current call sites only test for zero, so
nothing was broken yet.
BuildUtils.pm now provides both, shifting the status as buildrpms.pl did.
pod2usage reads the POD of the running program, so each builder keeps its own
help text while sharing how it is printed and the status it exits with.
sh() echoed the command under the builder's own --verbose flag, which a shared
function cannot see. $BuildUtils::VERBOSE is set once after option parsing
instead of threading the flag through every call site.
Covered by tests: that sh() returns the exit code rather than the wait status
it is packed into, and that a verbose run still echoes the command.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
buildrpms.pl derived the git revision and SOURCE_DATE_EPOCH with its own inline
backticks while BuildUtils.pm already provided git_revision() and
source_date_epoch() for builddebs.pl. Both did the same thing, and the copies
in buildrpms.pl were the weaker ones: neither fell back to the tracked Gitinfo
and Gitepoch files, so a build from a source tarball with no git history
recorded the revision as "unknown" and stamped the current time as the build
epoch, losing reproducibility exactly where it matters most.
Verified that the helper returns the same revision and epoch as the code it
replaces on a checkout with history.
buildrpms.pl now loads BuildUtils.pm from its own directory, so the source-only
test stages the module into its sandbox alongside Version. That test copies the
builder rather than running it in the checkout, because it rewrites Gitinfo and
creates $HOME/rpmbuild before it looks at its arguments.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
An xCAT installed from these debs could report no version at all: `lsxcatd -v`
printed a bare "Version" and there was no /opt/xcat/share/xcat/version to
consult.
xCAT::Version ships with #XCATVERSIONSUBHERE / #XCATSVNBUILDSUBHERE
placeholders that perl-xCAT/debian/rules fills by calling modifyUtils with the
version and the commit. It takes the commit from ../Gitinfo, falling back to
`git log`. Neither Debian builder wrote Gitinfo -- buildrpms.pl does, which is
why only the deb side was affected -- so a tree without a readable .git left
that argument empty. modifyUtils then hit `if [ -z "$2" ]` and returned 0
WITHOUT substituting, and neither caller checks the status, so the placeholders
shipped.
Two independent things had to be wrong for this to be silent, and both are
fixed:
modifyUtils now exits non-zero on a missing argument, so a build that cannot
stamp a version fails instead of producing one that cannot identify itself. Its
message was also wrong -- it checked $2 and complained about the version, which
is $1 -- so each argument now names itself.
builddebs.pl writes Gitinfo, as buildrpms.pl already does. BuildUtils::
git_revision prefers the checkout, falls back to an existing Gitinfo (a source
export carries the real revision that way, and overwriting it with a
placeholder would discard the only provenance the tree has), and only then
returns "unknown" -- never the empty string modifyUtils ignores.
"unknown" is now warned about rather than stamped quietly. The cause is usually
not a missing .git but git refusing one it considers dubiously owned: the tree
belongs to another user and the safe.directory exception sits in a config that
the build's own HOME override hides. That is exactly how this went unnoticed.
Not a regression -- build-ubunturepo produces byte-identical unsubstituted
placeholders in the same .git-less tree, verified by running it there. It is a
pre-existing hole that CD masks by writing Gitinfo in the pipeline.
Verified on xcat-master-ub: with a readable checkout the deb carries
'Version ' . '2.19.0' . ' (git commit 0030133c89)';
without one it carries '(git commit unknown)' and the build prints the warning
naming the tree, the command to run and the HOME to run it under. Nine
assertions drive modifyUtils itself -- stamping both forms, and each missing
argument -- and dropping either sed, or restoring the silent exit, reddens it.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
A second run in the same checkout died once the release string moved:
dpkg-genbuildinfo: error: cannot fstat file ../xcat_..._ppc64el.deb
debian/files accumulates one line per artifact and survives `dh_clean -d`,
which removes directories only. The next build's dpkg-genchanges reads the
stale entries and fstats artifacts collect_debs already moved away. The old
shell builder deleted debian/files explicitly; that step was not carried
over. --force does not help, since it only wipes the output repository.
Three further ways the build did not put the tree back as it found it:
xCAT/postscripts/{bmcsetup,getipmi} are TRACKED files that the xCAT build
rewrites from the genesis sources. They were recorded as created, so
cleanup deleted them from the checkout. They are claimed now.
Restoring a claimed file lost its mode: File::Copy::copy does not carry
permissions, so an executable came back 100644 with identical content --
visible only as a git mode change. backup_file/restore_file record and
reapply it.
debian/*.substvars are rewritten in place and several are tracked; they are
claimed too. debhelper's .debhelper/ and *.debhelper.log are never tracked
and are removed with the rest of the residue.
clean_debian_residue runs from collect_debs, which is OUTSIDE
with_prepared_tree -- the restore has already happened by then. That is why
it must not remove *.substvars: doing so would delete the tracked ones it
just put back. The claim mechanism handles those instead.
Verified on xcat-master-ub: two consecutive full 14-package builds in one
checkout both succeed, and a build against a git checkout now leaves zero
tracked files modified or deleted (was five, two of them deleted). The
remaining untracked residue -- pods/, share/, pod2htmd.tmp and the
substvars of packages that do not track one -- is inherited from
build-ubunturepo and unchanged here.
Also covers the guard that gives this PR its name: deleting
`return if $opts{source_only}` from buildall left buildrpms_source_only.t
green, so nothing checked that --source-only skips the binary rebuild. It
does now, and the mutation reddens two assertions. Same for the changelog
trailer: making its substitution global left the suite green because only
the older header was asserted, not its author and date.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
ubuntu_2604_pkglist.t matched regexes against Template.pm's `push @lines`
literals, and ubuntu_subiquity_storage.t was source greps end to end. Both
pass when the code is moved somewhere it never runs, and both break on a
reformat that changes nothing: reordering two keys inside the efi-part
stanza -- identical emitted config -- fails the old efi-part match.
The apt config is now rendered by calling ubuntu_subiquity_apt_config and
asserting on its output. Only the two collaborators that read the xCAT
database are stubbed; the release branch is driven for real through the
media directory the renderer parses, so 26.04, 24.04 and pre-Deb822 each
render their own shape. The Release-index requirement is exercised against
a real directory instead of matched as a shell fragment.
The partitioning moves to ubuntu_subiquity_storage.t, which owns that
topic and was asserting the same facts by grep. The script cannot run
here -- it stops syslog and carries xCAT template markers -- so the block
that writes the partition file is lifted out and executed with its one
bracket test shadowed by a shell function, which bash resolves ahead of
the builtin, leaving the script's own condition unmodified. The emitted
curtin config is parsed into id => attributes, so the assertions survive
reindentation and reordering.
Both extractions BAIL_OUT if their anchors stop matching, and the
partition-file redirect is asserted to have been pointed at the scratch
tree twice before anything runs, so a rewrite that stops matching fails
loudly instead of writing to /tmp.
Verified by mutation: dropping grub_device, formatting the ESP ext4,
moving its mount point, dropping the bios_grub flag, indenting the block
off column 0, dropping Enabled: no, rendering a live cdrom source on
26.04, dropping the Check-Date waiver, dropping the offline fallback and
dropping the Release requirement each redden the suite; the pure key
reorder that broke the old assertion leaves it green.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Vinícius Ferrão
Daniel Hilst