diff --git a/CHANGELOG.md b/CHANGELOG.md index b02e3b2..05e1082 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -630,6 +630,9 @@ are absent), so shell regressions are caught locally before the push. ### Bug fixes +- **`beetroot modes` no longer reports the redroid host/auto mode as `supported` when the Docker CLI is present but the daemon is stopped (#179).** The verdict now gates on a live `docker info` probe (relocated from `builder` into a shared `capabilities.docker_daemon_responsive()`): a ready host binder + installed CLI but an unresponsive `dockerd` reports `needs-setup` with the remedy "start the Docker daemon" instead of falsely advertising a mode that can't boot a container. +- **`beetroot doctor` on a `binder: vm` instance no longer reports `pass` when the QEMU binary or the guest kernel/rootfs artifacts are missing (#191).** `health()` gained a `vm.qemu` row (QEMU on `PATH`, reusing the same install remedy `beetroot modes` prints) and a `vm.artifacts` row (kernel + rootfs resolve, else a `beetroot build --vm-kernel` hint), so a green `vm.accel` alone can no longer imply a bootable VM. +- **`beetroot doctor` no longer false-fails a healthy micro-VM by reusing the USB-style `adb.serial` row (#164).** The vm backend now emits an `adb.connect` (connect-then-verify) row instead, matching the redroid path — a TCP adb target only appears after an explicit `adb connect`, so the always-listed-serial assumption was wrongly failing a fresh adb-server lifetime and making doctor exit non-zero. - **`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. diff --git a/docs/design/binderless-hosts-qemu-tcg.md b/docs/design/binderless-hosts-qemu-tcg.md index e00ced1..fd96323 100644 --- a/docs/design/binderless-hosts-qemu-tcg.md +++ b/docs/design/binderless-hosts-qemu-tcg.md @@ -382,8 +382,8 @@ expensive step. the instance's stride-allocated host ADB port. 4. **`doctor` + banners** — ✅ done (issue #44): `beetroot up` prints the §7.3 capability banner (quiet on KVM, loud on TCG) and `beetroot doctor` - runs `vm.process` + `vm.accel` rows. `binder: auto`/`host` keep their - existing behaviour. + runs `vm.process` + `vm.accel` + `vm.qemu` + `vm.artifacts` + `adb.connect` + rows (issue #191/#164). `binder: auto`/`host` keep their existing behaviour. 5. **KVM fast path** — ✅ done (issue #44): `detect_accel` probes `/dev/kvm` (read+write) and prefers `-accel kvm` (`-cpu host`); `auto` falls back to TCG, an explicit `kvm` request on a host without `/dev/kvm` diff --git a/docs/guides/running-in-ci.md b/docs/guides/running-in-ci.md index c79037a..bb5c406 100644 --- a/docs/guides/running-in-ci.md +++ b/docs/guides/running-in-ci.md @@ -155,7 +155,8 @@ hosted runner (the Option A path above) in three tiers: is heavy and non-blocking while it's hardened. * **Tier-VM** builds the binder-enabled guest kernel + rootfs, boots redroid inside the `binder: vm` QEMU micro-VM, and drives it through the adb backend - (`ls` / `shell` / the `doctor` `vm.process` + `vm.accel` rows; Frida is + (`ls` / `shell` / the `doctor` `vm.process` + `vm.accel` + `vm.qemu` + + `vm.artifacts` + `adb.connect` rows; Frida is asserted to report its "not yet supported on the vm backend" message). On a GitHub-hosted runner there is no `/dev/kvm`, so it runs under TCG — a slow (~100 s+) but real boot. The kernel + rootfs build is the long pole. diff --git a/docs/guides/sandbox-quickstart.md b/docs/guides/sandbox-quickstart.md index 28fb1f1..eec1cb9 100644 --- a/docs/guides/sandbox-quickstart.md +++ b/docs/guides/sandbox-quickstart.md @@ -203,7 +203,7 @@ uv run beetroot logs alpha # follow Android init → sys.boot_completed=1 Once it reports boot completion, confirm the instance is healthy: ```bash -uv run beetroot doctor alpha # adb, magisk, frida, plus the vm.process / vm.accel rows +uv run beetroot doctor alpha # vm.process / vm.accel / vm.qemu / vm.artifacts / adb.connect rows uv run beetroot shell alpha # interactive shell on the guest ``` diff --git a/docs/how-it-works/binder-and-modes.md b/docs/how-it-works/binder-and-modes.md index bcf73ee..cda6509 100644 --- a/docs/how-it-works/binder-and-modes.md +++ b/docs/how-it-works/binder-and-modes.md @@ -111,7 +111,7 @@ They answer different questions and you'll use both: | Scope | the **host** | one **existing instance** | | When | *before* creating an instance / picking `binder` | *after* `up`, to debug a running instance | | Needs an instance? | No | Yes | -| Answers | "what can this machine run?" | "is this instance healthy?" (adb, magisk, frida, `host.binder`, `vm.process`, `vm.accel`) | +| Answers | "what can this machine run?" | "is this instance healthy?" (adb, magisk, frida, `host.binder`, `vm.process`, `vm.accel`, `vm.qemu`, `vm.artifacts`) | `doctor`'s `host.binder` row reports the same binder probe `modes` uses, but only in the context of a single instance; `modes` is the standalone, diff --git a/docs/reference/cli.md b/docs/reference/cli.md index ff4018e..e079fd2 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -424,6 +424,8 @@ Redroid checks: `compose.status`, `host.binder`, `adb.connect`, `frida.handshake Adb checks: `adb.serial`, `frida.handshake`, `magisk.zygisk`, `magisk.denylist.com.google.android.gms`. `compose.status` is not applicable. +VM (`binder: vm`) checks: `vm.process` (is QEMU alive?), `vm.accel` (KVM vs the slow-TCG note), `vm.qemu` (the QEMU emulator is on `PATH`), `vm.artifacts` (the guest kernel + rootfs exist — else a `beetroot build --vm-kernel` hint), and `adb.connect` (connect-then-verify against the forwarded loopback port, not the USB-style `adb.serial`). The Frida and Magisk rows are omitted — the network-isolated guest has no Frida and boots a plain redroid image with no Magisk. + --- ## `modes` diff --git a/src/beetroot/backends/vm.py b/src/beetroot/backends/vm.py index 0dda53c..b845e4b 100644 --- a/src/beetroot/backends/vm.py +++ b/src/beetroot/backends/vm.py @@ -28,13 +28,13 @@ from pathlib import Path from typing import TYPE_CHECKING, Literal, Self -from beetroot import builder, config, console, paths, ports, registry +from beetroot import builder, capabilities, config, console, paths, ports, registry from beetroot.api import ( FRIDA_ADDRESS_UNSUPPORTED, AdbNotInstalledError, BackendCapabilityError, InstanceNotFoundError, - adb_device_health, + _check_adb_connect, ) from beetroot.backends import register_backend from beetroot.settings import settings @@ -973,14 +973,27 @@ def health(self) -> dict[str, CheckResult]: """ Aggregate the VM-backed health checks for this instance. - Includes a VM-specific ``vm.process`` row (is QEMU alive?) and a - ``vm.accel`` row (kvm vs the slow-tcg note), then the shared adb rows so - downstream tools grep uniformly across backend kinds. Three shared rows - are dropped because they can never pass on the network-isolated, plain - upstream redroid guest: ``frida.handshake`` (Frida is unsupported — - issue #44) and the ``magisk.*`` rows (the ``binder: vm`` guest boots an - unmodified redroid image with no Magisk — issue #163), so a permanent - ``fail`` row would be noise. + Emits VM-specific rows rather than reusing the shared adb health set, + which assumes a Magisk-flashed, USB-listed device the ``binder: vm`` + guest is not: + + * ``vm.process`` — is the QEMU micro-VM process alive? + * ``vm.accel`` — resolved accelerator (kvm vs the slow-tcg note). + * ``vm.qemu`` — is the QEMU emulator binary on ``PATH``? Without it the + VM can never boot (issue #191), so a green ``vm.accel`` alone is not + proof of readiness. + * ``vm.artifacts`` — do the configured guest kernel + rootfs exist on the + host? A missing artifact is a ``beetroot build --vm-kernel`` away + (issue #191). + * ``adb.connect`` — connect-then-verify against the forwarded loopback + adb port. A TCP adb target only appears *after* an explicit ``adb + connect``, so the USB-style always-listed ``adb.serial`` row false- + fails a healthy VM from a fresh adb-server lifetime (issue #164). + + ``frida.handshake`` and the ``magisk.*`` rows are intentionally absent: + the network-isolated guest has no Frida (issue #44) and boots an + unmodified upstream redroid image with no Magisk (issue #163), so a + permanent ``fail`` row would be noise. Returns: Ordered dict of check name → :class:`CheckResult`. @@ -995,16 +1008,61 @@ def health(self) -> dict[str, CheckResult]: else CheckResult(status="fail", reason="QEMU micro-VM is not running") ) checks["vm.accel"] = _accel_check(self._cfg.vm.accel) - shared = adb_device_health(self) - shared.pop("frida.handshake", None) - # The guest runs plain redroid (no Magisk), so the magisk rows would be a - # permanent fail — drop them rather than mislead (issue #163). - for name in list(shared): - if name.startswith("magisk."): - shared.pop(name) - checks.update(shared) + checks["vm.qemu"] = self._qemu_binary_check() + checks["vm.artifacts"] = self._artifacts_check() + checks["adb.connect"] = _check_adb_connect(self.adb_address) return checks + def _qemu_binary_check(self) -> CheckResult: + """ + Report whether the QEMU emulator binary is on ``PATH`` (issue #191). + + ``CheckResult`` has no dedicated remedy field, so the fix pointer is + folded into the ``reason`` — reusing the shared + :data:`capabilities._QEMU_INSTALL` string so the doctor row matches what + ``beetroot modes`` prints. + + Returns: + A ``pass`` row when ``settings.qemu_bin`` resolves, else a ``fail`` + row (a VM whose emulator is missing can never boot). + """ + from beetroot.api import CheckResult # noqa: PLC0415 # avoid import cycle with api.py + + if shutil.which(settings.qemu_bin) is not None: + return CheckResult(status="pass") + return CheckResult( + status="fail", + # Reuse the shared modes remedy verbatim (do not duplicate the literal). + reason=( + f"QEMU ({settings.qemu_bin}) not found on PATH — {capabilities._QEMU_INSTALL}" # noqa: SLF001 # shared cross-module remedy string + ), + ) + + def _artifacts_check(self) -> CheckResult: + """ + Report whether the guest kernel + rootfs artifacts exist (issue #191). + + Resolves both artifacts through :func:`_resolve_artifact` (which raises + :class:`qemu.QemuLaunchError` when the file is unset or missing). A green + ``vm.accel`` alone would otherwise imply the VM can boot when the kernel + or rootfs has never been built. The ``QemuLaunchError`` message already + names the missing artifact and points at ``beetroot build --vm-kernel``. + + Returns: + A ``pass`` row when both artifacts resolve, else a ``fail`` row whose + reason names the missing artifact (the resolver's own message already + points at ``beetroot build --vm-kernel``, matching + :data:`capabilities._BUILD_HINT`). + """ + from beetroot.api import CheckResult # noqa: PLC0415 # avoid import cycle with api.py + + try: + _resolve_artifact(self._cfg.vm.kernel, settings.vm_kernel, "kernel") + _resolve_artifact(self._cfg.vm.rootfs, settings.vm_rootfs, "rootfs") + except qemu.QemuLaunchError as exc: + return CheckResult(status="fail", reason=str(exc)) + return CheckResult(status="pass") + def _accel_check(requested: Literal["auto", "kvm", "tcg"]) -> CheckResult: """ diff --git a/src/beetroot/builder.py b/src/beetroot/builder.py index 6c21adf..ec33761 100644 --- a/src/beetroot/builder.py +++ b/src/beetroot/builder.py @@ -36,7 +36,7 @@ from pydantic import BaseModel, ConfigDict -from . import config, console, kernel_download, paths, rootfs_download +from . import capabilities, config, console, kernel_download, paths, rootfs_download from .settings import settings # The patcher CLI flag each GApps vendor needs. Keyed by the *resolved* vendor @@ -313,7 +313,7 @@ def build_image( # noqa: PLR0913 # 7 keyword-only params; each is a distinct i # Docker daemon, so fail fast with the same actionable remedy the bake path # uses instead of a generic ``command failed (exit 1)`` mid-build (issue # #193). - if not _docker_daemon_responsive(): + if not capabilities.docker_daemon_responsive(): raise BootstrapError( f"Docker daemon: `{settings.docker_bin} info` failed (daemon not running?) — " "start the daemon (e.g. `sudo systemctl start docker`)" @@ -1405,23 +1405,6 @@ class PreflightProblem(BaseModel): ("xtables_multi", "iptables"), ) -# Probe timeout for the ``docker info`` daemon check (seconds). -_DOCKER_INFO_TIMEOUT: Final[int] = 20 - - -def _docker_daemon_responsive() -> bool: - """Return ``True`` iff the host Docker daemon answers ``docker info``.""" - try: - result = subprocess.run( # noqa: S603 # docker bin from settings; fixed argv - [settings.docker_bin, "info"], - check=False, - capture_output=True, - timeout=_DOCKER_INFO_TIMEOUT, - ) - except (OSError, subprocess.SubprocessError): - return False - return result.returncode == 0 - def vm_fetch_preflight() -> list[PreflightProblem]: """ @@ -1532,7 +1515,7 @@ def vm_bake_preflight(*, redroid_tar: Path | None = None) -> list[PreflightProbl fix="install Docker Engine (apt-get install docker.io)", ) ) - elif not _docker_daemon_responsive(): + elif not capabilities.docker_daemon_responsive(): problems.append( PreflightProblem( requirement="Docker daemon", diff --git a/src/beetroot/capabilities.py b/src/beetroot/capabilities.py index 6715e18..a5f8916 100644 --- a/src/beetroot/capabilities.py +++ b/src/beetroot/capabilities.py @@ -27,13 +27,45 @@ from __future__ import annotations import shutil +import subprocess from pydantic import BaseModel, ConfigDict from . import hostcheck -from .settings import Settings +from .settings import Settings, settings from .vm import qemu +# Probe timeout for the ``docker info`` daemon check (seconds). +_DOCKER_INFO_TIMEOUT = 20 + + +def docker_daemon_responsive() -> bool: + """ + Return ``True`` iff the host Docker daemon answers ``docker info``. + + A ``shutil.which(docker_bin)`` presence probe only proves the *CLI* is + installed — it says nothing about whether ``dockerd`` is actually running. + Anything that needs to *launch* a container (a redroid boot, a + ``beetroot build``) must probe daemon liveness with ``docker info`` (issues + #179 / #193), so this lives here as the single shared probe rather than a + per-caller duplicate. + + Returns: + ``True`` iff ``docker info`` exits 0 within + :data:`_DOCKER_INFO_TIMEOUT` seconds. + """ + try: + result = subprocess.run( # noqa: S603 # docker bin from settings; fixed argv + [settings.docker_bin, "info"], + check=False, + capture_output=True, + timeout=_DOCKER_INFO_TIMEOUT, + ) + except (OSError, subprocess.SubprocessError): + return False + return result.returncode == 0 + + # Per-mode verdict (the ``status`` field of :class:`ModeSupport`): # # * ``supported`` — works on this host as-is (modulo runtime inputs like a @@ -79,27 +111,44 @@ class ModeSupport(BaseModel): remedy: str -def _redroid_host(binder: hostcheck.BinderStatus, *, docker: bool) -> ModeSupport: +def _redroid_host( + binder: hostcheck.BinderStatus, *, docker: bool, docker_daemon: bool +) -> ModeSupport: """ Classify the ``redroid`` backend on the host-binder path. Args: binder: The probed host binder capability. docker: Whether the Docker CLI is on ``PATH``. + docker_daemon: Whether the Docker *daemon* answers ``docker info``. + Only meaningful when ``docker`` is ``True`` (there's no daemon to + probe without the CLI). Returns: The :class:`ModeSupport` verdict for ``binder: host`` / ``auto``. """ if binder.state == "ready": - if docker: + if not docker: return ModeSupport( - mode=MODE_REDROID_HOST, status="supported", reason=binder.reason, remedy="" + mode=MODE_REDROID_HOST, + status="needs-setup", + reason="host binder is ready, but the Docker CLI was not found", + remedy="install Docker and ensure the daemon is running", + ) + if not docker_daemon: + # The CLI is present but ``dockerd`` isn't answering — redroid can't + # boot a container, so this is not "supported" (issue #179). + return ModeSupport( + mode=MODE_REDROID_HOST, + status="needs-setup", + reason=( + "host binder is ready and the Docker CLI is present, but the " + "Docker daemon is not responding" + ), + remedy="start the Docker daemon", ) return ModeSupport( - mode=MODE_REDROID_HOST, - status="needs-setup", - reason="host binder is ready, but the Docker CLI was not found", - remedy="install Docker and ensure the daemon is running", + mode=MODE_REDROID_HOST, status="supported", reason=binder.reason, remedy="" ) if binder.state == "loadable": return ModeSupport( @@ -206,12 +255,13 @@ def _adb_adopt(*, adb_present: bool) -> ModeSupport: ) -def classify_modes( +def classify_modes( # noqa: PLR0913 # each keyword-only param is a distinct host probe *, binder: hostcheck.BinderStatus, kvm: bool, qemu_present: bool, docker: bool, + docker_daemon: bool, adb_present: bool, ) -> list[ModeSupport]: """ @@ -225,13 +275,15 @@ def classify_modes( kvm: Whether a usable ``/dev/kvm`` is present. qemu_present: Whether the QEMU system emulator is on ``PATH``. docker: Whether the Docker CLI is on ``PATH``. + docker_daemon: Whether the Docker daemon answers ``docker info`` + (only meaningful when ``docker`` is ``True``). adb_present: Whether the ``adb`` client is on ``PATH``. Returns: One :class:`ModeSupport` per mode, in a stable display order. """ return [ - _redroid_host(binder, docker=docker), + _redroid_host(binder, docker=docker, docker_daemon=docker_daemon), _vm_kvm(kvm=kvm, qemu_present=qemu_present), _vm_tcg(qemu_present=qemu_present), _adb_adopt(adb_present=adb_present), @@ -254,11 +306,16 @@ def survey(settings: Settings | None = None) -> list[ModeSupport]: The per-mode support matrix. """ cfg = settings if settings is not None else Settings() + docker_present = shutil.which(cfg.docker_bin) is not None return classify_modes( binder=hostcheck.binder_status(), # ``detect_accel("auto")`` returns "kvm" only when /dev/kvm is usable. kvm=qemu.detect_accel("auto") == "kvm", qemu_present=shutil.which(cfg.qemu_bin) is not None, - docker=shutil.which(cfg.docker_bin) is not None, + docker=docker_present, + # The daemon probe only matters when the CLI is present — without it + # there's nothing to ask ``docker info``, and the classifier ignores + # the flag on the no-CLI path anyway. + docker_daemon=docker_present and docker_daemon_responsive(), adb_present=shutil.which("adb") is not None, ) diff --git a/src/beetroot/compose.py b/src/beetroot/compose.py index 7f9042e..1ff96c7 100644 --- a/src/beetroot/compose.py +++ b/src/beetroot/compose.py @@ -28,7 +28,7 @@ # 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``. +# ``status``, ``doctor``) — hang forever. Mirrors ``capabilities._DOCKER_INFO_TIMEOUT``. _PS_STATUS_TIMEOUT: Final[int] = 20 # Lowercased stderr markers the Docker CLI emits when the daemon is diff --git a/tests/test_builder.py b/tests/test_builder.py index aa3b779..ab4075c 100644 --- a/tests/test_builder.py +++ b/tests/test_builder.py @@ -15,7 +15,7 @@ import pytest -from beetroot import builder, config, kernel_download, rootfs_download +from beetroot import builder, capabilities, config, kernel_download, rootfs_download from beetroot.builder import ( GAPPS_VENDOR_FLAGS, BootstrapError, @@ -53,23 +53,18 @@ def run( raise BootstrapError(f"fake failure on {self.fail_on} (exit {self.fail_exit})") -# The real daemon-preflight callable, captured before the autouse fixture -# below stubs the module attribute — so the few tests that exercise the real -# implementation can restore it. -_REAL_DAEMON_RESPONSIVE = builder._docker_daemon_responsive - - @pytest.fixture(autouse=True) def _daemon_up(monkeypatch: pytest.MonkeyPatch) -> None: """ Default the Docker-daemon preflight to "up" for every builder test. ``build_image`` grew a daemon preflight (#193); without this the daemonless - test host would short-circuit every existing build_image test. Tests that - exercise the daemon-down branch re-patch it to ``False`` themselves; tests - of the real probe restore :data:`_REAL_DAEMON_RESPONSIVE`. + test host would short-circuit every existing build_image test. The probe now + lives in :mod:`beetroot.capabilities` (issue #179), and builder calls it via + the module attribute, so patching it there covers every builder call site. + Tests that exercise the daemon-down branch re-patch it to ``False``. """ - monkeypatch.setattr(builder, "_docker_daemon_responsive", lambda: True) + monkeypatch.setattr(capabilities, "docker_daemon_responsive", lambda: True) class TestGappsVendorFlags: @@ -1715,33 +1710,6 @@ def test_sleep_invokes_time_sleep(self, monkeypatch: pytest.MonkeyPatch) -> None assert slept == [0.0] -class TestDockerDaemonResponsive: - def test_true_when_info_succeeds(self, monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.setattr(builder, "_docker_daemon_responsive", _REAL_DAEMON_RESPONSIVE) - monkeypatch.setattr( - "beetroot.builder.subprocess.run", - lambda *_a, **_k: subprocess.CompletedProcess(args=[], returncode=0), - ) - assert builder._docker_daemon_responsive() is True - - def test_false_when_info_nonzero(self, monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.setattr(builder, "_docker_daemon_responsive", _REAL_DAEMON_RESPONSIVE) - monkeypatch.setattr( - "beetroot.builder.subprocess.run", - lambda *_a, **_k: subprocess.CompletedProcess(args=[], returncode=1), - ) - assert builder._docker_daemon_responsive() is False - - def test_false_when_docker_missing(self, monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.setattr(builder, "_docker_daemon_responsive", _REAL_DAEMON_RESPONSIVE) - - def _boom(*_a: object, **_k: object) -> object: - raise FileNotFoundError - - monkeypatch.setattr("beetroot.builder.subprocess.run", _boom) - assert builder._docker_daemon_responsive() is False - - class TestVmBuildPreflight: def _cfg(self, tmp_path: Path, *, present: tuple[str, ...]) -> builder._RootfsConfig: # Build a _RootfsConfig whose static-binary paths point under tmp_path; @@ -1771,7 +1739,7 @@ def _ready( # noqa: PLR0913 # test helper; each kwarg toggles one preflight br cfg = self._cfg(tmp_path, present=present) monkeypatch.setattr(builder._RootfsConfig, "from_env", lambda **_k: cfg) monkeypatch.setattr("beetroot.builder.shutil.which", which or (lambda _n: "/usr/bin/found")) - monkeypatch.setattr(builder, "_docker_daemon_responsive", lambda: daemon) + monkeypatch.setattr(capabilities, "docker_daemon_responsive", lambda: daemon) # The bake's root-privilege preflight (#231) — default to root so the # other branches stay isolated; root-specific tests override ``euid``. monkeypatch.setattr("beetroot.builder.os.geteuid", lambda: euid) @@ -1885,7 +1853,7 @@ class TestBuildImageDaemonPreflight: """issue #193: ``beetroot build`` runs a Docker-daemon preflight.""" def test_daemon_down_raises_friendly_error(self, monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.setattr(builder, "_docker_daemon_responsive", lambda: False) + monkeypatch.setattr(capabilities, "docker_daemon_responsive", lambda: False) runner = FakeRunner() with pytest.raises(BootstrapError, match="Docker daemon") as exc: build_image(runner=runner) @@ -1894,7 +1862,7 @@ def test_daemon_down_raises_friendly_error(self, monkeypatch: pytest.MonkeyPatch assert runner.calls == [] def test_daemon_up_proceeds_to_build(self, monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.setattr(builder, "_docker_daemon_responsive", lambda: True) + monkeypatch.setattr(capabilities, "docker_daemon_responsive", lambda: True) runner = FakeRunner() build_image(runner=runner) # rm, clone, patch, build all run when the daemon is up. diff --git a/tests/test_capabilities.py b/tests/test_capabilities.py index c52e5ca..372df66 100644 --- a/tests/test_capabilities.py +++ b/tests/test_capabilities.py @@ -10,6 +10,7 @@ from __future__ import annotations import json +import subprocess from unittest import mock from typer.testing import CliRunner @@ -30,30 +31,39 @@ def _by_mode(results: list[capabilities.ModeSupport]) -> dict[str, capabilities. # ---- _redroid_host -------------------------------------------------------- -def test_redroid_host_ready_with_docker_is_supported() -> None: - r = capabilities._redroid_host(_binder("ready"), docker=True) +def test_redroid_host_ready_with_docker_daemon_is_supported() -> None: + r = capabilities._redroid_host(_binder("ready"), docker=True, docker_daemon=True) assert r.status == "supported" +def test_redroid_host_ready_docker_cli_but_daemon_down_needs_setup() -> None: + # issue #179: CLI present but ``dockerd`` not answering is NOT "supported" — + # redroid can't boot a container without a live daemon. + r = capabilities._redroid_host(_binder("ready"), docker=True, docker_daemon=False) + assert r.status == "needs-setup" + assert "daemon is not responding" in r.reason + assert r.remedy == "start the Docker daemon" + + def test_redroid_host_ready_without_docker_needs_setup() -> None: - r = capabilities._redroid_host(_binder("ready"), docker=False) + r = capabilities._redroid_host(_binder("ready"), docker=False, docker_daemon=False) assert r.status == "needs-setup" assert "Docker" in r.remedy def test_redroid_host_loadable_needs_setup() -> None: - r = capabilities._redroid_host(_binder("loadable"), docker=True) + r = capabilities._redroid_host(_binder("loadable"), docker=True, docker_daemon=True) assert r.status == "needs-setup" assert r.remedy == "remedy-loadable" def test_redroid_host_unsupported() -> None: - r = capabilities._redroid_host(_binder("unsupported"), docker=True) + r = capabilities._redroid_host(_binder("unsupported"), docker=True, docker_daemon=True) assert r.status == "unsupported" def test_redroid_host_unknown() -> None: - r = capabilities._redroid_host(_binder("unknown"), docker=True) + r = capabilities._redroid_host(_binder("unknown"), docker=True, docker_daemon=True) assert r.status == "unknown" @@ -112,7 +122,12 @@ def test_adb_adopt_with_adb_supported() -> None: def test_classify_modes_returns_all_four_in_order() -> None: results = capabilities.classify_modes( - binder=_binder("ready"), kvm=True, qemu_present=True, docker=True, adb_present=True + binder=_binder("ready"), + kvm=True, + qemu_present=True, + docker=True, + docker_daemon=True, + adb_present=True, ) assert [r.mode for r in results] == [ capabilities.MODE_REDROID_HOST, @@ -123,6 +138,40 @@ def test_classify_modes_returns_all_four_in_order() -> None: assert all(r.status == "supported" for r in results) +def test_classify_modes_binder_ready_daemon_up_redroid_supported() -> None: + # issue #179: the redroid host verdict must gate on daemon liveness, so a + # ready binder + live daemon is the only path to "supported". + results = _by_mode( + capabilities.classify_modes( + binder=_binder("ready"), + kvm=False, + qemu_present=False, + docker=True, + docker_daemon=True, + adb_present=False, + ) + ) + assert results[capabilities.MODE_REDROID_HOST].status == "supported" + + +def test_classify_modes_binder_ready_daemon_down_redroid_needs_setup() -> None: + # issue #179: the misleading case — CLI present, binder ready, but the daemon + # is stopped. Must be needs-setup with the daemon-start remedy, not supported. + results = _by_mode( + capabilities.classify_modes( + binder=_binder("ready"), + kvm=False, + qemu_present=False, + docker=True, + docker_daemon=False, + adb_present=False, + ) + ) + redroid = results[capabilities.MODE_REDROID_HOST] + assert redroid.status == "needs-setup" + assert redroid.remedy == "start the Docker daemon" + + def test_classify_modes_binderless_kvmless_host_only_tcg_and_adb() -> None: # This is *this* environment's profile (CONFIG_ANDROID_BINDER_IPC unset, # no /dev/kvm, qemu+adb installed) — the exact case that misled us before. @@ -132,6 +181,7 @@ def test_classify_modes_binderless_kvmless_host_only_tcg_and_adb() -> None: kvm=False, qemu_present=True, docker=True, + docker_daemon=True, adb_present=True, ) ) @@ -149,12 +199,60 @@ def test_survey_wires_probes_into_classifier() -> None: mock.patch("beetroot.hostcheck.binder_status", return_value=_binder("ready")), mock.patch("beetroot.vm.qemu.detect_accel", return_value="kvm"), mock.patch("shutil.which", return_value="/usr/bin/x"), + mock.patch.object(capabilities, "docker_daemon_responsive", return_value=True), ): results = capabilities.survey() assert len(results) == 4 assert _by_mode(results)[capabilities.MODE_VM_KVM].status == "supported" +def test_survey_threads_live_docker_daemon_into_redroid_verdict() -> None: + # issue #179: with the CLI present + binder ready, a *live* daemon must land + # the redroid host verdict on "supported". + with ( + mock.patch("beetroot.hostcheck.binder_status", return_value=_binder("ready")), + mock.patch("beetroot.vm.qemu.detect_accel", return_value="tcg"), + mock.patch("shutil.which", return_value="/usr/bin/x"), + mock.patch.object(capabilities, "docker_daemon_responsive", return_value=True) as probe, + ): + results = _by_mode(capabilities.survey()) + probe.assert_called_once() + assert results[capabilities.MODE_REDROID_HOST].status == "supported" + + +def test_survey_threads_dead_docker_daemon_into_redroid_verdict() -> None: + # issue #179: CLI present + binder ready but the daemon probe fails → the + # threaded ``False`` must demote the redroid verdict to needs-setup. + with ( + mock.patch("beetroot.hostcheck.binder_status", return_value=_binder("ready")), + mock.patch("beetroot.vm.qemu.detect_accel", return_value="tcg"), + mock.patch("shutil.which", return_value="/usr/bin/x"), + mock.patch.object(capabilities, "docker_daemon_responsive", return_value=False) as probe, + ): + results = _by_mode(capabilities.survey()) + probe.assert_called_once() + redroid = results[capabilities.MODE_REDROID_HOST] + assert redroid.status == "needs-setup" + assert redroid.remedy == "start the Docker daemon" + + +def test_survey_skips_daemon_probe_when_docker_cli_absent() -> None: + # Without the CLI there's no daemon to probe — the classifier ignores the + # flag on that path anyway, and short-circuit avoids a pointless subprocess. + def fake_which(name: str) -> str | None: + return "/usr/bin/qemu" if name.startswith("qemu") else None + + with ( + mock.patch("beetroot.hostcheck.binder_status", return_value=_binder("ready")), + mock.patch("beetroot.vm.qemu.detect_accel", return_value="tcg"), + mock.patch("shutil.which", side_effect=fake_which), + mock.patch.object(capabilities, "docker_daemon_responsive", return_value=True) as probe, + ): + results = _by_mode(capabilities.survey()) + probe.assert_not_called() + assert results[capabilities.MODE_REDROID_HOST].status == "needs-setup" + + def test_survey_detects_tcg_only_host() -> None: def fake_which(name: str) -> str | None: return "/usr/bin/qemu" if name.startswith("qemu") else None @@ -182,6 +280,29 @@ def test_survey_accepts_injected_settings() -> None: assert len(results) == 4 +# ---- docker_daemon_responsive (relocated from builder, issue #179) -------- + + +class TestDockerDaemonResponsive: + def test_true_when_info_succeeds(self) -> None: + with mock.patch( + "beetroot.capabilities.subprocess.run", + return_value=subprocess.CompletedProcess(args=[], returncode=0), + ): + assert capabilities.docker_daemon_responsive() is True + + def test_false_when_info_nonzero(self) -> None: + with mock.patch( + "beetroot.capabilities.subprocess.run", + return_value=subprocess.CompletedProcess(args=[], returncode=1), + ): + assert capabilities.docker_daemon_responsive() is False + + def test_false_when_docker_missing(self) -> None: + with mock.patch("beetroot.capabilities.subprocess.run", side_effect=FileNotFoundError): + assert capabilities.docker_daemon_responsive() is False + + # ---- CLI verb ------------------------------------------------------------- diff --git a/tests/test_vm_backend.py b/tests/test_vm_backend.py index bf1ae4b..13e55c9 100644 --- a/tests/test_vm_backend.py +++ b/tests/test_vm_backend.py @@ -8,7 +8,7 @@ import pytest -from beetroot import api, config, frida_download, registry +from beetroot import api, capabilities, config, frida_download, registry from beetroot.backends import vm as vm_backend from beetroot.settings import Settings from beetroot.vm import boot_cache, qemu @@ -307,7 +307,9 @@ def _start(_self: object, argv: list[str]) -> int: return 1234 monkeypatch.setattr(qemu.QemuProcess, "start", _start) - monkeypatch.setattr(vm_backend.VmDeviceBackend, "_wait_for_adb_connect", lambda _self, *_a: None) + monkeypatch.setattr( + vm_backend.VmDeviceBackend, "_wait_for_adb_connect", lambda _self, *_a: None + ) backend.up() assert launched["argv"][0] == "qemu-system-x86_64" @@ -353,7 +355,9 @@ def _start(_self: object, argv: list[str]) -> int: return 1 monkeypatch.setattr(qemu.QemuProcess, "start", _start) - monkeypatch.setattr(vm_backend.VmDeviceBackend, "_wait_for_adb_connect", lambda _self, *_a: None) + monkeypatch.setattr( + vm_backend.VmDeviceBackend, "_wait_for_adb_connect", lambda _self, *_a: None + ) backend.up() argv = launched["argv"] assert argv[0] == "qemu-system-x86_64" @@ -426,7 +430,9 @@ def _cached_backend( ) backend = _make_backend(tmp_path, cfg=cfg) monkeypatch.setattr(qemu, "detect_accel", lambda _req: "tcg") - monkeypatch.setattr(vm_backend.VmDeviceBackend, "_wait_for_adb_connect", lambda _self, *_a: None) + monkeypatch.setattr( + vm_backend.VmDeviceBackend, "_wait_for_adb_connect", lambda _self, *_a: None + ) # Default: the guest boots (cold path gates savevm on this). The # not-booted branch is exercised explicitly below. monkeypatch.setattr( @@ -876,7 +882,9 @@ def _backend_with_rootfs( def _silence_launch(self, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr(qemu, "detect_accel", lambda _req: "tcg") monkeypatch.setattr(qemu.QemuProcess, "start", lambda _self, _argv: 1) - monkeypatch.setattr(vm_backend.VmDeviceBackend, "_wait_for_adb_connect", lambda _self, *_a: None) + monkeypatch.setattr( + vm_backend.VmDeviceBackend, "_wait_for_adb_connect", lambda _self, *_a: None + ) def test_up_warns_on_version_mismatch( self, @@ -1070,7 +1078,9 @@ def test_up_does_not_emit_inert_warning( backend = _make_backend(tmp_path, cfg=cfg) monkeypatch.setattr(qemu, "detect_accel", lambda _req: "tcg") monkeypatch.setattr(qemu.QemuProcess, "start", lambda _self, _argv: 1) - monkeypatch.setattr(vm_backend.VmDeviceBackend, "_wait_for_adb_connect", lambda _self, *_a: None) + monkeypatch.setattr( + vm_backend.VmDeviceBackend, "_wait_for_adb_connect", lambda _self, *_a: None + ) backend.up() err = capsys.readouterr().err assert "android.gapps: full" not in err @@ -1350,29 +1360,108 @@ def _run(cmd: list[str], **_k: object) -> subprocess.CompletedProcess[str]: # --------------------------------------------------------------------------- +def _healthy_adb_run(*_args: object, **_kwargs: object) -> subprocess.CompletedProcess[str]: + """Stub ``subprocess.run`` so ``_check_adb_connect`` reports a clean attach.""" + return subprocess.CompletedProcess(args=[], returncode=0, stdout="connected", stderr="") + + class TestHealth: - def test_health_includes_vm_rows(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + def test_health_happy_path_qemu_artifacts_and_adb_connect( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + # qemu on PATH + kernel/rootfs staged + a healthy adb connect ⇒ every row + # passes, and it's an ``adb.connect`` (connect-then-verify) row, not the + # USB-style always-listed ``adb.serial`` that false-fails a fresh VM (#164). + kernel, rootfs = _stage_artifacts(monkeypatch, tmp_path) + del kernel, rootfs backend = _make_backend(tmp_path) monkeypatch.setattr(qemu.QemuProcess, "is_running", lambda _self: True) monkeypatch.setattr(qemu, "detect_accel", lambda _req: "kvm") - # adb_device_health shells out — stub adb absent so it returns skip rows. - monkeypatch.setattr(shutil, "which", lambda _n: None) + monkeypatch.setattr(shutil, "which", lambda _n: "/usr/bin/found") + connect_calls: list[list[str]] = [] + + def _record_adb_run( + cmd: list[str], *_a: object, **_k: object + ) -> subprocess.CompletedProcess[str]: + connect_calls.append(list(cmd)) + return _healthy_adb_run() + + monkeypatch.setattr(subprocess, "run", _record_adb_run) rows = backend.health() assert rows["vm.process"].status == "pass" assert rows["vm.accel"].status == "pass" assert "near-native" in (rows["vm.accel"].reason or "") - # The shared adb.serial row is present (uniform check names). - assert "adb.serial" in rows - # Frida can never pass on the network-isolated guest (#44), so the - # handshake row is omitted rather than a permanent fail. + assert rows["vm.qemu"].status == "pass" + assert rows["vm.artifacts"].status == "pass" + # #164: a healthy VM from a fresh adb-server lifetime must pass via a + # connect-then-verify row, and the misleading serial-listed row is gone. + assert rows["adb.connect"].status == "pass" + assert "adb.serial" not in rows + # The connect row must target *this* instance's forwarded adb endpoint — + # a bare `adb devices` scan (the old serial-listed row) would not connect. + assert any( + c[:2] == ["adb", "connect"] and c[-1] == backend.adb_address for c in connect_calls + ), connect_calls + # Frida can never pass on the network-isolated guest (#44), and the guest + # runs plain redroid with no Magisk (#163) — both row families stay absent. assert "frida.handshake" not in rows - # The guest runs plain redroid with no Magisk (#163), so the magisk - # rows are dropped rather than reported as a permanent fail. assert not any(name.startswith("magisk.") for name in rows) + def test_health_qemu_missing_fails_with_install_remedy( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + # #191: qemu-system not on PATH ⇒ vm.qemu fails carrying the shared + # modes install remedy (a green vm.accel alone must not read as ready). + kernel, rootfs = _stage_artifacts(monkeypatch, tmp_path) + del kernel, rootfs + backend = _make_backend(tmp_path) + monkeypatch.setattr(qemu.QemuProcess, "is_running", lambda _self: True) + monkeypatch.setattr(qemu, "detect_accel", lambda _req: "tcg") + monkeypatch.setattr(shutil, "which", lambda _n: None) + rows = backend.health() + assert rows["vm.qemu"].status == "fail" + assert capabilities._QEMU_INSTALL in (rows["vm.qemu"].reason or "") + + def test_health_artifacts_missing_fails_with_build_hint( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + # #191: qemu present but the guest kernel/rootfs was never built ⇒ + # vm.artifacts fails, naming the missing artifact + the build hint. + monkeypatch.setattr(vm_backend, "settings", Settings(vm_kernel="", vm_rootfs="")) + backend = _make_backend(tmp_path) + monkeypatch.setattr(qemu.QemuProcess, "is_running", lambda _self: True) + monkeypatch.setattr(qemu, "detect_accel", lambda _req: "tcg") + monkeypatch.setattr(shutil, "which", lambda _n: "/usr/bin/qemu-system-x86_64") + rows = backend.health() + assert rows["vm.qemu"].status == "pass" + assert rows["vm.artifacts"].status == "fail" + assert "kernel" in (rows["vm.artifacts"].reason or "") + assert "beetroot build --vm-kernel" in (rows["vm.artifacts"].reason or "") + + def test_health_adb_absent_skips_connect_row( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + # adb not installed ⇒ the connect row skips (not fails), so a host with no + # adb doesn't false-fail doctor on connectivity it can't even attempt. + kernel, rootfs = _stage_artifacts(monkeypatch, tmp_path) + del kernel, rootfs + backend = _make_backend(tmp_path) + monkeypatch.setattr(qemu.QemuProcess, "is_running", lambda _self: True) + monkeypatch.setattr(qemu, "detect_accel", lambda _req: "kvm") + + def _which(name: str) -> str | None: + # qemu present (so vm.qemu passes) but adb absent (connect skips). + return None if name == "adb" else "/usr/bin/found" + + monkeypatch.setattr(shutil, "which", _which) + rows = backend.health() + assert rows["adb.connect"].status == "skip" + def test_health_process_down_fails( self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: + kernel, rootfs = _stage_artifacts(monkeypatch, tmp_path) + del kernel, rootfs backend = _make_backend(tmp_path) monkeypatch.setattr(qemu.QemuProcess, "is_running", lambda _self: False) monkeypatch.setattr(qemu, "detect_accel", lambda _req: "tcg")