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.
The probe spelled out every candidate path twice inside one nested condition,
once to test it and once to assign it, so adding an architecture meant adding
another branch of the same shape. Move the candidates into a table keyed by
architecture family and walk it in order.
Same paths, same precedence, same failure behaviour: a media tree that matches
nothing leaves the caller on the "install image not found" path as before.
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>
`makedhcp -q <node>` on Ubuntu's ISC-limited releases answers "no DHCP reservation
found" when it cannot read dhcpd.conf. The operator reads that as a node without a
reservation. An InfiniBand node also gets an answer with no hardware address.
_query_isc_static_host in dhcp.pm read the file with an -r test and dropped a failed
open. It also matched only a "hardware ethernet" line, while _add_isc_static_host
writes "hardware infiniband" for an InfiniBand node and adds a twin declaration
between the same markers.
_read_isc_conf_lines now returns the read error, _query_isc_static_host returns it to
listnode, and listnode answers the caller with an error. The parser accepts any
hardware type and keeps the first declaration of the block. The path of dhcpd.conf and
the distribution name are package variables, so a test can drive the query and
listnode.
dhcp_isc_static_host_query.t covers the InfiniBand address, the twin declaration, the
unreadable file and the listnode answer. It fails 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>
The respawn is forked from the middle of the service loop, so the child
inherits @pendingconnections -- the client sockets the parent has accepted and
not yet handed to a worker. The monitor never serves one, and it outlives the
worker that does, so its copy keeps that client's socket open until the daemon
exits.
Close them in the child, next to the listener and the rescanplugins channel it
already drops.
xcatd_install_monitor.t runs the lifted respawn block against stand-in
descriptors and requires every pending connection to be closed. It fails
without this change.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
The install monitor is forked while generic_reaper is the SIGCHLD handler.
ssl_reaper is only installed once the main service loop starts, and
generic_reaper comes back whenever connections are throttled.
Only ssl_reaper cleared $pid_MON. A death reaped by generic_reaper left
$pid_MON holding a dead pid, and the service loop re-forks only when $pid_MON
is clear, so xcatiport stayed dead for the life of the daemon.
Move that accounting into reap_install_monitor and call it from both reapers.
xcatd_install_monitor.t runs both reapers over a dead child and requires each
to clear $pid_MON and fold the death into the pacing. The generic_reaper case
fails without this change.
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>
The module overrides two methods of XML::Simple, one for a recent version and
one for an older version, and each built its own parser and set its own
handlers. The two bodies were the same apart from spacing, so a change to one
refusal had to be repeated in the other, and a reader had to compare them to
see that they agreed.
Build the parser in one routine that both call. Behaviour does not change.
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.
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 daemon reads the XML of every request through this parser. A request could
declare an entity in its own document type declaration, and the parser expanded
it. An entity that refers to other entities grows on each level, so a short
request expands into a large document and consumes the memory and the time of
the daemon. A client holds a certificate before it can send a request, so this
needs an account, but the daemon should not accept the work.
Refuse the declaration itself. The option that stops the parser from expanding
an entity does not cover an entity that a request names inside an attribute, so
it leaves the same growth available through a different part of the document.
Measured on XML::Parser 2.46, a request of 204 bytes that names its entity in
an attribute still grew to 1014 bytes with that option set, which is what the
parser does without it.
No request that xCAT sends carries a document type declaration. The client
builds every request with XML::Simple, which does not write one.
The handler that refuses an external entity stays, so a parser that reaches it
by another route still refuses to read the named file.
Both parser constructors passed a list of options to XML::Parser as an array
reference:
XML::Parser->new(Style => 'Tree', [ load_ext_dtd => 0, ... ]);
XML::Parser->new takes a flat list of pairs. The reference is one value in that
list, so the constructor reads the pairs as Style => 'Tree' and then the
reference as the name of an option with no value. Every option inside the
reference is dropped. The names are also the names that XML::LibXML uses, not
the names that XML::Parser uses, so the parser would ignore them even if it
received them.
The options therefore never did anything, and they give the reader the
impression that the parser refuses an external entity because of them. The
handler on the next line is what refuses an external entity.
Remove them. Behaviour does not change.
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>
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>
xCAT::RespawnUtils::policy() was called near the top of the file, some twenty lines above
the "use xCAT::RespawnUtils" that loads the module. It works, because use is compile-time
and perl compiles the whole file before running any of it, so the import has already
happened by the time that statement executes. But nothing at the call site says so. It
reads as a plain ordering mistake, and it stops working the moment someone converts the
import to require -- a routine thing to do to a daemon that loads this many modules -- with
the failure being an undefined subroutine at startup.
Move the declaration below the imports, next to the osver() call that already makes a
runtime call to a use'd module there. The only constraint on where it can go is that
ssl_reaper closes over $mon_respawn and so must be compiled after it is declared; the new
position clears that by a thousand lines, and compiling under strict is what proves it,
since a lexical declared after the sub would fail to compile rather than silently bind
elsewhere.
Pure relocation: the moved block is byte-identical and no behaviour changes.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
The respawn forks from the main service loop, much further down the program than the fork
at startup, so it inherits everything the parent has opened in between. That is the
rescanplugins socketpair from further up this file -- the channel a subcommand process uses
to hand a reloaded cmd_handlers hash back to the parent. The child closes the SSL listener
and the UDP control socket but not those two, so a respawned monitor holds both ends of a
channel it never reads or writes, for as long as it lives.
Measured on a live MN by diffing /proc/<pid>/fd between a monitor forked at startup and one
respawned after being killed: the respawned process carried one extra socket, and both ends
of that pair were also held by the SSL listener parent. The leak is two descriptors and it
does not accumulate, since each respawn forks afresh from the parent; the reason to fix it
is that the block is commented "serve only the install monitor" and no longer did, so a
monitor's file descriptors depended on whether it was the first one or a replacement. That
is the kind of difference that makes a later problem reproduce only on one path.
Close both ends in the respawn child. The monitor's own plugin-rescan channel is a
different socketpair, created before either fork, and is untouched. Verified afterwards on
the same MN: the respawned monitor no longer shares a socketpair with the parent, and still
binds xcatiport and serves it, with the SSL listener holding its pid throughout.
Not covered by a test. Both the unit suite and the xCAT-test case format work at the level
of processes and ports; this is an invariant about file descriptors that needs /proc on a
running daemon, and asserting it there would be more fragile than the line it guards.
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>
The respawn of the install monitor was paced by a retry budget that, once spent, made
the daemon stop trying for good. That put xcatiport back in the state the respawn was
added to fix: with no monitor alive there is nothing left to reset the counter, so the
port stays dead until the whole daemon is restarted, and a port that frees up a minute
later is never picked back up. It only reached that state more slowly than before.
Pacing itself is needed. do_installm_service dies when it cannot bind the port, so an
unguarded re-fork spins as fast as fork allows while something else holds it, and keeps
re-entering that function's USR2 socket-takeover handshake. Replace the budget with an
exponential backoff that has a ceiling but no end: the delay doubles from
XCATD_MON_RESPAWN_MIN_INTERVAL (default 5s) to XCATD_MON_RESPAWN_MAX_INTERVAL (default
300s) and stays there. A monitor that cannot start therefore costs one fork per five
minutes for as long as that lasts, and is back within five minutes of the port becoming
free, with no restart and no operator action.
A monitor that ran for XCATD_MON_RESPAWN_HEALTHY seconds (default 60) plainly got the
socket and served, so its eventual death resets the delay: an isolated death is retried
at once and the backoff only builds up during a real streak of failures to start. The
ceiling is reported once per streak rather than on every attempt, and says that xcatd is
still retrying instead of that it has stopped.
The pacing lives in a marked mon-respawn-policy region, free of forking and of daemon
state, so xCAT-test/unit/xcatd_monitor_respawn.t drives the real code rather than a copy
of it.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Re-fork the install monitor from the main service loop when $pid_MON has been cleared
and xcatiport is still configured, so a single death of that child no longer leaves
the port dead until the whole daemon is restarted. The forked child closes the SSL
listener and the UDP control socket before re-entering do_installm_service, so it
serves only the install monitor.
Rate limit the respawn. do_installm_service dies when it cannot bind the port after
its own retries, which is exactly the case where an unguarded re-fork would spin as
fast as fork allows and keep re-entering that function's USR2 socket-takeover
handshake against whatever still holds the socket. Consecutive attempts are separated
by XCATD_MON_RESPAWN_INTERVAL seconds (default 5) and capped at XCATD_MON_RESPAWN_MAX
(default 10), after which xcatd logs that it is giving up on the port rather than
retrying forever. A monitor that stayed up long enough to outlast the whole retry
budget resets the counter, so an unrelated death much later gets a full budget again.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
The NTP backend selector was well covered and nothing connected it to makentp:
copying the base makentp.pm over the head one left the whole unit suite
byte-identical, so the branches that consume choose()'s answer -- abort on a
selector error, warn on a downgrade, abort when neither daemon is installed --
and the --backend argument handed to setupntp were covered by nothing.
They were unreachable from a test because they sat inside process_request,
which needs a management node. Move the decisions into ntp_backend_action() and
setupntp_command(), which take their inputs and return an answer; the caller
keeps send_msg and runcmd. No behaviour changes -- the same messages are sent
on the same conditions, and the same command is built.
Verified by mutation rather than by reading: dropping the install abort reds 3
of 15, the downgrade note 1, the --backend argument 2, the server-list split 1,
and the selector-error abort 2.
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>
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>
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>
noderes.installnic names the adapter that deploys the operating system. The
kickstart network line ignored it and named the adapter from mac.mac alone, so
a node that sets installnic got a kernel command line that obeys installnic and
a kickstart that configures a different adapter. On a node with more than one
adapter the installer then brings up the wrong one and cannot reach the
repository.
Name the device from the shared resolution, which gives the interface name when
installnic or primarynic names one, and the address otherwise. A node that sets
neither attribute keeps the address it has today.
The unique local address still comes from the hardware address, because
autoulaaddress builds the address from it.
The order that names the install device is noderes.installnic, then
noderes.primarynic, then mac.mac. gen_net_boot_params owns that order for the
netboot kernel parameters. Only the Ubuntu template reused it. Every other
install template reads mac.mac on its own.
Move the reuse into install_device_params so that any install template can
share it. subiquity_install_netcfg keeps its own name and its own return
value, because netplan needs the pair of a name and an address. Behaviour
does not change.
Vinícius Ferrão
Daniel Hilst