Dashboard container role detection under s6-overlay v3

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #50600 fix(container): detect dashboard role under s6-overlay v3 (#49196)

  merged to main

  files: hermes_cli/container_boot.py, tests/hermes_cli/test_container_boot.py

  Show the diff

  • hermes_cli/container_boot.py

    @@ -199,28 +199,89 @@ def _maybe_migrate_legacy_gateway_run_state(
     
     
     def _read_container_argv() -> tuple[str, ...]:
    -    """Best-effort read of the container PID 1 argv."""
    +    """Best-effort read of the container's main program argv.
    +
    +    Under s6-overlay v2, PID 1 is ``/init`` and its argv contains the
    +    ``main-wrapper.sh`` path.  Under s6-overlay v3, PID 1 is
    +    ``s6-svscan`` and the actual command (``rc.init top main-wrapper.sh
    +    ...``) lives on a different PID.  We try PID 1 first (fast path,
    +    covers v2 and pre-s6 images), then fall back to scanning
    +    ``/proc/*/cmdline`` for a process whose argv contains
    +    ``main-wrapper.sh`` (the rc.init-launched PID in v3).
    +    """
    +    # Fast path: PID 1 is the command itself (s6-overlay v2 / tini).
         try:
             raw = Path("/proc/1/cmdline").read_bytes()
    +        argv = tuple(
    +            part.decode("utf-8", "replace") for part in raw.split(b"\0") if part
    +        )
    +        if any("main-wrapper.sh" in part for part in argv):
    +            return argv
    +    except OSError:
    +        pass
    +
    +    # Slow path: s6-overlay v3 — PID 1 is s6-svscan; find the
    +    # rc.init-launched process whose argv contains main-wrapper.sh.
    +    try:
    +        proc_dir = Path("/proc")
    +        for entry in proc_dir.iterdir():
    +            if not entry.name.isdigit():
    +                continue
    +            try:
    +                raw = (entry / "cmdline").read_bytes()
    +            except OSError:
    +                continue
    +            argv = tuple(
    +                part.decode("utf-8", "replace")
    +                for part in raw.split(b"\0")
    +                if part
    +            )
    +            if any("main-wrapper.sh" in part for part in argv):
    +                return argv
         except OSError:
    -        return ()
    -    return tuple(part.decode("utf-8", "replace") for part in raw.split(b"\0") if part)
    +        pass
     
    +    return ()
     
    -def _strip_container_argv_prefix(argv: Sequence[str]) -> list[str]:
    -    """Strip the s6/wrapper prefix off PID 1 argv, leaving the hermes args.
     
    -    The container PID 1 argv looks like
    -    ``/init /opt/hermes/docker/main-wrapper.sh <subcommand> [args...]`` and
    -    the wrapper re-execs ``hermes <subcommand>``. Peel ``init`` →
    -    ``main-wrapper.sh`` → ``hermes`` so callers can match on the bare
    -    subcommand. Shared by the legacy-gateway and dashboard role detectors.
    +def _strip_container_argv_prefix(argv: Sequence[str]) -> list[str]:
    +    """Strip the s6/wrapper prefix off the container argv, leaving the hermes args.
    +
    +    Two container-command argv shapes are handled:
    +
    +    * **s6-overlay v2 / tini:** PID 1 argv is
    +      ``/init /opt/hermes/docker/main-wrapper.sh <subcommand> [args...]``.
    +    * **s6-overlay v3:** PID 1 is ``s6-svscan`` and the command lives on the
    +      rc.init-launched process as ``/bin/sh -e
    +      /run/s6/basedir/scripts/rc.init top /opt/hermes/docker/main-wrapper.sh
    +      <subcommand> [args...]`` (see :func:`_read_container_argv`).
    +
    +    Rather than peel each leading token positionally (which silently breaks
    +    the moment s6 changes its launcher shape again — exactly what happened
    +    in the v2→v3 bump), drop everything up to and including the
    +    ``main-wrapper.sh`` token: that wrapper path is the stable boundary the
    +    image owns, and the subcommand always follows it. Pre-s6 / direct
    +    ``hermes`` invocations carry no wrapper, so fall back to peeling a bare
    +    ``init`` prefix. The wrapper re-execs ``hermes <subcommand>``, so an
    +    explicit leading ``hermes`` is peeled too. Shared by the legacy-gateway
    +    and dashboard role detectors.
         """
         args = list(argv)
    -    if args and Path(args[0]).name == "init":
    -        args = args[1:]
    -    if args and args[0].endswith("main-wrapper.sh"):
    +
    +    # Preferred boundary: everything through main-wrapper.sh is launcher
    +    # prefix. Covers s6-overlay v2 (`/init …main-wrapper.sh …`) and v3
    +    # (`/bin/sh -e …rc.init top …main-wrapper.sh …`) with one rule.
    +    wrapper_idx = next(
    +        (i for i, a in enumerate(args) if a.endswith("main-wrapper.sh")),
    +        None,
    +    )
    +    if wrapper_idx is not None:
    +        args = args[wrapper_idx + 1 :]
    +    elif args and Path(args[0]).name == "init":
    +        # Defensive: an `init` prefix with no wrapper token in argv.
             args = args[1:]
    +
    +    # The wrapper re-execs `hermes <subcommand>`; peel an explicit hermes.
         if args and Path(args[0]).name == "hermes":
             args = args[1:]
         return args

  • tests/hermes_cli/test_container_boot.py

    @@ -25,6 +25,29 @@
     # ---------------------------------------------------------------------------
     
     
    [redacted:email](autouse=True)
    +def _hermetic_container_argv(monkeypatch: pytest.MonkeyPatch) -> None:
    +    """Default ``_read_container_argv()`` to empty for the whole module.
    +
    +    ``_read_container_argv()`` walks the entire ``/proc`` table looking for
    +    a process whose argv contains ``main-wrapper.sh`` (the s6-overlay v3
    +    fallback). On a host that is *also* running hermes containers, those
    +    containers' ``main-wrapper.sh`` processes are visible in the host's
    +    ``/proc`` (shared PID view), so the scan would pick up a foreign
    +    ``gateway run`` argv and make ``_maybe_migrate_legacy_gateway_run_state``
    +    synthesize ``running`` state — flaking any test that reconciles without
    +    injecting ``container_argv``. Inside the real container ``/proc`` is the
    +    container's own PID namespace, so production is unaffected; this fixture
    +    just makes the unit suite hermetic. Tests that need a specific argv
    +    either pass ``container_argv=`` to ``reconcile_profile_gateways`` or
    +    monkeypatch ``_read_container_argv`` themselves (both override this).
    +    """
    +    monkeypatch.setattr(
    +        "hermes_cli.container_boot._read_container_argv",
    +        lambda: (),
    +    )
    +
    +
     def _make_profile(
         hermes_home: Path,
         name: str,
    @@ -733,6 +756,24 @@ def test_profiles_default_subdir_is_skipped_with_warning(
             ),
             # Wrapper that kept the explicit `hermes` argv0.
             ("/init", "/opt/hermes/docker/main-wrapper.sh", "hermes", "dashboard"),
    +        # s6-overlay v3: PID 1 is s6-svscan, so the role is read off the
    +        # rc.init-launched process whose argv is
    +        # `/bin/sh -e .../rc.init top .../main-wrapper.sh dashboard ...`.
    +        # This is the exact shape that regressed in issue #49196.
    +        (
    +            "/bin/sh",
    +            "-e",
    +            "/run/s6/basedir/scripts/rc.init",
    +            "top",
    +            "/opt/hermes/docker/main-wrapper.sh",
    +            "dashboard",
    +            "--host",
    +            "0.0.0.0",
    +            "--port",
    +            "9119",
    +            "--no-open",
    +            "--insecure",
    +        ),
         ],
     )
     def test_is_dashboard_container_true_for_dashboard_argv(
    @@ -756,6 +797,17 @@ def test_is_dashboard_container_true_for_dashboard_argv(
             # we key on is the SUBCOMMAND, and `gateway run -p dashboard` is a
             # gateway container.
             ("gateway", "run", "-p", "dashboard"),
    +        # s6-overlay v3 gateway container — the rc.init-launched argv for a
    +        # gateway role must still read as non-dashboard (issue #49196 shape).
    +        (
    +            "/bin/sh",
    +            "-e",
    +            "/run/s6/basedir/scripts/rc.init",
    +            "top",
    +            "/opt/hermes/docker/main-wrapper.sh",
    +            "gateway",
    +            "run",
    +        ),
         ],
     )
     def test_is_dashboard_container_false_for_non_dashboard_argv(
    @@ -798,6 +850,54 @@ def test_main_skips_reconcile_in_dashboard_container(
         assert "skipping (dashboard container" in capsys.readouterr().out
     
     
    +def test_main_skips_reconcile_in_dashboard_container_s6v3(
    +    tmp_path: Path,
    +    monkeypatch: pytest.MonkeyPatch,
    +    capsys: pytest.CaptureFixture[str],
    +) -> None:
    +    """The dashboard skip must fire under the s6-overlay v3 argv shape.
    +
    +    Regression test for issue #49196: under s6-overlay v3 the container
    +    command is read off the rc.init-launched process, whose argv is
    +    ``/bin/sh -e .../rc.init top .../main-wrapper.sh dashboard ...`` — not a
    +    bare ``/init`` prefix. Before the fix, the prefix-strip left ``/bin/sh``
    +    at args[0], so the role read as non-dashboard, the dashboard container
    +    reconciled, and it started its own gateway-default (dual Telegram
    +    getUpdates 409). Asserting the slot is absent proves the skip fires.
    +    """
    +    from hermes_cli import container_boot
    +
    +    scandir = tmp_path / "run-service"; scandir.mkdir()
    +    _make_profile(tmp_path, "worker", state="running")
    +    monkeypatch.setenv("HERMES_HOME", str(tmp_path))
    +    monkeypatch.setenv("S6_PROFILE_GATEWAY_SCANDIR", str(scandir))
    +    monkeypatch.setattr(
    +        container_boot,
    +        "_read_container_argv",
    +        lambda: (
    +            "/bin/sh",
    +            "-e",
    +            "/run/s6/basedir/scripts/rc.init",
    +            "top",
    +            "/opt/hermes/docker/main-wrapper.sh",
    +            "dashboard",
    +            "--host",
    +            "0.0.0.0",
    +            "--port",
    +            "9119",
    +            "--no-open",
    +            "--insecure",
    +        ),
    +    )
    +
    +    rc = container_boot.main()
    +
    +    assert rc == 0
    +    assert not (scandir / "gateway-worker").exists()
    +    assert not (scandir / "gateway-default").exists()
    +    assert "skipping (dashboard container" in capsys.readouterr().out
    +
    +
     def test_main_reconciles_in_gateway_container(
         tmp_path: Path,
         monkeypatch: pytest.MonkeyPatch,