mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
feat(sandbox): pluggable per-engine registry (postgres/redis/mongo) (#324)
* feat(sandbox): pluggable per-engine registry (postgres/redis/mongo) Replaces the hardcoded postgres+redis branches in the provisioner and the env emitter with a registry of SandboxEngine specs (image, run args, readiness probe, connection, ROBOCO_TEST_* env) in a pure low module (roboco/models/sandbox.py). VALID_SANDBOX_SERVICES is derived from the registry — single source of truth — and the provisioner + orchestrator iterate it, so adding an engine is one class + one registry line, not another branch. Adds a mongo:8-alpine engine (ROBOCO_TEST_MONGO_*) as the third service alongside postgres/redis. Also fixes the cold-pull loop that stranded v0.19.0 board agents with empty error strings: docker run pulled inline under a 20s deadline, so a NAS cold pull was killed, cancelled, and re-pulled from scratch forever. _ensure_image now inspects + pulls (300s) before run; provisioning errors log type+message so a bare TimeoutError no longer shows as "". Panel edit-project dialog: postgres/redis toggles -> a Set<string> multi-select driven by a SANDBOX_SERVICES catalog, so new engines appear in the UI by adding to the catalog. Tests: engine parity (allowlist==registry, unique slugs/images, no None leak in env, SandboxInfo aggregates every engine), mongo provision + env injection, plus the existing postgres/redis provision/env/spawn/janitor suite updated to the registry shape. 821 unit / 5 skip green; ruff + mypy (360 files) clean. * docs(sandbox): reflect pluggable engine registry + mongo across docs CHANGELOG (0.19.0): Added entry for the pluggable sandbox engine registry (postgres/redis/mongo) + Fixed entry for the cold-pull loop/empty-error strand that boarded v0.19.0 board agents. docs/map (9 files): sandbox subsystem blurbs, SandboxProvisioner rows, _maybe_provision_sandbox/_append_sandbox_env rows, feature-flag rows, the migration-057 row + v0.17.0 delta, and the models.md VALID_SANDBOX_SERVICES note — all retitled to DB/Redis/Mongo via the engine registry (roboco/models/sandbox.py), with the one-class-one-line extension story and the _ensure_image cold-pull fix. Production-network (roboco_data) lines left as postgres+redis — mongo is sandbox-only, not a prod service. docs/rag (3 files): sandbox-db.md rewritten around the registry (engine list, generic _provision_engine, image pre-pull, ROBOCO_TEST_DB_*/REDIS_*/MONGO_* incl. MONGO_AUTH_DB=admin, single emit_env); config-reference sandbox flag row + subsection retitled; db-network-isolation framing broadened to postgres/redis/mongo. preconditions-and-rejections left untouched (its hit was an unrelated gateway see-also link). * test(e2e): harden umbrella close terminal reads with bounded wait-for-state The MegaTask umbrella close test flaked once on CI (ceo-approve returned 200 but the re-fetch saw awaiting_pm_review) then passed on re-run. The production path is deterministic: complete -> main_pm_complete -> submit_pm_review -> escalate_to_ceo -> ceo_approve -> commit, all on one session, all awaited; the fire-and-forget completion hooks are isolated (own session, best-effort, never touch task.status or the request session). 20 local runs could not reproduce it. The one real surface is the read pattern: the e2e stack commits on the uvicorn thread's loop and reads via a separate loop (run_db -> asyncio.run with a fresh engine), so a terminal single point-read can race a still-draining completion hook on a contended runner. Replace the two terminal point-reads with a bounded wait_for_status poll. Strictly better than a one-shot read: absorbs the transient, and a genuine state bug still surfaces via the timeout branch asserting against the last-read state. --------- Co-authored-by: Renn F <rennf93@users.noreply.github.com>
This commit is contained in:
@@ -1,9 +1,9 @@
|
||||
"""Project.sandbox_services / ProjectUpdate.sandbox_services validation.
|
||||
|
||||
Only "postgres" and "redis" are recognized sandbox services (mirrors the
|
||||
provisioner's VALID_SANDBOX_SERVICES) — an unknown value must be rejected with
|
||||
a clear message rather than silently accepted and later failing at provision
|
||||
time inside a container spawn.
|
||||
Recognized services are whatever the engine registry exposes
|
||||
(``VALID_SANDBOX_SERVICES`` in ``roboco.models.sandbox`` — postgres, redis,
|
||||
mongo) — an unknown value must be rejected with a clear message rather than
|
||||
silently accepted and later failing at provision time inside a container spawn.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -32,6 +32,11 @@ def test_project_accepts_valid_sandbox_services() -> None:
|
||||
assert project.sandbox_services == ["postgres", "redis"]
|
||||
|
||||
|
||||
def test_project_accepts_mongo() -> None:
|
||||
project = _project(sandbox_services=["mongo"])
|
||||
assert project.sandbox_services == ["mongo"]
|
||||
|
||||
|
||||
def test_project_normalizes_sandbox_services_order_and_dupes() -> None:
|
||||
project = _project(sandbox_services=["redis", "postgres", "redis"])
|
||||
assert project.sandbox_services == ["postgres", "redis"]
|
||||
|
||||
@@ -0,0 +1,52 @@
|
||||
"""Engine registry / allowlist parity + per-engine internal consistency.
|
||||
|
||||
The valid-service allowlist is derived from the registry
|
||||
(``VALID_SANDBOX_SERVICES = frozenset(SANDBOX_ENGINES)``), so the two must stay
|
||||
in lockstep — a drift guard against adding an engine class without registering
|
||||
it (or vice versa). Each engine's emitted env must also reference only the
|
||||
connection fields it actually populates (no ``None`` leaking into an env value).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from roboco.models.sandbox import (
|
||||
SANDBOX_ENGINES,
|
||||
VALID_SANDBOX_SERVICES,
|
||||
SandboxInfo,
|
||||
)
|
||||
|
||||
_ENV_HOST_PREFIX = {
|
||||
"postgres": "ROBOCO_TEST_DB_HOST",
|
||||
"redis": "ROBOCO_TEST_REDIS_HOST",
|
||||
"mongo": "ROBOCO_TEST_MONGO_HOST",
|
||||
}
|
||||
|
||||
|
||||
def test_allowlist_matches_registry() -> None:
|
||||
assert frozenset(SANDBOX_ENGINES) == VALID_SANDBOX_SERVICES
|
||||
assert set(SANDBOX_ENGINES) == {"postgres", "redis", "mongo"}
|
||||
|
||||
|
||||
def test_each_engine_has_unique_container_slug_and_image() -> None:
|
||||
slugs = {e.container_slug for e in SANDBOX_ENGINES.values()}
|
||||
images = {e.image for e in SANDBOX_ENGINES.values()}
|
||||
assert len(slugs) == len(SANDBOX_ENGINES)
|
||||
assert len(images) == len(SANDBOX_ENGINES)
|
||||
|
||||
|
||||
def test_each_engine_emit_env_references_only_populated_fields() -> None:
|
||||
# An engine that does not set `user`/`database` must not emit a `None` value.
|
||||
for engine in SANDBOX_ENGINES.values():
|
||||
conn = engine.connection(host=f"h-{engine.name}", password="pw")
|
||||
env = " ".join(engine.emit_env(conn))
|
||||
assert "None" not in env, f"{engine.name} leaked None into env: {env}"
|
||||
|
||||
|
||||
def test_sandbox_info_emit_env_aggregates_every_engine() -> None:
|
||||
services = {
|
||||
name: engine.connection(host=f"h-{name}", password="pw")
|
||||
for name, engine in SANDBOX_ENGINES.items()
|
||||
}
|
||||
flat = " ".join(SandboxInfo(services=services).emit_env())
|
||||
for name in SANDBOX_ENGINES:
|
||||
assert _ENV_HOST_PREFIX[name] in flat
|
||||
@@ -12,12 +12,8 @@ from pathlib import Path
|
||||
from unittest.mock import AsyncMock
|
||||
|
||||
import pytest
|
||||
from roboco.models.runtime import (
|
||||
OrchestratorAgentConfig,
|
||||
PostgresSandbox,
|
||||
RedisSandbox,
|
||||
SandboxInfo,
|
||||
)
|
||||
from roboco.models.runtime import OrchestratorAgentConfig
|
||||
from roboco.models.sandbox import SandboxConnection, SandboxInfo
|
||||
from roboco.runtime.orchestrator import AgentOrchestrator
|
||||
|
||||
|
||||
@@ -32,16 +28,18 @@ def _config(sandbox_info: SandboxInfo | None = None) -> OrchestratorAgentConfig:
|
||||
|
||||
def test_append_sandbox_env_injects_postgres_and_redis() -> None:
|
||||
info = SandboxInfo(
|
||||
postgres=PostgresSandbox(
|
||||
host="roboco-sandbox-pg-dev-1",
|
||||
port=5432,
|
||||
user="sandbox",
|
||||
password="pgpw",
|
||||
database="sandbox",
|
||||
),
|
||||
redis=RedisSandbox(
|
||||
host="roboco-sandbox-redis-dev-1", port=6379, password="rdpw"
|
||||
),
|
||||
services={
|
||||
"postgres": SandboxConnection(
|
||||
host="roboco-sandbox-pg-dev-1",
|
||||
port=5432,
|
||||
password="pgpw",
|
||||
user="sandbox",
|
||||
database="sandbox",
|
||||
),
|
||||
"redis": SandboxConnection(
|
||||
host="roboco-sandbox-redis-dev-1", port=6379, password="rdpw"
|
||||
),
|
||||
}
|
||||
)
|
||||
cmd: list[str] = []
|
||||
AgentOrchestrator._append_sandbox_env(cmd, _config(info))
|
||||
@@ -58,13 +56,15 @@ def test_append_sandbox_env_injects_postgres_and_redis() -> None:
|
||||
|
||||
def test_append_sandbox_env_postgres_only_omits_redis_vars() -> None:
|
||||
info = SandboxInfo(
|
||||
postgres=PostgresSandbox(
|
||||
host="roboco-sandbox-pg-dev-1",
|
||||
port=5432,
|
||||
user="sandbox",
|
||||
password="pgpw",
|
||||
database="sandbox",
|
||||
)
|
||||
services={
|
||||
"postgres": SandboxConnection(
|
||||
host="roboco-sandbox-pg-dev-1",
|
||||
port=5432,
|
||||
password="pgpw",
|
||||
user="sandbox",
|
||||
database="sandbox",
|
||||
)
|
||||
}
|
||||
)
|
||||
cmd: list[str] = []
|
||||
AgentOrchestrator._append_sandbox_env(cmd, _config(info))
|
||||
@@ -73,6 +73,29 @@ def test_append_sandbox_env_postgres_only_omits_redis_vars() -> None:
|
||||
assert not any(v.startswith("ROBOCO_TEST_REDIS_") for v in cmd)
|
||||
|
||||
|
||||
def test_append_sandbox_env_injects_mongo() -> None:
|
||||
info = SandboxInfo(
|
||||
services={
|
||||
"mongo": SandboxConnection(
|
||||
host="roboco-sandbox-mongo-dev-1",
|
||||
port=27017,
|
||||
password="mpw",
|
||||
user="sandbox",
|
||||
database="admin",
|
||||
)
|
||||
}
|
||||
)
|
||||
cmd: list[str] = []
|
||||
AgentOrchestrator._append_sandbox_env(cmd, _config(info))
|
||||
|
||||
assert "ROBOCO_TEST_MONGO_HOST=roboco-sandbox-mongo-dev-1" in cmd
|
||||
assert "ROBOCO_TEST_MONGO_PORT=27017" in cmd
|
||||
assert "ROBOCO_TEST_MONGO_USER=sandbox" in cmd
|
||||
assert "ROBOCO_TEST_MONGO_PASSWORD=mpw" in cmd
|
||||
assert "ROBOCO_TEST_MONGO_AUTH_DB=admin" in cmd
|
||||
assert not any(v.startswith("ROBOCO_TEST_DB_") for v in cmd)
|
||||
|
||||
|
||||
def test_append_sandbox_env_noop_without_sandbox_info() -> None:
|
||||
cmd: list[str] = []
|
||||
AgentOrchestrator._append_sandbox_env(cmd, _config(None))
|
||||
@@ -120,9 +143,11 @@ async def test_spawn_container_uses_sandbox_env_when_sandbox_active(
|
||||
_stub_spawn_container_collaborators(monkeypatch, orch, calls)
|
||||
|
||||
info = SandboxInfo(
|
||||
postgres=PostgresSandbox(
|
||||
host="h", port=5432, user="sandbox", password="pw", database="sandbox"
|
||||
)
|
||||
services={
|
||||
"postgres": SandboxConnection(
|
||||
host="h", port=5432, password="pw", user="sandbox", database="sandbox"
|
||||
)
|
||||
}
|
||||
)
|
||||
await orch._spawn_container(_config(info))
|
||||
|
||||
@@ -155,9 +180,11 @@ async def test_spawn_container_stale_clear_spares_fresh_sandbox(
|
||||
monkeypatch.setattr(orch, "_remove_container", remove)
|
||||
|
||||
info = SandboxInfo(
|
||||
postgres=PostgresSandbox(
|
||||
host="h", port=5432, user="sandbox", password="pw", database="sandbox"
|
||||
)
|
||||
services={
|
||||
"postgres": SandboxConnection(
|
||||
host="h", port=5432, password="pw", user="sandbox", database="sandbox"
|
||||
)
|
||||
}
|
||||
)
|
||||
await orch._spawn_container(_config(info))
|
||||
|
||||
|
||||
@@ -14,7 +14,7 @@ from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
from roboco.config import settings
|
||||
from roboco.models.runtime import PostgresSandbox, SandboxInfo
|
||||
from roboco.models.sandbox import SandboxConnection, SandboxInfo
|
||||
from roboco.runtime.orchestrator import AgentOrchestrator, AgentReadinessError
|
||||
|
||||
|
||||
@@ -100,9 +100,11 @@ async def test_opted_in_project_provisions_sandbox(
|
||||
project_service = MagicMock()
|
||||
project_service.get_by_slug = AsyncMock(return_value=project)
|
||||
info = SandboxInfo(
|
||||
postgres=PostgresSandbox(
|
||||
host="h", port=5432, user="sandbox", password="pw", database="sandbox"
|
||||
)
|
||||
services={
|
||||
"postgres": SandboxConnection(
|
||||
host="h", port=5432, password="pw", user="sandbox", database="sandbox"
|
||||
)
|
||||
}
|
||||
)
|
||||
sandbox.provision.return_value = info
|
||||
|
||||
|
||||
@@ -15,6 +15,7 @@ from roboco.runtime.sandbox import SandboxProvisioner, SandboxProvisionError
|
||||
_NETWORK = "roboco_default"
|
||||
_PG_PORT = 5432
|
||||
_REDIS_PORT = 6379
|
||||
_MONGO_PORT = 27017
|
||||
|
||||
|
||||
class _FakeRunner:
|
||||
@@ -71,10 +72,12 @@ class _FakeRunner:
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _fast_readiness_deadlines(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Shrink the polling deadlines so the timeout path is fast in tests."""
|
||||
monkeypatch.setattr(sandbox_module, "_PG_READY_DEADLINE_SECONDS", 0.05)
|
||||
monkeypatch.setattr(sandbox_module, "_REDIS_READY_DEADLINE_SECONDS", 0.05)
|
||||
"""Shrink the polling cadence + every engine's deadline so the timeout path
|
||||
is fast in tests. Deadlines live on the engine instances now (not module
|
||||
constants), so monkeypatch them on the registry."""
|
||||
monkeypatch.setattr(sandbox_module, "_READY_POLL_INTERVAL_SECONDS", 0.01)
|
||||
for engine in sandbox_module.SANDBOX_ENGINES.values():
|
||||
monkeypatch.setattr(engine, "ready_deadline", 0.05)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -84,16 +87,16 @@ async def test_provision_both_services_happy_path() -> None:
|
||||
|
||||
info = await provisioner.provision("dev-1", ["postgres", "redis"])
|
||||
|
||||
assert info.postgres is not None
|
||||
assert info.postgres.host == "roboco-sandbox-pg-dev-1"
|
||||
assert info.postgres.port == _PG_PORT
|
||||
assert info.postgres.user == "sandbox"
|
||||
assert info.postgres.database == "sandbox"
|
||||
assert info.redis is not None
|
||||
assert info.redis.host == "roboco-sandbox-redis-dev-1"
|
||||
assert info.redis.port == _REDIS_PORT
|
||||
pg = info.services["postgres"]
|
||||
assert pg.host == "roboco-sandbox-pg-dev-1"
|
||||
assert pg.port == _PG_PORT
|
||||
assert pg.user == "sandbox"
|
||||
assert pg.database == "sandbox"
|
||||
rd = info.services["redis"]
|
||||
assert rd.host == "roboco-sandbox-redis-dev-1"
|
||||
assert rd.port == _REDIS_PORT
|
||||
# Passwords are per-sandbox random tokens, not equal to each other.
|
||||
assert info.postgres.password != info.redis.password
|
||||
assert pg.password != rd.password
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -112,6 +115,27 @@ async def test_provision_labels_are_correct() -> None:
|
||||
assert "roboco.sandbox.owner=roboco-agent-dev-2" in labels
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_provision_mongo_engine() -> None:
|
||||
runner = _FakeRunner(run_rc=0, exec_rc=0)
|
||||
provisioner = SandboxProvisioner(network=_NETWORK, runner=runner)
|
||||
|
||||
info = await provisioner.provision("dev-mongo", ["mongo"])
|
||||
|
||||
mongo = info.services["mongo"]
|
||||
assert mongo.host == "roboco-sandbox-mongo-dev-mongo"
|
||||
assert mongo.port == _MONGO_PORT
|
||||
assert mongo.user == "sandbox"
|
||||
assert mongo.database == "admin"
|
||||
run_call = next(c for c in runner.calls if c[0] == "run")
|
||||
assert "mongo:8-alpine" in run_call
|
||||
# MONGO_INITDB_ROOT_PASSWORD env is baked into the run.
|
||||
assert any(a.startswith("MONGO_INITDB_ROOT_PASSWORD=") for a in run_call)
|
||||
# /data/db tmpfs mount for the engine.
|
||||
assert "--tmpfs" in run_call
|
||||
assert run_call[run_call.index("--tmpfs") + 1] == "/data/db"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_provision_readiness_timeout_tears_down_and_raises() -> None:
|
||||
runner = _FakeRunner(run_rc=0, exec_rc=1) # container starts, never ready
|
||||
|
||||
Reference in New Issue
Block a user