fix(orchestrator): gateway image builds cache-aware, not build-if-missing
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WBMWTEtQdJ4W5UrWuLHCck
This commit is contained in:
@@ -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()}")
|
||||
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user