* fix(gateway): walk /proc/*/cmdline to find main-wrapper.sh under s6-overlay v3 (#49196) (cherry picked from commit 3a108c2df0edce4ce0e6f9f3a8eb8db3839a4630) * fix(container): peel s6-v3 rc.init prefix so dashboard role is detected kyssta-exe's preceding commit (#49238) fixed _read_container_argv() to locate the rc.init-launched main-wrapper.sh process under s6-overlay v3, but the skip still never fired: _strip_container_argv_prefix() only peeled a prefix when args[0] was init/main-wrapper.sh/hermes. Under s6 v3 the matched argv is /bin/sh -e /run/s6/basedir/scripts/rc.init top /opt/hermes/docker/main-wrapper.sh dashboard ... so args[0] stayed /bin/sh, _is_dashboard_container() returned False, and the dashboard container reconciled + started its own gateway-default — the exact dual Telegram getUpdates 409 in issue #49196. Fix: strip everything up to and including the main-wrapper.sh token (the stable boundary the image owns), covering both the v2 (/init ...) and v3 (/bin/sh ... rc.init top ...) shapes with one rule, instead of matching launcher tokens positionally. This also repairs _is_legacy_gateway_run_request() under v3, which shares the same strip helper (the issue called this out). Tests: extend the dashboard true/false parametrize sets with the s6-v3 argv shape, and add test_main_skips_reconcile_in_dashboard_container_s6v3 exercising main() end-to-end with the v3 argv. Verified via mutation that both new v3 assertions fail under the old positional strip and pass with the fix. --------- Co-authored-by: kyssta-exe <kyssta-exe@users.noreply.github.com>
This commit is contained in:
Ben Barclay
GitHub
parent
e448b21414
commit
6202fdfc35
2 changed files with 173 additions and 12 deletions
|
|
@ -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:
|
||||
return ()
|
||||
return tuple(part.decode("utf-8", "replace") for part in raw.split(b"\0") if part)
|
||||
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:
|
||||
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.
|
||||
"""Strip the s6/wrapper prefix off the container 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.
|
||||
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
|
||||
|
|
|
|||
|
|
@ -25,6 +25,29 @@ from hermes_cli.container_boot import (
|
|||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.fixture(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,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue