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>
Own collection, sensitivity, finalization, and reset as one request-scoped state object so xcatd only forwards callbacks and appends the finalized text.
The commands.log response classifier used a "passw" text match on the
request arguments. A secret whose name has no such text passed the
check, so a read of an authentication key, a privacy key or the snmpc
site value logged its bare value in the response. A command that
expands an argument also passed the check: nodels with a table name
returns every column of the table, and lsdef returns attributes that
the request never names. The daemon also ran redact_password over the
whole connection log on each request, so the redactor split at the
first request of the connection and the change signal swept the text of
earlier requests and responses.
Add secret_in_request. The routine reports a request that names a
secret attribute, selects a secret site key, or dumps a table that owns
a secret column through tabdump or nodels, from the same secret set
that the argument redaction uses. The response classifier calls it, so
the response of such a request logs as redacted.
Add secret_in_response. The routine reports response text that holds
"passw" or a secret attribute name in assignment or column form. The
response finalizer calls it in place of the bare text match, so an
expanded listing that carries an authentication key or a product key
logs as redacted even when the request never names it. The lsvm
response is the directory entry, whose passwords are positional, so
the classifier marks the command itself.
Build each request segment alone, redact the segment, and then append
it to the connection log. The redactor now always sees the current
command, and the change signal covers only the current request.
The daemon redacted secret attributes on the joined command string. The
match failed when a value held a space. The match also failed for a "+="
splice assignment. The validate() path did not quote the arguments, so a
multi-word secret value kept its later words in syslog and in the
auditlog table. A password that a command receives through an option or
a positional operand was not redacted at all. The debug dispatch trace
wrote the raw arguments to syslog when site.xcatdebugmode was set.
Redact the argument vector before the daemon joins it. Add
redact_password_args for this task. The routine masks the value of a
secret attribute in any argument, at the start or embedded after another
token. An embedded secret assignment masks to the end of the argument,
because a shell value may hold quotes and spaces. The routine allows
spaces around the operator. It accepts the "=", "+=", ",=", "^=", "!=",
"=~" and "!~" operators that chdef, nodech and node selection use. It
masks a password option value in each form that Getopt::Long accepts: a
separate argument, a compact short option, a bundle of short options
with the "?" help letter, a "+" option prefix, a single-letter option
with two dashes, a long option, a long option with an equals sign, and
an abbreviated long option. The long-name match runs first, so a long
option keeps its name and masks its value. A walk over each bundle then
finds the first secret letter, so the mask always starts at the option
and the result does not depend on hash order. The walk knows which
other letters of a command take a value, so a secret letter inside such
a value does not redact and the audit text stays correct. The walk also
knows which letters take an integer, because the z/VM cpu option
consumes only its signed digits and the parser then continues the
bundle into the password option. The value stops match letter case,
because a bundle keeps short options case sensitive and an unknown
capital letter does not absorb the rest. The mkvm secret match ignores
letter case, because the z/VM parser keeps the Getopt::Long default for
long names. The mkhwconn match keeps letter case, because -p
is the hardware control point and -P is the password. The routine knows
the password options of bmcdiscover, switchdiscover, mkhwconn, mkvm,
createvcluster, lsvcluster and rmvcluster, the rspconfig password
assignments, the mkvm clone pw= operand, and the positional password
operands of chvm. It masks the site.value argument of tabch and chtab
when a selector or a site.key assignment names snmpc. An exact short
option that takes a non-secret value stays visible, so the PPC mkvm -p
profile is not an abbreviation of --password. The dispatch trace builds
its text from the redacted vector.
Add snmpc, productkey, prodkey.key, tokenid and token.tokenid to the
secret list, with community and pdu.community. The secret list holds
only attributes that map to a secret column, so key and sshkeydir stay
visible.
redact_password keeps a second pass over the joined string. This pass
masks an embedded secret assignment to the end of the line, because the
argument boundaries are gone after the join.
The commands.log response classifier marks a response sensitive when the
request was redacted. The argument vector pass sets that signal, so a
secret whose name has no "passw" text still marks its response.
xcatd redacts the request in commands.log but appends the command response
verbatim. A command whose output holds a secret writes it in clear text.
Examples are tabdump passwd, gettab of a passwd column, and getcredentials.
Collect the response into a per-command buffer. Set a sensitive flag when the
command is getcredentials, an argument names a password, or the request was
redacted. When the command finishes, replace the whole buffer if the flag is
set or the buffer still holds password content, then append the buffer. A
connection can carry more than one command, so the buffer is finalized at the
next command's start and at the end of the connection.
The buffer holds the full response, so a secret split across several callbacks
is also redacted. A per-callback check cannot do this.
The word-content check is a fallback. The request classification is the main
signal. A secret with no password marker, such as the output of an xdsh cat of
a shadow file, is a pre-existing leak of the root-only log. It is out of scope.
Recovered from the lenovobuild branch. Reimplemented against master.
xcatd expands the request noderange before it authorizes the caller: once to
count the nodes, and once in validate() to match the policy rules. The ^
operator makes xcatd open a caller-named file at that point. A client can
connect without a certificate, because the listener does not require one, and
such a client has no peername.
Expand these two pre-authorization noderanges with nofile when the caller has
no peername (checked with defined, so the identity "0" still counts as
authenticated). If validate() finds a rejected ^file atom on such a request,
deny it. An authenticated caller expands ^file as before.
xcatd warns "xCAT Version mismatch!" when a node's xCAT version differs
from the server's. It compared the full version strings, which include a
build-specific suffix such as " (git commit <hash>)". Two nodes at the
same release built from different snapshots then reported a version
mismatch on every request, even though the same release is ABI
compatible.
Keep warning when the versions differ, but tell the two cases apart. A
different release is still "xCAT Version mismatch!". The same release
built from a different commit now reports "xCAT build level differs (same
release):" instead, so the build difference is still visible without
being called a mismatch. Both messages show the full version strings.
Add xCAT::Version->Release, which returns the version without the
build-specific suffix, to make that distinction.
This was recovered from the lenovobuild branch, which stripped the older
"built <date>" suffix and dropped the same-release warning entirely; this
reimplements it for the current version format and keeps the build
difference visible.
* Fix silent failure when site.master is not set (#6157)
Hardware control commands (rpower, rinv, etc.) silently return no output
and exit 0 when site.master is empty. The original fix (#6074) was
reverted (#6158) because it warned per-node with the wrong hostname.
Check once in plugin_command before dispatching to plugins, so the error
appears exactly once with the correct command name.
* Also reject empty site.master, not only undef
The else branch handles a rare case where XS libraries (Sys::Virt,
Net::SNMP) crash without setting $@. This IS a plugin bug, so keep
that label and the debug hint. Only the common case (die with $@)
gets the clean passthrough.
When a plugin dies during request processing, xcatd wrapped the error
in a misleading "plugin bug" message that hid the real cause (e.g.
"No space left on device"). Now passes through the actual error from
the eval, making the output useful for any failure, not just disk full.
Fixes#2719
- detect provisioning when updateing status to installing/netbooting
- consistent message format "xcat.updatestatus - ($node): changing status=($status)" (no need to log the time as syslog has the timestamp)
* site cache when run plugin does not work very well
- using cache from plugin when getNodesAttribs/getNodeAttribs (pass it into DB process from plugin process)
- Site cache is a whole hash, so to use cache when by the hash is there, instead of the specified key is there.
It is because that there might be no key defined in site table.
* with XCATBYPASS, to populate site hash before scan_plugins. Then only 1 query for site table to do whole things.
* cache site when init plugins on service nodes
* missing to comment the old codes query from xCAT DB process
- modify rsyslog conf and add "%syslogseverity-text:::uppercase%" to input the text Severity
- remove duplicate Severity in log message
- modify MsgUtils::trace for better performance and code structure
- make sure $pid_UDP is set to zero when UDP process quit in CHLD handler
- disable the signal handler for CHLD in the end of SSL lister, and make itself to handle reaper via `wait`
* Fix the issue 'install monitor' exit unexpected (#4582).
- add eval block to cover all possible codes which might die in do_installm_service
- add more logs in install monitor to record useful inforamtion
- add sleep 0.01 to avoid CPU too high
- fix the wrong timeout usage (alarm - die)
- recover the default INT/TERM singal handler to avoid UDP listener will quick when kill 'install monitor'
* Avoid SSL CPU 100% when hit max open file number
- clear logs in the key components during provisioning (not only debug mode, but debug mode will have more logs)
- now only cover rhel7 stateful and stateless
- make more syslogs into cluster.log (tag = xcat or xcat.*)
- the changes will not impact other distro
Daniel Hilst
Vinícius Ferrão
Vinícius Ferrão
Mark Gurevich
zet809
yangsong
Gᴏɴɢ Jie
bxuxa
cxhong
Bin Xu
yangsbj
Casandra Qiu
Matt Ezell
hu-weihua