Skip to content

fix(gateway): restart via service in containers - #25263

Closed
he-yufeng wants to merge 1 commit into
NousResearch:mainfrom
he-yufeng:fix/container-restart-via-service
Closed

fix(gateway): restart via service in containers#25263
he-yufeng wants to merge 1 commit into
NousResearch:mainfrom
he-yufeng:fix/container-restart-via-service

Conversation

@he-yufeng

Copy link
Copy Markdown

Summary

Closes #25217.

/restart currently uses a detached helper whenever the gateway is not running under systemd. That works for a normal shell process, but not for Docker/Podman: once the gateway exits as PID 1, the container stops and the detached helper goes with it.

This routes container restarts through the existing service-restart path instead:

  • systemd: unchanged, detached=False, via_service=True
  • Docker/Podman: now also detached=False, via_service=True
  • plain non-container process: still uses the detached helper

The service path exits with the existing restart code, so a Docker/Podman restart policy such as unless-stopped or on-failure can bring the container back.

Tests

python -m py_compile gateway\run.py tests\gateway\test_restart_notification.py tests\gateway\test_restart_drain.py
python -m ruff check gateway\run.py tests\gateway\test_restart_notification.py tests\gateway\test_restart_drain.py
python -m pytest -o addopts='' tests\gateway\test_restart_notification.py tests\gateway\test_restart_drain.py::test_restart_command_while_busy_requests_drain_without_interrupt -q --basetemp .tmp\pytest -p no:cacheprovider

Result: 27 passed.

Note: the full tests\gateway\test_restart_drain.py file has an existing Windows-only failure in test_launch_detached_restart_command_uses_setsid: this Windows run enters the sys.executable -c detached watcher branch, while that test expects the POSIX setsid bash path.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery area/docker Docker image, Compose, packaging labels May 13, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #25219 — same fix (Docker/Podman container detection for /restart service path) targeting the same issue #25217. Also competes with #25264. This PR uses is_container() from hermes_constants and includes test updates.

@liuhao1024

Copy link
Copy Markdown
Contributor

Heads up: duplicate of #25264

PR #25264 solves the same issue (#25217) but takes a cleaner approach — it imports the existing is_container() utility from hermes_constants instead of inlining the Docker/Podman file checks. The shared helper is more maintainable if additional container runtimes need detection in the future.

Both PRs have test coverage. Worth consolidating into one.

@he-yufeng

Copy link
Copy Markdown
Author

Thanks for the heads-up. Agreed this overlaps with #25219 / #25264 and targets the same restart path issue. Closing this one to avoid another competing duplicate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/docker Docker image, Compose, packaging comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

/restart shuts down gateway instead of restarting in Docker containers

3 participants