copycd translated the architecture the Ubuntu media reports with its own
if/elsif chain, and genimage translates the same names back for debootstrap with
another one. Neither can be reused, so a new architecture has to be added to
both.
Put both directions in xCAT::Utils and have copycd read from there. The names
and the fallback do not change: media that xCAT has no name for still leave the
architecture as the media reported it.
Use xCAT::Utils->xfork so child-side setup or exec failures do not destroy database handles inherited from the caller.
Signed-off-by: Vinícius Ferrão <2031761+viniciusferrao@users.noreply.github.com>
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>
makentp reads site.ntpbackend to select the NTP daemon, and setupntp takes the same value as
--backend, but no help text names the attribute. An admin who needs ntpd on a host that has
chrony has no way to find out the attribute exists. site.dhcpbackend, which selects the DHCP
implementation the same way, is documented in the site table help.
The site table description in xCAT::Schema now carries ntpbackend beside ntpservers, with its
valid values and the auto default. The makentp man page lists it with the other site attributes
the command honors, and names the setupntp --backend option that carries the value to the nodes.
ntp_backend_selection.t reads the site help from the loaded schema and the makentp pod. Five
assertions fail without this change.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
xCAT::NTP::Backend->available reported chrony as available on chronyd alone, while makentp
configured chrony only where systemctl was present too. On a host with chronyd and no systemctl
the selector returned chrony with no downgrade, makentp fell through to the ntpd path, and the
admin saw either a silent switch or "Please make sure ntpd is installed".
available now requires chronyd and systemctl for chrony, so the selector answers on the same
terms makentp acts on, and makentp branches on the name alone. choose therefore downgrades to
ntpd, or reports install, in the case it used to pass over. A commands argument injects the
command probe, in the same shape as the existing available argument.
ntp_backend_selection.t covers both commands. Six of its assertions fail without this change.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
The comments around the install monitor respawn retell the failure, defend the
design and repeat the same causal chain in three places. Reduce them to the
facts that are not visible at the site: the ordering rules, why there is no
attempt limit, and what each fork site inherits. The rest is in the commit
messages and the PR.
Comment only. RespawnUtils.pm loses 26 lines and no code changes; xcatd loses
comment lines only. Both unit test files still pass.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Fall back to an available DHCP backend on auto-selection. When the request is "auto"
and the backend chosen for this OS is not installed, use the other one if it is,
recording fallback_from so process_request can tell the operator which preferred
backend is missing. A backend the admin forced through site.dhcpbackend still fails
hard when absent, and "neither installed" still errors clearly. Fixes#7710.
Answer makedhcp -q from the static host block on Ubuntu's ISC-limited releases.
listnode now branches on _isc_static_host_fallback() before any omapi work and reads
the node's fixed-address and hardware ethernet straight out of dhcpd.conf, so the
query path never spawns the omshell its own write paths already avoid. A node with no
reservation is now reported rather than answered with silence.
Match the host-block markers exactly. _add_isc_static_host writes a fully determined
pair -- "#xCAT host declaration for <node> aka host <hostname> start" and the "}"
line carrying the matching end -- so both scans anchor on that whole shape through
shared _isc_host_start_re/_isc_host_end_re helpers. The previous /\Q$node\E\b.*/ also
matched at a hyphen, letting node "compute" act on "compute-01"'s block: the query
could return another node's address and the delete could remove another node's
reservation. _delete_isc_static_host also accepts an explicit line list now, so the
scan is unit testable without file-scoped state.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
redact_directory_entry masked only the first record of a COMMAND
statement, and did not recognize the CMD spelling. A record ending with
a comma continues on the record below, so a password there reached
syslog and the client.
Mask every record of the statement, in both spellings. Read the
continuation comma in columns 1 to 71, so a sequence number does not
open or close a statement. A blank or a comment record does not end one.
Every in-tree caller now uses isValidIp. The old helper accepted any
value with a colon as a valid address, so there is no reason to keep
it as a wrapper.
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>
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.
supervise() blocks SIGCHLD across the fork but unblocks it before returning, and the caller
installs the pid afterwards:
($mon_respawn, $pid_MON) = xCAT::RespawnUtils::supervise { ... } ...;
so the assignment is outside the blocked region -- the same unprotected window that existed
before e0b0ac6, moved from xcatd into the helper that was meant to make it impossible to get
wrong. ssl_reaper matches the dead child against $pid_MON and folds the death into
$mon_respawn; a monitor dying in that gap is compared against a pid still holding 0, missed,
and the caller then overwrites both with a pid that no longer exists. !$pid_MON never fires
again, so the respawn loop never runs and xcatiport stays dead until xcatd is restarted --
the failure this PR exists to remove.
Have supervise() install them itself, which is why `state` and `pid` are now passed by
reference: the pacing state is recorded and the pid assigned while SIGCHLD is still blocked,
and only then is it unblocked, so there is no point at which a reaper can run and see either
of them stale. Nothing is left for the caller to do afterwards, so both call sites become
plain statements that read $pid_MON when they need it. The child unblocks before running its
body, as it did when the unblock sat ahead of the fork's branch. The new pid is returned as
well, for a caller that wants it inline.
Verified on a live MN (xcat54-mn, AlmaLinux 10.2, xCAT 2.19.0): the startup fork produces a
monitor holding xcatiport 3002; killing it is recovered in 5s, killing the replacement at
once in 11s -- the backoff -- and killing one that had served past the healthy interval is
recovered in 1s, with the port reclaimed and xcatd active throughout. The unit test's window
subtest, red in the preceding commit, now passes.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Both places that fork the install monitor repeat the same careful sequence: record the
attempt, block SIGCHLD, fork, unblock, and on failure record the exit so the next attempt
backs off. Two of those steps are ordering requirements rather than steps -- the attempt
must be recorded before the fork, because the child can die and be reaped before fork()
returns, and SIGCHLD must be blocked across the fork and the assignment, or the reaper
compares the dead child against a stale pid and misses it. Neither is apparent from
reading the code, and both were got wrong at least once while writing it. Leaving them
open-coded means the next caller -- $pid_UDP has the same never-respawned shape -- gets to
rediscover them.
Move the sequence into xCAT::RespawnUtils::supervise(), which takes the child body as a
block and the rest as named arguments:
($mon_respawn, $pid_MON) = supervise {
...the child...
} state => $mon_respawn, pid => $pid_MON, now => time();
The (&@) prototype is what allows the leading block, and it applies to a fully qualified
call, so no Exporter machinery is needed. It does require the module to be loaded with
`use` rather than `require`: under `require` the sub is unknown when the call is compiled,
the block is then read as a bare block and its value arrives as the first argument, which
fails at runtime rather than at compile time. Both call sites and the test use `use`, and
the constraint is written down next to the sub. Passing a live pid is a no-op, so a caller
that forgets to check does not end up with two children.
The module gains its first impure function, which is why it sits under its own heading with
the pure ones stated to be pure above it: those return new state and touch nothing, which is
what keeps them testable on a made-up clock and safe inside a signal handler. supervise()
forks, so it is tested by the fork-and-port case instead, which now drives it rather than
its own copy of the same sequence. POSIX and xCAT::Utils are required inside supervise()
rather than at the top, so loading the module for the pure functions still pulls in nothing.
xcatd loses $mon_chldmask and its :signal_h import along with the duplication.
Verified on a live MN: the startup fork goes through supervise() and produces a monitor
holding xcatiport, and two consecutive kills are recovered in 5s then 10s -- the backoff --
with the port reclaimed and the SSL listener holding its pid throughout.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
The module is seven short subs over a hash of counters, and nothing in it says what is
being paced or why the pacing is shaped this way. A reader can follow every line and still
not know that the delay ceilings rather than terminates, that `healthy` means "stayed up
long enough to have claimed its resource", or that returning a new state instead of
mutating one is load-bearing rather than stylistic.
Add a short header giving the module's intent -- what it paces, why it backs off, why it
never stops, and why the functions are pure -- and one line per sub in
(inputs) -> output form. Not the banner from
docs/source/developers/guides/code/code_standard.rst: every one of these takes a state and
a timestamp, so seven Arguments:/Returns: blocks would restate the same signature and bury
the line that carries meaning.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Three defects found by running the respawn against a live xcatd on an MN rather than only
against its unit tests.
A monitor whose child dies between xfork() returning and the assignment to $pid_MON is
lost for good. ssl_reaper matches $CHILDPID against $pid_MON, so a child reaped in that
window is compared against a stale value and missed, and $pid_MON is then left naming a
pid that no longer exists. The service loop reads !$pid_MON to decide whether to respawn,
so it never respawns again -- the same permanently dead xcatiport this whole change exists
to prevent, reached by a different route. Block SIGCHLD across the fork and the assignment
at both fork sites; the child unblocks on the same line, since it needs to reap its own
children. Reproduced with a widened window before the fix and confirmed closed after.
Recovery took 30 seconds on an idle daemon. The respawn only gets a turn when the service
loop comes round, and the loop parks in $bothwatcher->can_read(30) when there is nothing
to serve, so the full select timeout was being added to the respawn delay. Wait in 5s hops
while the monitor is down and at the usual 30s otherwise, so an idle daemon pays a few
extra wakeups only while xcatiport is actually dead. Measured on the MN afterwards: a
killed monitor returns in 5s, then 10s, then 21s across three kills in a row -- the
backoff, visible in wall-clock time -- reclaiming the port each time, with the SSL listener
holding the same pid throughout.
The tunables are read from %ENV and were compared before being validated, so an empty or
misspelt XCATD_MON_RESPAWN_* put "Argument isn't numeric" in the daemon log at every start.
Anything that is not a plain non-negative integer is now treated as unset.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
The backoff that decides when to re-fork the install monitor is arithmetic over a handful
of counters, but it lives inline in xcatd among the daemon's globals, its signal handlers
and its fork. xcatd needs the database, SSL, the plugin tree and /var/run/xcat before it
will run, so nothing in a unit test can execute that arithmetic; a test can only match
patterns against the script's source and hope the shape it finds behaves. That is how a
retry budget which ran out and could never be refilled passed a green test run.
Move the pacing to xCAT::RespawnUtils as pure functions: each takes the current state and
the current time and returns the next state, reading no clock, no globals and no files.
Passing the time in is what makes the schedule checkable over a virtual clock instead of
in real seconds, and returning a new state rather than mutating one is what makes it safe
to call from the SIGCHLD handler -- the result is built before the caller installs it, so
a signal arriving partway through cannot leave the pacing half-updated.
The behaviour is unchanged from the previous commit and stays covered by
xCAT-test/unit/xcatd_monitor_respawn.t, which now executes these functions instead of
grepping for them: the delay doubles from XCATD_MON_RESPAWN_MIN_INTERVAL (5s) to
XCATD_MON_RESPAWN_MAX_INTERVAL (300s) and holds there without ever refusing a retry, and a
monitor that stayed up XCATD_MON_RESPAWN_HEALTHY seconds (60s) resets the backoff when it
later dies. policy() now also refuses a floor below one second, which would double to
itself and give a fork storm rather than a backoff, and a ceiling under the floor.
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>
Add xCAT::NTP::Backend, a selector for the NTP daemon xCAT configures, in the same
spirit as xCAT::DHCP::Backend. It honours site.ntpbackend, defaults per distro family
(EL7+/SLES15+ chrony, older ntpd, Ubuntu/Debian chrony), and downgrades to whichever
of chrony/ntpd is actually installed rather than installing a second daemon. makentp
selects through it instead of probing /usr/sbin/chronyd directly, so the choice is one
unit-tested code path.
Guarantee the daemon at install time: the xcat metapackage now Depends on
"chrony | ntp" and the xCAT rpm Requires "(chrony or ntp)". Both sit beside the
existing service dependencies the metapackage already declares -- isc-dhcp-server|kea,
apache2, nfs-kernel-server -- because an MN that cannot serve time cannot serve its
compute nodes, and Ubuntu's default systemd-timesyncd is a client only.
Stop requiring hwclock in setupntp. It only persists the already-stepped system clock
to the RTC, and Ubuntu 24.04 moved it to util-linux-extra, absent on minimal images --
so a fatal check_exec_or_exit aborted the entire NTP setup, including the clock step
that does not use it. Use it when present, log and continue when not, and pull
util-linux-extra through Recommends and the diskless pkglist so it usually is.
Disable systemd-timesyncd there too. It ships as part of systemd rather than a
time-daemon package, so nothing displaces it, and it keeps disciplining the clock
against the daemon being configured.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
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.
The option flags of a client class -- always-send and its siblings -- were
passed through as whatever the caller set, so a plain Perl 1 reached the
configuration as the number 1 and Kea refuses to parse that. The class
flag next to it is already normalised; do the same for the option data, so
callers can stay free of JSON.
RISC-V 64-bit UEFI firmware identifies itself with DHCP option 93
client-system-architecture 27 (0x001b, IANA processor architecture
types). Neither DHCP backend knew the value: Kea handed such clients no
boot file and ISC dhcpd fell through to the /yaboot catch-all.
Add an xcat-riscv64 Kea client class and an ISC subnet branch that send
them boot/grub2/grub2.riscv64, the same shape as the aarch64 entries.
The UEFI HTTP boot id (0x001c) is left alone: it needs a URL boot file
and the HTTPClient vendor class, which is a separate change.
riscv64 nodes boot through UEFI and grub2 only, like ppc64le PowerKVM
guests and aarch64. Teach the three places that declare which
noderes.netboot methods an architecture accepts about it:
- xCAT::Utils::lookupNetboot returns grub2,grub2-tftp,grub2-http for
riscv64 so nodeset and rinstall accept those methods.
- the profiled-node rule table in xCAT::ProfiledNodeUtils defaults
riscv64 nodes to grub2.
- the nodetype.arch, osimage.osarch and noderes.netboot schema
descriptions document riscv64.
The arch token is riscv64 everywhere (rpm, dpkg and uname -m agree), so
no alias handling is added.
Vinícius Ferrão
Daniel Hilst