fix: make orchestrator teardown timeout configurable (closes #435)
Resolves via ENV VAR -> orchestrator DB config -> default (30 s, up from 5 s): BOT_BOTTLE_ORCHESTRATOR_TEARDOWN_TIMEOUT_SECONDS teardown_timeout_seconds key in new orchestrator_config table (bot-bottle.db) New OrchestratorConfigStore (same DbStore/TableMigrations pattern as the registry) stores the DB-level setting. resolve_teardown_timeout() implements the priority chain and is called at stack.callback registration time in all three backends (macos_container, docker, firecracker).
This commit is contained in:
@@ -142,11 +142,19 @@ def launch_consolidated(
|
|||||||
|
|
||||||
|
|
||||||
def teardown_consolidated(
|
def teardown_consolidated(
|
||||||
bottle_id: str, *, orchestrator_url: str, gateway_name: str = GATEWAY_NAME,
|
bottle_id: str,
|
||||||
|
*,
|
||||||
|
orchestrator_url: str,
|
||||||
|
gateway_name: str = GATEWAY_NAME,
|
||||||
|
timeout: float | None = None,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Deregister the bottle and remove its git-gate state from the gateway.
|
"""Deregister the bottle and remove its git-gate state from the gateway.
|
||||||
Both steps are idempotent so this is safe from a cleanup trap."""
|
Both steps are idempotent so this is safe from a cleanup trap."""
|
||||||
OrchestratorClient(orchestrator_url).teardown_bottle(bottle_id)
|
from ...orchestrator.config_store import DEFAULT_TEARDOWN_TIMEOUT_SECONDS
|
||||||
|
OrchestratorClient(
|
||||||
|
orchestrator_url,
|
||||||
|
timeout=timeout if timeout is not None else DEFAULT_TEARDOWN_TIMEOUT_SECONDS,
|
||||||
|
).teardown_bottle(bottle_id)
|
||||||
deprovision_git_gate(DockerGatewayTransport(gateway_name), bottle_id)
|
deprovision_git_gate(DockerGatewayTransport(gateway_name), bottle_id)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -62,6 +62,7 @@ from .compose import (
|
|||||||
write_compose_file,
|
write_compose_file,
|
||||||
)
|
)
|
||||||
from .consolidated_compose import consolidated_agent_compose
|
from .consolidated_compose import consolidated_agent_compose
|
||||||
|
from ...orchestrator.config_store import resolve_teardown_timeout
|
||||||
from .consolidated_launch import launch_consolidated, teardown_consolidated
|
from .consolidated_launch import launch_consolidated, teardown_consolidated
|
||||||
from ...orchestrator.gateway import DockerGateway
|
from ...orchestrator.gateway import DockerGateway
|
||||||
|
|
||||||
@@ -137,7 +138,9 @@ def launch(
|
|||||||
plan.egress_plan, git_gate_plan, image_ref=plan.image, tokens=token_values,
|
plan.egress_plan, git_gate_plan, image_ref=plan.image, tokens=token_values,
|
||||||
)
|
)
|
||||||
stack.callback(
|
stack.callback(
|
||||||
teardown_consolidated, ctx.bottle_id, orchestrator_url=ctx.orchestrator_url,
|
teardown_consolidated, ctx.bottle_id,
|
||||||
|
orchestrator_url=ctx.orchestrator_url,
|
||||||
|
timeout=resolve_teardown_timeout(),
|
||||||
)
|
)
|
||||||
|
|
||||||
# Step 4: install the SHARED gateway CA into the agent (replaces the
|
# Step 4: install the SHARED gateway CA into the agent (replaces the
|
||||||
|
|||||||
@@ -91,12 +91,18 @@ def launch_consolidated(
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
def teardown_consolidated(bottle_id: str, *, orchestrator_url: str) -> None:
|
def teardown_consolidated(
|
||||||
|
bottle_id: str, *, orchestrator_url: str, timeout: float | None = None,
|
||||||
|
) -> None:
|
||||||
"""Deregister the bottle and remove its git-gate state from the gateway
|
"""Deregister the bottle and remove its git-gate state from the gateway
|
||||||
VM. Both steps are idempotent so this is safe from a cleanup trap. Does
|
VM. Both steps are idempotent so this is safe from a cleanup trap. Does
|
||||||
NOT stop the infra VM — it's a persistent per-host singleton shared by
|
NOT stop the infra VM — it's a persistent per-host singleton shared by
|
||||||
every bottle."""
|
every bottle."""
|
||||||
OrchestratorClient(orchestrator_url).teardown_bottle(bottle_id)
|
from ...orchestrator.config_store import DEFAULT_TEARDOWN_TIMEOUT_SECONDS
|
||||||
|
OrchestratorClient(
|
||||||
|
orchestrator_url,
|
||||||
|
timeout=timeout if timeout is not None else DEFAULT_TEARDOWN_TIMEOUT_SECONDS,
|
||||||
|
).teardown_bottle(bottle_id)
|
||||||
deprovision_git_gate(infra_vm.gateway_transport(), bottle_id)
|
deprovision_git_gate(infra_vm.gateway_transport(), bottle_id)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -52,6 +52,7 @@ from ..util import AGENT_CA_BUNDLE, AGENT_CA_PATH
|
|||||||
from . import firecracker_vm, image_builder, isolation_probe, netpool, util
|
from . import firecracker_vm, image_builder, isolation_probe, netpool, util
|
||||||
from .bottle import FirecrackerBottle
|
from .bottle import FirecrackerBottle
|
||||||
from .bottle_plan import FirecrackerBottlePlan
|
from .bottle_plan import FirecrackerBottlePlan
|
||||||
|
from ...orchestrator.config_store import resolve_teardown_timeout
|
||||||
from .consolidated_launch import (
|
from .consolidated_launch import (
|
||||||
launch_consolidated,
|
launch_consolidated,
|
||||||
teardown_consolidated,
|
teardown_consolidated,
|
||||||
@@ -121,6 +122,7 @@ def launch(
|
|||||||
stack.callback(
|
stack.callback(
|
||||||
teardown_consolidated, ctx.bottle_id,
|
teardown_consolidated, ctx.bottle_id,
|
||||||
orchestrator_url=ctx.orchestrator_url,
|
orchestrator_url=ctx.orchestrator_url,
|
||||||
|
timeout=resolve_teardown_timeout(),
|
||||||
)
|
)
|
||||||
|
|
||||||
# Step 5: install the SHARED gateway CA (replaces the per-bottle CA).
|
# Step 5: install the SHARED gateway CA (replaces the per-bottle CA).
|
||||||
|
|||||||
@@ -163,11 +163,17 @@ def register_agent(
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
def teardown_consolidated(bottle_id: str, *, orchestrator_url: str) -> None:
|
def teardown_consolidated(
|
||||||
|
bottle_id: str, *, orchestrator_url: str, timeout: float | None = None,
|
||||||
|
) -> None:
|
||||||
"""Deregister the bottle and remove its git-gate state from the gateway.
|
"""Deregister the bottle and remove its git-gate state from the gateway.
|
||||||
Both steps are idempotent so this is safe from a cleanup trap. Does NOT
|
Both steps are idempotent so this is safe from a cleanup trap. Does NOT
|
||||||
stop the gateway — it's a persistent per-host singleton."""
|
stop the gateway — it's a persistent per-host singleton."""
|
||||||
OrchestratorClient(orchestrator_url).teardown_bottle(bottle_id)
|
from ...orchestrator.config_store import DEFAULT_TEARDOWN_TIMEOUT_SECONDS
|
||||||
|
OrchestratorClient(
|
||||||
|
orchestrator_url,
|
||||||
|
timeout=timeout if timeout is not None else DEFAULT_TEARDOWN_TIMEOUT_SECONDS,
|
||||||
|
).teardown_bottle(bottle_id)
|
||||||
deprovision_git_gate(AppleGatewayTransport(), bottle_id)
|
deprovision_git_gate(AppleGatewayTransport(), bottle_id)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -65,6 +65,7 @@ from .gateway_hosts import (
|
|||||||
set_gateway_host,
|
set_gateway_host,
|
||||||
)
|
)
|
||||||
from .bottle_plan import MacosContainerBottlePlan
|
from .bottle_plan import MacosContainerBottlePlan
|
||||||
|
from ...orchestrator.config_store import resolve_teardown_timeout
|
||||||
from .consolidated_launch import (
|
from .consolidated_launch import (
|
||||||
GatewayEndpoint,
|
GatewayEndpoint,
|
||||||
ensure_gateway,
|
ensure_gateway,
|
||||||
@@ -153,6 +154,7 @@ def launch(
|
|||||||
stack.callback(
|
stack.callback(
|
||||||
teardown_consolidated, ctx.bottle_id,
|
teardown_consolidated, ctx.bottle_id,
|
||||||
orchestrator_url=ctx.orchestrator_url,
|
orchestrator_url=ctx.orchestrator_url,
|
||||||
|
timeout=resolve_teardown_timeout(),
|
||||||
)
|
)
|
||||||
info(
|
info(
|
||||||
f"agent {plan.container_name} registered "
|
f"agent {plan.container_name} registered "
|
||||||
|
|||||||
@@ -0,0 +1,112 @@
|
|||||||
|
"""Per-host orchestrator configuration store (key/value settings in bot-bottle.db).
|
||||||
|
|
||||||
|
Co-tenants the shared `bot-bottle.db` via the `DbStore` framework. Settings
|
||||||
|
are readable by the host launch path directly (no HTTP round-trip to the
|
||||||
|
orchestrator), so they take effect even before the orchestrator is reachable.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import os
|
||||||
|
import sqlite3
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
from ..db_store import DbStore
|
||||||
|
from ..migrations import TableMigrations
|
||||||
|
from ..paths import host_db_path
|
||||||
|
|
||||||
|
TEARDOWN_TIMEOUT_KEY = "teardown_timeout_seconds"
|
||||||
|
TEARDOWN_TIMEOUT_ENV = "BOT_BOTTLE_ORCHESTRATOR_TEARDOWN_TIMEOUT_SECONDS"
|
||||||
|
DEFAULT_TEARDOWN_TIMEOUT_SECONDS = 30.0
|
||||||
|
|
||||||
|
_MIGRATIONS = TableMigrations(
|
||||||
|
"orchestrator_config",
|
||||||
|
[
|
||||||
|
"""
|
||||||
|
CREATE TABLE IF NOT EXISTS orchestrator_config (
|
||||||
|
key TEXT PRIMARY KEY,
|
||||||
|
value TEXT NOT NULL
|
||||||
|
)
|
||||||
|
""",
|
||||||
|
],
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
class OrchestratorConfigStore(DbStore):
|
||||||
|
"""Key/value store for orchestrator settings in the shared host DB."""
|
||||||
|
|
||||||
|
def __init__(self, db_path: Path | None = None) -> None:
|
||||||
|
super().__init__(db_path or host_db_path(), _MIGRATIONS)
|
||||||
|
|
||||||
|
def _connect(self) -> sqlite3.Connection:
|
||||||
|
conn = super()._connect()
|
||||||
|
conn.execute("PRAGMA busy_timeout=5000")
|
||||||
|
return conn
|
||||||
|
|
||||||
|
def get(self, key: str) -> str | None:
|
||||||
|
"""Return the value for `key`, or None if absent."""
|
||||||
|
try:
|
||||||
|
with self._connection() as conn:
|
||||||
|
row = conn.execute(
|
||||||
|
"SELECT value FROM orchestrator_config WHERE key = ?", (key,)
|
||||||
|
).fetchone()
|
||||||
|
except sqlite3.OperationalError:
|
||||||
|
return None
|
||||||
|
return row["value"] if row else None
|
||||||
|
|
||||||
|
def set(self, key: str, value: str) -> None:
|
||||||
|
"""Upsert a setting."""
|
||||||
|
with self._connection() as conn:
|
||||||
|
conn.execute(
|
||||||
|
"INSERT OR REPLACE INTO orchestrator_config (key, value) VALUES (?, ?)",
|
||||||
|
(key, value),
|
||||||
|
)
|
||||||
|
self._chmod()
|
||||||
|
|
||||||
|
def delete(self, key: str) -> bool:
|
||||||
|
"""Remove a setting. Returns True if the key existed."""
|
||||||
|
with self._connection() as conn:
|
||||||
|
cur = conn.execute(
|
||||||
|
"DELETE FROM orchestrator_config WHERE key = ?", (key,)
|
||||||
|
)
|
||||||
|
return cur.rowcount > 0
|
||||||
|
|
||||||
|
|
||||||
|
def resolve_teardown_timeout(db_path: Path | None = None) -> float:
|
||||||
|
"""Return the teardown timeout to use, in priority order:
|
||||||
|
|
||||||
|
1. ``BOT_BOTTLE_ORCHESTRATOR_TEARDOWN_TIMEOUT_SECONDS`` env var
|
||||||
|
2. ``teardown_timeout_seconds`` key in the orchestrator config DB
|
||||||
|
3. ``DEFAULT_TEARDOWN_TIMEOUT_SECONDS`` (30 s)
|
||||||
|
"""
|
||||||
|
raw = os.environ.get(TEARDOWN_TIMEOUT_ENV, "").strip()
|
||||||
|
if raw:
|
||||||
|
try:
|
||||||
|
value = float(raw)
|
||||||
|
if value > 0:
|
||||||
|
return value
|
||||||
|
except ValueError:
|
||||||
|
pass
|
||||||
|
|
||||||
|
store = OrchestratorConfigStore(db_path)
|
||||||
|
if not store.is_migrated():
|
||||||
|
store.migrate()
|
||||||
|
raw_db = store.get(TEARDOWN_TIMEOUT_KEY)
|
||||||
|
if raw_db is not None:
|
||||||
|
try:
|
||||||
|
value = float(raw_db)
|
||||||
|
if value > 0:
|
||||||
|
return value
|
||||||
|
except ValueError:
|
||||||
|
pass
|
||||||
|
|
||||||
|
return DEFAULT_TEARDOWN_TIMEOUT_SECONDS
|
||||||
|
|
||||||
|
|
||||||
|
__all__ = [
|
||||||
|
"OrchestratorConfigStore",
|
||||||
|
"resolve_teardown_timeout",
|
||||||
|
"TEARDOWN_TIMEOUT_KEY",
|
||||||
|
"TEARDOWN_TIMEOUT_ENV",
|
||||||
|
"DEFAULT_TEARDOWN_TIMEOUT_SECONDS",
|
||||||
|
]
|
||||||
Reference in New Issue
Block a user