fix(docker): use run_docker in docker_exec, docker_cp, verify_agent_image

The rebase onto lazy-backend-imports converts existing helpers (image_exists,
container_exists, etc.) to run_docker; the three new functions added in this
branch still called subprocess.run directly. Switch them over for consistency.

bot_bottle/backend/__init__.py

@@ -572,28 +572,63 @@ class BottleBackend(ABC, Generic[PlanT, CleanupT]):
         Not called by the launch path or the test suite."""
 
 
-# Import concrete backend classes AFTER the base types are defined, so
+# _BACKENDS is None until the first call to _get_backends(), at which
-# each backend module can pull BottleSpec / BottlePlan / BottleBackend
+# point all three concrete backend classes are imported and instantiated.
-# via `from . import ...` without hitting a partially-initialized module.
+# Keeping the imports out of module scope means that importing any
-from .docker import DockerBottleBackend  # noqa: E402  # pylint: disable=wrong-import-position
+# backend sub-module (e.g. `backend.docker.util`) no longer drags the
-from .firecracker import FirecrackerBottleBackend  # noqa: E402  # pylint: disable=wrong-import-position
+# firecracker and macos-container implementations into memory.
-from .macos_container import MacosContainerBottleBackend  # noqa: E402  # pylint: disable=wrong-import-position
+#
-
+# Tests may replace _BACKENDS with a {name: fake} dict via patch.object;
-# Freezer is imported after the backend classes for the same reason:
+# _get_backends() returns the current module-level value as-is when it
-# Freezer.commit_slug constructs ActiveAgent, which must be fully
+# is not None, so test fakes take effect without triggering real imports.
-# defined first.
+_BACKENDS: dict[str, BottleBackend[Any, Any]] | None = None
-from .freeze import CommitCancelled, Freezer, get_freezer  # noqa: E402  # pylint: disable=wrong-import-position
 
 
-# The dict is heterogeneous: each value is a BottleBackend specialized
+def _get_backends() -> dict[str, BottleBackend[Any, Any]]:
-# over its own plan type. Concrete plan types are erased here because
+    """Return the registry of all backend instances, loading lazily on first call."""
-# the registry is selected at runtime and the CLI only needs the
+    global _BACKENDS  # pylint: disable=global-statement
-# unparameterized methods (prepare → plan → launch(plan), cleanup, etc.).
+    if _BACKENDS is None:
-_BACKENDS: dict[str, BottleBackend[Any, Any]] = {
+        from .docker import DockerBottleBackend
-    "docker": DockerBottleBackend(),
+        from .firecracker import FirecrackerBottleBackend
-    "firecracker": FirecrackerBottleBackend(),
+        from .macos_container import MacosContainerBottleBackend
-    "macos-container": MacosContainerBottleBackend(),
+        _BACKENDS = {
-}
+            "docker": DockerBottleBackend(),
+            "firecracker": FirecrackerBottleBackend(),
+            "macos-container": MacosContainerBottleBackend(),
+        }
+    return _BACKENDS
+
+
+def __getattr__(name: str) -> Any:
+    """Lazily surface concrete backend classes and freeze symbols at the
+    package level so existing `from bot_bottle.backend import X` and
+    `patch.object(backend_mod, X, ...)` call-sites keep working without
+    forcing an import of every backend at module-init time."""
+    if name == "DockerBottleBackend":
+        from .docker import DockerBottleBackend
+        globals()[name] = DockerBottleBackend
+        return DockerBottleBackend
+    if name == "FirecrackerBottleBackend":
+        from .firecracker import FirecrackerBottleBackend
+        globals()[name] = FirecrackerBottleBackend
+        return FirecrackerBottleBackend
+    if name == "MacosContainerBottleBackend":
+        from .macos_container import MacosContainerBottleBackend
+        globals()[name] = MacosContainerBottleBackend
+        return MacosContainerBottleBackend
+    if name == "CommitCancelled":
+        from .freeze import CommitCancelled
+        globals()[name] = CommitCancelled
+        return CommitCancelled
+    if name == "Freezer":
+        from .freeze import Freezer
+        globals()[name] = Freezer
+        return Freezer
+    if name == "get_freezer":
+        from .freeze import get_freezer
+        globals()[name] = get_freezer
+        return get_freezer
+    raise AttributeError(f"module {__name__!r} has no attribute {name!r}")
 
 
 def get_bottle_backend(
@@ -611,10 +646,11 @@ def get_bottle_backend(
     Dies with a pointer at the known backends if the chosen name
     isn't implemented."""
     resolved = name or os.environ.get("BOT_BOTTLE_BACKEND") or _default_backend_name()
