fix(install): fail closed when a stopped gateway leaves an empty survivor probe
Review follow-up (#78574): the aborted-restart handler only flagged the fleet stale when the post-failure survivor probe was None or non-empty. A positive empty probe was treated as proof-of-safety — but `[]` is only safe when nothing was running before the phase. If a gateway was discovered, stopped (SIGTERM/drain), and its replacement never came back, the probe is empty at exactly that unsafe moment and the update reported success — the fail-open contract this fix exists to close. Snapshot the pre-restart gateway PIDs before any stop/drain and route the handler decision through a pure _restart_phase_failure_is_incomplete() helper that fails closed on an empty survivor set whenever a gateway existed pre-restart (or the pre-state could not be read). Add decision-level regression tests covering the stopped-without-replacement gap, unknown pre-state, and the truly-no-gateway positive control.
This commit is contained in:
@@ -5234,6 +5234,7 @@ from hermes_cli.update_cmd import ( # noqa: F401
|
||||
_reload_updated_runtime_modules,
|
||||
_resolve_pre_update_backup_mode,
|
||||
_resolve_stash_selector,
|
||||
_restart_phase_failure_is_incomplete,
|
||||
_restore_stashed_changes,
|
||||
_resume_windows_gateways_after_update,
|
||||
_run_logged_subprocess,
|
||||
|
||||
@@ -5184,6 +5184,13 @@ def _cmd_update_impl(args, gateway_mode: bool):
|
||||
pass
|
||||
|
||||
gateway_fleet_restart_incomplete = False
|
||||
# Snapshot of gateways running before we touch anything. Stays empty
|
||||
# until we successfully import the probe and are about to stop/drain —
|
||||
# so an exception raised before we touch any gateway keeps this empty
|
||||
# (nothing to fail closed on), while a failure after we have stopped a
|
||||
# discovered gateway lets the handler fail closed on an empty survivor
|
||||
# probe rather than reporting a clean update (#78574).
|
||||
_pre_restart_gateway_pids: list | None = []
|
||||
|
||||
# Auto-restart ALL gateways after update.
|
||||
# The code update (git pull) is shared across all profiles, so every
|
||||
@@ -5363,6 +5370,17 @@ def _cmd_update_impl(args, gateway_mode: bool):
|
||||
relaunched_profiles = []
|
||||
externally_supervised_profiles = []
|
||||
|
||||
# Record which gateways are running before any stop/drain, so a
|
||||
# later failure that leaves the survivor probe empty can still be
|
||||
# recognised as "a running gateway was stopped and did not come
|
||||
# back" rather than "nothing was running" (#78574). Best-effort:
|
||||
# if the probe itself raises, leave the snapshot as-is (the
|
||||
# survivor probe's own None result already fails closed).
|
||||
try:
|
||||
_pre_restart_gateway_pids = list(find_gateway_pids(all_profiles=True))
|
||||
except Exception:
|
||||
_pre_restart_gateway_pids = None
|
||||
|
||||
# --- Systemd services (Linux) ---
|
||||
# Discover all hermes-gateway* units (default + profiles)
|
||||
if supports_systemd_services():
|
||||
@@ -5849,8 +5867,16 @@ def _cmd_update_impl(args, gateway_mode: bool):
|
||||
# output the user relies on never printed. Don't let that pass for
|
||||
# a clean update: surface it and treat the fleet as stale unless we
|
||||
# can positively prove no gateway is running (#78574).
|
||||
#
|
||||
# A positive-empty ``_surviving`` is only proof-of-safety when
|
||||
# nothing was running before we touched anything. If a gateway was
|
||||
# discovered pre-restart and none survive now, it was stopped and
|
||||
# its replacement was never verified — the same fail-open contract
|
||||
# this fix closes — so we must still fail closed on ``[]``.
|
||||
_surviving = _surviving_gateway_pids_after_failed_restart()
|
||||
if _surviving is None or _surviving:
|
||||
if _restart_phase_failure_is_incomplete(
|
||||
_surviving, _pre_restart_gateway_pids
|
||||
):
|
||||
gateway_fleet_restart_incomplete = True
|
||||
_warn_gateway_restart_phase_aborted(e, _surviving)
|
||||
if gateway_mode:
|
||||
@@ -5920,6 +5946,27 @@ def _cmd_update_impl(args, gateway_mode: bool):
|
||||
|
||||
# --- Hoisted from the body of _cmd_update_impl (self-contained, no closure state) ---
|
||||
|
||||
def _restart_phase_failure_is_incomplete(surviving, pre_restart_pids) -> bool:
|
||||
"""Whether an escaped gateway-restart-phase exception must fail the update.
|
||||
|
||||
Fail closed unless we can positively prove the fleet is safe:
|
||||
|
||||
* ``surviving is None`` — the survivor probe could not determine state
|
||||
(typically the freshly-pulled ``hermes_cli.gateway`` no longer imports,
|
||||
one of the ways the phase aborts). Assume stale.
|
||||
* ``surviving`` non-empty — a gateway is still running pre-update code.
|
||||
* ``surviving == []`` — nothing is running now. That is proof-of-safety
|
||||
ONLY when nothing was running before we touched anything. If a gateway
|
||||
was discovered pre-restart (``pre_restart_pids`` non-empty, or ``None``
|
||||
meaning the pre-state could not be read), it was stopped without a
|
||||
verified replacement, so we still fail closed (#78574).
|
||||
"""
|
||||
if surviving is None or surviving:
|
||||
return True
|
||||
# surviving == []: safe only if we know nothing was running beforehand.
|
||||
return pre_restart_pids is None or bool(pre_restart_pids)
|
||||
|
||||
|
||||
def _print_items(items, label, key, fallback_key=None):
|
||||
if not items:
|
||||
return
|
||||
|
||||
@@ -16,6 +16,7 @@ import sys
|
||||
import types
|
||||
|
||||
from hermes_cli.main import (
|
||||
_restart_phase_failure_is_incomplete,
|
||||
_surviving_gateway_pids_after_failed_restart,
|
||||
_warn_gateway_restart_phase_aborted,
|
||||
)
|
||||
@@ -51,6 +52,37 @@ class TestSurvivingGatewayProbe:
|
||||
assert _surviving_gateway_pids_after_failed_restart() is None
|
||||
|
||||
|
||||
class TestRestartPhaseFailureIsIncomplete:
|
||||
"""The fail-closed decision behind the survivor probe.
|
||||
|
||||
An empty ``surviving`` probe is only proof-of-safety when nothing was
|
||||
running before the phase touched anything. A gateway that was discovered
|
||||
pre-restart, stopped, and never verified back up leaves the probe empty at
|
||||
exactly the unsafe moment — the fail-open contract #78574 exists to close.
|
||||
"""
|
||||
|
||||
def test_stale_when_a_gateway_still_survives(self):
|
||||
assert _restart_phase_failure_is_incomplete([4321], [4321]) is True
|
||||
|
||||
def test_stale_when_survivor_probe_is_undeterminable(self):
|
||||
assert _restart_phase_failure_is_incomplete(None, []) is True
|
||||
|
||||
def test_stale_when_preexisting_gateway_stopped_without_replacement(self):
|
||||
# The gap egilewski flagged: a gateway was running, we stopped it, and
|
||||
# the post-failure probe is empty because the replacement never came
|
||||
# back. `[]` here means "gone", not "safe".
|
||||
assert _restart_phase_failure_is_incomplete([], [4321]) is True
|
||||
|
||||
def test_stale_when_pre_restart_state_could_not_be_read(self):
|
||||
# Unknown pre-state (probe raised before we recorded it) also fails
|
||||
# closed on an empty survivor set — we cannot prove nothing was running.
|
||||
assert _restart_phase_failure_is_incomplete([], None) is True
|
||||
|
||||
def test_clean_only_when_nothing_ran_before_and_none_survive(self):
|
||||
# Positive control: truly no gateway anywhere, before or after.
|
||||
assert _restart_phase_failure_is_incomplete([], []) is False
|
||||
|
||||
|
||||
class TestAbortedRestartWarning:
|
||||
def test_warns_with_recovery_command_and_cause(self, capsys):
|
||||
_warn_gateway_restart_phase_aborted(
|
||||
|
||||
Reference in New Issue
Block a user