From 563469bc5e27ae6132195e4e08ad152c98751c4e Mon Sep 17 00:00:00 2001 From: didericis Date: Tue, 14 Jul 2026 02:07:35 -0400 Subject: [PATCH] fix(orchestrator): recreate the gateway when its image is stale MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Container-level counterpart to the ensure_built cache-aware fix. ensure_built now rebuilds the gateway image on a source change, but ensure_running still reused an already-running container built from the OLD image — so a rebuild (even BOT_BOTTLE_NO_CACHE) never took effect on a warm gateway, and it kept running the pre-fix flat daemons (this is why token injection stayed broken after the rebuild). ensure_running now compares the running container's image to the current image and recreates on mismatch. Validated on docker: a stale gateway is detected and recreated with the current image + new egress addon. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01WBMWTEtQdJ4W5UrWuLHCck --- bot_bottle/orchestrator/gateway.py | 22 ++++++++++++-- tests/unit/test_orchestrator_gateway.py | 40 ++++++++++++++++++++++--- 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/bot_bottle/orchestrator/gateway.py b/bot_bottle/orchestrator/gateway.py index 8cc0bbf..17dccd1 100644 --- a/bot_bottle/orchestrator/gateway.py +++ b/bot_bottle/orchestrator/gateway.py @@ -144,6 +144,18 @@ class DockerGateway(Gateway): ]) return self.name in proc.stdout.split() + def _running_image_is_current(self) -> bool: + """True iff the running gateway was created from the *current* + `image_ref`. When `ensure_built` rebuilds the image (a source change), + the running container is still the OLD image running the OLD flat + daemons — so this is how a rebuild actually takes effect: a mismatch + means recreate.""" + running = run_docker(["docker", "inspect", "--format", "{{.Image}}", self.name]) + current = run_docker(["docker", "image", "inspect", "--format", "{{.Id}}", self.image_ref]) + if running.returncode != 0 or current.returncode != 0: + return True # can't compare → don't churn a working container + return running.stdout.strip() == current.stdout.strip() + 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'). @@ -157,11 +169,15 @@ class DockerGateway(Gateway): ) def ensure_running(self) -> None: - if self.is_running(): + # 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. + if self.is_running() and self._running_image_is_current(): return self._ensure_network() - # Clear any stale (stopped) container holding the fixed name, then - # start fresh. `rm --force` on an absent name is a tolerated no-op. + # 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. run_docker(["docker", "rm", "--force", self.name]) argv = [ "docker", "run", "--detach", diff --git a/tests/unit/test_orchestrator_gateway.py b/tests/unit/test_orchestrator_gateway.py index 484ca61..2836528 100644 --- a/tests/unit/test_orchestrator_gateway.py +++ b/tests/unit/test_orchestrator_gateway.py @@ -35,11 +35,43 @@ class TestDockerGateway(unittest.TestCase): with patch(_RUN_DOCKER, return_value=_proc(stdout="")): self.assertFalse(self.sc.is_running()) - def test_ensure_running_is_noop_when_already_up(self) -> None: - with patch(_RUN_DOCKER, return_value=_proc(stdout=self.sc.name)) as m: + def test_ensure_running_noop_when_up_and_image_current(self) -> None: + calls: list[list[str]] = [] + + def fake(argv: list[str]) -> Mock: + calls.append(argv) + if argv[:2] == ["docker", "ps"]: + return _proc(stdout=self.sc.name) # running + if argv[:3] == ["docker", "image", "inspect"]: + return _proc(stdout="img-A") # current image id + if argv[:2] == ["docker", "inspect"]: + return _proc(stdout="img-A") # container's image (same) + return _proc() + + with patch(_RUN_DOCKER, side_effect=fake): self.sc.ensure_running() - # Only the is_running() ps probe — no rm / run. - self.assertEqual(1, m.call_count) + self.assertEqual([], [c for c in calls if c[:2] == ["docker", "run"]]) + self.assertEqual([], [c for c in calls if c[:2] == ["docker", "rm"]]) + + def test_ensure_running_recreates_when_image_is_stale(self) -> None: + # Running, but the container was built from an OLD image → recreate so + # a rebuild's new flat daemons take effect. + calls: list[list[str]] = [] + + def fake(argv: list[str]) -> Mock: + calls.append(argv) + if argv[:2] == ["docker", "ps"]: + return _proc(stdout=self.sc.name) + if argv[:3] == ["docker", "image", "inspect"]: + return _proc(stdout="img-NEW") + if argv[:2] == ["docker", "inspect"]: + return _proc(stdout="img-OLD") + return _proc() + + with patch(_RUN_DOCKER, side_effect=fake): + self.sc.ensure_running() + self.assertEqual(1, len([c for c in calls if c[:2] == ["docker", "run"]])) + self.assertTrue(any(c[:2] == ["docker", "rm"] for c in calls)) def test_ensure_running_starts_the_singleton_when_absent(self) -> None: calls: list[list[str]] = []