fix(docker): configure the attributed gateway subnet
tracker-policy-pr / check-pr (pull_request) Successful in 12s
prd-number-check / require-numbered-prds (pull_request) Successful in 42s
lint / lint (push) Successful in 1m4s
test / unit (pull_request) Successful in 57s
test / integration-docker (pull_request) Failing after 1m5s
test / coverage (pull_request) Has been skipped
tracker-policy-pr / check-pr (pull_request) Successful in 12s
prd-number-check / require-numbered-prds (pull_request) Successful in 42s
lint / lint (push) Successful in 1m4s
test / unit (pull_request) Successful in 57s
test / integration-docker (pull_request) Failing after 1m5s
test / coverage (pull_request) Has been skipped
This commit is contained in:
@@ -60,6 +60,9 @@ jobs:
|
|||||||
|
|
||||||
integration-docker:
|
integration-docker:
|
||||||
runs-on: ubuntu-latest
|
runs-on: ubuntu-latest
|
||||||
|
concurrency:
|
||||||
|
group: integration-docker-infra
|
||||||
|
cancel-in-progress: false
|
||||||
steps:
|
steps:
|
||||||
- name: Checkout
|
- name: Checkout
|
||||||
uses: actions/checkout@v4
|
uses: actions/checkout@v4
|
||||||
|
|||||||
@@ -81,6 +81,9 @@ jobs:
|
|||||||
|
|
||||||
integration-docker:
|
integration-docker:
|
||||||
runs-on: ubuntu-latest
|
runs-on: ubuntu-latest
|
||||||
|
concurrency:
|
||||||
|
group: integration-docker-infra
|
||||||
|
cancel-in-progress: false
|
||||||
steps:
|
steps:
|
||||||
- name: Checkout
|
- name: Checkout
|
||||||
uses: actions/checkout@v4
|
uses: actions/checkout@v4
|
||||||
|
|||||||
@@ -17,6 +17,10 @@ from ...gateway import (
|
|||||||
DEFAULT_CA_TIMEOUT_SECONDS, CA_POLL_SECONDS, GATEWAY_CA_CERT, GatewayError
|
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):
|
class DockerGateway(Gateway):
|
||||||
"""The consolidated gateway as a single, fixed-name Docker container.
|
"""The consolidated gateway as a single, fixed-name Docker container.
|
||||||
|
|
||||||
@@ -36,6 +40,7 @@ class DockerGateway(Gateway):
|
|||||||
dockerfile: str | None = GATEWAY_DOCKERFILE,
|
dockerfile: str | None = GATEWAY_DOCKERFILE,
|
||||||
host_port_bindings: tuple[int, ...] = (),
|
host_port_bindings: tuple[int, ...] = (),
|
||||||
ca_mount_source: str | Path | None = None,
|
ca_mount_source: str | Path | None = None,
|
||||||
|
subnet: str | None = None,
|
||||||
) -> None:
|
) -> None:
|
||||||
self.image_ref = image_ref
|
self.image_ref = image_ref
|
||||||
self.name = name
|
self.name = name
|
||||||
@@ -60,6 +65,11 @@ class DockerGateway(Gateway):
|
|||||||
# backend's dev-harness gateway so VMs can reach it via their TAP link;
|
# 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.
|
# Docker's DNAT + the nft `ct status dnat accept` rule handle the rest.
|
||||||
self._host_port_bindings = host_port_bindings
|
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()
|
configured_ca = os.environ.get("BOT_BOTTLE_DOCKER_CA_MOUNT", "").strip()
|
||||||
self._ca_mount_source = str(
|
self._ca_mount_source = str(
|
||||||
ca_mount_source or configured_ca or host_gateway_ca_dir()
|
ca_mount_source or configured_ca or host_gateway_ca_dir()
|
||||||
@@ -114,10 +124,34 @@ class DockerGateway(Gateway):
|
|||||||
def _ensure_network(self) -> None:
|
def _ensure_network(self) -> None:
|
||||||
"""Create the shared gateway network if it doesn't exist. Idempotent —
|
"""Create the shared gateway network if it doesn't exist. Idempotent —
|
||||||
a concurrent create loses harmlessly (the loser sees 'already exists').
|
a concurrent create loses harmlessly (the loser sees 'already exists').
|
||||||
Docker picks the subnet; the launcher reads it back to allocate IPs."""
|
The explicit subnet is required because bottle attribution pins source
|
||||||
if run_docker(["docker", "network", "inspect", self.network]).returncode == 0:
|
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
|
return
|
||||||
proc = run_docker(["docker", "network", "create", self.network])
|
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:
|
if proc.returncode != 0 and "already exists" not in proc.stderr:
|
||||||
raise GatewayError(
|
raise GatewayError(
|
||||||
f"gateway network {self.network} failed to create: {proc.stderr.strip()}"
|
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),
|
# Recreate when the running container's image is stale (a rebuild),
|
||||||
# so source changes to the gateway's flat daemons take effect — not
|
# so source changes to the gateway's flat daemons take effect — not
|
||||||
# just when the container is absent.
|
# just when the container is absent.
|
||||||
|
self._ensure_network()
|
||||||
if self.is_running() and self._running_image_is_current():
|
if self.is_running() and self._running_image_is_current():
|
||||||
return
|
return
|
||||||
self._ensure_network()
|
|
||||||
# Clear any stale (stopped OR outdated-image) container holding the
|
# Clear any stale (stopped OR outdated-image) container holding the
|
||||||
# fixed name, then start fresh. `rm --force` on an absent name is a
|
# fixed name, then start fresh. `rm --force` on an absent name is a
|
||||||
# tolerated no-op.
|
# tolerated no-op.
|
||||||
|
|||||||
@@ -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
|
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
|
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.
|
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
|
`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,
|
tests must execute and none may skip. This includes the real gateway-image,
|
||||||
|
|||||||
@@ -7,7 +7,10 @@ import unittest
|
|||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from unittest.mock import Mock, patch
|
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 (
|
from bot_bottle.gateway import (
|
||||||
GATEWAY_CA_CERT,
|
GATEWAY_CA_CERT,
|
||||||
GATEWAY_NAME,
|
GATEWAY_NAME,
|
||||||
@@ -209,7 +212,38 @@ class TestDockerGateway(unittest.TestCase):
|
|||||||
with patch(_RUN_DOCKER, side_effect=fake):
|
with patch(_RUN_DOCKER, side_effect=fake):
|
||||||
self.sc.connect_to_orchestrator(_ORCH_URL, _TOKEN)
|
self.sc.connect_to_orchestrator(_ORCH_URL, _TOKEN)
|
||||||
creates = [c for c in calls if c[:3] == ["docker", "network", "create"]]
|
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="<no value>\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:
|
def test_ca_cert_pem_reads_from_container(self) -> None:
|
||||||
with patch(_RUN_DOCKER, return_value=_proc(stdout=_CA_PEM)) as m:
|
with patch(_RUN_DOCKER, return_value=_proc(stdout=_CA_PEM)) as m:
|
||||||
|
|||||||
Reference in New Issue
Block a user