From 003af7f85ce2ebe24b38556d735a0d7678a4a8dd Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Fri, 31 Jul 2026 23:26:35 -0700 Subject: [PATCH] fix(docker): update runtime tests and docs for the entrypoint dispatcher Follow-ups from sweeper review of #43763: - tests/docker/test_tini_compat_shim.py asserts the dispatcher ENTRYPOINT (with /init delegation check) instead of a bare /init - tests/docker/test_smoke.py gains a docker run --init regression for the non-PID-1 fallback (#38349) - website/docs/user-guide/docker.md and the s6 supervision skill document the dispatcher and its wrapped-runtime fallback --- .../hermes-s6-container-supervision/SKILL.md | 5 +-- tests/docker/test_smoke.py | 27 ++++++++++++++++ tests/docker/test_tini_compat_shim.py | 31 ++++++++++++++----- website/docs/user-guide/docker.md | 6 ++-- 4 files changed, 57 insertions(+), 12 deletions(-) diff --git a/optional-skills/devops/hermes-s6-container-supervision/SKILL.md b/optional-skills/devops/hermes-s6-container-supervision/SKILL.md index 93726c62c5..dc3184feac 100644 --- a/optional-skills/devops/hermes-s6-container-supervision/SKILL.md +++ b/optional-skills/devops/hermes-s6-container-supervision/SKILL.md @@ -63,7 +63,8 @@ If you're just running the Hermes Agent and want to use Docker, see `website/doc | Path | Role | |---|---| -| `Dockerfile` | s6-overlay install + cont-init.d wiring + `ENTRYPOINT ["/init", "/opt/hermes/docker/main-wrapper.sh"]` | +| `Dockerfile` | s6-overlay install + cont-init.d wiring + `ENTRYPOINT ["/opt/hermes/docker/entrypoint-dispatch.sh"]` | +| `docker/entrypoint-dispatch.sh` | PID-1 dispatcher: exec's `/init` + main-wrapper when the image owns PID 1; on wrapped runtimes (Fly Machines, `docker run --init`) falls back to stage2-hook + main-wrapper directly, restoring the s6 helper PATH first (#38349). | | `docker/stage2-hook.sh` | The "old entrypoint logic" — UID remap, chown, seed, skills sync. Runs as cont-init.d/01-hermes-setup. | | `docker/cont-init.d/02-reconcile-profiles` | Calls `hermes_cli.container_boot` on every boot to restore profile gateway slots from the persistent volume. | | `docker/main-wrapper.sh` | The container's CMD. Routes user args, drops to hermes via `s6-setuidgid`, exec's the chosen program. | @@ -81,7 +82,7 @@ The original plan (v1–v3) called for main hermes to run as a supervised s6-rc 1. **cont-init.d scripts receive no CMD args** — so the stage2 hook can't parse `docker run chat -q "hi"` to set `HERMES_ARGS` for a service `run` script to consume. 2. **`/run/s6/basedir/bin/halt` does NOT propagate the exit code** written to `/run/s6-linux-init-container-results/exitcode`. Containers always exit 143 (SIGTERM) regardless. Confirmed by skarnet (s6 author) in [issue #477](https://github.com/just-containers/s6-overlay/issues/477): _"if you want a container shutdown, you need to either have your CMD exit, or, if you have no CMD, write the container exit code you want then call halt"_. -So we use the s6-overlay-native CMD pattern: `ENTRYPOINT ["/init", "/opt/hermes/docker/main-wrapper.sh"]`. /init prepends the wrapper to user args automatically — so `docker run --version` becomes `/init main-wrapper.sh --version`, and `--version` doesn't get intercepted by /init's POSIX shell. The wrapper drops to hermes via `s6-setuidgid`, then exec's the chosen program. The program's exit code becomes the container exit code, exactly matching the pre-s6 tini contract. +So we use the s6-overlay-native CMD pattern via the dispatcher: `ENTRYPOINT ["/opt/hermes/docker/entrypoint-dispatch.sh"]`, which under PID 1 exec's `/init /opt/hermes/docker/main-wrapper.sh "$@"`. The wrapper is prepended to user args automatically — so `docker run --version` becomes `/init main-wrapper.sh --version`, and `--version` doesn't get intercepted by /init's POSIX shell. The wrapper drops to hermes via `s6-setuidgid`, then exec's the chosen program. The program's exit code becomes the container exit code, exactly matching the pre-s6 tini contract. When the entrypoint is NOT PID 1 (Fly Machines, `docker run --init`), the dispatcher skips `/init` entirely (it would abort with `can only run as pid 1`), restores the s6 helper PATH, runs stage2-hook.sh, and exec's main-wrapper.sh directly — no supervised services on that path (#38349). Trade-off: main hermes is unsupervised under s6. That exactly matches its behavior under tini (the pre-s6 image). Dashboard supervision is the only **new** guarantee — and per-profile gateways under `/run/service/` get full supervision. diff --git a/tests/docker/test_smoke.py b/tests/docker/test_smoke.py index 9b9eed1d2e..a1ddd03b53 100644 --- a/tests/docker/test_smoke.py +++ b/tests/docker/test_smoke.py @@ -58,3 +58,30 @@ def test_dashboard_subcommand_present(built_image: str) -> None: assert "dashboard" in combined or "usage" in combined, ( f"dashboard --help output unexpected: {combined[-2000:]!r}" ) + + +def test_hermes_help_under_wrapped_init(built_image: str) -> None: + """``docker run --init --rm --help`` must exit 0. + + Regression guard for #38349: platforms whose own init owns PID 1 + (Fly Machines, ``docker run --init``, podman on some hosts) exec the + image entrypoint as a child. s6-overlay's ``/init`` hard-aborts + there with ``s6-overlay-suexec: fatal: can only run as pid 1``. The + entrypoint dispatcher must detect the non-PID-1 case and fall back + to the direct bootstrap path so the requested command still runs. + """ + r = subprocess.run( + ["docker", "run", "--init", "--rm", built_image, "--help"], + capture_output=True, text=True, timeout=120, + ) + assert "can only run as pid 1" not in (r.stdout + r.stderr), ( + f"s6-overlay-suexec aborted under a wrapped init (#38349): " + f"stderr={r.stderr[-2000:]!r}" + ) + assert r.returncode == 0, ( + f"hermes --help failed under `docker run --init` (exit {r.returncode}): " + f"stdout={r.stdout[-2000:]!r} stderr={r.stderr[-2000:]!r}" + ) + assert "Traceback" not in r.stderr, ( + f"hermes --help produced a traceback under --init: {r.stderr[-2000:]!r}" + ) diff --git a/tests/docker/test_tini_compat_shim.py b/tests/docker/test_tini_compat_shim.py index ac63759703..ff72e3039f 100644 --- a/tests/docker/test_tini_compat_shim.py +++ b/tests/docker/test_tini_compat_shim.py @@ -40,11 +40,15 @@ def test_tini_compat_shim_exists(built_image: str) -> None: ) -def test_entrypoint_is_init_not_tini(built_image: str) -> None: - """The image's actual ENTRYPOINT must be /init (s6-overlay). +def test_entrypoint_is_dispatcher_not_tini(built_image: str) -> None: + """The image's actual ENTRYPOINT must be the PID-1 dispatcher. - The tini shim is only for legacy external wrappers; the image's own - runtime must continue to use the canonical /init. + Since #38349 the ENTRYPOINT is ``entrypoint-dispatch.sh``, which + exec's the canonical ``/init`` (s6-overlay) when the image owns + PID 1 and falls back to a direct bootstrap on wrapped runtimes + (Fly Machines, ``docker run --init``). The tini shim is only for + legacy external wrappers; the image's own runtime must route + through the dispatcher, never through /usr/bin/tini. """ r = subprocess.run( ["docker", "inspect", built_image, @@ -53,13 +57,24 @@ def test_entrypoint_is_init_not_tini(built_image: str) -> None: ) assert r.returncode == 0, f"docker inspect failed: {r.stderr}" entrypoint = r.stdout.strip() - assert "/init" in entrypoint, ( - f"ENTRYPOINT is not /init: {entrypoint!r}" + assert "entrypoint-dispatch.sh" in entrypoint, ( + f"ENTRYPOINT is not the PID-1 dispatcher: {entrypoint!r}" ) - # The entrypoint array should be ["/init", "/opt/hermes/docker/main-wrapper.sh"] # /usr/bin/tini should NOT be in the entrypoint. assert "tini" not in entrypoint.lower(), ( - f"ENTRYPOINT references tini instead of /init: {entrypoint!r}" + f"ENTRYPOINT references tini instead of the dispatcher: {entrypoint!r}" + ) + # The dispatcher must still hand PID-1 execution to /init inside + # the image, preserving the supervision tree. + r2 = subprocess.run( + ["docker", "run", "--rm", "--entrypoint", "sh", built_image, "-c", + "grep -q 'exec /init /opt/hermes/docker/main-wrapper.sh' " + "/opt/hermes/docker/entrypoint-dispatch.sh"], + capture_output=True, text=True, timeout=60, + ) + assert r2.returncode == 0, ( + "entrypoint-dispatch.sh in the image does not delegate the " + f"PID-1 path to /init: stderr={r2.stderr[-500:]!r}" ) diff --git a/website/docs/user-guide/docker.md b/website/docs/user-guide/docker.md index c4b8c73908..c072a01804 100644 --- a/website/docs/user-guide/docker.md +++ b/website/docs/user-guide/docker.md @@ -482,7 +482,9 @@ The official image is based on `debian:13.4` and includes: The image treats `/opt/hermes` as an immutable install tree at runtime. Optional Python extras, Node workspaces, and TUI assets that must be available inside Docker need to be baked during the image build; runtime lazy installs are disabled so supervised gateways and `docker exec hermes …` commands do not try to write dependency artifacts back into the read-only source tree. -The container's `ENTRYPOINT` is s6-overlay's `/init`. On boot it: +The container's `ENTRYPOINT` is a small dispatcher (`docker/entrypoint-dispatch.sh`). When the container owns PID 1 (normal Docker / Podman), it exec's s6-overlay's `/init` and you get the full supervision tree described below. When a platform wraps the image entrypoint under its own PID-1 init (Fly.io Machines, `docker run --init`, some Nomad/Kubernetes setups), `/init` would abort with `s6-overlay-suexec: fatal: can only run as pid 1` — so the dispatcher instead runs the stage2 bootstrap directly and exec's the main wrapper without s6. On that fallback path the requested command still runs, but supervised services (dashboard, per-profile gateways) are unavailable. + +On the PID-1 path, `/init`: 1. Runs `/etc/cont-init.d/01-hermes-setup` (= `docker/stage2-hook.sh`) as root: optional UID/GID remap, fixes volume ownership, seeds `.env` / `config.yaml` / `SOUL.md` on first boot, runs non-interactive config-schema migrations unless `HERMES_SKIP_CONFIG_MIGRATION=1`, syncs bundled skills. 2. Runs `/etc/cont-init.d/02-reconcile-profiles` (= `hermes_cli.container_boot`): walks `$HERMES_HOME/profiles//`, recreates the per-profile gateway s6 service slot under `/run/service/gateway-/`, and auto-starts only those whose last recorded state was `running` (see [Per-profile gateway supervision](#per-profile-gateway-supervision)). 3. Starts the static `main-hermes` and `dashboard` s6-rc services. @@ -493,7 +495,7 @@ The container's `ENTRYPOINT` is s6-overlay's `/init`. On boot it: The container exits when this main program exits, with its exit code. :::warning Breaking change vs. pre-s6 images -The container ENTRYPOINT is now `/init` (s6-overlay), not `/usr/bin/tini`. All five documented `docker run` invocation patterns (no args, `chat -q "…"`, `sleep infinity`, `bash`, `--tui`) behave identically to the tini-based image. If you have a downstream wrapper that depended on tini-specific signal behavior or hard-coded `/usr/bin/tini --` invocation, pin to the previous image tag. +The container ENTRYPOINT is now the `entrypoint-dispatch.sh` dispatcher (which delegates to s6-overlay's `/init` under PID 1), not `/usr/bin/tini`. All five documented `docker run` invocation patterns (no args, `chat -q "…"`, `sleep infinity`, `bash`, `--tui`) behave identically to the tini-based image. If you have a downstream wrapper that depended on tini-specific signal behavior or hard-coded `/usr/bin/tini --` invocation, pin to the previous image tag. ::: :::warning Privilege model