fix(update): classify a Desktop SSH serve as its remote client's, not manual-serve
The serve a remote Desktop spawns over SSH has no local spawner, so the inventory read it as manual-serve: an update filed a manual-restart reminder nobody on this host can discharge, reported the stale process as unaccounted, and abort recovery could try an argv respawn without the client's token file and owner nonce. Classify it as desktop-ssh (using the canonical argv predicate, so rows written before the ledger carried isolated are covered too) and treat it like the local Desktop's own serve: skipped by the restart phase, deferred to its client, never owed by abort recovery. A hand-started serve --isolated stays manual-serve.
This commit is contained in:
@@ -248,7 +248,9 @@ def _owed_stale_serve_rows(rows) -> list[dict]:
|
||||
:func:`_warn_stale_serve_runtimes` and recorded in the receipt. See #111494. (The
|
||||
fleet-restart-pending marker draws the same boundary for its own inventory, so a supervisor-owned
|
||||
serve row no longer keeps that warning armed either.)"""
|
||||
return [row for row in (rows or []) if row.get("supervisor") != "desktop"]
|
||||
from hermes_cli.update_inventory import CLIENT_OWNED_SERVE_SUPERVISORS
|
||||
|
||||
return [row for row in (rows or []) if row.get("supervisor") not in CLIENT_OWNED_SERVE_SUPERVISORS]
|
||||
|
||||
|
||||
def _abort_recovery_is_complete(
|
||||
|
||||
@@ -242,11 +242,13 @@ def _receipt_reports_stale_runtime(receipt: dict, expected_sha: str | None = Non
|
||||
)
|
||||
|
||||
|
||||
_SUPERVISED_SERVE_BACKENDS = frozenset({"manual-serve", "desktop", "systemd", "launchd", "windows-service", "service"})
|
||||
_SUPERVISED_SERVE_BACKENDS = frozenset(
|
||||
{"manual-serve", "desktop", "desktop-ssh", "systemd", "launchd", "windows-service", "service"}
|
||||
)
|
||||
# Backends whose supervisor restarts the process without any updater bookkeeping. ``manual-serve``
|
||||
# is excluded: it owes a durable handoff (``defer_manual_serve``) before it stops counting.
|
||||
# ``systemd``/``windows-service``/``service`` mirror ``_SUPERVISED_SERVE_BACKENDS`` for parity only —
|
||||
# the inventory writer classifies a serve/dashboard row as exactly launchd, desktop or manual-serve
|
||||
# the inventory writer classifies a serve/dashboard row as exactly launchd, desktop, desktop-ssh or manual-serve
|
||||
# (``update_inventory._collect_ledger_runtimes``); those three are set for gateway rows alone.
|
||||
_SUPERVISOR_OWNED_SERVE_BACKENDS = _SUPERVISED_SERVE_BACKENDS - {"manual-serve"}
|
||||
|
||||
@@ -1101,7 +1103,9 @@ def _gateway_recovery_partition(plan, *, skip_profiles: set[str] | None = None)
|
||||
continue
|
||||
reason = _MANUAL_GATEWAY_SKIP_REASON
|
||||
elif kind in ("serve", "dashboard"):
|
||||
if supervisor == "desktop":
|
||||
from hermes_cli.update_inventory import CLIENT_OWNED_SERVE_SUPERVISORS
|
||||
|
||||
if supervisor in CLIENT_OWNED_SERVE_SUPERVISORS:
|
||||
reason = _DESKTOP_SERVE_SKIP_REASON
|
||||
elif supervisor == "launchd":
|
||||
reason = _LAUNCHD_SERVE_SKIP_REASON
|
||||
|
||||
@@ -85,6 +85,7 @@ def _detect_supervisor_for_pid(pid: int, service_pids: set, windows_service_pids
|
||||
_RESTART_MECHANISMS = {
|
||||
"systemd": "systemd", "launchd": "launchd", "desktop": "desktop",
|
||||
"windows-service": "windows-service", "manual-serve": "respawn-argv",
|
||||
"desktop-ssh": "desktop-ssh",
|
||||
}
|
||||
|
||||
_MECHANISM_DESCRIPTIONS = {
|
||||
@@ -93,9 +94,14 @@ _MECHANISM_DESCRIPTIONS = {
|
||||
"desktop": "Desktop app respawns its serve backend",
|
||||
"windows-service": "sc.exe stop before venv mutation, sc.exe start after update",
|
||||
"respawn-argv": "stop before code swap, relaunch with recorded launch args",
|
||||
"desktop-ssh": "the remote Desktop that spawned it over SSH respawns it when it reconnects",
|
||||
}
|
||||
|
||||
_SERVE_KINDS = ("serve", "dashboard")
|
||||
# Serve backends a Desktop client owns and recycles: this app's own pool child (``desktop``) or one
|
||||
# another machine's Desktop spawned here over SSH (``desktop-ssh``). The updater never restarts
|
||||
# either; stopping one out from under its client only makes the client respawn it.
|
||||
CLIENT_OWNED_SERVE_SUPERVISORS = frozenset({"desktop", "desktop-ssh"})
|
||||
|
||||
|
||||
def _restart_mechanism(supervisor: str, profile: str) -> str:
|
||||
@@ -250,6 +256,18 @@ def _launchd_owner_for_ledger_entry(entry: dict, pid: int, jobs: list) -> "tuple
|
||||
return None
|
||||
|
||||
|
||||
def _is_desktop_ssh_ledger_entry(entry: dict) -> bool:
|
||||
"""Is this row the backend a (possibly remote) Desktop spawned over SSH? The canonical argv
|
||||
predicate also classifies rows written before the ledger carried ``isolated``, which is exactly
|
||||
the pre-update serve the first update after this change inventories."""
|
||||
from hermes_cli._startup_fast import is_desktop_ssh_backend_argv
|
||||
|
||||
try:
|
||||
return is_desktop_ssh_backend_argv(shlex.split(str(entry.get("argv") or "")))
|
||||
except ValueError:
|
||||
return False
|
||||
|
||||
|
||||
def _collect_ledger_runtimes(plan: UpdatePlan, seen: set[int]) -> None:
|
||||
"""Serve/dashboard backends from the spawn ledger — runtimes the gateway collectors can never see
|
||||
(a manual `hermes serve --host <ip>` for a remote Desktop, a long-lived `hermes dashboard`).
|
||||
@@ -275,6 +293,10 @@ def _collect_ledger_runtimes(plan: UpdatePlan, seen: set[int]) -> None:
|
||||
job = _launchd_owner_for_ledger_entry(entry, pid, launchd_jobs) if launchd_jobs else None
|
||||
if job:
|
||||
supervisor, detail["launchd_domain"], detail["launchd_label"] = "launchd", job[0], job[1]
|
||||
elif _is_desktop_ssh_ledger_entry(entry):
|
||||
# No local spawner, so the probe below would read manual-serve and file a reminder
|
||||
# nobody here can discharge; its token file and owner nonce belong to the client.
|
||||
supervisor = "desktop-ssh"
|
||||
else:
|
||||
supervisor = "desktop" if spawner_is_dead(entry) is False else "manual-serve"
|
||||
plan.runtimes.append(_runtime(
|
||||
@@ -406,10 +428,10 @@ def match_runtime_outcomes(
|
||||
# Incarnation-verified: the pre-update process is gone (replaced by its unit / the
|
||||
# dashboard cleanup respawn / the Desktop app).
|
||||
return "restarted"
|
||||
if r.supervisor == "desktop":
|
||||
if r.supervisor in CLIENT_OWNED_SERVE_SUPERVISORS:
|
||||
if stale_serves is not None:
|
||||
# Still alive on pre-update code, but the Desktop app owns it and the restart phase
|
||||
# must not kill it (_DESKTOP_SERVE_SKIP_REASON); only the app can pick up the new code.
|
||||
# Still alive on pre-update code, but a Desktop client owns it and the restart phase
|
||||
# must not kill it (_DESKTOP_SERVE_SKIP_REASON); only that client can pick up the new code.
|
||||
return "deferred"
|
||||
return "unaccounted"
|
||||
if stale_serves is not None:
|
||||
@@ -453,7 +475,9 @@ def report_unaccounted_runtimes(outcomes: list[dict[str, Any]]) -> bool:
|
||||
print()
|
||||
print(" ℹ Left to the Desktop app (still on pre-update code until it is relaunched):")
|
||||
for o in deferred:
|
||||
print(f" • {o['kind']} [{o['profile']}] pid {o['pid']} — relaunch the Desktop app to pick up the update")
|
||||
action = ("owned by a Desktop connected over SSH; it picks up the update when that Desktop reconnects"
|
||||
if o.get("mechanism") == "desktop-ssh" else "relaunch the Desktop app to pick up the update")
|
||||
print(f" • {o['kind']} [{o['profile']}] pid {o['pid']} — {action}")
|
||||
missed = [o for o in outcomes if o.get("outcome") == "unaccounted"]
|
||||
if not missed:
|
||||
return False
|
||||
|
||||
@@ -9,6 +9,7 @@ update inventory and the dashboard process scan.
|
||||
from __future__ import annotations
|
||||
|
||||
import sys
|
||||
from dataclasses import asdict
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import patch
|
||||
|
||||
@@ -109,6 +110,60 @@ def test_inventory_classifies_desktop_owned_serve(monkeypatch):
|
||||
assert serves and serves[0].supervisor == "desktop"
|
||||
assert serves[0].restart_via == "desktop"
|
||||
|
||||
|
||||
_SSH_ARGV = ("hermes serve --isolated --host 127.0.0.1 --port 0 "
|
||||
"--ssh-session-token-file /h/.hermes/desktop-ssh/a/b.token --ssh-owner-nonce 0123456789abcdef")
|
||||
|
||||
|
||||
def test_inventory_classifies_remote_desktop_ssh_serve_as_its_clients(monkeypatch):
|
||||
"""A serve another machine's Desktop spawned over SSH has no local spawner, so the spawner
|
||||
probe alone reads ``manual-serve``: the update then files a manual-restart reminder nobody on
|
||||
this host can discharge, and the recovery pass may try an argv respawn of a process whose
|
||||
token file and owner nonce only its remote client holds. The remote client owns its restart."""
|
||||
from hermes_cli.update_serve_obligations import defer_manual_serve
|
||||
|
||||
entry = _ledger_entry(argv=_SSH_ARGV, host="127.0.0.1", port=57474, isolated=True)
|
||||
fake_pi = SimpleNamespace(ledger_entries=lambda **k: [entry], spawner_is_dead=lambda e: None)
|
||||
monkeypatch.setitem(sys.modules, "hermes_cli.process_identity", fake_pi)
|
||||
plan = update_inventory.collect_runtime_inventory()
|
||||
row = next(r for r in plan.runtimes if r.kind == "serve")
|
||||
assert row.supervisor not in ("manual-serve", "desktop")
|
||||
assert row.restart_via != "respawn-argv"
|
||||
monkeypatch.undo() # real process_identity: no durable manual-restart reminder may be filed
|
||||
assert defer_manual_serve(asdict(row)) is False
|
||||
|
||||
|
||||
def test_hand_started_isolated_serve_stays_manual(monkeypatch):
|
||||
"""``--isolated`` alone is an opt-out of the host singleton, not remote ownership: a user's own
|
||||
``hermes serve --isolated`` keeps its manual-serve relaunch."""
|
||||
entry = _ledger_entry(argv="hermes serve --isolated --host 127.0.0.1 --port 9119", isolated=True)
|
||||
fake_pi = SimpleNamespace(ledger_entries=lambda **k: [entry], spawner_is_dead=lambda e: None)
|
||||
monkeypatch.setitem(sys.modules, "hermes_cli.process_identity", fake_pi)
|
||||
row = next(r for r in update_inventory.collect_runtime_inventory().runtimes if r.kind == "serve")
|
||||
assert row.supervisor == "manual-serve"
|
||||
|
||||
|
||||
def test_stale_remote_desktop_ssh_serve_is_deferred_to_its_client_not_unaccounted(monkeypatch):
|
||||
"""Still on pre-update code after the update, the SSH serve is its remote client's to recycle:
|
||||
reported as deferred (not an unaccounted failure that fails the update), and never owed by the
|
||||
abort-recovery pass."""
|
||||
from hermes_cli.update_abort_recovery import _owed_stale_serve_rows
|
||||
|
||||
entry = _ledger_entry(argv=_SSH_ARGV, host="127.0.0.1", port=57474, isolated=True)
|
||||
fake_pi = SimpleNamespace(ledger_entries=lambda **k: [entry], spawner_is_dead=lambda e: None)
|
||||
monkeypatch.setitem(sys.modules, "hermes_cli.process_identity", fake_pi)
|
||||
plan = update_inventory.collect_runtime_inventory()
|
||||
outcomes = update_inventory.match_runtime_outcomes(
|
||||
plan, restarted_services=[], relaunched_profiles=[], externally_supervised_profiles=[],
|
||||
killed_pids=set(), failed_units=[], stale_serve_pids={entry["pid"]},
|
||||
)
|
||||
serve = next(o for o in outcomes if o["kind"] == "serve")
|
||||
assert serve["outcome"] == "deferred"
|
||||
assert update_inventory.report_unaccounted_runtimes(outcomes) is False
|
||||
row = next(r for r in plan.runtimes if r.kind == "serve")
|
||||
assert _owed_stale_serve_rows([{"supervisor": row.supervisor}]) == []
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# dashboard_procs: ledger augmentation of the scan (#81564 half)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user