fix(update): startup hint no longer blames an update that did restart the fleet
`hermes` printed "A previous `hermes update` pulled new code but did not restart running gateways" on every launch after an update whose receipt was `failed` for a post-restart crash: `fleet` was empty (the matrix never ran), so the check fell back to the pre-pull `plan.runtimes[].code_sha` against today's checkout and the live gateway, correctly restarted onto the pulled commit, still read as "not restarted" once a manual `git pull` had moved the checkout again. Reproduced on the maintainer's box against the #112604 receipt: gateway on the update's `post_update.sha`, checkout two pulls ahead. The startup hint now holds a receipt whose restart phase completed (`gateway_restart` present, `incomplete` false, no `phase_error`) to the code it actually pulled: every owed gateway serving `post_update.sha` discharges the update. Later drift is `gateway/code_skew.py`'s job, not a warning that misreports the update. The catch-up predicate `hermes update` itself runs keeps its checkout semantics (the fleet must reach HEAD), so the same box still gets its gateway restarted on the next update. `_live_fleet_covers_receipt` compares stamped `code_sha` against the expected sha for `current` AND `stale` rows (both labels are relative to the checkout, not to what the update owed); `unknown`/`down` rows still never cover.
This commit is contained in:
@@ -171,7 +171,7 @@ def _receipt_owed_gateways() -> set[tuple[str, str]] | None:
|
||||
return owed
|
||||
|
||||
|
||||
def _live_fleet_covers_receipt(expected_sha: str | None) -> bool:
|
||||
def _live_fleet_covers_receipt(expected_sha: str | None, *, accept_states: tuple = ("current",)) -> bool:
|
||||
"""Require current successors for every recorded runtime, not just any live row.
|
||||
|
||||
A PID changes on restart; the stable identity is (runtime kind, profile).
|
||||
@@ -187,8 +187,12 @@ def _live_fleet_covers_receipt(expected_sha: str | None) -> bool:
|
||||
if not owed:
|
||||
return False
|
||||
fleet = collect_fleet_versions()
|
||||
# ``current``/``stale`` are labels relative to the checkout. A caller asking about the
|
||||
# code a completed update restarted the fleet onto passes ``accept_states`` with
|
||||
# ``stale`` too: the row's stamped ``code_sha`` is the identity that matters there.
|
||||
# ``unknown``/``down`` rows never cover.
|
||||
if not fleet or any(
|
||||
row.get("state") != "current" or row.get("code_sha") != expected_sha
|
||||
row.get("state") not in accept_states or row.get("code_sha") != expected_sha
|
||||
for row in fleet
|
||||
):
|
||||
return False
|
||||
@@ -265,8 +269,32 @@ def _marker_only_restart_obsolete() -> bool:
|
||||
return True
|
||||
|
||||
|
||||
def _receipt_restart_phase_completed(receipt: dict) -> str | None:
|
||||
"""``post_update.sha`` when the receipt's restart phase ran to completion, else None.
|
||||
|
||||
A receipt can be ``failed`` for reasons that have nothing to do with the fleet (a
|
||||
post-restart step crashed, a notice raised) after every gateway was already brought to
|
||||
the pulled code. Its pre-pull ``plan.runtimes[].code_sha`` then no longer describes an
|
||||
obligation; the update owed the fleet ``post_update.sha`` and that is what the live
|
||||
fleet must be checked against. Any later drift (a manual ``git pull`` moving the
|
||||
checkout past a running gateway) belongs to ``gateway/code_skew.py``, not to a warning
|
||||
that blames an update which did restart the fleet.
|
||||
"""
|
||||
gateway_restart = receipt.get("gateway_restart")
|
||||
if not isinstance(gateway_restart, dict) or not gateway_restart:
|
||||
return None
|
||||
if gateway_restart.get("incomplete") or gateway_restart.get("phase_error"):
|
||||
return None
|
||||
post_sha = (receipt.get("post_update") or {}).get("sha")
|
||||
return str(post_sha) if post_sha else None
|
||||
|
||||
|
||||
def _pending_fleet_restart_needed() -> bool:
|
||||
"""Reconcile old restart obligations against current, identity-matched gateways."""
|
||||
"""Reconcile old restart obligations against current, identity-matched gateways.
|
||||
|
||||
Catch-up semantics (``hermes update`` on a current checkout): the fleet must reach the
|
||||
checkout HEAD, whatever moved it there.
|
||||
"""
|
||||
from hermes_cli.update_cmd import _current_checkout_sha
|
||||
|
||||
# The marker has no runtime inventory and may belong to a newer, killed update
|
||||
@@ -281,6 +309,26 @@ def _pending_fleet_restart_needed() -> bool:
|
||||
return not _live_fleet_covers_receipt(_current_checkout_sha())
|
||||
|
||||
|
||||
def _update_owes_fleet_restart() -> bool:
|
||||
"""Startup-warning semantics: does the LAST UPDATE still owe the fleet a restart?
|
||||
|
||||
Same evidence as :func:`_pending_fleet_restart_needed`, except that a receipt whose
|
||||
restart phase completed is held to the code it pulled, not to today's checkout: the
|
||||
update kept its promise, and a checkout moved later by hand is not its unfinished work.
|
||||
"""
|
||||
with suppress(OSError):
|
||||
if _fleet_restart_pending_marker_path().is_file():
|
||||
return not _marker_only_restart_obsolete()
|
||||
if not _receipt_reports_stale_runtime():
|
||||
return False
|
||||
from hermes_cli.update_cmd import _current_checkout_sha
|
||||
from hermes_cli.update_receipt import read_latest_receipt
|
||||
restarted_to = _receipt_restart_phase_completed(read_latest_receipt() or {})
|
||||
if restarted_to:
|
||||
return not _live_fleet_covers_receipt(restarted_to, accept_states=("current", "stale"))
|
||||
return not _live_fleet_covers_receipt(_current_checkout_sha())
|
||||
|
||||
|
||||
def _warn_pending_fleet_restart(*, startup: bool = False) -> None:
|
||||
"""Print the specific interrupted-update fleet-restart warning."""
|
||||
stream = sys.stderr if startup else sys.stdout
|
||||
@@ -293,7 +341,7 @@ def _warn_pending_fleet_restart(*, startup: bool = False) -> None:
|
||||
def _warn_pending_fleet_restart_on_startup() -> None:
|
||||
"""Cheap CLI-startup hint. Never restarts; never raises."""
|
||||
with suppress(Exception):
|
||||
if _pending_fleet_restart_needed():
|
||||
if _update_owes_fleet_restart():
|
||||
_warn_pending_fleet_restart(startup=True)
|
||||
|
||||
|
||||
|
||||
@@ -765,3 +765,41 @@ def test_startup_warn_kept_when_receipt_owed_gateway_is_down(monkeypatch, capsys
|
||||
assert "did not restart running gateways" in capsys.readouterr().err
|
||||
assert update_cmd._fleet_restart_pending_marker_path().exists()
|
||||
|
||||
|
||||
def test_startup_warn_silent_when_failed_receipt_already_restarted_fleet(monkeypatch, capsys):
|
||||
"""#112604 aftermath: the update pulled ``pulled``, restarted every gateway onto it, then a
|
||||
post-restart step crashed (receipt ``failed``, empty ``fleet`` matrix). Later a manual
|
||||
``git pull`` moved the checkout again. The startup hint must not blame that update for a
|
||||
restart it performed; ``hermes update``'s catch-up still owes the fleet the checkout."""
|
||||
pre, pulled, checkout = "a" * 40, "b" * 40, "c" * 40
|
||||
_patch_marker_sha(monkeypatch, checkout)
|
||||
receipt_dir = get_hermes_home() / "logs" / "update_receipts"
|
||||
receipt_dir.mkdir(parents=True)
|
||||
(receipt_dir / "latest.json").write_text(
|
||||
json.dumps(
|
||||
{
|
||||
"outcome": "failed", "exit_code": 1,
|
||||
"stop_reason": "AttributeError: module 'hermes_cli.main_dashboard' has no attribute 'x'",
|
||||
"pre_update": {"sha": pre}, "post_update": {"sha": pulled},
|
||||
"gateway_restart": {
|
||||
"restarted_services": ["hermes-gateway"], "relaunched_profiles": [],
|
||||
"externally_supervised_profiles": [], "killed_pids": [], "failed_units": [],
|
||||
"incomplete": False, "phase_error": "",
|
||||
},
|
||||
"fleet": [],
|
||||
"plan": {"runtimes": [{"kind": "gateway", "profile": "default", "code_sha": pre, "pid": 1}]},
|
||||
}
|
||||
),
|
||||
encoding="utf-8",
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.update_receipt.collect_fleet_versions",
|
||||
lambda **kwargs: [
|
||||
{"profile": "default", "pid": 42, "code_sha": pulled, "code_version": "0.21.3", "state": "stale"}
|
||||
],
|
||||
)
|
||||
|
||||
update_cmd._warn_pending_fleet_restart_on_startup()
|
||||
|
||||
assert capsys.readouterr().err == ""
|
||||
assert update_cmd_fleet._pending_fleet_restart_needed() is True
|
||||
|
||||
Reference in New Issue
Block a user