Gate so-salt-minion-wait on real minion readiness

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.
This commit is contained in:
Josh Patterson
2026-07-09 16:55:30 -04:00
parent 66a1141b84
commit 8a3f5d0f81
2 changed files with 138 additions and 17 deletions
+8 -5
View File
@@ -131,11 +131,14 @@ salt_minion_service:
{% endif %}
- order: last
# block until the just-restarted salt-minion is back and can execute modules locally, so
# follow-on jobs and the next highstate iteration do not race the restart. onchanges +
# require on salt_minion_service catches every restart trigger uniformly because watch
# mod_watch results replace the service state's running entry. wait logic lives in
# /usr/sbin/so-salt-minion-wait (deployed by common_sbin from common/tools/sbin/).
# block until the just-restarted salt-minion daemon logs "Minion is ready to receive requests!"
# for the current instance, so follow-on jobs and the next highstate iteration do not race the
# restart. onchanges + require on salt_minion_service catches every restart trigger uniformly
# because watch mod_watch results replace the service state's running entry. wait logic lives in
# /usr/sbin/so-salt-minion-wait (deployed by salt_sbin from salt/tools/sbin/); it keys the ready
# line to the current daemon pid (resolved via systemd, not the pidfile) and corroborates with the
# master req/publish sockets. set_log_levels above enforces the log_level_logfile: info that the
# ready line depends on.
wait_for_salt_minion_ready:
cmd.run:
- name: /usr/sbin/so-salt-minion-wait
+130 -12
View File
@@ -5,31 +5,149 @@
# https://securityonion.net/license; you may not use this file except in compliance with the
# Elastic License 2.0.
# Block until the local salt-minion service is back up and can execute modules locally.
# Invoked from the wait_for_salt_minion_ready state in salt/minion/init.sls after
# salt_minion_service fires its watch-driven mod_watch (a non-blocking systemctl restart),
# so follow-on jobs and the next highstate iteration do not race the in-flight restart.
# Block until the just-restarted salt-minion daemon reaches the point where salt itself logs
# "Minion is ready to receive requests!". Invoked from the wait_for_salt_minion_ready state in
# salt/minion/init.sls after salt_minion_service fires its watch-driven restart, so follow-on jobs
# and the next highstate iteration do not race it.
#
# Salt logs that line from Minion.tune_in() only after sync_connect_master() returns, which means
# the pub channel authenticated, the long-running req channel connected, and _post_master_init()
# finished loading modules and compiling pillar. Two signals reproduce that:
#
# 1. Primary the pid-tagged ready line in the minion log. Salt's log_fmt_logfile embeds
# [%(process)d] just before the message, so this is keyed to one daemon instance.
# 2. Corroborating that same pid holds an ESTABLISHED req connection to a master on 4506 plus a
# second (publish) connection to that same master IP on another port. The publish
# port is learned from the master's auth reply and is absent from minion config,
# so it is derived from the connection rather than read from config.
#
# The daemon pid is resolved from systemd, never from /var/run/salt-minion.pid. salt_minion() runs
# the real minion in a multiprocessing child; that child writes the pidfile, owns the sockets and
# logs the ready line, while systemd's MainPID is the parent. During a restart the pidfile can still
# name the OLD child, whose own ready line is already in the log -- matching it would report ready
# instantly. Children of the current MainPID structurally exclude the old instance.
. /usr/sbin/so-common
# Initial sleep gives the systemctl restart (--no-block by default for salt-minion on
# >=3006.15) time to begin tearing down the old process before we probe for readiness.
set -u
INITIAL_SLEEP=3
TIMEOUT=120
PING_TIMEOUT=5
MASTER_PORT=4506
LOG_TAIL_LINES=10000
DEFAULT_LOG_FILE="/opt/so/log/salt/minion"
LOG_FILE="$DEFAULT_LOG_FILE"
# Decide whether the ready line can ever appear. salt-call --local sets file_client=local, so this
# reads the merged config (honoring minion.d overrides) without contacting the master. salt defaults
# log_level_logfile to None, meaning it inherits log_level, so resolve that before deciding.
LOG_LEVEL_LOGFILE=$(salt-call --local --out=newline_values_only config.get log_level_logfile 2>/dev/null | head -n1)
case "${LOG_LEVEL_LOGFILE,,}" in
""|none) LOG_LEVEL_LOGFILE=$(salt-call --local --out=newline_values_only config.get log_level 2>/dev/null | head -n1) ;;
esac
case "${LOG_LEVEL_LOGFILE,,}" in
all|garbage|trace|debug|profile|info) USE_LOG_GATE=1 ;;
*) USE_LOG_GATE=0 ;;
esac
if [ "$USE_LOG_GATE" -eq 1 ]; then
LOG_FILE=$(salt-call --local --out=newline_values_only config.get log_file 2>/dev/null | head -n1)
[ -z "$LOG_FILE" ] && LOG_FILE="$DEFAULT_LOG_FILE"
[ -d "$(dirname "$LOG_FILE")" ] || USE_LOG_GATE=0
fi
if command -v ss >/dev/null 2>&1; then
USE_SOCKET_GATE=1
else
USE_SOCKET_GATE=0
fi
if [ "$USE_LOG_GATE" -eq 0 ] && [ "$USE_SOCKET_GATE" -eq 0 ]; then
echo "so-salt-minion-wait: no usable readiness signal (log_level_logfile='${LOG_LEVEL_LOGFILE:-unset}', ss not found)" >&2
exit 1
fi
if [ "$USE_LOG_GATE" -eq 1 ] && [ "$USE_SOCKET_GATE" -eq 1 ]; then
echo "so-salt-minion-wait: gating on pid-tagged ready line in ${LOG_FILE} plus master sockets"
elif [ "$USE_LOG_GATE" -eq 1 ]; then
echo "so-salt-minion-wait: ss not found; gating on pid-tagged ready line in ${LOG_FILE} only"
else
echo "so-salt-minion-wait: INFO file logging unavailable (log_level_logfile='${LOG_LEVEL_LOGFILE:-unset}'); gating on master sockets only"
fi
# Emit the pid(s) of the current daemon instance. systemd's MainPID is the parent keepalive process;
# its child runs tune_in. Fall back to MainPID when there is no child (--disable-keepalive path).
resolve_daemon_pids() {
local mainpid children
mainpid=$(systemctl show -p MainPID --value salt-minion 2>/dev/null)
if [ -z "$mainpid" ] || [ "$mainpid" = "0" ]; then
return 1
fi
children=$(pgrep -P "$mainpid" 2>/dev/null)
printf '%s\n' "${children:-$mainpid}"
}
# True iff the ready line tagged with this pid is in the current or most recently rotated log.
ready_logged() {
local pid=$1 f
for f in "$LOG_FILE" "$LOG_FILE.1"; do
[ -r "$f" ] || continue
if tail -n "$LOG_TAIL_LINES" "$f" 2>/dev/null | grep -Fq "[$pid] Minion is ready to receive requests!"; then
return 0
fi
done
return 1
}
# True iff this pid holds an ESTABLISHED req connection to a master on MASTER_PORT and a second
# ESTABLISHED connection to that same master IP on another port. The trailing comma in "pid=N,"
# keeps pid=123 from matching pid=1234. Grid comms are IPv4 (the unit's ExecStartPre gates on ip -4).
socket_ready() {
local pid=$1 mip master_ips
master_ips=$(ss -tnp state established "dport = :${MASTER_PORT}" 2>/dev/null \
| grep -F "pid=${pid}," \
| grep -oE "[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+:${MASTER_PORT}" \
| sed "s/:${MASTER_PORT}\$//" \
| sort -u)
[ -z "$master_ips" ] && return 1
for mip in $master_ips; do
if ss -tnp state established "dst ${mip} and dport != :${MASTER_PORT}" 2>/dev/null | grep -qF "pid=${pid},"; then
return 0
fi
done
return 1
}
instance_ready() {
local pid=$1
if [ "$USE_LOG_GATE" -eq 1 ] && ! ready_logged "$pid"; then
return 1
fi
if [ "$USE_SOCKET_GATE" -eq 1 ] && ! socket_ready "$pid"; then
return 1
fi
return 0
}
sleep "$INITIAL_SLEEP"
elapsed="$INITIAL_SLEEP"
pids=""
while [ "$elapsed" -lt "$TIMEOUT" ]; do
if systemctl is-active --quiet salt-minion \
&& salt-call --local --timeout="$PING_TIMEOUT" --out=quiet test.ping >/dev/null 2>&1; then
echo "salt-minion ready after ${elapsed}s"
exit 0
if pids=$(resolve_daemon_pids); then
# shellcheck disable=SC2086
for pid in $pids; do
if instance_ready "$pid"; then
echo "salt-minion (pid ${pid}) ready after ${elapsed}s"
exit 0
fi
done
fi
sleep 1
elapsed=$((elapsed + 1))
done
echo "salt-minion did not become ready within ${TIMEOUT}s" >&2
mainpid=$(systemctl show -p MainPID --value salt-minion 2>/dev/null)
echo "salt-minion did not become ready within ${TIMEOUT}s (MainPID=${mainpid:-unknown}, candidate pids='${pids:-none}', log_gate=${USE_LOG_GATE}, socket_gate=${USE_SOCKET_GATE}, log_file=${LOG_FILE})" >&2
exit 1