From eab9d15130d8f579ec94960d4b98fc29027c62b3 Mon Sep 17 00:00:00 2001 From: didericis Date: Fri, 24 Jul 2026 15:58:23 -0400 Subject: [PATCH] refactor(docker): fold run_docker into backend/docker/util; drop the docker_cmd shim docker_cmd.py existed as a top-level module solely so the orchestrator's docker components could share run_docker without dragging the (then-heavy) backend layer in. Now that backend/__init__ and backend/docker/__init__ are thin, importing backend.docker.util costs 6 modules instead of 76, so the shim's whole reason to exist is gone. Move run_docker into backend/docker/util.py (where the other docker subprocess primitives live) and delete docker_cmd.py. Backend siblings import it via `from .util import run_docker`; the two docker-specific orchestrator modules (docker_broker, rotate_ca) import `from ..backend.docker.util import run_docker`. No import cycle (backend.docker.util pulls nothing from orchestrator); orchestrator.__main__ stays lean at 28 modules. run_docker patch targets are unchanged (tests patch it in the importing module). Full unit suite green (2243). Co-Authored-By: Claude Opus 4.8 --- .../backend/docker/consolidated_launch.py | 2 +- bot_bottle/backend/docker/gateway.py | 2 +- .../backend/docker/gateway_provision.py | 2 +- bot_bottle/backend/docker/infra.py | 2 +- bot_bottle/backend/docker/util.py | 25 +++++++++++--- bot_bottle/docker_cmd.py | 34 ------------------- bot_bottle/orchestrator/docker_broker.py | 2 +- bot_bottle/orchestrator/rotate_ca.py | 2 +- 8 files changed, 26 insertions(+), 45 deletions(-) delete mode 100644 bot_bottle/docker_cmd.py diff --git a/bot_bottle/backend/docker/consolidated_launch.py b/bot_bottle/backend/docker/consolidated_launch.py index 2b7097fc..7efabc61 100644 --- a/bot_bottle/backend/docker/consolidated_launch.py +++ b/bot_bottle/backend/docker/consolidated_launch.py @@ -16,7 +16,7 @@ from __future__ import annotations from dataclasses import dataclass from ... import log -from ...docker_cmd import run_docker +from .util import run_docker from ...egress import EgressPlan from ...git_gate import GitGatePlan from ...orchestrator.client import OrchestratorClient diff --git a/bot_bottle/backend/docker/gateway.py b/bot_bottle/backend/docker/gateway.py index edea8e7d..c45ff8ee 100644 --- a/bot_bottle/backend/docker/gateway.py +++ b/bot_bottle/backend/docker/gateway.py @@ -5,7 +5,7 @@ import time from pathlib import Path from ...control_auth import ROLE_GATEWAY, mint -from ...docker_cmd import run_docker +from .util import run_docker from ...paths import ( CONTROL_AUTH_JWT_ENV, host_control_plane_token, diff --git a/bot_bottle/backend/docker/gateway_provision.py b/bot_bottle/backend/docker/gateway_provision.py index d1094dcc..3eaadcbc 100644 --- a/bot_bottle/backend/docker/gateway_provision.py +++ b/bot_bottle/backend/docker/gateway_provision.py @@ -17,7 +17,7 @@ from __future__ import annotations import re from typing import Protocol -from ...docker_cmd import run_docker +from .util import run_docker from ...git_gate import GitGatePlan, git_gate_render_provision # bottle ids index the gateway's per-bottle repo + creds dirs; they land in diff --git a/bot_bottle/backend/docker/infra.py b/bot_bottle/backend/docker/infra.py index 2471082e..5a1709eb 100644 --- a/bot_bottle/backend/docker/infra.py +++ b/bot_bottle/backend/docker/infra.py @@ -26,7 +26,7 @@ from pathlib import Path from ... import log from ...control_auth import ROLE_GATEWAY, mint -from ...docker_cmd import run_docker +from .util import run_docker from ...paths import ( CONTROL_AUTH_JWT_ENV, CONTROL_PLANE_TOKEN_ENV, diff --git a/bot_bottle/backend/docker/util.py b/bot_bottle/backend/docker/util.py index fb2091b8..b05cbd6e 100644 --- a/bot_bottle/backend/docker/util.py +++ b/bot_bottle/backend/docker/util.py @@ -1,6 +1,6 @@ -"""Docker host-side primitives used by DockerBottleBackend: probing -for docker on PATH, slugifying agent names, checking image/container -existence, and building images.""" +"""Docker host-side primitives used by DockerBottleBackend: the lean +`run_docker` subprocess wrapper, probing for docker on PATH, slugifying agent +names, checking image/container existence, and building images.""" from __future__ import annotations @@ -11,9 +11,24 @@ import shutil import subprocess from typing import Iterator -from ...docker_cmd import run_docker from ...log import die, info -# from ...workspace import WorkspacePlan + + +def run_docker( + argv: list[str], *, env: dict[str, str] | None = None, +) -> subprocess.CompletedProcess[str]: + """Run a `docker` command, capturing stdout/stderr as text. Never raises on + a non-zero exit — callers inspect `returncode` / `stderr` so they can stay + fail-closed or tolerate idempotent no-ops (e.g. removing an already-absent + container). + + `env` sets the child process environment — used to hand a secret to a bare + `--env NAME` flag (docker inherits its value from this process) so the value + never lands on argv or in `docker inspect`'s recorded command line.""" + return subprocess.run( + argv, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, + check=False, env=env, + ) # Cap on the suffix the container-name conflict logic will try before diff --git a/bot_bottle/docker_cmd.py b/bot_bottle/docker_cmd.py deleted file mode 100644 index 32a617ed..00000000 --- a/bot_bottle/docker_cmd.py +++ /dev/null @@ -1,34 +0,0 @@ -"""Lean, framework-free `docker` subprocess primitive. - -Deliberately a top-level module with a single stdlib import so it can be -reused from anywhere without cost. It is intentionally *not* placed in -`backend.docker.util`: importing that module runs `backend/__init__.py`, -which eagerly loads all three bottle backends (docker + firecracker + -macos) plus the manifest/egress/git-gate/supervise framework — ~76 modules -— which would drag the whole backend layer into the deliberately-lean -orchestrator. This primitive stays free of that so both the orchestrator's -docker components and (in time) `backend.docker.util` can share it.""" - -from __future__ import annotations - -import subprocess - - -def run_docker( - argv: list[str], *, env: dict[str, str] | None = None, -) -> subprocess.CompletedProcess[str]: - """Run a `docker` command, capturing stdout/stderr as text. Never raises - on a non-zero exit — callers inspect `returncode` / `stderr` so they can - stay fail-closed or tolerate idempotent no-ops (e.g. removing an - already-absent container). - - `env` sets the child process environment — used to hand a secret to a bare - `--env NAME` flag (docker inherits its value from this process) so the - value never lands on argv or in `docker inspect`'s recorded command line.""" - return subprocess.run( - argv, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, - check=False, env=env, - ) - - -__all__ = ["run_docker"] diff --git a/bot_bottle/orchestrator/docker_broker.py b/bot_bottle/orchestrator/docker_broker.py index e5af91c5..2e22845e 100644 --- a/bot_bottle/orchestrator/docker_broker.py +++ b/bot_bottle/orchestrator/docker_broker.py @@ -15,7 +15,7 @@ from __future__ import annotations import subprocess -from ..docker_cmd import run_docker +from ..backend.docker.util import run_docker from .broker import LaunchBroker, LaunchRequest CONTAINER_PREFIX = "bot-bottle-orch-" diff --git a/bot_bottle/orchestrator/rotate_ca.py b/bot_bottle/orchestrator/rotate_ca.py index 0cae3004..93030054 100644 --- a/bot_bottle/orchestrator/rotate_ca.py +++ b/bot_bottle/orchestrator/rotate_ca.py @@ -23,7 +23,7 @@ from __future__ import annotations import sys from pathlib import Path -from ..docker_cmd import run_docker +from ..backend.docker.util import run_docker from ..paths import host_gateway_ca_dir from ..gateway import GATEWAY_NAME, rotate_gateway_ca from ..backend.docker.infra import INFRA_NAME