From 52c0df7741cb72099d611fdddc3890922d649f17 Mon Sep 17 00:00:00 2001 From: claude Date: Mon, 20 Jul 2026 19:35:33 +0000 Subject: [PATCH] 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 | 55 +++ .../backend/docker/consolidated_launch.py | 74 ++-- .../firecracker/consolidated_launch.py | 21 +- .../backend/firecracker/infra_artifact.py | 2 +- bot_bottle/backend/firecracker/infra_vm.py | 13 +- .../macos_container/consolidated_launch.py | 19 +- 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 | 7 +- tests/unit/test_orchestrator_lifecycle.py | 131 +++---- 15 files changed, 372 insertions(+), 380 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..d9977e0 --- /dev/null +++ b/bot_bottle/backend/consolidated_util.py @@ -0,0 +1,55 @@ +"""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, +) -> None: + """Deregister the bottle and remove its git-gate state. Both steps are + idempotent so this is safe from a cleanup trap.""" + OrchestratorClient(orchestrator_url).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 2d7c230..ea6223d 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,12 +121,11 @@ 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, ) -> 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.""" - OrchestratorClient(orchestrator_url).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) __all__ = [ diff --git a/bot_bottle/backend/firecracker/consolidated_launch.py b/bot_bottle/backend/firecracker/consolidated_launch.py index 899676b..c3e285e 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( @@ -96,8 +88,7 @@ def teardown_consolidated(bottle_id: str, *, orchestrator_url: str) -> None: 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.""" - OrchestratorClient(orchestrator_url).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) __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 5b7efc1..945a32b 100644 --- a/bot_bottle/backend/macos_container/consolidated_launch.py +++ b/bot_bottle/backend/macos_container/consolidated_launch.py @@ -37,8 +37,7 @@ from dataclasses import dataclass 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 deprovision_git_gate, provision_git_gate +from ..consolidated_util import provision_bottle, teardown_consolidated as _teardown_util from .gateway import GATEWAY_NETWORK from .gateway_provision import AppleGatewayTransport from .infra import MacosInfraService, OrchestratorStartError @@ -103,17 +102,10 @@ def register_agent( container — it is the attribution key the gateway resolves policy by. Raises on failure; the caller tears down.""" client = OrchestratorClient(endpoint.orchestrator_url) - 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, @@ -128,8 +120,7 @@ def teardown_consolidated(bottle_id: str, *, orchestrator_url: str) -> 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. Does NOT stop the gateway — it's a persistent per-host singleton.""" - OrchestratorClient(orchestrator_url).teardown_bottle(bottle_id) - deprovision_git_gate(AppleGatewayTransport(), bottle_id) + _teardown_util(bottle_id, AppleGatewayTransport(), orchestrator_url=orchestrator_url) __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 f3e395d..9f06ce6 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()): return register_agent( _egress_plan(), _git_plan(), source_ip=source_ip, endpoint=_endpoint(), image_ref="img:1", @@ -125,8 +126,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]) 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__":