Hoisting the systemd-timesyncd stop/disable above the ntpd hand-off moved it
inside the window makentp_ntp_deps.t extracts as $select, and that harness
stubbed only logger and check_executes. So the extracted region called the real
systemctl -- and the unit suite runs as root in CI. Probed with a recording
systemctl first on PATH, `prove makentp_ntp_deps.t` made 12 real invocations,
stop and disable for each of the six selection cases; on the Ubuntu review MN a
stand-in timesyncd unit went from enabled/active to disabled/inactive while the
suite reported PASS.
That is the "never let a test escape its scratch tree" rule, and it failed
silently: the assertions passed either way, so nothing said the host had been
changed.
Shadow every command the region can reach, not only the ones it reached when
this was written -- the region moves. Bash resolves functions ahead of $PATH,
so these win without touching PATH.
The stubs use echo rather than printf '...': the harness runs `bash -c '...'`,
so a single quote inside the prelude closes that string early. The existing
logger stub gets away with it; three more did not, and the ntpd selection cases
failed until they were rewritten.
Verified after: zero real systemctl invocations from the whole unit suite under
the same probe, and 18/18 still pass.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
The stop/disable sat below the `exec setupntp.traditional` that the ntpd path
takes, and setupntp.traditional never mentions timesyncd, so on that path it
was never reached: ntpd and timesyncd both ended up disciplining the clock.
Hoist it above the hand-off so both backends get it, guarded on systemctl
existing -- the ntpd path is taken precisely when systemctl may be absent.
The comment justifying it was also wrong, and is corrected: chrony and ntpsec
both Conflicts: time-daemon, so installing either already displaces timesyncd.
That only helps when an install actually happens; on a re-run, or where the
daemon was already present, timesyncd is still enabled, which is what this
covers.
The four timesyncd assertions in makentp_ntp_deps.t are dropped rather than
repaired: they ran over the section from `check_exec_or_exit cp cat logger grep`
onwards, which only the chrony path reaches, so the code they covered is no
longer in their window. setupntp_timesyncd_both_backends.t drives both paths
instead.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
setupntp stops and disables systemd-timesyncd so it does not discipline the
clock against the daemon being configured. That block sits ~24 lines below the
`exec setupntp.traditional` the ntpd path takes, and setupntp.traditional never
mentions timesyncd -- so on the ntpd path the disable is never reached and both
end up stepping the clock.
setupntp resets PATH at the top, so PATH stubs cannot shadow anything in it;
the region is extracted and driven with systemctl shadowed by a shell function,
which bash resolves ahead of PATH. `exec` is a builtin and cannot be shadowed,
so the harness puts a recording stand-in where the script execs -- the run
ending there is the behaviour under test.
Five of the eight assertions pass already, pinning the chrony path and the
hand-off itself. The three that fail are the ntpd path.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
ntp_backend_selection.t covers xCAT::NTP::Backend thoroughly, but copying the
base makentp.pm over the head one leaves the entire unit suite byte-identical.
Zero assertions cover the file the fix is named after: the call site that
consumes the selector, the --backend pass-through to setupntp, and the abort
branches for "selector errored" and "neither daemon installed" all run in no
test. A helper can be perfectly covered while nothing calls it.
This drives the decisions the call site makes -- abort vs configure, the
downgrade note, and the command handed to setupntp -- rather than
process_request, which needs a management node. It fails at the extraction
guard until those decisions are routines 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>
makentp picks the NTP daemon through xCAT::NTP::Backend, but then runs setupntp -- on the
management node and, through updatenode -P, on every service node -- and setupntp decided for
itself with `check_executes chronyd || USE_NTPD=yes`. A cluster with site.ntpbackend=ntpd and
chronyd present therefore configured ntpd on the MN and chrony everywhere else. The selector was
one code path only on the side that does not write the config.
setupntp now takes --backend chrony|ntpd, and makentp passes what it chose on both call sites.
The service-node dispatch passes the cluster's intent rather than this host's availability: a
service node may have a different daemon installed, and the requested backend is a preference --
a node without chronyd still falls back to ntpd and logs that it did, rather than failing.
--use-ntpd keeps working.
Two results of choose() were computed and never read. A downgrade is now reported, so an admin
who asked for one daemon and got the other is told. install=1 -- neither daemon present -- is an
error naming the daemon that is missing, instead of falling through to the ntpd branch and
reporting "Please make sure ntpd is installed" even when chrony was the preferred choice.
Six cases cover the selection: the backend honoured in both directions, the probe still used when
none is given, and the fallback when the requested daemon is absent. Removing the --backend case
fails one; ignoring the preference fails two.
Also worth stating plainly, since the PR reads as a management-node fix: setupntp stops and
disables systemd-timesyncd wherever it runs, nodes included. It has to -- timesyncd disciplines
the clock against the daemon being configured -- but a node that was relying on it loses it.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
setupntp explained systemd-timesyncd in four lines where two carry the reason nothing displaces
it, and repeated the hwclock rationale that the guard below already implies. NTP::Backend's
header narrated that the module exists to be unit-tested.
The test header restated all of it a third time, and said "the first three are behaviours of the
script" about a list of three items whose third is the packaging check. Corrected and shortened;
the note on why setupntp cannot simply be executed is kept, since that is not obvious from
reading it.
Interface documentation is left alone: choose()'s parameter list and default_backend()'s
per-family table are what a caller needs.
18 comment lines removed, no behaviour change.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
The test added with this fix matched regexes against the text of setupntp and makentp.pm. A
source match cannot tell whether the code it found ever runs, and it describes the fix rather
than the behaviour: unlike(qr/check_exec_or_exit[^\n]*hwclock/) says "this line does not mention
hwclock", where what matters is that a management node without hwclock still gets its clock set.
Run the script instead.
setupntp cannot simply be executed: it forces its own PATH, so its commands cannot be stubbed
from outside, and it exits unless UID is 0, so it cannot run as an ordinary user. The test takes
the script's own helper functions and the section that configures the daemon and drives them with
shell functions, which bash resolves ahead of PATH and which both `type` and `command -v` report
as present -- the two probes the script uses. A node without hwclock is simulated by hiding it
from both, rather than by asserting on the shape of the check.
Every assertion now fails when the behaviour it describes is removed:
hwclock required by check_exec_or_exit again 5 assertions fail
the hwclock guard removed 2 assertions fail
systemd-timesyncd left running 2 assertions fail
systemd-timesyncd left enabled 2 assertions fail
The debian/control and xCAT.spec checks are kept as they were. Those are manifest contents, not
behaviour -- there is nothing to execute, and the assertion is on a package name that survives
reformatting.
The match on makentp.pm for xCAT::NTP::Backend->choose is dropped. It asserted that a call site
exists; ntp_backend_selection.t already drives the selector itself across 33 assertions, which is
the decision that matters.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
An xCAT management node must serve time to its compute nodes (they point at
ntpservers=<xcatmaster>). Ubuntu ships only systemd-timesyncd, an SNTP client that
disciplines the local clock but cannot serve time, and makentp supports chronyd and
ntpd only. On a stock Ubuntu MN makentp therefore fails outright with "Please make
sure ntpd is installed", reddening reg_linux_diskfull_installation_flat.
Two further defects compound it. setupntp hard-requires hwclock through
check_exec_or_exit, but Ubuntu 24.04 moved hwclock into util-linux-extra, which is
absent from minimal images -- so the whole NTP setup, including the clock step that
does not need hwclock at all, aborts. And systemd-timesyncd is part of systemd rather
than a time-daemon package, so it coexists at the package level and keeps disciplining
the clock against whichever daemon xCAT just configured.
Assert a shared, unit-tested daemon selector in the spirit of xCAT::DHCP::Backend, the
package dependencies that guarantee a server-capable daemon and hwclock, and the
setupntp changes. All fail today.
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.
Pin the riscv64 %ifarch blocks in the xCAT, xCATsn and genesis specs
(genesis token, ipmitool-xcat, no x86 PXE loaders) and the riscv64
entries in buildcore.sh, buildlocal.sh and the buildrpms.pl notes.
Resolve the rocky10/rhels10 point releases through
imgutils::get_profile_def_filename with the OS search list emulated, and
assert the riscv64 files exist, win the lookup, match the x86_64 package
lists and point service images at the riscv64 dependency repository.
Pin the riscv64 installer kernel lookup in anaconda.pm and
geninitrd.pm, and evaluate the rh/genimage default driver and resolver
library blocks for riscv64 and the existing architectures.
Add an autotest nodeset case with bogus riscv64 grub2 nodes and a
rhels10.99 riscv64 osimage, checking the per-node grub2 files, the
grub2.riscv64 loader link and the hex-ip/mac links.
Drive mknb riscv64 --configfileonly through the existing stubs and
assert the grub2 network configuration: name, cpu guard, kernel, initrd,
xcatd endpoint, serial console, lzma preference, no PXELINUX/xNBA/
petitboot output, removal without artifacts and for :noboot interfaces.
Also install a riscv64 Genesis export and reject it for another
architecture.
Exercise _default_netboot() with the database-backed modules stubbed:
riscv64 defaults to grub2 and keeps any grub2 variant, the x86, PowerNV,
ppc, onie and aarch64 outcomes are unchanged, and undefined inputs do
not warn.
Assert the Kea xcat-riscv64 class (architecture 0x001b only, grub2
boot file, present with and without xNBA loaders) and pin the ISC
subnet block so the riscv64 branch stays ahead of the /yaboot
fallback that would otherwise shadow it.
Pin lookupNetboot, the profiled-node netboot rule table and the schema
descriptions for riscv64 without loading the database-backed modules:
the shipped subroutines are extracted from the source and evaluated
directly, so the test runs without DBI.
Daniel Hilst
Vinícius Ferrão