-    if resolved not in _BACKENDS:
+    backends = _get_backends()
-        known = ", ".join(sorted(_BACKENDS))
+    if resolved not in backends:
+        known = ", ".join(sorted(backends))
         die(f"unknown backend {resolved!r}; known backends: {known}")
-    return _BACKENDS[resolved]
+    return backends[resolved]
 
 
 def _default_backend_name() -> str:
@@ -624,16 +660,17 @@ def _default_backend_name() -> str:
     # `firecracker` binary isn't installed yet: selecting it here routes
     # start through firecracker's preflight, which prints an install
     # pointer, instead of silently falling back to docker.
+    from .firecracker import FirecrackerBottleBackend
     if FirecrackerBottleBackend.is_host_capable():
         return "firecracker"
     return "docker"
 
 
 def known_backend_names() -> tuple[str, ...]:
-    """Sorted tuple of all backend keys in `_BACKENDS`. Used by
+    """Sorted tuple of all backend keys in `_get_backends()`. Used by
     argparse (`--backend` choices) and the dashboard's backend
     picker."""
-    return tuple(sorted(_BACKENDS))
+    return tuple(sorted(_get_backends()))
 
 
 def has_backend(name: str) -> bool:
@@ -645,9 +682,10 @@ def has_backend(name: str) -> bool:
 
     Returns False for unknown names so callers can pass
     arbitrary input without separate validation."""
-    if name not in _BACKENDS:
+    backends = _get_backends()
+    if name not in backends:
         return False
-    return _BACKENDS[name].is_available()
+    return backends[name].is_available()
 
 
 def enumerate_active_agents() -> list[ActiveAgent]:
@@ -663,10 +701,11 @@ def enumerate_active_agents() -> list[ActiveAgent]:
     deterministic tiebreaker. Agents with missing metadata
     (`started_at == ""`) sort first."""
     out: list[ActiveAgent] = []
-    for name in known_backend_names():
+    backends = _get_backends()
-        if not has_backend(name):
+    for name in sorted(backends):
+        if not backends[name].is_available():
             continue
-        out.extend(_BACKENDS[name].enumerate_active())
+        out.extend(backends[name].enumerate_active())
     out.sort(key=lambda a: (a.started_at, a.slug))
     return out
 
@@ -678,12 +717,12 @@ __all__ = [
     "BottleCleanupPlan",
     "BottlePlan",
     "BottleSpec",
-    "CommitCancelled",
+    "CommitCancelled",  # pylint: disable=undefined-all-variable
     "ExecResult",
-    "Freezer",
+    "Freezer",  # pylint: disable=undefined-all-variable
     "enumerate_active_agents",
     "get_bottle_backend",
-    "get_freezer",
+    "get_freezer",  # pylint: disable=undefined-all-variable
     "has_backend",
     "known_backend_names",
 ]

bot_bottle/backend/docker/util.py

@@ -8,7 +8,7 @@ import os
 import re
 import shutil
 import subprocess
-from typing import Iterable, Iterator
+from typing import Iterator
 
 from ...docker_cmd import run_docker
 from ...log import die, info
@@ -32,12 +32,7 @@ def container_name_candidates(base: str) -> Iterator[str]:
 def runsc_available() -> bool:
     """Return True if the Docker daemon has the gVisor (`runsc`) runtime
     registered. Called once per prepare; the result lives on the plan."""
-    r = subprocess.run(
+    r = run_docker(["docker", "info", "--format", "{{json .Runtimes}}"])
-        ["docker", "info", "--format", "{{json .Runtimes}}"],
-        capture_output=True,
-        text=True,
-        check=False,
-    )
     return r.returncode == 0 and "runsc" in r.stdout
 
 
@@ -51,20 +46,15 @@ def require_docker() -> None:
 
 
 def image_exists(ref: str) -> bool:
-    return _silent_run(["docker", "image", "inspect", ref]) == 0
+    return run_docker(["docker", "image", "inspect", ref]).returncode == 0
 
 
 def container_exists(name: str) -> bool:
     """Returns True if a container (running or stopped) with the given
     name exists. Uses `docker ps -a -q -f name=^<name>$` so substring
     matches don't false-positive."""
