diff --git a/bot_bottle/backend/docker/launch.py b/bot_bottle/backend/docker/launch.py index 8537056..f4a3043 100644 --- a/bot_bottle/backend/docker/launch.py +++ b/bot_bottle/backend/docker/launch.py @@ -134,13 +134,14 @@ def launch( token_values = egress_resolve_token_values( plan.egress_plan.token_env_map, effective_env, ) + teardown_timeout = resolve_teardown_timeout() ctx = launch_consolidated( plan.egress_plan, git_gate_plan, image_ref=plan.image, tokens=token_values, ) stack.callback( teardown_consolidated, ctx.bottle_id, orchestrator_url=ctx.orchestrator_url, - timeout=resolve_teardown_timeout(), + timeout=teardown_timeout, ) # Step 4: install the SHARED gateway CA into the agent (replaces the diff --git a/bot_bottle/backend/firecracker/launch.py b/bot_bottle/backend/firecracker/launch.py index daed1fc..a5efc00 100644 --- a/bot_bottle/backend/firecracker/launch.py +++ b/bot_bottle/backend/firecracker/launch.py @@ -113,6 +113,7 @@ def launch( token_values = egress_resolve_token_values( plan.egress_plan.token_env_map, effective_env, ) + teardown_timeout = resolve_teardown_timeout() ctx = launch_consolidated( plan.egress_plan, git_gate_plan, guest_ip=slot.guest_ip, @@ -122,7 +123,7 @@ def launch( stack.callback( teardown_consolidated, ctx.bottle_id, orchestrator_url=ctx.orchestrator_url, - timeout=resolve_teardown_timeout(), + timeout=teardown_timeout, ) # Step 5: install the SHARED gateway CA (replaces the per-bottle CA). diff --git a/bot_bottle/backend/macos_container/launch.py b/bot_bottle/backend/macos_container/launch.py index 0d66d5c..c498130 100644 --- a/bot_bottle/backend/macos_container/launch.py +++ b/bot_bottle/backend/macos_container/launch.py @@ -143,6 +143,7 @@ def launch( token_values = egress_resolve_token_values( plan.egress_plan.token_env_map, effective_env, ) + teardown_timeout = resolve_teardown_timeout() ctx = register_agent( plan.egress_plan, plan.git_gate_plan, @@ -154,7 +155,7 @@ def launch( stack.callback( teardown_consolidated, ctx.bottle_id, orchestrator_url=ctx.orchestrator_url, - timeout=resolve_teardown_timeout(), + timeout=teardown_timeout, ) info( f"agent {plan.container_name} registered " diff --git a/tests/unit/test_orchestrator_config_store.py b/tests/unit/test_orchestrator_config_store.py new file mode 100644 index 0000000..d56043d --- /dev/null +++ b/tests/unit/test_orchestrator_config_store.py @@ -0,0 +1,149 @@ +"""Unit: OrchestratorConfigStore and resolve_teardown_timeout. + +Also verifies the lifecycle ordering invariant: resolve_teardown_timeout() +must be called before launch_consolidated() / register_agent() so that a +resolver failure cannot leave an orphaned registration with no teardown +callback. +""" + +from __future__ import annotations + +import inspect +import os +import tempfile +import unittest +from pathlib import Path +from unittest.mock import patch + +from bot_bottle.orchestrator.config_store import ( + DEFAULT_TEARDOWN_TIMEOUT_SECONDS, + TEARDOWN_TIMEOUT_ENV, + TEARDOWN_TIMEOUT_KEY, + OrchestratorConfigStore, + resolve_teardown_timeout, +) + + +class TestOrchestratorConfigStore(unittest.TestCase): + def setUp(self) -> None: + self._tmp = tempfile.TemporaryDirectory() + self.db = Path(self._tmp.name) / "test.db" + self.store = OrchestratorConfigStore(self.db) + self.store.migrate() + + def tearDown(self) -> None: + self._tmp.cleanup() + + def test_get_returns_none_for_absent_key(self) -> None: + self.assertIsNone(self.store.get("missing")) + + def test_set_and_get_roundtrip(self) -> None: + self.store.set("foo", "bar") + self.assertEqual("bar", self.store.get("foo")) + + def test_set_upserts_existing_key(self) -> None: + self.store.set("k", "v1") + self.store.set("k", "v2") + self.assertEqual("v2", self.store.get("k")) + + def test_delete_removes_key(self) -> None: + self.store.set("k", "v") + deleted = self.store.delete("k") + self.assertTrue(deleted) + self.assertIsNone(self.store.get("k")) + + def test_delete_absent_returns_false(self) -> None: + self.assertFalse(self.store.delete("nope")) + + def test_is_migrated_true_after_migrate(self) -> None: + self.assertTrue(self.store.is_migrated()) + + def test_is_migrated_false_before_migrate(self) -> None: + store = OrchestratorConfigStore(Path(self._tmp.name) / "new.db") + self.assertFalse(store.is_migrated()) + + +class TestResolveTeardownTimeout(unittest.TestCase): + def setUp(self) -> None: + self._tmp = tempfile.TemporaryDirectory() + self.db = Path(self._tmp.name) / "cfg.db" + + def tearDown(self) -> None: + self._tmp.cleanup() + os.environ.pop(TEARDOWN_TIMEOUT_ENV, None) + + def test_returns_default_when_nothing_configured(self) -> None: + self.assertEqual( + DEFAULT_TEARDOWN_TIMEOUT_SECONDS, + resolve_teardown_timeout(self.db), + ) + + def test_env_var_overrides_default(self) -> None: + os.environ[TEARDOWN_TIMEOUT_ENV] = "99" + self.assertEqual(99.0, resolve_teardown_timeout(self.db)) + + def test_env_var_overrides_db_value(self) -> None: + store = OrchestratorConfigStore(self.db) + store.migrate() + store.set(TEARDOWN_TIMEOUT_KEY, "55") + os.environ[TEARDOWN_TIMEOUT_ENV] = "77" + self.assertEqual(77.0, resolve_teardown_timeout(self.db)) + + def test_db_value_overrides_default(self) -> None: + store = OrchestratorConfigStore(self.db) + store.migrate() + store.set(TEARDOWN_TIMEOUT_KEY, "42") + self.assertEqual(42.0, resolve_teardown_timeout(self.db)) + + def test_invalid_env_var_falls_through_to_default(self) -> None: + os.environ[TEARDOWN_TIMEOUT_ENV] = "not-a-number" + self.assertEqual(DEFAULT_TEARDOWN_TIMEOUT_SECONDS, resolve_teardown_timeout(self.db)) + + def test_non_positive_env_var_falls_through_to_default(self) -> None: + os.environ[TEARDOWN_TIMEOUT_ENV] = "0" + self.assertEqual(DEFAULT_TEARDOWN_TIMEOUT_SECONDS, resolve_teardown_timeout(self.db)) + + def test_invalid_db_value_falls_through_to_default(self) -> None: + store = OrchestratorConfigStore(self.db) + store.migrate() + store.set(TEARDOWN_TIMEOUT_KEY, "bad") + self.assertEqual(DEFAULT_TEARDOWN_TIMEOUT_SECONDS, resolve_teardown_timeout(self.db)) + + def test_migrates_db_on_first_call(self) -> None: + # DB does not exist yet; resolve_teardown_timeout must not raise. + result = resolve_teardown_timeout(self.db) + self.assertEqual(DEFAULT_TEARDOWN_TIMEOUT_SECONDS, result) + self.assertTrue(OrchestratorConfigStore(self.db).is_migrated()) + + +class TestTeardownTimeoutResolvedBeforeRegistration(unittest.TestCase): + """Ordering invariant: if resolve_teardown_timeout() raises, the bottle + must not yet be registered — no orphaned state can result.""" + + def _src(self, module) -> str: + return inspect.getsource(module) + + def test_docker_resolves_timeout_before_launch_consolidated(self) -> None: + from bot_bottle.backend.docker import launch + src = self._src(launch) + resolve_at = src.index("teardown_timeout = resolve_teardown_timeout()") + launch_at = src.index("ctx = launch_consolidated(") + self.assertLess(resolve_at, launch_at) + + def test_firecracker_resolves_timeout_before_launch_consolidated(self) -> None: + from bot_bottle.backend.firecracker import launch + src = self._src(launch) + resolve_at = src.index("teardown_timeout = resolve_teardown_timeout()") + launch_at = src.index("ctx = launch_consolidated(") + self.assertLess(resolve_at, launch_at) + + def test_macos_resolves_timeout_before_register_agent(self) -> None: + from bot_bottle.backend.macos_container import launch + src = self._src(launch) + resolve_at = src.index("teardown_timeout = resolve_teardown_timeout()") + register_at = src.index("ctx = register_agent(") + self.assertLess(resolve_at, register_at) + + +if __name__ == "__main__": + unittest.main()