From e97366dad55237c4e6892949e709ae70835ddc2c Mon Sep 17 00:00:00 2001 From: didericis Date: Tue, 14 Jul 2026 01:13:11 -0400 Subject: [PATCH] fix(orchestrator): gateway image builds cache-aware, not build-if-missing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cut-over dropped compose (whose `build:` rebuilt the sidecar image on every up), and DockerGateway.ensure_built was build-if-missing — so a change to the gateway's flat sources (egress addon / git-http / policy_resolver / supervise) left a STALE image silently running the old single-tenant daemons (this bit e2e validation). ensure_built now always `docker build`s (cache-aware: a cache check when nothing changed, a real rebuild when sources moved), matching the old compose behavior. BOT_BOTTLE_NO_CACHE forces a full rebuild (parity with #354's `start --no-cache`). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01WBMWTEtQdJ4W5UrWuLHCck --- bot_bottle/orchestrator/gateway.py | 26 +++++++++------- tests/unit/test_orchestrator_gateway.py | 40 +++++++++++++------------ 2 files changed, 37 insertions(+), 29 deletions(-) diff --git a/bot_bottle/orchestrator/gateway.py b/bot_bottle/orchestrator/gateway.py index 573938d..4f0806b 100644 --- a/bot_bottle/orchestrator/gateway.py +++ b/bot_bottle/orchestrator/gateway.py @@ -109,17 +109,23 @@ class DockerGateway(Gateway): return run_docker(["docker", "image", "inspect", self.image_ref]).returncode == 0 def ensure_built(self) -> None: - """Build the bundle image from its Dockerfile when it's missing. - No-op when the image is already present, or when no dockerfile is - configured (e.g. a pre-pulled image).""" - if self._dockerfile is None or self.image_exists(): + """Build the bundle image from its Dockerfile, **cache-aware** — cheap + (a cache check) when nothing changed, a real rebuild when the flat + sources (egress addon / git-http / policy_resolver / supervise) moved. + + This deliberately builds every time rather than build-if-missing: the + per-bottle model kept the image fresh via compose's `build:` on up, and + a stale image silently runs the OLD single-tenant daemons. No-op only + when no dockerfile is configured (a pre-pulled image). BOT_BOTTLE_NO_CACHE + forces a full rebuild (parity with `start --no-cache`).""" + if self._dockerfile is None: return - proc = run_docker([ - "docker", "build", - "-t", self.image_ref, - "-f", str(self._build_context / self._dockerfile), - str(self._build_context), - ]) + argv = ["docker", "build", "-t", self.image_ref, + "-f", str(self._build_context / self._dockerfile), + str(self._build_context)] + if os.environ.get("BOT_BOTTLE_NO_CACHE"): + argv.insert(2, "--no-cache") + proc = run_docker(argv) if proc.returncode != 0: raise GatewayError(f"gateway image build failed: {proc.stderr.strip()}") diff --git a/tests/unit/test_orchestrator_gateway.py b/tests/unit/test_orchestrator_gateway.py index 4af3aaa..0f84029 100644 --- a/tests/unit/test_orchestrator_gateway.py +++ b/tests/unit/test_orchestrator_gateway.py @@ -129,28 +129,35 @@ class TestDockerGatewayBuild(unittest.TestCase): with patch(_RUN_DOCKER, return_value=_proc(returncode=1)): self.assertFalse(self.sc.image_exists()) - def test_ensure_built_is_noop_when_image_present(self) -> None: - with patch(_RUN_DOCKER, return_value=_proc(returncode=0)) as m: - self.sc.ensure_built() - self.assertEqual(1, m.call_count) # only the image-inspect probe - - def test_ensure_built_builds_from_dockerfile_when_missing(self) -> None: + def test_ensure_built_builds_even_when_image_present(self) -> None: + # Always build (cache-aware) so a flat-source change rebuilds; the old + # build-if-missing silently ran a stale single-tenant image. calls: list[list[str]] = [] - def fake(argv: list[str]) -> Mock: + def rec(argv: list[str]) -> Mock: calls.append(argv) - if argv[:3] == ["docker", "image", "inspect"]: - return _proc(returncode=1) # missing - return _proc() + return _proc() # image present, build succeeds - with patch(_RUN_DOCKER, side_effect=fake): + with patch(_RUN_DOCKER, side_effect=rec): self.sc.ensure_built() - builds = [c for c in calls if c[:2] == ["docker", "build"]] self.assertEqual(1, len(builds)) self.assertIn(self.sc.image_ref, builds[0]) - self.assertIn("-f", builds[0]) self.assertTrue(any(a.endswith("Dockerfile.sidecars") for a in builds[0])) + self.assertNotIn("--no-cache", builds[0]) + + def test_ensure_built_no_cache_env_forces_full_rebuild(self) -> None: + calls: list[list[str]] = [] + + def rec(argv: list[str]) -> Mock: + calls.append(argv) + return _proc() + + with patch(_RUN_DOCKER, side_effect=rec), \ + patch.dict("os.environ", {"BOT_BOTTLE_NO_CACHE": "1"}): + self.sc.ensure_built() + builds = [c for c in calls if c[:2] == ["docker", "build"]] + self.assertIn("--no-cache", builds[0]) def test_ensure_built_noop_when_no_dockerfile(self) -> None: sc = DockerGateway("busybox", dockerfile=None) @@ -159,12 +166,7 @@ class TestDockerGatewayBuild(unittest.TestCase): m.assert_not_called() def test_ensure_built_raises_on_build_failure(self) -> None: - def fake(argv: list[str]) -> Mock: - if argv[:3] == ["docker", "image", "inspect"]: - return _proc(returncode=1) - return _proc(returncode=1, stderr="build boom") - - with patch(_RUN_DOCKER, side_effect=fake): + with patch(_RUN_DOCKER, return_value=_proc(returncode=1, stderr="build boom")): with self.assertRaises(GatewayError): self.sc.ensure_built()