fix(update): completion tail skips the fleet restart it does not owe
Every update route now finishes through update_completion._complete_selected,
which restarted the whole fleet unconditionally -- including the "Already up to
date" route that main sent through the pending-restart catch-up. Net effect:
each cron tick and each profile's `hermes update` drained and re-killed the one
multiplexed gateway.
Port the catch-up path's two live guards into the completion tail:
- host_restart_already_completed(checkout sha): a sibling profile attaches to
the restart this host already stamped (#95294); the restart phase now stamps
it via mark_host_restart_completed.
- every planned runtime AND every live fleet row current at the checkout sha
(#117051, d6b0d37ece). Both are required: the live matrix lists gateways
only, so a planned serve still on pre-update code keeps the restart.
The already-current route also arms the host obligation with the checkout sha;
an SHA-less arm replaced the standing record and wiped the restarted proof.
This commit is contained in:
@@ -49,7 +49,7 @@ from hermes_cli.update_cmd_fleet import ( # noqa: F401
|
||||
_FLEET_RESTART_PENDING_NAME, _FRESH_RESTART_SUPERVISORS, _GatewayRestartOutcome,
|
||||
_clear_fleet_restart_pending_marker,
|
||||
_current_checkout_sha, _drain_or_signal_gateway_for_update, _fleet_probe_expected_runtimes,
|
||||
_fleet_restart_pending_marker_path, _for_each_systemd_gateway_unit,
|
||||
_fleet_restart_pending_marker_path, _fleet_restart_skip_reason, _for_each_systemd_gateway_unit,
|
||||
_gateway_recovery_partition, _gateway_service_matches_profile, _pending_fleet_restart_needed,
|
||||
_receipt_looks_unfinished, _receipt_reports_stale_runtime, _resolve_manage_cmd,
|
||||
_restart_gateway_fleet_after_update, _restart_launchd_gateway_after_update,
|
||||
@@ -1373,6 +1373,9 @@ def _finish_already_up_to_date(
|
||||
_git_run(git_cmd, ["checkout", current_branch])
|
||||
|
||||
if completion_request is not None:
|
||||
# Same code, same host obligation: an SHA-less arm would REPLACE the standing record
|
||||
# (and its restarted proof), so a sibling profile's no-op update re-kills the multiplexer.
|
||||
completion_request["expected_sha"] = _capture_head_sha(git_cmd, _m().PROJECT_ROOT) or ""
|
||||
completion_request["completion_message"] = (
|
||||
"✓ Already up to date!" if _plan.upstream_checked
|
||||
else "✓ Up to date with your fork (official repo not checked).")
|
||||
|
||||
@@ -567,6 +567,30 @@ def _restart_identity_sha() -> str:
|
||||
return ""
|
||||
|
||||
|
||||
def _fleet_restart_skip_reason(plan) -> str | None:
|
||||
"""Why the completion tail may leave the fleet alone, or ``None`` when a restart is owed.
|
||||
|
||||
Every route (pulled, already-current, ZIP) now finishes through the same completion
|
||||
tail, so the guards the old catch-up path carried live here: one host runs ONE
|
||||
multiplexing gateway, so a second profile's ``hermes update`` attaches to the restart the
|
||||
first one already stamped (#95294), and a fleet already serving the checkout code (a
|
||||
no-op update, a manual ``hermes gateway restart`` seconds ago) is not re-killed (#117051).
|
||||
|
||||
The second guard needs BOTH the pre-update plan and the live probe: the live matrix only
|
||||
lists gateways, so a planned ``serve`` still on pre-update code (or any runtime without a
|
||||
stamped identity) keeps the restart — the reconciliation there is what surfaces it.
|
||||
"""
|
||||
from hermes_cli.update_host_obligation import host_restart_already_completed
|
||||
checkout_sha = _restart_identity_sha()
|
||||
if host_restart_already_completed(checkout_sha):
|
||||
return "this host's gateway was already restarted for this update"
|
||||
if (checkout_sha and plan is not None and plan.runtimes
|
||||
and all(str(runtime.code_sha) == checkout_sha for runtime in plan.runtimes)
|
||||
and _live_fleet_current_rows() is not None):
|
||||
return "every running gateway already serves the checkout code"
|
||||
return None
|
||||
|
||||
|
||||
def _run_pending_fleet_restart() -> bool:
|
||||
"""Historical retry hook; new retries use the ordinary completion owner."""
|
||||
from hermes_cli._old_updater import stop_for_relaunch
|
||||
@@ -1550,6 +1574,11 @@ def _restart_gateway_fleet_after_update(_pre_update_plan, gateway_mode: bool):
|
||||
)
|
||||
|
||||
out.restarted_scoped_units = set(restarted_scoped_units)
|
||||
if not out.incomplete:
|
||||
# Stamp the HOST obligation so every other profile's CLI knows this update's restart
|
||||
# already happened; without it each profile re-kills the one shared multiplexer.
|
||||
from hermes_cli.update_host_obligation import mark_host_restart_completed
|
||||
mark_host_restart_completed(_restart_identity_sha())
|
||||
return out
|
||||
|
||||
|
||||
|
||||
@@ -189,6 +189,20 @@ def _complete_selected(request: dict) -> None:
|
||||
if not complete:
|
||||
raise SystemExit(1)
|
||||
return
|
||||
skip = update_cmd._fleet_restart_skip_reason(plan)
|
||||
if skip:
|
||||
from hermes_cli.update_receipt import record_skip
|
||||
|
||||
record_skip("gateway_restart", skip)
|
||||
print(f" ✓ Gateway restart skipped: {skip}.")
|
||||
# Discharges the obligation this run armed when the live fleet vouches for it; a
|
||||
# fleet still owing the restart fails closed exactly like a stale matrix would.
|
||||
if update_cmd._pending_fleet_restart_needed():
|
||||
print(" ⚠ Gateways are still off the checkout code. Recover with: hermes gateway restart")
|
||||
raise SystemExit(1)
|
||||
if not complete:
|
||||
raise SystemExit(1)
|
||||
return
|
||||
restart = update_cmd._restart_gateway_fleet_after_update(plan, request["gateway_mode"])
|
||||
update_cmd._resume_windows_gateways_and_merge_outcome(restart, request["windows_resume"], request["gateway_mode"])
|
||||
update_cmd._verify_fleet_after_update(
|
||||
|
||||
@@ -98,6 +98,7 @@ def transition(tmp_path):
|
||||
"_sweep_bytecode_after_update = lambda branch: event('bytecode')\n"
|
||||
"_write_fleet_restart_pending_marker = lambda **kw: event('pending')\n"
|
||||
"_write_gateway_update_exit_code = lambda ok: event('exit_marker', ok=ok)\n"
|
||||
"_fleet_restart_skip_reason = lambda plan: None\n"
|
||||
"def _restart_gateway_fleet_after_update(plan, gateway_mode):\n"
|
||||
" event('restart', profiles=[r.profile for r in plan.runtimes])\n"
|
||||
" return object()\n"
|
||||
|
||||
@@ -929,3 +929,90 @@ def test_pending_fleet_restart_cleared_instead_of_exit_1(monkeypatch, tmp_path):
|
||||
|
||||
assert seen["ran"] is False
|
||||
assert not marker.exists()
|
||||
|
||||
|
||||
# ── Completion tail guards (#117051 / #95294): a no-op update must not re-kill the fleet ──
|
||||
|
||||
|
||||
def _current_row(sha):
|
||||
return [{"profile": "default", "pid": 42, "code_sha": sha, "code_version": "0.21.0", "state": "current"}]
|
||||
|
||||
|
||||
def _plan_with_current_gateway(monkeypatch, sha):
|
||||
"""Pre-update inventory: one gateway already stamped with the checkout code."""
|
||||
import hermes_cli.update_inventory as ui
|
||||
|
||||
plan = ui.UpdatePlan()
|
||||
plan.runtimes = [ui.RuntimeRecord(kind="gateway", profile="default", pid=42, supervisor="systemd",
|
||||
code_sha=sha, restart_via=ui._restart_mechanism("systemd", "default"))]
|
||||
monkeypatch.setattr(ui, "collect_runtime_inventory", lambda: plan)
|
||||
|
||||
|
||||
def _spy_fleet_restart(monkeypatch):
|
||||
"""Record restart-phase entries; the phase itself (drain waits, unit restarts) is not under test."""
|
||||
calls = []
|
||||
|
||||
def _spy(plan, gateway_mode):
|
||||
calls.append(plan)
|
||||
raise SystemExit(3)
|
||||
|
||||
monkeypatch.setattr(update_cmd, "_restart_gateway_fleet_after_update", _spy)
|
||||
return calls
|
||||
|
||||
|
||||
def test_up_to_date_update_leaves_current_fleet_alone(monkeypatch, tmp_path, capsys):
|
||||
"""Every live gateway already serves the checkout code: `hermes update` (cron, a second
|
||||
profile) must not drain and restart the shared multiplexer again, and must still discharge
|
||||
the obligation it armed and finish clean."""
|
||||
args = _update_args()
|
||||
_patch_update_deps(monkeypatch, tmp_path, _make_up_to_date_side_effect("abc123"))
|
||||
_patch_marker_sha(monkeypatch, "abc123")
|
||||
_plan_with_current_gateway(monkeypatch, "abc123")
|
||||
monkeypatch.setattr("hermes_cli.update_receipt.collect_fleet_versions", lambda **k: _current_row("abc123"))
|
||||
restarts = _spy_fleet_restart(monkeypatch)
|
||||
|
||||
hermes_main.cmd_update(args)
|
||||
|
||||
assert restarts == []
|
||||
assert not update_cmd_fleet._fleet_restart_obligation_armed()
|
||||
assert "Gateway restart skipped" in capsys.readouterr().out
|
||||
from hermes_cli.update_receipt import read_latest_receipt
|
||||
receipt = read_latest_receipt()
|
||||
assert receipt["outcome"] == "success"
|
||||
assert any(skip.get("name") == "gateway_restart" for skip in receipt.get("skips", []))
|
||||
|
||||
|
||||
def test_up_to_date_update_still_restarts_a_stale_gateway(monkeypatch, tmp_path):
|
||||
"""The guard is evidence-based: one gateway still on pre-update code means the restart runs."""
|
||||
args = _update_args()
|
||||
_patch_update_deps(monkeypatch, tmp_path, _make_up_to_date_side_effect("abc123"))
|
||||
_patch_marker_sha(monkeypatch, "abc123")
|
||||
_plan_with_current_gateway(monkeypatch, "abc123")
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.update_receipt.collect_fleet_versions",
|
||||
lambda **k: _current_row("abc123") + [
|
||||
{"profile": "work", "pid": 43, "code_sha": "0" * 40, "code_version": "0.20.0", "state": "stale"}],
|
||||
)
|
||||
restarts = _spy_fleet_restart(monkeypatch)
|
||||
|
||||
with pytest.raises(SystemExit) as error:
|
||||
hermes_main.cmd_update(args)
|
||||
|
||||
assert error.value.code == 3
|
||||
assert len(restarts) == 1
|
||||
|
||||
|
||||
def test_second_profile_attaches_to_completed_host_restart(monkeypatch):
|
||||
"""One host process serves every profile: once its restart is stamped for this checkout,
|
||||
another profile's completion skips the restart instead of killing the multiplexer again."""
|
||||
_patch_marker_sha(monkeypatch, "abc123")
|
||||
monkeypatch.setattr("hermes_cli.update_receipt.collect_fleet_versions", lambda **k: [])
|
||||
update_cmd._write_fleet_restart_pending_marker(expected_sha="abc123")
|
||||
assert update_cmd_fleet._fleet_restart_skip_reason(None) is None
|
||||
|
||||
host_obligation.mark_host_restart_completed("abc123")
|
||||
assert update_cmd_fleet._fleet_restart_skip_reason(None)
|
||||
|
||||
# A stamp for other code proves nothing about this checkout.
|
||||
host_obligation.mark_host_restart_completed("def456")
|
||||
assert update_cmd_fleet._fleet_restart_skip_reason(None) is None
|
||||
|
||||
Reference in New Issue
Block a user