diff --git a/CHANGELOG.md b/CHANGELOG.md index efc3fe3..b02e3b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -630,6 +630,7 @@ are absent), so shell regressions are caught locally before the push. ### Bug fixes +- **`compose.ps_status` now recognises every daemon-unreachable phrasing and can't hang (#178).** It maps both `cannot connect to the docker daemon` and `failed to connect to the docker API at ...` (the custom/rootless `DOCKER_HOST` phrasing) to `docker-unreachable` instead of misreporting a live container as `not-created`, and it runs `docker compose ps` under a bounded 20 s timeout (`subprocess.TimeoutExpired` → `docker-unreachable`) so a wedged daemon or an unresponsive TCP `DOCKER_HOST` degrades gracefully rather than hanging `ls`/`status`/`doctor` forever. Scoped to the read-only `ps_status` probe; lifecycle/streaming verbs are unchanged. - **The Magisk denylist now hides root in the Play-Integrity/DroidGuard process (#170).** Denylist entries take a `package[/process]` shape, and `magisk-config.sh` inserts the real package into `package_name` and the process into `process` — never copying one value into both columns. The default enrolls `com.google.android.gms.unstable` (DroidGuard) as a **process** of the `com.google.android.gms` package instead of a bogus `package_name` that matched no installed app, so vanilla (non-Shamiko) Magisk actually hides root in the attestation process. The grammar validates both halves (SQL-injection prophylaxis), the health check matches enrolment by the package half, and the example configs demonstrate the corrected form. - **`registry._read()` no longer silently drops an instance row that fails row-level validation (#252).** A row with a known backend kind but a rejected payload is now preserved opaquely so its port index stays reserved and it round-trips; a row too broken to salvage (bad `created_at`, missing/non-int index) is surfaced loudly (backed up to `.bak` with a hint) instead of being dropped and having its index silently reused. - **`create`/`register` now refuse a directory that nests inside — or contains — another registered instance (#255).** Previously the overlap guard existed only for `restore`, so a nested `create` could later be wiped out by a `destroy` of the outer instance. The guard runs before any `mkdir`/registry write, so a refused operation is a no-op. diff --git a/src/beetroot/compose.py b/src/beetroot/compose.py index 9388fc2..7f9042e 100644 --- a/src/beetroot/compose.py +++ b/src/beetroot/compose.py @@ -20,11 +20,27 @@ import subprocess from collections.abc import Sequence from pathlib import Path -from typing import Literal +from typing import Final, Literal from . import paths from .settings import settings +# Bounded deadline for the read-only ``docker compose ps`` probe. A wedged +# (reachable-but-unresponsive) daemon or an unresponsive TCP ``DOCKER_HOST`` +# would otherwise make ``ps_status`` — and every verb that reads it (``ls``, +# ``status``, ``doctor``) — hang forever. Mirrors ``builder._DOCKER_INFO_TIMEOUT``. +_PS_STATUS_TIMEOUT: Final[int] = 20 + +# Lowercased stderr markers the Docker CLI emits when the daemon is +# unreachable. The CLI uses more than one phrasing — ``cannot connect to the +# docker daemon`` (default socket) and ``failed to connect to the docker API +# at ...`` (custom/rootless socket via ``DOCKER_HOST``) — so we match a +# family of substrings rather than a single exact string. +_DAEMON_UNREACHABLE_MARKERS: Final[tuple[str, ...]] = ( + "cannot connect to the docker daemon", + "failed to connect to the docker", +) + # Closed enum of the strings ``ps_status`` may return. The compose # subcommand reports a free-form ``State`` field; this enum gates every # string we ever surface to callers so verbs (``doctor``, ``status``) @@ -201,7 +217,9 @@ def ps_status(name: str, instance_root: Path) -> ComposeStatus: Queries ``docker compose ps --format json`` live; never reads from cache. Distinguishes "docker daemon unreachable" from "not-created" so callers (``beetroot doctor``, ``ls --json``) can give a precise - diagnostic. + diagnostic. The probe is bounded by :data:`_PS_STATUS_TIMEOUT`: a + wedged-but-reachable daemon (or an unresponsive TCP ``DOCKER_HOST``) + degrades to ``"docker-unreachable"`` instead of hanging the verb. Args: name: Instance name. @@ -220,17 +238,21 @@ def ps_status(name: str, instance_root: Path) -> ComposeStatus: ["ps", "--format", "json"], capture_output=True, text=True, + timeout=_PS_STATUS_TIMEOUT, ) - except ComposeError: - # ``_ensure_docker`` raised — docker binary not on PATH. + except (ComposeError, subprocess.TimeoutExpired): + # ``ComposeError``: ``_ensure_docker`` raised — docker binary not on + # PATH. ``TimeoutExpired``: a reachable-but-wedged daemon, or a TCP + # ``DOCKER_HOST`` that accepts the connection but never answers. Both + # degrade gracefully rather than block ``ls``/``status``/``doctor``. return "docker-unreachable" if res.returncode != 0: # Non-zero is most commonly "no such project" → not created. - # We could distinguish "daemon not running" via stderr scraping, - # but that's fragile; map to docker-unreachable only when - # compose stderr explicitly reports it. + # A daemon-unreachable failure is distinguished by scraping stderr + # for any of the CLI's connection-failure phrasings; everything + # else is treated as a missing project. stderr = (res.stderr or "").lower() - if "cannot connect to the docker daemon" in stderr: + if any(marker in stderr for marker in _DAEMON_UNREACHABLE_MARKERS): return "docker-unreachable" return "not-created" if not res.stdout.strip(): diff --git a/tests/test_compose.py b/tests/test_compose.py index 81acf6b..fe9b055 100644 --- a/tests/test_compose.py +++ b/tests/test_compose.py @@ -227,6 +227,35 @@ def test_docker_binary_missing_returns_docker_unreachable( monkeypatch.setattr(shutil, "which", lambda name: None) assert compose.ps_status("alpha", tmp_path) == "docker-unreachable" + def test_failed_to_connect_api_phrasing_is_docker_unreachable(self, tmp_path: Path) -> None: + # #178: a custom/rootless socket via DOCKER_HOST fails with the + # "failed to connect to the docker API at ..." phrasing, which lacks + # the "cannot connect to the docker daemon" substring. It must still + # map to ``docker-unreachable`` (not the misleading ``not-created``). + stderr = ( + "failed to connect to the docker API at unix:///tmp/missing.sock; " + "check if the path is correct and if the daemon is running" + ) + res = subprocess.CompletedProcess(args=[], returncode=1, stdout="", stderr=stderr) + with patch("subprocess.run", return_value=res): + assert compose.ps_status("alpha", tmp_path) == "docker-unreachable" + + def test_timeout_returns_docker_unreachable(self, tmp_path: Path) -> None: + # #178: a reachable-but-wedged daemon (or an unresponsive TCP + # DOCKER_HOST) must degrade to ``docker-unreachable`` rather than + # hang the verb forever. + with patch( + "subprocess.run", + side_effect=subprocess.TimeoutExpired(cmd="docker compose ps", timeout=1), + ): + assert compose.ps_status("alpha", tmp_path) == "docker-unreachable" + + def test_ps_status_passes_bounded_timeout(self, tmp_path: Path) -> None: + # The read-only probe must be bounded so ls/status/doctor can't hang. + with patch("subprocess.run", return_value=_ok_result("")) as mock_run: + compose.ps_status("alpha", tmp_path) + assert mock_run.call_args.kwargs["timeout"] == compose._PS_STATUS_TIMEOUT + class TestComposeError: def test_is_runtime_error(self) -> None: