diff --git a/headroom/cli/install.py b/headroom/cli/install.py index 12a1727ad..ed9883d96 100644 --- a/headroom/cli/install.py +++ b/headroom/cli/install.py @@ -18,6 +18,7 @@ from headroom.install.models import ( from headroom.install.planner import build_manifest from headroom.install.providers import apply_mutations, revert_mutations from headroom.install.runtime import ( + acquire_runtime_start_lock, run_foreground, runtime_status, start_detached_agent, @@ -330,6 +331,9 @@ def install_agent_run(profile: str) -> None: raise SystemExit(run_foreground(manifest)) +_STARTUP_READY_TIMEOUT_SECONDS = 15 + + @install_agent.command("ensure") @click.option("--profile", default="default", show_default=True, help="Deployment profile name.") def install_agent_ensure(profile: str) -> None: @@ -339,5 +343,21 @@ def install_agent_ensure(profile: str) -> None: if probe_ready(manifest.health_url): click.echo(f"Deployment '{profile}' is already healthy.") return - _start_deployment(manifest) + with acquire_runtime_start_lock(manifest.profile) as acquired: + if not acquired: + click.echo(f"Deployment '{profile}' start is already in progress.") + return + # Double-check after acquiring the lock — another ensure may have + # started the runtime while we waited for the lock. + if probe_ready(manifest.health_url): + click.echo(f"Deployment '{profile}' is already healthy.") + return + if runtime_status(manifest) == "running": + # Runtime exists but isn't ready yet — give it a grace period + # before deciding it's wedged and restarting. + if wait_ready(manifest, timeout_seconds=_STARTUP_READY_TIMEOUT_SECONDS): + click.echo(f"Deployment '{profile}' is healthy.") + return + stop_runtime(manifest) + _start_deployment(manifest) click.echo(f"Deployment '{profile}' is healthy.") diff --git a/tests/test_cli/test_install_cli.py b/tests/test_cli/test_install_cli.py index 94510724c..ed1600892 100644 --- a/tests/test_cli/test_install_cli.py +++ b/tests/test_cli/test_install_cli.py @@ -341,3 +341,189 @@ def test_install_agent_run_exits_with_foreground_status(monkeypatch) -> None: result = runner.invoke(main, ["install", "agent", "run"]) assert result.exit_code == 7 + + +def test_install_agent_ensure_no_spawn_when_lock_not_acquired(monkeypatch) -> None: + """Ensure does not spawn a runtime when the start lock is contended.""" + runner = CliRunner() + calls: list[str] = [] + + class Manifest: + profile = "default" + health_url = "http://127.0.0.1:8787/readyz" + + monkeypatch.setattr("headroom.cli.install.load_manifest", lambda profile: Manifest()) + monkeypatch.setattr("headroom.cli.install.probe_ready", lambda url: False) + + import contextlib + + @contextlib.contextmanager + def fake_lock(profile): + yield False + + monkeypatch.setattr("headroom.cli.install.acquire_runtime_start_lock", fake_lock) + monkeypatch.setattr( + "headroom.cli.install.start_detached_agent", + lambda profile: calls.append("start_agent"), + ) + monkeypatch.setattr( + "headroom.cli.install.start_persistent_docker", + lambda manifest: calls.append("start_docker"), + ) + + result = runner.invoke(main, ["install", "agent", "ensure"]) + assert result.exit_code == 0, result.output + assert "already in progress" in result.output + assert calls == [] + + +def test_install_agent_ensure_stops_wedged_runtime_before_restart(monkeypatch) -> None: + """Ensure stops a wedged runtime (running but not ready) before starting fresh.""" + runner = CliRunner() + calls: list[str] = [] + + class Manifest: + profile = "default" + health_url = "http://127.0.0.1:8787/readyz" + preset = "persistent-task" + supervisor_kind = "none" + + monkeypatch.setattr("headroom.cli.install.load_manifest", lambda profile: Manifest()) + monkeypatch.setattr("headroom.cli.install.probe_ready", lambda url: False) + monkeypatch.setattr("headroom.cli.install.runtime_status", lambda manifest: "running") + monkeypatch.setattr("headroom.cli.install.wait_ready", lambda manifest, timeout_seconds: False) + monkeypatch.setattr("headroom.cli.install.stop_runtime", lambda manifest: calls.append("stop")) + monkeypatch.setattr( + "headroom.cli.install.start_detached_agent", + lambda profile: calls.append("start_agent"), + ) + monkeypatch.setattr( + "headroom.cli.install.start_persistent_docker", + lambda manifest: calls.append("start_docker"), + ) + + import contextlib + + @contextlib.contextmanager + def fake_lock(profile): + yield True + + monkeypatch.setattr("headroom.cli.install.acquire_runtime_start_lock", fake_lock) + monkeypatch.setattr( + "headroom.cli.install._start_deployment", lambda manifest: calls.append("start_deployment") + ) + + result = runner.invoke(main, ["install", "agent", "ensure"]) + assert result.exit_code == 0, result.output + # stop must come before start_deployment — that's the bug guard. + assert calls.index("stop") < calls.index("start_deployment") + assert "start_agent" not in calls + assert "start_docker" not in calls + + +def test_install_agent_ensure_starts_when_stopped_and_lock_acquired(monkeypatch) -> None: + """Ensure starts a runtime when none is running and lock is acquired.""" + runner = CliRunner() + calls: list[str] = [] + + class Manifest: + profile = "default" + health_url = "http://127.0.0.1:8787/readyz" + preset = "persistent-task" + supervisor_kind = "none" + + monkeypatch.setattr("headroom.cli.install.load_manifest", lambda profile: Manifest()) + monkeypatch.setattr("headroom.cli.install.probe_ready", lambda url: False) + monkeypatch.setattr("headroom.cli.install.runtime_status", lambda manifest: "stopped") + monkeypatch.setattr( + "headroom.cli.install.start_detached_agent", + lambda profile: calls.append("start_agent"), + ) + monkeypatch.setattr( + "headroom.cli.install.start_persistent_docker", + lambda manifest: calls.append("start_docker"), + ) + + import contextlib + + @contextlib.contextmanager + def fake_lock(profile): + yield True + + monkeypatch.setattr("headroom.cli.install.acquire_runtime_start_lock", fake_lock) + monkeypatch.setattr("headroom.cli.install.wait_ready", lambda manifest, timeout_seconds: True) + + result = runner.invoke(main, ["install", "agent", "ensure"]) + assert result.exit_code == 0, result.output + assert calls == ["start_agent"] + + +def test_install_agent_ensure_no_duplicate_spawn_after_lock_recheck(monkeypatch) -> None: + """Ensure does not spawn if proxy becomes ready between initial probe and lock.""" + runner = CliRunner() + calls: list[str] = [] + + class Manifest: + profile = "default" + health_url = "http://127.0.0.1:8787/readyz" + + # First probe_ready (before lock) returns False, second (after lock) returns True + probe_results = iter([False, True]) + monkeypatch.setattr("headroom.cli.install.load_manifest", lambda profile: Manifest()) + monkeypatch.setattr("headroom.cli.install.probe_ready", lambda url: next(probe_results)) + + monkeypatch.setattr( + "headroom.cli.install.start_detached_agent", + lambda profile: calls.append("start_agent"), + ) + + import contextlib + + @contextlib.contextmanager + def fake_lock(profile): + yield True + + monkeypatch.setattr("headroom.cli.install.acquire_runtime_start_lock", fake_lock) + + result = runner.invoke(main, ["install", "agent", "ensure"]) + assert result.exit_code == 0, result.output + assert "already healthy" in result.output + assert calls == [] + + +def test_install_agent_ensure_propagates_start_deployment_failure(monkeypatch) -> None: + """Ensure must exit non-zero and surface the error when _start_deployment fails. + + Regression for review feedback on PR #1301: the previous implementation wrapped + the guarded block in `except Exception` and returned normally, which made + a failed ensure indistinguishable from a successful one. Automation callers + need a non-zero exit code to detect that the deployment did not come up. + """ + runner = CliRunner() + + class Manifest: + profile = "default" + health_url = "http://127.0.0.1:8787/readyz" + preset = "persistent-task" + supervisor_kind = "none" + + monkeypatch.setattr("headroom.cli.install.load_manifest", lambda profile: Manifest()) + monkeypatch.setattr("headroom.cli.install.probe_ready", lambda url: False) + monkeypatch.setattr("headroom.cli.install.runtime_status", lambda manifest: "stopped") + + import contextlib + + @contextlib.contextmanager + def fake_lock(profile): + yield True + + monkeypatch.setattr("headroom.cli.install.acquire_runtime_start_lock", fake_lock) + + def boom(manifest): + raise click.ClickException("simulated start failure") + + monkeypatch.setattr("headroom.cli.install._start_deployment", boom) + + result = runner.invoke(main, ["install", "agent", "ensure"]) + assert result.exit_code != 0, f"expected non-zero exit, got {result.exit_code}: {result.output}" + assert "simulated start failure" in result.output