diff --git a/bot_bottle/backend/docker/consolidated_launch.py b/bot_bottle/backend/docker/consolidated_launch.py index 7efabc6..8b5c53e 100644 --- a/bot_bottle/backend/docker/consolidated_launch.py +++ b/bot_bottle/backend/docker/consolidated_launch.py @@ -156,14 +156,17 @@ def launch_consolidated( they regain egress access before the new bottle is registered.""" service = service or DockerInfraService() url = service.ensure_running() - _reprovision_running_bottles(url, network=network, infra_name=infra_name) + # Agents attribute against the *gateway* container (data plane), not the + # orchestrator — the two planes are now separate containers. + gateway_name = service.gateway_name + _reprovision_running_bottles(url, network=network, infra_name=gateway_name) client = OrchestratorClient(url) cidr = _network_cidr(network) - gateway_ip = _container_ip(infra_name, network) + gateway_ip = _container_ip(gateway_name, network) source_ip = next_free_ip(cidr, _network_container_ips(network)) - transport = DockerGatewayTransport(infra_name) + transport = DockerGatewayTransport(gateway_name) reg = provision_bottle( client, source_ip, egress_plan, git_gate_plan, transport, image_ref=image_ref, tokens=tokens, diff --git a/bot_bottle/backend/docker/gateway.py b/bot_bottle/backend/docker/gateway.py index 221de21..f188ed7 100644 --- a/bot_bottle/backend/docker/gateway.py +++ b/bot_bottle/backend/docker/gateway.py @@ -31,6 +31,7 @@ class DockerGateway(Gateway): *, name: str = GATEWAY_NAME, network: str = GATEWAY_NETWORK, + control_network: str = "", orchestrator_url: str = "", build_context: Path | None = None, dockerfile: str | None = GATEWAY_DOCKERFILE, @@ -39,6 +40,11 @@ class DockerGateway(Gateway): self.image_ref = image_ref self.name = name self.network = network + # When set, the gateway is dual-homed: it also joins this `--internal` + # control network (shared only with the orchestrator) so it can resolve + # `orchestrator_url` by container name over docker DNS. Agents are never + # on it, so they get no route to the control plane (PRD 0070). + self._control_network = control_network # The control-plane URL the gateway's data plane resolves per bottle # against — reached by container name over docker DNS on the shared # network (container↔container, no host firewall). Mandatory to *run* @@ -164,6 +170,30 @@ class DockerGateway(Gateway): proc = run_docker(argv, env=run_env) if proc.returncode != 0: raise GatewayError(f"gateway failed to start: {proc.stderr.strip()}") + # Dual-home onto the control network so the daemons resolve the + # orchestrator by name. Docker attaches only one network at `run`, so + # the second is a `network connect` — the daemons tolerate the brief + # pre-connect window (they retry /resolve per request). + if self._control_network: + self._connect_control_network() + + def _connect_control_network(self) -> None: + """Ensure the `--internal` control network exists and attach the gateway + to it (idempotent — an already-connected container is a tolerated + no-op).""" + if run_docker(["docker", "network", "inspect", self._control_network]).returncode != 0: + proc = run_docker(["docker", "network", "create", "--internal", self._control_network]) + if proc.returncode != 0 and "already exists" not in proc.stderr: + raise GatewayError( + f"control network {self._control_network} failed to create: " + f"{proc.stderr.strip()}" + ) + proc = run_docker(["docker", "network", "connect", self._control_network, self.name]) + if proc.returncode != 0 and "already exists" not in proc.stderr: + raise GatewayError( + f"gateway failed to join control network {self._control_network}: " + f"{proc.stderr.strip()}" + ) def ca_cert_pem(self, *, timeout: float = DEFAULT_CA_TIMEOUT_SECONDS) -> str: """The gateway's CA certificate (PEM) that agents install to trust its diff --git a/bot_bottle/backend/docker/infra.py b/bot_bottle/backend/docker/infra.py index d9d119d..48da335 100644 --- a/bot_bottle/backend/docker/infra.py +++ b/bot_bottle/backend/docker/infra.py @@ -1,19 +1,22 @@ -"""The per-host infra container for the docker backend (PRD 0070). +"""The per-host control plane + gateway for the docker backend (PRD 0070). -Runs both the orchestrator control plane and the gateway data plane inside a -single `bot-bottle-infra` container on the shared gateway network — the docker -analogue of the macOS infra container (`backend/macos_container/infra.py`) and -the Firecracker infra VM (`backend/firecracker/infra_vm.py`). `gateway_init` is -PID 1 and supervises both; the infra container is an idempotent per-host -singleton. +Runs the orchestrator (control plane) and the gateway (data plane) as **two +separate containers**, split now that #469 got the DB off the data plane: -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). + * `bot-bottle-orchestrator` — the lean control-plane container. Joins the + `bot-bottle-orchestrator` **control network** (`--internal`) only, plus a + host-loopback publish for the CLI. Sole opener of `bot-bottle.db`; holds the + signing key. + * `bot-bottle-gateway` — the data-plane container (`DockerGateway`). + **Dual-homed** on the agent-facing `bot-bottle-gateway` network *and* the + control network, so it reaches the orchestrator by name over docker DNS + (`http://bot-bottle-orchestrator:8099`) while agents — which are never on + the control network — cannot reach the control plane at all (the L3 block + Firecracker's nft already has). Holds the `gateway` JWT + the mitmproxy CA. -The shared, backend-neutral pieces (control-plane port, startup timeout, -`OrchestratorStartError`, `source_hash`) live in `orchestrator.lifecycle`. +`DockerInfraService` brings both up as an idempotent per-host pair. Callers use +`ensure_running()` (returns the host control-plane URL) + `gateway_name` (the +container the launch flow attributes agents against). """ from __future__ import annotations @@ -25,21 +28,19 @@ import urllib.request from pathlib import Path from ... import log -from ...orchestrator_auth import ROLE_GATEWAY, mint from .util import run_docker +from .gateway import DockerGateway from ...paths import ( - ORCHESTRATOR_AUTH_JWT_ENV, ORCHESTRATOR_TOKEN_ENV, bot_bottle_root, host_orchestrator_token, - host_gateway_ca_dir, ) from ...gateway import ( GATEWAY_DOCKERFILE, GATEWAY_IMAGE, + GATEWAY_NAME, GATEWAY_NETWORK, GatewayError, - MITMPROXY_HOME, ) from ...orchestrator.lifecycle import ( DEFAULT_PORT, @@ -48,39 +49,39 @@ from ...orchestrator.lifecycle import ( source_hash, ) -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. +# The control plane's own container + the dedicated control network the gateway +# reaches it over. The network is created `--internal`: the orchestrator and +# gateway join it, agents never do, so agents have no route to the control +# plane (PRD 0070 "Separating the planes"). Same name across docker + macOS. +ORCHESTRATOR_NAME = "bot-bottle-orchestrator" +ORCHESTRATOR_LABEL = "bot-bottle-orchestrator=1" +ORCHESTRATOR_NETWORK = "bot-bottle-orchestrator" ORCHESTRATOR_IMAGE = os.environ.get( "BOT_BOTTLE_ORCHESTRATOR_IMAGE", "bot-bottle-orchestrator:latest" ) ORCHESTRATOR_DOCKERFILE = "Dockerfile.orchestrator" +# Baked as a container label so `ensure_running` can detect whether the running +# orchestrator is executing the current bind-mounted source. +ORCHESTRATOR_SOURCE_HASH_LABEL = "bot-bottle-orchestrator-source-hash" -# 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. +# The bind-mount path for the live control-plane source inside the orchestrator +# container. PYTHONPATH points here so a code change takes effect on the next +# launch without an image rebuild. _SRC_IN_CONTAINER = "/bot-bottle-src" -# Bot-bottle host-root bind-mount inside the container (DB + state). The -# orchestrator control plane opens bot-bottle.db under here (via BOT_BOTTLE_ROOT -# -> host_db_path()); it is the ONLY process in the container with a file -# handle on it (PRD 0070 / issue #469). +# Bot-bottle host-root bind-mount (DB + state) inside the orchestrator. The +# control plane opens bot-bottle.db under here (via BOT_BOTTLE_ROOT -> +# host_db_path()); it is the ONLY container with a handle on it (issue #469). _ROOT_IN_CONTAINER = "/bot-bottle-root" +# The URL the gateway's data plane resolves policy against — the orchestrator +# container by name on the control network (docker DNS, container↔container). +ORCHESTRATOR_URL_IN_NETWORK = f"http://{ORCHESTRATOR_NAME}:{DEFAULT_PORT}" + +# Back-compat aliases: the split retired the combined `bot-bottle-infra` +# container, but the fixed-name constants some callers/tests import still map to +# the pair's public identity. +INFRA_NAME = GATEWAY_NAME # the container agents attribute against is the gateway + _HEALTH_POLL_SECONDS = 0.25 _HEALTH_REQUEST_TIMEOUT_SECONDS = 1.0 @@ -88,35 +89,46 @@ _REPO_ROOT = Path(__file__).resolve().parents[3] class DockerInfraService: - """Manages the single per-host docker infra container (control plane + - gateway). Callers only need `ensure_running()` + `url`. + """Brings up the per-host control plane + gateway as two containers. - `infra_name` / `infra_label` let callers 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).""" + `orchestrator_name` / `gateway_name` let callers run independent pairs on + one host without name collisions (e.g. isolated integration tests that + can't share the production singletons).""" def __init__( self, *, port: int = DEFAULT_PORT, network: str = GATEWAY_NETWORK, - image: str = INFRA_IMAGE, + control_network: str = ORCHESTRATOR_NETWORK, + orchestrator_image: str = ORCHESTRATOR_IMAGE, + gateway_image: str = GATEWAY_IMAGE, repo_root: Path = _REPO_ROOT, host_root: Path | None = None, - infra_name: str = INFRA_NAME, - infra_label: str = INFRA_LABEL, + orchestrator_name: str = ORCHESTRATOR_NAME, + orchestrator_label: str = ORCHESTRATOR_LABEL, + gateway_name: str = GATEWAY_NAME, ) -> None: self.port = port self.network = network - self.image = image + self.control_network = control_network + self.orchestrator_image = orchestrator_image + self.gateway_image = gateway_image self._repo_root = repo_root self._host_root = host_root or bot_bottle_root() - self._infra_name = infra_name - self._infra_label = infra_label + self._orchestrator_name = orchestrator_name + self._orchestrator_label = orchestrator_label + self._gateway_name = gateway_name + + @property + def gateway_name(self) -> str: + """The gateway container — the address the launch flow attributes agents + against (gateway IP, git-gate provisioning transport, CA fetch).""" + return self._gateway_name @property def url(self) -> str: - """Host-side control-plane URL (published loopback port).""" + """Host-side control-plane URL (the orchestrator's published loopback).""" return f"http://127.0.0.1:{self.port}" def is_healthy(self, *, timeout: float = _HEALTH_REQUEST_TIMEOUT_SECONDS) -> bool: @@ -130,36 +142,38 @@ class DockerInfraService: proc = run_docker(["docker", "ps", "--filter", f"name=^/{name}$", "--format", "{{.Names}}"]) return name in proc.stdout.split() - 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): + def _orchestrator_source_current(self, current_hash: str) -> bool: + """True iff the running orchestrator was started from the current + bind-mounted source.""" + if not self._container_running(self._orchestrator_name): return False proc = run_docker([ "docker", "inspect", "--format", - "{{ index .Config.Labels \"" + INFRA_SOURCE_HASH_LABEL + "\" }}", - self._infra_name, + "{{ index .Config.Labels \"" + ORCHESTRATOR_SOURCE_HASH_LABEL + "\" }}", + self._orchestrator_name, ]) if proc.returncode != 0: 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: + def _ensure_network(self, name: str, *, internal: bool) -> None: + if run_docker(["docker", "network", "inspect", name]).returncode == 0: return - proc = run_docker(["docker", "network", "create", self.network]) + argv = ["docker", "network", "create"] + if internal: + argv.append("--internal") + argv.append(name) + proc = run_docker(argv) if proc.returncode != 0 and "already exists" not in proc.stderr: - raise GatewayError( - f"gateway network {self.network} failed to create: {proc.stderr.strip()}" - ) + raise GatewayError(f"network {name} failed to create: {proc.stderr.strip()}") def _build_images(self) -> None: - """Build the gateway base, the orchestrator intermediate, then the - infra image. All are cache-aware: a no-op when nothing changed.""" + """Build the gateway (data plane) and orchestrator (control plane) + images. Cache-aware: a no-op when nothing changed. The combined + `Dockerfile.infra` is no longer built — the planes ship as two images.""" for tag, dockerfile in ( - (GATEWAY_IMAGE, GATEWAY_DOCKERFILE), - (ORCHESTRATOR_IMAGE, ORCHESTRATOR_DOCKERFILE), - (self.image, INFRA_DOCKERFILE), + (self.gateway_image, GATEWAY_DOCKERFILE), + (self.orchestrator_image, ORCHESTRATOR_DOCKERFILE), ): argv = ["docker", "build", "-t", tag, "-f", str(self._repo_root / dockerfile), @@ -170,92 +184,102 @@ class DockerInfraService: 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]) + def _run_orchestrator_container(self, current_hash: str) -> None: + """Start the lean control-plane container on the control network only, + published to host loopback for the CLI. Idempotent (clears a stale + fixed-name container first).""" + self._ensure_network(self.control_network, internal=True) + run_docker(["docker", "rm", "--force", self._orchestrator_name]) _signing_key = host_orchestrator_token() 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). - # gateway_init always starts the orchestrator on DEFAULT_PORT (8099) - # inside the container; self.port is the host-side published port. + "--name", self._orchestrator_name, + "--label", self._orchestrator_label, + "--label", f"{ORCHESTRATOR_SOURCE_HASH_LABEL}={current_hash}", + # Control network only — agents are never on it, so they have no + # route to the control plane (the L3 block, not just the JWT). + "--network", self.control_network, + # Host CLI reaches the control plane here (loopback only). The + # orchestrator listens on the fixed DEFAULT_PORT 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 on the host so it survives container - # recreation AND docker volume pruning (issue #450): every agent - # trusts this one CA, so a fresh one would break all running bottles. - "--volume", f"{host_gateway_ca_dir()}:{MITMPROXY_HOME}", - # Live control-plane source, mounted to a path that does not - # overlay the gateway's baked /app scripts. + # Live control-plane source (code changes without an image rebuild). "--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 signing key (orchestrator: verifies tokens) + the - # pre-minted `gateway` JWT (data-plane daemons: present it). gateway_init - # scopes each to its process, so a compromised data-plane daemon never - # sees the key and can't mint a `cli` token (issue #469 review). + # The signing key — held ONLY by the orchestrator (it verifies + # tokens); the gateway gets the pre-minted `gateway` JWT, never the + # key (issue #469). Bare `--env NAME` keeps the value off argv. "--env", ORCHESTRATOR_TOKEN_ENV, - "--env", ORCHESTRATOR_AUTH_JWT_ENV, - # 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, - ], env={ - **os.environ, - ORCHESTRATOR_TOKEN_ENV: _signing_key, - ORCHESTRATOR_AUTH_JWT_ENV: mint(ROLE_GATEWAY, _signing_key), - }) + self.orchestrator_image, + # Dockerfile.orchestrator ENTRYPOINT is `python3 -m bot_bottle.orchestrator`; + # these are its args. + "--host", "0.0.0.0", "--port", str(DEFAULT_PORT), "--broker", "stub", + ], env={**os.environ, ORCHESTRATOR_TOKEN_ENV: _signing_key}) if proc.returncode != 0: raise OrchestratorStartError( - f"infra container failed to start: {proc.stderr.strip()}" + f"orchestrator container failed to start: {proc.stderr.strip()}" ) + def _gateway(self) -> DockerGateway: + """The data-plane gateway, dual-homed on the agent network + the control + network, resolving policy against the orchestrator by name.""" + return DockerGateway( + self.gateway_image, + name=self._gateway_name, + network=self.network, + control_network=self.control_network, + orchestrator_url=ORCHESTRATOR_URL_IN_NETWORK, + build_context=self._repo_root, + dockerfile=None, # images are built by _build_images + ) + def ensure_running( self, *, startup_timeout: float = DEFAULT_STARTUP_TIMEOUT_SECONDS, ) -> str: - """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.""" + """Ensure the orchestrator + gateway containers are up; return the host + control-plane URL. Idempotent — a healthy orchestrator on current source + (and a current-image gateway) is left untouched. Raises + `OrchestratorStartError` on control-plane startup timeout.""" self._build_images() + self._ensure_network(self.network, internal=False) current_hash = source_hash(self._repo_root) - if self.is_healthy() and self._infra_source_current(current_hash): - return self.url + if not (self.is_healthy() and self._orchestrator_source_current(current_hash)): + 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}) + break + time.sleep(_HEALTH_POLL_SECONDS) + else: + raise OrchestratorStartError( + f"orchestrator at {self.url} did not become healthy " + f"within {startup_timeout:g}s" + ) - deadline = time.monotonic() + startup_timeout - while time.monotonic() < deadline: - if self.is_healthy(): - log.info("infra container healthy", context={"url": self.url}) - return self.url - time.sleep(_HEALTH_POLL_SECONDS) - raise OrchestratorStartError( - f"infra container at {self.url} did not become healthy within {startup_timeout:g}s" - ) + # Bring up (or refresh) the gateway once the control plane it resolves + # against is healthy. `ensure_running` is itself idempotent. + self._gateway().ensure_running() + return self.url def stop(self) -> None: - """Remove the infra container (idempotent).""" - run_docker(["docker", "rm", "--force", self._infra_name]) + """Remove both containers (idempotent).""" + run_docker(["docker", "rm", "--force", self._gateway_name]) + run_docker(["docker", "rm", "--force", self._orchestrator_name]) __all__ = [ "DockerInfraService", - "INFRA_NAME", - "INFRA_IMAGE", - "INFRA_SOURCE_HASH_LABEL", + "ORCHESTRATOR_NAME", + "ORCHESTRATOR_NETWORK", "ORCHESTRATOR_IMAGE", + "ORCHESTRATOR_SOURCE_HASH_LABEL", + "INFRA_NAME", ] diff --git a/bot_bottle/backend/docker/launch.py b/bot_bottle/backend/docker/launch.py index 4f68303..f4dfc83 100644 --- a/bot_bottle/backend/docker/launch.py +++ b/bot_bottle/backend/docker/launch.py @@ -157,10 +157,9 @@ def launch( ) # Step 4: install the SHARED gateway CA into the agent (replaces the - # per-bottle CA) — read it out of the running gateway. The consolidated - # flow runs the gateway data plane inside the per-host infra container - # (INFRA_NAME), so read the CA from there, not the legacy standalone - # gateway container name. + # per-bottle CA) — read it out of the running gateway container + # (INFRA_NAME now aliases the gateway container name; the orchestrator, + # split into its own container, holds no CA). ca_dir = egress_state_dir(plan.slug) / "gateway-ca" ca_dir.mkdir(parents=True, exist_ok=True) ca_file = ca_dir / "gateway-ca.pem" diff --git a/bot_bottle/orchestrator/rotate_ca.py b/bot_bottle/orchestrator/rotate_ca.py index 9303005..264c5e7 100644 --- a/bot_bottle/orchestrator/rotate_ca.py +++ b/bot_bottle/orchestrator/rotate_ca.py @@ -26,11 +26,11 @@ from pathlib import Path from ..backend.docker.util import run_docker from ..paths import host_gateway_ca_dir from ..gateway import GATEWAY_NAME, rotate_gateway_ca -from ..backend.docker.infra import INFRA_NAME -# The containers whose mitmproxy would still be serving the old CA from memory: -# the consolidated infra container and the standalone per-host gateway. -_GATEWAY_CONTAINERS = (INFRA_NAME, GATEWAY_NAME) +# The container whose mitmproxy would still be serving the old CA from memory: +# the per-host gateway (the data plane holds the CA; the split-out orchestrator +# never does). +_GATEWAY_CONTAINERS = (GATEWAY_NAME,) def _out(msg: str) -> None: diff --git a/tests/integration/test_orchestrator_docker_auth.py b/tests/integration/test_orchestrator_docker_auth.py index 559f08a..40dae8b 100644 --- a/tests/integration/test_orchestrator_docker_auth.py +++ b/tests/integration/test_orchestrator_docker_auth.py @@ -35,7 +35,6 @@ from tests._backend import skip_unless_backend # 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_backend("docker") @@ -71,17 +70,23 @@ class TestDockerOrchestratorAuthIntegration(unittest.TestCase): os.environ["BOT_BOTTLE_ROOT"] = cls._tmp.name cls.addClassCleanup(_restore_root) - infra_name = f"bot-bottle-infra-itest-{suffix}" + orchestrator_name = f"bot-bottle-orchestrator-itest-{suffix}" + gateway_name = f"bot-bottle-gateway-itest-{suffix}" network = f"bot-bottle-net-itest-{suffix}" + control_network = f"bot-bottle-ctrl-itest-{suffix}" host_root = Path(cls._tmp.name) cls.addClassCleanup( - cls._teardown_docker, infra_name, network, host_root + cls._teardown_docker, + orchestrator_name, gateway_name, network, control_network, host_root, ) cls.svc = DockerInfraService( - infra_name=infra_name, + orchestrator_name=orchestrator_name, + gateway_name=gateway_name, network=network, - image=_TEST_INFRA_IMAGE, + control_network=control_network, + orchestrator_image=_TEST_ORCHESTRATOR_IMAGE, + gateway_image=_TEST_GATEWAY_IMAGE, port=20000 + secrets.randbelow(10000), host_root=host_root, ) @@ -94,23 +99,26 @@ class TestDockerOrchestratorAuthIntegration(unittest.TestCase): @staticmethod def _teardown_docker( - infra_name: str, network: str, host_root: Path + orchestrator_name: str, gateway_name: str, + network: str, control_network: str, host_root: Path, ) -> None: - subprocess.run( - ["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 infra container (no USER directive) wrote the registry + for name in (gateway_name, orchestrator_name): + subprocess.run( + ["docker", "rm", "--force", name], + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=False, + ) + for net in (network, control_network): + subprocess.run( + ["docker", "network", "rm", net], + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=False, + ) + # The orchestrator 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_INFRA_IMAGE, "-R", + "--entrypoint", "chown", _TEST_ORCHESTRATOR_IMAGE, "-R", f"{os.getuid()}:{os.getgid()}", "/r"], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=False, ) diff --git a/tests/unit/test_backend_secret_reprovision.py b/tests/unit/test_backend_secret_reprovision.py index 33e9dc5..3c25092 100644 --- a/tests/unit/test_backend_secret_reprovision.py +++ b/tests/unit/test_backend_secret_reprovision.py @@ -82,8 +82,10 @@ class TestMacosReprovision(unittest.TestCase): class TestDockerReprovision(unittest.TestCase): def test_maps_network_containers_to_keys(self) -> None: + # The gateway container (excluded — it's on the data network but isn't + # an agent) + one agent + a malformed line. inspect = _proc(stdout=( - "bot-bottle-infra 172.18.0.2/16\n" + "bot-bottle-orch-gateway 172.18.0.2/16\n" "bot-bottle-a 172.18.0.3/16\n" "malformed\n" )) diff --git a/tests/unit/test_docker_infra.py b/tests/unit/test_docker_infra.py index bf85eb4..1adc093 100644 --- a/tests/unit/test_docker_infra.py +++ b/tests/unit/test_docker_infra.py @@ -1,4 +1,9 @@ -"""Unit: infra container lifecycle — idempotent singleton (PRD 0070).""" +"""Unit: orchestrator + gateway container lifecycle (PRD 0070 plane split). + +`DockerInfraService` brings up two containers — the lean orchestrator on the +`--internal` control network + host loopback, and the dual-homed gateway. These +tests isolate the orchestrator-container logic by mocking `_gateway` (the +gateway runs via `DockerGateway`, exercised in its own test).""" from __future__ import annotations @@ -10,15 +15,16 @@ from unittest.mock import MagicMock, Mock, patch from bot_bottle.gateway import GatewayError from bot_bottle.backend.docker.infra import ( - INFRA_NAME, - INFRA_SOURCE_HASH_LABEL, + ORCHESTRATOR_NAME, + ORCHESTRATOR_NETWORK, + ORCHESTRATOR_SOURCE_HASH_LABEL, DockerInfraService, ) +from bot_bottle.gateway import GATEWAY_NAME from bot_bottle.orchestrator.lifecycle import ( OrchestratorStartError, source_hash, ) -from bot_bottle.paths import GATEWAY_CA_DIRNAME from tests.unit import use_bottle_root _URLOPEN = "bot_bottle.backend.docker.infra.urllib.request.urlopen" @@ -43,168 +49,147 @@ class TestDockerInfraService(unittest.TestCase): self.addCleanup(self._tmp.cleanup) self.addCleanup(use_bottle_root(Path(self._tmp.name))) self.svc = DockerInfraService(port=8099) + # Isolate the orchestrator-container logic: the gateway is brought up by + # DockerGateway (its own module/run_docker), tested separately. + self.gw = MagicMock() + patcher = patch.object(self.svc, "_gateway", return_value=self.gw) + patcher.start() + self.addCleanup(patcher.stop) def test_url(self) -> None: self.assertEqual("http://127.0.0.1:8099", self.svc.url) + def test_gateway_name(self) -> None: + self.assertEqual(GATEWAY_NAME, self.svc.gateway_name) + def test_is_healthy(self) -> None: with patch(_URLOPEN, return_value=_health(200)): self.assertTrue(self.svc.is_healthy()) with patch(_URLOPEN, side_effect=urllib.error.URLError("refused")): self.assertFalse(self.svc.is_healthy()) - def test_ensure_running_noop_when_healthy_and_source_unchanged(self) -> None: - # A healthy container on current source is left alone — recreating it - # on every launch drops in-memory egress tokens (#381). + def test_noop_when_healthy_and_source_unchanged(self) -> None: + # A healthy orchestrator on current source is left alone — recreating it + # on every launch drops in-memory egress tokens (#381). The gateway is + # still refreshed (idempotent). 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=INFRA_NAME) + return _proc(stdout=ORCHESTRATOR_NAME) if argv[:2] == ["docker", "inspect"]: return _proc(stdout=current) return _proc() with patch(_URLOPEN, return_value=_health(200)), \ - patch(_RUN, side_effect=fake), patch(_SLEEP): + patch(_RUN, side_effect=fake) as run, patch(_SLEEP): self.assertEqual(self.svc.url, self.svc.ensure_running()) - runs = [c for c in calls if c[:2] == ["docker", "run"]] - rms = [c for c in calls if c[:3] == ["docker", "rm", "--force"] and INFRA_NAME in c] + runs = [c.args[0] for c in run.call_args_list if c.args[0][:2] == ["docker", "run"]] self.assertEqual([], runs) - self.assertEqual([], rms) - - def test_ensure_running_recreates_when_source_changed(self) -> None: - calls: list[list[str]] = [] + self.gw.ensure_running.assert_called_once_with() + def test_recreates_orchestrator_when_source_changed(self) -> None: def fake(argv: list[str], **_kw: object) -> Mock: - calls.append(argv) if argv[:2] == ["docker", "ps"]: - return _proc(stdout=INFRA_NAME) + return _proc(stdout=ORCHESTRATOR_NAME) if argv[:2] == ["docker", "inspect"]: return _proc(stdout="stale-hash") return _proc() with patch(_URLOPEN, return_value=_health(200)), \ - patch(_RUN, side_effect=fake), patch(_SLEEP): + patch(_RUN, side_effect=fake) as run, patch(_SLEEP): self.assertEqual(self.svc.url, self.svc.ensure_running()) - runs = [c for c in calls if c[:2] == ["docker", "run"]] + runs = [c.args[0] for c in run.call_args_list if c.args[0][:2] == ["docker", "run"]] self.assertEqual(1, len(runs)) - self.assertIn(INFRA_NAME, runs[0]) + self.assertIn(ORCHESTRATOR_NAME, runs[0]) current = source_hash(self.svc._repo_root) - self.assertIn(f"{INFRA_SOURCE_HASH_LABEL}={current}", runs[0]) - - def test_ensure_running_starts_infra_container_when_absent(self) -> None: - calls: list[list[str]] = [] + self.assertIn(f"{ORCHESTRATOR_SOURCE_HASH_LABEL}={current}", runs[0]) + def test_starts_orchestrator_when_absent(self) -> None: 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): + patch(_RUN, side_effect=fake) as run, patch(_SLEEP): self.assertEqual(self.svc.url, self.svc.ensure_running()) - runs = [c for c in calls if c[:2] == ["docker", "run"]] + runs = [c.args[0] for c in run.call_args_list if c.args[0][: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.assertIn(ORCHESTRATOR_NAME, argv) + # Control network only — agents are never on it. + self.assertEqual(ORCHESTRATOR_NETWORK, argv[argv.index("--network") + 1]) + # Published on loopback for the CLI, mapped to the fixed internal 8099. 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. - daemons_flag = "BOT_BOTTLE_GATEWAY_DAEMONS=egress,git-http,supervise,orchestrator" - self.assertIn("orchestrator", argv[argv.index(daemons_flag)]) - # The mitmproxy CA persists on a HOST bind-mount under the app-data root - # (not a docker named volume `docker volume prune` would wipe — #450), so - # a restarted infra container keeps the CA every running bottle trusts. - ca_mounts = [a for a in argv if a.endswith(":/home/mitmproxy/.mitmproxy")] - self.assertEqual(1, len(ca_mounts)) - src = ca_mounts[0].rsplit(":", 1)[0] - self.assertTrue(src.startswith(self._tmp.name), src) - self.assertTrue(src.endswith("/" + GATEWAY_CA_DIRNAME), src) - - def test_ensure_running_builds_all_images(self) -> None: - calls: list[list[str]] = [] + # The lean control plane: no mitmproxy CA mount, no gateway daemons. + self.assertFalse([a for a in argv if a.endswith(":/home/mitmproxy/.mitmproxy")]) + self.assertNotIn("BOT_BOTTLE_GATEWAY_DAEMONS", " ".join(argv)) + # Orchestrator entrypoint args (image ENTRYPOINT is `-m bot_bottle.orchestrator`). + self.assertIn("--broker", argv) + self.assertIn("stub", argv) + # Gateway is brought up after the control plane is healthy. + self.gw.ensure_running.assert_called_once_with() + def test_builds_gateway_and_orchestrator_images(self) -> None: 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): + patch(_RUN, side_effect=fake) as run, patch(_SLEEP): self.svc.ensure_running() - builds = [c for c in calls if c[:2] == ["docker", "build"]] - # Gateway base + orchestrator intermediate + infra image — all three built. - self.assertEqual(3, len(builds)) + builds = [c.args[0] for c in run.call_args_list if c.args[0][:2] == ["docker", "build"]] + # Two images now (the combined Dockerfile.infra is retired). + self.assertEqual(2, len(builds)) dockerfiles = [next(a for a in b if "Dockerfile" in a) for b in builds] 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.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 = DockerInfraService(port=20001) - with patch(_URLOPEN, side_effect=[urllib.error.URLError("down"), _health(200)]), \ - patch(_RUN, side_effect=fake), patch(_SLEEP): + with patch.object(svc, "_gateway", return_value=MagicMock()), \ + patch(_URLOPEN, side_effect=[urllib.error.URLError("down"), _health(200)]), \ + patch(_RUN, side_effect=fake) as run, patch(_SLEEP): svc.ensure_running() - runs = [c for c in calls if c[:2] == ["docker", "run"]] - argv = runs[0] + argv = next(c.args[0] for c in run.call_args_list if c.args[0][:2] == ["docker", "run"]) 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: + def test_raises_on_control_plane_timeout(self) -> None: with patch(_URLOPEN, side_effect=urllib.error.URLError("down")), \ - patch(_RUN, return_value=Mock(returncode=0, stdout="", stderr="")), \ + patch(_RUN, return_value=_proc()), \ 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_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) + return _proc(stdout=ORCHESTRATOR_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): + patch(_RUN, side_effect=fake) as run, patch(_SLEEP): self.svc.ensure_running() - # no docker run — the working container was left alone + runs = [c.args[0] for c in run.call_args_list if c.args[0][:2] == ["docker", "run"]] + self.assertEqual([], runs) 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")): + patch(_RUN, return_value=_proc(returncode=1, stderr="no space left")): 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 test_creates_control_network_internal(self) -> None: 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"]: @@ -212,19 +197,23 @@ class TestDockerInfraService(unittest.TestCase): return _proc() with patch(_URLOPEN, side_effect=[urllib.error.URLError("down"), _health(200)]), \ - patch(_RUN, side_effect=fake), patch(_SLEEP): + patch(_RUN, side_effect=fake) as run, patch(_SLEEP): self.svc.ensure_running() - creates = [c for c in calls if c[:3] == ["docker", "network", "create"]] - self.assertEqual(1, len(creates)) + creates = [c.args[0] for c in run.call_args_list if c.args[0][:3] == ["docker", "network", "create"]] + # The control network is created --internal; the data network isn't. + control = [c for c in creates if ORCHESTRATOR_NETWORK in c] + self.assertEqual(1, len(control)) + self.assertIn("--internal", control[0]) - def test_stop_removes_infra_container(self) -> None: + def test_stop_removes_both_containers(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(INFRA_NAME in a for a in rms)) + self.assertTrue(any(GATEWAY_NAME in a for a in rms)) + self.assertTrue(any(ORCHESTRATOR_NAME in a for a in rms)) if __name__ == "__main__":