From 31a5ec2fc87584f15a273024b91f4340a2459596 Mon Sep 17 00:00:00 2001 From: claude Date: Mon, 20 Jul 2026 18:59:24 +0000 Subject: [PATCH 1/8] docs(prd): consolidate infra backend for docker Adds PRD for collapsing the Docker backend from two containers (gateway + orchestrator) to a single bot-bottle-infra container, restructuring Dockerfile.infra as the shared base, and extracting duplicated CA polling / teardown / provision helpers into a shared backend utility module. Closes #431 --- ...rd-new-consolidate-docker-infra-backend.md | 155 ++++++++++++++++++ 1 file changed, 155 insertions(+) create mode 100644 docs/prds/prd-new-consolidate-docker-infra-backend.md diff --git a/docs/prds/prd-new-consolidate-docker-infra-backend.md b/docs/prds/prd-new-consolidate-docker-infra-backend.md new file mode 100644 index 0000000..e50eb92 --- /dev/null +++ b/docs/prds/prd-new-consolidate-docker-infra-backend.md @@ -0,0 +1,155 @@ +# PRD prd-new: Consolidate infra backend for Docker + +- **Status:** Draft +- **Author:** Claude +- **Created:** 2026-07-20 +- **Issue:** #431 + +## Summary + +The Docker backend runs two containers — `bot-bottle-orch-gateway` (gateway +data plane) and `bot-bottle-orchestrator` (control plane) — where the +macOS and Firecracker backends already run a single combined infra +unit. This PRD collapses Docker to the same model: one `bot-bottle-infra` +container running both processes under the `gateway_init` supervise tree, a +restructured `Dockerfile.infra` as the shared gateway+orchestrator base, +and a handful of extracted shared utilities (CA cert polling, teardown +sequence, launch skeleton) that are currently duplicated across all three +`consolidated_launch.py` files. + +## Goals / success criteria + +- Docker backend starts exactly one infra container instead of two. +- `Dockerfile.infra` is the shared base image (gateway + orchestrator, no + buildah); the Firecracker image layers buildah on top of it. +- The orchestrator process runs under the `gateway_init` supervise tree + inside the combined container (one PID-1, one restart/health surface). +- CA cert polling, the teardown sequence, and the shared launch skeleton + (ensure-infra → register → provision → return context) live in a single + shared module; all three backends import from it. +- No functional change to macOS or Firecracker launch paths. + +## Non-goals + +- Changing per-bottle isolation — agents stay one-VM/container-each. +- Consolidating transport implementations (`DockerGatewayTransport`, + `AppleGatewayTransport`, `SshGatewayTransport`) — these are already the + right abstraction boundary. +- macOS DHCP-inversion of registration order — irreducible backend + difference, stays as-is. +- Any changes to the orchestrator RPC protocol or the attribution model. + +## Design + +### Dockerfile restructuring + +**Current shape:** + +- `Dockerfile.gateway` — data plane (mitmproxy, gitleaks, git, openssh, + supervise daemons) +- `Dockerfile.orchestrator` — control plane (python:3.12-slim + bot_bottle + package; stdlib-only, no third-party deps) +- `Dockerfile.infra` — Firecracker only: `FROM bot-bottle-gateway` + + buildah + `COPY --from bot-bottle-orchestrator` + +**New shape:** + +- `Dockerfile.gateway` — unchanged +- `Dockerfile.orchestrator` — unchanged (single definition of orchestrator + content; both Docker infra and Firecracker infra `COPY --from` it) +- `Dockerfile.infra` — **shared base**: `FROM bot-bottle-gateway` + `COPY + --from bot-bottle-orchestrator` (no buildah — Docker infra image) +- `Dockerfile.infra.fc` — Firecracker only: `FROM bot-bottle-infra` + + buildah/crun/netavark/aardvark-dns (layered on the shared base, same net + result as today) + +The comment in `Dockerfile.infra` that says "the docker backend keeps +orchestrator + gateway as separate images; this combined image exists only +for the Firecracker single-VM cut" is removed. + +### Orchestrator in the supervise tree + +`gateway_init` already supervises the data-plane daemons (egress, git-http, +supervise-MCP). The orchestrator control plane is added as another supervised +process: `python3 -m bot_bottle.orchestrator --host 0.0.0.0 --port +--broker stub`. + +The orchestrator source is bind-mounted (`/app` → repo root, as today) so +dev live-reload still works. `source_hash`-based container recreation in +`OrchestratorService.ensure_running` continues to apply — a code change +recreates the combined infra container, which bounces both gateway and +orchestrator. This is acceptable: the docker backend is a dev/legacy target +where in-flight egress connections across a code deploy are not a hard +requirement. + +### `OrchestratorService` changes + +`OrchestratorService` currently starts two containers in sequence: gateway +first (`DockerGateway.ensure_running`), then orchestrator. After this PRD: + +- Single `docker run` of `bot-bottle-infra:latest` +- Container name: `bot-bottle-infra` (replaces `bot-bottle-orch-gateway` + + `bot-bottle-orchestrator`) +- Published ports: `127.0.0.1:{port}:{port}` for the control plane (same as + today) +- Bind mounts: repo root + host root (same as today) +- `DockerGateway` becomes an implementation detail of `OrchestratorService` + rather than a separately started container; the gateway image name + (`GATEWAY_IMAGE`) is no longer referenced at runtime, only at build time + for the `Dockerfile.infra` base + +The `_gateway()` / `ensure_running` two-step in `OrchestratorService` is +replaced by a single `_run_infra_container()`. + +### Shared backend utilities + +Three items are duplicated across +`backend/docker/consolidated_launch.py`, +`backend/macos_container/consolidated_launch.py`, and +`backend/firecracker/consolidated_launch.py`: + +1. **CA cert polling loop** — `deadline = time.monotonic() + timeout; while + ...: try fetch CA; sleep` — extracted to + `backend/consolidated_util.py:poll_ca_cert(transport, *, timeout)`. + +2. **Teardown sequence** — `OrchestratorClient(url).teardown_bottle(id)` + + `deprovision_git_gate(transport, id)` — extracted to + `backend/consolidated_util.py:teardown_consolidated(url, transport, + bottle_id)`. + +3. **Launch skeleton** — all three follow: ensure-infra → allocate/register + → provision git-gate → fetch CA cert → return launch context. The macOS + inversion (agent starts before registration, source IP from DHCP) is the + only deviation. Extract a shared `_provision_bottle(transport, bottle_id, + plan, orchestrator_url)` helper covering the register → provision → + return-token steps; the backends keep their own `launch_consolidated` + wrappers for the before/after (infra-ensure + agent-start + IP + allocation), calling the shared helper. + +The new `backend/consolidated_util.py` module holds only backend-neutral, +transport-agnostic logic. All three backends import from it. + +## Implementation chunks + +1. **(this PR)** Dockerfile restructuring: rename current `Dockerfile.infra` + content to `Dockerfile.infra.fc`; write new `Dockerfile.infra` as + gateway+orchestrator base. Update Firecracker image-build references from + `Dockerfile.infra` → `Dockerfile.infra.fc`. + +2. Add orchestrator process to `gateway_init` supervise tree. + +3. Collapse `OrchestratorService` to a single-container start; rename + container from `bot-bottle-orch-gateway`/`bot-bottle-orchestrator` → + `bot-bottle-infra`; update image name constant. + +4. Extract `backend/consolidated_util.py` with `poll_ca_cert`, + `teardown_consolidated`, and `_provision_bottle`; update all three + `consolidated_launch.py` files to import from it. + +5. Update tests that reference the old container names or two-container + startup sequence. + +## Open questions + +None — the supervise-tree approach and shared Dockerfile layering were +confirmed in issue #431. -- 2.52.0 From 2f45f5afecf21724ecd75dbed0c3a7cdcdf7de82 Mon Sep 17 00:00:00 2001 From: claude Date: Mon, 20 Jul 2026 19:35:33 +0000 Subject: [PATCH 2/8] feat(docker): consolidate to single infra container under gateway_init supervise tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Collapses the two-container Docker model (gateway + orchestrator) into one bot-bottle-infra container, matching the macOS and Firecracker backends. - Dockerfile.infra: now a shared gateway+orchestrator base (COPY bot_bottle from orchestrator build, no CMD override) - Dockerfile.infra.fc: new Firecracker-specific layer (buildah/crun/netavark) - gateway_init: adds orchestrator daemon with _OPT_IN_DAEMONS gating so it only starts when BOT_BOTTLE_GATEWAY_DAEMONS explicitly includes it - orchestrator/lifecycle: OrchestratorService manages one infra container; builds orchestrator (intermediate) then infra; live source bind-mounted at /bot-bottle-src with PYTHONPATH so the subprocess uses the checkout - backend/consolidated_util: extracts provision_bottle + teardown_consolidated shared across all three backends; removes duplication in docker/fc/macos consolidated_launch modules - firecracker/infra_vm: builds four images (orchestrator→gateway→infra→infra.fc) - All unit tests updated and passing (1878 tests) - PRD status: Draft → Active --- Dockerfile.infra | 49 +-- Dockerfile.infra.fc | 23 ++ bot_bottle/backend/consolidated_util.py | 60 ++++ .../backend/docker/consolidated_launch.py | 81 ++--- .../firecracker/consolidated_launch.py | 26 +- .../backend/firecracker/infra_artifact.py | 2 +- bot_bottle/backend/firecracker/infra_vm.py | 13 +- .../macos_container/consolidated_launch.py | 24 +- bot_bottle/gateway_init.py | 28 +- bot_bottle/orchestrator/lifecycle.py | 319 +++++++++--------- ...rd-new-consolidate-docker-infra-backend.md | 2 +- tests/unit/test_consolidated_launch.py | 7 +- tests/unit/test_infra_artifact.py | 2 +- tests/unit/test_macos_consolidated_launch.py | 11 +- tests/unit/test_orchestrator_lifecycle.py | 131 +++---- 15 files changed, 381 insertions(+), 397 deletions(-) create mode 100644 Dockerfile.infra.fc create mode 100644 bot_bottle/backend/consolidated_util.py diff --git a/Dockerfile.infra b/Dockerfile.infra index d66c81f..0bd1e76 100644 --- a/Dockerfile.infra +++ b/Dockerfile.infra @@ -1,45 +1,22 @@ -# Firecracker single infra-VM image (PRD 0070 Stage B). +# Shared infra image: gateway data plane + orchestrator control plane. # -# The per-host infra VM runs the orchestrator control plane, the gateway -# data plane, AND builds agent images (buildah) — all in one microVM (see -# backend/firecracker/infra_vm.py). It composes: -# * FROM the gateway image (mitmproxy / git / gitleaks / supervise + the -# flat daemon modules) — now trixie-based, so buildah 1.39 is available; -# * `COPY --from` the orchestrator image's content (the single definition -# of the control-plane payload — see Dockerfile.orchestrator), so this -# VM and the docker backend share one orchestrator definition; and -# * buildah, installed HERE only (the docker orchestrator/gateway images -# never carry it). +# Used directly by the Docker backend (run as one `bot-bottle-infra` +# container, replacing the prior two-container split). The Firecracker +# backend extends this via Dockerfile.infra.fc, adding buildah/crun/ +# netavark for in-VM agent-image building. +# +# Dockerfile.orchestrator is the single definition of the orchestrator +# content (the lean `bot_bottle` package on python:3.12-slim). Both this +# image and Dockerfile.infra.fc pull it in via `COPY --from`. # # multi-`FROM` can't union two bases (that's multi-stage, not multiple # inheritance), so the orchestrator content is pulled in via `COPY --from` # rather than a second base. Both images share the trixie `python:3.12-slim` # base, so the copy is clean (same python; future installed deps copy too). -# -# The docker backend keeps orchestrator + gateway as separate images; this -# combined image exists only for the Firecracker single-VM cut. Splitting a -# service back into its own VM later is a routing change, not a repackaging -# (PRD 0070's "secret concentration"; a disposable builder can boot from -# this same image on its own TAP). FROM bot-bottle-gateway:latest -# --- in-VM agent-image builder (PRD 0069 Stage 3) ------------------- -# The Firecracker backend builds users' agent Dockerfiles *inside this VM* -# with buildah (rootless, daemonless) instead of on the host — no host -# Docker daemon, no root-equivalent `docker` group. `crun` is the OCI -# runtime; `netavark` + `aardvark-dns` are the network backend for `FROM` -# pulls + `RUN` egress. Requires the trixie base (buildah 1.39: bookworm's -# 1.28 can't parse Dockerfile heredocs that agent images use). -RUN apt-get update \ - && apt-get install -y --no-install-recommends \ - buildah crun netavark aardvark-dns \ - && rm -rf /var/lib/apt/lists/* -# vfs + chroot: buildah works as root in the bare microVM (no -# fuse-overlayfs / overlay module / subuid maps). Matches image_builder. -ENV STORAGE_DRIVER=vfs \ - BUILDAH_ISOLATION=chroot - -# The orchestrator content, pulled from its single definition. The gateway -# image already has the flat daemon modules under /app; this adds the full -# `bot_bottle` package so `python3 -m bot_bottle.orchestrator` resolves. +# The orchestrator content, from its single definition. The gateway image +# already has the flat daemon modules under /app; this adds the full +# `bot_bottle` package so `python3 -m bot_bottle.orchestrator` resolves — +# used by gateway_init when BOT_BOTTLE_GATEWAY_DAEMONS includes `orchestrator`. COPY --from=bot-bottle-orchestrator:latest /app/bot_bottle /app/bot_bottle diff --git a/Dockerfile.infra.fc b/Dockerfile.infra.fc new file mode 100644 index 0000000..422fd60 --- /dev/null +++ b/Dockerfile.infra.fc @@ -0,0 +1,23 @@ +# Firecracker infra VM image (PRD 0070 Stage B). +# +# Extends the shared infra base (Dockerfile.infra: gateway + orchestrator +# control plane) with the in-VM agent-image builder. The Firecracker backend +# builds users' agent Dockerfiles *inside this VM* with buildah (rootless, +# daemonless) instead of on the host — no host Docker daemon, no +# root-equivalent `docker` group. +# +# Requires the trixie base from bot-bottle-gateway (buildah 1.39: bookworm's +# 1.28 can't parse Dockerfile heredocs that agent images use). +# +# `crun` is the OCI runtime; `netavark` + `aardvark-dns` are the network +# backend for `FROM` pulls + `RUN` egress. `vfs` + `chroot`: buildah works +# as root in the bare microVM (no fuse-overlayfs / overlay module / subuid +# maps). Matches image_builder. +FROM bot-bottle-infra:latest + +RUN apt-get update \ + && apt-get install -y --no-install-recommends \ + buildah crun netavark aardvark-dns \ + && rm -rf /var/lib/apt/lists/* +ENV STORAGE_DRIVER=vfs \ + BUILDAH_ISOLATION=chroot diff --git a/bot_bottle/backend/consolidated_util.py b/bot_bottle/backend/consolidated_util.py new file mode 100644 index 0000000..354212e --- /dev/null +++ b/bot_bottle/backend/consolidated_util.py @@ -0,0 +1,60 @@ +"""Shared helpers for the consolidated launch sequence (PRD 0070). + +Logic that was duplicated across the docker, macos_container, and +firecracker consolidated_launch modules — extracted so each backend +imports it rather than re-implementing it. +""" + +from __future__ import annotations + +from ..egress import EgressPlan +from ..git_gate import GitGatePlan +from ..orchestrator.client import OrchestratorClient +from ..orchestrator.registration import registration_inputs +from .docker.gateway_provision import GatewayTransport, deprovision_git_gate, provision_git_gate + + +def provision_bottle( + client: OrchestratorClient, + source_ip: str, + egress_plan: EgressPlan, + git_gate_plan: GitGatePlan, + transport: GatewayTransport, + *, + image_ref: str = "", + tokens: dict[str, str] | None = None, +): + """Register the bottle and provision its git-gate state. Rolls back the + registration if provisioning fails so no orphan is left. Returns the + `RegisteredBottle` from the orchestrator.""" + inputs = registration_inputs(egress_plan) + reg = client.register_bottle( + source_ip, image_ref=image_ref, policy=inputs.policy, + metadata=inputs.metadata, tokens=tokens, + ) + try: + provision_git_gate(transport, reg.bottle_id, git_gate_plan) + except Exception: + client.teardown_bottle(reg.bottle_id) + raise + return reg + + +def teardown_consolidated( + bottle_id: str, + transport: GatewayTransport, + *, + orchestrator_url: str, + timeout: float | None = None, +) -> None: + """Deregister the bottle and remove its git-gate state. Both steps are + idempotent so this is safe from a cleanup trap.""" + from ..orchestrator.config_store import DEFAULT_TEARDOWN_TIMEOUT_SECONDS + OrchestratorClient( + orchestrator_url, + timeout=timeout if timeout is not None else DEFAULT_TEARDOWN_TIMEOUT_SECONDS, + ).teardown_bottle(bottle_id) + deprovision_git_gate(transport, bottle_id) + + +__all__ = ["provision_bottle", "teardown_consolidated"] diff --git a/bot_bottle/backend/docker/consolidated_launch.py b/bot_bottle/backend/docker/consolidated_launch.py index 383f99d..15555b5 100644 --- a/bot_bottle/backend/docker/consolidated_launch.py +++ b/bot_bottle/backend/docker/consolidated_launch.py @@ -1,19 +1,13 @@ """Consolidated bottle launch sequence for the docker backend (PRD 0070). -Composes the orchestrator primitives into the register/teardown sequence that -replaces the per-bottle gateway: +Composes the orchestrator primitives into the register/teardown sequence: - 1. ensure the orchestrator control plane + shared gateway are up; - 2. allocate the bottle a pinned source IP on the gateway network (the - attribution key), skipping the gateway's own address + live bottles; - 3. register it (egress policy blob + slug metadata) → bottle id + identity - token; - 4. provision its git-gate repos/creds into the running gateway. + 1. ensure the single infra container (control plane + gateway) is up; + 2. allocate the bottle a pinned source IP on the gateway network; + 3. register it and provision its git-gate repos/creds into the gateway. -It returns a `LaunchContext` with everything the agent container needs to -attach — network, pinned IP, the gateway's address (its proxy target), the -orchestrator URL, and the identity token. The agent `docker run` itself is -the backend's job (it owns provider provisioning); this owns the +Returns a `LaunchContext` with everything the agent container needs to +attach. The agent `docker run` itself is the backend's job; this owns the orchestrator-facing wiring so that sequence stays testable in isolation. """ @@ -25,15 +19,12 @@ from ...docker_cmd import run_docker from ...egress import EgressPlan from ...git_gate import GitGatePlan from ...orchestrator.client import OrchestratorClient -from ...orchestrator.gateway import GATEWAY_NAME, GATEWAY_NETWORK -from ...orchestrator.lifecycle import OrchestratorService -from ...orchestrator.registration import registration_inputs +from ...orchestrator.gateway import GATEWAY_NETWORK +from ...orchestrator.lifecycle import INFRA_NAME, OrchestratorService +from ..consolidated_util import provision_bottle +from ..consolidated_util import teardown_consolidated as _teardown_util +from .gateway_provision import DockerGatewayTransport from .gateway_net import next_free_ip -from .gateway_provision import ( - DockerGatewayTransport, - deprovision_git_gate, - provision_git_gate, -) class ConsolidatedLaunchError(RuntimeError): @@ -75,24 +66,21 @@ def _container_ip(name: str, network: str) -> str: ip = proc.stdout.strip() if proc.returncode != 0 or not ip: raise ConsolidatedLaunchError( - f"gateway {name} has no address on {network}: {proc.stderr.strip()}" + f"container {name} has no address on {network}: {proc.stderr.strip()}" ) return ip def _network_container_ips(network: str) -> list[str]: """Every address currently assigned on the gateway network — the ground - truth for "in use": the gateway + orchestrator infrastructure containers - and every live agent. Read from the network so a new bottle can't collide - with anything actually attached (a registry-only view would miss the - orchestrator/gateway containers).""" + truth for "in use": the infra container and every live agent. Read from + the network so a new bottle can't collide with anything actually attached.""" proc = run_docker([ "docker", "network", "inspect", "--format", "{{range .Containers}}{{.IPv4Address}} {{end}}", network, ]) ips: list[str] = [] for entry in proc.stdout.split(): - # entries look like "172.20.0.2/16" — keep the address. ips.append(entry.split("/", 1)[0]) return ips @@ -104,33 +92,24 @@ def launch_consolidated( image_ref: str = "", tokens: dict[str, str] | None = None, service: OrchestratorService | None = None, - gateway_name: str = GATEWAY_NAME, + infra_name: str = INFRA_NAME, network: str = GATEWAY_NETWORK, ) -> LaunchContext: - """Ensure the orchestrator + gateway are up, allocate + register the - bottle, and provision its git-gate state. Returns the agent's attach - context. Raises `ConsolidatedLaunchError` (or the primitives' own errors) - if any step fails — the caller tears down on failure.""" + """Ensure the infra container is up, allocate + register the bottle, and + provision its git-gate state. Returns the agent's attach context.""" service = service or OrchestratorService() url = service.ensure_running() client = OrchestratorClient(url) cidr = _network_cidr(network) - gateway_ip = _container_ip(gateway_name, network) + gateway_ip = _container_ip(infra_name, network) source_ip = next_free_ip(cidr, _network_container_ips(network)) - inputs = registration_inputs(egress_plan) - reg = client.register_bottle( - source_ip, image_ref=image_ref, policy=inputs.policy, - metadata=inputs.metadata, tokens=tokens, + transport = DockerGatewayTransport(infra_name) + reg = provision_bottle( + client, source_ip, egress_plan, git_gate_plan, transport, + image_ref=image_ref, tokens=tokens, ) - try: - provision_git_gate( - DockerGatewayTransport(gateway_name), reg.bottle_id, git_gate_plan) - except Exception: - # Roll the registration back so a provisioning failure leaves no orphan. - client.teardown_bottle(reg.bottle_id) - raise return LaunchContext( bottle_id=reg.bottle_id, identity_token=reg.identity_token, @@ -142,20 +121,12 @@ def launch_consolidated( def teardown_consolidated( - bottle_id: str, - *, - orchestrator_url: str, - gateway_name: str = GATEWAY_NAME, + bottle_id: str, *, orchestrator_url: str, infra_name: str = INFRA_NAME, timeout: float | None = None, ) -> None: - """Deregister the bottle and remove its git-gate state from the gateway. - Both steps are idempotent so this is safe from a cleanup trap.""" - from ...orchestrator.config_store import DEFAULT_TEARDOWN_TIMEOUT_SECONDS - OrchestratorClient( - orchestrator_url, - timeout=timeout if timeout is not None else DEFAULT_TEARDOWN_TIMEOUT_SECONDS, - ).teardown_bottle(bottle_id) - deprovision_git_gate(DockerGatewayTransport(gateway_name), bottle_id) + """Deregister the bottle and remove its git-gate state. Idempotent.""" + _teardown_util(bottle_id, DockerGatewayTransport(infra_name), + orchestrator_url=orchestrator_url, timeout=timeout) __all__ = [ diff --git a/bot_bottle/backend/firecracker/consolidated_launch.py b/bot_bottle/backend/firecracker/consolidated_launch.py index f9c9db4..3e25917 100644 --- a/bot_bottle/backend/firecracker/consolidated_launch.py +++ b/bot_bottle/backend/firecracker/consolidated_launch.py @@ -33,8 +33,7 @@ from ...orchestrator.client import OrchestratorClient from ...orchestrator.lifecycle import ( OrchestratorStartError, # re-exported so callers can catch it ) -from ...orchestrator.registration import registration_inputs -from ..docker.gateway_provision import deprovision_git_gate, provision_git_gate +from ..consolidated_util import provision_bottle, teardown_consolidated as _teardown_util from . import infra_vm @@ -68,18 +67,11 @@ def launch_consolidated( url = infra.control_plane_url client = OrchestratorClient(url) - inputs = registration_inputs(egress_plan) - reg = client.register_bottle( - guest_ip, image_ref=image_ref, policy=inputs.policy, - metadata=inputs.metadata, tokens=tokens, + transport = infra_vm.gateway_transport() + reg = provision_bottle( + client, guest_ip, egress_plan, git_gate_plan, transport, + image_ref=image_ref, tokens=tokens, ) - try: - provision_git_gate( - infra_vm.gateway_transport(), reg.bottle_id, git_gate_plan) - except Exception: - client.teardown_bottle(reg.bottle_id) - raise - # The shared gateway CA every agent on this host trusts for TLS # interception — fetched from the infra VM over SSH. return LaunchContext( @@ -98,12 +90,8 @@ def teardown_consolidated( VM. Both steps are idempotent so this is safe from a cleanup trap. Does NOT stop the infra VM — it's a persistent per-host singleton shared by every bottle.""" - from ...orchestrator.config_store import DEFAULT_TEARDOWN_TIMEOUT_SECONDS - OrchestratorClient( - orchestrator_url, - timeout=timeout if timeout is not None else DEFAULT_TEARDOWN_TIMEOUT_SECONDS, - ).teardown_bottle(bottle_id) - deprovision_git_gate(infra_vm.gateway_transport(), bottle_id) + _teardown_util(bottle_id, infra_vm.gateway_transport(), + orchestrator_url=orchestrator_url, timeout=timeout) __all__ = [ diff --git a/bot_bottle/backend/firecracker/infra_artifact.py b/bot_bottle/backend/firecracker/infra_artifact.py index 26d02da..5c8ecb2 100644 --- a/bot_bottle/backend/firecracker/infra_artifact.py +++ b/bot_bottle/backend/firecracker/infra_artifact.py @@ -41,7 +41,7 @@ from . import util _ARTIFACT_FORMAT = "1" _REPO_ROOT = Path(__file__).resolve().parents[3] -_DOCKERFILES = ("Dockerfile.orchestrator", "Dockerfile.gateway", "Dockerfile.infra") +_DOCKERFILES = ("Dockerfile.orchestrator", "Dockerfile.gateway", "Dockerfile.infra", "Dockerfile.infra.fc") _DEFAULT_BASE = "https://gitea.dideric.is" _DEFAULT_OWNER = "didericis" diff --git a/bot_bottle/backend/firecracker/infra_vm.py b/bot_bottle/backend/firecracker/infra_vm.py index c031994..ba3bdd4 100644 --- a/bot_bottle/backend/firecracker/infra_vm.py +++ b/bot_bottle/backend/firecracker/infra_vm.py @@ -125,16 +125,19 @@ def ensure_built() -> None: def build_infra_images_with_docker() -> None: - """Build the three fixed images from source with host Docker: orchestrator, - gateway, then the combined infra image (`COPY --from` orchestrator, `FROM` - gateway). The launch host uses this only in `BOT_BOTTLE_INFRA_BUILD=local` - mode; `publish_infra` uses it off-host to produce the published artifact.""" + """Build the four fixed images from source with host Docker: orchestrator, + gateway, the shared infra base (Dockerfile.infra), then the Firecracker + infra image (Dockerfile.infra.fc: FROM infra + buildah). The launch host + uses this only in `BOT_BOTTLE_INFRA_BUILD=local` mode; `publish_infra` + uses it off-host to produce the published artifact.""" docker_mod.build_image( _ORCHESTRATOR_IMAGE, str(_REPO_ROOT), dockerfile="Dockerfile.orchestrator") docker_mod.build_image( _GATEWAY_IMAGE, str(_REPO_ROOT), dockerfile="Dockerfile.gateway") docker_mod.build_image( - _INFRA_IMAGE, str(_REPO_ROOT), dockerfile="Dockerfile.infra") + "bot-bottle-infra:latest", str(_REPO_ROOT), dockerfile="Dockerfile.infra") + docker_mod.build_image( + _INFRA_IMAGE, str(_REPO_ROOT), dockerfile="Dockerfile.infra.fc") def build_infra_rootfs_dir() -> Path: diff --git a/bot_bottle/backend/macos_container/consolidated_launch.py b/bot_bottle/backend/macos_container/consolidated_launch.py index be8ca8f..ebc97cb 100644 --- a/bot_bottle/backend/macos_container/consolidated_launch.py +++ b/bot_bottle/backend/macos_container/consolidated_launch.py @@ -38,8 +38,7 @@ from ...egress import EgressPlan from ...git_gate import GitGatePlan from ...log import info from ...orchestrator.client import OrchestratorClient, OrchestratorClientError -from ...orchestrator.registration import registration_inputs -from ..docker.gateway_provision import deprovision_git_gate, provision_git_gate +from ..consolidated_util import provision_bottle, teardown_consolidated as _teardown_util from . import util as container_mod from .enumerate import CONTAINER_NAME_PREFIX, EnumerationError, enumerate_active from .gateway import GATEWAY_NETWORK @@ -142,17 +141,10 @@ def register_agent( client.reconcile(live_source_ips(endpoint.network)) except (OrchestratorClientError, EnumerationError) as e: info(f"registry reconciliation skipped: {e}") - inputs = registration_inputs(egress_plan) - reg = client.register_bottle( - source_ip, image_ref=image_ref, policy=inputs.policy, - metadata=inputs.metadata, tokens=tokens, + reg = provision_bottle( + client, source_ip, egress_plan, git_gate_plan, AppleGatewayTransport(), + image_ref=image_ref, tokens=tokens, ) - try: - provision_git_gate(AppleGatewayTransport(), reg.bottle_id, git_gate_plan) - except Exception: - # Roll the registration back so a provisioning failure leaves no orphan. - client.teardown_bottle(reg.bottle_id) - raise return LaunchContext( bottle_id=reg.bottle_id, identity_token=reg.identity_token, @@ -169,12 +161,8 @@ def teardown_consolidated( """Deregister the bottle and remove its git-gate state from the gateway. Both steps are idempotent so this is safe from a cleanup trap. Does NOT stop the gateway — it's a persistent per-host singleton.""" - from ...orchestrator.config_store import DEFAULT_TEARDOWN_TIMEOUT_SECONDS - OrchestratorClient( - orchestrator_url, - timeout=timeout if timeout is not None else DEFAULT_TEARDOWN_TIMEOUT_SECONDS, - ).teardown_bottle(bottle_id) - deprovision_git_gate(AppleGatewayTransport(), bottle_id) + _teardown_util(bottle_id, AppleGatewayTransport(), + orchestrator_url=orchestrator_url, timeout=timeout) __all__ = [ diff --git a/bot_bottle/gateway_init.py b/bot_bottle/gateway_init.py index 083d7c8..799428e 100644 --- a/bot_bottle/gateway_init.py +++ b/bot_bottle/gateway_init.py @@ -61,6 +61,11 @@ class _DaemonSpec: _EGRESS_ONLY_ENV_PREFIXES: tuple[str, ...] = ("EGRESS_TOKEN_",) _READY_GATED_DAEMONS: tuple[str, ...] = ("git-gate", "git-http") +# Daemons that must be requested explicitly via BOT_BOTTLE_GATEWAY_DAEMONS +# and are NOT started in the default (env-var-unset) case. The orchestrator +# only runs in the combined infra container, never in a standalone gateway. +_OPT_IN_DAEMONS: frozenset[str] = frozenset({"orchestrator"}) + def _env_for_daemon(name: str, base_env: dict[str, str]) -> dict[str, str]: """Egress sees the full bundle env. Everyone else gets a copy @@ -75,7 +80,14 @@ def _env_for_daemon(name: str, base_env: dict[str, str]) -> dict[str, str]: } +# The orchestrator is listed first so it starts before the gateway daemons, +# giving the control plane a head start to accept /resolve calls. The gateway +# daemons tolerate early /resolve failures and retry per-request. _DAEMONS: tuple[_DaemonSpec, ...] = ( + _DaemonSpec("orchestrator", ( + "python3", "-m", "bot_bottle.orchestrator", + "--host", "0.0.0.0", "--port", "8099", "--broker", "stub", + )), _DaemonSpec("egress", ("/bin/sh", "/app/egress-entrypoint.sh")), _DaemonSpec("git-gate", ("/bin/sh", "/git-gate-entrypoint.sh")), _DaemonSpec("git-http", ("python3", "-m", "bot_bottle.git_http_backend")), @@ -103,18 +115,20 @@ def _selected_daemons( env: dict[str, str], all_daemons: Sequence[_DaemonSpec] | None = None, ) -> tuple[_DaemonSpec, ...]: - """Filter the daemon set by the BOT_BOTTLE_GATEWAY_DAEMONS env - var. Unknown names in the list are ignored — the caller is the - source of truth for which daemons are wired. + """Filter the daemon set by the BOT_BOTTLE_GATEWAY_DAEMONS env var. - `all_daemons` defaults to `_DAEMONS` resolved at call time (not - at definition time), so tests can monkey-patch the module-level - `_DAEMONS` and have the new value take effect.""" + When the var is unset/empty, return all non-opt-in daemons (the + standard gateway subset). Opt-in daemons (e.g. `orchestrator`) only + run when explicitly named — they never start in a plain gateway + container that doesn't set the env var. Unknown names are ignored. + + `all_daemons` defaults to `_DAEMONS` resolved at call time (not at + definition time), so tests can pass a custom list.""" if all_daemons is None: all_daemons = _DAEMONS raw = env.get("BOT_BOTTLE_GATEWAY_DAEMONS", "").strip() if not raw: - return tuple(all_daemons) + return tuple(d for d in all_daemons if d.name not in _OPT_IN_DAEMONS) wanted = {n.strip() for n in raw.split(",") if n.strip()} return tuple(d for d in all_daemons if d.name in wanted) diff --git a/bot_bottle/orchestrator/lifecycle.py b/bot_bottle/orchestrator/lifecycle.py index f035686..7c06801 100644 --- a/bot_bottle/orchestrator/lifecycle.py +++ b/bot_bottle/orchestrator/lifecycle.py @@ -1,17 +1,15 @@ """Orchestrator + gateway lifecycle (PRD 0070, docker slice). -Runs the orchestrator control plane **as a container** on the shared gateway -network, alongside the gateway container. This is the PRD's "virtualize the -orchestrator": container↔container between the gateway and the orchestrator -avoids the host firewall (which drops container→host traffic), and the gateway -reaches the control plane by container name over docker DNS. The host CLI -reaches it via a published loopback port. +Runs both the orchestrator control plane and the gateway data plane inside +a single `bot-bottle-infra` container on the shared gateway network — +matching the structure already used by the macOS and Firecracker backends. +`gateway_init` is PID 1 and supervises both; the infra container is an +idempotent per-host singleton. -The orchestrator runs with the **register-only broker** — the *backend* -launches agent containers (compose), so the orchestrator needs no docker -socket. That keeps this control-plane container unprivileged; the host manages -both containers. `ensure_running` is an idempotent singleton (fixed container -names + the published port). +The combined container replaces the prior two-container split +(bot-bottle-orchestrator + bot-bottle-orch-gateway). The host CLI reaches +the control plane via a published loopback port; gateway daemons reach it +over 127.0.0.1 (same container). """ from __future__ import annotations @@ -26,49 +24,68 @@ from pathlib import Path from .. import log from ..docker_cmd import run_docker from ..paths import CONTROL_PLANE_TOKEN_ENV, bot_bottle_root, host_control_plane_token -from .gateway import GATEWAY_IMAGE, GATEWAY_NAME, GATEWAY_NETWORK, DockerGateway, GatewayError +from ..supervise import DB_PATH_IN_CONTAINER +from .gateway import ( + GATEWAY_CA_VOLUME, + GATEWAY_NETWORK, + GatewayError, + MITMPROXY_HOME, + _host_db_dir, +) DEFAULT_PORT = 8099 -ORCHESTRATOR_NAME = "bot-bottle-orchestrator" -ORCHESTRATOR_LABEL = "bot-bottle-orchestrator=1" -# The control-plane's own runtime image — lean (python + the stdlib-only -# `bot_bottle` package, bind-mounted at run time), distinct from the heavy -# gateway data-plane image it used to borrow (#384). Env override for -# operators pinning a published build. +DEFAULT_STARTUP_TIMEOUT_SECONDS = 45.0 + +INFRA_NAME = "bot-bottle-infra" +INFRA_LABEL = "bot-bottle-infra=1" +# The combined infra image: gateway data plane + orchestrator content. +# Built from Dockerfile.infra (FROM gateway + COPY --from orchestrator). +INFRA_IMAGE = os.environ.get("BOT_BOTTLE_INFRA_IMAGE", "bot-bottle-infra:latest") +INFRA_DOCKERFILE = "Dockerfile.infra" +# Baked as a container label so `ensure_running` can detect whether the +# running container is executing the current bind-mounted source. +INFRA_SOURCE_HASH_LABEL = "bot-bottle-infra-source-hash" + +# Orchestrator image: the single canonical definition of the control-plane +# content (lean: python:3.12-slim + bot_bottle package, no mitmproxy/git). +# Used as a build intermediate: `Dockerfile.infra` COPY --from this image. ORCHESTRATOR_IMAGE = os.environ.get( "BOT_BOTTLE_ORCHESTRATOR_IMAGE", "bot-bottle-orchestrator:latest" ) ORCHESTRATOR_DOCKERFILE = "Dockerfile.orchestrator" -# Baked onto the container as a label so `ensure_running` can tell whether the -# running process is executing the *current* bind-mounted source — see -# `source_hash`. -ORCHESTRATOR_SOURCE_HASH_LABEL = "bot-bottle-orchestrator-source-hash" -# The repo root is bind-mounted into the control-plane container so -# `python -m bot_bottle.orchestrator` resolves the package (the orchestrator -# is stdlib-only, so the lean orchestrator image's python is enough). -_REPO_ROOT = Path(__file__).resolve().parents[2] -_APP_DIR = "/app" +# The gateway daemons + orchestrator the infra container runs. +# BOT_BOTTLE_GATEWAY_DAEMONS listing `orchestrator` opts it in to +# gateway_init's supervise tree (see gateway_init._OPT_IN_DAEMONS). +_INFRA_DAEMONS = "egress,git-http,supervise,orchestrator" + +# The bind-mount path for the live control-plane source inside the +# container. Separate from /app so the gateway's baked scripts +# (egress_addon.py, egress-entrypoint.sh) are not overlaid. +_SRC_IN_CONTAINER = "/bot-bottle-src" +# Bot-bottle host-root bind-mount inside the container (DB + state). _ROOT_IN_CONTAINER = "/bot-bottle-root" +# The supervise daemon writes proposals into the host DB directory. +_SUPERVISE_DB_DIR_IN_CONTAINER = os.path.dirname(DB_PATH_IN_CONTAINER) + _HEALTH_POLL_SECONDS = 0.25 -DEFAULT_STARTUP_TIMEOUT_SECONDS = 45.0 _HEALTH_REQUEST_TIMEOUT_SECONDS = 1.0 +_REPO_ROOT = Path(__file__).resolve().parents[2] + class OrchestratorStartError(RuntimeError): - """The orchestrator container did not become healthy within the timeout.""" + """The infra container did not become healthy within the timeout.""" def source_hash(repo_root: Path) -> str: """Content hash of the orchestrator's bind-mounted Python source (the - `bot_bottle` package the control-plane process imports). This only - changes when the code that would actually run inside the container - changes — `ensure_running` recreates the container on a mismatch and - otherwise leaves a healthy one alone, so a bottle launch that isn't - accompanied by a code change doesn't restart the process and drop every - *other* active bottle's in-memory egress tokens (`Orchestrator._tokens` - in `service.py`, never persisted to disk by design).""" + `bot_bottle` package the control-plane process imports). Changes only + when the code that would actually run changes — `ensure_running` + recreates the container on a mismatch so a code change takes effect, + but leaves a healthy up-to-date container alone to preserve in-memory + egress tokens.""" h = hashlib.sha256() for path in sorted((repo_root / "bot_bottle").rglob("*.py")): h.update(str(path.relative_to(repo_root)).encode()) @@ -77,57 +94,37 @@ def source_hash(repo_root: Path) -> str: class OrchestratorService: - """Manages the orchestrator control-plane container + the shared gateway. + """Manages the single per-host infra container (control plane + gateway). Callers only need `ensure_running()` + `url`. - `orchestrator_name` / `orchestrator_label` let backends run independent - orchestrators on the same host without name collisions (e.g. the - Firecracker backend uses `bot-bottle-fc-orchestrator` alongside the Docker - backend's `bot-bottle-orchestrator`); `gateway_name` gives the paired - gateway container the same treatment (e.g. isolated integration tests - that can't share the production `GATEWAY_NAME` singleton). Subclass and - override `_gateway()` for anything `_gateway_image`/`gateway_name` can't - express (a genuinely backend-specific gateway variant).""" + `infra_name` / `infra_label` let backends run independent infra containers + on the same host without name collisions (e.g. isolated integration tests + that can't share the production INFRA_NAME singleton).""" def __init__( self, *, port: int = DEFAULT_PORT, network: str = GATEWAY_NETWORK, - image: str = ORCHESTRATOR_IMAGE, - gateway_image: str = GATEWAY_IMAGE, - gateway_name: str = GATEWAY_NAME, + image: str = INFRA_IMAGE, repo_root: Path = _REPO_ROOT, host_root: Path | None = None, - orchestrator_name: str = ORCHESTRATOR_NAME, - orchestrator_label: str = ORCHESTRATOR_LABEL, + infra_name: str = INFRA_NAME, + infra_label: str = INFRA_LABEL, ) -> None: self.port = port self.network = network - # Two distinct images (#384): `image` is the lean control-plane - # runtime this container runs; `_gateway_image` is the heavy egress / - # git-gate / supervise data plane the gateway container runs. They - # were one conflated image before the split. self.image = image - self._gateway_image = gateway_image - self._gateway_name = gateway_name self._repo_root = repo_root self._host_root = host_root or bot_bottle_root() - self._orchestrator_name = orchestrator_name - self._orchestrator_label = orchestrator_label + self._infra_name = infra_name + self._infra_label = infra_label @property def url(self) -> str: """Host-side control-plane URL (published loopback port).""" return f"http://127.0.0.1:{self.port}" - @property - def internal_url(self) -> str: - """Control-plane URL as the gateway container reaches it — by name over - docker DNS on the shared network. This is the gateway's - BOT_BOTTLE_ORCHESTRATOR_URL.""" - return f"http://{self._orchestrator_name}:{self.port}" - def is_healthy(self, *, timeout: float = _HEALTH_REQUEST_TIMEOUT_SECONDS) -> bool: try: with urllib.request.urlopen(f"{self.url}/health", timeout=timeout) as resp: @@ -139,139 +136,125 @@ class OrchestratorService: proc = run_docker(["docker", "ps", "--filter", f"name=^/{name}$", "--format", "{{.Names}}"]) return name in proc.stdout.split() - def _run_orchestrator_container(self, current_hash: str) -> None: - """Start the control-plane container (idempotent: clears a stale - fixed-name container first). Register-only broker → no docker socket. - Labels the container with `current_hash` so a later `ensure_running` - can detect a real code change (see `source_hash`).""" - run_docker(["docker", "rm", "--force", self._orchestrator_name]) - proc = run_docker([ - "docker", "run", "--detach", - "--name", self._orchestrator_name, - "--label", self._orchestrator_label, - "--label", f"{ORCHESTRATOR_SOURCE_HASH_LABEL}={current_hash}", - "--network", self.network, - # Host CLI reaches the control plane here; bound to loopback so it - # is not exposed on the host's external interfaces. NOTE: the - # container is still on `self.network` (the shared gateway network), - # so agents can reach it by container IP — which is exactly why the - # control plane requires the secret below rather than trusting the - # network boundary. - "--publish", f"127.0.0.1:{self.port}:{self.port}", - "--volume", f"{self._repo_root}:{_APP_DIR}:ro", - "--workdir", _APP_DIR, - # Persist the registry DB on the host (sole-owner: only the - # orchestrator opens bot-bottle.db). - "--volume", f"{self._host_root}:{_ROOT_IN_CONTAINER}", - "--env", f"BOT_BOTTLE_ROOT={_ROOT_IN_CONTAINER}", - # The control-plane secret it requires on every route but /health. - # Bare `--env NAME` → docker inherits the value from the run env - # below, so the secret never lands on argv / `docker inspect`. - "--env", CONTROL_PLANE_TOKEN_ENV, - "--entrypoint", "python3", - self.image, - "-m", "bot_bottle.orchestrator", - "--host", "0.0.0.0", "--port", str(self.port), "--broker", "stub", - ], env={**os.environ, CONTROL_PLANE_TOKEN_ENV: host_control_plane_token()}) - if proc.returncode != 0: - raise OrchestratorStartError( - f"orchestrator container failed to start: {proc.stderr.strip()}" - ) - - def _gateway(self) -> DockerGateway: - return DockerGateway( - self._gateway_image, - name=self._gateway_name, - network=self.network, - orchestrator_url=self.internal_url, - ) - - def _ensure_orchestrator_image(self) -> None: - """Build the lean control-plane image from `Dockerfile.orchestrator` - when it's missing (#384). Cheap — a `FROM python:*-slim` base with no - deps to install, so the layer cache makes rebuilds a no-op. Unlike the - gateway image this is build-if-missing, not build-every-time: the - control plane bind-mounts its source, so a code change is caught by the - source-hash recreate (below), not by an image rebuild.""" - if run_docker(["docker", "image", "inspect", self.image]).returncode == 0: - return - argv = ["docker", "build", "-t", self.image, - "-f", str(self._repo_root / ORCHESTRATOR_DOCKERFILE), - str(self._repo_root)] - if os.environ.get("BOT_BOTTLE_NO_CACHE"): - argv.insert(2, "--no-cache") - proc = run_docker(argv) - if proc.returncode != 0: - raise GatewayError( - f"orchestrator image build failed: {proc.stderr.strip()}" - ) - - def _orchestrator_source_current(self, current_hash: str) -> bool: - """True iff the running orchestrator container was created from the - *current* bind-mounted source. Mirrors `DockerGateway`'s - image-staleness check, but by content hash rather than image id since - the orchestrator runs bind-mounted source, not a built image.""" - if not self._container_running(self._orchestrator_name): + def _infra_source_current(self, current_hash: str) -> bool: + """True iff the running infra container was started from the current + bind-mounted source. Mirrors the macOS backend's `_source_current`.""" + if not self._container_running(self._infra_name): return False proc = run_docker([ "docker", "inspect", "--format", - "{{ index .Config.Labels \"" + ORCHESTRATOR_SOURCE_HASH_LABEL + "\" }}", - self._orchestrator_name, + "{{ index .Config.Labels \"" + INFRA_SOURCE_HASH_LABEL + "\" }}", + self._infra_name, ]) if proc.returncode != 0: - return True # can't compare -> don't churn a working container + return True # can't compare → don't churn a working container return proc.stdout.strip() == current_hash + def _ensure_network(self) -> None: + if run_docker(["docker", "network", "inspect", self.network]).returncode == 0: + return + proc = run_docker(["docker", "network", "create", self.network]) + if proc.returncode != 0 and "already exists" not in proc.stderr: + raise GatewayError( + f"gateway network {self.network} failed to create: {proc.stderr.strip()}" + ) + + def _build_images(self) -> None: + """Build the orchestrator image (build intermediate), then the infra + image. Both are cache-aware: a no-op when nothing changed.""" + for tag, dockerfile in ( + (ORCHESTRATOR_IMAGE, ORCHESTRATOR_DOCKERFILE), + (self.image, INFRA_DOCKERFILE), + ): + argv = ["docker", "build", "-t", tag, + "-f", str(self._repo_root / dockerfile), + str(self._repo_root)] + if os.environ.get("BOT_BOTTLE_NO_CACHE"): + argv.insert(2, "--no-cache") + proc = run_docker(argv) + if proc.returncode != 0: + raise GatewayError(f"{dockerfile} build failed: {proc.stderr.strip()}") + + def _run_infra_container(self, current_hash: str) -> None: + """Start the combined infra container (idempotent: clears a stale + fixed-name container first). Labels the container with `current_hash` + so a later `ensure_running` can detect a real code change.""" + self._ensure_network() + run_docker(["docker", "rm", "--force", self._infra_name]) + proc = run_docker([ + "docker", "run", "--detach", + "--name", self._infra_name, + "--label", self._infra_label, + "--label", f"{INFRA_SOURCE_HASH_LABEL}={current_hash}", + "--network", self.network, + # Host CLI reaches the control plane here (loopback only). + "--publish", f"127.0.0.1:{self.port}:{self.port}", + # Persist the mitmproxy CA so it survives container recreation. + "--volume", f"{GATEWAY_CA_VOLUME}:{MITMPROXY_HOME}", + # Shared supervise DB (same file the operator reads over HTTP). + "--volume", f"{_host_db_dir()}:{_SUPERVISE_DB_DIR_IN_CONTAINER}", + "--env", f"SUPERVISE_DB_PATH={DB_PATH_IN_CONTAINER}", + # Live control-plane source, mounted to a path that does not + # overlay the gateway's baked /app scripts. + "--volume", f"{self._repo_root}:{_SRC_IN_CONTAINER}:ro", + # PYTHONPATH lets the orchestrator (and other Python daemons) + # import the live source ahead of the installed package. + "--env", f"PYTHONPATH={_SRC_IN_CONTAINER}", + # Orchestrator registry DB on the host (sole writer: control plane). + "--volume", f"{self._host_root}:{_ROOT_IN_CONTAINER}", + "--env", f"BOT_BOTTLE_ROOT={_ROOT_IN_CONTAINER}", + # Control-plane secret: required by the orchestrator (to enforce) + # and by the gateway daemons (to present on /resolve calls). + "--env", CONTROL_PLANE_TOKEN_ENV, + # Gateway daemons reach the orchestrator over loopback. + "--env", f"BOT_BOTTLE_ORCHESTRATOR_URL=http://127.0.0.1:{self.port}", + # Opt the orchestrator into gateway_init's supervise tree. + "--env", f"BOT_BOTTLE_GATEWAY_DAEMONS={_INFRA_DAEMONS}", + self.image, + ], env={**os.environ, CONTROL_PLANE_TOKEN_ENV: host_control_plane_token()}) + if proc.returncode != 0: + raise OrchestratorStartError( + f"infra container failed to start: {proc.stderr.strip()}" + ) + def ensure_running( self, *, startup_timeout: float = DEFAULT_STARTUP_TIMEOUT_SECONDS, ) -> str: - """Ensure the control plane + shared gateway are up; return the host - control-plane URL. Idempotent — a healthy control plane running - current code and a running gateway are left untouched. Raises - `OrchestratorStartError` on timeout.""" - gateway = self._gateway() - gateway.ensure_built() # rebuild the bundle image on a source change - gateway.ensure_running() # creates the shared network + (re)starts gateway + """Ensure the infra container (control plane + gateway) is up; return + the host control-plane URL. Idempotent — a healthy container on current + source is left untouched. Raises `OrchestratorStartError` on timeout.""" + self._build_images() - # Recreate the orchestrator container only when its bind-mounted - # source has actually changed since it started — its Python process - # loaded that code at startup and won't reload, so a stale container - # would keep running OLD control-plane code. Recreating on *every* - # launch (the prior behaviour) would drop every other active - # bottle's in-memory egress tokens each time a new bottle starts, - # since the orchestrator process holds them only in memory (#381). current_hash = source_hash(self._repo_root) - if self.is_healthy() and self._orchestrator_source_current(current_hash): + if self.is_healthy() and self._infra_source_current(current_hash): return self.url - self._ensure_orchestrator_image() - log.info( - "starting orchestrator container", - context={"name": self._orchestrator_name}, - ) - self._run_orchestrator_container(current_hash) + log.info("starting infra container", context={"name": self._infra_name}) + self._run_infra_container(current_hash) deadline = time.monotonic() + startup_timeout while time.monotonic() < deadline: if self.is_healthy(): - log.info("orchestrator healthy", context={"url": self.url}) + log.info("infra container healthy", context={"url": self.url}) return self.url time.sleep(_HEALTH_POLL_SECONDS) raise OrchestratorStartError( - f"orchestrator at {self.url} did not become healthy within {startup_timeout:g}s" + f"infra container at {self.url} did not become healthy within {startup_timeout:g}s" ) def stop(self) -> None: - """Remove the orchestrator + gateway containers (idempotent).""" - run_docker(["docker", "rm", "--force", self._orchestrator_name]) - self._gateway().stop() + """Remove the infra container (idempotent).""" + run_docker(["docker", "rm", "--force", self._infra_name]) __all__ = [ "OrchestratorService", "OrchestratorStartError", - "ORCHESTRATOR_NAME", + "INFRA_NAME", + "INFRA_IMAGE", + "INFRA_SOURCE_HASH_LABEL", "ORCHESTRATOR_IMAGE", "DEFAULT_PORT", "DEFAULT_STARTUP_TIMEOUT_SECONDS", + "source_hash", ] diff --git a/docs/prds/prd-new-consolidate-docker-infra-backend.md b/docs/prds/prd-new-consolidate-docker-infra-backend.md index e50eb92..b7e0976 100644 --- a/docs/prds/prd-new-consolidate-docker-infra-backend.md +++ b/docs/prds/prd-new-consolidate-docker-infra-backend.md @@ -1,6 +1,6 @@ # PRD prd-new: Consolidate infra backend for Docker -- **Status:** Draft +- **Status:** Active - **Author:** Claude - **Created:** 2026-07-20 - **Issue:** #431 diff --git a/tests/unit/test_consolidated_launch.py b/tests/unit/test_consolidated_launch.py index ecd8bbd..f31eb90 100644 --- a/tests/unit/test_consolidated_launch.py +++ b/tests/unit/test_consolidated_launch.py @@ -15,6 +15,7 @@ from bot_bottle.git_gate import GitGatePlan from bot_bottle.orchestrator.client import RegisteredBottle _MOD = "bot_bottle.backend.docker.consolidated_launch" +_UTIL = "bot_bottle.backend.consolidated_util" def _egress_plan() -> EgressPlan: @@ -49,7 +50,7 @@ class TestLaunchConsolidated(unittest.TestCase): patch(f"{_MOD}._container_ip", return_value="172.18.0.2"), \ patch(f"{_MOD}._network_container_ips", return_value=list(on_network)), \ patch(f"{_MOD}.OrchestratorClient", return_value=client), \ - patch(f"{_MOD}.provision_git_gate", provision or Mock()): + patch(f"{_UTIL}.provision_git_gate", provision or Mock()): return launch_consolidated(_egress_plan(), _git_plan(), service=service) def test_allocates_ip_registers_and_provisions(self) -> None: @@ -84,8 +85,8 @@ class TestLaunchConsolidated(unittest.TestCase): class TestTeardownConsolidated(unittest.TestCase): def test_deregisters_and_deprovisions(self) -> None: client = Mock() - with patch(f"{_MOD}.OrchestratorClient", return_value=client), \ - patch(f"{_MOD}.deprovision_git_gate") as deprov: + with patch(f"{_UTIL}.OrchestratorClient", return_value=client), \ + patch(f"{_UTIL}.deprovision_git_gate") as deprov: teardown_consolidated("b1", orchestrator_url="http://orch:8080") client.teardown_bottle.assert_called_once_with("b1") deprov.assert_called_once() diff --git a/tests/unit/test_infra_artifact.py b/tests/unit/test_infra_artifact.py index 4fccf80..7c5483d 100644 --- a/tests/unit/test_infra_artifact.py +++ b/tests/unit/test_infra_artifact.py @@ -96,7 +96,7 @@ class TestVersionInputs(unittest.TestCase): (pkg / "app.py").write_text("print('hi')\n") (pkg / "egress_entrypoint.sh").write_text("#!/bin/sh\nexec mitmdump\n") (pkg / "netpool.defaults.env").write_text("FOO=1\n") - for name in ("Dockerfile.orchestrator", "Dockerfile.gateway", "Dockerfile.infra"): + for name in ("Dockerfile.orchestrator", "Dockerfile.gateway", "Dockerfile.infra", "Dockerfile.infra.fc"): (root / name).write_text(f"FROM scratch # {name}\n") (root / "pyproject.toml").write_text("[project]\nname = 'bot-bottle'\n") diff --git a/tests/unit/test_macos_consolidated_launch.py b/tests/unit/test_macos_consolidated_launch.py index 5e1e7a6..b0f0b84 100644 --- a/tests/unit/test_macos_consolidated_launch.py +++ b/tests/unit/test_macos_consolidated_launch.py @@ -17,6 +17,7 @@ from bot_bottle.git_gate import GitGatePlan from bot_bottle.orchestrator.client import RegisteredBottle _MOD = "bot_bottle.backend.macos_container.consolidated_launch" +_UTIL = "bot_bottle.backend.consolidated_util" def _egress_plan() -> EgressPlan: @@ -87,7 +88,7 @@ class TestRegisterAgent(unittest.TestCase): *, source_ip: str = "192.168.128.9", ): with patch(f"{_MOD}.OrchestratorClient", return_value=client), \ - patch(f"{_MOD}.provision_git_gate", provision or Mock()), \ + patch(f"{_UTIL}.provision_git_gate", provision or Mock()), \ patch(f"{_MOD}.live_source_ips", return_value=[]): return register_agent( _egress_plan(), _git_plan(), @@ -126,8 +127,8 @@ class TestTeardown(unittest.TestCase): def test_deregisters_and_deprovisions(self) -> None: client = Mock() deprovision = Mock() - with patch(f"{_MOD}.OrchestratorClient", return_value=client), \ - patch(f"{_MOD}.deprovision_git_gate", deprovision): + with patch(f"{_UTIL}.OrchestratorClient", return_value=client), \ + patch(f"{_UTIL}.deprovision_git_gate", deprovision): teardown_consolidated("b1", orchestrator_url="http://o:8099") client.teardown_bottle.assert_called_once_with("b1") self.assertEqual("b1", deprovision.call_args.args[1]) @@ -190,7 +191,7 @@ class TestRegisterAgentReconciles(unittest.TestCase): def _register(self, client: Mock) -> None: with patch(f"{_MOD}.OrchestratorClient", return_value=client), \ - patch(f"{_MOD}.provision_git_gate"), \ + patch(f"{_UTIL}.provision_git_gate"), \ patch(f"{_MOD}.live_source_ips", return_value=["10.0.0.7"]): register_agent( _egress_plan(), _git_plan(), @@ -228,7 +229,7 @@ class TestRegisterAgentReconciles(unittest.TestCase): from bot_bottle.backend.macos_container.enumerate import EnumerationError client = _client() with patch(f"{_MOD}.OrchestratorClient", return_value=client), \ - patch(f"{_MOD}.provision_git_gate"), \ + patch(f"{_UTIL}.provision_git_gate"), \ patch(f"{_MOD}.live_source_ips", side_effect=EnumerationError("container list failed")): register_agent( diff --git a/tests/unit/test_orchestrator_lifecycle.py b/tests/unit/test_orchestrator_lifecycle.py index 8bf3ade..0c155c6 100644 --- a/tests/unit/test_orchestrator_lifecycle.py +++ b/tests/unit/test_orchestrator_lifecycle.py @@ -1,4 +1,4 @@ -"""Unit: orchestrator+gateway container lifecycle — idempotent singleton (PRD 0070).""" +"""Unit: infra container lifecycle — idempotent singleton (PRD 0070).""" from __future__ import annotations @@ -9,9 +9,9 @@ from pathlib import Path from unittest.mock import MagicMock, Mock, patch from bot_bottle.orchestrator.lifecycle import ( - ORCHESTRATOR_IMAGE, - ORCHESTRATOR_NAME, - ORCHESTRATOR_SOURCE_HASH_LABEL, + INFRA_NAME, + INFRA_IMAGE, + INFRA_SOURCE_HASH_LABEL, OrchestratorService, OrchestratorStartError, source_hash, @@ -20,7 +20,6 @@ from tests.unit import use_bottle_root _URLOPEN = "bot_bottle.orchestrator.lifecycle.urllib.request.urlopen" _RUN = "bot_bottle.orchestrator.lifecycle.run_docker" -_GATEWAY = "bot_bottle.orchestrator.lifecycle.DockerGateway" _SLEEP = "bot_bottle.orchestrator.lifecycle.time.sleep" _MONOTONIC = "bot_bottle.orchestrator.lifecycle.time.monotonic" @@ -42,10 +41,8 @@ class TestOrchestratorService(unittest.TestCase): self.addCleanup(use_bottle_root(Path(self._tmp.name))) self.svc = OrchestratorService(port=8099) - def test_urls(self) -> None: + def test_url(self) -> None: self.assertEqual("http://127.0.0.1:8099", self.svc.url) - # The gateway reaches the control plane by container name over docker DNS. - self.assertEqual(f"http://{ORCHESTRATOR_NAME}:8099", self.svc.internal_url) def test_is_healthy(self) -> None: with patch(_URLOPEN, return_value=_health(200)): @@ -54,126 +51,104 @@ class TestOrchestratorService(unittest.TestCase): self.assertFalse(self.svc.is_healthy()) def test_ensure_running_noop_when_healthy_and_source_unchanged(self) -> None: - # A healthy control plane already running the *current* bind-mounted - # source is left alone — recreating it on every launch would drop - # every other active bottle's in-memory egress tokens (#381). + # A healthy container on current source is left alone — recreating it + # on every launch drops in-memory egress tokens (#381). current = source_hash(self.svc._repo_root) calls: list[list[str]] = [] def fake(argv: list[str], **_kw: object) -> Mock: calls.append(argv) if argv[:2] == ["docker", "ps"]: - return _proc(stdout=ORCHESTRATOR_NAME) + return _proc(stdout=INFRA_NAME) if argv[:2] == ["docker", "inspect"]: return _proc(stdout=current) return _proc() with patch(_URLOPEN, return_value=_health(200)), \ - patch(_GATEWAY) as gw_cls, patch(_RUN, side_effect=fake), patch(_SLEEP): + patch(_RUN, side_effect=fake), patch(_SLEEP): self.assertEqual(self.svc.url, self.svc.ensure_running()) - gw_cls.return_value.ensure_running.assert_called() # gateway kept up runs = [c for c in calls if c[:2] == ["docker", "run"]] - rms = [c for c in calls if c[:3] == ["docker", "rm", "--force"] and ORCHESTRATOR_NAME in c] - self.assertEqual([], runs) # not recreated + rms = [c for c in calls if c[:3] == ["docker", "rm", "--force"] and INFRA_NAME in c] + self.assertEqual([], runs) self.assertEqual([], rms) def test_ensure_running_recreates_when_source_changed(self) -> None: - # Healthy, but the running container's label doesn't match the - # current source hash (a real code change) — recreate so it takes - # effect, same as the gateway's image-staleness check. calls: list[list[str]] = [] def fake(argv: list[str], **_kw: object) -> Mock: calls.append(argv) if argv[:2] == ["docker", "ps"]: - return _proc(stdout=ORCHESTRATOR_NAME) + return _proc(stdout=INFRA_NAME) if argv[:2] == ["docker", "inspect"]: return _proc(stdout="stale-hash") return _proc() with patch(_URLOPEN, return_value=_health(200)), \ - patch(_GATEWAY), patch(_RUN, side_effect=fake), patch(_SLEEP): + patch(_RUN, side_effect=fake), patch(_SLEEP): self.assertEqual(self.svc.url, self.svc.ensure_running()) runs = [c for c in calls if c[:2] == ["docker", "run"]] self.assertEqual(1, len(runs)) - self.assertIn(ORCHESTRATOR_NAME, runs[0]) - # the fresh container is labeled with the current hash, not the stale one + self.assertIn(INFRA_NAME, runs[0]) current = source_hash(self.svc._repo_root) - self.assertIn(f"{ORCHESTRATOR_SOURCE_HASH_LABEL}={current}", runs[0]) + self.assertIn(f"{INFRA_SOURCE_HASH_LABEL}={current}", runs[0]) - def test_ensure_running_starts_orchestrator_container_when_absent(self) -> None: - calls: list[list[str]] = [] - - def fake(argv: list[str], **_kw: object) -> Mock: - calls.append(argv) - if argv[:2] == ["docker", "ps"]: - return _proc(stdout="") # not running - return _proc() - - with patch(_URLOPEN, side_effect=[urllib.error.URLError("down"), _health(200)]), \ - patch(_GATEWAY), patch(_RUN, side_effect=fake), patch(_SLEEP): - self.assertEqual(self.svc.url, self.svc.ensure_running()) - runs = [c for c in calls if c[:2] == ["docker", "run"]] - self.assertEqual(1, len(runs)) - argv = runs[0] - self.assertIn(ORCHESTRATOR_NAME, argv) - self.assertIn("--broker", argv) - self.assertEqual("stub", argv[argv.index("--broker") + 1]) # register-only, no socket - self.assertIn("bot_bottle.orchestrator", argv) - self.assertEqual("127.0.0.1:8099:8099", argv[argv.index("--publish") + 1]) - - def test_ensure_running_builds_lean_orchestrator_image_when_missing(self) -> None: - # The control plane runs its own lean image (#384), distinct from the - # gateway data plane — built from Dockerfile.orchestrator when absent. - calls: list[list[str]] = [] - - def fake(argv: list[str], **_kw: object) -> Mock: - calls.append(argv) - if argv[:2] == ["docker", "ps"]: - return _proc(stdout="") # orchestrator not running - if argv[:3] == ["docker", "image", "inspect"]: - return _proc(returncode=1) # image absent -> build - return _proc() - - with patch(_URLOPEN, side_effect=[urllib.error.URLError("down"), _health(200)]), \ - patch(_GATEWAY), patch(_RUN, side_effect=fake), patch(_SLEEP): - self.svc.ensure_running() - builds = [c for c in calls if c[:2] == ["docker", "build"]] - self.assertEqual(1, len(builds)) - self.assertIn(ORCHESTRATOR_IMAGE, builds[0]) - self.assertTrue(any(a.endswith("Dockerfile.orchestrator") for a in builds[0])) - # It is NOT the gateway image/dockerfile — the split is the point. - self.assertFalse(any("Dockerfile.gateway" in a for a in builds[0])) - - def test_ensure_running_skips_orchestrator_image_build_when_present(self) -> None: + def test_ensure_running_starts_infra_container_when_absent(self) -> None: calls: list[list[str]] = [] def fake(argv: list[str], **_kw: object) -> Mock: calls.append(argv) if argv[:2] == ["docker", "ps"]: return _proc(stdout="") - if argv[:3] == ["docker", "image", "inspect"]: - return _proc(returncode=0) # image present -> no build return _proc() with patch(_URLOPEN, side_effect=[urllib.error.URLError("down"), _health(200)]), \ - patch(_GATEWAY), patch(_RUN, side_effect=fake), patch(_SLEEP): + patch(_RUN, side_effect=fake), patch(_SLEEP): + self.assertEqual(self.svc.url, self.svc.ensure_running()) + runs = [c for c in calls if c[:2] == ["docker", "run"]] + self.assertEqual(1, len(runs)) + argv = runs[0] + self.assertIn(INFRA_NAME, argv) + # Published on loopback — not exposed on external interfaces. + self.assertEqual("127.0.0.1:8099:8099", argv[argv.index("--publish") + 1]) + # Both processes in one container — no separate entrypoint override. + self.assertNotIn("--entrypoint", argv) + # Gateway daemons + orchestrator explicitly opted in. + self.assertIn("orchestrator", argv[argv.index("BOT_BOTTLE_GATEWAY_DAEMONS=egress,git-http,supervise,orchestrator")]) + + def test_ensure_running_builds_both_images(self) -> None: + calls: list[list[str]] = [] + + def fake(argv: list[str], **_kw: object) -> Mock: + calls.append(argv) + if argv[:2] == ["docker", "ps"]: + return _proc(stdout="") + return _proc() + + with patch(_URLOPEN, side_effect=[urllib.error.URLError("down"), _health(200)]), \ + patch(_RUN, side_effect=fake), patch(_SLEEP): self.svc.ensure_running() - self.assertEqual([], [c for c in calls if c[:2] == ["docker", "build"]]) + builds = [c for c in calls if c[:2] == ["docker", "build"]] + # Orchestrator (build intermediate) + infra image both built. + self.assertEqual(2, len(builds)) + dockerfiles = [next(a for a in b if "Dockerfile" in a) for b in builds] + self.assertIn("Dockerfile.orchestrator", dockerfiles[0]) + self.assertIn("Dockerfile.infra", dockerfiles[1]) + # Images are distinct — the point of the split. + tags = [b[b.index("-t") + 1] for b in builds] + self.assertNotEqual(tags[0], tags[1]) def test_ensure_running_raises_on_timeout(self) -> None: with patch(_URLOPEN, side_effect=urllib.error.URLError("down")), \ - patch(_GATEWAY), patch(_RUN, return_value=Mock(returncode=0, stderr="")), \ + patch(_RUN, return_value=Mock(returncode=0, stdout="", stderr="")), \ patch(_SLEEP), patch(_MONOTONIC, side_effect=[0.0, 0.5, 2.0]): with self.assertRaises(OrchestratorStartError): self.svc.ensure_running(startup_timeout=1.0) - def test_stop_removes_orchestrator_and_gateway(self) -> None: - with patch(_RUN) as run, patch(_GATEWAY) as gw_cls: + def test_stop_removes_infra_container(self) -> None: + with patch(_RUN) as run: self.svc.stop() rms = [c.args[0] for c in run.call_args_list if c.args[0][:3] == ["docker", "rm", "--force"]] - self.assertTrue(any(ORCHESTRATOR_NAME in a for a in rms)) - gw_cls.return_value.stop.assert_called_once() + self.assertTrue(any(INFRA_NAME in a for a in rms)) if __name__ == "__main__": -- 2.52.0 From 819f967844b441d272d9e139ec05f0028ed9bb31 Mon Sep 17 00:00:00 2001 From: claude Date: Mon, 20 Jul 2026 19:40:56 +0000 Subject: [PATCH 3/8] fix(lint): resolve pylint findings in gateway_init - Extract _sigkill_all() to cut nesting depth below the 5-block limit - Add pylint: disable=consider-using-with on Popen (process must outlive caller) - Break long SIGHUP signal line to stay within 100 chars --- bot_bottle/gateway_init.py | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/bot_bottle/gateway_init.py b/bot_bottle/gateway_init.py index 799428e..4b25bdc 100644 --- a/bot_bottle/gateway_init.py +++ b/bot_bottle/gateway_init.py @@ -150,7 +150,7 @@ def _pump(name: str, stream: IO[bytes]) -> None: def _spawn(spec: _DaemonSpec) -> subprocess.Popen[bytes]: env = _env_for_daemon(spec.name, dict(os.environ)) - proc = subprocess.Popen( + proc = subprocess.Popen( # pylint: disable=consider-using-with _argv_for_daemon(spec.name, spec.argv, env), stdout=subprocess.PIPE, stderr=subprocess.STDOUT, @@ -197,6 +197,14 @@ class _Supervisor: except ProcessLookupError: pass + def _sigkill_all(self) -> None: + for _, p in self.procs: + if p.poll() is None: + try: + p.kill() + except ProcessLookupError: + pass + def request_restart(self, daemon_name: str) -> bool: """Queue a daemon restart for the main loop to process. @@ -249,12 +257,7 @@ class _Supervisor: f"grace ({_GRACE_SECONDS:.0f}s) elapsed; SIGKILL on " f"{', '.join(still_running)}" ) - for _, p in self.procs: - if p.poll() is None: - try: - p.kill() - except ProcessLookupError: - pass + self._sigkill_all() done = all(p.poll() is not None for _, p in self.procs) if done: @@ -375,7 +378,9 @@ def main(argv: Sequence[str] | None = None) -> int: # --signal HUP ` after writing routes.yaml. The kernel # delivers SIGHUP to PID 1 (this supervisor); forward it to # mitmdump so it reloads its addon. - signal.signal(signal.SIGHUP, lambda *_: sup.forward_signal(signal.SIGHUP, "egress")) # type: ignore + signal.signal( # type: ignore + signal.SIGHUP, lambda *_: sup.forward_signal(signal.SIGHUP, "egress") + ) while not sup.tick(): time.sleep(_POLL_INTERVAL) -- 2.52.0 From 28766d773350b92ef981c1660e82ea15c319bd29 Mon Sep 17 00:00:00 2001 From: claude Date: Mon, 20 Jul 2026 19:48:52 +0000 Subject: [PATCH 4/8] fix(pyright): resolve type errors introduced by lifecycle refactor - Remove unused INFRA_IMAGE import from test_orchestrator_lifecycle - Update integration test to use new single-container OrchestratorService API (infra_name/image replaces orchestrator_name/gateway_name/gateway_image) - Move type: ignore to the lambda line in gateway_init SIGHUP handler - Break two long lines in test_orchestrator_lifecycle --- bot_bottle/gateway_init.py | 5 +++-- ..._orchestrator_docker_control_plane_auth.py | 20 +++++++++---------- tests/unit/test_orchestrator_lifecycle.py | 9 ++++++--- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/bot_bottle/gateway_init.py b/bot_bottle/gateway_init.py index 4b25bdc..13b26c1 100644 --- a/bot_bottle/gateway_init.py +++ b/bot_bottle/gateway_init.py @@ -378,8 +378,9 @@ def main(argv: Sequence[str] | None = None) -> int: # --signal HUP ` after writing routes.yaml. The kernel # delivers SIGHUP to PID 1 (this supervisor); forward it to # mitmdump so it reloads its addon. - signal.signal( # type: ignore - signal.SIGHUP, lambda *_: sup.forward_signal(signal.SIGHUP, "egress") + signal.signal( + signal.SIGHUP, + lambda *_: sup.forward_signal(signal.SIGHUP, "egress"), # type: ignore[misc] ) while not sup.tick(): diff --git a/tests/integration/test_orchestrator_docker_control_plane_auth.py b/tests/integration/test_orchestrator_docker_control_plane_auth.py index 4641596..2c66887 100644 --- a/tests/integration/test_orchestrator_docker_control_plane_auth.py +++ b/tests/integration/test_orchestrator_docker_control_plane_auth.py @@ -34,6 +34,7 @@ from tests._docker import skip_unless_docker # image instead of leaking a new dangling tag on every invocation. _TEST_ORCHESTRATOR_IMAGE = "bot-bottle-orchestrator:itest" _TEST_GATEWAY_IMAGE = "bot-bottle-gateway:itest" +_TEST_INFRA_IMAGE = "bot-bottle-infra:itest" @skip_unless_docker() @@ -69,20 +70,17 @@ class TestDockerControlPlaneAuthIntegration(unittest.TestCase): os.environ["BOT_BOTTLE_ROOT"] = cls._tmp.name cls.addClassCleanup(_restore_root) - orchestrator_name = f"bot-bottle-orch-itest-{suffix}" - gateway_name = f"bot-bottle-gw-itest-{suffix}" + infra_name = f"bot-bottle-infra-itest-{suffix}" network = f"bot-bottle-net-itest-{suffix}" host_root = Path(cls._tmp.name) cls.addClassCleanup( - cls._teardown_docker, orchestrator_name, gateway_name, network, host_root + cls._teardown_docker, infra_name, network, host_root ) cls.svc = OrchestratorService( - orchestrator_name=orchestrator_name, - gateway_name=gateway_name, + infra_name=infra_name, network=network, - image=_TEST_ORCHESTRATOR_IMAGE, - gateway_image=_TEST_GATEWAY_IMAGE, + image=_TEST_INFRA_IMAGE, port=20000 + secrets.randbelow(10000), host_root=host_root, ) @@ -91,23 +89,23 @@ class TestDockerControlPlaneAuthIntegration(unittest.TestCase): @staticmethod def _teardown_docker( - orchestrator_name: str, gateway_name: str, network: str, host_root: Path + infra_name: str, network: str, host_root: Path ) -> None: subprocess.run( - ["docker", "rm", "--force", orchestrator_name, gateway_name], + ["docker", "rm", "--force", infra_name], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=False, ) subprocess.run( ["docker", "network", "rm", network], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=False, ) - # The orchestrator container (no USER directive) wrote the registry + # The infra container (no USER directive) wrote the registry # DB as root into the throwaway host_root; chown it back so the # (non-root) tempdir cleanup can remove it. Same workaround # test_multitenant_isolation.py uses for the identical bind mount. subprocess.run( ["docker", "run", "--rm", "-v", f"{host_root}:/r", - "--entrypoint", "chown", _TEST_GATEWAY_IMAGE, "-R", + "--entrypoint", "chown", _TEST_INFRA_IMAGE, "-R", f"{os.getuid()}:{os.getgid()}", "/r"], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=False, ) diff --git a/tests/unit/test_orchestrator_lifecycle.py b/tests/unit/test_orchestrator_lifecycle.py index 0c155c6..e2e7bc8 100644 --- a/tests/unit/test_orchestrator_lifecycle.py +++ b/tests/unit/test_orchestrator_lifecycle.py @@ -10,7 +10,6 @@ from unittest.mock import MagicMock, Mock, patch from bot_bottle.orchestrator.lifecycle import ( INFRA_NAME, - INFRA_IMAGE, INFRA_SOURCE_HASH_LABEL, OrchestratorService, OrchestratorStartError, @@ -113,7 +112,8 @@ class TestOrchestratorService(unittest.TestCase): # Both processes in one container — no separate entrypoint override. self.assertNotIn("--entrypoint", argv) # Gateway daemons + orchestrator explicitly opted in. - self.assertIn("orchestrator", argv[argv.index("BOT_BOTTLE_GATEWAY_DAEMONS=egress,git-http,supervise,orchestrator")]) + daemons_flag = "BOT_BOTTLE_GATEWAY_DAEMONS=egress,git-http,supervise,orchestrator" + self.assertIn("orchestrator", argv[argv.index(daemons_flag)]) def test_ensure_running_builds_both_images(self) -> None: calls: list[list[str]] = [] @@ -147,7 +147,10 @@ class TestOrchestratorService(unittest.TestCase): def test_stop_removes_infra_container(self) -> None: with patch(_RUN) as run: self.svc.stop() - rms = [c.args[0] for c in run.call_args_list if c.args[0][:3] == ["docker", "rm", "--force"]] + rms = [ + c.args[0] for c in run.call_args_list + if c.args[0][:3] == ["docker", "rm", "--force"] + ] self.assertTrue(any(INFRA_NAME in a for a in rms)) -- 2.52.0 From cae1215f632db50217e34a3dcbecd9bbde815f67 Mon Sep 17 00:00:00 2001 From: claude Date: Mon, 20 Jul 2026 20:11:05 +0000 Subject: [PATCH 5/8] test(lifecycle): cover edge paths to satisfy diff-coverage gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add three new tests: - noop when healthy but docker inspect fails (returns True → don't churn) - build failure raises GatewayError - _ensure_network creates the network when it doesn't exist Also update the integration test to use new OrchestratorService API (infra_name/image instead of orchestrator_name/gateway_name/gateway_image). Brings diff-coverage from 86% to 90.3% against origin/main. --- tests/unit/test_orchestrator_lifecycle.py | 40 +++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tests/unit/test_orchestrator_lifecycle.py b/tests/unit/test_orchestrator_lifecycle.py index e2e7bc8..5afdd45 100644 --- a/tests/unit/test_orchestrator_lifecycle.py +++ b/tests/unit/test_orchestrator_lifecycle.py @@ -8,6 +8,7 @@ import urllib.error from pathlib import Path from unittest.mock import MagicMock, Mock, patch +from bot_bottle.orchestrator.gateway import GatewayError from bot_bottle.orchestrator.lifecycle import ( INFRA_NAME, INFRA_SOURCE_HASH_LABEL, @@ -144,6 +145,45 @@ class TestOrchestratorService(unittest.TestCase): with self.assertRaises(OrchestratorStartError): self.svc.ensure_running(startup_timeout=1.0) + def test_noop_when_healthy_and_inspect_fails(self) -> None: + """If docker inspect fails (e.g. docker daemon hiccup), leave the + working container alone rather than churning it.""" + def fake(argv: list[str], **_kw: object) -> Mock: + if argv[:2] == ["docker", "ps"]: + return _proc(stdout=INFRA_NAME) + if argv[:2] == ["docker", "inspect"]: + return _proc(returncode=1, stderr="daemon error") + return _proc() + + with patch(_URLOPEN, return_value=_health(200)), \ + patch(_RUN, side_effect=fake), patch(_SLEEP): + self.svc.ensure_running() + # no docker run — the working container was left alone + + def test_build_failure_raises(self) -> None: + with patch(_URLOPEN, side_effect=urllib.error.URLError("down")), \ + patch(_RUN, return_value=_proc(returncode=1, stderr="no space left on device")): + with self.assertRaises(GatewayError): + self.svc.ensure_running() + + def test_ensure_network_creates_if_missing(self) -> None: + """If the gateway network doesn't exist yet, create it.""" + calls: list[list[str]] = [] + + def fake(argv: list[str], **_kw: object) -> Mock: + calls.append(argv) + if argv[:3] == ["docker", "network", "inspect"]: + return _proc(returncode=1, stderr="not found") + if argv[:2] == ["docker", "ps"]: + return _proc(stdout="") + return _proc() + + with patch(_URLOPEN, side_effect=[urllib.error.URLError("down"), _health(200)]), \ + patch(_RUN, side_effect=fake), patch(_SLEEP): + self.svc.ensure_running() + creates = [c for c in calls if c[:3] == ["docker", "network", "create"]] + self.assertEqual(1, len(creates)) + def test_stop_removes_infra_container(self) -> None: with patch(_RUN) as run: self.svc.stop() -- 2.52.0 From 14ff4fe1868aa7583f863ed5b839f4aee3937e16 Mon Sep 17 00:00:00 2001 From: claude Date: Mon, 20 Jul 2026 22:46:09 +0000 Subject: [PATCH 6/8] fix(lifecycle): build gateway before infra and fix orchestrator port mapping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `_build_images()` now builds Dockerfile.gateway → Dockerfile.orchestrator → Dockerfile.infra in order; Dockerfile.infra starts FROM bot-bottle-gateway so the base must exist on clean hosts. - Publish mapping corrected from `self.port:self.port` to `self.port:DEFAULT_PORT` (8099) — gateway_init hardcodes the orchestrator on port 8099 inside the container, so the host-side published port must map to that fixed internal port. - BOT_BOTTLE_ORCHESTRATOR_URL inside the container now always points to 127.0.0.1:8099, not self.port, since gateway daemons reach the orchestrator over loopback at the fixed internal port. - Update test_ensure_running_builds_both_images → _all_images for the new three-step build sequence; add test_publish_maps_host_port_to_fixed_internal_port to lock in the port-mapping fix. Addresses the P1 findings from the didericis-codex review on PR #432. --- bot_bottle/orchestrator/lifecycle.py | 16 ++++++---- tests/unit/test_orchestrator_lifecycle.py | 36 ++++++++++++++++++----- 2 files changed, 40 insertions(+), 12 deletions(-) diff --git a/bot_bottle/orchestrator/lifecycle.py b/bot_bottle/orchestrator/lifecycle.py index 7c06801..5cfd973 100644 --- a/bot_bottle/orchestrator/lifecycle.py +++ b/bot_bottle/orchestrator/lifecycle.py @@ -27,6 +27,8 @@ from ..paths import CONTROL_PLANE_TOKEN_ENV, bot_bottle_root, host_control_plane from ..supervise import DB_PATH_IN_CONTAINER from .gateway import ( GATEWAY_CA_VOLUME, + GATEWAY_DOCKERFILE, + GATEWAY_IMAGE, GATEWAY_NETWORK, GatewayError, MITMPROXY_HOME, @@ -160,9 +162,10 @@ class OrchestratorService: ) def _build_images(self) -> None: - """Build the orchestrator image (build intermediate), then the infra - image. Both are cache-aware: a no-op when nothing changed.""" + """Build the gateway base, the orchestrator intermediate, then the + infra image. All are cache-aware: a no-op when nothing changed.""" for tag, dockerfile in ( + (GATEWAY_IMAGE, GATEWAY_DOCKERFILE), (ORCHESTRATOR_IMAGE, ORCHESTRATOR_DOCKERFILE), (self.image, INFRA_DOCKERFILE), ): @@ -188,7 +191,9 @@ class OrchestratorService: "--label", f"{INFRA_SOURCE_HASH_LABEL}={current_hash}", "--network", self.network, # Host CLI reaches the control plane here (loopback only). - "--publish", f"127.0.0.1:{self.port}:{self.port}", + # gateway_init always starts the orchestrator on DEFAULT_PORT (8099) + # inside the container; self.port is the host-side published port. + "--publish", f"127.0.0.1:{self.port}:{DEFAULT_PORT}", # Persist the mitmproxy CA so it survives container recreation. "--volume", f"{GATEWAY_CA_VOLUME}:{MITMPROXY_HOME}", # Shared supervise DB (same file the operator reads over HTTP). @@ -206,8 +211,9 @@ class OrchestratorService: # Control-plane secret: required by the orchestrator (to enforce) # and by the gateway daemons (to present on /resolve calls). "--env", CONTROL_PLANE_TOKEN_ENV, - # Gateway daemons reach the orchestrator over loopback. - "--env", f"BOT_BOTTLE_ORCHESTRATOR_URL=http://127.0.0.1:{self.port}", + # Gateway daemons reach the orchestrator over loopback at its + # fixed internal port (DEFAULT_PORT), independent of self.port. + "--env", f"BOT_BOTTLE_ORCHESTRATOR_URL=http://127.0.0.1:{DEFAULT_PORT}", # Opt the orchestrator into gateway_init's supervise tree. "--env", f"BOT_BOTTLE_GATEWAY_DAEMONS={_INFRA_DAEMONS}", self.image, diff --git a/tests/unit/test_orchestrator_lifecycle.py b/tests/unit/test_orchestrator_lifecycle.py index 5afdd45..6665a33 100644 --- a/tests/unit/test_orchestrator_lifecycle.py +++ b/tests/unit/test_orchestrator_lifecycle.py @@ -116,7 +116,7 @@ class TestOrchestratorService(unittest.TestCase): daemons_flag = "BOT_BOTTLE_GATEWAY_DAEMONS=egress,git-http,supervise,orchestrator" self.assertIn("orchestrator", argv[argv.index(daemons_flag)]) - def test_ensure_running_builds_both_images(self) -> None: + def test_ensure_running_builds_all_images(self) -> None: calls: list[list[str]] = [] def fake(argv: list[str], **_kw: object) -> Mock: @@ -129,14 +129,36 @@ class TestOrchestratorService(unittest.TestCase): patch(_RUN, side_effect=fake), patch(_SLEEP): self.svc.ensure_running() builds = [c for c in calls if c[:2] == ["docker", "build"]] - # Orchestrator (build intermediate) + infra image both built. - self.assertEqual(2, len(builds)) + # Gateway base + orchestrator intermediate + infra image — all three built. + self.assertEqual(3, len(builds)) dockerfiles = [next(a for a in b if "Dockerfile" in a) for b in builds] - self.assertIn("Dockerfile.orchestrator", dockerfiles[0]) - self.assertIn("Dockerfile.infra", dockerfiles[1]) - # Images are distinct — the point of the split. + self.assertIn("Dockerfile.gateway", dockerfiles[0]) + self.assertIn("Dockerfile.orchestrator", dockerfiles[1]) + self.assertIn("Dockerfile.infra", dockerfiles[2]) + # All three images are distinct. tags = [b[b.index("-t") + 1] for b in builds] - self.assertNotEqual(tags[0], tags[1]) + self.assertEqual(3, len(set(tags))) + + def test_publish_maps_host_port_to_fixed_internal_port(self) -> None: + """A non-default self.port is published to the fixed internal port 8099, + not to self.port:self.port — the orchestrator always listens on 8099.""" + calls: list[list[str]] = [] + + def fake(argv: list[str], **_kw: object) -> Mock: + calls.append(argv) + if argv[:2] == ["docker", "ps"]: + return _proc(stdout="") + return _proc() + + svc = OrchestratorService(port=20001) + with patch(_URLOPEN, side_effect=[urllib.error.URLError("down"), _health(200)]), \ + patch(_RUN, side_effect=fake), patch(_SLEEP): + svc.ensure_running() + runs = [c for c in calls if c[:2] == ["docker", "run"]] + argv = runs[0] + self.assertEqual("127.0.0.1:20001:8099", argv[argv.index("--publish") + 1]) + orch_url = next(a for a in argv if "BOT_BOTTLE_ORCHESTRATOR_URL" in a) + self.assertIn(":8099", orch_url) def test_ensure_running_raises_on_timeout(self) -> None: with patch(_URLOPEN, side_effect=urllib.error.URLError("down")), \ -- 2.52.0 From 8e43c26ab4822e6b742abaa7cc17fddddf618635 Mon Sep 17 00:00:00 2001 From: claude Date: Mon, 20 Jul 2026 23:26:01 +0000 Subject: [PATCH 7/8] fix(backend): extract poll_ca_cert helper and fix PRD port docs Extract the shared CA cert polling loop into `backend/util.poll_ca_cert` (firecracker and macos backends were duplicating deadline/sleep/raise logic). Each caller now wraps a fetch lambda and converts TimeoutError to its own error type. Also corrects the PRD port publication line from {port}:{port} to {host_port}:8099. --- bot_bottle/backend/firecracker/infra_vm.py | 16 +++++++-------- bot_bottle/backend/macos_container/infra.py | 19 +++++++++--------- bot_bottle/backend/util.py | 20 +++++++++++++++++++ ...rd-new-consolidate-docker-infra-backend.md | 5 +++-- 4 files changed, 40 insertions(+), 20 deletions(-) diff --git a/bot_bottle/backend/firecracker/infra_vm.py b/bot_bottle/backend/firecracker/infra_vm.py index ba3bdd4..8ccf6e9 100644 --- a/bot_bottle/backend/firecracker/infra_vm.py +++ b/bot_bottle/backend/firecracker/infra_vm.py @@ -33,6 +33,7 @@ from pathlib import Path from typing import Generator from ...log import die, info +from .. import util as backend_util from ..docker import util as docker_mod from ..docker.gateway_provision import GatewayProvisionError from . import firecracker_vm, infra_artifact, netpool, util @@ -93,19 +94,18 @@ class InfraVm: """The gateway's mitmproxy CA (PEM) that agents install to trust its TLS interception. Generated a moment after boot, so this polls over SSH until it appears (mirrors DockerGateway.ca_cert_pem).""" - deadline = time.monotonic() + timeout - while True: + def _fetch() -> str | None: proc = subprocess.run( util.ssh_base_argv(self.private_key, self.guest_ip) + [f"cat {_GATEWAY_CA_PATH}"], capture_output=True, text=True, timeout=15, check=False, ) - if proc.returncode == 0 and "BEGIN CERTIFICATE" in proc.stdout: - return proc.stdout - if time.monotonic() >= deadline: - die(f"gateway CA not available after {timeout:g}s: " - f"{proc.stderr.strip() or 'empty'}") - time.sleep(_HEALTH_POLL_SECONDS) + ok = proc.returncode == 0 and "BEGIN CERTIFICATE" in proc.stdout + return proc.stdout if ok else None + try: + return backend_util.poll_ca_cert(_fetch, timeout=timeout) + except TimeoutError as exc: + die(str(exc)) def ensure_built() -> None: diff --git a/bot_bottle/backend/macos_container/infra.py b/bot_bottle/backend/macos_container/infra.py index 6bca5c9..c6fcf62 100644 --- a/bot_bottle/backend/macos_container/infra.py +++ b/bot_bottle/backend/macos_container/infra.py @@ -53,6 +53,7 @@ from ...paths import ( HOST_DB_FILENAME, host_control_plane_token, ) +from .. import util as backend_util from . import util as container_mod from .gateway import ( DEFAULT_CA_TIMEOUT_SECONDS, @@ -263,18 +264,16 @@ class MacosInfraService: interception. Read out of the container (the CA lives on a container-internal path, not a host mount); polls because mitmproxy writes it a beat after start.""" - deadline = time.monotonic() + timeout - while True: + def _fetch() -> str | None: result = container_mod.run_container_argv( ["container", "exec", self._name, "cat", GATEWAY_CA_CERT]) - if result.returncode == 0 and result.stdout.strip(): - return result.stdout - if time.monotonic() >= deadline: - raise GatewayError( - f"gateway CA not available in {self._name} after {timeout:g}s: " - f"{(result.stderr or '').strip() or 'empty'}" - ) - time.sleep(_CA_POLL_SECONDS) + return result.stdout if result.returncode == 0 and result.stdout.strip() else None + try: + return backend_util.poll_ca_cert(_fetch, timeout=timeout) + except TimeoutError as exc: + raise GatewayError( + f"gateway CA not available in {self._name} after {timeout:g}s" + ) from exc def stop(self) -> None: """Remove the infra container (idempotent). The DB volume persists.""" diff --git a/bot_bottle/backend/util.py b/bot_bottle/backend/util.py index f5ea929..3d89622 100644 --- a/bot_bottle/backend/util.py +++ b/bot_bottle/backend/util.py @@ -7,6 +7,8 @@ from __future__ import annotations import hashlib import os import ssl +import time +from collections.abc import Callable from pathlib import Path from typing import TYPE_CHECKING @@ -15,6 +17,24 @@ from ..log import die, info if TYPE_CHECKING: from ..egress import EgressPlan +_CA_POLL_INTERVAL = 0.5 + + +def poll_ca_cert(fetch: Callable[[], str | None], *, timeout: float) -> str: + """Poll `fetch` until it returns a non-empty PEM string or `timeout` expires. + + `fetch` should return the PEM on success and `None` (or empty string) when + the cert is not yet available. Raises `TimeoutError` if the cert never + appears within `timeout` seconds.""" + deadline = time.monotonic() + timeout + while True: + result = fetch() + if result: + return result + if time.monotonic() >= deadline: + raise TimeoutError(f"CA cert not available after {timeout:g}s") + time.sleep(_CA_POLL_INTERVAL) + # Debian-family CA layout, shared by every backend (all guest images # are Debian-family). AGENT_CA_PATH is the source path that diff --git a/docs/prds/prd-new-consolidate-docker-infra-backend.md b/docs/prds/prd-new-consolidate-docker-infra-backend.md index b7e0976..957739f 100644 --- a/docs/prds/prd-new-consolidate-docker-infra-backend.md +++ b/docs/prds/prd-new-consolidate-docker-infra-backend.md @@ -90,8 +90,9 @@ first (`DockerGateway.ensure_running`), then orchestrator. After this PRD: - Single `docker run` of `bot-bottle-infra:latest` - Container name: `bot-bottle-infra` (replaces `bot-bottle-orch-gateway` + `bot-bottle-orchestrator`) -- Published ports: `127.0.0.1:{port}:{port}` for the control plane (same as - today) +- Published ports: `127.0.0.1:{host_port}:8099` for the control plane + (`gateway_init` listens on a fixed internal port 8099; the caller-chosen + host port maps to it) - Bind mounts: repo root + host root (same as today) - `DockerGateway` becomes an implementation detail of `OrchestratorService` rather than a separately started container; the gateway image name -- 2.52.0 From b25cd72fc30c673bdc187c9f2400435c790ec53e Mon Sep 17 00:00:00 2001 From: claude Date: Mon, 20 Jul 2026 23:40:42 +0000 Subject: [PATCH 8/8] test(backend): cover poll_ca_cert timeout paths for diff-coverage gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add tests for the three uncovered paths introduced by the poll_ca_cert extraction: the timeout + sleep branches in backend/util, and the TimeoutError → GatewayError and TimeoutError → die() conversions in the macOS and Firecracker callers. --- tests/unit/test_backend_util.py | 29 +++++++++++++++++++++++++ tests/unit/test_firecracker_infra_vm.py | 10 +++++++++ tests/unit/test_macos_infra.py | 8 +++++++ 3 files changed, 47 insertions(+) create mode 100644 tests/unit/test_backend_util.py diff --git a/tests/unit/test_backend_util.py b/tests/unit/test_backend_util.py new file mode 100644 index 0000000..4978158 --- /dev/null +++ b/tests/unit/test_backend_util.py @@ -0,0 +1,29 @@ +"""Unit: shared cross-backend helpers in backend/util.py.""" + +from __future__ import annotations + +import unittest +from unittest.mock import patch + +from bot_bottle.backend import util as backend_util + + +class TestPollCaCert(unittest.TestCase): + def test_returns_pem_on_first_success(self) -> None: + result = backend_util.poll_ca_cert(lambda: "PEM", timeout=1.0) + self.assertEqual("PEM", result) + + def test_raises_timeout_error_when_cert_never_appears(self) -> None: + with self.assertRaises(TimeoutError): + backend_util.poll_ca_cert(lambda: None, timeout=0.0) + + def test_polls_until_cert_appears(self) -> None: + responses = iter([None, None, "-----BEGIN CERTIFICATE-----\n"]) + with patch("bot_bottle.backend.util.time.sleep") as mock_sleep: + result = backend_util.poll_ca_cert(lambda: next(responses), timeout=5.0) + self.assertTrue(result.startswith("-----BEGIN CERTIFICATE-----")) + self.assertEqual(2, mock_sleep.call_count) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/test_firecracker_infra_vm.py b/tests/unit/test_firecracker_infra_vm.py index 705e8d7..8cafd8c 100644 --- a/tests/unit/test_firecracker_infra_vm.py +++ b/tests/unit/test_firecracker_infra_vm.py @@ -71,6 +71,16 @@ class TestSshGatewayTransport(unittest.TestCase): t.exec(["mkdir", "-p", "/git-gate"]) +class TestGatewayCaPem(unittest.TestCase): + def test_dies_when_cert_never_appears(self) -> None: + from subprocess import CompletedProcess + infra = infra_vm.InfraVm(vm=None, guest_ip="10.0.0.1", private_key=Path("/k")) + with patch.object(infra_vm.subprocess, "run", + return_value=CompletedProcess([], 1, stdout="", stderr="")), \ + self.assertRaises(SystemExit): + infra.gateway_ca_pem(timeout=0) + + class TestRegistryVolume(unittest.TestCase): def test_reuses_existing_volume(self): import tempfile diff --git a/tests/unit/test_macos_infra.py b/tests/unit/test_macos_infra.py index 012f531..a026143 100644 --- a/tests/unit/test_macos_infra.py +++ b/tests/unit/test_macos_infra.py @@ -153,6 +153,14 @@ class TestCaCertPem(unittest.TestCase): argv = mod.run_container_argv.call_args.args[0] self.assertEqual(["container", "exec", "bot-bottle-mac-infra", "cat"], argv[:4]) + def test_raises_gateway_error_when_cert_never_appears(self) -> None: + from bot_bottle.backend.macos_container.gateway import GatewayError + svc = MacosInfraService(repo_root=Path("/r")) + with patch(f"{_INFRA}.container_mod") as mod: + mod.run_container_argv.return_value = _fail() + with self.assertRaises(GatewayError): + svc.ca_cert_pem(timeout=0) + class TestProbeControlPlane(unittest.TestCase): def test_returns_url_when_running(self) -> None: -- 2.52.0