diff --git a/bot_bottle/backend/__init__.py b/bot_bottle/backend/__init__.py index b00b3c6..e58b92d 100644 --- a/bot_bottle/backend/__init__.py +++ b/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 -# each backend module can pull BottleSpec / BottlePlan / BottleBackend -# via `from . import ...` without hitting a partially-initialized module. -from .docker import DockerBottleBackend # noqa: E402 # pylint: disable=wrong-import-position -from .firecracker import FirecrackerBottleBackend # noqa: E402 # pylint: disable=wrong-import-position -from .macos_container import MacosContainerBottleBackend # noqa: E402 # pylint: disable=wrong-import-position - -# Freezer is imported after the backend classes for the same reason: -# Freezer.commit_slug constructs ActiveAgent, which must be fully -# defined first. -from .freeze import CommitCancelled, Freezer, get_freezer # noqa: E402 # pylint: disable=wrong-import-position +# _BACKENDS is None until the first call to _get_backends(), at which +# point all three concrete backend classes are imported and instantiated. +# Keeping the imports out of module scope means that importing any +# backend sub-module (e.g. `backend.docker.util`) no longer drags the +# firecracker and macos-container implementations into memory. +# +# Tests may replace _BACKENDS with a {name: fake} dict via patch.object; +# _get_backends() returns the current module-level value as-is when it +# is not None, so test fakes take effect without triggering real imports. +_BACKENDS: dict[str, BottleBackend[Any, Any]] | None = None -# The dict is heterogeneous: each value is a BottleBackend specialized -# over its own plan type. Concrete plan types are erased here because -# the registry is selected at runtime and the CLI only needs the -# unparameterized methods (prepare → plan → launch(plan), cleanup, etc.). -_BACKENDS: dict[str, BottleBackend[Any, Any]] = { - "docker": DockerBottleBackend(), - "firecracker": FirecrackerBottleBackend(), - "macos-container": MacosContainerBottleBackend(), -} +def _get_backends() -> dict[str, BottleBackend[Any, Any]]: + """Return the registry of all backend instances, loading lazily on first call.""" + global _BACKENDS + if _BACKENDS is None: + from .docker import DockerBottleBackend + from .firecracker import FirecrackerBottleBackend + from .macos_container import 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: - known = ", ".join(sorted(_BACKENDS)) + backends = _get_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(): - if not has_backend(name): + backends = _get_backends() + 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 diff --git a/bot_bottle/backend/docker/util.py b/bot_bottle/backend/docker/util.py index 6a4abdd..e44d478 100644 --- a/bot_bottle/backend/docker/util.py +++ b/bot_bottle/backend/docker/util.py @@ -7,8 +7,9 @@ from __future__ import annotations 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 # from ...workspace import WorkspacePlan @@ -30,12 +31,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( - ["docker", "info", "--format", "{{json .Runtimes}}"], - capture_output=True, - text=True, - check=False, - ) + r = run_docker(["docker", "info", "--format", "{{json .Runtimes}}"]) return r.returncode == 0 and "runsc" in r.stdout @@ -49,20 +45,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=^$` so substring matches don't false-positive.""" - result = subprocess.run( - ["docker", "ps", "-a", "-q", "-f", f"name=^{name}$"], - capture_output=True, - text=True, - check=True, - ) - return bool(result.stdout.strip()) + result = run_docker(["docker", "ps", "-a", "-q", "-f", f"name=^{name}$"]) + return result.returncode == 0 and bool(result.stdout.strip()) def force_remove_container(name: str) -> None: @@ -70,12 +61,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( - ["docker", "rm", "-f", name], - stdout=subprocess.DEVNULL, - stderr=subprocess.DEVNULL, - check=False, - ) + run_docker(["docker", "rm", "-f", name]) def docker_exec_root(container: str, argv: list[str]) -> None: @@ -155,22 +141,10 @@ def build_image(ref: str, context: str, *, dockerfile: str = "") -> None: def commit_container(container_name: str, image_tag: str) -> None: """Run `docker commit ` to snapshot the running container's filesystem state as a local Docker image.""" - result = subprocess.run( - ["docker", "commit", container_name, image_tag], - capture_output=True, text=True, check=False, - ) + result = run_docker(["docker", "commit", container_name, image_tag]) if result.returncode != 0: die( f"docker commit {container_name!r} → {image_tag!r} failed: " f"{(result.stderr or '').strip() or ''}" ) 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 diff --git a/tests/unit/test_docker_util_image.py b/tests/unit/test_docker_util_image.py index 2231dc6..a48ee7e 100644 --- a/tests/unit/test_docker_util_image.py +++ b/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: