fix(lifecycle): build gateway before infra and fix orchestrator port mapping
- `_build_images()` now builds Dockerfile.gateway → Dockerfile.orchestrator → Dockerfile.infra in order; Dockerfile.infra starts FROM bot-bottle-gateway so the base must exist on clean hosts. - Publish mapping corrected from `self.port:self.port` to `self.port:DEFAULT_PORT` (8099) — gateway_init hardcodes the orchestrator on port 8099 inside the container, so the host-side published port must map to that fixed internal port. - BOT_BOTTLE_ORCHESTRATOR_URL inside the container now always points to 127.0.0.1:8099, not self.port, since gateway daemons reach the orchestrator over loopback at the fixed internal port. - Update test_ensure_running_builds_both_images → _all_images for the new three-step build sequence; add test_publish_maps_host_port_to_fixed_internal_port to lock in the port-mapping fix. Addresses the P1 findings from the didericis-codex review on PR #432.
This commit is contained in:
@@ -27,6 +27,8 @@ from ..paths import CONTROL_PLANE_TOKEN_ENV, bot_bottle_root, host_control_plane
|
|||||||
from ..supervise import DB_PATH_IN_CONTAINER
|
from ..supervise import DB_PATH_IN_CONTAINER
|
||||||
from .gateway import (
|
from .gateway import (
|
||||||
GATEWAY_CA_VOLUME,
|
GATEWAY_CA_VOLUME,
|
||||||
|
GATEWAY_DOCKERFILE,
|
||||||
|
GATEWAY_IMAGE,
|
||||||
GATEWAY_NETWORK,
|
GATEWAY_NETWORK,
|
||||||
GatewayError,
|
GatewayError,
|
||||||
MITMPROXY_HOME,
|
MITMPROXY_HOME,
|
||||||
@@ -160,9 +162,10 @@ class OrchestratorService:
|
|||||||
)
|
)
|
||||||
|
|
||||||
def _build_images(self) -> None:
|
def _build_images(self) -> None:
|
||||||
"""Build the orchestrator image (build intermediate), then the infra
|
"""Build the gateway base, the orchestrator intermediate, then the
|
||||||
image. Both are cache-aware: a no-op when nothing changed."""
|
infra image. All are cache-aware: a no-op when nothing changed."""
|
||||||
for tag, dockerfile in (
|
for tag, dockerfile in (
|
||||||
|
(GATEWAY_IMAGE, GATEWAY_DOCKERFILE),
|
||||||
(ORCHESTRATOR_IMAGE, ORCHESTRATOR_DOCKERFILE),
|
(ORCHESTRATOR_IMAGE, ORCHESTRATOR_DOCKERFILE),
|
||||||
(self.image, INFRA_DOCKERFILE),
|
(self.image, INFRA_DOCKERFILE),
|
||||||
):
|
):
|
||||||
@@ -188,7 +191,9 @@ class OrchestratorService:
|
|||||||
"--label", f"{INFRA_SOURCE_HASH_LABEL}={current_hash}",
|
"--label", f"{INFRA_SOURCE_HASH_LABEL}={current_hash}",
|
||||||
"--network", self.network,
|
"--network", self.network,
|
||||||
# Host CLI reaches the control plane here (loopback only).
|
# Host CLI reaches the control plane here (loopback only).
|
||||||
"--publish", f"127.0.0.1:{self.port}:{self.port}",
|
# gateway_init always starts the orchestrator on DEFAULT_PORT (8099)
|
||||||
|
# inside the container; self.port is the host-side published port.
|
||||||
|
"--publish", f"127.0.0.1:{self.port}:{DEFAULT_PORT}",
|
||||||
# Persist the mitmproxy CA so it survives container recreation.
|
# Persist the mitmproxy CA so it survives container recreation.
|
||||||
"--volume", f"{GATEWAY_CA_VOLUME}:{MITMPROXY_HOME}",
|
"--volume", f"{GATEWAY_CA_VOLUME}:{MITMPROXY_HOME}",
|
||||||
# Shared supervise DB (same file the operator reads over HTTP).
|
# Shared supervise DB (same file the operator reads over HTTP).
|
||||||
@@ -206,8 +211,9 @@ class OrchestratorService:
|
|||||||
# Control-plane secret: required by the orchestrator (to enforce)
|
# Control-plane secret: required by the orchestrator (to enforce)
|
||||||
# and by the gateway daemons (to present on /resolve calls).
|
# and by the gateway daemons (to present on /resolve calls).
|
||||||
"--env", CONTROL_PLANE_TOKEN_ENV,
|
"--env", CONTROL_PLANE_TOKEN_ENV,
|
||||||
# Gateway daemons reach the orchestrator over loopback.
|
# Gateway daemons reach the orchestrator over loopback at its
|
||||||
"--env", f"BOT_BOTTLE_ORCHESTRATOR_URL=http://127.0.0.1:{self.port}",
|
# fixed internal port (DEFAULT_PORT), independent of self.port.
|
||||||
|
"--env", f"BOT_BOTTLE_ORCHESTRATOR_URL=http://127.0.0.1:{DEFAULT_PORT}",
|
||||||
# Opt the orchestrator into gateway_init's supervise tree.
|
# Opt the orchestrator into gateway_init's supervise tree.
|
||||||
"--env", f"BOT_BOTTLE_GATEWAY_DAEMONS={_INFRA_DAEMONS}",
|
"--env", f"BOT_BOTTLE_GATEWAY_DAEMONS={_INFRA_DAEMONS}",
|
||||||
self.image,
|
self.image,
|
||||||
|
|||||||
@@ -116,7 +116,7 @@ class TestOrchestratorService(unittest.TestCase):
|
|||||||
daemons_flag = "BOT_BOTTLE_GATEWAY_DAEMONS=egress,git-http,supervise,orchestrator"
|
daemons_flag = "BOT_BOTTLE_GATEWAY_DAEMONS=egress,git-http,supervise,orchestrator"
|
||||||
self.assertIn("orchestrator", argv[argv.index(daemons_flag)])
|
self.assertIn("orchestrator", argv[argv.index(daemons_flag)])
|
||||||
|
|
||||||
def test_ensure_running_builds_both_images(self) -> None:
|
def test_ensure_running_builds_all_images(self) -> None:
|
||||||
calls: list[list[str]] = []
|
calls: list[list[str]] = []
|
||||||
|
|
||||||
def fake(argv: list[str], **_kw: object) -> Mock:
|
def fake(argv: list[str], **_kw: object) -> Mock:
|
||||||
@@ -129,14 +129,36 @@ class TestOrchestratorService(unittest.TestCase):
|
|||||||
patch(_RUN, side_effect=fake), patch(_SLEEP):
|
patch(_RUN, side_effect=fake), patch(_SLEEP):
|
||||||
self.svc.ensure_running()
|
self.svc.ensure_running()
|
||||||
builds = [c for c in calls if c[:2] == ["docker", "build"]]
|
builds = [c for c in calls if c[:2] == ["docker", "build"]]
|
||||||
# Orchestrator (build intermediate) + infra image both built.
|
# Gateway base + orchestrator intermediate + infra image — all three built.
|
||||||
self.assertEqual(2, len(builds))
|
self.assertEqual(3, len(builds))
|
||||||
dockerfiles = [next(a for a in b if "Dockerfile" in a) for b in builds]
|
dockerfiles = [next(a for a in b if "Dockerfile" in a) for b in builds]
|
||||||
self.assertIn("Dockerfile.orchestrator", dockerfiles[0])
|
self.assertIn("Dockerfile.gateway", dockerfiles[0])
|
||||||
self.assertIn("Dockerfile.infra", dockerfiles[1])
|
self.assertIn("Dockerfile.orchestrator", dockerfiles[1])
|
||||||
# Images are distinct — the point of the split.
|
self.assertIn("Dockerfile.infra", dockerfiles[2])
|
||||||
|
# All three images are distinct.
|
||||||
tags = [b[b.index("-t") + 1] for b in builds]
|
tags = [b[b.index("-t") + 1] for b in builds]
|
||||||
self.assertNotEqual(tags[0], tags[1])
|
self.assertEqual(3, len(set(tags)))
|
||||||
|
|
||||||
|
def test_publish_maps_host_port_to_fixed_internal_port(self) -> None:
|
||||||
|
"""A non-default self.port is published to the fixed internal port 8099,
|
||||||
|
not to self.port:self.port — the orchestrator always listens on 8099."""
|
||||||
|
calls: list[list[str]] = []
|
||||||
|
|
||||||
|
def fake(argv: list[str], **_kw: object) -> Mock:
|
||||||
|
calls.append(argv)
|
||||||
|
if argv[:2] == ["docker", "ps"]:
|
||||||
|
return _proc(stdout="")
|
||||||
|
return _proc()
|
||||||
|
|
||||||
|
svc = OrchestratorService(port=20001)
|
||||||
|
with patch(_URLOPEN, side_effect=[urllib.error.URLError("down"), _health(200)]), \
|
||||||
|
patch(_RUN, side_effect=fake), patch(_SLEEP):
|
||||||
|
svc.ensure_running()
|
||||||
|
runs = [c for c in calls if c[:2] == ["docker", "run"]]
|
||||||
|
argv = runs[0]
|
||||||
|
self.assertEqual("127.0.0.1:20001:8099", argv[argv.index("--publish") + 1])
|
||||||
|
orch_url = next(a for a in argv if "BOT_BOTTLE_ORCHESTRATOR_URL" in a)
|
||||||
|
self.assertIn(":8099", orch_url)
|
||||||
|
|
||||||
def test_ensure_running_raises_on_timeout(self) -> None:
|
def test_ensure_running_raises_on_timeout(self) -> None:
|
||||||
with patch(_URLOPEN, side_effect=urllib.error.URLError("down")), \
|
with patch(_URLOPEN, side_effect=urllib.error.URLError("down")), \
|
||||||
|
|||||||
Reference in New Issue
Block a user