-    result = subprocess.run(
+    result = run_docker(["docker", "ps", "-a", "-q", "-f", f"name=^{name}$"])
-        ["docker", "ps", "-a", "-q", "-f", f"name=^{name}$"],
+    return result.returncode == 0 and bool(result.stdout.strip())
-        capture_output=True,
-        text=True,
-        check=True,
-    )
-    return bool(result.stdout.strip())
 
 
 def force_remove_container(name: str) -> None:
@@ -72,12 +62,7 @@ def force_remove_container(name: str) -> None:
     doesn't — and the rm itself is best-effort (errors swallowed) so
     this is safe to register as a teardown callback."""
     if container_exists(name):
-        subprocess.run(
+        run_docker(["docker", "rm", "-f", name])
-            ["docker", "rm", "-f", name],
-            stdout=subprocess.DEVNULL,
-            stderr=subprocess.DEVNULL,
-            check=False,
-        )
 
 
 def docker_exec_root(container: str, argv: list[str]) -> None:
@@ -205,22 +190,10 @@ def verify_agent_image(image: str, argv: tuple[str, ...]) -> None:
 def commit_container(container_name: str, image_tag: str) -> None:
     """Run `docker commit <container_name> <image_tag>` to snapshot the
     running container's filesystem state as a local Docker image."""
-    result = subprocess.run(
+    result = run_docker(["docker", "commit", container_name, image_tag])
-        ["docker", "commit", container_name, image_tag],
-        capture_output=True, text=True, check=False,
-    )
     if result.returncode != 0:
         die(
             f"docker commit {container_name!r} → {image_tag!r} failed: "
             f"{(result.stderr or '').strip() or '<no stderr>'}"
         )
     info(f"committed {container_name!r} → {image_tag!r}")
-
-
-def _silent_run(cmd: Iterable[str]) -> int:
-    return subprocess.run(
-        list(cmd),
-        stdout=subprocess.DEVNULL,
-        stderr=subprocess.DEVNULL,
-        check=False,
-    ).returncode

tests/unit/test_docker_util_image.py

@@ -29,7 +29,7 @@ def _fail(stderr: str = "boom") -> subprocess.CompletedProcess:  # type: ignore
 class TestCommitContainer(unittest.TestCase):
     def test_runs_docker_commit(self):
         with patch.object(
-            docker_mod.subprocess, "run", return_value=_ok(),
+            docker_mod, "run_docker", return_value=_ok(),
         ) as run, patch.object(docker_mod, "info"):
             docker_mod.commit_container(
                 "bot-bottle-dev-abc12",
@@ -47,7 +47,7 @@ class TestCommitContainer(unittest.TestCase):
 
     def test_dies_on_docker_commit_failure(self):
         with patch.object(
-            docker_mod.subprocess, "run", return_value=_fail("No such container"),
+            docker_mod, "run_docker", return_value=_fail("No such container"),
         ), patch.object(
             docker_mod, "die", side_effect=SystemExit("die"),
         ) as die:
@@ -58,7 +58,7 @@ class TestCommitContainer(unittest.TestCase):
 
     def test_die_message_includes_image_tag(self):
         with patch.object(
-            docker_mod.subprocess, "run", return_value=_fail("boom"),
+            docker_mod, "run_docker", return_value=_fail("boom"),
         ), patch.object(
             docker_mod, "die", side_effect=SystemExit("die"),
         ) as die: