fix(orchestrator): recreate the gateway when its image is stale
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WBMWTEtQdJ4W5UrWuLHCck
This commit is contained in:
@@ -144,6 +144,18 @@ class DockerGateway(Gateway):
|
|||||||
])
|
])
|
||||||
return self.name in proc.stdout.split()
|
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:
|
def _ensure_network(self) -> None:
|
||||||
"""Create the shared gateway network if it doesn't exist. Idempotent —
|
"""Create the shared gateway network if it doesn't exist. Idempotent —
|
||||||
a concurrent create loses harmlessly (the loser sees 'already exists').
|
a concurrent create loses harmlessly (the loser sees 'already exists').
|
||||||
@@ -157,11 +169,15 @@ class DockerGateway(Gateway):
|
|||||||
)
|
)
|
||||||
|
|
||||||
def ensure_running(self) -> None:
|
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
|
return
|
||||||
self._ensure_network()
|
self._ensure_network()
|
||||||
# Clear any stale (stopped) container holding the fixed name, then
|
# Clear any stale (stopped OR outdated-image) container holding the
|
||||||
# start fresh. `rm --force` on an absent name is a tolerated no-op.
|
# fixed name, then start fresh. `rm --force` on an absent name is a
|
||||||
|
# tolerated no-op.
|
||||||
run_docker(["docker", "rm", "--force", self.name])
|
run_docker(["docker", "rm", "--force", self.name])
|
||||||
argv = [
|
argv = [
|
||||||
"docker", "run", "--detach",
|
"docker", "run", "--detach",
|
||||||
|
|||||||
@@ -35,11 +35,43 @@ class TestDockerGateway(unittest.TestCase):
|
|||||||
with patch(_RUN_DOCKER, return_value=_proc(stdout="")):
|
with patch(_RUN_DOCKER, return_value=_proc(stdout="")):
|
||||||
self.assertFalse(self.sc.is_running())
|
self.assertFalse(self.sc.is_running())
|
||||||
|
|
||||||
def test_ensure_running_is_noop_when_already_up(self) -> None:
|
def test_ensure_running_noop_when_up_and_image_current(self) -> None:
|
||||||
with patch(_RUN_DOCKER, return_value=_proc(stdout=self.sc.name)) as m:
|
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()
|
self.sc.ensure_running()
|
||||||
# Only the is_running() ps probe — no rm / run.
|
self.assertEqual([], [c for c in calls if c[:2] == ["docker", "run"]])
|
||||||
self.assertEqual(1, m.call_count)
|
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:
|
def test_ensure_running_starts_the_singleton_when_absent(self) -> None:
|
||||||
calls: list[list[str]] = []
|
calls: list[list[str]] = []
|
||||||
|
|||||||
Reference in New Issue
Block a user