fix: hand back a custom-unit gateway even when identify answers systemd/launchd
gateway_declares_external_supervisor treated the control-socket identify answer as final whenever `supervisor` was non-empty and only accepted "external". _detect_supervisor() answers "systemd" as soon as INVOCATION_ID is set (and "launchd" under the XPC name), so a gateway run by a custom, non-canonical systemd unit / launchd agent with `ExecStart=... gateway run --external-supervisor` — the documented contract — was classified non-external and `hermes gateway restart` still took the stop + foreground run_gateway path that stamps the CLI's PID and wedges every respawn (#110637). The update path's argv check on the same gateway already said external-supervisor, so the two restart paths disagreed. Any self-declared supervisor other than "manual" now means hand back (the declaring supervisor owns the respawn); otherwise the argv/state-file marker decides as before. The drain-failure message no longer claims a timeout when SIGUSR1 could not be sent at all. Review finding: socket-first detection classified a custom systemd unit's --external-supervisor gateway as manual and took the wedge path.
This commit is contained in:
@@ -157,7 +157,7 @@ blockers for the hook) and the `gateway.auto_multiplex_migration` opt-out (#1099
|
||||
new HTTP-inbound adapter when it answers the prefix, never extend a list here.
|
||||
`hermes gateway restart` for a gateway Hermes did not install (custom launchd agent / unit running
|
||||
`gateway run --external-supervisor`): `gateway_supervised_restart.py` — the gateway's SELF-declared
|
||||
supervisor (control-socket `identify`, then the argv marker) decides; hand back via SIGUSR1 and wait
|
||||
supervisor (control-socket `identify` answering anything but `manual`, OR the argv marker) decides; hand back via SIGUSR1 and wait
|
||||
for a fresh supervised PID, never stop + foreground `run_gateway` (that stamps the CLI's PID and wedges
|
||||
every KeepAlive respawn, #110637).
|
||||
|
||||
|
||||
@@ -23,9 +23,12 @@ def gateway_declares_external_supervisor(pid: int, home: Path | None = None) ->
|
||||
"""True when the running gateway ``pid`` was launched for an external supervisor.
|
||||
|
||||
The supervisor is SELF-declared by the gateway from its launch context: the control socket
|
||||
``identify`` answer (``supervisor: "external"``), else the ``--external-supervisor`` argv marker
|
||||
read live (same marker ``_prepare_profile_gateway_update_restart`` trusts), else the argv the
|
||||
gateway stamped into ``gateway_state.json`` when psutil cannot read the live command line.
|
||||
``identify`` answer (any ``supervisor`` other than ``"manual"`` — a custom systemd unit or
|
||||
launchd agent sets INVOCATION_ID / the XPC name, so the gateway answers ``systemd``/``launchd``
|
||||
even though ``_installed_service_kind_for`` saw no canonical unit, and the same supervisor owns
|
||||
the respawn), OR the ``--external-supervisor`` argv marker read live (same marker
|
||||
``_prepare_profile_gateway_update_restart`` trusts), else the argv the gateway stamped into
|
||||
``gateway_state.json`` when psutil cannot read the live command line.
|
||||
"""
|
||||
if not pid or pid <= 1:
|
||||
return False
|
||||
@@ -35,8 +38,8 @@ def gateway_declares_external_supervisor(pid: int, home: Path | None = None) ->
|
||||
|
||||
home = home or _get_process_hermes_home()
|
||||
identity = identify_gateway(home) or {}
|
||||
if identity.get("pid") == pid and identity.get("supervisor"):
|
||||
return identity.get("supervisor") == "external"
|
||||
if identity.get("pid") == pid and identity.get("supervisor") not in (None, "", "manual"):
|
||||
return True
|
||||
argv = _capture_gateway_argv(pid)
|
||||
if argv is None:
|
||||
record = read_runtime_status(home / "gateway_state.json") or {}
|
||||
@@ -90,7 +93,7 @@ def restart_externally_supervised_gateway(supervised_pid: int) -> None:
|
||||
return
|
||||
print("⚠ Supervisor did not relaunch the gateway after its graceful exit")
|
||||
else:
|
||||
print(f"⚠ Gateway drain timed out after {wait_budget:.0f}s")
|
||||
print(f"⚠ Gateway did not exit within {wait_budget:.0f}s of SIGUSR1 (or the signal could not be sent)")
|
||||
_print_lines(
|
||||
"",
|
||||
"✗ Not stopping or foreground-running a supervisor-owned gateway.",
|
||||
|
||||
@@ -67,6 +67,18 @@ def test_external_supervisor_gateway_restarts_via_sigusr1_handback(restart_calls
|
||||
)
|
||||
|
||||
|
||||
def test_custom_systemd_unit_gateway_hands_back_despite_socket_saying_systemd(restart_calls, monkeypatch):
|
||||
# A custom (non-canonical) unit running `gateway run --external-supervisor` sets INVOCATION_ID,
|
||||
# so the gateway self-identifies as "systemd", not "external"; that unit still owns the respawn,
|
||||
# so the socket answer must not demote the argv contract back to the stop + foreground wedge.
|
||||
monkeypatch.setattr(
|
||||
"gateway.control_socket.identify_gateway", lambda home, **k: {"pid": 4321, "supervisor": "systemd"}
|
||||
)
|
||||
_run_restart()
|
||||
assert restart_calls["sigusr1"] == (4321, 7.0)
|
||||
assert not restart_calls["stopped"] and not restart_calls["started"]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("sigusr1_returns, replacement", [(True, None), (False, 5555)])
|
||||
def test_handback_failure_never_takes_ownership(restart_calls, sigusr1_returns, replacement):
|
||||
# An unloaded supervisor (clean exit, no replacement) or a drain timeout must fail loudly;
|
||||
|
||||
Reference in New Issue
Block a user