fix(gateway): exit 0 on systemctl stop instead of exit 1 (failed unit) - #41642
liuhao1024 wants to merge 1 commit into
Conversation
When the gateway runs under systemd and receives SIGTERM (e.g. from `systemctl stop`), it exits with code 1, leaving the unit in 'failed' state. This requires `systemctl reset-failed` before a clean restart and pollutes health monitoring. Root cause: the signal handler treats any SIGTERM without a planned-stop marker as an unexpected kill, but `systemctl stop` is a deliberate operator action. Since the installed unit uses `Restart=always`, exit code doesn't affect restart behavior — a non-zero exit only creates the spurious 'failed' state. Fix: detect systemd-managed SIGTERM (via INVOCATION_ID / ppid==1) and treat it as a planned stop → exit 0 → unit goes 'inactive'. Closes NousResearch#41631
|
Thanks for flagging @alt-glitch. I've left a comparison on #41639 — this PR (#41642) uses the existing |
Cherry-pick of open upstream PR NousResearch#41642 (fixes NousResearch#41631). Railway's container manager sends SIGTERM on every redeploy; without this, the gateway exits 1 and the supervisor treats a planned stop as a crash.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for addressing the real failed-unit symptom.
Problems
gateway/run.py:19911treatsunder_systemdas evidence that systemd sent the signal. It only establishes the process environment: an externalkill -TERMagainst a systemd-managed gateway takes the same branch and is incorrectly classified as planned. Current main intentionally treats only SIGINT or a consumed planned-stop marker as planned (gateway/run.py:20611-20623), preserving the unexpected-signal path (gateway/run.py:20652-20660,20920-20925).tests/gateway/test_systemd_stop_exit_code.py:71-76reimplements the condition instead of exercisingshutdown_signal_handlerand its exit result.
Suggested changes
- Rework this around the existing marker mechanism: generate a non-recursive systemd
ExecStophelper that writes the planned-stop marker for$MAINPIDbefore systemd sends SIGTERM. This is the direction documented in #42517 and implemented by open PR #42555. - Test the real handler path for both marked systemctl stops and unmarked external SIGTERM.
Automated hermes-sweeper review.
| ) | ||
| elif ( | ||
| _shutdown_ctx | ||
| and _shutdown_ctx.get("under_systemd") |
There was a problem hiding this comment.
under_systemd says only that this process was launched by systemd; it does not identify the SIGTERM sender. An external kill -TERM of this process matches this branch and is then treated as a clean stop. Preserve the existing marker-based distinction instead, ideally by writing the marker from the generated unit's ExecStop path.
| monkeypatch.setenv("INVOCATION_ID", "test-invocation") | ||
| ctx = snapshot_shutdown_context(signal.SIGTERM) | ||
|
|
||
| # This is the exact condition used in gateway/run.py |
There was a problem hiding this comment.
This duplicates the proposed predicate rather than exercising shutdown_signal_handler, so it cannot verify the real flag or exit-code behavior. Please test the handler path and include an unmarked external SIGTERM while the process is systemd-managed.
Summary
When the gateway runs under systemd and receives SIGTERM (e.g. from
systemctl stop), it exits with code 1, leaving the unit in"failed"state. This requiressystemctl reset-failedbefore a clean restart and pollutes any health monitoring that reads unit state.Root Cause
The signal handler treats any SIGTERM without a planned-stop marker as an unexpected kill. But
systemctl stopis a deliberate operator action that sends SIGTERM without writing a marker first.The exit-1 rationale is self-defeating: the installed unit uses
Restart=always, under which exit 0 is also restarted. So a non-zero exit buys nothing for revival — it only converts a clean stop into a"failed"unit.Fix
In
gateway/run.py, the signal handler now checks if the gateway is running under systemd (viaINVOCATION_IDenv var orppid == 1) and the received signal is SIGTERM. If so, it treats it as a planned stop → exit 0 → unit goes"inactive"instead of"failed".This preserves the exit-1 behavior for:
Testing
tests/gateway/test_systemd_stop_exit_code.pyunder_systemddetection with/withoutINVOCATION_ID, signal discrimination (SIGTERM vs SIGINT), and the decision-logic conditiontest_clean_shutdown_marker.pypasses (no regression)Reproduce → Expected Behavior
Before:
After:
Closes #41631