Running the wait on a healthy, steady-state minion always reported readiness
after a 3s floor:
salt-minion (pid 4114640) ready after 3s
That floor came entirely from the unconditional sleep "$INITIAL_SLEEP" (3s)
before the poll loop. The sleep is vestigial: it predates restart_pending and
never even covered the restart race (see 89e6a746c -- "INITIAL_SLEEP=3 expired
inside that window"). Salt restarts the unit with --no-block and the restart
job is enqueued before service.restart returns, so an in-flight restart is
visible to restart_pending on the first loop iteration; the sleep protects
nothing now.
Drop the INITIAL_SLEEP constant and the pre-loop sleep and start elapsed at 0.
The loop already sleeps at the bottom of each iteration, so the first readiness
check now runs immediately: an already-ready minion returns "ready after 0s",
while the restart path (guarded by restart_pending) and the mid-startup log
gate are unchanged.
The wait required both a socket gate and a log gate to pass. The log gate
greps the minion log for salt's one-time startup line "Minion is ready to
receive requests!", which scrolls out of the log tail on a minion that has
not restarted recently. On such a minion the log gate could never pass, so
the script ran to its full 120s timeout and exited 1 even though the minion
was healthy and connected. This also false-timed-out when salt_minion_service
reported a non-restart change (e.g. an enable toggle).
The log gate's only remaining purpose is closing the ~2.8s post-connect window
where the master sockets are up but _post_master_init() is still loading. Gate
it on the current pid's uptime: enforce the ready line only within
READY_LINE_WINDOW (90s) of (re)start, and let the already restart-independent
socket gate be the steady-state authority past that. The fresh-restart path is
unchanged, and if uptime can't be read the strict behavior is kept.
Add 100%-coverage unit tests for the three custom salt beacons
(postgres_pillar_beacon, rules_beacon, zeek) and add salt/_beacons to
the python-test workflow's paths trigger and matrix.
To pass the workflow's flake8 lint over the whole directory:
- zeek.py: reindent to 4 spaces, drop trailing blank line, noqa the
Salt-injected __salt__ references (F821); no logic change.
- postgres_pillar_beacon.py: noqa C901 on beacon() (complexity 13 > 12).
Containers now start on boot via restart_policy unless-stopped, so a
highstate is no longer required to bring them up. Gather and display the
container table even when no highstate has completed since reboot, while
still warning the user. The exit code / JSON status_code stays 2 in that
state so SOC's Grid continues to show the restarting message unchanged.
Before making any changes, verify the grid is in a good state:
- check_cluster_health: waits for Elasticsearch to reach at least 'yellow'
(blocks only on red/unreachable, since yellow is normal), modeled on the
wait in so-elasticsearch-roles-load.
- check_fleet_server: confirms the Fleet Server status API returns HTTP 200,
modeled on the wait_for_so-elastic-fleet state in elasticfleet/enabled.sls.
Both run alongside the existing check_pillar_items (manager pillar render) and
verify_es_version_compatibility, before soup modifies anything, so a failure
exits cleanly with an actionable message and no partial changes. Valid on all
manager roles soup runs on (eval/standalone/manager/managerhype/managersearch/
import), which all run Elasticsearch and the Fleet Server.
After a partial upgrade, /etc/soversion already reads the target version, so
soup's startup line "Found that Security Onion X is currently installed" made
it look finished even as soup resumed. When a resume marker is present and
differs from the installed version, print an explicit NOTE that the grid is only
partially upgraded and this run will resume and complete it.
Also clear any stale resume marker in the already-latest path so a successfully
completed upgrade is never mistaken for a partial one and re-run on a later
invocation (the marker is normally removed at the end of postupgrade_changes;
this is a belt-and-suspenders guard).
When soup fails via the EXIT trap after it has begun modifying the system, print
a prominent UPGRADE INCOMPLETE banner instructing the user to run soup again to
resume and complete the update. Gated on a new SOUP_UPGRADE_STARTED flag set at
the start of the hotfix and upgrade branches, so pre-flight gate failures (ES
compatibility, disk, network) that abort before any changes are made do not show
it.
A failed highstate mid-upgrade left /etc/soversion already advanced to the
target version (the highstate stamps it from the pillar via the soversionfile
state), so a re-run of soup saw INSTALLEDVERSION == NEWVERSION and reported
"already running the latest version", stranding the box with post-upgrade
steps never run.
Introduce /etc/sopostversion, a soup-owned marker (no salt state manages it)
that records post-upgrade walk progress. It is seeded from the pre-upgrade
version before the highstate, advanced after each post_to_* step, and removed
on successful completion. upgrade_check treats a leftover marker as "upgrade
not finished" and resumes the remaining post steps instead of bailing.
Also fix the hotfix path: /etc/sohotfix was written before the hotfix
highstate, so a failed hotfix highstate looked already-applied on re-run.
Since no salt state manages /etc/sohotfix, defer its write (update_version)
until after the highstate succeeds so it is an honest completion marker.
With two independent checks now writing to the same log, messages like
"system uptime only N seconds does not meet 1800 second requirement" were
ambiguous about which check they came from. Prefix every line with a
[minion-restart-check] or [boot-highstate-check] tag and reword the uptime,
threshold, and healthy messages to say what was evaluated and why it was
skipped.
Restructure the boot-highstate check from a nested if into an if/elif chain so
each outcome (restart already queued, uptime too low, healthy, already running,
forcing) logs its own reason instead of silently doing nothing.
Add a second, independent trigger to the every-5-minute health check: if the
host has been up >= 15 minutes (HIGHSTATE_UPTIME_REQ) and no highstate has
completed since this boot (lasthighstate mtime older than boot time), run
salt-call state.highstate. This recovers a host whose boot highstate
(so-boot-highstate.service) failed or was skipped, even while the minion is
otherwise healthy and touching state-apply-test.
The new path deliberately does not enable highstate, so a soup-disabled
highstate is respected and never forced mid-upgrade. A saltutil.running guard
plus queue=True prevents stacking across successive cron runs, and a RESTARTED
flag suppresses the new block when the existing minion-restart path already
queued a highstate.
Live testing on a standalone node found the previous commit still reported
ready on the OUTGOING daemon. Reproduced by running the production sequence:
systemctl restart --no-block salt-minion # what service.restart issues
/usr/sbin/so-salt-minion-wait # what cmd.run then runs
so-salt-minion-wait: gating on pid-tagged ready line ... plus master sockets
salt-minion (pid 2750297) ready after 3s # 2750297 is the OLD child
salt restarts this unit with --no-block -- _no_block_default() in
salt/modules/systemd_service.py returns True when the unit is the salt-minion
service -- so service.restart returns as soon as the job is enqueued. Measured
on the host, systemd does not swap MainPID until ~7.3s later. Throughout that
window the old daemon is still running, still holds its master sockets, and
its own ready line is already in the log, so every gate passes on the instance
that is about to be torn down. INITIAL_SLEEP=3 expired inside that window.
Wait for systemd's job queue for the unit to drain before resolving MainPID.
That is deterministic rather than a timing guess: the job exists from the
moment --no-block returns until the new instance signals READY, and MainPID is
new by the time it clears. Measured transition:
t=0.0s job pending, child=OLD, sockets up, ready line present
t=7.9s job drained, child=NEW, sockets up, ready line ABSENT
t=10.7s ready line for NEW child appears <- script returns here
The same run also confirms empirically why the log line is required in
addition to the sockets: for 2.8s the new child has both master connections
while _post_master_init() is still loading modules and compiling pillar, so a
socket-only gate would return that much too early.
Correct the comment claim from the previous commit. The --no-block restart is
real; it lives in salt's systemd_service module, not in this repo, which is
why searching the repo for it turned up nothing.
The reposync section in repodownload.conf and the client repo assigned in
repo/client/oracle.sls both used the bare name securityonionkernel, colliding
across the two roles. Rename the reposync-side section (and its --repoid, the
so-repo-sync guard, and the so-kernel-upgrade presence check) to
securityonionkernelsync, mirroring the existing securityonion/securityonionsync
split for the main repo. The client repo stays securityonionkernel. Also give
the section its own name=Security Onion Kernel Repo repo.
When the minion is deemed hung and restarted, wait for it to become
ready via so-salt-minion-wait, then kick off salt-call state.highstate
(queued, backgrounded) so the box re-applies its states and recovers on
its own rather than waiting for the next scheduled highstate.
The previous gate did not detect whether the restarted minion was back:
systemctl is-active --quiet salt-minion \
&& salt-call --local --timeout=5 --out=quiet test.ping
Both halves are near-vacuous. `--local` sets file_client=local, so test.ping
runs in a throwaway minion that never contacts the master and never inspects
the running daemon; it only proves python and the module loader work. And the
shipped unit is Type=notify with notify_systemd() called before the daemon
imports salt.cli.daemons, so is-active goes true at process launch, not at
connection. The script could return ready while the minion was still
authenticating, which is the race it exists to prevent.
Gate instead on the condition salt itself uses to log "Minion is ready to
receive requests!", requiring both signals of the current daemon instance:
1. the pid-tagged ready line in the minion log. tune_in() emits it only
after sync_connect_master() returns, i.e. the pub channel authenticated,
the req channel connected, and _post_master_init() finished loading
modules and compiling pillar.
2. that same pid holding an ESTABLISHED req connection to a master on 4506
plus a second (publish) connection to the same master IP. The publish
port is absent from minion config -- the minion learns it from the
master's auth reply -- so it is derived from the connection.
Resolve the daemon pid from systemd (MainPID -> pgrep -P), never from
/var/run/salt-minion.pid. salt_minion() runs the minion in a multiprocessing
child; that child writes the pidfile, owns the sockets and logs the ready
line, while MainPID is the parent. During a restart the pidfile still names
the old child, whose own ready line is already in the log, so keying off it
reports ready instantly. Children of the current MainPID exclude the old
instance structurally, with no timing assumptions.
Degrade deterministically rather than spinning to the timeout: if
log_level_logfile does not emit INFO records the ready line can never appear,
so detect that up front from the merged config and fall back to the socket
check. log_level_logfile defaults to None (inherit log_level), so resolve the
inheritance before deciding. If ss is unavailable, fall back to the log gate.
If neither signal is usable, fail immediately with a clear message.
Requiring master connectivity adds no new dependency: every path that applies
salt.minion or a highstate does so without --local, so file_client=remote
already required a reachable master to fetch salt:// files. No salt-call
master round-trip is added; the daemon's own successful auth already proves
the key is accepted.
Also fix the comment above wait_for_salt_minion_ready, which attributed the
script to common_sbin/common/tools/sbin (it is deployed by salt_sbin from
salt/tools/sbin) and asserted a --no-block restart that appears nowhere in
the repo. No state logic changed.
Josh Patterson
Jason ErtelandGitHub
Jason Ertel
Doug BurksandGitHub
Jorge ReyesandGitHub
Mike ReevesandGitHub
Mike Reeves