getipaddr returns a cached address before it resolves, and the bypass tests
OnlyV6 and GetAllAddresses only. A caller asking OnlyV4 therefore receives
whatever the first lookup stored. An unrestricted lookup asks for AF_UNSPEC, so
on a dual-stack management node it stores the AAAA record, and %::hostiphash is
a global in a long-lived xcatd.
debian.pm resolves the install server with OnlyV4 and writes
nfsroot=<address>:/install. Given the cached IPv6 that renders
nfsroot=2001:db8::1:/install, which does not parse, so the Subiquity installer
never mounts and the node never completes. dhcp.pm and mknb.pm hold four more
OnlyV4 callers with the same exposure.
Skip the cache when OnlyV4 is asked for and the cached address is IPv6. The
lookup that follows requests AF_INET and replaces the entry with the IPv4
address, so the next caller of either kind gets a usable one. An IPv4 entry is
still served from the cache: the bypass is about the family of the cached
answer, not about disabling the cache for OnlyV4.
networkutils_getipaddr_onlyv4.t fails without this change.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
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 boot flip in compute.subiquity.tmpl addressed 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 back into the installer. The flip now reads
site.xcatiport and keeps 3002 as the default. TABLEBLANKOKAY, because the key is
optional and a plain TABLE lookup of an absent key fails the whole template.
The flip also counted any reply as an accepted request. It now requires the
monitor's "ready" greeting before it sends "next", and "done" afterwards, so a
different service on that port is not read as a flipped node.
subiquity_nfsroot_server in debian.pm called getipaddr without a family. A
dual-stack management node answers with its IPv6 address, and casper takes
everything after the first colon in nfsroot= as the path, so the live filesystem
never mounts. It now asks for IPv4, as dhcp.pm and mknb.pm do.
The DNS setup wrote the xcatmaster name as a nameserver when getent found no
address, which is the case the step exists to prevent. It now keeps the
resolv.conf DHCP gave the live installer.
ubuntu_subiquity_boot_flip.t, debian_subiquity_boot_params.t and
ubuntu_resolvconf_ip.t fail on the parent commit and pass here.
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>
defer_syncfiles_to_postboot has tests, but deleting the whole block out of
makescript -- the provmethod override and the call -- left the entire unit
suite green. That is precisely how the wrong deletion shipped two commits ago:
the override was removed on the mistaken grounds that %image_hash never carries
a provmethod, and nothing noticed.
makescript needs a management node and a database, so the block is lifted out
and eval'd into a scratch package, driven with the hash makescript actually
builds. What it pins is the resolution -- an osimage NAME becoming the
osimage's real provmethod -- and that the resolved value is what reaches the
helper.
Verified by mutation rather than by shape: an override that is present but
never fires reddens 1-3, and passing $provmethod instead of
$effective_provmethod reddens 2-3. Deleting the block, or the override, trips
the extraction guard instead, which is a loud failure rather than a silent
pass.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
The previous commit deleted the $effective_provmethod override on the grounds
that %image_hash never carries a provmethod. That was wrong, and the review
caught it: makescript builds %image_hash, calls getImage() on it, and then
hands the SAME hashref to getScripts(), which fills provmethod for every
osimage from the osimage table. getDisklessNet() already reads that key the
same way. The override was live, not dead.
Restore it and say what is actually true in the comment. nodetype.provmethod is
frequently an osimage name rather than 'install', and resolving it is the point
of the lookup.
Also close the gap that made the wrong deletion so easy to ship: reverting
mkinstall's subiquity branch to its pre-fix body left the whole unit suite
green. debian_mkinstall_subiquity_branch.t lifts that branch out and drives it
inside a real loop, so the `next` it performs is the one under test, with
report_node_error and the getipaddr seam stood in for. It calls
subiquity_boot_params with no injected resolver, exactly as production does.
The branch is selected out of debian.pm by what it contains rather than by
where it sits -- there are four `if (using_subiquity(...))` in that file, and an
earlier draft of this test silently matched the wrong one and ran past its
block.
Now observable, each verified by mutation: swapping $pkgdir and $httpport at
the call site reddens the nfsroot assertion; reverting the branch wholesale
fails the extraction guard rather than passing.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
The syncfiles deferral resolved the node's provmethod through
$image_hash{$osimgname}{provmethod} when the node names an osimage. makescript
fills %image_hash from getImage(), which stores pkglist, pkgdir, otherpkglist,
otherpkgdir and environvar -- and no provmethod. getScripts() has a separate
hash that does store one, which is where the pattern was copied from. So the
lookup was always undef, the override never fired, and the code claimed a
behaviour it did not have.
Pass $provmethod directly and say in the comment why there is nothing to
resolve it with. No behaviour changes -- the branch was inert -- so there is no
red to show first; what the deletion needs is coverage that the path it was
supposed to serve still works.
That is what the two new assertions do: an osimage-named provmethod with
nodesetstate 'install' still defers, and the same name with no nodesetstate is
not mistaken for a diskful install. nodesetstate is what carries the install
signal here, which is why the override was never load-bearing. Making the
deferral ignore nodesetstate and require provmethod eq 'install' reddens both.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Two things the review found, both in code added by this branch.
Reverting mkinstall's call site -- putting xCAT::NetworkUtils->getipaddr back
in place of subiquity_nfsroot_server, the exact regression the fix removes --
left the entire unit suite green. The helpers were covered; nothing linked them
to production. Compose the two steps in subiquity_boot_params(), which takes its
inputs and returns either a command line or the reason there isn't one, so the
composition can be driven; mkinstall keeps report_node_error and the loop's
`next`. That same revert now reddens 6 of 9 assertions.
The test stubs xCAT::NetworkUtils::getipaddr deliberately. Without it a call
site that bypassed the injected resolver died on a missing module -- a red, but
for the wrong reason. With it, bypassing the resolver returns the wrong answer,
which is what the assertions are there to catch.
The resolv.conf sandbox guard matched `/etc/` with a trailing slash, so the one
respelling its own comment names -- `etcdir=/etc; rm -f "$etcdir/resolv.conf"`
-- walked straight past it and the fragment would rm the runner's real
resolv.conf, as root in CI. `/etc\b` catches it: applying that respelling now
BAIL_OUTs instead of running.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
subiquity_nfsroot_server and subiquity_kcmdline each have tests, but reverting
the call site in mkinstall -- putting xCAT::NetworkUtils->getipaddr back in
place of subiquity_nfsroot_server, which is precisely the regression the fix
removes -- leaves the whole unit suite green. A helper can be perfectly covered
while nothing links it to production, and that is the shape the review found.
mkinstall needs a management node, so this drives the composition it performs:
resolve the install server, then build the command line, or explain why not.
It fails at the extraction guard until that composition is a routine that can
be called, so the behavioural proof is the mutation on top of the fix, not this
red alone.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
ubuntu_resolvconf_ip.t sandboxes the fragment by rewriting /etc/resolv.conf to
a path inside a tempdir. The fragment contains `rm -f /etc/resolv.conf` and the
unit suite runs as root in CI, so if that substitution ever stops matching the
test deletes the runner's resolver configuration instead of failing.
It matches today. It is one respelling away from not: writing the path in the
template as `etcdir=/etc; rm -f "$etcdir/resolv.conf"` slips straight past it,
and the existing BAIL_OUT does not catch that -- it guards only the fragment
extraction, not the rewrite.
Check the rewritten script for any /etc path outside the scratch tree and
BAIL_OUT rather than execute it. AGENTS.md asks that a test never escape its
scratch tree and notes that rewriting paths in the source under test is the
fragile way to arrange it; this makes the fragile part fail closed.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
ubuntu_subiquity_boot_flip.t bound 3002 on the loopback to check it was free
and skip_all'd when it was not. 3002 is the install-monitor port, so on any
management node xcatd is already listening there -- which is precisely where
the suite runs. The CI log for #7761 reads
ubuntu_subiquity_boot_flip.t ... skipped: port 3002 is not available on
the loopback interface
so all 139 lines and 12 assertions never executed once, and never would have.
Coverage that reports as a skip is worse than none, because the gap is
invisible.
Take an ephemeral port from the kernel and rewrite the extracted command to use
it, in both the /dev/tcp target and the log message. The port number is not what
is under test -- the retry-and-log behaviour is.
The template's own port is read out of the command rather than hard-coded, and
asserted to be 3002, so moving the install-monitor still leaves the file
covering something instead of silently testing a port nothing uses.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
The subiquity path resolved $instserver with getipaddr and `next`ed the node
when that returned nothing. For a node whose noderes.xcatmaster is unset,
$instserver is the literal '!myipfn!' -- a placeholder that pxe.pm:176 and
grub2.pm:129 substitute with my_ip_facing($node) when they write the boot
config. Resolving it as a name always fails, so those nodes went from a working
numeric nfsroot to no boot config and a "Could not resolve the install server"
error.
noderes.5.rst:125 documents an unset xcatmaster as supported, and anaconda.pm
and sles.pm both already guard the placeholder with
`unless ($instserver eq '!myipfn!')`.
Exempt the placeholder, keeping the guard for a real name that does not
resolve -- klibc's nfsmount still cannot resolve one of those, which is what
the original commit was for.
The decision moves into subiquity_nfsroot_server(), taking the resolver as an
optional argument, so it can be driven directly instead of through a routine
that needs a management node.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
A node whose noderes.xcatmaster is unset gets $instserver = '!myipfn!'. That is
a placeholder, not a name: pxe.pm:176 and grub2.pm:129 substitute it with
my_ip_facing($node) -- an address -- when they write the boot config. The
subiquity path resolves $instserver with getipaddr, which returns undef for the
placeholder, so it reports "Could not resolve the install server" and `next`s
past the node. Before this branch it produced exactly the numeric nfsroot the
change is trying to guarantee.
anaconda.pm and sles.pm both guard the same placeholder with
`unless ($instserver eq '!myipfn!')`. noderes.5.rst:125 documents an unset
xcatmaster as supported.
No pipeline can catch it: reg_linux_diskfull_installation_flat chdefs
xcatmaster=$$MN and no ci/conf/pipelines/*.conf leaves it unset, so CI always
takes the resolvable branch.
This drives the decision with an injected resolver, so it also checks the
resolver is never ASKED about the placeholder rather than only checking the
return value. It fails at the extraction guard until the routine exists, so the
behavioural proof is the mutation on top of the fix, not this red alone.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Two ways the Subiquity install could fail without saying anything useful.
The boot flip read from the install monitor with no timeout. A monitor that accepts the
connection and then never answers -- which #7759 shows is a state it gets into -- blocked the read
forever, and with it the Subiquity late-command and the install. The retry loop could not help:
it never reached the retry. Both reads now take -t 10, so five attempts are bounded at roughly two
minutes and end in the failure that is already logged. The regression test stands up a listener
that accepts and holds the connection; removing the timeouts fails it.
mkinstall resolved the install server for nfsroot and fell back to the name when that failed:
"getipaddr($instserver) || $instserver". The name is the original defect -- klibc's nfsmount
cannot resolve one -- so the node panicked "can't parse IP address" at boot, on the node, with
nothing reported on the management node. The management node knows at template time, so it says
so there and skips the node, as the other unrecoverable per-node conditions in this routine do.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
A node may list syncfiles in both postscripts and postbootscripts -- once for the install and
once for the booted node. The deferral prepended its own copy regardless, so the booted node ran
syncfiles twice in a row inside a marker block that made it look like xCAT had generated the
duplicate.
Keep the entry the node already has and still remove the install-time one, which is the copy
that cannot work in the in-target chroot.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
toram copies the live filesystem into RAM, so the node needs memory for it on top of the
installer -- about 1.5G on 24.04 and growing per release. Nothing enforced or documented that.
A node with too little fails part-way and reboots into the installer, which looks like a
boot-flip failure rather than an out-of-memory condition, so the troubleshooting page says
where to look and how to raise vmmemory.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
The comment blamed mawk: updateflag.awk needs gawk's |& and Ubuntu's /usr/bin/awk is mawk. That
is wrong on this path -- compute.subiquity.tmpl has installed gawk in the target since Ubuntu
20.04 support was added, so updateflag.awk would have found it.
The flip was going unanswered because xcatd's install monitor, which serves port 3002, dies and
is never re-forked. That is fixed in #7759. The /dev/tcp exchange here is hardening: it retries
and reports, rather than reinstalling in silence.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Three passes of the same reasoning had accumulated: in the code, in the POD of the routines the
previous commit extracted, and again in the test headers. Say each once, where the reader needs it.
The POD blocks were the worst of it -- extracting subiquity_kcmdline() and
defer_syncfiles_to_postboot() moved the essays out of the routines but did not shorten them. The
toram paragraph ran eleven lines for one kernel argument; the exact systemd-shutdown message and
the size of the 24.04 layers are colour, not reasoning.
The template's two comments and the apt-sources pair are trimmed the same way: the sources.list
explanation lives in Template.pm, and the test says which case it is checking and points there.
That the tests still pass after rewriting the template is the point of the previous commit -- a
test matching its text would have broken here.
47 comment and POD lines removed, no behaviour change.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
The tests added with this fix matched regexes against the text of debian.pm, Postage.pm and
compute.subiquity.tmpl. A source match cannot tell whether the code it found ever runs: moving
the Subiquity command line into its own routine leaves every one of those assertions passing,
and reformatting a line fails them while the behaviour is untouched. One of them pinned Perl
syntax outright, qr/\$kcmdline\s*\.=\s*" ---";/, and another matched the text of a substitution.
Run the code instead.
Two decisions were lifted out of the routines that had grown around them, so a test can call
them: subiquity_kcmdline() in debian.pm builds the installer command line from its inputs, and
defer_syncfiles_to_postboot() in Postage.pm returns the adjusted postscript lists. Both are pure
and carry the reasoning that used to sit inline. The callers keep their behaviour exactly.
The template's two shell fragments are extracted and executed: the boot flip runs against a
stand-in for xcatd on the install-monitor port, and the resolv.conf step runs with a getent that
answers as the case requires.
Every assertion now fails when the behaviour it describes is removed, which is what the source
matches only appeared to do:
boot=casper dropped from the command line 1 assertion fails
toram dropped 1 assertion fails
nfsroot given the host name instead of the IP 2 assertions fail
the deferral made a no-op 7 assertions fail
the wrong token sent to xcatd 1 assertion fails
the failed-flip log line removed 3 assertions fail
the retry loop reduced to one attempt 2 assertions fail
resolv.conf given the host name 3 assertions fail
the resolution fallback removed 2 assertions fail
ubuntu_subiquity_bootparams.t is removed. Its three matches are covered by execution in
debian_subiquity_netboot.t, and it demonstrated the problem: it still passed after the command
line moved into another routine.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Boot the live installer correctly. Add boot=casper so casper actually processes
netboot=nfs instead of scanning local disks and panicking, resolve the install server
to a literal IP because casper mounts the live filesystem with klibc's nfsmount which
has no resolver, and add toram so casper copies the squashfs into RAM and unmounts the
NFS source. That last one is what lets the node reboot at all: with the NFS root still
mounted, systemd-shutdown waits forever on an lvm/pvscan wedged in uninterruptible I/O
on it and the node never power-cycles into the disk it just installed. casper has no
cmdline knob for NFS mount options -- it parses only nfsroot= and takes the whole value
as the path -- so toram is its supported way to avoid a network root.
Write the installer's resolv.conf nameserver as an IP. glibc's resolver discards a
hostname given on a nameserver line, so the xcatmaster name left the installer, and
the in-target apt that inherits the file, with no DNS at all.
Add the online archive through apt `sources:` on classic-sources releases, where
Subiquity renders the target sources.list from the install media alone and in-target
apt cannot find packages the ISO does not carry. Deb822 releases are excluded: their
primary mirror already lands in ubuntu.sources, so legacy .list files would configure
the same suites twice.
Flip the node to local-disk boot from the live installer over bash's /dev/tcp instead
of relying on updateflag.awk, which needs gawk's |& coprocess while Ubuntu's
/usr/bin/awk is mawk. The exchange is checked, and a failure is recorded in the
install log rather than silently PXE-looping into another install.
Defer syncfiles to the postbootscripts on the diskful install path, so it runs on the
booted node with sshd up rather than inside the in-target chroot where the MN cannot
reach it.
Make the test harness's provision wait overridable through WAIT_FOR_PROVISION; the
default stays short so a boot loop still fails fast.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
The gating reg_linux_diskfull_installation_flat case cannot pass on Ubuntu: the
compute node never reaches a booted, installed OS, and the case only ever reports
"ssh: connect ... port 22: Connection refused". It is not one defect but a chain, each
reachable only once the one before it is fixed.
The installer never boots. Without boot=casper, casper never processes netboot=nfs --
it scans the local disks, finds no live media and panics "Unable to find a medium
containing a live file system". nfsroot must also be a literal IP: casper mounts the
live filesystem with klibc's nfsmount, which cannot resolve a hostname. And with the
NFS root still mounted at end of install, systemd-shutdown blocks forever on an
lvm/pvscan wedged in uninterruptible I/O on it, so the node never power-cycles into
the disk it just installed.
The installer has no usable DNS. A nameserver line in /etc/resolv.conf must hold an IP
-- glibc's resolver discards a hostname written there -- so writing the xcatmaster name
leaves the installer, and the in-target apt that inherits the file, hanging on
archive.ubuntu.com.
In-target apt cannot find its packages on classic-sources releases, because Subiquity
renders the target sources.list from the install media alone.
The node never leaves the installer. The boot flip to local disk goes through
updateflag.awk, which needs gawk's |& /inet coprocess, but Ubuntu's /usr/bin/awk is
mawk -- so the flip fails silently and the node reinstalls forever.
And syncfiles runs inside the in-target chroot, asking the MN to scp files into a node
that has no sshd yet, so it times out and the node reports failed on a good install.
Cover each stage. All fail today.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Add a unit test for the option that selects fping instead of nmap.
The test takes the specification out of the plugin source and gives it to
Getopt::Long with the settings that the daemon uses, so it drives the
specification that the plugin ships.
It shows that -f, --usefping and the older --useping each select fping, that
--use and --us still select usemon and do not select fping, that the bundles
-mf and -fm select both options, and that both places parse through the one
specification.
Add a unit test for the routine that chooses the compression program. The
test lifts the routine out of the plugin source, because the plugin needs a
management node to load.
The test shows that lzma is used when it is there, that xz stands in when it
is not, and that xz is asked for the lzma container rather than its own. It
also shows that the caller takes the command from the routine, that the file
keeps its name and its suffix, and that the gzip fallback and the rename into
place both remain.
The manual page and the usage message of nodestat give the option
-f|--usefping. The preprocessor of the request knows no option f, and the
handler of the request spells the long name useping, so --usefping does
nothing. An administrator who follows the manual page gets the nmap path, and
gets no message that says why.
The two places also read different specifications, so an option that one
place accepts can reach the other place and take a different meaning. Put the
specification in one routine, and let both places read that routine.
Give the name usefping to the option, and keep useping as a second name. That
spelling has worked since 2.14.2, so a site can have it in a script.
The long name of the fping option starts with the same letters as usemon, so
--use and --us become names that Getopt::Long cannot decide. Those two
abbreviations select usemon today. Keep them with usemon, or an administrator
who monitors with them loses the monitoring and gets no message.
The change has two other effects. The abbreviations --use and --us no longer
select fping as well, which they did only because the two places read
different specifications. The bundles -mf and -fm now select both options,
which they did not do before.
Recovered from the lenovobuild branch.
The genesis image goes into a file whose name ends with .lzma. The plugin
writes that file only when /usr/bin/lzma is there, and it falls back to gzip
when it is not.
Red Hat ships no lzma binary. On AlmaLinux 9 and on AlmaLinux 10 that test
fails, the plugin falls back to gzip, and it gives no message that says why.
The image is larger on each run of mknb. Debian and Ubuntu ship lzma as a
second name for xz, so those systems still get the smaller image.
Ask xz for the same container when lzma is absent. The command
"xz --format=lzma" writes the same bytes as "lzma", so the file keeps its
name, its container and its size. Keep the gzip fallback for a system that
has neither program.
Recovered from the lenovobuild branch, which asked xz for the xz container.
That container is not the lzma container, and the name of the file says lzma.
Daniel Hilst
Vinícius Ferrão
Vinícius Ferrão