From bd8a146a463be8b52011b5ca7ab7935a9804e646 Mon Sep 17 00:00:00 2001 From: claude Date: Mon, 27 Jul 2026 01:23:25 +0000 Subject: [PATCH] fix(docker): heal a poisoned IPv6 gateway network, not just avoid creating one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .gitea/workflows/test.yml | 14 +++++++ bot_bottle/backend/docker/gateway.py | 25 +++++++++-- tests/unit/test_orchestrator_gateway.py | 55 +++++++++++++++++++++++++ 3 files changed, 90 insertions(+), 4 deletions(-) diff --git a/.gitea/workflows/test.yml b/.gitea/workflows/test.yml index 09ee4e9c..dc6f2c7a 100644 --- a/.gitea/workflows/test.yml +++ b/.gitea/workflows/test.yml @@ -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 diff --git a/bot_bottle/backend/docker/gateway.py b/bot_bottle/backend/docker/gateway.py index 3014efda..71d630d4 100644 --- a/bot_bottle/backend/docker/gateway.py +++ b/bot_bottle/backend/docker/gateway.py @@ -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: " diff --git a/tests/unit/test_orchestrator_gateway.py b/tests/unit/test_orchestrator_gateway.py index 61d16800..cc48e8c2 100644 --- a/tests/unit/test_orchestrator_gateway.py +++ b/tests/unit/test_orchestrator_gateway.py @@ -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())