From e24b62b6b989378ae1fb2a7c80440bca96cd0b6a Mon Sep 17 00:00:00 2001 From: didericis Date: Fri, 17 Jul 2026 03:26:11 -0400 Subject: [PATCH] =?UTF-8?q?fix(macos):=20review=20fixes=20=E2=80=94=20toke?= =?UTF-8?q?n=20on=20plan,=20self-heal,=20symmetric=20digest,=20DHCP=20poll?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses findings from a high-effort review of the PRD 0070 macOS backend. Correctness: - Stamp identity_token onto MacosContainerBottlePlan after registration. git's gitconfig extraHeader and the supervise MCP --header read getattr(plan,"identity_token","") at provision time, and both reach the gateway on NO_PROXY (bypassing the egress proxy that carries the token). The plan never carried it, so /resolve fail-closed and every git fetch/push and supervise call from a macOS bottle would have been denied. Registration precedes provision(), so — unlike the run-time env — the plan can carry it. - Self-heal the orchestrator: recreate when it is not (source-current AND answering /health), not on the source-hash label alone. A container running current code but with a wedged HTTP server was left alone and polled to death, failing every launch until manual deletion. - image_digest and container_image_digest now read the same descriptor.digest field; dropped image_digest's id/tag fallback that could yield a value the container side can't produce — a permanent mismatch would have recreated the shared gateway on every launch (severing every live bottle's egress, since the replacement gets a new DHCP address). - Poll for the agent's and gateway's DHCP address instead of a fatal read right after `container run` (there is no --ip; the address can lag start). Cleanup: - One _inspect_first + _descriptor_digest behind the four inspect readers. - Shared bind_mount_spec (util) and host_db_dir (paths) replace per-module copies; _GIT_HTTP_PORT now imports git_http_backend.DEFAULT_PORT. - Drop the dead _url cache / url property and the write-only agent_proxy_url. Deferred (noted on the PR, not fixed here): the gateway image rebuilding on every launch (needs source-hash-labeled build), SQLite shared across VM guests, and the sh -lc profile-override edge — each is design-level or behavior-risk beyond a review fix. Verified: real Apple Container bring-up is green and idempotent; 1826 unit tests pass with `container` absent (CI parity), pyright clean, pylint 9.86. Co-Authored-By: Claude Fable 5 --- .../backend/macos_container/bottle_plan.py | 6 +- bot_bottle/backend/macos_container/gateway.py | 30 ++-- bot_bottle/backend/macos_container/launch.py | 22 ++- .../macos_container/orchestrator_service.py | 51 +++---- bot_bottle/backend/macos_container/util.py | 132 +++++++++--------- bot_bottle/paths.py | 11 +- .../test_macos_container_launch_wiring.py | 30 ++++ tests/unit/test_macos_container_util.py | 50 +++++++ tests/unit/test_macos_gateway.py | 22 +++ 9 files changed, 242 insertions(+), 112 deletions(-) diff --git a/bot_bottle/backend/macos_container/bottle_plan.py b/bot_bottle/backend/macos_container/bottle_plan.py index 6d10a60..f93a092 100644 --- a/bot_bottle/backend/macos_container/bottle_plan.py +++ b/bot_bottle/backend/macos_container/bottle_plan.py @@ -13,9 +13,13 @@ from .. import BottlePlan class MacosContainerBottlePlan(BottlePlan): slug: str forwarded_env: dict[str, str] = field(repr=False) - agent_proxy_url: str = "" agent_git_gate_url: str = "" agent_supervise_url: str = "" + # Read by provision-time consumers (git extraHeader, supervise MCP header) + # via getattr(plan, "identity_token", ""); stamped in launch after the + # bottle is registered. See launch.py's stamp for why it lives here and not + # only in the exec-time proxy env. + identity_token: str = "" @property def container_name(self) -> str: diff --git a/bot_bottle/backend/macos_container/gateway.py b/bot_bottle/backend/macos_container/gateway.py index c04413b..e4e2903 100644 --- a/bot_bottle/backend/macos_container/gateway.py +++ b/bot_bottle/backend/macos_container/gateway.py @@ -36,9 +36,10 @@ from ...orchestrator.gateway import ( Gateway, GatewayError, ) -from ...paths import host_db_path +from ...paths import host_db_dir from ...supervise import DB_PATH_IN_CONTAINER from . import util as container_mod +from .util import bind_mount_spec as _mount # Distinct from the docker gateway's names so both backends' gateways can # coexist on one host (a macOS host can run the docker backend too). @@ -69,7 +70,7 @@ def gateway_ca_dir() -> Path: The docker gateway uses a named volume for this; a plain host dir is the same guarantee with fewer moving parts, and it lets `ca_cert_pem` read the PEM straight off the host instead of shelling into the container.""" - path = host_db_path().parent / "mac-gateway-ca" + path = host_db_dir() / "mac-gateway-ca" path.mkdir(parents=True, exist_ok=True) return path @@ -88,19 +89,6 @@ def ensure_networks( container_mod.create_network(network, internal=True) -def _host_db_dir() -> str: - db_dir = host_db_path().parent - db_dir.mkdir(parents=True, exist_ok=True) - return str(db_dir) - - -def _mount(source: str, target: str, *, readonly: bool = False) -> str: - spec = f"type=bind,source={source},target={target}" - if readonly: - spec += ",readonly" - return spec - - class AppleGateway(Gateway): """The consolidated gateway as a single, fixed-name Apple container.""" @@ -187,7 +175,7 @@ class AppleGateway(Gateway): # The NAT gateway routes but does not resolve, so DNS is explicit. "--dns", container_mod.dns_server(), "--mount", _mount(str(gateway_ca_dir()), MITMPROXY_HOME), - "--mount", _mount(_host_db_dir(), _SUPERVISE_DB_DIR_IN_CONTAINER), + "--mount", _mount(str(host_db_dir()), _SUPERVISE_DB_DIR_IN_CONTAINER), "--env", f"SUPERVISE_DB_PATH={DB_PATH_IN_CONTAINER}", ] if self._orchestrator_url: @@ -204,8 +192,14 @@ class AppleGateway(Gateway): def ip_on_shared_network(self) -> str: """The gateway's address on the shared host-only network — what agents - point their proxy / git-http / supervise URLs at.""" - return container_mod.container_ipv4_on_network(self.name, self.network) + point their proxy / git-http / supervise URLs at. Polls: a freshly + run gateway can be up before vmnet's DHCP has assigned the address.""" + ip = container_mod.wait_container_ipv4_on_network(self.name, self.network) + if not ip: + raise GatewayError( + f"gateway {self.name} never got an address on {self.network}" + ) + return ip def ca_cert_pem(self, *, timeout: float = DEFAULT_CA_TIMEOUT_SECONDS) -> str: """The gateway's CA certificate (PEM) that agents install to trust its diff --git a/bot_bottle/backend/macos_container/launch.py b/bot_bottle/backend/macos_container/launch.py index a451761..97bef37 100644 --- a/bot_bottle/backend/macos_container/launch.py +++ b/bot_bottle/backend/macos_container/launch.py @@ -52,6 +52,7 @@ from ...git_gate import ( provision_git_gate_dynamic_keys, revoke_git_gate_provisioned_keys, ) +from ...git_http_backend import DEFAULT_PORT as _GIT_HTTP_PORT from ...log import die, info, warn from ...supervise import SUPERVISE_PORT from ..docker.egress import EGRESS_PORT @@ -68,7 +69,6 @@ from .consolidated_launch import ( _REPO_DIR = str(Path(__file__).resolve().parent.parent.parent.parent) _AGENT_SLEEP_SECONDS = "2147483647" -_GIT_HTTP_PORT = 9420 @contextmanager @@ -115,10 +115,16 @@ def launch( # Step 4: read the assigned address and register by it. This is the # attribution key; `--cap-drop CAP_NET_RAW` at run is what makes it - # unforgeable. - source_ip = container_mod.container_ipv4_on_network( + # unforgeable. Poll: `container run --detach` can return before vmnet's + # DHCP has assigned the address. + source_ip = container_mod.wait_container_ipv4_on_network( plan.container_name, endpoint.network, ) + if not source_ip: + die( + f"agent {plan.container_name} never got an address on " + f"{endpoint.network}" + ) effective_env = {**os.environ, **plan.agent_provision.provisioned_env} token_values = egress_resolve_token_values( plan.egress_plan.token_env_map, effective_env, @@ -140,6 +146,15 @@ def launch( f"(gateway {endpoint.gateway_ip}, ip {source_ip})" ) + # Stamp the token onto the plan so provision-time consumers can read it, + # not only the exec-time egress proxy. git-gate's gitconfig extraHeader + # and the supervise MCP --header both reach the gateway on NO_PROXY (they + # bypass the egress proxy that carries the token), so without this the + # gateway's /resolve fail-closes and every git fetch/push and supervise + # call from the bottle is denied. Registration already produced the + # token above, so — unlike the run-time env — the plan CAN carry it. + plan = dataclasses.replace(plan, identity_token=ctx.identity_token) + bottle = MacosContainerBottle( plan.container_name, teardown, @@ -227,7 +242,6 @@ def _stamp_agent_urls( ) return dataclasses.replace( plan, - agent_proxy_url=f"http://{endpoint.gateway_ip}:{EGRESS_PORT}", agent_git_gate_url=git_gate_url, agent_supervise_url=supervise_url, ) diff --git a/bot_bottle/backend/macos_container/orchestrator_service.py b/bot_bottle/backend/macos_container/orchestrator_service.py index 09d8616..70b666d 100644 --- a/bot_bottle/backend/macos_container/orchestrator_service.py +++ b/bot_bottle/backend/macos_container/orchestrator_service.py @@ -49,6 +49,7 @@ from .gateway import ( AppleGateway, ensure_networks, ) +from .util import bind_mount_spec as _mount # Distinct from the docker backend's container name so both can run on one host. ORCHESTRATOR_NAME = "bot-bottle-mac-orchestrator" @@ -61,13 +62,6 @@ _HEALTH_POLL_SECONDS = 0.25 _HEALTH_REQUEST_TIMEOUT_SECONDS = 1.0 -def _mount(source: str, target: str, *, readonly: bool = False) -> str: - spec = f"type=bind,source={source},target={target}" - if readonly: - spec += ",readonly" - return spec - - class MacosOrchestratorService: """Manages the orchestrator control-plane container + the shared Apple gateway. Callers only need `ensure_running()`, which returns the URL.""" @@ -92,30 +86,24 @@ class MacosOrchestratorService: self._repo_root = repo_root self._host_root = host_root or bot_bottle_root() self._orchestrator_name = orchestrator_name - # Resolved once the container is up — there is no name to fall back on. - self._url = "" - - @property - def url(self) -> str: - """The control-plane URL, or "" before the container is up. One URL for - both the host CLI and the gateway (see the module docstring).""" - return self._url def _resolve_url(self) -> str: - """The control-plane URL, or "" while the container has no address.""" + """The control-plane URL, or "" while the container has no address. + Resolved fresh each time (there is no name to fall back on, and a + recreated orchestrator can come back on a different DHCP address), so + nothing caches a URL that could go stale.""" ip = container_mod.try_container_ipv4_on_network( self._orchestrator_name, self.network, ) return f"http://{ip}:{self.port}" if ip else "" def is_healthy( - self, url: str = "", *, timeout: float = _HEALTH_REQUEST_TIMEOUT_SECONDS, + self, url: str, *, timeout: float = _HEALTH_REQUEST_TIMEOUT_SECONDS, ) -> bool: - target = url or self._url - if not target: + if not url: return False try: - with urllib.request.urlopen(f"{target}/health", timeout=timeout) as resp: + with urllib.request.urlopen(f"{url}/health", timeout=timeout) as resp: return resp.status == 200 except (urllib.error.URLError, TimeoutError, OSError): return False @@ -192,16 +180,15 @@ class MacosOrchestratorService: control-plane URL. Idempotent — a healthy control plane running current code and a running gateway are left untouched.""" current_hash = source_hash(self._repo_root) - if not self._orchestrator_source_current(current_hash): + url = self._running_healthy_url(current_hash) + if not url: self._ensure_orchestrator_image() log.info( "starting orchestrator container", context={"name": self._orchestrator_name}, ) self._run_orchestrator_container(current_hash) - - url = self._wait_healthy(startup_timeout) - self._url = url + url = self._wait_healthy(startup_timeout) # Gateway second: it can only reach the control plane by IP, which does # not exist until the orchestrator container is up (see the docstring). @@ -210,6 +197,21 @@ class MacosOrchestratorService: gateway.ensure_running() return url + def _running_healthy_url(self, current_hash: str) -> str: + """The control-plane URL if the running orchestrator is BOTH current + and answering /health, else "" (→ recreate). + + Checking health, not just the source-hash label, is what lets the + service self-heal: a container that is running the current code but + whose HTTP server is wedged (bind failure, deadlock, OOM'd thread) + would otherwise be left alone and polled to death on every launch + forever. Mirrors the docker service's `is_healthy() and source_current` + gate.""" + if not self._orchestrator_source_current(current_hash): + return "" + url = self._resolve_url() + return url if url and self.is_healthy(url) else "" + def _wait_healthy(self, startup_timeout: float) -> str: """Poll until the control plane answers /health, resolving its address each time: the container is up before it has an IP, and it has an IP @@ -231,7 +233,6 @@ class MacosOrchestratorService: """Remove the orchestrator + gateway containers (idempotent).""" container_mod.force_remove_container(self._orchestrator_name) self.gateway("").stop() - self._url = "" __all__ = [ diff --git a/bot_bottle/backend/macos_container/util.py b/bot_bottle/backend/macos_container/util.py index d400120..06a0a93 100644 --- a/bot_bottle/backend/macos_container/util.py +++ b/bot_bottle/backend/macos_container/util.py @@ -453,77 +453,73 @@ def run_container_argv(argv: list[str]) -> subprocess.CompletedProcess[str]: return subprocess.run(argv, capture_output=True, text=True, check=False) +def bind_mount_spec(source: str, target: str, *, readonly: bool = False) -> str: + """A `container run --mount` bind spec. One definition so the gateway and + orchestrator emit an identical string — a divergence here would silently + break one backend's mounts while the other kept working.""" + spec = f"type=bind,source={source},target={target}" + if readonly: + spec += ",readonly" + return spec + + def _normalize_digest(value: str) -> str: return value.split(":", 1)[1] if ":" in value else value -def image_digest(ref: str) -> str: - """The digest of image `ref`, or "" if it can't be read. Empty is a - 'don't know' signal — callers treat it as "don't churn a working - container" rather than as a mismatch.""" - result = run_container_argv([_CONTAINER, "image", "inspect", ref]) +def _inspect_first(argv: list[str]) -> dict[str, object]: + """Run an inspect command and return its first JSON object, or {} on any + failure (non-zero exit, malformed JSON, unexpected shape). {} is the shared + 'don't know' signal all the non-fatal inspect readers below build on — a + caller comparing against it treats it as 'leave the working container + alone', never as a mismatch.""" + result = run_container_argv(argv) if result.returncode != 0: - return "" + return {} try: - data = json.loads(result.stdout or "{}") + data = json.loads(result.stdout or "[]") except json.JSONDecodeError: + return {} + if isinstance(data, list): + data = data[0] if data else {} + return data if isinstance(data, dict) else {} + + +def _descriptor_digest(node: object) -> str: + """The normalized digest under a `{... "descriptor": {"digest": ...}}` + node, or "". Both the image and container inspect shapes nest the image's + identity this way, so the digest readers stay symmetric — a difference + between them is what would spuriously recreate a container.""" + if not isinstance(node, dict): return "" - if isinstance(data, list) and data: - data = data[0] - if not isinstance(data, dict): - return "" - config = data.get("configuration") - if isinstance(config, dict): - descriptor = config.get("descriptor") - if isinstance(descriptor, dict) and descriptor.get("digest"): - return _normalize_digest(str(descriptor["digest"])) - value = data.get("id") - return _normalize_digest(str(value)) if value else "" + descriptor = node.get("descriptor") + if isinstance(descriptor, dict) and descriptor.get("digest"): + return _normalize_digest(str(descriptor["digest"])) + return "" + + +def image_digest(ref: str) -> str: + """The digest of image `ref`, or "" if it can't be read. Reads exactly the + field `container_image_digest` reads (`configuration.descriptor.digest`) so + the two are comparable; "" means 'don't know' → callers don't churn.""" + data = _inspect_first([_CONTAINER, "image", "inspect", ref]) + return _descriptor_digest(data.get("configuration")) def container_image_digest(name: str) -> str: """The digest of the image container `name` was created from, or "" if it can't be read. Compare with `image_digest(ref)` to tell whether a running container predates an image rebuild.""" - result = run_container_argv([_CONTAINER, "inspect", name]) - if result.returncode != 0: - return "" - try: - data = json.loads(result.stdout or "[]") - except json.JSONDecodeError: - return "" - if isinstance(data, list) and data: - data = data[0] - if not isinstance(data, dict): - return "" - config = data.get("configuration") - if not isinstance(config, dict): - return "" - image = config.get("image") - if not isinstance(image, dict): - return "" - descriptor = image.get("descriptor") - if isinstance(descriptor, dict) and descriptor.get("digest"): - return _normalize_digest(str(descriptor["digest"])) - return "" + config = _inspect_first([_CONTAINER, "inspect", name]).get("configuration") + image = config.get("image") if isinstance(config, dict) else None + return _descriptor_digest(image) def container_env(name: str) -> dict[str, str]: """The env container `name` was started with, or {} if unreadable. Lets a caller tell whether a running container's baked-in configuration still matches what it would pass today.""" - result = run_container_argv([_CONTAINER, "inspect", name]) - if result.returncode != 0: - return {} - try: - data = json.loads(result.stdout or "[]") - except json.JSONDecodeError: - return {} - if isinstance(data, list) and data: - data = data[0] - if not isinstance(data, dict): - return {} - config = data.get("configuration") + config = _inspect_first([_CONTAINER, "inspect", name]).get("configuration") init = config.get("initProcess") if isinstance(config, dict) else None entries = init.get("environment") if isinstance(init, dict) else None if not isinstance(entries, list): @@ -540,18 +536,7 @@ def try_container_ipv4_on_network(name: str, network: str) -> str: """`container_ipv4_on_network` without the fatal exit: "" when the address isn't readable yet. For pollers — a container is created before it has an address, so "not yet" is an expected state there, not an error.""" - result = run_container_argv([_CONTAINER, "inspect", name]) - if result.returncode != 0: - return "" - try: - data = json.loads(result.stdout or "[]") - except json.JSONDecodeError: - return "" - if isinstance(data, list) and data: - data = data[0] - if not isinstance(data, dict): - return "" - status = data.get("status") + status = _inspect_first([_CONTAINER, "inspect", name]).get("status") networks = status.get("networks") if isinstance(status, dict) else None if not isinstance(networks, list): return "" @@ -564,6 +549,27 @@ def try_container_ipv4_on_network(name: str, network: str) -> str: return "" +def wait_container_ipv4_on_network( + name: str, network: str, *, timeout: float = 15.0, poll: float = 0.25, +) -> str: + """Poll for the container's DHCP-assigned address on `network`, returning + it once available or "" on timeout. + + Apple Container has no `--ip`: `container run --detach` can return before + vmnet's DHCP has populated `status.networks[].ipv4Address`, so a bare read + right after start races the assignment. Callers that need the address (the + attribution key, the gateway's proxy target) poll through here instead of + the fatal `container_ipv4_on_network`.""" + deadline = time.monotonic() + timeout + while True: + ip = try_container_ipv4_on_network(name, network) + if ip: + return ip + if time.monotonic() >= deadline: + return "" + time.sleep(poll) + + def image_id(ref: str) -> str: """Return the image digest/ID from `container image inspect`. diff --git a/bot_bottle/paths.py b/bot_bottle/paths.py index 15bf2e3..13136fd 100644 --- a/bot_bottle/paths.py +++ b/bot_bottle/paths.py @@ -40,4 +40,13 @@ def host_db_path() -> Path: return bot_bottle_root() / "db" / HOST_DB_FILENAME -__all__ = ["HOST_DB_FILENAME", "bot_bottle_root", "host_db_path"] +def host_db_dir() -> Path: + """The directory holding the shared host state DB, created if missing. + Backends bind-mount this into their gateway so the supervise daemon writes + to the one DB the orchestrator (and the operator over HTTP) reads.""" + db_dir = host_db_path().parent + db_dir.mkdir(parents=True, exist_ok=True) + return db_dir + + +__all__ = ["HOST_DB_FILENAME", "bot_bottle_root", "host_db_path", "host_db_dir"] diff --git a/tests/unit/test_macos_container_launch_wiring.py b/tests/unit/test_macos_container_launch_wiring.py index fd8355a..a8cc720 100644 --- a/tests/unit/test_macos_container_launch_wiring.py +++ b/tests/unit/test_macos_container_launch_wiring.py @@ -158,6 +158,36 @@ class TestIdentityTokenDelivery(unittest.TestCase): argv = bottle.agent_argv(["--help"], tty=False) self.assertNotIn("--env", argv) + +class TestPlanIdentityToken(unittest.TestCase): + """git-gate's gitconfig extraHeader and the supervise MCP --header read + `getattr(plan, "identity_token", "")` at provision time and both bypass the + egress proxy (NO_PROXY), so the exec-time proxy token never reaches them — + the plan must carry the token or /resolve fail-closes and git + supervise + are denied on macOS.""" + + def test_macos_plan_has_the_identity_token_field(self) -> None: + from dataclasses import fields + + from bot_bottle.backend.macos_container.bottle_plan import ( + MacosContainerBottlePlan, + ) + names = {f.name for f in fields(MacosContainerBottlePlan)} + self.assertIn("identity_token", names) + + def test_matches_the_docker_plan(self) -> None: + """Both consolidated backends must expose identity_token so a shared + provision-time consumer degrades to neither backend silently.""" + from dataclasses import fields + + from bot_bottle.backend.docker.bottle_plan import DockerBottlePlan + from bot_bottle.backend.macos_container.bottle_plan import ( + MacosContainerBottlePlan, + ) + docker = {f.name for f in fields(DockerBottlePlan)} + macos = {f.name for f in fields(MacosContainerBottlePlan)} + self.assertIn("identity_token", docker & macos) + def test_provisioning_exec_also_carries_the_token(self) -> None: """`provision` runs through `exec`; a provider whose provision step fetches anything would otherwise egress token-less and be denied.""" diff --git a/tests/unit/test_macos_container_util.py b/tests/unit/test_macos_container_util.py index 54e604c..5fcd994 100644 --- a/tests/unit/test_macos_container_util.py +++ b/tests/unit/test_macos_container_util.py @@ -273,5 +273,55 @@ resolver #2 ) +def _completed(stdout: str, returncode: int = 0): + return util.subprocess.CompletedProcess(args=[], returncode=returncode, stdout=stdout, stderr="") + + +class TestInspectDigests(unittest.TestCase): + """image_digest and container_image_digest must read the SAME field shape + (a `descriptor.digest`) so a running container and its image are + comparable. An asymmetry recreates the gateway on every launch.""" + + _IMAGE = '{"configuration": {"descriptor": {"digest": "sha256:abc123"}}, "id": "abc123"}' + _CONTAINER = '[{"configuration": {"image": {"descriptor": {"digest": "sha256:abc123"}}}}]' + + def test_image_and_container_digests_agree_for_the_same_image(self): + with patch.object(util, "run_container_argv") as run: + run.return_value = _completed(self._IMAGE) + img = util.image_digest("bot-bottle-gateway:latest") + run.return_value = _completed(self._CONTAINER) + ctr = util.container_image_digest("bot-bottle-mac-gateway") + self.assertEqual("abc123", img) + self.assertEqual(img, ctr) + + def test_image_digest_never_falls_back_to_a_tag_or_id(self): + """The old `id` fallback could yield a value container_image_digest + can't produce, permanently mismatching. Missing descriptor → '' (don't + churn), never a stray id/tag.""" + with patch.object(util, "run_container_argv") as run: + run.return_value = _completed('{"id": "bot-bottle-gateway:latest"}') + self.assertEqual("", util.image_digest("bot-bottle-gateway:latest")) + + def test_unreadable_inspect_returns_empty(self): + with patch.object(util, "run_container_argv") as run: + run.return_value = _completed("", returncode=1) + self.assertEqual("", util.image_digest("x")) + self.assertEqual("", util.container_image_digest("x")) + self.assertEqual({}, util.container_env("x")) + + +class TestWaitContainerIpv4(unittest.TestCase): + def test_returns_address_once_dhcp_assigns_it(self): + with patch.object(util, "try_container_ipv4_on_network", side_effect=["", "", "192.168.128.4"]), \ + patch.object(util.time, "sleep"): + ip = util.wait_container_ipv4_on_network("c", "net", timeout=5, poll=0) + self.assertEqual("192.168.128.4", ip) + + def test_returns_empty_on_timeout(self): + with patch.object(util, "try_container_ipv4_on_network", return_value=""), \ + patch.object(util.time, "sleep"): + self.assertEqual("", util.wait_container_ipv4_on_network("c", "net", timeout=-1)) + + if __name__ == "__main__": unittest.main() diff --git a/tests/unit/test_macos_gateway.py b/tests/unit/test_macos_gateway.py index 588bad6..359c344 100644 --- a/tests/unit/test_macos_gateway.py +++ b/tests/unit/test_macos_gateway.py @@ -194,6 +194,28 @@ class TestMacosOrchestratorService(unittest.TestCase): svc.ensure_running() run.assert_not_called() + def test_wedged_but_current_orchestrator_is_recreated(self) -> None: + """A container running the current code but whose HTTP server is dead + must be recreated, not left alone and polled to death forever. Checking + the source-hash label alone (without health) would strand the host.""" + svc = MacosOrchestratorService(repo_root=Path("/r"), host_root=Path("/h")) + run = Mock() + # Unhealthy on the pre-check, healthy once recreated + waited. + health = Mock(side_effect=[False, True]) + with patch(f"{_ORCH}.container_mod") as mod, \ + patch(f"{_ORCH}.source_hash", return_value="h1"), \ + patch.object(svc, "_run_orchestrator_container", run), \ + patch.object(svc, "_ensure_orchestrator_image"), \ + patch.object(svc, "gateway"), \ + patch.object(svc, "is_healthy", health): + mod.container_is_running.return_value = True + mod.inspect_container.return_value = { + "configuration": {"labels": {"bot-bottle-orchestrator-source-hash": "h1"}} + } + mod.try_container_ipv4_on_network.return_value = "192.168.128.2" + svc.ensure_running() + run.assert_called_once() + def test_changed_source_recreates_the_orchestrator(self) -> None: """The control-plane process loaded its bind-mounted source at startup and won't reload it."""