From d744bec7b15c8b1189ab05cbc399d89b9902d536 Mon Sep 17 00:00:00 2001 From: codex Date: Sun, 26 Jul 2026 08:35:37 +0000 Subject: [PATCH] fix(docker): configure the attributed gateway subnet --- .gitea/workflows/pre-release-test.yml | 3 ++ .gitea/workflows/test.yml | 3 ++ bot_bottle/backend/docker/gateway.py | 44 ++++++++++++++++++++++--- docs/ci.md | 4 +++ tests/unit/test_orchestrator_gateway.py | 38 +++++++++++++++++++-- 5 files changed, 85 insertions(+), 7 deletions(-) diff --git a/.gitea/workflows/pre-release-test.yml b/.gitea/workflows/pre-release-test.yml index 73571b2b..4263914f 100644 --- a/.gitea/workflows/pre-release-test.yml +++ b/.gitea/workflows/pre-release-test.yml @@ -60,6 +60,9 @@ jobs: integration-docker: runs-on: ubuntu-latest + concurrency: + group: integration-docker-infra + cancel-in-progress: false steps: - name: Checkout uses: actions/checkout@v4 diff --git a/.gitea/workflows/test.yml b/.gitea/workflows/test.yml index 26a60e36..963e7d0f 100644 --- a/.gitea/workflows/test.yml +++ b/.gitea/workflows/test.yml @@ -81,6 +81,9 @@ jobs: integration-docker: runs-on: ubuntu-latest + concurrency: + group: integration-docker-infra + cancel-in-progress: false steps: - name: Checkout uses: actions/checkout@v4 diff --git a/bot_bottle/backend/docker/gateway.py b/bot_bottle/backend/docker/gateway.py index 6212c63b..689ba5f0 100644 --- a/bot_bottle/backend/docker/gateway.py +++ b/bot_bottle/backend/docker/gateway.py @@ -17,6 +17,10 @@ from ...gateway import ( DEFAULT_CA_TIMEOUT_SECONDS, CA_POLL_SECONDS, GATEWAY_CA_CERT, GatewayError ) +DEFAULT_GATEWAY_SUBNET = "10.242.255.0/24" +_GATEWAY_SUBNET_LABEL = "bot-bottle.gateway-subnet" + + class DockerGateway(Gateway): """The consolidated gateway as a single, fixed-name Docker container. @@ -36,6 +40,7 @@ class DockerGateway(Gateway): dockerfile: str | None = GATEWAY_DOCKERFILE, host_port_bindings: tuple[int, ...] = (), ca_mount_source: str | Path | None = None, + subnet: str | None = None, ) -> None: self.image_ref = image_ref self.name = name @@ -60,6 +65,11 @@ class DockerGateway(Gateway): # backend's dev-harness gateway so VMs can reach it via their TAP link; # Docker's DNAT + the nft `ct status dnat accept` rule handle the rest. self._host_port_bindings = host_port_bindings + self._subnet = ( + subnet + or os.environ.get("BOT_BOTTLE_DOCKER_GATEWAY_SUBNET", "").strip() + or DEFAULT_GATEWAY_SUBNET + ) configured_ca = os.environ.get("BOT_BOTTLE_DOCKER_CA_MOUNT", "").strip() self._ca_mount_source = str( ca_mount_source or configured_ca or host_gateway_ca_dir() @@ -114,10 +124,34 @@ class DockerGateway(Gateway): def _ensure_network(self) -> None: """Create the shared gateway network if it doesn't exist. Idempotent — a concurrent create loses harmlessly (the loser sees 'already exists'). - Docker picks the subnet; the launcher reads it back to allocate IPs.""" - if run_docker(["docker", "network", "inspect", self.network]).returncode == 0: - return - proc = run_docker(["docker", "network", "create", self.network]) + The explicit subnet is required because bottle attribution pins source + IPs; Docker rejects static endpoint addresses on an auto-IPAM network.""" + inspected = run_docker([ + "docker", "network", "inspect", + "--format", f'{{{{index .Labels "{_GATEWAY_SUBNET_LABEL}"}}}}', + self.network, + ]) + if inspected.returncode == 0: + marker = inspected.stdout.strip() + if marker in {"", self._subnet}: + return + if inspected.returncode == 0: + # Migrate the stale auto-IPAM network created by older releases. + # Removing the fixed gateway is safe here: this launch recreates it. + run_docker(["docker", "rm", "--force", self.name]) + removed = run_docker(["docker", "network", "rm", self.network]) + if removed.returncode != 0: + raise GatewayError( + f"gateway network {self.network} needs explicit subnet " + f"{self._subnet} but could not be replaced: " + f"{removed.stderr.strip()}" + ) + proc = run_docker([ + "docker", "network", "create", + "--subnet", self._subnet, + "--label", f"{_GATEWAY_SUBNET_LABEL}={self._subnet}", + 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()}" @@ -148,9 +182,9 @@ class DockerGateway(Gateway): # Recreate when the running container's image is stale (a rebuild), # so source changes to the gateway's flat daemons take effect — not # just when the container is absent. + self._ensure_network() if self.is_running() and self._running_image_is_current(): return - self._ensure_network() # Clear any stale (stopped OR outdated-image) container holding the # fixed name, then start fresh. `rm --force` on an absent name is a # tolerated no-op. diff --git a/docs/ci.md b/docs/ci.md index 6b9e23fe..17c019f4 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -12,6 +12,10 @@ reaches control-plane siblings through the job's Docker network and uses named Docker volumes for orchestrator/CA state the host daemon must mount. The orchestrator runs the package baked into the image built from the checkout; it does not bind the job container's invisible workspace into a sibling container. +Docker integration jobs share fixed singleton names, so required and manual +runs use one non-cancelling concurrency group. The shared agent/gateway network +has an explicit subnet, which Docker requires for the pinned source IPs used as +the isolation/attribution key. `scripts.unittest_gate` enforces the Docker job's contract: all 22 integration tests must execute and none may skip. This includes the real gateway-image, diff --git a/tests/unit/test_orchestrator_gateway.py b/tests/unit/test_orchestrator_gateway.py index 20d7039a..7c8cbd2e 100644 --- a/tests/unit/test_orchestrator_gateway.py +++ b/tests/unit/test_orchestrator_gateway.py @@ -7,7 +7,10 @@ import unittest from pathlib import Path from unittest.mock import Mock, patch -from bot_bottle.backend.docker.gateway import DockerGateway +from bot_bottle.backend.docker.gateway import ( + DEFAULT_GATEWAY_SUBNET, + DockerGateway, +) from bot_bottle.gateway import ( GATEWAY_CA_CERT, GATEWAY_NAME, @@ -209,7 +212,38 @@ class TestDockerGateway(unittest.TestCase): with patch(_RUN_DOCKER, side_effect=fake): self.sc.connect_to_orchestrator(_ORCH_URL, _TOKEN) creates = [c for c in calls if c[:3] == ["docker", "network", "create"]] - self.assertEqual([["docker", "network", "create", self.sc.network]], creates) + self.assertEqual( + [[ + "docker", "network", "create", + "--subnet", DEFAULT_GATEWAY_SUBNET, + "--label", + f"bot-bottle.gateway-subnet={DEFAULT_GATEWAY_SUBNET}", + self.sc.network, + ]], + creates, + ) + + def test_ensure_running_replaces_stale_auto_ipam_network(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", "network", "inspect"]: + return _proc(stdout="\n") + return _proc() + + with patch(_RUN_DOCKER, side_effect=fake): + self.sc.connect_to_orchestrator(_ORCH_URL, _TOKEN) + self.assertIn( + ["docker", "rm", "--force", self.sc.name], + calls, + ) + self.assertIn( + ["docker", "network", "rm", self.sc.network], + calls, + ) def test_ca_cert_pem_reads_from_container(self) -> None: with patch(_RUN_DOCKER, return_value=_proc(stdout=_CA_PEM)) as m: