fix(docker): heal a poisoned IPv6 gateway network, not just avoid creating one
test / integration-docker (pull_request) Successful in 1m15s
test / image-input-builds (pull_request) Successful in 1m14s
lint / lint (push) Successful in 3m28s
tracker-policy-pr / check-pr (pull_request) Failing after 10m28s
test / unit (pull_request) Failing after 10m39s
prd-number-check / require-numbered-prds (pull_request) Failing after 10m45s
test / coverage (pull_request) Has been skipped
test / integration-docker (pull_request) Successful in 1m15s
test / image-input-builds (pull_request) Successful in 1m14s
lint / lint (push) Successful in 3m28s
tracker-policy-pr / check-pr (pull_request) Failing after 10m28s
test / unit (pull_request) Failing after 10m39s
prd-number-check / require-numbered-prds (pull_request) Failing after 10m45s
test / coverage (pull_request) Has been skipped
PR #515 stopped bot-bottle from *creating* a gateway network with a malformed IPv6 subnet (--ipv6=false), but it can't recover a network that is *already* poisoned. On a daemon that default-enables IPv6, the fixed-name `bot-bottle-gateway` network gets an `fdd0::/64` subnet whose `::1/64` gateway trips docker's own netip.ParseAddr, so `docker network inspect`/`ls` exit non-zero — poisoning every command that reads networks. Such a network can survive on a shared runner from a pre-fix or concurrent launch. `_ensure_network` never healed it: its migrate/recreate branch only ran when `network inspect` *succeeded*, but a poisoned network makes inspect *fail*, so the code fell through to `network create`, which no-ops on "already exists" — leaving the poison in place. The next subnet read then failed with ConsolidatedLaunchError (test_multitenant_isolation), and a bare `docker network ls` failed too (test_orphan_cleanup). Fix, two parts: - gateway.py: when `network inspect` fails for a reason other than "no such network", treat the network as poisoned and force-remove + recreate it IPv4-only. Absent-vs-poisoned is distinguished by the inspect stderr. - test.yml: add an integration-docker preflight that drops the leftover gateway network (and its container) before the suite, so direct `network ls`/`inspect` calls in tests are clean even on a daemon where the in-code heal can't run because inspect itself is what's broken. Adds unit coverage for the poisoned-heal and the absent-create split. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -102,6 +102,20 @@ jobs:
|
||||
python3 --version
|
||||
python3 cli.py backend status --backend=docker
|
||||
|
||||
- name: Preflight — clear any leftover poisoned gateway network
|
||||
run: |
|
||||
# The gateway network has a fixed name and persists across jobs on
|
||||
# this shared runner. A pre-fix or concurrent launch can leave it with
|
||||
# a malformed IPv6 subnet that trips docker's own ParseAddr in
|
||||
# `network inspect` (see PR #515); the code now self-heals it, but the
|
||||
# heal can't run if `network inspect` is what's broken on some daemon
|
||||
# versions. Drop the network here so this run recreates it IPv4-only.
|
||||
# Remove the attached gateway container first (else `network rm` fails
|
||||
# on active endpoints); both are recreated by ensure_running. Harmless
|
||||
# when absent.
|
||||
docker rm --force bot-bottle-orch-gateway 2>/dev/null || true
|
||||
docker network rm bot-bottle-gateway 2>/dev/null || true
|
||||
|
||||
- name: Run integration tests (docker) with coverage
|
||||
env:
|
||||
BOT_BOTTLE_BACKEND: docker
|
||||
|
||||
@@ -140,12 +140,29 @@ class DockerGateway(Gateway):
|
||||
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.
|
||||
# Inspectable but mislabelled: the stale auto-IPAM network created
|
||||
# by older releases. Replace it below.
|
||||
stale = True
|
||||
else:
|
||||
# inspect failed: either the network is absent (create it below) or
|
||||
# it exists but docker can't parse it. A daemon that default-enables
|
||||
# IPv6 attaches an fdd0::/64 subnet whose `::1/64` gateway trips
|
||||
# docker's own netip.ParseAddr in `network inspect`/`ls`, so the
|
||||
# command exits non-zero. A fixed release never *creates* such a
|
||||
# network, but one can survive on a shared host from an older or
|
||||
# concurrent launch — and `--ipv6=false` alone can't heal it, since
|
||||
# the create below only no-ops on "already exists". Force-replace it
|
||||
# so later reads (e.g. `_network_cidr` pinning a source IP) stop
|
||||
# failing. An absent network's inspect error is "No such network";
|
||||
# anything else means it exists but is poisoned.
|
||||
err = inspected.stderr.lower()
|
||||
stale = "no such network" not in err and "not found" not in err
|
||||
if stale:
|
||||
# Migrate the stale/poisoned network. 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:
|
||||
if removed.returncode != 0 and "no such network" not in removed.stderr.lower():
|
||||
raise GatewayError(
|
||||
f"gateway network {self.network} needs explicit subnet "
|
||||
f"{self._subnet} but could not be replaced: "
|
||||
|
||||
@@ -248,6 +248,61 @@ class TestDockerGateway(unittest.TestCase):
|
||||
calls,
|
||||
)
|
||||
|
||||
def test_ensure_running_replaces_poisoned_ipv6_network(self) -> None:
|
||||
# A daemon that default-enables IPv6 leaves the gateway network with a
|
||||
# malformed fdd0::/64 gateway, so `docker network inspect` exits
|
||||
# non-zero with a ParseAddr error (not "No such network"). `--ipv6=false`
|
||||
# can't heal an already-poisoned network — the create just no-ops on
|
||||
# "already exists" — so _ensure_network must force-remove and recreate
|
||||
# it, else every later subnet read keeps failing.
|
||||
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(
|
||||
returncode=1,
|
||||
stderr='ParseAddr("fdd0:0:0:4::1/64"): unexpected character, '
|
||||
'want colon (at "/64")',
|
||||
)
|
||||
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)
|
||||
creates = [c for c in calls if c[:3] == ["docker", "network", "create"]]
|
||||
self.assertEqual(
|
||||
[[
|
||||
"docker", "network", "create",
|
||||
"--ipv6=false",
|
||||
"--subnet", DEFAULT_GATEWAY_SUBNET,
|
||||
"--label",
|
||||
f"bot-bottle.gateway-subnet={DEFAULT_GATEWAY_SUBNET}",
|
||||
self.sc.network,
|
||||
]],
|
||||
creates,
|
||||
)
|
||||
|
||||
def test_ensure_running_creates_network_when_inspect_reports_absent(self) -> None:
|
||||
# The absent case (inspect fails with "No such network") must NOT try to
|
||||
# remove anything — it just creates. Guards the poisoned-vs-absent split.
|
||||
calls: list[list[str]] = []
|
||||
|
||||
def fake(argv: list[str], **_kw: object) -> Mock:
|
||||
calls.append(argv)
|
||||
if argv[:3] == ["docker", "network", "inspect"]:
|
||||
return _proc(returncode=1, stderr="Error: No such network: x")
|
||||
return _proc(stdout="") if argv[:2] == ["docker", "ps"] else _proc()
|
||||
|
||||
with patch(_RUN_DOCKER, side_effect=fake):
|
||||
self.sc.connect_to_orchestrator(_ORCH_URL, _TOKEN)
|
||||
self.assertNotIn(["docker", "network", "rm", self.sc.network], calls)
|
||||
creates = [c for c in calls if c[:3] == ["docker", "network", "create"]]
|
||||
self.assertEqual(1, len(creates))
|
||||
|
||||
def test_ca_cert_pem_reads_from_container(self) -> None:
|
||||
with patch(_RUN_DOCKER, return_value=_proc(stdout=_CA_PEM)) as m:
|
||||
self.assertEqual(_CA_PEM, self.sc.ca_cert_pem())
|
||||
|
||||
Reference in New Issue
Block a user