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
|
return run_docker(["docker", "image", "inspect", self.image_ref]).returncode == 0
|
||||||
|
|
||||||
def ensure_built(self) -> None:
|
def ensure_built(self) -> None:
|
||||||
"""Build the bundle image from its Dockerfile when it's missing.
|
"""Build the bundle image from its Dockerfile, **cache-aware** — cheap
|
||||||
No-op when the image is already present, or when no dockerfile is
|
(a cache check) when nothing changed, a real rebuild when the flat
|
||||||
configured (e.g. a pre-pulled image)."""
|
sources (egress addon / git-http / policy_resolver / supervise) moved.
|
||||||
if self._dockerfile is None or self.image_exists():
|
|
||||||
|
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
|
return
|
||||||
proc = run_docker([
|
argv = ["docker", "build", "-t", self.image_ref,
|
||||||
"docker", "build",
|
"-f", str(self._build_context / self._dockerfile),
|
||||||
"-t", self.image_ref,
|
str(self._build_context)]
|
||||||
"-f", str(self._build_context / self._dockerfile),
|
if os.environ.get("BOT_BOTTLE_NO_CACHE"):
|
||||||
str(self._build_context),
|
argv.insert(2, "--no-cache")
|
||||||
])
|
proc = run_docker(argv)
|
||||||
if proc.returncode != 0:
|
if proc.returncode != 0:
|
||||||
raise GatewayError(f"gateway image build failed: {proc.stderr.strip()}")
|
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)):
|
with patch(_RUN_DOCKER, return_value=_proc(returncode=1)):
|
||||||
self.assertFalse(self.sc.image_exists())
|
self.assertFalse(self.sc.image_exists())
|
||||||
|
|
||||||
def test_ensure_built_is_noop_when_image_present(self) -> None:
|
def test_ensure_built_builds_even_when_image_present(self) -> None:
|
||||||
with patch(_RUN_DOCKER, return_value=_proc(returncode=0)) as m:
|
# Always build (cache-aware) so a flat-source change rebuilds; the old
|
||||||
self.sc.ensure_built()
|
# build-if-missing silently ran a stale single-tenant image.
|
||||||
self.assertEqual(1, m.call_count) # only the image-inspect probe
|
|
||||||
|
|
||||||
def test_ensure_built_builds_from_dockerfile_when_missing(self) -> None:
|
|
||||||
calls: list[list[str]] = []
|
calls: list[list[str]] = []
|
||||||
|
|
||||||
def fake(argv: list[str]) -> Mock:
|
def rec(argv: list[str]) -> Mock:
|
||||||
calls.append(argv)
|
calls.append(argv)
|
||||||
if argv[:3] == ["docker", "image", "inspect"]:
|
return _proc() # image present, build succeeds
|
||||||
return _proc(returncode=1) # missing
|
|
||||||
return _proc()
|
|
||||||
|
|
||||||
with patch(_RUN_DOCKER, side_effect=fake):
|
with patch(_RUN_DOCKER, side_effect=rec):
|
||||||
self.sc.ensure_built()
|
self.sc.ensure_built()
|
||||||
|
|
||||||
builds = [c for c in calls if c[:2] == ["docker", "build"]]
|
builds = [c for c in calls if c[:2] == ["docker", "build"]]
|
||||||
self.assertEqual(1, len(builds))
|
self.assertEqual(1, len(builds))
|
||||||
self.assertIn(self.sc.image_ref, builds[0])
|
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.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:
|
def test_ensure_built_noop_when_no_dockerfile(self) -> None:
|
||||||
sc = DockerGateway("busybox", dockerfile=None)
|
sc = DockerGateway("busybox", dockerfile=None)
|
||||||
@@ -159,12 +166,7 @@ class TestDockerGatewayBuild(unittest.TestCase):
|
|||||||
m.assert_not_called()
|
m.assert_not_called()
|
||||||
|
|
||||||
def test_ensure_built_raises_on_build_failure(self) -> None:
|
def test_ensure_built_raises_on_build_failure(self) -> None:
|
||||||
def fake(argv: list[str]) -> Mock:
|
with patch(_RUN_DOCKER, return_value=_proc(returncode=1, stderr="build boom")):
|
||||||
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 self.assertRaises(GatewayError):
|
with self.assertRaises(GatewayError):
|
||||||
self.sc.ensure_built()
|
self.sc.ensure_built()
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user