From 2575f7a72bd23dd0e491d3f12b7c2e942180bc4c Mon Sep 17 00:00:00 2001 From: Paulo Date: Mon, 24 Aug 2026 19:40:40 +0200 Subject: [PATCH 1/8] ENG-877 - Sandbox templates are records: async create, poll to available MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Templates are keyed by content — unique (provider, base_image, requirements_hash) — with status building -> available -> failed. POST /templates answers 202 and the caller polls; the build runs as a background task against the TemplateCapability a provider may implement (implementations land next). Concurrent identical creates resolve through the unique index: the losing racer re-reads the winner. Co-Authored-By: Claude Fable 5 --- alembic/env.py | 1 + alembic/versions/0004_templates.py | 48 ++++ pyproject.toml | 1 + src/api/app.py | 2 + src/conftest.py | 1 + src/hosts/models.py | 16 +- src/providers/capabilities.py | 20 ++ src/templates/__init__.py | 1 + src/templates/api.py | 89 ++++++++ src/templates/deps.py | 13 ++ src/templates/exceptions.py | 11 + src/templates/models.py | 46 ++++ src/templates/schemas.py | 47 ++++ src/templates/service.py | 148 ++++++++++++ src/templates/tests/__init__.py | 1 + src/templates/tests/conftest.py | 84 +++++++ src/templates/tests/test_api.py | 353 +++++++++++++++++++++++++++++ 17 files changed, 874 insertions(+), 8 deletions(-) create mode 100644 alembic/versions/0004_templates.py create mode 100644 src/templates/__init__.py create mode 100644 src/templates/api.py create mode 100644 src/templates/deps.py create mode 100644 src/templates/exceptions.py create mode 100644 src/templates/models.py create mode 100644 src/templates/schemas.py create mode 100644 src/templates/service.py create mode 100644 src/templates/tests/__init__.py create mode 100644 src/templates/tests/conftest.py create mode 100644 src/templates/tests/test_api.py diff --git a/alembic/env.py b/alembic/env.py index 0bdea12..5163f95 100644 --- a/alembic/env.py +++ b/alembic/env.py @@ -6,6 +6,7 @@ from core.database import Base from core.settings import get_settings from hosts import models # noqa: F401 +from templates import models as template_models # noqa: F401 config = context.config diff --git a/alembic/versions/0004_templates.py b/alembic/versions/0004_templates.py new file mode 100644 index 0000000..8e71ec0 --- /dev/null +++ b/alembic/versions/0004_templates.py @@ -0,0 +1,48 @@ +"""reusable provider templates + +Revision ID: 0004_templates +Revises: 0003_host_public_key +""" + +from collections.abc import Sequence + +import sqlalchemy as sa +from alembic import op + +revision: str = "0004_templates" +down_revision: str | None = "0003_host_public_key" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +def upgrade() -> None: + op.create_table( + "templates", + sa.Column("id", sa.Uuid(), nullable=False), + sa.Column("provider", sa.String(length=20), nullable=False), + sa.Column("base_image", sa.Text(), nullable=False), + sa.Column("requirements_hash", sa.String(length=64), nullable=False), + sa.Column("setup_script", sa.Text(), nullable=False), + sa.Column("label", sa.Text(), nullable=False), + sa.Column("handle", sa.Text(), nullable=False), + sa.Column("status", sa.String(length=32), nullable=False), + sa.Column("last_error", sa.Text(), nullable=False), + sa.Column("created_at", sa.DateTime(timezone=True), nullable=False), + sa.Column("updated_at", sa.DateTime(timezone=True), nullable=False), + sa.Column("last_used_at", sa.DateTime(timezone=True), nullable=True), + sa.PrimaryKeyConstraint("id"), + ) + op.create_index( + "ix_templates_provider_base_image_requirements_hash", + "templates", + ["provider", "base_image", "requirements_hash"], + unique=True, + ) + + +def downgrade() -> None: + op.drop_index( + "ix_templates_provider_base_image_requirements_hash", + table_name="templates", + ) + op.drop_table("templates") diff --git a/pyproject.toml b/pyproject.toml index 500cce2..4693371 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -12,6 +12,7 @@ packages = [ "src/http_proxies", "src/networking", "src/providers", + "src/templates", ] exclude = ["**/tests", "**/tests/**"] diff --git a/src/api/app.py b/src/api/app.py index 3074653..bdec4c7 100644 --- a/src/api/app.py +++ b/src/api/app.py @@ -13,6 +13,7 @@ from http_proxies.api import router as http_proxies_router from networking.tailscale import Tailscale from providers.registry import iter_initialized_vm_providers +from templates.api import router as templates_router _log_level = getattr(logging, os.environ.get("LOG_LEVEL", "INFO").upper(), logging.INFO) @@ -71,4 +72,5 @@ async def healthz() -> dict[str, str]: app.include_router(hosts_router) app.include_router(http_proxies_router) +app.include_router(templates_router) app.include_router(diagnostics_router) diff --git a/src/conftest.py b/src/conftest.py index f8b4336..48acd26 100644 --- a/src/conftest.py +++ b/src/conftest.py @@ -41,6 +41,7 @@ def load_test_env() -> None: async def reset_database() -> AsyncGenerator[None]: from core.database import Base, engine from hosts import models # noqa: F401 + from templates import models as template_models # noqa: F401 async with engine.begin() as connection: await connection.run_sync(Base.metadata.drop_all) diff --git a/src/hosts/models.py b/src/hosts/models.py index 7be3aa0..8d529e8 100644 --- a/src/hosts/models.py +++ b/src/hosts/models.py @@ -41,7 +41,7 @@ def process_result_value(self, value: datetime | None, dialect: object) -> datet return value -_DateTimeUTC = _UTCDateTime() +UTCDateTime = _UTCDateTime() _HOST_NAME_PREFIX = "sb-" # 48 bits of UUIDv7 entropy for a short readable name. UUIDv7's leading 48 @@ -93,20 +93,20 @@ class Host(Base): internal_ssh_host: Mapped[str | None] = mapped_column(Text, nullable=True, default=None) known_hosts: Mapped[str] = mapped_column(Text, default="") tailscale_device_id: Mapped[str | None] = mapped_column(Text, nullable=True, default=None) - created_at: Mapped[datetime] = mapped_column(_DateTimeUTC) - updated_at: Mapped[datetime] = mapped_column(_DateTimeUTC) + created_at: Mapped[datetime] = mapped_column(UTCDateTime) + updated_at: Mapped[datetime] = mapped_column(UTCDateTime) activated_at: Mapped[datetime | None] = mapped_column( - _DateTimeUTC, + UTCDateTime, nullable=True, default=None, ) expires_at: Mapped[datetime | None] = mapped_column( - _DateTimeUTC, + UTCDateTime, nullable=True, default=None, ) claimed_at: Mapped[datetime | None] = mapped_column( - _DateTimeUTC, + UTCDateTime, nullable=True, default=None, ) @@ -147,5 +147,5 @@ class IdempotencyKey(Base): ForeignKey("hosts.id", ondelete="CASCADE"), nullable=False, ) - created_at: Mapped[datetime] = mapped_column(_DateTimeUTC) - expires_at: Mapped[datetime] = mapped_column(_DateTimeUTC, index=True) + created_at: Mapped[datetime] = mapped_column(UTCDateTime) + expires_at: Mapped[datetime] = mapped_column(UTCDateTime, index=True) diff --git a/src/providers/capabilities.py b/src/providers/capabilities.py index 9ab4554..7a2d98e 100644 --- a/src/providers/capabilities.py +++ b/src/providers/capabilities.py @@ -41,3 +41,23 @@ async def attach_http_proxy(self, name: str, *, attach_vm: str) -> None: ... @abc.abstractmethod async def detach_http_proxy(self, name: str, *, attach_vm: str) -> None: ... + + +class TemplateCapability(abc.ABC): + """Mix-in declaring a VMProvider can materialize reusable templates. + + Modeled as an ABC so resolve_capability checks explicit provider support, + rather than accepting any object that happens to expose these method names. + """ + + @abc.abstractmethod + async def materialize_template( + self, + *, + base_image: str, + setup_script: str, + label: str, + ) -> str: ... + + @abc.abstractmethod + async def delete_template(self, handle: str) -> None: ... diff --git a/src/templates/__init__.py b/src/templates/__init__.py new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/src/templates/__init__.py @@ -0,0 +1 @@ + diff --git a/src/templates/api.py b/src/templates/api.py new file mode 100644 index 0000000..a3a3c1c --- /dev/null +++ b/src/templates/api.py @@ -0,0 +1,89 @@ +import logging +import uuid +from typing import Annotated + +from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, Response, status +from sqlalchemy.exc import SQLAlchemyError + +from hosts.auth import require_service_auth +from providers.exceptions import ProviderError, UnknownProviderError +from templates.deps import get_template_service +from templates.exceptions import TemplateTeardownError +from templates.models import Template +from templates.schemas import TemplateCreate, TemplateOut +from templates.service import TemplateService + +logger = logging.getLogger(__name__) + +router = APIRouter(prefix="/templates", tags=["templates"]) + +TemplateServiceDep = Annotated[TemplateService, Depends(get_template_service)] + + +@router.post( + "", + response_model=TemplateOut, + status_code=status.HTTP_202_ACCEPTED, + dependencies=[Depends(require_service_auth)], +) +async def create_template( + payload: TemplateCreate, + background_tasks: BackgroundTasks, + service: TemplateServiceDep, +) -> Template: + try: + template, created = await service.get_or_create( + provider=payload.provider, + base_image=payload.base_image, + setup_script=payload.setup_script, + label=payload.label, + ) + except UnknownProviderError as exc: + raise HTTPException(status_code=400, detail=str(exc)) from exc + except SQLAlchemyError as exc: + logger.exception("unexpected database error during template creation") + raise HTTPException( + status_code=503, + detail="template creation could not be completed", + ) from exc + + if created: + background_tasks.add_task(service.build, template.id) + return template + + +@router.get( + "", + response_model=list[TemplateOut], + dependencies=[Depends(require_service_auth)], +) +async def list_templates(service: TemplateServiceDep) -> list[Template]: + return await service.list() + + +@router.get( + "/{template_id}", + response_model=TemplateOut, + dependencies=[Depends(require_service_auth)], +) +async def get_template(template_id: uuid.UUID, service: TemplateServiceDep) -> Template: + if template := await service.get(template_id): + return template + raise HTTPException(status_code=404, detail="template not found") + + +@router.delete( + "/{template_id}", + status_code=status.HTTP_204_NO_CONTENT, + dependencies=[Depends(require_service_auth)], +) +async def delete_template(template_id: uuid.UUID, service: TemplateServiceDep) -> Response: + try: + await service.delete(template_id) + except ProviderError as exc: + logger.exception("unexpected error deleting template") + raise TemplateTeardownError("template teardown could not be completed") from exc + except SQLAlchemyError as exc: + logger.exception("unexpected database error during template teardown") + raise TemplateTeardownError("template teardown could not be completed") from exc + return Response(status_code=status.HTTP_204_NO_CONTENT) diff --git a/src/templates/deps.py b/src/templates/deps.py new file mode 100644 index 0000000..e9fc08e --- /dev/null +++ b/src/templates/deps.py @@ -0,0 +1,13 @@ +from typing import Annotated + +from fastapi import Depends +from sqlalchemy.ext.asyncio import AsyncSession + +from core.database import get_session +from templates.service import TemplateService + + +async def get_template_service( + session: Annotated[AsyncSession, Depends(get_session)], +) -> TemplateService: + return TemplateService(session) diff --git a/src/templates/exceptions.py b/src/templates/exceptions.py new file mode 100644 index 0000000..6bc40b0 --- /dev/null +++ b/src/templates/exceptions.py @@ -0,0 +1,11 @@ +from core.exceptions import AppException + + +class TemplateStateError(AppException): + status_code = 409 + error_code = "TEMPLATE_STATE" + + +class TemplateTeardownError(AppException): + status_code = 503 + error_code = "TEMPLATE_TEARDOWN" diff --git a/src/templates/models.py b/src/templates/models.py new file mode 100644 index 0000000..c64f933 --- /dev/null +++ b/src/templates/models.py @@ -0,0 +1,46 @@ +import uuid +from datetime import datetime +from enum import StrEnum + +from sqlalchemy import Index, String, Text, Uuid +from sqlalchemy.orm import Mapped, mapped_column +from uuid6 import uuid7 + +from core.database import Base +from hosts.models import UTCDateTime + + +class TemplateStatus(StrEnum): + BUILDING = "building" + AVAILABLE = "available" + FAILED = "failed" + + +class Template(Base): + __tablename__ = "templates" + __table_args__ = ( + Index( + "ix_templates_provider_base_image_requirements_hash", + "provider", + "base_image", + "requirements_hash", + unique=True, + ), + ) + + id: Mapped[uuid.UUID] = mapped_column(Uuid(as_uuid=True), primary_key=True, default=uuid7) + provider: Mapped[str] = mapped_column(String(20)) + base_image: Mapped[str] = mapped_column(Text) + requirements_hash: Mapped[str] = mapped_column(String(64)) + setup_script: Mapped[str] = mapped_column(Text) + label: Mapped[str] = mapped_column(Text, default="") + handle: Mapped[str] = mapped_column(Text, default="") + status: Mapped[str] = mapped_column(String(32), default=TemplateStatus.BUILDING.value) + last_error: Mapped[str] = mapped_column(Text, default="") + created_at: Mapped[datetime] = mapped_column(UTCDateTime) + updated_at: Mapped[datetime] = mapped_column(UTCDateTime) + last_used_at: Mapped[datetime | None] = mapped_column( + UTCDateTime, + nullable=True, + default=None, + ) diff --git a/src/templates/schemas.py b/src/templates/schemas.py new file mode 100644 index 0000000..480e42d --- /dev/null +++ b/src/templates/schemas.py @@ -0,0 +1,47 @@ +import uuid +from datetime import datetime + +from pydantic import BaseModel, ConfigDict, Field, field_validator + + +class TemplateCreate(BaseModel): + provider: str | None = Field( + default=None, + description="VM provider to materialize on. Omit to use the service default.", + ) + base_image: str | None = Field( + default=None, + description="Provider image to build from. Omit to use the provider default.", + ) + setup_script: str + label: str = "" + + @field_validator("base_image") + @classmethod + def reject_blank_base_image(cls, value: str | None) -> str | None: + if value is not None and not value.strip(): + raise ValueError("base_image must not be blank") + return value + + @field_validator("setup_script") + @classmethod + def reject_blank_setup_script(cls, value: str) -> str: + if not value.strip(): + raise ValueError("setup_script must not be blank") + return value + + +class TemplateOut(BaseModel): + model_config = ConfigDict(from_attributes=True) + + id: uuid.UUID + provider: str + base_image: str + requirements_hash: str + label: str + handle: str + status: str + last_error: str + created_at: datetime + updated_at: datetime + last_used_at: datetime | None diff --git a/src/templates/service.py b/src/templates/service.py new file mode 100644 index 0000000..45553ab --- /dev/null +++ b/src/templates/service.py @@ -0,0 +1,148 @@ +import hashlib +import logging +import uuid + +from sqlalchemy import select +from sqlalchemy.exc import IntegrityError +from sqlalchemy.ext.asyncio import AsyncSession + +from core.database import async_session_factory +from core.exceptions import ResourceNotFoundError +from hosts.service import utc_now +from providers.capabilities import TemplateCapability, resolve_capability +from providers.exceptions import ProviderNotFoundError, UnknownProviderError +from providers.registry import get_provider_names, get_vm_provider +from templates.exceptions import TemplateStateError +from templates.models import Template, TemplateStatus + +logger = logging.getLogger(__name__) + + +class TemplateService: + def __init__(self, session: AsyncSession) -> None: + self.session = session + + async def get_or_create( + self, + *, + provider: str | None, + base_image: str | None, + setup_script: str, + label: str, + ) -> tuple[Template, bool]: + if provider: + registered = get_provider_names() + if provider not in registered: + available = ", ".join(sorted(registered)) + raise UnknownProviderError(f"unknown provider {provider!r}; available: {available}") + + vm = get_vm_provider(provider) + resolved_base_image = base_image or vm.default_image + requirements_hash = hashlib.sha256(setup_script.encode("utf-8")).hexdigest() + now = utc_now() + template = Template( + provider=vm.name, + base_image=resolved_base_image, + requirements_hash=requirements_hash, + setup_script=setup_script, + label=label, + handle="", + status=TemplateStatus.BUILDING.value, + last_error="", + created_at=now, + updated_at=now, + ) + + async with async_session_factory() as create_session: + create_session.add(template) + try: + await create_session.commit() + except IntegrityError: + await create_session.rollback() + else: + await create_session.refresh(template) + return template, True + + winner = ( + await self.session.execute( + select(Template) + .where(Template.provider == vm.name) + .where(Template.base_image == resolved_base_image) + .where(Template.requirements_hash == requirements_hash) + ) + ).scalar_one_or_none() + if not winner: + raise TemplateStateError("template creation race could not be resolved") + return winner, False + + async def build(self, template_id: uuid.UUID) -> None: + async with async_session_factory() as session: + template = await session.get(Template, template_id) + if not template: + raise ResourceNotFoundError("template not found") + # Release the read transaction before a minutes-long provider build. + await session.commit() + + try: + capability = resolve_capability( + get_vm_provider(template.provider), + TemplateCapability, + ) + handle = await capability.materialize_template( + base_image=template.base_image, + setup_script=template.setup_script, + label=template.label, + ) + except Exception as exc: + logger.exception( + "template build failed: template_id=%s provider=%s", + template.id, + template.provider, + ) + template.status = TemplateStatus.FAILED.value + template.last_error = f"{type(exc).__name__}: {exc}" + else: + template.handle = handle + template.status = TemplateStatus.AVAILABLE.value + template.last_error = "" + + template.updated_at = utc_now() + await session.commit() + + async def get(self, template_id: uuid.UUID) -> Template | None: + return await self.session.get(Template, template_id) + + async def list(self) -> list[Template]: + result = await self.session.execute(select(Template).order_by(Template.created_at.desc())) + return list(result.scalars()) + + async def delete(self, template_id: uuid.UUID) -> None: + result = await self.session.execute( + select(Template).where(Template.id == template_id).with_for_update() + ) + template = result.scalar_one_or_none() + + if not template: + raise ResourceNotFoundError("template not found") + + if template.status == TemplateStatus.BUILDING.value: + raise TemplateStateError("template is still building") + + if template.handle: + capability = resolve_capability( + get_vm_provider(template.provider), + TemplateCapability, + ) + try: + await capability.delete_template(template.handle) + except ProviderNotFoundError: + logger.warning( + "template already absent at provider during teardown: " + "template_id=%s handle=%s provider=%s", + template.id, + template.handle, + template.provider, + ) + + await self.session.delete(template) + await self.session.commit() diff --git a/src/templates/tests/__init__.py b/src/templates/tests/__init__.py new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/src/templates/tests/__init__.py @@ -0,0 +1 @@ + diff --git a/src/templates/tests/conftest.py b/src/templates/tests/conftest.py new file mode 100644 index 0000000..953516b --- /dev/null +++ b/src/templates/tests/conftest.py @@ -0,0 +1,84 @@ +from collections.abc import Iterator + +import pytest + +from providers import registry as registry_module +from providers.base import VMCreateResult, VMProvider +from providers.capabilities import TemplateCapability +from providers.exceptions import ProviderError + + +class StubTemplateProvider(TemplateCapability, VMProvider): + name = "template-stub" + diagnose_hint = "check_template_stub" + + def __init__(self) -> None: + self.materialized: list[tuple[str, str, str]] = [] + self.deleted: list[str] = [] + self.build_error: Exception | None = None + self.delete_error: ProviderError | None = None + + @classmethod + def from_settings(cls) -> "StubTemplateProvider": + return cls() + + @property + def default_image(self) -> str: + return "stub:base" + + @property + def bootstrap_ssh_timeout_seconds(self) -> float: + return 0.1 + + async def create_vm( + self, + *, + name: str, + image: str, + env: dict[str, str] | None = None, + setup_script: str | None = None, + instance_type: str | None = None, + disk_gb: int | None = None, + ) -> VMCreateResult: + return VMCreateResult(provider_id=name, name=name, ssh_port=22, ssh_username="stub") + + async def delete_vm(self, name: str) -> None: + return + + async def diagnose(self) -> str: + return "stub ok" + + async def aclose(self) -> None: + return + + async def materialize_template( + self, + *, + base_image: str, + setup_script: str, + label: str, + ) -> str: + self.materialized.append((base_image, setup_script, label)) + if self.build_error: + raise self.build_error + return f"stub-template:{len(self.materialized)}" + + async def delete_template(self, handle: str) -> None: + self.deleted.append(handle) + if self.delete_error: + raise self.delete_error + + +@pytest.fixture +def template_provider() -> Iterator[StubTemplateProvider]: + factories = dict(registry_module._factories) + instances = dict(registry_module._instances) + provider = StubTemplateProvider() + registry_module._factories[provider.name] = lambda: provider + try: + yield provider + finally: + registry_module._factories.clear() + registry_module._factories.update(factories) + registry_module._instances.clear() + registry_module._instances.update(instances) diff --git a/src/templates/tests/test_api.py b/src/templates/tests/test_api.py new file mode 100644 index 0000000..cc02e35 --- /dev/null +++ b/src/templates/tests/test_api.py @@ -0,0 +1,353 @@ +import asyncio +import hashlib +import uuid +from datetime import UTC, datetime, timedelta +from unittest.mock import AsyncMock, MagicMock + +import pytest +from sqlalchemy import func, select +from sqlalchemy.exc import IntegrityError +from uuid6 import uuid7 + +from core.database import async_session_factory +from providers.exceptions import ProviderNotFoundError, ProviderTransportError +from templates.exceptions import TemplateStateError +from templates.models import Template, TemplateStatus +from templates.service import TemplateService + +AUTH_HEADERS = {"Authorization": "Bearer service-token"} +SETUP_SCRIPT = "apt-get update && apt-get install -y nodejs" + + +async def test_create_template_returns_building_then_materializes(client, template_provider): + """Create returns the pollable building record before the background build result.""" + response = await client.post( + "/templates", + headers=AUTH_HEADERS, + json={ + "provider": template_provider.name, + "setup_script": SETUP_SCRIPT, + "label": "Node tools", + }, + ) + + assert response.status_code == 202 + payload = response.json() + assert uuid.UUID(payload["id"]).version == 7 + assert payload["provider"] == template_provider.name + assert payload["base_image"] == template_provider.default_image + assert payload["requirements_hash"] == hashlib.sha256(SETUP_SCRIPT.encode()).hexdigest() + assert payload["label"] == "Node tools" + assert payload["handle"] == "" + assert payload["status"] == TemplateStatus.BUILDING.value + assert payload["last_error"] == "" + assert payload["last_used_at"] is None + assert "setup_script" not in payload + + polled = await client.get(f"/templates/{payload['id']}", headers=AUTH_HEADERS) + + assert polled.status_code == 200 + assert polled.json()["status"] == TemplateStatus.AVAILABLE.value + assert polled.json()["handle"] == "stub-template:1" + assert "setup_script" not in polled.json() + assert template_provider.materialized == [ + (template_provider.default_image, SETUP_SCRIPT, "Node tools") + ] + + +async def test_build_failure_is_pollable(client, template_provider): + """Provider build failures persist a typed diagnostic on the template record.""" + template_provider.build_error = ProviderTransportError("builder unavailable") + + response = await client.post( + "/templates", + headers=AUTH_HEADERS, + json={"provider": template_provider.name, "setup_script": SETUP_SCRIPT}, + ) + polled = await client.get(f"/templates/{response.json()['id']}", headers=AUTH_HEADERS) + + assert response.status_code == 202 + assert response.json()["status"] == TemplateStatus.BUILDING.value + assert polled.json()["status"] == TemplateStatus.FAILED.value + assert polled.json()["handle"] == "" + assert polled.json()["last_error"] == "ProviderTransportError: builder unavailable" + + +async def test_unexpected_build_crash_is_pollable(client, template_provider): + """Unexpected strategy crashes persist as failed template diagnostics.""" + template_provider.build_error = OSError("builder crashed") + + response = await client.post( + "/templates", + headers=AUTH_HEADERS, + json={"provider": template_provider.name, "setup_script": SETUP_SCRIPT}, + ) + polled = await client.get(f"/templates/{response.json()['id']}", headers=AUTH_HEADERS) + + assert response.status_code == 202 + assert polled.json()["status"] == TemplateStatus.FAILED.value + assert polled.json()["last_error"] == "OSError: builder crashed" + + +async def test_unsupported_capability_becomes_failed_build(client): + """A provider without template support reports failure through polling.""" + response = await client.post( + "/templates", + headers=AUTH_HEADERS, + json={"provider": "docker", "setup_script": SETUP_SCRIPT}, + ) + polled = await client.get(f"/templates/{response.json()['id']}", headers=AUTH_HEADERS) + + assert response.status_code == 202 + assert polled.json()["status"] == TemplateStatus.FAILED.value + assert polled.json()["last_error"].startswith("CapabilityUnsupportedError:") + assert "TemplateCapability" in polled.json()["last_error"] + + +async def test_duplicate_create_returns_existing_without_rebuilding(client, template_provider): + """The same provider, base, and script reuse one record and one build.""" + first = await client.post( + "/templates", + headers=AUTH_HEADERS, + json={ + "provider": template_provider.name, + "base_image": "stub:custom", + "setup_script": SETUP_SCRIPT, + "label": "first label", + }, + ) + second = await client.post( + "/templates", + headers=AUTH_HEADERS, + json={ + "provider": template_provider.name, + "base_image": "stub:custom", + "setup_script": SETUP_SCRIPT, + "label": "ignored label", + }, + ) + + assert first.status_code == 202 + assert second.status_code == 202 + assert second.json()["id"] == first.json()["id"] + assert second.json()["status"] == TemplateStatus.AVAILABLE.value + assert second.json()["label"] == "first label" + assert template_provider.materialized == [("stub:custom", SETUP_SCRIPT, "first label")] + + +async def test_concurrent_creates_resolve_unique_index_race(template_provider): + """Concurrent identical inserts converge on the unique-index winner.""" + async with ( + async_session_factory() as first_session, + async_session_factory() as second_session, + ): + first_service = TemplateService(first_session) + second_service = TemplateService(second_session) + results = await asyncio.gather( + first_service.get_or_create( + provider=template_provider.name, + base_image="stub:race", + setup_script=SETUP_SCRIPT, + label="first", + ), + second_service.get_or_create( + provider=template_provider.name, + base_image="stub:race", + setup_script=SETUP_SCRIPT, + label="second", + ), + ) + + assert results[0][0].id == results[1][0].id + assert sorted(created for _, created in results) == [False, True] + + async with async_session_factory() as session: + count = await session.scalar(select(func.count()).select_from(Template)) + assert count == 1 + + +async def test_create_race_without_winner_returns_typed_conflict(monkeypatch, template_provider): + """A vanished unique-index winner returns a retryable template state conflict.""" + create_session = AsyncMock() + create_session.add = MagicMock() + create_session.commit.side_effect = IntegrityError("insert", {}, Exception("unique")) + create_context = MagicMock() + create_context.__aenter__ = AsyncMock(return_value=create_session) + create_context.__aexit__ = AsyncMock(return_value=False) + create_session_factory = MagicMock(return_value=create_context) + result = MagicMock() + result.scalar_one_or_none.return_value = None + request_session = AsyncMock() + request_session.execute.return_value = result + monkeypatch.setattr("templates.service.async_session_factory", create_session_factory) + + with pytest.raises( + TemplateStateError, + match="template creation race could not be resolved", + ): + await TemplateService(request_session).get_or_create( + provider=template_provider.name, + base_image="stub:race", + setup_script=SETUP_SCRIPT, + label="race", + ) + + create_session.rollback.assert_awaited_once() + + +async def test_get_template_returns_not_found(client): + """A missing template returns the resource-specific 404 detail.""" + template_id = uuid.UUID("00000000-0000-0000-0000-000000000877") + + response = await client.get(f"/templates/{template_id}", headers=AUTH_HEADERS) + + assert response.status_code == 404 + assert response.json()["detail"] == "template not found" + + +async def test_list_templates_returns_newest_first(client, template_provider): + """List orders template records from newest to oldest without setup scripts.""" + now = datetime.now(UTC) + older = await create_template_record( + provider=template_provider.name, + status=TemplateStatus.FAILED.value, + created_at=now, + label="older", + ) + newer = await create_template_record( + provider=template_provider.name, + status=TemplateStatus.AVAILABLE.value, + created_at=now + timedelta(seconds=1), + base_image="stub:newer", + label="newer", + handle="stub-template:newer", + ) + + response = await client.get("/templates", headers=AUTH_HEADERS) + + assert response.status_code == 200 + assert [item["id"] for item in response.json()] == [str(newer.id), str(older.id)] + assert all("setup_script" not in item for item in response.json()) + + +async def test_delete_building_template_returns_conflict(client, template_provider): + """Deletion refuses a template whose provider build may still be running.""" + template = await create_template_record( + provider=template_provider.name, + status=TemplateStatus.BUILDING.value, + ) + + response = await client.delete(f"/templates/{template.id}", headers=AUTH_HEADERS) + + assert response.status_code == 409 + assert response.json() == { + "detail": "template is still building", + "error_code": "TEMPLATE_STATE", + } + assert template_provider.deleted == [] + + +async def test_delete_available_template_removes_provider_artifact(client, template_provider): + """Deleting an available template removes its artifact and database row.""" + template = await create_template_record( + provider=template_provider.name, + status=TemplateStatus.AVAILABLE.value, + handle="stub-template:available", + ) + + response = await client.delete(f"/templates/{template.id}", headers=AUTH_HEADERS) + + assert response.status_code == 204 + assert response.content == b"" + assert template_provider.deleted == ["stub-template:available"] + async with async_session_factory() as session: + assert await session.get(Template, template.id) is None + + +async def test_delete_tolerates_missing_provider_artifact(client, template_provider): + """An already-absent provider artifact does not strand the template row.""" + template_provider.delete_error = ProviderNotFoundError("already gone") + template = await create_template_record( + provider=template_provider.name, + status=TemplateStatus.AVAILABLE.value, + handle="stub-template:missing", + ) + + response = await client.delete(f"/templates/{template.id}", headers=AUTH_HEADERS) + + assert response.status_code == 204 + async with async_session_factory() as session: + assert await session.get(Template, template.id) is None + + +async def test_delete_provider_failure_preserves_record(client, template_provider): + """A provider teardown failure returns 503 and leaves the row retryable.""" + template_provider.delete_error = ProviderTransportError("provider unavailable") + template = await create_template_record( + provider=template_provider.name, + status=TemplateStatus.AVAILABLE.value, + handle="stub-template:retry", + ) + + response = await client.delete(f"/templates/{template.id}", headers=AUTH_HEADERS) + + assert response.status_code == 503 + assert response.json()["detail"] == "template teardown could not be completed" + async with async_session_factory() as session: + assert await session.get(Template, template.id) is not None + + +async def test_create_template_rejects_unknown_provider(client): + """An unregistered provider returns 400 with the available-provider context.""" + response = await client.post( + "/templates", + headers=AUTH_HEADERS, + json={"provider": "does-not-exist", "setup_script": SETUP_SCRIPT}, + ) + + assert response.status_code == 400 + assert "does-not-exist" in response.json()["detail"] + assert "available" in response.json()["detail"] + + +async def test_create_template_rejects_blank_script(client, template_provider): + """Whitespace-only setup scripts stop at the wire boundary.""" + response = await client.post( + "/templates", + headers=AUTH_HEADERS, + json={"provider": template_provider.name, "setup_script": " \n"}, + ) + + assert response.status_code == 422 + assert response.json()["detail"][0]["loc"] == ["body", "setup_script"] + assert template_provider.materialized == [] + + +async def create_template_record( + *, + provider: str, + status: str, + created_at: datetime | None = None, + base_image: str = "stub:base", + label: str = "", + handle: str = "", +) -> Template: + now = created_at or datetime.now(UTC) + template = Template( + id=uuid7(), + provider=provider, + base_image=base_image, + requirements_hash=hashlib.sha256(SETUP_SCRIPT.encode()).hexdigest(), + setup_script=SETUP_SCRIPT, + label=label, + handle=handle, + status=status, + last_error="", + created_at=now, + updated_at=now, + ) + async with async_session_factory() as session: + session.add(template) + await session.commit() + await session.refresh(template) + return template From 2d66fa839e78f294ec31110f5faa1741b984b58a Mon Sep 17 00:00:00 2001 From: Paulo Date: Mon, 24 Aug 2026 19:58:10 +0200 Subject: [PATCH 2/8] ENG-878 - Derived-image template strategy for exe, docker, docker-sbx MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The TemplateCapability input is (base_image, setup_script) — no live host: the strategy synthesizes FROM plus the script as a bake-time root step, builds with the docker CLI, and the resulting ref is the handle. docker keeps the local tag, docker-sbx builds into the daemon sandboxd reads templates from, and exe pushes to a registry exe.dev can pull (exe.dev has no snapshot verb — the image is its snapshot; registry credentials are EXE_TEMPLATE_REGISTRY/_USERNAME/_PASSWORD). Providers stay dumb: the handle rides host.image and create_vm is unchanged. Co-Authored-By: Claude Fable 5 --- docs/deploy.md | 3 + docs/security.md | 7 +- src/providers/derived_image.py | 63 ++++++++++ src/providers/docker/api.py | 45 +++++-- src/providers/docker/exceptions.py | 4 + src/providers/docker/provider.py | 20 +++- src/providers/docker/tests/test_api.py | 79 +++++++++++- src/providers/docker/tests/test_provider.py | 60 +++++++++- src/providers/docker_sbx/provider.py | 31 ++++- .../docker_sbx/tests/test_diagnose.py | 2 +- .../docker_sbx/tests/test_provider.py | 81 +++++++++++-- src/providers/exe/provider.py | 64 ++++++++-- src/providers/exe/settings.py | 12 ++ src/providers/exe/tests/test_diagnose.py | 4 +- src/providers/exe/tests/test_provider.py | 112 +++++++++++++++++- src/providers/tests/test_derived_image.py | 24 ++++ src/templates/tests/conftest.py | 3 +- src/templates/tests/test_api.py | 18 ++- 18 files changed, 584 insertions(+), 48 deletions(-) create mode 100644 src/providers/derived_image.py create mode 100644 src/providers/tests/test_derived_image.py diff --git a/docs/deploy.md b/docs/deploy.md index d695876..d9aced9 100644 --- a/docs/deploy.md +++ b/docs/deploy.md @@ -319,6 +319,9 @@ exe.dev provider: | --- | --- | --- | | `EXE_API_TOKEN` | — (required) | Bearer token for the exe.dev exec API. | | `EXE_DEFAULT_IMAGE` | — (required) | Image used when the caller omits `image`. | +| `EXE_TEMPLATE_REGISTRY` | — | Repository prefix for derived template images that exe.dev can pull. | +| `EXE_REGISTRY_USERNAME` | — | Username for the derived-template image registry. | +| `EXE_REGISTRY_PASSWORD` | — | Password or token for the derived-template image registry. | | `EXE_API_URL` | `https://exe.dev` | API base URL. | | `EXE_API_TIMEOUT` | `30.0` | Timeout for exe.dev API calls. | | `EXE_BOOTSTRAP_SSH_TIMEOUT_SECONDS` | `30.0` | ssh-keyscan retry budget for a fresh exe.dev sandbox. | diff --git a/docs/security.md b/docs/security.md index c97a045..274797e 100644 --- a/docs/security.md +++ b/docs/security.md @@ -76,9 +76,10 @@ covered in [Networking](networking.md). The security-relevant summary: ## Secrets and in-VM metadata -Provider tokens (`EXE_API_TOKEN`, `HETZNER_API_TOKEN`, Tailscale OAuth) -and AWS credentials are read from the environment / the AWS SDK default -chain and never written to the database or returned by the API. Caller +Provider tokens (`EXE_API_TOKEN`, `EXE_REGISTRY_PASSWORD`, +`HETZNER_API_TOKEN`, Tailscale OAuth) and AWS credentials are read from +the environment / the AWS SDK default chain and never written to the +database or returned by the API. Caller `env` is write-only: it is delivered to the VM but never echoed in any response, and reserved keys (`TAILSCALE_AUTHKEY`) are rejected at the schema. diff --git a/src/providers/derived_image.py b/src/providers/derived_image.py new file mode 100644 index 0000000..78ee736 --- /dev/null +++ b/src/providers/derived_image.py @@ -0,0 +1,63 @@ +import contextlib +import hashlib +import tempfile +from collections.abc import Iterator +from pathlib import Path + +from providers.docker.api import DockerCLI +from providers.docker.exceptions import DockerImageNotFoundError, DockerProviderError +from providers.exceptions import ProviderNotFoundError, ProviderTransportError + + +def derived_image_tag( + *, + base_image: str, + setup_script: str, + repository: str = "drukbox-template", +) -> str: + identity = base_image.encode("utf-8") + b"\0" + setup_script.encode("utf-8") + digest = hashlib.sha256(identity).hexdigest()[:12] + return f"{repository}:{digest}" + + +@contextlib.contextmanager +def derived_image_context(*, base_image: str, setup_script: str) -> Iterator[Path]: + with tempfile.TemporaryDirectory(prefix="drukbox-template-") as directory: + context = Path(directory) + context.joinpath("setup.sh").write_bytes(setup_script.encode("utf-8")) + context.joinpath("Dockerfile").write_text( + f"FROM {base_image}\n" + "COPY setup.sh /drukbox-setup.sh\n" + "RUN sh /drukbox-setup.sh && rm /drukbox-setup.sh\n", + encoding="utf-8", + ) + yield context + + +async def build_derived_image( + docker: DockerCLI, + *, + base_image: str, + setup_script: str, + repository: str = "drukbox-template", +) -> str: + tag = derived_image_tag( + base_image=base_image, + setup_script=setup_script, + repository=repository, + ) + try: + with derived_image_context(base_image=base_image, setup_script=setup_script) as context: + await docker.build_image(tag, context) + except DockerProviderError as exc: + raise ProviderTransportError(str(exc)) from exc + return tag + + +async def remove_derived_image(docker: DockerCLI, handle: str) -> None: + try: + await docker.remove_image(handle) + except DockerImageNotFoundError as exc: + raise ProviderNotFoundError(f"docker image '{handle}' was not found") from exc + except DockerProviderError as exc: + raise ProviderTransportError(str(exc)) from exc diff --git a/src/providers/docker/api.py b/src/providers/docker/api.py index 59e65df..5c65e06 100644 --- a/src/providers/docker/api.py +++ b/src/providers/docker/api.py @@ -1,8 +1,11 @@ import asyncio import os import tempfile +from pathlib import Path -from .exceptions import DockerTransportError, DockerVMNotFoundError +from .exceptions import DockerImageNotFoundError, DockerTransportError, DockerVMNotFoundError + +_MAX_ERROR_DETAIL_CHARS = 8_000 class DockerCLI: @@ -29,7 +32,7 @@ async def run_container( for key, value in labels.items(): args.extend(["--label", f"{key}={value}"]) env_file = _write_env_file(env) if env else None - if env_file is not None: + if env_file: # --env-file keeps caller secrets off argv (world-readable via ps/proc # for the lifetime of `docker run`); only the path is passed, and the # file is removed in the finally once docker has read it. @@ -38,7 +41,7 @@ async def run_container( try: return (await self._run(*args)).strip() finally: - if env_file is not None: + if env_file: os.unlink(env_file) async def published_ssh_port(self, name: str) -> int: @@ -60,14 +63,34 @@ async def published_ssh_port(self, name: str) -> int: async def remove_container(self, name: str) -> None: await self._run("rm", "--force", "--volumes", name) + async def build_image(self, tag: str, context_dir: Path) -> None: + await self._run("build", "--tag", tag, str(context_dir)) + + async def remove_image(self, tag: str) -> None: + await self._run("image", "rm", tag) + + async def push_image(self, tag: str) -> None: + await self._run("push", tag) + + async def login(self, registry: str, username: str, password: str) -> None: + await self._run( + "login", + registry, + "--username", + username, + "--password-stdin", + stdin=f"{password}\n".encode(), + ) + async def server_version(self) -> str: return (await self._run("version", "--format", "{{.Server.Version}}")).strip() - async def _run(self, *args: str) -> str: + async def _run(self, *args: str, stdin: bytes | None = None) -> str: try: process = await asyncio.create_subprocess_exec( "docker", *args, + stdin=asyncio.subprocess.PIPE if stdin else asyncio.subprocess.DEVNULL, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, ) @@ -76,13 +99,21 @@ async def _run(self, *args: str) -> str: # (PermissionError) — translate both so a launch failure can't # escape the provider boundary as a raw OSError. raise DockerTransportError(f"docker CLI could not be started: {error}") from error - stdout, stderr = await process.communicate() + stdout, stderr = await process.communicate(stdin) if process.returncode != 0: - detail = stderr.decode().strip() or f"docker {args[0]} exited {process.returncode}" + detail = "\n".join( + output.decode(errors="replace") for output in (stdout, stderr) if output + ).strip() + if detail: + detail = detail[-_MAX_ERROR_DETAIL_CHARS:] + else: + detail = f"docker {args[0]} exited {process.returncode}" if "no such container" in detail.lower(): raise DockerVMNotFoundError(detail) + if "no such image" in detail.lower(): + raise DockerImageNotFoundError(detail) raise DockerTransportError(detail) - return stdout.decode() + return stdout.decode(errors="replace") def _write_env_file(env: dict[str, str]) -> str: diff --git a/src/providers/docker/exceptions.py b/src/providers/docker/exceptions.py index b3e1fb4..89dd985 100644 --- a/src/providers/docker/exceptions.py +++ b/src/providers/docker/exceptions.py @@ -6,5 +6,9 @@ class DockerVMNotFoundError(DockerProviderError): """No container matched the lookup.""" +class DockerImageNotFoundError(DockerProviderError): + """No image matched the lookup.""" + + class DockerTransportError(DockerProviderError): """Invoking the docker CLI failed for transport reasons.""" diff --git a/src/providers/docker/provider.py b/src/providers/docker/provider.py index af1acac..1fb2e58 100644 --- a/src/providers/docker/provider.py +++ b/src/providers/docker/provider.py @@ -3,6 +3,8 @@ from core.settings import get_settings from providers.base import VMCreateResult, VMProvider +from providers.capabilities import TemplateCapability +from providers.derived_image import build_derived_image, remove_derived_image from providers.exceptions import ( ProviderCommandError, ProviderNotFoundError, @@ -19,7 +21,7 @@ _RESERVED_ENV_KEYS = frozenset({_AUTHORIZED_KEY_ENV, _ENV_KEYS_ENV}) -class DockerProvider(VMProvider): +class DockerProvider(VMProvider, TemplateCapability): name: ClassVar[str] = "docker" diagnose_hint: ClassVar[str] = "check_docker_daemon_is_running" # A local container has no path onto the tailnet; its hosts keep the @@ -127,6 +129,22 @@ async def delete_vm(self, name: str) -> None: except DockerProviderError as exc: raise ProviderTransportError(str(exc)) from exc + async def materialize_template( + self, + *, + base_image: str, + setup_script: str, + label: str, + ) -> str: + return await build_derived_image( + self.api, + base_image=base_image, + setup_script=setup_script, + ) + + async def delete_template(self, handle: str) -> None: + await remove_derived_image(self.api, handle) + async def diagnose(self) -> str: return f"docker server {await self.api.server_version()}" diff --git a/src/providers/docker/tests/test_api.py b/src/providers/docker/tests/test_api.py index 357fef4..fb28943 100644 --- a/src/providers/docker/tests/test_api.py +++ b/src/providers/docker/tests/test_api.py @@ -1,3 +1,4 @@ +import asyncio import os from types import SimpleNamespace from unittest.mock import AsyncMock @@ -5,7 +6,11 @@ import pytest from providers.docker.api import DockerCLI -from providers.docker.exceptions import DockerTransportError, DockerVMNotFoundError +from providers.docker.exceptions import ( + DockerImageNotFoundError, + DockerTransportError, + DockerVMNotFoundError, +) def _process(*, returncode: int = 0, stdout: bytes = b"", stderr: bytes = b"") -> SimpleNamespace: @@ -110,6 +115,78 @@ async def test_missing_container_maps_to_not_found(monkeypatch): await DockerCLI().remove_container("sb-test") +@pytest.mark.asyncio +async def test_build_image_uses_the_given_tag_and_context(monkeypatch, tmp_path): + create = AsyncMock(return_value=_process()) + monkeypatch.setattr("providers.docker.api.asyncio.create_subprocess_exec", create) + + await DockerCLI().build_image("drukbox-template:123456789abc", tmp_path) + + assert create.await_args + assert create.await_args.args == ( + "docker", + "build", + "--tag", + "drukbox-template:123456789abc", + str(tmp_path), + ) + + +@pytest.mark.asyncio +async def test_missing_image_maps_to_not_found(monkeypatch): + create = AsyncMock( + return_value=_process( + returncode=1, + stderr=b"Error response from daemon: No such image: drukbox-template:missing", + ) + ) + monkeypatch.setattr("providers.docker.api.asyncio.create_subprocess_exec", create) + + with pytest.raises(DockerImageNotFoundError): + await DockerCLI().remove_image("drukbox-template:missing") + + +@pytest.mark.asyncio +async def test_login_passes_the_password_only_over_stdin(monkeypatch): + process = _process() + captured: dict = {} + + async def fake_exec(*args, **kwargs): + captured["args"] = args + captured["stdin"] = kwargs["stdin"] + return process + + monkeypatch.setattr("providers.docker.api.asyncio.create_subprocess_exec", fake_exec) + + await DockerCLI().login("ghcr.io", "builder", "registry-secret") + + assert captured["args"] == ( + "docker", + "login", + "ghcr.io", + "--username", + "builder", + "--password-stdin", + ) + assert all("registry-secret" not in arg for arg in captured["args"]) + assert captured["stdin"] == asyncio.subprocess.PIPE + process.communicate.assert_awaited_once_with(b"registry-secret\n") + + +@pytest.mark.asyncio +async def test_failure_detail_keeps_the_log_tail_and_caps_its_size(monkeypatch): + stderr = f"leading-secret\n{'x' * 8_100}\nbuild-tail".encode() + create = AsyncMock(return_value=_process(returncode=1, stderr=stderr)) + monkeypatch.setattr("providers.docker.api.asyncio.create_subprocess_exec", create) + + with pytest.raises(DockerTransportError) as error: + await DockerCLI().push_image("registry/template:tag") + + assert len(str(error.value)) == 8_000 + assert str(error.value).endswith("build-tail") + assert "leading-secret" not in str(error.value) + + @pytest.mark.asyncio async def test_other_failure_maps_to_transport_error(monkeypatch): create = AsyncMock(return_value=_process(returncode=1, stderr=b"Cannot connect to daemon")) diff --git a/src/providers/docker/tests/test_provider.py b/src/providers/docker/tests/test_provider.py index 128d6f6..c762e8a 100644 --- a/src/providers/docker/tests/test_provider.py +++ b/src/providers/docker/tests/test_provider.py @@ -3,7 +3,11 @@ import pytest -from providers.docker.exceptions import DockerTransportError, DockerVMNotFoundError +from providers.docker.exceptions import ( + DockerImageNotFoundError, + DockerTransportError, + DockerVMNotFoundError, +) from providers.docker.provider import DockerProvider from providers.docker.settings import DockerSettings from providers.exceptions import ( @@ -22,6 +26,8 @@ def _api_mock() -> MagicMock: api.run_container = AsyncMock(return_value="container-id") api.published_ssh_port = AsyncMock(return_value=49160) api.remove_container = AsyncMock() + api.build_image = AsyncMock() + api.remove_image = AsyncMock() api.server_version = AsyncMock(return_value="27.0.3") return api @@ -43,7 +49,7 @@ async def test_create_vm_runs_container_and_returns_loopback_coords(): assert result.ssh_host == "127.0.0.1" assert result.ssh_port == 49160 assert result.ssh_username == "root" - assert result.private_key is not None + assert result.private_key assert "-----BEGIN OPENSSH PRIVATE KEY-----" in result.private_key @@ -129,6 +135,56 @@ async def test_delete_vm_raises_not_found_when_container_missing(): await provider.delete_vm("sb-test") +@pytest.mark.asyncio +async def test_materialize_template_builds_and_returns_the_derived_tag(): + api = _api_mock() + provider = DockerProvider(api, _settings()) + + handle = await provider.materialize_template( + base_image="sandbox:base", + setup_script="apt-get update", + label="Node tools", + ) + + assert handle.startswith("drukbox-template:") + assert len(handle.removeprefix("drukbox-template:")) == 12 + assert api.build_image.await_args.args[0] == handle + + +@pytest.mark.asyncio +async def test_materialize_template_translates_build_failure(): + api = _api_mock() + api.build_image.side_effect = DockerTransportError("build log tail") + provider = DockerProvider(api, _settings()) + + with pytest.raises(ProviderTransportError, match="build log tail"): + await provider.materialize_template( + base_image="sandbox:base", + setup_script="apt-get update", + label="Node tools", + ) + + +@pytest.mark.asyncio +async def test_delete_template_removes_the_local_image(): + api = _api_mock() + provider = DockerProvider(api, _settings()) + + await provider.delete_template("drukbox-template:123456789abc") + + api.remove_image.assert_awaited_once_with("drukbox-template:123456789abc") + + +@pytest.mark.asyncio +async def test_delete_template_translates_a_missing_image(): + api = _api_mock() + api.remove_image.side_effect = DockerImageNotFoundError("No such image") + provider = DockerProvider(api, _settings()) + + with pytest.raises(ProviderNotFoundError, match="was not found"): + await provider.delete_template("drukbox-template:missing") + + @pytest.mark.asyncio async def test_diagnose_returns_server_version(): api = _api_mock() diff --git a/src/providers/docker_sbx/provider.py b/src/providers/docker_sbx/provider.py index 7a553f0..f417206 100644 --- a/src/providers/docker_sbx/provider.py +++ b/src/providers/docker_sbx/provider.py @@ -5,6 +5,9 @@ from typing import ClassVar, Self from providers.base import VMCreateResult, VMProvider +from providers.capabilities import TemplateCapability +from providers.derived_image import build_derived_image, remove_derived_image +from providers.docker.api import DockerCLI from providers.exceptions import ( ProviderCommandError, ProviderNotFoundError, @@ -42,7 +45,7 @@ def _bootstrap_script(*, public_key: str, env: dict[str, str], ssh_username: str return "\n".join(lines) + "\n" -class DockerSbxProvider(VMProvider): +class DockerSbxProvider(VMProvider, TemplateCapability): name: ClassVar[str] = "docker-sbx" diagnose_hint: ClassVar[str] = "check_sandboxd_is_running_and_logged_in" # Sandboxes have no dialable sshd; the gateway serves them, and there is @@ -54,15 +57,23 @@ class DockerSbxProvider(VMProvider): # healthy daemon. diagnose_timeout_seconds: ClassVar[float] = 15.0 - def __init__(self, api: SbxCLI, settings: DockerSbxSettings) -> None: + def __init__( + self, + api: SbxCLI, + settings: DockerSbxSettings, + *, + docker: DockerCLI, + ) -> None: self.api = api self.settings = settings + self.docker = docker @classmethod def from_settings(cls) -> Self: return cls( SbxCLI(), DockerSbxSettings(), # pyright: ignore[reportCallIssue] + docker=DockerCLI(), ) @property @@ -168,6 +179,22 @@ async def delete_vm(self, name: str) -> None: self._remove_workspace(name) + async def materialize_template( + self, + *, + base_image: str, + setup_script: str, + label: str, + ) -> str: + return await build_derived_image( + self.docker, + base_image=base_image, + setup_script=setup_script, + ) + + async def delete_template(self, handle: str) -> None: + await remove_derived_image(self.docker, handle) + async def diagnose(self) -> str: # The sandbox list is one fast check of the CLI, the daemon # connection, and the Docker login. diff --git a/src/providers/docker_sbx/tests/test_diagnose.py b/src/providers/docker_sbx/tests/test_diagnose.py index 7ad772c..9228d32 100644 --- a/src/providers/docker_sbx/tests/test_diagnose.py +++ b/src/providers/docker_sbx/tests/test_diagnose.py @@ -8,7 +8,7 @@ def _provider(api: MagicMock) -> DockerSbxProvider: - return DockerSbxProvider(api, DockerSbxSettings()) + return DockerSbxProvider(api, DockerSbxSettings(), docker=MagicMock()) @pytest.mark.asyncio diff --git a/src/providers/docker_sbx/tests/test_provider.py b/src/providers/docker_sbx/tests/test_provider.py index f2a35f6..9fdf8ac 100644 --- a/src/providers/docker_sbx/tests/test_provider.py +++ b/src/providers/docker_sbx/tests/test_provider.py @@ -4,6 +4,7 @@ import pytest +from providers.docker.exceptions import DockerImageNotFoundError from providers.docker_sbx.exceptions import ( DockerSbxNotFoundError, DockerSbxTransportError, @@ -30,10 +31,26 @@ def _api_mock() -> MagicMock: return api +def _docker_mock() -> MagicMock: + docker = MagicMock() + docker.build_image = AsyncMock() + docker.remove_image = AsyncMock() + return docker + + +def _provider( + api: MagicMock, + settings: DockerSbxSettings, + *, + docker: MagicMock | None = None, +) -> DockerSbxProvider: + return DockerSbxProvider(api, settings, docker=docker or _docker_mock()) + + @pytest.mark.asyncio async def test_create_vm_creates_a_sized_sandbox_and_returns_key_material(tmp_path): api = _api_mock() - provider = DockerSbxProvider(api, _settings(tmp_path, cpus=4, memory="8g")) + provider = _provider(api, _settings(tmp_path, cpus=4, memory="8g")) result = await provider.create_vm(name="sb-test", image="drukbox/sbx-sandbox:latest", env={}) @@ -52,16 +69,16 @@ async def test_create_vm_creates_a_sized_sandbox_and_returns_key_material(tmp_pa assert result.ssh_host == "" assert result.ssh_port == 0 assert result.ssh_username == "root" - assert result.private_key is not None + assert result.private_key assert "-----BEGIN OPENSSH PRIVATE KEY-----" in result.private_key - assert result.public_key is not None + assert result.public_key assert result.public_key.startswith("ssh-ed25519 ") @pytest.mark.asyncio async def test_create_vm_bootstrap_installs_the_public_key_and_caller_env(tmp_path): api = _api_mock() - provider = DockerSbxProvider(api, _settings(tmp_path)) + provider = _provider(api, _settings(tmp_path)) await provider.create_vm(name="sb-test", image="img", env={"API_TOKEN": "s3cr3t"}) @@ -77,7 +94,7 @@ async def test_create_vm_bootstrap_installs_the_public_key_and_caller_env(tmp_pa @pytest.mark.asyncio async def test_create_vm_installs_the_key_for_the_configured_ssh_user(tmp_path): api = _api_mock() - provider = DockerSbxProvider(api, _settings(tmp_path, ssh_username="dev")) + provider = _provider(api, _settings(tmp_path, ssh_username="dev")) result = await provider.create_vm(name="sb-test", image="img", env={}) @@ -91,7 +108,7 @@ async def test_create_vm_installs_the_key_for_the_configured_ssh_user(tmp_path): @pytest.mark.asyncio async def test_create_vm_rejects_setup_script_because_tailscale_is_unsupported(tmp_path): api = _api_mock() - provider = DockerSbxProvider(api, _settings(tmp_path)) + provider = _provider(api, _settings(tmp_path)) # The service does not send a script, because supports_tailnet is # False. This guard finds a defect in the caller. @@ -103,7 +120,7 @@ async def test_create_vm_rejects_setup_script_because_tailscale_is_unsupported(t @pytest.mark.asyncio async def test_create_vm_rejects_env_values_that_would_forge_environment_entries(tmp_path): api = _api_mock() - provider = DockerSbxProvider(api, _settings(tmp_path)) + provider = _provider(api, _settings(tmp_path)) with pytest.raises(ProviderCommandError, match="NUL or newline"): await provider.create_vm(name="sb-test", image="img", env={"EVIL": "value\nINJECTED=x"}) @@ -115,7 +132,7 @@ async def test_create_vm_rejects_env_values_that_would_forge_environment_entries async def test_create_vm_cleans_up_when_the_sandbox_cannot_be_created(tmp_path): api = _api_mock() api.create_sandbox.side_effect = DockerSbxTransportError("daemon unavailable") - provider = DockerSbxProvider(api, _settings(tmp_path)) + provider = _provider(api, _settings(tmp_path)) with pytest.raises(ProviderTransportError): await provider.create_vm(name="sb-test", image="img", env={}) @@ -132,7 +149,7 @@ async def test_create_vm_translates_an_unwritable_workspace_root(tmp_path): # the same OSError type as for a missing bind mount. blocked_root = tmp_path / "blocked" blocked_root.touch() - provider = DockerSbxProvider(api, _settings(blocked_root)) + provider = _provider(api, _settings(blocked_root)) with pytest.raises(ProviderTransportError, match="workspace"): await provider.create_vm(name="sb-test", image="img", env={}) @@ -146,7 +163,7 @@ async def test_create_vm_tears_down_after_a_failed_bootstrap_and_keeps_the_first api = _api_mock() api.run_bootstrap.side_effect = DockerSbxTransportError("exec failed") api.remove_sandbox.side_effect = DockerSbxTransportError("cleanup failed") - provider = DockerSbxProvider(api, _settings(tmp_path)) + provider = _provider(api, _settings(tmp_path)) with pytest.raises(ProviderTransportError, match="exec failed"): await provider.create_vm(name="sb-test", image="img", env={}) @@ -159,7 +176,7 @@ async def test_delete_vm_removes_the_sandbox_and_its_workspace(tmp_path): api = _api_mock() workspace = tmp_path / "sb-test" workspace.mkdir(parents=True) - provider = DockerSbxProvider(api, _settings(tmp_path)) + provider = _provider(api, _settings(tmp_path)) await provider.delete_vm("sb-test") @@ -173,7 +190,7 @@ async def test_delete_vm_drops_the_workspace_of_a_sandbox_that_never_existed(tmp api.remove_sandbox.side_effect = DockerSbxNotFoundError("sandbox 'sb-test' not found") workspace = tmp_path / "sb-test" workspace.mkdir(parents=True) - provider = DockerSbxProvider(api, _settings(tmp_path)) + provider = _provider(api, _settings(tmp_path)) with pytest.raises(ProviderNotFoundError): await provider.delete_vm("sb-test") @@ -188,8 +205,46 @@ async def test_delete_vm_keeps_the_workspace_when_teardown_fails(tmp_path): api.remove_sandbox.side_effect = DockerSbxTransportError("daemon unavailable") workspace = tmp_path / "sb-test" workspace.mkdir(parents=True) - provider = DockerSbxProvider(api, _settings(tmp_path)) + provider = _provider(api, _settings(tmp_path)) with pytest.raises(ProviderTransportError): await provider.delete_vm("sb-test") assert workspace.is_dir() + + +@pytest.mark.asyncio +async def test_materialize_template_builds_and_returns_the_local_image_tag(tmp_path): + api = _api_mock() + docker = _docker_mock() + provider = _provider(api, _settings(tmp_path), docker=docker) + + handle = await provider.materialize_template( + base_image="sandbox:base", + setup_script="apt-get update", + label="Node tools", + ) + + assert handle.startswith("drukbox-template:") + assert docker.build_image.await_args.args[0] == handle + + +@pytest.mark.asyncio +async def test_delete_template_removes_the_local_image(tmp_path): + api = _api_mock() + docker = _docker_mock() + provider = _provider(api, _settings(tmp_path), docker=docker) + + await provider.delete_template("drukbox-template:123456789abc") + + docker.remove_image.assert_awaited_once_with("drukbox-template:123456789abc") + + +@pytest.mark.asyncio +async def test_delete_template_translates_a_missing_image(tmp_path): + api = _api_mock() + docker = _docker_mock() + docker.remove_image.side_effect = DockerImageNotFoundError("No such image") + provider = _provider(api, _settings(tmp_path), docker=docker) + + with pytest.raises(ProviderNotFoundError, match="was not found"): + await provider.delete_template("drukbox-template:missing") diff --git a/src/providers/exe/provider.py b/src/providers/exe/provider.py index 38d753c..aad92de 100644 --- a/src/providers/exe/provider.py +++ b/src/providers/exe/provider.py @@ -2,12 +2,17 @@ from core.settings import get_settings from providers.base import VMCreateResult, VMProvider -from providers.capabilities import HttpProxyCapability +from providers.capabilities import HttpProxyCapability, TemplateCapability +from providers.derived_image import build_derived_image, remove_derived_image +from providers.docker.api import DockerCLI +from providers.docker.exceptions import DockerProviderError from providers.exceptions import ( + ProviderCommandError, ProviderHttpProxyExistsError, ProviderHttpProxyNotFoundError, ProviderNotFoundError, ProviderTargetVMNotFoundError, + ProviderTransportError, ) from providers.exe.api import ExeAPI from providers.exe.exceptions import ( @@ -18,7 +23,7 @@ from providers.exe.settings import ExeSettings -class ExeProvider(VMProvider, HttpProxyCapability): +class ExeProvider(VMProvider, HttpProxyCapability, TemplateCapability): name: ClassVar[str] = "exe" diagnose_hint: ClassVar[str] = "check_exe_dev_api_token_and_url" @@ -27,10 +32,12 @@ def __init__( api: ExeAPI, settings: ExeSettings, *, + docker: DockerCLI, service_label: str = "drukbox", ) -> None: self.api = api self.settings = settings + self.docker = docker self._service_label = service_label @classmethod @@ -39,6 +46,7 @@ def from_settings(cls) -> Self: return cls( ExeAPI.from_settings(), ExeSettings(), # pyright: ignore[reportCallIssue] + docker=DockerCLI(), service_label=core.service_label, ) @@ -60,12 +68,13 @@ async def create_vm( instance_type: str | None = None, disk_gb: int | None = None, ) -> VMCreateResult: + # Tags are operator-facing: `exe ls --tag=managed-by-` shows what this deployment owns. payload = await self.api.create_vm( name=name, image=image, env=env, setup_script=setup_script, - tags=self._tags_for(name), + tags=[f"managed-by-{self._service_label}"], ) return VMCreateResult( provider_id=str(payload["vm_name"]), @@ -78,17 +87,56 @@ async def create_vm( ssh_username=self.settings.ssh_username, ) - def _tags_for(self, name: str) -> list[str]: - # Tags are an operator-facing convenience: `exe ls --tag=managed-by-` - # answers "what VMs does this deployment own?" - return [f"managed-by-{self._service_label}"] - async def delete_vm(self, name: str) -> None: try: await self.api.delete_vm(name) except ExeVMNotFoundError as exc: raise ProviderNotFoundError(str(exc)) from exc + async def materialize_template( + self, + *, + base_image: str, + setup_script: str, + label: str, + ) -> str: + registry = self.settings.template_registry + username = self.settings.registry_username + password = self.settings.registry_password + + if not (registry and username and password): + missing_settings = [ + name + for name, value in ( + ("EXE_TEMPLATE_REGISTRY", registry), + ("EXE_REGISTRY_USERNAME", username), + ("EXE_REGISTRY_PASSWORD", password), + ) + if not value + ] + raise ProviderCommandError( + f"exe template registry is not configured; missing settings: " + f"{', '.join(missing_settings)}" + ) + + tag = await build_derived_image( + self.docker, + base_image=base_image, + setup_script=setup_script, + repository=registry, + ) + registry_host, *_ = registry.partition("/") + try: + await self.docker.login(registry_host, username, password) + await self.docker.push_image(tag) + except DockerProviderError as exc: + raise ProviderTransportError(str(exc)) from exc + return tag + + async def delete_template(self, handle: str) -> None: + # Registry deletion is registry-specific; this provider only owns the local build tag. + await remove_derived_image(self.docker, handle) + async def aclose(self) -> None: await self.api.aclose() diff --git a/src/providers/exe/settings.py b/src/providers/exe/settings.py index 7572654..e089e2c 100644 --- a/src/providers/exe/settings.py +++ b/src/providers/exe/settings.py @@ -22,6 +22,18 @@ class ExeSettings(BaseSettings): default_image: str = Field( description="Default VM image passed to exe.dev when provisioning.", ) + template_registry: str | None = Field( + default=None, + description="Repository prefix for derived template images that exe.dev can pull.", + ) + registry_username: str | None = Field( + default=None, + description="Username for the derived-template image registry.", + ) + registry_password: str | None = Field( + default=None, + description="Password or token for the derived-template image registry.", + ) api_timeout: float = Field( default=30.0, description="Timeout in seconds for exe.dev API calls.", diff --git a/src/providers/exe/tests/test_diagnose.py b/src/providers/exe/tests/test_diagnose.py index ffe8433..e7dbf20 100644 --- a/src/providers/exe/tests/test_diagnose.py +++ b/src/providers/exe/tests/test_diagnose.py @@ -15,7 +15,7 @@ async def test_diagnose_returns_email_from_whoami() -> None: """The probe surfaces the authenticated identity directly from whoami.""" api = MagicMock() api.whoami = AsyncMock(return_value={"email": "ops@example.com"}) - provider = ExeProvider(api, _settings()) + provider = ExeProvider(api, _settings(), docker=MagicMock()) assert await provider.diagnose() == "ops@example.com" @@ -25,7 +25,7 @@ async def test_diagnose_raises_on_whoami_failure() -> None: """A whoami error surfaces so the orchestrator can classify it.""" api = MagicMock() api.whoami = AsyncMock(side_effect=RuntimeError("403")) - provider = ExeProvider(api, _settings()) + provider = ExeProvider(api, _settings(), docker=MagicMock()) with pytest.raises(RuntimeError, match="403"): await provider.diagnose() diff --git a/src/providers/exe/tests/test_provider.py b/src/providers/exe/tests/test_provider.py index 29c20fe..a83bfea 100644 --- a/src/providers/exe/tests/test_provider.py +++ b/src/providers/exe/tests/test_provider.py @@ -4,7 +4,11 @@ import pytest +from providers.docker.api import DockerCLI +from providers.docker.exceptions import DockerImageNotFoundError, DockerTransportError +from providers.exceptions import ProviderCommandError, ProviderNotFoundError, ProviderTransportError from providers.exe.api import ExeAPI +from providers.exe.exceptions import ExeVMNotFoundError from providers.exe.provider import ExeProvider from providers.exe.settings import ExeSettings @@ -14,8 +18,17 @@ def _settings(**overrides: Any) -> ExeSettings: return ExeSettings(**{**defaults, **overrides}) +def _docker_mock() -> SimpleNamespace: + return SimpleNamespace( + build_image=AsyncMock(), + remove_image=AsyncMock(), + push_image=AsyncMock(), + login=AsyncMock(), + ) + + def _make_provider(api: object) -> ExeProvider: - return ExeProvider(api, _settings()) # type: ignore[arg-type] + return ExeProvider(api, _settings(), docker=_docker_mock()) # type: ignore[arg-type] async def test_create_vm_forwards_kwargs_and_maps_result() -> None: @@ -60,9 +73,6 @@ async def test_delete_vm_delegates_to_api() -> None: async def test_delete_vm_translates_not_found_to_provider_not_found() -> None: - from providers.exceptions import ProviderNotFoundError - from providers.exe.exceptions import ExeVMNotFoundError - api = SimpleNamespace( delete_vm=AsyncMock(side_effect=ExeVMNotFoundError("vm 'sb-1' not found")), ) @@ -108,3 +118,97 @@ async def test_http_proxy_methods_delegate_to_api(method_name: str, kwargs: dict def test_from_settings_constructs_with_exeapi() -> None: provider = ExeProvider.from_settings() assert isinstance(provider.api, ExeAPI) + assert isinstance(provider.docker, DockerCLI) + + +async def test_materialize_template_builds_logs_in_and_pushes() -> None: + docker = _docker_mock() + provider = ExeProvider( + SimpleNamespace(), # type: ignore[arg-type] + _settings( + template_registry="ghcr.io/acme/drukbox-templates", + registry_username="builder", + registry_password="registry-secret", + ), + docker=docker, # type: ignore[arg-type] + ) + + handle = await provider.materialize_template( + base_image="exe/base:latest", + setup_script="apt-get update", + label="Node tools", + ) + + assert handle.startswith("ghcr.io/acme/drukbox-templates:") + assert len(handle.rpartition(":")[2]) == 12 + assert docker.build_image.await_args.args[0] == handle + docker.login.assert_awaited_once_with("ghcr.io", "builder", "registry-secret") + docker.push_image.assert_awaited_once_with(handle) + + +async def test_materialize_template_names_each_missing_registry_setting() -> None: + docker = _docker_mock() + provider = ExeProvider( + SimpleNamespace(), # type: ignore[arg-type] + _settings(), + docker=docker, # type: ignore[arg-type] + ) + + with pytest.raises(ProviderCommandError) as error: + await provider.materialize_template( + base_image="exe/base:latest", + setup_script="apt-get update", + label="Node tools", + ) + + assert "EXE_TEMPLATE_REGISTRY" in str(error.value) + assert "EXE_REGISTRY_USERNAME" in str(error.value) + assert "EXE_REGISTRY_PASSWORD" in str(error.value) + docker.build_image.assert_not_awaited() + + +async def test_materialize_template_translates_push_failure() -> None: + docker = _docker_mock() + docker.push_image.side_effect = DockerTransportError("push log tail") + provider = ExeProvider( + SimpleNamespace(), # type: ignore[arg-type] + _settings( + template_registry="ghcr.io/acme/drukbox-templates", + registry_username="builder", + registry_password="registry-secret", + ), + docker=docker, # type: ignore[arg-type] + ) + + with pytest.raises(ProviderTransportError, match="push log tail"): + await provider.materialize_template( + base_image="exe/base:latest", + setup_script="apt-get update", + label="Node tools", + ) + + +async def test_delete_template_removes_the_local_image() -> None: + docker = _docker_mock() + provider = ExeProvider( + SimpleNamespace(), # type: ignore[arg-type] + _settings(), + docker=docker, # type: ignore[arg-type] + ) + + await provider.delete_template("ghcr.io/acme/drukbox-templates:123456789abc") + + docker.remove_image.assert_awaited_once_with("ghcr.io/acme/drukbox-templates:123456789abc") + + +async def test_delete_template_translates_a_missing_local_image() -> None: + docker = _docker_mock() + docker.remove_image.side_effect = DockerImageNotFoundError("No such image") + provider = ExeProvider( + SimpleNamespace(), # type: ignore[arg-type] + _settings(), + docker=docker, # type: ignore[arg-type] + ) + + with pytest.raises(ProviderNotFoundError, match="was not found"): + await provider.delete_template("ghcr.io/acme/drukbox-templates:missing") diff --git a/src/providers/tests/test_derived_image.py b/src/providers/tests/test_derived_image.py new file mode 100644 index 0000000..c575e57 --- /dev/null +++ b/src/providers/tests/test_derived_image.py @@ -0,0 +1,24 @@ +from providers.derived_image import derived_image_context, derived_image_tag + + +def test_derived_image_context_contains_the_base_and_verbatim_script() -> None: + setup_script = "printf 'first\\nsecond\\n' | tee /tmp/output" + + with derived_image_context(base_image="sandbox:base", setup_script=setup_script) as context: + assert context.joinpath("setup.sh").read_bytes() == setup_script.encode("utf-8") + assert context.joinpath("Dockerfile").read_text(encoding="utf-8") == ( + "FROM sandbox:base\n" + "COPY setup.sh /drukbox-setup.sh\n" + "RUN sh /drukbox-setup.sh && rm /drukbox-setup.sh\n" + ) + + +def test_derived_image_tag_is_deterministic_and_base_specific() -> None: + first = derived_image_tag(base_image="sandbox:base", setup_script="apt-get update") + repeated = derived_image_tag(base_image="sandbox:base", setup_script="apt-get update") + different_base = derived_image_tag(base_image="sandbox:other", setup_script="apt-get update") + + assert first == repeated + assert first.startswith("drukbox-template:") + assert len(first.removeprefix("drukbox-template:")) == 12 + assert different_base != first diff --git a/src/templates/tests/conftest.py b/src/templates/tests/conftest.py index 953516b..bff631e 100644 --- a/src/templates/tests/conftest.py +++ b/src/templates/tests/conftest.py @@ -5,6 +5,7 @@ from providers import registry as registry_module from providers.base import VMCreateResult, VMProvider from providers.capabilities import TemplateCapability +from providers.derived_image import derived_image_tag from providers.exceptions import ProviderError @@ -61,7 +62,7 @@ async def materialize_template( self.materialized.append((base_image, setup_script, label)) if self.build_error: raise self.build_error - return f"stub-template:{len(self.materialized)}" + return derived_image_tag(base_image=base_image, setup_script=setup_script) async def delete_template(self, handle: str) -> None: self.deleted.append(handle) diff --git a/src/templates/tests/test_api.py b/src/templates/tests/test_api.py index cc02e35..528113b 100644 --- a/src/templates/tests/test_api.py +++ b/src/templates/tests/test_api.py @@ -10,6 +10,9 @@ from uuid6 import uuid7 from core.database import async_session_factory +from providers import registry as registry_module +from providers.base import VMProvider +from providers.derived_image import derived_image_tag from providers.exceptions import ProviderNotFoundError, ProviderTransportError from templates.exceptions import TemplateStateError from templates.models import Template, TemplateStatus @@ -48,7 +51,10 @@ async def test_create_template_returns_building_then_materializes(client, templa assert polled.status_code == 200 assert polled.json()["status"] == TemplateStatus.AVAILABLE.value - assert polled.json()["handle"] == "stub-template:1" + assert polled.json()["handle"] == derived_image_tag( + base_image=template_provider.default_image, + setup_script=SETUP_SCRIPT, + ) assert "setup_script" not in polled.json() assert template_provider.materialized == [ (template_provider.default_image, SETUP_SCRIPT, "Node tools") @@ -89,12 +95,18 @@ async def test_unexpected_build_crash_is_pollable(client, template_provider): assert polled.json()["last_error"] == "OSError: builder crashed" -async def test_unsupported_capability_becomes_failed_build(client): +async def test_unsupported_capability_becomes_failed_build(client, monkeypatch): """A provider without template support reports failure through polling.""" + provider = MagicMock(spec=VMProvider) + provider.name = "without-templates" + provider.default_image = "stub:base" + monkeypatch.setitem(registry_module._factories, provider.name, lambda: provider) + monkeypatch.setitem(registry_module._instances, provider.name, provider) + response = await client.post( "/templates", headers=AUTH_HEADERS, - json={"provider": "docker", "setup_script": SETUP_SCRIPT}, + json={"provider": provider.name, "setup_script": SETUP_SCRIPT}, ) polled = await client.get(f"/templates/{response.json()['id']}", headers=AUTH_HEADERS) From f022095b3822bf5356099d7c520cc77654554caf Mon Sep 17 00:00:00 2001 From: Paulo Date: Mon, 24 Aug 2026 20:08:18 +0200 Subject: [PATCH 3/8] ENG-879 - create_host forks from an available template MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit HostCreate accepts template — an id or a requirements hash — and the resolution is explicit image > template handle > provider default, with the resolved handle riding host.image into the unchanged provision path. A hash matching several bases prefers the available ones, newest first. Naming a template that is not available fails 409 with the status (and last_error for failed builds); drukbox never builds on miss — the caller owns when to build. A template request is a customization, so it never claims a warm pool host, and each fork stamps last_used_at for GC. Co-Authored-By: Claude Fable 5 --- docs/architecture.md | 12 +- src/hosts/api.py | 1 + src/hosts/schemas.py | 6 +- src/hosts/service.py | 49 ++++- src/hosts/tests/test_templates.py | 297 ++++++++++++++++++++++++++++++ src/templates/exceptions.py | 10 + 6 files changed, 368 insertions(+), 7 deletions(-) create mode 100644 src/hosts/tests/test_templates.py diff --git a/docs/architecture.md b/docs/architecture.md index cc7c19a..216d6c9 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -96,6 +96,12 @@ successful key returns the original host instead of a duplicate. Caller `env` is stored for provisioning and never returned by the API; keys in `hosts.schemas.RESERVED_HOST_ENV_KEYS` are rejected. +An available template can be requested by its ID or requirements hash. The +template's provider handle becomes the host image; an explicit `image` takes +precedence, followed by the template, then the provider default. Host creation +never builds a missing or unavailable template: it returns a clear client error +so the caller retains control of the asynchronous build lifecycle. + Every host is a renewable lease. A create without `expires_at` gets `now + LEASE_DEFAULT_TTL`, so a host whose owner disappears lapses and self-reaps instead of leaking VM cost; an explicit `expires_at: null` @@ -110,9 +116,9 @@ Two maintenance commands run as cron jobs from the same image: warm pool of pre-provisioned hosts per provider (`POOL_SIZES`, with `POOL_SIZE` as the default provider's target) to hide provider cold starts. Pool members are warmed with the provider's default image and -size, so a request that customizes its host — `image`, `env`, -`instance_type`, or `disk_gb` — always provisions fresh instead of -claiming a warm host. +size, so a request that customizes its host — `image`, `env`, `template`, +`instance_type`, or `disk_gb` — always provisions fresh instead of claiming a +warm host. ## Diagnostics diff --git a/src/hosts/api.py b/src/hosts/api.py index f665cee..8afa95d 100644 --- a/src/hosts/api.py +++ b/src/hosts/api.py @@ -54,6 +54,7 @@ async def create_host( return await service.get_or_create_host( env=host_create.env, image=host_create.image, + template=host_create.template, expires_at=expires_at, idempotency_key=idempotency_key, provider=host_create.provider, diff --git a/src/hosts/schemas.py b/src/hosts/schemas.py index 8dd4efd..1490558 100644 --- a/src/hosts/schemas.py +++ b/src/hosts/schemas.py @@ -20,6 +20,10 @@ def _expires_at_must_be_future_and_tz_aware(expires_at: datetime | None) -> date class HostCreate(BaseModel): image: str | None = None + template: str | None = Field( + default=None, + description="Template ID or requirements hash. Used only when image is omitted.", + ) env: dict[str, str] = Field(default_factory=dict) expires_at: datetime | None = None provider: str | None = Field( @@ -39,7 +43,7 @@ class HostCreate(BaseModel): description="Root disk size in GB. Omit to use the provider's configured default.", ) - @field_validator("image", "instance_type") + @field_validator("image", "template", "instance_type") @classmethod def reject_blank(cls, value: str | None, info: ValidationInfo) -> str | None: if value is not None and not value.strip(): diff --git a/src/hosts/service.py b/src/hosts/service.py index b9c5af1..6dc3d20 100644 --- a/src/hosts/service.py +++ b/src/hosts/service.py @@ -30,6 +30,8 @@ UnsupportedSizingError, ) from providers.registry import get_provider_names, get_vm_provider +from templates.exceptions import TemplateNotAvailableError, TemplateReferenceError +from templates.models import Template, TemplateStatus logger = logging.getLogger(__name__) @@ -92,6 +94,7 @@ async def get_or_create_host( *, env: dict[str, str], image: str | None, + template: str | None = None, expires_at: datetime | None | EllipsisType = ..., idempotency_key: str | None = None, provider: str | None = None, @@ -117,10 +120,10 @@ async def get_or_create_host( host: Host | None = None # Warm hosts are provider-specific, so the claim is scoped to the # requested provider's pool. A request is pool-eligible only when it - # doesn't customize the host: default image, no env, and no per-request - # sizing — pool members are warmed at the provider's default size. + # doesn't customize the host: no image, template, env, or per-request + # sizing — pool members are warmed at the provider's defaults. requested_provider = provider or self.settings.default_host_provider - customized = env or image or instance_type or disk_gb + customized = env or image or template or instance_type or disk_gb if not customized and self.settings.get_pool_targets().get(requested_provider): host = await self._try_claim_pool_host( provider=requested_provider, expires_at=expires_at @@ -129,6 +132,7 @@ async def get_or_create_host( host = await self.create_host( env=env, image=image, + template=template, expires_at=expires_at, provider=provider, instance_type=instance_type, @@ -199,6 +203,7 @@ async def create_host( *, env: dict[str, str], image: str | None, + template: str | None = None, expires_at: datetime | None | EllipsisType = ..., provider: str | None = None, instance_type: str | None = None, @@ -217,6 +222,8 @@ async def create_host( raise UnsupportedSizingError( f"provider {vm.name!r} does not support a per-request disk_gb" ) + if template and not image: + image = await self._resolve_template_image(reference=template, provider=vm.name) uid = uuid7() name = Host.build_name(uid) now = utc_now() @@ -275,6 +282,42 @@ async def create_host( await self.session.refresh(host) return host + async def _resolve_template_image(self, *, reference: str, provider: str) -> str: + try: + template_id = uuid.UUID(reference) + except ValueError: + result = await self.session.execute( + select(Template) + .where(Template.provider == provider) + .where(Template.requirements_hash == reference) + .order_by( + (Template.status == TemplateStatus.AVAILABLE.value).desc(), + Template.created_at.desc(), + ) + .limit(1) + ) + else: + result = await self.session.execute( + select(Template) + .where(Template.id == template_id) + .where(Template.provider == provider) + ) + template = result.scalar_one_or_none() + + if not template: + raise TemplateReferenceError( + f"template {reference!r} not found for provider {provider!r}" + ) + + if template.status != TemplateStatus.AVAILABLE.value: + detail = f"template {template.id} is {template.status}" + if template.status == TemplateStatus.FAILED.value: + detail = f"{detail}: {template.last_error}" + raise TemplateNotAvailableError(detail) + + template.last_used_at = utc_now() + return template.handle + async def _lookup_idempotency_key(self, key: str) -> Host | None: record = ( await self.session.execute(select(IdempotencyKey).where(IdempotencyKey.key == key)) diff --git a/src/hosts/tests/test_templates.py b/src/hosts/tests/test_templates.py new file mode 100644 index 0000000..73151b0 --- /dev/null +++ b/src/hosts/tests/test_templates.py @@ -0,0 +1,297 @@ +import uuid +from datetime import datetime, timedelta +from unittest.mock import AsyncMock + +from sqlalchemy import func, select +from uuid6 import uuid7 + +from core.database import async_session_factory +from core.settings import get_settings +from hosts.models import Host, HostStatus +from hosts.service import utc_now +from providers.exe.settings import ExeSettings +from templates.models import Template, TemplateStatus + +AUTH_HEADERS = {"Authorization": "Bearer service-token"} +REQUIREMENTS_HASH = "a" * 64 +SETUP_SCRIPT = "apt-get update && apt-get install -y nodejs" + + +async def create_template_record( + *, + provider: str = "exe", + base_image: str = "base:image", + requirements_hash: str = REQUIREMENTS_HASH, + handle: str = "", + status: str = TemplateStatus.AVAILABLE.value, + last_error: str = "", + created_at: datetime | None = None, +) -> Template: + now = created_at or utc_now() + template = Template( + id=uuid7(), + provider=provider, + base_image=base_image, + requirements_hash=requirements_hash, + setup_script=SETUP_SCRIPT, + label="", + handle=handle, + status=status, + last_error=last_error, + created_at=now, + updated_at=now, + ) + async with async_session_factory() as session: + session.add(template) + await session.commit() + await session.refresh(template) + return template + + +async def test_create_host_resolves_template_id(client, monkeypatch): + """An available template ID becomes the stored image and records its use.""" + template = await create_template_record(handle="derived:image-by-id") + monkeypatch.setattr("hosts.service.HostService.provision", AsyncMock()) + + before = utc_now() + response = await client.post( + "/hosts", + headers=AUTH_HEADERS, + json={"template": str(template.id)}, + ) + after = utc_now() + + assert response.status_code == 201 + assert response.json()["image"] == "derived:image-by-id" + async with async_session_factory() as session: + host = await session.get(Host, uuid.UUID(response.json()["id"])) + used_template = await session.get(Template, template.id) + assert host is not None + assert host.image == "derived:image-by-id" + assert used_template is not None + assert used_template.last_used_at is not None + assert before <= used_template.last_used_at <= after + + +async def test_create_host_resolves_template_requirements_hash(client, monkeypatch): + """An available requirements hash becomes the stored image and records its use.""" + template = await create_template_record(handle="derived:image-by-hash") + monkeypatch.setattr("hosts.service.HostService.provision", AsyncMock()) + + response = await client.post( + "/hosts", + headers=AUTH_HEADERS, + json={"template": template.requirements_hash}, + ) + + assert response.status_code == 201 + assert response.json()["image"] == "derived:image-by-hash" + async with async_session_factory() as session: + used_template = await session.get(Template, template.id) + assert used_template is not None + assert used_template.last_used_at is not None + + +async def test_create_host_prefers_available_template_for_requirements_hash(client, monkeypatch): + """An available hash match wins over a newer unavailable base-image variant.""" + now = utc_now() + available = await create_template_record( + base_image="base:available", + handle="derived:available", + created_at=now, + ) + building = await create_template_record( + base_image="base:building", + status=TemplateStatus.BUILDING.value, + created_at=now + timedelta(seconds=1), + ) + monkeypatch.setattr("hosts.service.HostService.provision", AsyncMock()) + + response = await client.post( + "/hosts", + headers=AUTH_HEADERS, + json={"template": REQUIREMENTS_HASH}, + ) + + assert response.status_code == 201 + assert response.json()["image"] == "derived:available" + async with async_session_factory() as session: + used_available = await session.get(Template, available.id) + untouched_building = await session.get(Template, building.id) + assert used_available is not None + assert used_available.last_used_at is not None + assert untouched_building is not None + assert untouched_building.last_used_at is None + + +async def test_create_host_rejects_building_template(client): + """A building template returns a conflict naming its current status.""" + template = await create_template_record(status=TemplateStatus.BUILDING.value) + + response = await client.post( + "/hosts", + headers=AUTH_HEADERS, + json={"template": str(template.id)}, + ) + + assert response.status_code == 409 + assert str(template.id) in response.json()["detail"] + assert TemplateStatus.BUILDING.value in response.json()["detail"] + assert response.json()["error_code"] == "TEMPLATE_NOT_AVAILABLE" + async with async_session_factory() as session: + host_count = await session.scalar(select(func.count()).select_from(Host)) + assert host_count == 0 + + +async def test_create_host_rejects_failed_template_with_last_error(client): + """A failed template conflict includes both its status and build diagnostic.""" + template = await create_template_record( + status=TemplateStatus.FAILED.value, + last_error="ProviderTransportError: builder unavailable", + ) + + response = await client.post( + "/hosts", + headers=AUTH_HEADERS, + json={"template": str(template.id)}, + ) + + assert response.status_code == 409 + assert str(template.id) in response.json()["detail"] + assert TemplateStatus.FAILED.value in response.json()["detail"] + assert "ProviderTransportError: builder unavailable" in response.json()["detail"] + + +async def test_create_host_reports_newest_unavailable_hash_match(client): + """An unavailable hash reports the newest matching base-image variant.""" + now = utc_now() + await create_template_record( + base_image="base:older-building", + status=TemplateStatus.BUILDING.value, + created_at=now, + ) + newest = await create_template_record( + base_image="base:newer-failed", + status=TemplateStatus.FAILED.value, + last_error="ProviderTransportError: newest failure", + created_at=now + timedelta(seconds=1), + ) + + response = await client.post( + "/hosts", + headers=AUTH_HEADERS, + json={"template": REQUIREMENTS_HASH}, + ) + + assert response.status_code == 409 + assert str(newest.id) in response.json()["detail"] + assert TemplateStatus.FAILED.value in response.json()["detail"] + assert "ProviderTransportError: newest failure" in response.json()["detail"] + + +async def test_create_host_rejects_unknown_template_reference(client): + """An unknown template reference is bad host-create input, not a route 404.""" + response = await client.post( + "/hosts", + headers=AUTH_HEADERS, + json={"template": "missing-template"}, + ) + + assert response.status_code == 400 + assert "missing-template" in response.json()["detail"] + assert "not found" in response.json()["detail"] + assert response.json()["error_code"] == "TEMPLATE_REFERENCE" + + +async def test_create_host_explicit_image_wins_without_touching_template(client, monkeypatch): + """An explicit image bypasses template resolution and leaves usage unstamped.""" + template = await create_template_record(handle="derived:ignored") + monkeypatch.setattr("hosts.service.HostService.provision", AsyncMock()) + + response = await client.post( + "/hosts", + headers=AUTH_HEADERS, + json={"image": "explicit:image", "template": str(template.id)}, + ) + + assert response.status_code == 201 + assert response.json()["image"] == "explicit:image" + async with async_session_factory() as session: + untouched_template = await session.get(Template, template.id) + assert untouched_template is not None + assert untouched_template.last_used_at is None + + +async def test_create_host_template_request_bypasses_pool(client, monkeypatch): + """A template request provisions fresh instead of claiming a default-image pool host.""" + monkeypatch.setenv("POOL_SIZE", "1") + monkeypatch.delenv("POOL_SIZES", raising=False) + get_settings.cache_clear() + now = utc_now() + pool_host = Host( + id=uuid7(), + name="sb-template-pool", + status=HostStatus.ACTIVE.value, + provider="exe", + image=ExeSettings().default_image, # pyright: ignore[reportCallIssue] + env={}, + internal_ssh_host="sb-template-pool.example.ts.net", + external_ssh_host="", + external_ssh_port=22, + known_hosts="", + created_at=now, + updated_at=now, + expires_at=now + timedelta(hours=4), + pool_member=True, + last_error="", + ) + template = await create_template_record(handle="derived:fresh") + async with async_session_factory() as session: + session.add(pool_host) + await session.commit() + mocked_provision = AsyncMock() + monkeypatch.setattr("hosts.service.HostService.provision", mocked_provision) + + try: + response = await client.post( + "/hosts", + headers=AUTH_HEADERS, + json={"template": str(template.id)}, + ) + finally: + get_settings.cache_clear() + + assert response.status_code == 201 + assert response.json()["id"] != str(pool_host.id) + assert response.json()["image"] == "derived:fresh" + mocked_provision.assert_awaited_once() + + +async def test_create_host_cannot_resolve_another_providers_template(client): + """A template belonging to another provider is invisible to host creation.""" + template = await create_template_record( + provider="other-provider", + handle="derived:other-provider", + ) + + response = await client.post( + "/hosts", + headers=AUTH_HEADERS, + json={"template": str(template.id)}, + ) + + assert response.status_code == 400 + assert str(template.id) in response.json()["detail"] + assert "provider 'exe'" in response.json()["detail"] + + +async def test_create_host_rejects_blank_template(client): + """A whitespace-only template reference stops at the wire boundary.""" + response = await client.post( + "/hosts", + headers=AUTH_HEADERS, + json={"template": " \n"}, + ) + + assert response.status_code == 422 + assert response.json()["detail"][0]["loc"] == ["body", "template"] diff --git a/src/templates/exceptions.py b/src/templates/exceptions.py index 6bc40b0..ce3895e 100644 --- a/src/templates/exceptions.py +++ b/src/templates/exceptions.py @@ -1,6 +1,16 @@ from core.exceptions import AppException +class TemplateReferenceError(AppException): + status_code = 400 + error_code = "TEMPLATE_REFERENCE" + + +class TemplateNotAvailableError(AppException): + status_code = 409 + error_code = "TEMPLATE_NOT_AVAILABLE" + + class TemplateStateError(AppException): status_code = 409 error_code = "TEMPLATE_STATE" From baa2e9aad3dfbbf51855994707534e21d3958894 Mon Sep 17 00:00:00 2001 From: Paulo Date: Mon, 24 Aug 2026 20:23:12 +0200 Subject: [PATCH 4/8] ENG-880 - Template GC: templates outlive hosts and get their own reaping Templates cost money independent of hosts, so python -m templates.janitor gets its own cron entry. Three sweeps: an unfinished build older than TEMPLATE_BUILD_TIMEOUT_MINUTES becomes failed (the only exit for a build whose process died); failed templates keep their diagnostics for TEMPLATE_FAILED_RETENTION_HOURS, then go; available templates unleased for TEMPLATE_UNUSED_TTL_DAYS age out through the strategy's delete -- which is also how a superseded hash dies, since edits mint a new one. Deletes re-validate under the row lock so a fresh lease or status change spares the row, hosts-janitor style. Co-Authored-By: Claude Fable 5 --- AGENTS.md | 5 +- docs/api.md | 2 + docs/architecture.md | 43 +++--- docs/deploy.md | 24 ++-- src/core/settings.py | 18 +++ src/core/tests/test_settings.py | 19 +++ src/templates/janitor.py | 125 ++++++++++++++++ src/templates/service.py | 23 ++- src/templates/tests/test_janitor.py | 213 ++++++++++++++++++++++++++++ 9 files changed, 445 insertions(+), 27 deletions(-) create mode 100644 src/templates/janitor.py create mode 100644 src/templates/tests/test_janitor.py diff --git a/AGENTS.md b/AGENTS.md index 303b776..fea0634 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -10,6 +10,7 @@ a service, not a library. It owns: - Host records and lifecycle state in Postgres +- Template records, asynchronous builds, and provider artifact lifecycle - Inline provisioning in `POST /hosts` - Provider VM creation and deletion (exe.dev, AWS, Hetzner, Exoscale, local Docker, Docker Sandboxes) @@ -19,7 +20,8 @@ It owns: - Account-bound exe.dev HTTP proxy resources Periodic maintenance runs as cron jobs: `python -m hosts.janitor` reaps -expired hosts, `python -m hosts.pool` tops up the warm pool. +expired hosts, `python -m templates.janitor` reaps abandoned and unused +templates, and `python -m hosts.pool` tops up the warm pool. No backwards compatibility is required unless a caller contract is explicitly documented in this repo. @@ -49,6 +51,7 @@ src/ http_proxies/ # HTTP proxy API, schemas, service, deps providers/ # VM provider ABC, capabilities, registry, adapters networking/ # Network provider framework and Tailscale adapter + templates/ # Template API, models, service, and janitor conftest.py # Test env defaults and database reset fixture alembic/ # Database migrations api-tests/ # Playwright black-box API tests diff --git a/docs/api.md b/docs/api.md index 5178091..431fc30 100644 --- a/docs/api.md +++ b/docs/api.md @@ -13,6 +13,8 @@ Every endpoint except `GET /healthz` requires ## Endpoints - `POST /hosts` · `GET /hosts` · `GET /hosts/{id}` · `DELETE /hosts/{id}` +- `POST /templates` · `GET /templates` · `GET /templates/{id}` · + `DELETE /templates/{id}` - `POST /http-proxies` · `DELETE /http-proxies/{name}` · `POST|DELETE /http-proxies/{name}/hosts/{host_id}` - `GET /doctor` — read-only dependency diagnostics diff --git a/docs/architecture.md b/docs/architecture.md index 216d6c9..5423a68 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -29,6 +29,8 @@ that true: ```text hosts.api HTTP request/response concerns only hosts.service host lifecycle behavior (HostService) +templates.api template request/response concerns only +templates.service template build and artifact lifecycle (TemplateService) providers/ one package per VM provider networking/ network provider framework + Tailscale adapter core/ settings, database, exception base @@ -71,14 +73,15 @@ the core settings knowing any provider exists. Not every provider supports every feature, and the host contract must not grow provider-shaped warts. Optional features are capability -mix-ins: `HttpProxyCapability` declares the http-proxy surface and the -exe provider implements it. `resolve_capability` narrows a specific -provider instance to a capability — the default provider for -account-bound operations, the host's own provider for host-bound ones -— and raises the shared `CapabilityUnsupportedError` when that -provider doesn't implement it, which the routes surface as a clear -error. New provider-specific features should follow this pattern -rather than widening `VMProvider` or the host schema. +mix-ins: `HttpProxyCapability` declares the http-proxy surface, while +`TemplateCapability` owns provider-specific template materialization +and teardown. `resolve_capability` narrows a specific provider instance +to a capability — the default provider for account-bound operations, +the host's own provider for host-bound ones — and raises the shared +`CapabilityUnsupportedError` when that provider doesn't implement it, +which the routes surface as a clear error. New provider-specific +features should follow this pattern rather than widening `VMProvider` +or the host schema. The review question that guards the whole design: *does this change leak a provider into the contract?* @@ -96,6 +99,12 @@ successful key returns the original host instead of a duplicate. Caller `env` is stored for provisioning and never returned by the API; keys in `hosts.schemas.RESERVED_HOST_ENV_KEYS` are rejected. +Templates are durable provider artifacts keyed by provider, base image, +and setup-script hash. `POST /templates` creates a `building` record and +returns `202 Accepted`; callers poll until it becomes `available` or +`failed`. Templates outlive individual hosts, and provider-specific +materialization stays behind `TemplateCapability`. + An available template can be requested by its ID or requirements hash. The template's provider handle becomes the host image; an explicit `image` takes precedence, followed by the template, then the provider default. Host creation @@ -111,14 +120,16 @@ is the keepalive: it bumps `expires_at` to the requested instant, or by hosts renew — unclaimed warm-pool members belong to pool maintenance and refuse with `409`. -Two maintenance commands run as cron jobs from the same image: -`hosts.janitor` reaps expired and orphaned hosts, `hosts.pool` keeps a -warm pool of pre-provisioned hosts per provider (`POOL_SIZES`, with -`POOL_SIZE` as the default provider's target) to hide provider cold -starts. Pool members are warmed with the provider's default image and -size, so a request that customizes its host — `image`, `env`, `template`, -`instance_type`, or `disk_gb` — always provisions fresh instead of claiming a -warm host. +Three maintenance commands run as cron jobs from the same image: +`hosts.janitor` reaps expired and orphaned hosts, `templates.janitor` +fails abandoned builds and reaps failed or unused artifacts, and +`hosts.pool` keeps a warm pool of pre-provisioned hosts per provider +(`POOL_SIZES`, with `POOL_SIZE` as the default provider's target) to hide +provider cold starts. Editing a template setup script changes its hash; +the superseded artifact then ages out after its last lease. Pool members +are warmed with the provider's default image and size, so a request that +customizes its host — `image`, `env`, `template`, `instance_type`, or +`disk_gb` — always provisions fresh instead of claiming a warm host. ## Diagnostics diff --git a/docs/deploy.md b/docs/deploy.md index d9aced9..6f44950 100644 --- a/docs/deploy.md +++ b/docs/deploy.md @@ -22,13 +22,16 @@ docker run --rm --env-file drukbox.env "$IMAGE" .venv/bin/alembic upgrade head # Maintenance (cron, e.g. every 10-15 min) docker run --rm --env-file drukbox.env "$IMAGE" .venv/bin/python -m hosts.janitor docker run --rm --env-file drukbox.env "$IMAGE" .venv/bin/python -m hosts.pool +docker run --rm --env-file drukbox.env "$IMAGE" .venv/bin/python -m templates.janitor ``` -The janitor reaps expired and orphaned hosts. The pool maintainer +The host janitor reaps expired and orphaned hosts. The template janitor +fails abandoned builds, retains their diagnostics for a bounded window, +and reaps failed or unused provider artifacts. The pool maintainer pre-provisions warm hosts per provider and only does anything when at least one provider has a warm target (`POOL_SIZES` / `POOL_SIZE`). -Schedule both under your cron infrastructure (k8s `CronJob`, systemd -timer) from the same image and env file. +Schedule all three under your cron infrastructure (k8s `CronJob`, +systemd timer) from the same image and env file. Use Postgres in production (`postgresql+psycopg://...`). SQLite (`sqlite+aiosqlite:///./drukbox.db`) is for single-process demos and @@ -99,10 +102,10 @@ talks to the host's Docker daemon, and granting drukbox access to that socket is host-root-equivalent. Do not expose a docker-backed drukbox to untrusted callers. -Janitor and pool one-off containers using the Docker provider need the -same socket mount and socket-GID supplemental group. `DOCKER_HOST` remains -available when the daemon is remote or rootless instead of exposed through -`/var/run/docker.sock`. +Host-janitor, template-janitor, and pool one-off containers using the +Docker provider need the same socket mount and socket-GID supplemental +group. `DOCKER_HOST` remains available when the daemon is remote or +rootless instead of exposed through `/var/run/docker.sock`. ## Local microVMs with Docker Sandboxes @@ -154,8 +157,8 @@ docker run --rm --network host \ The daemon reads workspace paths on its own filesystem. Thus the workspace mount must have the same path on the host and in the -container. The janitor and pool containers need the same mounts and -variables. +container. The host-janitor, template-janitor, and pool containers need +the same mounts and variables. Callers reach the sandboxes through [the SSH gateway](#the-ssh-gateway); the provider requires it. The key @@ -294,6 +297,9 @@ Core, optional: | `SERVICE_LABEL` | `drukbox` | Label stamped onto provider resources (VM tags, SG tags). | | `UVICORN_HOST` | `0.0.0.0` | API bind address. Set `127.0.0.1` to restrict to loopback. | | `PROVISIONING_GRACE_SECONDS` | `600` | Safety TTL on in-flight hosts so the janitor reaps row + VM if the client disconnects mid-provision. Must exceed the worst-case provision duration. | +| `TEMPLATE_BUILD_TIMEOUT_MINUTES` | `60` | Max age of an unfinished template build before the template janitor marks it failed. | +| `TEMPLATE_FAILED_RETENTION_HOURS` | `24` | How long failed template records and diagnostics remain before the template janitor deletes them. | +| `TEMPLATE_UNUSED_TTL_DAYS` | `14` | How long an available template remains after its last use, or creation when never used. | | `LEASE_DEFAULT_TTL` | `86400` | Lease TTL in seconds for hosts created without an explicit `expires_at`, and the extension applied by an empty `POST /hosts/{id}/renew`. An explicit `expires_at: null` at create time opts out of expiry entirely. | | `IDEMPOTENCY_KEY_TTL_HOURS` | `24` | Retention period for successful `Idempotency-Key` mappings. | | `POOL_SIZES` | `{}` | Warm hosts to keep ready per provider, as JSON (e.g. `{"exe": 2, "hetzner": 1}`). Overrides `POOL_SIZE` for the providers it names. | diff --git a/src/core/settings.py b/src/core/settings.py index 03c40b2..2513335 100644 --- a/src/core/settings.py +++ b/src/core/settings.py @@ -70,6 +70,24 @@ class Settings(BaseSettings): validation_alias="PROVISIONING_GRACE_SECONDS", description="Safety TTL on the host row while provisioning is in flight.", ) + template_build_timeout_minutes: int = Field( + default=60, + gt=0, + validation_alias="TEMPLATE_BUILD_TIMEOUT_MINUTES", + description="Minutes before an unfinished template build is considered abandoned.", + ) + template_failed_retention_hours: int = Field( + default=24, + gt=0, + validation_alias="TEMPLATE_FAILED_RETENTION_HOURS", + description="Hours to retain failed templates for diagnosis before deletion.", + ) + template_unused_ttl_days: int = Field( + default=14, + gt=0, + validation_alias="TEMPLATE_UNUSED_TTL_DAYS", + description="Days to retain an available template after its last use.", + ) lease_default_ttl: int = Field( default=86400, gt=0, diff --git a/src/core/tests/test_settings.py b/src/core/tests/test_settings.py index e404508..71f66d9 100644 --- a/src/core/tests/test_settings.py +++ b/src/core/tests/test_settings.py @@ -92,6 +92,9 @@ def test_tailscale_disabled_ignores_missing_credentials(monkeypatch: pytest.Monk "POOL_SIZE", "POOL_HOST_MAX_AGE_HOURS", "POOL_MAX_CREATES_PER_TICK", + "TEMPLATE_BUILD_TIMEOUT_MINUTES", + "TEMPLATE_FAILED_RETENTION_HOURS", + "TEMPLATE_UNUSED_TTL_DAYS", ], ) def test_numeric_settings_reject_negative_values(monkeypatch: pytest.MonkeyPatch, key: str) -> None: @@ -100,6 +103,22 @@ def test_numeric_settings_reject_negative_values(monkeypatch: pytest.MonkeyPatch _settings_with(monkeypatch, env) +def test_template_maintenance_defaults(monkeypatch: pytest.MonkeyPatch) -> None: + settings = _settings_with( + monkeypatch, + { + **_base_env(), + "TEMPLATE_BUILD_TIMEOUT_MINUTES": None, + "TEMPLATE_FAILED_RETENTION_HOURS": None, + "TEMPLATE_UNUSED_TTL_DAYS": None, + }, + ) + + assert settings.template_build_timeout_minutes == 60 + assert settings.template_failed_retention_hours == 24 + assert settings.template_unused_ttl_days == 14 + + def test_pool_size_seeds_the_default_providers_target(monkeypatch: pytest.MonkeyPatch) -> None: env: dict[str, str | None] = { **_base_env(), diff --git a/src/templates/janitor.py b/src/templates/janitor.py new file mode 100644 index 0000000..6c4dbd7 --- /dev/null +++ b/src/templates/janitor.py @@ -0,0 +1,125 @@ +import logging +from datetime import timedelta + +from sqlalchemy import func, select + +from core.database import async_session_factory +from core.exceptions import ResourceNotFoundError +from core.settings import get_settings +from hosts.service import utc_now +from templates.models import Template, TemplateStatus +from templates.service import TemplateService + +logger = logging.getLogger(__name__) + + +async def reap_templates() -> None: + settings = get_settings() + now = utc_now() + build_cutoff = now - timedelta(minutes=settings.template_build_timeout_minutes) + failed_cutoff = now - timedelta(hours=settings.template_failed_retention_hours) + unused_cutoff = now - timedelta(days=settings.template_unused_ttl_days) + + async with async_session_factory() as session: + result = await session.execute( + select(Template.id) + .where(Template.status == TemplateStatus.BUILDING.value) + .where(Template.updated_at < build_cutoff) + .order_by(Template.updated_at, Template.id) + ) + abandoned_ids = list(result.scalars()) + + for template_id in abandoned_ids: + try: + async with async_session_factory() as session: + result = await session.execute( + select(Template).where(Template.id == template_id).with_for_update() + ) + template = result.scalar_one_or_none() + if not template: + continue + if ( + template.status != TemplateStatus.BUILDING.value + or template.updated_at >= build_cutoff + ): + continue + + template.status = TemplateStatus.FAILED.value + template.last_error = ( + "build abandoned: exceeded " + f"{settings.template_build_timeout_minutes}-minute timeout" + ) + template.updated_at = utc_now() + await session.commit() + except Exception: + logger.exception( + "template janitor: failed to mark abandoned build: template_id=%s", + template_id, + ) + else: + logger.info( + "template janitor: marked abandoned build failed: template_id=%s", + template_id, + ) + + last_used_at = func.coalesce(Template.last_used_at, Template.created_at) + reap_sweeps = ( + ( + TemplateStatus.FAILED, + failed_cutoff, + "failed", + select(Template.id) + .where(Template.status == TemplateStatus.FAILED.value) + .where(Template.updated_at < failed_cutoff) + .order_by(Template.updated_at, Template.id), + ), + ( + TemplateStatus.AVAILABLE, + unused_cutoff, + "unused", + select(Template.id) + .where(Template.status == TemplateStatus.AVAILABLE.value) + .where(last_used_at < unused_cutoff) + .order_by(last_used_at, Template.id), + ), + ) + + for reap_status, reap_before, reap_reason, candidate_query in reap_sweeps: + async with async_session_factory() as session: + result = await session.execute(candidate_query) + candidate_ids = list(result.scalars()) + + for template_id in candidate_ids: + try: + async with async_session_factory() as session: + deleted = await TemplateService(session).delete( + template_id, + reap_status=reap_status, + reap_before=reap_before, + ) + except ResourceNotFoundError: + continue + except Exception: + logger.exception( + "template janitor: failed to reap %s template: template_id=%s", + reap_reason, + template_id, + ) + else: + if deleted: + logger.info( + "template janitor: reaped %s template: template_id=%s", + reap_reason, + template_id, + ) + + +if __name__ == "__main__": + # Cron entry point: `python -m templates.janitor`. + import asyncio + + logging.basicConfig( + level=logging.INFO, + format="%(asctime)s %(levelname)s %(name)s %(message)s", + ) + asyncio.run(reap_templates()) diff --git a/src/templates/service.py b/src/templates/service.py index 45553ab..0d26178 100644 --- a/src/templates/service.py +++ b/src/templates/service.py @@ -1,6 +1,7 @@ import hashlib import logging import uuid +from datetime import datetime from sqlalchemy import select from sqlalchemy.exc import IntegrityError @@ -116,7 +117,14 @@ async def list(self) -> list[Template]: result = await self.session.execute(select(Template).order_by(Template.created_at.desc())) return list(result.scalars()) - async def delete(self, template_id: uuid.UUID) -> None: + async def delete( + self, + template_id: uuid.UUID, + *, + reap_status: TemplateStatus | None = None, + reap_before: datetime | None = None, + ) -> bool: + """Delete the template; return False when the maintenance guard spares it.""" result = await self.session.execute( select(Template).where(Template.id == template_id).with_for_update() ) @@ -125,6 +133,18 @@ async def delete(self, template_id: uuid.UUID) -> None: if not template: raise ResourceNotFoundError("template not found") + if reap_status and reap_before: + if template.status != reap_status: + return False + + last_active_at = ( + template.updated_at + if reap_status == TemplateStatus.FAILED + else template.last_used_at or template.created_at + ) + if last_active_at >= reap_before: + return False + if template.status == TemplateStatus.BUILDING.value: raise TemplateStateError("template is still building") @@ -146,3 +166,4 @@ async def delete(self, template_id: uuid.UUID) -> None: await self.session.delete(template) await self.session.commit() + return True diff --git a/src/templates/tests/test_janitor.py b/src/templates/tests/test_janitor.py new file mode 100644 index 0000000..81b4d9a --- /dev/null +++ b/src/templates/tests/test_janitor.py @@ -0,0 +1,213 @@ +import hashlib +from datetime import timedelta +from unittest.mock import AsyncMock + +from uuid6 import uuid7 + +from core.database import async_session_factory +from core.settings import get_settings +from hosts.service import utc_now +from providers.exceptions import ProviderNotFoundError, ProviderTransportError +from templates.janitor import reap_templates +from templates.models import Template, TemplateStatus +from templates.service import TemplateService + + +async def _create_template( + *, + provider: str, + status: TemplateStatus, + age: timedelta, + name: str, + handle: str = "", + last_used_age: timedelta | None = None, +) -> Template: + now = utc_now() + setup_script = f"echo {name}" + template = Template( + id=uuid7(), + provider=provider, + base_image="stub:base", + requirements_hash=hashlib.sha256(setup_script.encode()).hexdigest(), + setup_script=setup_script, + label=name, + handle=handle, + status=status.value, + last_error="", + created_at=now - age, + updated_at=now - age, + last_used_at=now - last_used_age if last_used_age is not None else None, + ) + async with async_session_factory() as session: + session.add(template) + await session.commit() + await session.refresh(template) + return template + + +async def test_janitor_marks_only_abandoned_builds_failed(template_provider): + settings = get_settings() + abandoned = await _create_template( + provider=template_provider.name, + status=TemplateStatus.BUILDING, + age=timedelta(minutes=settings.template_build_timeout_minutes + 1), + name="abandoned", + ) + fresh = await _create_template( + provider=template_provider.name, + status=TemplateStatus.BUILDING, + age=timedelta(minutes=settings.template_build_timeout_minutes - 1), + name="fresh", + ) + + await reap_templates() + + async with async_session_factory() as session: + abandoned = await session.get(Template, abandoned.id) + fresh = await session.get(Template, fresh.id) + + assert abandoned + assert abandoned.status == TemplateStatus.FAILED.value + assert abandoned.last_error == ( + f"build abandoned: exceeded {settings.template_build_timeout_minutes}-minute timeout" + ) + assert fresh + assert fresh.status == TemplateStatus.BUILDING.value + assert fresh.last_error == "" + + +async def test_janitor_reaps_only_failed_templates_past_retention(template_provider): + settings = get_settings() + expired = await _create_template( + provider=template_provider.name, + status=TemplateStatus.FAILED, + age=timedelta(hours=settings.template_failed_retention_hours + 1), + name="expired-failure", + ) + recent = await _create_template( + provider=template_provider.name, + status=TemplateStatus.FAILED, + age=timedelta(hours=settings.template_failed_retention_hours - 1), + name="recent-failure", + ) + + await reap_templates() + + async with async_session_factory() as session: + assert await session.get(Template, expired.id) is None + assert await session.get(Template, recent.id) is not None + + +async def test_janitor_reaps_unused_templates_by_last_use(template_provider): + settings = get_settings() + expired_age = timedelta(days=settings.template_unused_ttl_days + 1) + fresh_age = timedelta(days=settings.template_unused_ttl_days - 1) + never_used = await _create_template( + provider=template_provider.name, + status=TemplateStatus.AVAILABLE, + age=expired_age, + name="never-used", + handle="stub-template:never-used", + ) + used_long_ago = await _create_template( + provider=template_provider.name, + status=TemplateStatus.AVAILABLE, + age=expired_age + timedelta(days=1), + name="used-long-ago", + handle="stub-template:used-long-ago", + last_used_age=expired_age, + ) + recently_used = await _create_template( + provider=template_provider.name, + status=TemplateStatus.AVAILABLE, + age=expired_age + timedelta(days=1), + name="recently-used", + handle="stub-template:recently-used", + last_used_age=fresh_age, + ) + + await reap_templates() + + assert set(template_provider.deleted) == { + "stub-template:used-long-ago", + "stub-template:never-used", + } + async with async_session_factory() as session: + assert await session.get(Template, never_used.id) is None + assert await session.get(Template, used_long_ago.id) is None + assert await session.get(Template, recently_used.id) is not None + + +async def test_delete_spares_template_used_after_candidate_selection(template_provider): + settings = get_settings() + reap_before = utc_now() - timedelta(days=settings.template_unused_ttl_days) + template = await _create_template( + provider=template_provider.name, + status=TemplateStatus.AVAILABLE, + age=timedelta(days=settings.template_unused_ttl_days + 1), + name="leased-in-race", + handle="stub-template:leased-in-race", + ) + + async with async_session_factory() as session: + leased = await session.get(Template, template.id) + assert leased + leased.last_used_at = utc_now() + await session.commit() + + async with async_session_factory() as session: + deleted = await TemplateService(session).delete( + template.id, + reap_status=TemplateStatus.AVAILABLE, + reap_before=reap_before, + ) + + assert not deleted + assert template_provider.deleted == [] + async with async_session_factory() as session: + assert await session.get(Template, template.id) is not None + + +async def test_janitor_removes_row_when_provider_artifact_is_already_gone(template_provider): + settings = get_settings() + template_provider.delete_error = ProviderNotFoundError("already gone") + template = await _create_template( + provider=template_provider.name, + status=TemplateStatus.AVAILABLE, + age=timedelta(days=settings.template_unused_ttl_days + 1), + name="missing-artifact", + handle="stub-template:missing-artifact", + ) + + await reap_templates() + + async with async_session_factory() as session: + assert await session.get(Template, template.id) is None + + +async def test_janitor_keeps_transport_failure_and_continues(template_provider, monkeypatch): + settings = get_settings() + delete_template = AsyncMock(side_effect=[ProviderTransportError("offline"), None]) + monkeypatch.setattr(template_provider, "delete_template", delete_template) + failed_delete = await _create_template( + provider=template_provider.name, + status=TemplateStatus.AVAILABLE, + age=timedelta(days=settings.template_unused_ttl_days + 2), + name="failed-delete", + handle="stub-template:failed-delete", + ) + next_candidate = await _create_template( + provider=template_provider.name, + status=TemplateStatus.AVAILABLE, + age=timedelta(days=settings.template_unused_ttl_days + 1), + name="next-candidate", + handle="stub-template:next-candidate", + ) + + await reap_templates() + + assert delete_template.await_args_list[0].args == ("stub-template:failed-delete",) + assert delete_template.await_args_list[1].args == ("stub-template:next-candidate",) + async with async_session_factory() as session: + assert await session.get(Template, failed_delete.id) is not None + assert await session.get(Template, next_candidate.id) is None From 646bbbdf7db3c882f50d428ec8cd6454c51a07aa Mon Sep 17 00:00:00 2001 From: Paulo Date: Tue, 25 Aug 2026 07:40:30 +0200 Subject: [PATCH 5/8] Rename the template surface, plain-English docs, private-image pull auth MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit create_template replaces materialize_template on TemplateCapability and its three providers, and the DockerCLI attribute is docker_cli. The docs and comments this branch added are rewritten to ASD-STE100 shape: short sentences, active voice, one name per concept ("persistent" replaces "durable"; "template" replaces "artifact"). exe.dev assumes a public image, and templates land in a private registry, so create_vm now passes --registry-auth when the host image lives on the configured template registry — and only then, so the credentials never reach another registry (exe.dev docs: private-image). Co-Authored-By: Claude Fable 5 --- AGENTS.md | 4 +- docs/architecture.md | 46 +++++++------ docs/deploy.md | 6 +- src/hosts/schemas.py | 2 +- src/hosts/service.py | 2 +- src/providers/capabilities.py | 8 +-- src/providers/derived_image.py | 8 +-- src/providers/docker/provider.py | 2 +- src/providers/docker/tests/test_provider.py | 8 +-- src/providers/docker_sbx/provider.py | 12 ++-- .../docker_sbx/tests/test_diagnose.py | 2 +- .../docker_sbx/tests/test_provider.py | 14 ++-- src/providers/exe/api.py | 4 ++ src/providers/exe/provider.py | 37 +++++++--- src/providers/exe/settings.py | 5 +- src/providers/exe/tests/test_api.py | 17 +++++ src/providers/exe/tests/test_diagnose.py | 4 +- src/providers/exe/tests/test_provider.py | 68 +++++++++++++++---- src/templates/schemas.py | 2 +- src/templates/service.py | 4 +- src/templates/tests/conftest.py | 6 +- src/templates/tests/test_api.py | 8 +-- 22 files changed, 177 insertions(+), 92 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index fea0634..94c8207 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -10,7 +10,7 @@ a service, not a library. It owns: - Host records and lifecycle state in Postgres -- Template records, asynchronous builds, and provider artifact lifecycle +- Template records, async template builds, and their provider images - Inline provisioning in `POST /hosts` - Provider VM creation and deletion (exe.dev, AWS, Hetzner, Exoscale, local Docker, Docker Sandboxes) @@ -20,7 +20,7 @@ It owns: - Account-bound exe.dev HTTP proxy resources Periodic maintenance runs as cron jobs: `python -m hosts.janitor` reaps -expired hosts, `python -m templates.janitor` reaps abandoned and unused +expired hosts, `python -m templates.janitor` reaps abandoned, failed, and unused templates, and `python -m hosts.pool` tops up the warm pool. No backwards compatibility is required unless a caller contract is explicitly diff --git a/docs/architecture.md b/docs/architecture.md index 5423a68..9bcffdd 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -30,7 +30,7 @@ that true: hosts.api HTTP request/response concerns only hosts.service host lifecycle behavior (HostService) templates.api template request/response concerns only -templates.service template build and artifact lifecycle (TemplateService) +templates.service template build and delete behavior (TemplateService) providers/ one package per VM provider networking/ network provider framework + Tailscale adapter core/ settings, database, exception base @@ -73,14 +73,14 @@ the core settings knowing any provider exists. Not every provider supports every feature, and the host contract must not grow provider-shaped warts. Optional features are capability -mix-ins: `HttpProxyCapability` declares the http-proxy surface, while -`TemplateCapability` owns provider-specific template materialization -and teardown. `resolve_capability` narrows a specific provider instance +mix-ins: `HttpProxyCapability` declares the http-proxy surface, and +`TemplateCapability` declares the template create and delete surface. +`resolve_capability` narrows a specific provider instance to a capability — the default provider for account-bound operations, the host's own provider for host-bound ones — and raises the shared -`CapabilityUnsupportedError` when that provider doesn't implement it, +`CapabilityUnsupportedError` when that provider does not implement it, which the routes surface as a clear error. New provider-specific -features should follow this pattern rather than widening `VMProvider` +features must follow this pattern rather than widening `VMProvider` or the host schema. The review question that guards the whole design: *does this change @@ -99,17 +99,17 @@ successful key returns the original host instead of a duplicate. Caller `env` is stored for provisioning and never returned by the API; keys in `hosts.schemas.RESERVED_HOST_ENV_KEYS` are rejected. -Templates are durable provider artifacts keyed by provider, base image, +A template is a persistent provider image keyed by provider, base image, and setup-script hash. `POST /templates` creates a `building` record and -returns `202 Accepted`; callers poll until it becomes `available` or -`failed`. Templates outlive individual hosts, and provider-specific -materialization stays behind `TemplateCapability`. +returns `202 Accepted`. Callers poll until the template becomes +`available` or `failed`. Templates outlive hosts. Each provider builds +and deletes its own templates behind `TemplateCapability`. -An available template can be requested by its ID or requirements hash. The -template's provider handle becomes the host image; an explicit `image` takes -precedence, followed by the template, then the provider default. Host creation -never builds a missing or unavailable template: it returns a clear client error -so the caller retains control of the asynchronous build lifecycle. +A host request can name an available template by its ID or requirements +hash. The template's handle becomes the host image. An explicit `image` +wins over the template, and the template wins over the provider default. +Host creation never builds a missing or unavailable template. It returns +a client error, and the caller decides when to build. Every host is a renewable lease. A create without `expires_at` gets `now + LEASE_DEFAULT_TTL`, so a host whose owner disappears lapses and @@ -121,12 +121,16 @@ hosts renew — unclaimed warm-pool members belong to pool maintenance and refuse with `409`. Three maintenance commands run as cron jobs from the same image: -`hosts.janitor` reaps expired and orphaned hosts, `templates.janitor` -fails abandoned builds and reaps failed or unused artifacts, and -`hosts.pool` keeps a warm pool of pre-provisioned hosts per provider -(`POOL_SIZES`, with `POOL_SIZE` as the default provider's target) to hide -provider cold starts. Editing a template setup script changes its hash; -the superseded artifact then ages out after its last lease. Pool members + +- `hosts.janitor` reaps expired and orphaned hosts. +- `templates.janitor` marks abandoned builds `failed` and deletes failed + or unused templates. +- `hosts.pool` keeps a warm pool of pre-provisioned hosts per provider + (`POOL_SIZES`, with `POOL_SIZE` as the default provider's target) to + hide provider cold starts. + +When you edit a template setup script, the hash changes. The old +template ages out after its last lease. Pool members are warmed with the provider's default image and size, so a request that customizes its host — `image`, `env`, `template`, `instance_type`, or `disk_gb` — always provisions fresh instead of claiming a warm host. diff --git a/docs/deploy.md b/docs/deploy.md index 6f44950..857b798 100644 --- a/docs/deploy.md +++ b/docs/deploy.md @@ -26,8 +26,8 @@ docker run --rm --env-file drukbox.env "$IMAGE" .venv/bin/python -m templates.ja ``` The host janitor reaps expired and orphaned hosts. The template janitor -fails abandoned builds, retains their diagnostics for a bounded window, -and reaps failed or unused provider artifacts. The pool maintainer +marks abandoned builds failed, keeps failed builds for diagnosis, and +deletes failed or unused templates. The pool maintainer pre-provisions warm hosts per provider and only does anything when at least one provider has a warm target (`POOL_SIZES` / `POOL_SIZE`). Schedule all three under your cron infrastructure (k8s `CronJob`, @@ -325,7 +325,7 @@ exe.dev provider: | --- | --- | --- | | `EXE_API_TOKEN` | — (required) | Bearer token for the exe.dev exec API. | | `EXE_DEFAULT_IMAGE` | — (required) | Image used when the caller omits `image`. | -| `EXE_TEMPLATE_REGISTRY` | — | Repository prefix for derived template images that exe.dev can pull. | +| `EXE_TEMPLATE_REGISTRY` | — | Repository prefix for derived template images. A VM created from this registry gets `--registry-auth` so exe.dev can pull a private image. | | `EXE_REGISTRY_USERNAME` | — | Username for the derived-template image registry. | | `EXE_REGISTRY_PASSWORD` | — | Password or token for the derived-template image registry. | | `EXE_API_URL` | `https://exe.dev` | API base URL. | diff --git a/src/hosts/schemas.py b/src/hosts/schemas.py index 1490558..937a24e 100644 --- a/src/hosts/schemas.py +++ b/src/hosts/schemas.py @@ -22,7 +22,7 @@ class HostCreate(BaseModel): image: str | None = None template: str | None = Field( default=None, - description="Template ID or requirements hash. Used only when image is omitted.", + description="Template ID or requirements hash. Used only when the request has no image.", ) env: dict[str, str] = Field(default_factory=dict) expires_at: datetime | None = None diff --git a/src/hosts/service.py b/src/hosts/service.py index 6dc3d20..ee6d175 100644 --- a/src/hosts/service.py +++ b/src/hosts/service.py @@ -120,7 +120,7 @@ async def get_or_create_host( host: Host | None = None # Warm hosts are provider-specific, so the claim is scoped to the # requested provider's pool. A request is pool-eligible only when it - # doesn't customize the host: no image, template, env, or per-request + # does not customize the host: no image, template, env, or per-request # sizing — pool members are warmed at the provider's defaults. requested_provider = provider or self.settings.default_host_provider customized = env or image or template or instance_type or disk_gb diff --git a/src/providers/capabilities.py b/src/providers/capabilities.py index 7a2d98e..cdcb8ae 100644 --- a/src/providers/capabilities.py +++ b/src/providers/capabilities.py @@ -44,14 +44,14 @@ async def detach_http_proxy(self, name: str, *, attach_vm: str) -> None: ... class TemplateCapability(abc.ABC): - """Mix-in declaring a VMProvider can materialize reusable templates. + """Mix-in declaring a VMProvider can create and delete templates. - Modeled as an ABC so resolve_capability checks explicit provider support, - rather than accepting any object that happens to expose these method names. + Modeled as an ABC so resolve_capability checks the inheritance chain + instead of accepting any object that has these method names. """ @abc.abstractmethod - async def materialize_template( + async def create_template( self, *, base_image: str, diff --git a/src/providers/derived_image.py b/src/providers/derived_image.py index 78ee736..69a77a7 100644 --- a/src/providers/derived_image.py +++ b/src/providers/derived_image.py @@ -35,7 +35,7 @@ def derived_image_context(*, base_image: str, setup_script: str) -> Iterator[Pat async def build_derived_image( - docker: DockerCLI, + docker_cli: DockerCLI, *, base_image: str, setup_script: str, @@ -48,15 +48,15 @@ async def build_derived_image( ) try: with derived_image_context(base_image=base_image, setup_script=setup_script) as context: - await docker.build_image(tag, context) + await docker_cli.build_image(tag, context) except DockerProviderError as exc: raise ProviderTransportError(str(exc)) from exc return tag -async def remove_derived_image(docker: DockerCLI, handle: str) -> None: +async def remove_derived_image(docker_cli: DockerCLI, handle: str) -> None: try: - await docker.remove_image(handle) + await docker_cli.remove_image(handle) except DockerImageNotFoundError as exc: raise ProviderNotFoundError(f"docker image '{handle}' was not found") from exc except DockerProviderError as exc: diff --git a/src/providers/docker/provider.py b/src/providers/docker/provider.py index 1fb2e58..df12e52 100644 --- a/src/providers/docker/provider.py +++ b/src/providers/docker/provider.py @@ -129,7 +129,7 @@ async def delete_vm(self, name: str) -> None: except DockerProviderError as exc: raise ProviderTransportError(str(exc)) from exc - async def materialize_template( + async def create_template( self, *, base_image: str, diff --git a/src/providers/docker/tests/test_provider.py b/src/providers/docker/tests/test_provider.py index c762e8a..4b9374e 100644 --- a/src/providers/docker/tests/test_provider.py +++ b/src/providers/docker/tests/test_provider.py @@ -136,11 +136,11 @@ async def test_delete_vm_raises_not_found_when_container_missing(): @pytest.mark.asyncio -async def test_materialize_template_builds_and_returns_the_derived_tag(): +async def test_create_template_builds_and_returns_the_derived_tag(): api = _api_mock() provider = DockerProvider(api, _settings()) - handle = await provider.materialize_template( + handle = await provider.create_template( base_image="sandbox:base", setup_script="apt-get update", label="Node tools", @@ -152,13 +152,13 @@ async def test_materialize_template_builds_and_returns_the_derived_tag(): @pytest.mark.asyncio -async def test_materialize_template_translates_build_failure(): +async def test_create_template_translates_build_failure(): api = _api_mock() api.build_image.side_effect = DockerTransportError("build log tail") provider = DockerProvider(api, _settings()) with pytest.raises(ProviderTransportError, match="build log tail"): - await provider.materialize_template( + await provider.create_template( base_image="sandbox:base", setup_script="apt-get update", label="Node tools", diff --git a/src/providers/docker_sbx/provider.py b/src/providers/docker_sbx/provider.py index f417206..b4ed0b7 100644 --- a/src/providers/docker_sbx/provider.py +++ b/src/providers/docker_sbx/provider.py @@ -62,18 +62,18 @@ def __init__( api: SbxCLI, settings: DockerSbxSettings, *, - docker: DockerCLI, + docker_cli: DockerCLI, ) -> None: self.api = api self.settings = settings - self.docker = docker + self.docker_cli = docker_cli @classmethod def from_settings(cls) -> Self: return cls( SbxCLI(), DockerSbxSettings(), # pyright: ignore[reportCallIssue] - docker=DockerCLI(), + docker_cli=DockerCLI(), ) @property @@ -179,7 +179,7 @@ async def delete_vm(self, name: str) -> None: self._remove_workspace(name) - async def materialize_template( + async def create_template( self, *, base_image: str, @@ -187,13 +187,13 @@ async def materialize_template( label: str, ) -> str: return await build_derived_image( - self.docker, + self.docker_cli, base_image=base_image, setup_script=setup_script, ) async def delete_template(self, handle: str) -> None: - await remove_derived_image(self.docker, handle) + await remove_derived_image(self.docker_cli, handle) async def diagnose(self) -> str: # The sandbox list is one fast check of the CLI, the daemon diff --git a/src/providers/docker_sbx/tests/test_diagnose.py b/src/providers/docker_sbx/tests/test_diagnose.py index 9228d32..71d5ec1 100644 --- a/src/providers/docker_sbx/tests/test_diagnose.py +++ b/src/providers/docker_sbx/tests/test_diagnose.py @@ -8,7 +8,7 @@ def _provider(api: MagicMock) -> DockerSbxProvider: - return DockerSbxProvider(api, DockerSbxSettings(), docker=MagicMock()) + return DockerSbxProvider(api, DockerSbxSettings(), docker_cli=MagicMock()) @pytest.mark.asyncio diff --git a/src/providers/docker_sbx/tests/test_provider.py b/src/providers/docker_sbx/tests/test_provider.py index 9fdf8ac..c2872ec 100644 --- a/src/providers/docker_sbx/tests/test_provider.py +++ b/src/providers/docker_sbx/tests/test_provider.py @@ -42,9 +42,9 @@ def _provider( api: MagicMock, settings: DockerSbxSettings, *, - docker: MagicMock | None = None, + docker_cli: MagicMock | None = None, ) -> DockerSbxProvider: - return DockerSbxProvider(api, settings, docker=docker or _docker_mock()) + return DockerSbxProvider(api, settings, docker_cli=docker_cli or _docker_mock()) @pytest.mark.asyncio @@ -213,12 +213,12 @@ async def test_delete_vm_keeps_the_workspace_when_teardown_fails(tmp_path): @pytest.mark.asyncio -async def test_materialize_template_builds_and_returns_the_local_image_tag(tmp_path): +async def test_create_template_builds_and_returns_the_local_image_tag(tmp_path): api = _api_mock() docker = _docker_mock() - provider = _provider(api, _settings(tmp_path), docker=docker) + provider = _provider(api, _settings(tmp_path), docker_cli=docker) - handle = await provider.materialize_template( + handle = await provider.create_template( base_image="sandbox:base", setup_script="apt-get update", label="Node tools", @@ -232,7 +232,7 @@ async def test_materialize_template_builds_and_returns_the_local_image_tag(tmp_p async def test_delete_template_removes_the_local_image(tmp_path): api = _api_mock() docker = _docker_mock() - provider = _provider(api, _settings(tmp_path), docker=docker) + provider = _provider(api, _settings(tmp_path), docker_cli=docker) await provider.delete_template("drukbox-template:123456789abc") @@ -244,7 +244,7 @@ async def test_delete_template_translates_a_missing_image(tmp_path): api = _api_mock() docker = _docker_mock() docker.remove_image.side_effect = DockerImageNotFoundError("No such image") - provider = _provider(api, _settings(tmp_path), docker=docker) + provider = _provider(api, _settings(tmp_path), docker_cli=docker) with pytest.raises(ProviderNotFoundError, match="was not found"): await provider.delete_template("drukbox-template:missing") diff --git a/src/providers/exe/api.py b/src/providers/exe/api.py index 3acba77..4cbcdaa 100644 --- a/src/providers/exe/api.py +++ b/src/providers/exe/api.py @@ -75,6 +75,7 @@ async def create_vm( setup_script: str | None = None, tags: list[str] | None = None, no_email: bool = True, + registry_auth: str | None = None, ) -> dict[str, Any]: command_parts = ["new", "--json"] @@ -86,6 +87,9 @@ async def create_vm( if vm_image: command_parts.append(f"--image={shlex.quote(vm_image)}") + if registry_auth: + command_parts.append(f"--registry-auth={shlex.quote(registry_auth)}") + if setup_script: setup_script = inject_env_exports(setup_script, env) command_parts.append(f"--setup-script={_encode_setup_script(setup_script)}") diff --git a/src/providers/exe/provider.py b/src/providers/exe/provider.py index aad92de..5e85d5a 100644 --- a/src/providers/exe/provider.py +++ b/src/providers/exe/provider.py @@ -32,12 +32,12 @@ def __init__( api: ExeAPI, settings: ExeSettings, *, - docker: DockerCLI, + docker_cli: DockerCLI, service_label: str = "drukbox", ) -> None: self.api = api self.settings = settings - self.docker = docker + self.docker_cli = docker_cli self._service_label = service_label @classmethod @@ -46,7 +46,7 @@ def from_settings(cls) -> Self: return cls( ExeAPI.from_settings(), ExeSettings(), # pyright: ignore[reportCallIssue] - docker=DockerCLI(), + docker_cli=DockerCLI(), service_label=core.service_label, ) @@ -68,6 +68,20 @@ async def create_vm( instance_type: str | None = None, disk_gb: int | None = None, ) -> VMCreateResult: + # exe.dev assumes a public image. A template pushed to the configured + # registry is private, so its pull gets --registry-auth (see + # https://exe.dev/docs/private-image). The credentials go only to + # the registry that they belong to. + registry_auth = None + registry = self.settings.template_registry + if ( + registry + and self.settings.registry_username + and self.settings.registry_password + and image.partition("/")[0] == registry.partition("/")[0] + ): + registry_auth = f"{self.settings.registry_username}:{self.settings.registry_password}" + # Tags are operator-facing: `exe ls --tag=managed-by-` shows what this deployment owns. payload = await self.api.create_vm( name=name, @@ -75,6 +89,7 @@ async def create_vm( env=env, setup_script=setup_script, tags=[f"managed-by-{self._service_label}"], + registry_auth=registry_auth, ) return VMCreateResult( provider_id=str(payload["vm_name"]), @@ -93,7 +108,7 @@ async def delete_vm(self, name: str) -> None: except ExeVMNotFoundError as exc: raise ProviderNotFoundError(str(exc)) from exc - async def materialize_template( + async def create_template( self, *, base_image: str, @@ -115,27 +130,27 @@ async def materialize_template( if not value ] raise ProviderCommandError( - f"exe template registry is not configured; missing settings: " - f"{', '.join(missing_settings)}" + f"exe template registry is not configured. Set: {', '.join(missing_settings)}" ) tag = await build_derived_image( - self.docker, + self.docker_cli, base_image=base_image, setup_script=setup_script, repository=registry, ) registry_host, *_ = registry.partition("/") try: - await self.docker.login(registry_host, username, password) - await self.docker.push_image(tag) + await self.docker_cli.login(registry_host, username, password) + await self.docker_cli.push_image(tag) except DockerProviderError as exc: raise ProviderTransportError(str(exc)) from exc return tag async def delete_template(self, handle: str) -> None: - # Registry deletion is registry-specific; this provider only owns the local build tag. - await remove_derived_image(self.docker, handle) + # Registry deletion is registry-specific. This provider only removes + # the local build tag. + await remove_derived_image(self.docker_cli, handle) async def aclose(self) -> None: await self.api.aclose() diff --git a/src/providers/exe/settings.py b/src/providers/exe/settings.py index e089e2c..99cf77d 100644 --- a/src/providers/exe/settings.py +++ b/src/providers/exe/settings.py @@ -24,7 +24,10 @@ class ExeSettings(BaseSettings): ) template_registry: str | None = Field( default=None, - description="Repository prefix for derived template images that exe.dev can pull.", + description=( + "Repository prefix for derived template images. A VM created from " + "this registry gets --registry-auth so exe.dev can pull a private image." + ), ) registry_username: str | None = Field( default=None, diff --git a/src/providers/exe/tests/test_api.py b/src/providers/exe/tests/test_api.py index cc9b850..55e2e6c 100644 --- a/src/providers/exe/tests/test_api.py +++ b/src/providers/exe/tests/test_api.py @@ -114,6 +114,23 @@ async def test_create_vm_emits_tag_flag_for_each_tag(respx_mock): assert "--tag=managed-by-drukbox-prod" in body +@pytest.mark.asyncio +@respx.mock(base_url="https://exe.dev") +async def test_create_vm_emits_registry_auth_flag(respx_mock): + route = respx_mock.post("/exec").mock( + return_value=httpx.Response(200, content=b'{"vm_name": "sb-1", "ssh_port": 22}'), + ) + + await _api().create_vm( + name="sb-1", + image="ghcr.io/acme/templates:abc123", + registry_auth="bot:secret", + ) + + body = route.calls.last.request.content.decode() + assert "--registry-auth=bot:secret" in body + + @pytest.mark.asyncio @respx.mock(base_url="https://exe.dev") async def test_create_vm_omits_setup_script_when_none(respx_mock): diff --git a/src/providers/exe/tests/test_diagnose.py b/src/providers/exe/tests/test_diagnose.py index e7dbf20..34bbcdc 100644 --- a/src/providers/exe/tests/test_diagnose.py +++ b/src/providers/exe/tests/test_diagnose.py @@ -15,7 +15,7 @@ async def test_diagnose_returns_email_from_whoami() -> None: """The probe surfaces the authenticated identity directly from whoami.""" api = MagicMock() api.whoami = AsyncMock(return_value={"email": "ops@example.com"}) - provider = ExeProvider(api, _settings(), docker=MagicMock()) + provider = ExeProvider(api, _settings(), docker_cli=MagicMock()) assert await provider.diagnose() == "ops@example.com" @@ -25,7 +25,7 @@ async def test_diagnose_raises_on_whoami_failure() -> None: """A whoami error surfaces so the orchestrator can classify it.""" api = MagicMock() api.whoami = AsyncMock(side_effect=RuntimeError("403")) - provider = ExeProvider(api, _settings(), docker=MagicMock()) + provider = ExeProvider(api, _settings(), docker_cli=MagicMock()) with pytest.raises(RuntimeError, match="403"): await provider.diagnose() diff --git a/src/providers/exe/tests/test_provider.py b/src/providers/exe/tests/test_provider.py index a83bfea..05418d1 100644 --- a/src/providers/exe/tests/test_provider.py +++ b/src/providers/exe/tests/test_provider.py @@ -28,7 +28,7 @@ def _docker_mock() -> SimpleNamespace: def _make_provider(api: object) -> ExeProvider: - return ExeProvider(api, _settings(), docker=_docker_mock()) # type: ignore[arg-type] + return ExeProvider(api, _settings(), docker_cli=_docker_mock()) # type: ignore[arg-type] async def test_create_vm_forwards_kwargs_and_maps_result() -> None: @@ -57,6 +57,7 @@ async def test_create_vm_forwards_kwargs_and_maps_result() -> None: env={"K": "V"}, setup_script="#!/bin/bash\necho hello", tags=["managed-by-drukbox"], + registry_auth=None, ) assert result.provider_id == "sb-1234" assert result.name == "sb-1234" @@ -64,6 +65,47 @@ async def test_create_vm_forwards_kwargs_and_maps_result() -> None: assert result.ssh_host == "sb-1234.public.exe.dev" +def _vm_payload() -> dict[str, str]: + return {"vm_name": "sb-1", "ssh_port": "22", "ssh_dest": "sb-1.public.exe.dev"} + + +async def test_create_vm_sends_registry_auth_for_template_registry_images() -> None: + api = SimpleNamespace(create_vm=AsyncMock(return_value=_vm_payload())) + settings = _settings( + template_registry="ghcr.io/acme/templates", + registry_username="bot", + registry_password="secret", + ) + provider = ExeProvider(api, settings, docker_cli=_docker_mock()) # type: ignore[arg-type] + + await provider.create_vm(name="sb-1", image="ghcr.io/acme/templates:abc123") + + assert api.create_vm.await_args.kwargs["registry_auth"] == "bot:secret" + + +async def test_create_vm_keeps_credentials_off_other_registries() -> None: + api = SimpleNamespace(create_vm=AsyncMock(return_value=_vm_payload())) + settings = _settings( + template_registry="ghcr.io/acme/templates", + registry_username="bot", + registry_password="secret", + ) + provider = ExeProvider(api, settings, docker_cli=_docker_mock()) # type: ignore[arg-type] + + await provider.create_vm(name="sb-1", image="docker.io/library/ubuntu:24.04") + + assert api.create_vm.await_args.kwargs["registry_auth"] is None + + +async def test_create_vm_omits_registry_auth_when_registry_is_not_configured() -> None: + api = SimpleNamespace(create_vm=AsyncMock(return_value=_vm_payload())) + provider = _make_provider(api) + + await provider.create_vm(name="sb-1", image="img:latest") + + assert api.create_vm.await_args.kwargs["registry_auth"] is None + + async def test_delete_vm_delegates_to_api() -> None: api = SimpleNamespace(delete_vm=AsyncMock()) provider = _make_provider(api) @@ -118,10 +160,10 @@ async def test_http_proxy_methods_delegate_to_api(method_name: str, kwargs: dict def test_from_settings_constructs_with_exeapi() -> None: provider = ExeProvider.from_settings() assert isinstance(provider.api, ExeAPI) - assert isinstance(provider.docker, DockerCLI) + assert isinstance(provider.docker_cli, DockerCLI) -async def test_materialize_template_builds_logs_in_and_pushes() -> None: +async def test_create_template_builds_logs_in_and_pushes() -> None: docker = _docker_mock() provider = ExeProvider( SimpleNamespace(), # type: ignore[arg-type] @@ -130,10 +172,10 @@ async def test_materialize_template_builds_logs_in_and_pushes() -> None: registry_username="builder", registry_password="registry-secret", ), - docker=docker, # type: ignore[arg-type] + docker_cli=docker, # type: ignore[arg-type] ) - handle = await provider.materialize_template( + handle = await provider.create_template( base_image="exe/base:latest", setup_script="apt-get update", label="Node tools", @@ -146,16 +188,16 @@ async def test_materialize_template_builds_logs_in_and_pushes() -> None: docker.push_image.assert_awaited_once_with(handle) -async def test_materialize_template_names_each_missing_registry_setting() -> None: +async def test_create_template_names_each_missing_registry_setting() -> None: docker = _docker_mock() provider = ExeProvider( SimpleNamespace(), # type: ignore[arg-type] _settings(), - docker=docker, # type: ignore[arg-type] + docker_cli=docker, # type: ignore[arg-type] ) with pytest.raises(ProviderCommandError) as error: - await provider.materialize_template( + await provider.create_template( base_image="exe/base:latest", setup_script="apt-get update", label="Node tools", @@ -167,7 +209,7 @@ async def test_materialize_template_names_each_missing_registry_setting() -> Non docker.build_image.assert_not_awaited() -async def test_materialize_template_translates_push_failure() -> None: +async def test_create_template_translates_push_failure() -> None: docker = _docker_mock() docker.push_image.side_effect = DockerTransportError("push log tail") provider = ExeProvider( @@ -177,11 +219,11 @@ async def test_materialize_template_translates_push_failure() -> None: registry_username="builder", registry_password="registry-secret", ), - docker=docker, # type: ignore[arg-type] + docker_cli=docker, # type: ignore[arg-type] ) with pytest.raises(ProviderTransportError, match="push log tail"): - await provider.materialize_template( + await provider.create_template( base_image="exe/base:latest", setup_script="apt-get update", label="Node tools", @@ -193,7 +235,7 @@ async def test_delete_template_removes_the_local_image() -> None: provider = ExeProvider( SimpleNamespace(), # type: ignore[arg-type] _settings(), - docker=docker, # type: ignore[arg-type] + docker_cli=docker, # type: ignore[arg-type] ) await provider.delete_template("ghcr.io/acme/drukbox-templates:123456789abc") @@ -207,7 +249,7 @@ async def test_delete_template_translates_a_missing_local_image() -> None: provider = ExeProvider( SimpleNamespace(), # type: ignore[arg-type] _settings(), - docker=docker, # type: ignore[arg-type] + docker_cli=docker, # type: ignore[arg-type] ) with pytest.raises(ProviderNotFoundError, match="was not found"): diff --git a/src/templates/schemas.py b/src/templates/schemas.py index 480e42d..446bdf3 100644 --- a/src/templates/schemas.py +++ b/src/templates/schemas.py @@ -7,7 +7,7 @@ class TemplateCreate(BaseModel): provider: str | None = Field( default=None, - description="VM provider to materialize on. Omit to use the service default.", + description="VM provider that builds the template. Omit to use the service default.", ) base_image: str | None = Field( default=None, diff --git a/src/templates/service.py b/src/templates/service.py index 0d26178..d1a572d 100644 --- a/src/templates/service.py +++ b/src/templates/service.py @@ -89,7 +89,7 @@ async def build(self, template_id: uuid.UUID) -> None: get_vm_provider(template.provider), TemplateCapability, ) - handle = await capability.materialize_template( + handle = await capability.create_template( base_image=template.base_image, setup_script=template.setup_script, label=template.label, @@ -124,7 +124,7 @@ async def delete( reap_status: TemplateStatus | None = None, reap_before: datetime | None = None, ) -> bool: - """Delete the template; return False when the maintenance guard spares it.""" + """Delete the template. Return False when the maintenance guard spares it.""" result = await self.session.execute( select(Template).where(Template.id == template_id).with_for_update() ) diff --git a/src/templates/tests/conftest.py b/src/templates/tests/conftest.py index bff631e..78af52a 100644 --- a/src/templates/tests/conftest.py +++ b/src/templates/tests/conftest.py @@ -14,7 +14,7 @@ class StubTemplateProvider(TemplateCapability, VMProvider): diagnose_hint = "check_template_stub" def __init__(self) -> None: - self.materialized: list[tuple[str, str, str]] = [] + self.created: list[tuple[str, str, str]] = [] self.deleted: list[str] = [] self.build_error: Exception | None = None self.delete_error: ProviderError | None = None @@ -52,14 +52,14 @@ async def diagnose(self) -> str: async def aclose(self) -> None: return - async def materialize_template( + async def create_template( self, *, base_image: str, setup_script: str, label: str, ) -> str: - self.materialized.append((base_image, setup_script, label)) + self.created.append((base_image, setup_script, label)) if self.build_error: raise self.build_error return derived_image_tag(base_image=base_image, setup_script=setup_script) diff --git a/src/templates/tests/test_api.py b/src/templates/tests/test_api.py index 528113b..9ecb94f 100644 --- a/src/templates/tests/test_api.py +++ b/src/templates/tests/test_api.py @@ -22,7 +22,7 @@ SETUP_SCRIPT = "apt-get update && apt-get install -y nodejs" -async def test_create_template_returns_building_then_materializes(client, template_provider): +async def test_create_template_returns_building_then_becomes_available(client, template_provider): """Create returns the pollable building record before the background build result.""" response = await client.post( "/templates", @@ -56,7 +56,7 @@ async def test_create_template_returns_building_then_materializes(client, templa setup_script=SETUP_SCRIPT, ) assert "setup_script" not in polled.json() - assert template_provider.materialized == [ + assert template_provider.created == [ (template_provider.default_image, SETUP_SCRIPT, "Node tools") ] @@ -144,7 +144,7 @@ async def test_duplicate_create_returns_existing_without_rebuilding(client, temp assert second.json()["id"] == first.json()["id"] assert second.json()["status"] == TemplateStatus.AVAILABLE.value assert second.json()["label"] == "first label" - assert template_provider.materialized == [("stub:custom", SETUP_SCRIPT, "first label")] + assert template_provider.created == [("stub:custom", SETUP_SCRIPT, "first label")] async def test_concurrent_creates_resolve_unique_index_race(template_provider): @@ -332,7 +332,7 @@ async def test_create_template_rejects_blank_script(client, template_provider): assert response.status_code == 422 assert response.json()["detail"][0]["loc"] == ["body", "setup_script"] - assert template_provider.materialized == [] + assert template_provider.created == [] async def create_template_record( From f5f6d4f7164457904ff2b358f12dbd9e0c5fabb9 Mon Sep 17 00:00:00 2001 From: Paulo Date: Tue, 25 Aug 2026 08:26:56 +0200 Subject: [PATCH 6/8] HostCreate.template is a template ID, not a hash union MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The requirements hash is script-only, so one hash can match two base images on the same provider — hash leasing had to guess with a newest-available tie-break. The guess is gone: template is a UUID, the lookup is exact, and the wire boundary validates the type. Callers do not lose content addressing — POST /templates is idempotent by content and always returns the current record with its ID. Co-Authored-By: Claude Fable 5 --- docs/architecture.md | 6 +- src/hosts/schemas.py | 6 +- src/hosts/service.py | 32 +++-------- src/hosts/tests/test_templates.py | 92 +++---------------------------- 4 files changed, 23 insertions(+), 113 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index 9bcffdd..c78c8c7 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -105,8 +105,10 @@ returns `202 Accepted`. Callers poll until the template becomes `available` or `failed`. Templates outlive hosts. Each provider builds and deletes its own templates behind `TemplateCapability`. -A host request can name an available template by its ID or requirements -hash. The template's handle becomes the host image. An explicit `image` +A host request can name an available template by its ID. The `POST +/templates` create is idempotent by content, so the caller can always +repost the same declaration to get the current ID back. +The template's handle becomes the host image. An explicit `image` wins over the template, and the template wins over the provider default. Host creation never builds a missing or unavailable template. It returns a client error, and the caller decides when to build. diff --git a/src/hosts/schemas.py b/src/hosts/schemas.py index 937a24e..15eb634 100644 --- a/src/hosts/schemas.py +++ b/src/hosts/schemas.py @@ -20,9 +20,9 @@ def _expires_at_must_be_future_and_tz_aware(expires_at: datetime | None) -> date class HostCreate(BaseModel): image: str | None = None - template: str | None = Field( + template: uuid.UUID | None = Field( default=None, - description="Template ID or requirements hash. Used only when the request has no image.", + description="Template ID to fork from. Used only when the request has no image.", ) env: dict[str, str] = Field(default_factory=dict) expires_at: datetime | None = None @@ -43,7 +43,7 @@ class HostCreate(BaseModel): description="Root disk size in GB. Omit to use the provider's configured default.", ) - @field_validator("image", "template", "instance_type") + @field_validator("image", "instance_type") @classmethod def reject_blank(cls, value: str | None, info: ValidationInfo) -> str | None: if value is not None and not value.strip(): diff --git a/src/hosts/service.py b/src/hosts/service.py index ee6d175..bc5959b 100644 --- a/src/hosts/service.py +++ b/src/hosts/service.py @@ -94,7 +94,7 @@ async def get_or_create_host( *, env: dict[str, str], image: str | None, - template: str | None = None, + template: uuid.UUID | None = None, expires_at: datetime | None | EllipsisType = ..., idempotency_key: str | None = None, provider: str | None = None, @@ -203,7 +203,7 @@ async def create_host( *, env: dict[str, str], image: str | None, - template: str | None = None, + template: uuid.UUID | None = None, expires_at: datetime | None | EllipsisType = ..., provider: str | None = None, instance_type: str | None = None, @@ -223,7 +223,7 @@ async def create_host( f"provider {vm.name!r} does not support a per-request disk_gb" ) if template and not image: - image = await self._resolve_template_image(reference=template, provider=vm.name) + image = await self._resolve_template_image(template_id=template, provider=vm.name) uid = uuid7() name = Host.build_name(uid) now = utc_now() @@ -282,31 +282,15 @@ async def create_host( await self.session.refresh(host) return host - async def _resolve_template_image(self, *, reference: str, provider: str) -> str: - try: - template_id = uuid.UUID(reference) - except ValueError: - result = await self.session.execute( - select(Template) - .where(Template.provider == provider) - .where(Template.requirements_hash == reference) - .order_by( - (Template.status == TemplateStatus.AVAILABLE.value).desc(), - Template.created_at.desc(), - ) - .limit(1) - ) - else: - result = await self.session.execute( - select(Template) - .where(Template.id == template_id) - .where(Template.provider == provider) - ) + async def _resolve_template_image(self, *, template_id: uuid.UUID, provider: str) -> str: + result = await self.session.execute( + select(Template).where(Template.id == template_id).where(Template.provider == provider) + ) template = result.scalar_one_or_none() if not template: raise TemplateReferenceError( - f"template {reference!r} not found for provider {provider!r}" + f"template {template_id} not found for provider {provider!r}" ) if template.status != TemplateStatus.AVAILABLE.value: diff --git a/src/hosts/tests/test_templates.py b/src/hosts/tests/test_templates.py index 73151b0..75dd064 100644 --- a/src/hosts/tests/test_templates.py +++ b/src/hosts/tests/test_templates.py @@ -73,57 +73,6 @@ async def test_create_host_resolves_template_id(client, monkeypatch): assert before <= used_template.last_used_at <= after -async def test_create_host_resolves_template_requirements_hash(client, monkeypatch): - """An available requirements hash becomes the stored image and records its use.""" - template = await create_template_record(handle="derived:image-by-hash") - monkeypatch.setattr("hosts.service.HostService.provision", AsyncMock()) - - response = await client.post( - "/hosts", - headers=AUTH_HEADERS, - json={"template": template.requirements_hash}, - ) - - assert response.status_code == 201 - assert response.json()["image"] == "derived:image-by-hash" - async with async_session_factory() as session: - used_template = await session.get(Template, template.id) - assert used_template is not None - assert used_template.last_used_at is not None - - -async def test_create_host_prefers_available_template_for_requirements_hash(client, monkeypatch): - """An available hash match wins over a newer unavailable base-image variant.""" - now = utc_now() - available = await create_template_record( - base_image="base:available", - handle="derived:available", - created_at=now, - ) - building = await create_template_record( - base_image="base:building", - status=TemplateStatus.BUILDING.value, - created_at=now + timedelta(seconds=1), - ) - monkeypatch.setattr("hosts.service.HostService.provision", AsyncMock()) - - response = await client.post( - "/hosts", - headers=AUTH_HEADERS, - json={"template": REQUIREMENTS_HASH}, - ) - - assert response.status_code == 201 - assert response.json()["image"] == "derived:available" - async with async_session_factory() as session: - used_available = await session.get(Template, available.id) - untouched_building = await session.get(Template, building.id) - assert used_available is not None - assert used_available.last_used_at is not None - assert untouched_building is not None - assert untouched_building.last_used_at is None - - async def test_create_host_rejects_building_template(client): """A building template returns a conflict naming its current status.""" template = await create_template_record(status=TemplateStatus.BUILDING.value) @@ -162,43 +111,18 @@ async def test_create_host_rejects_failed_template_with_last_error(client): assert "ProviderTransportError: builder unavailable" in response.json()["detail"] -async def test_create_host_reports_newest_unavailable_hash_match(client): - """An unavailable hash reports the newest matching base-image variant.""" - now = utc_now() - await create_template_record( - base_image="base:older-building", - status=TemplateStatus.BUILDING.value, - created_at=now, - ) - newest = await create_template_record( - base_image="base:newer-failed", - status=TemplateStatus.FAILED.value, - last_error="ProviderTransportError: newest failure", - created_at=now + timedelta(seconds=1), - ) - - response = await client.post( - "/hosts", - headers=AUTH_HEADERS, - json={"template": REQUIREMENTS_HASH}, - ) - - assert response.status_code == 409 - assert str(newest.id) in response.json()["detail"] - assert TemplateStatus.FAILED.value in response.json()["detail"] - assert "ProviderTransportError: newest failure" in response.json()["detail"] - +async def test_create_host_rejects_unknown_template_id(client): + """An unknown template ID is bad host-create input, not a route 404.""" + missing_id = str(uuid7()) -async def test_create_host_rejects_unknown_template_reference(client): - """An unknown template reference is bad host-create input, not a route 404.""" response = await client.post( "/hosts", headers=AUTH_HEADERS, - json={"template": "missing-template"}, + json={"template": missing_id}, ) assert response.status_code == 400 - assert "missing-template" in response.json()["detail"] + assert missing_id in response.json()["detail"] assert "not found" in response.json()["detail"] assert response.json()["error_code"] == "TEMPLATE_REFERENCE" @@ -285,12 +209,12 @@ async def test_create_host_cannot_resolve_another_providers_template(client): assert "provider 'exe'" in response.json()["detail"] -async def test_create_host_rejects_blank_template(client): - """A whitespace-only template reference stops at the wire boundary.""" +async def test_create_host_rejects_a_non_uuid_template(client): + """A template value that is not a UUID stops at the wire boundary.""" response = await client.post( "/hosts", headers=AUTH_HEADERS, - json={"template": " \n"}, + json={"template": "not-a-uuid"}, ) assert response.status_code == 422 From f95b8199809abf850b23b15226c2e855daefbae5 Mon Sep 17 00:00:00 2001 From: Paulo Date: Tue, 25 Aug 2026 08:38:36 +0200 Subject: [PATCH 7/8] The template ID comes from the create response, not a repost The repost-as-lookup suggestion blurred the contract: POST /templates is the only path that builds, and naming a template on POST /hosts never creates one. Co-Authored-By: Claude Fable 5 --- docs/architecture.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index c78c8c7..021fb92 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -105,10 +105,9 @@ returns `202 Accepted`. Callers poll until the template becomes `available` or `failed`. Templates outlive hosts. Each provider builds and deletes its own templates behind `TemplateCapability`. -A host request can name an available template by its ID. The `POST -/templates` create is idempotent by content, so the caller can always -repost the same declaration to get the current ID back. -The template's handle becomes the host image. An explicit `image` +A host request can name an available template by its ID — the ID that +the create returned. The template's handle becomes the host image. An +explicit `image` wins over the template, and the template wins over the provider default. Host creation never builds a missing or unavailable template. It returns a client error, and the caller decides when to build. From 3c6cf2fa1b00830310cc1fa735b15503ea7b41f0 Mon Sep 17 00:00:00 2001 From: Paulo Date: Tue, 25 Aug 2026 08:48:30 +0200 Subject: [PATCH 8/8] One janitor: python -m janitor reaps hosts and templates Reaping is one concern, and every resource type adding its own cron entry is operator sprawl. The reapers stay in their packages; the janitor package is the single cron entry that runs both. The pool maintainer stays its own command because it creates resources instead of reaping them. Co-Authored-By: Claude Fable 5 --- AGENTS.md | 7 ++++--- docs/architecture.md | 7 +++---- docs/deploy.md | 20 +++++++++----------- pyproject.toml | 1 + src/hosts/janitor.py | 11 ----------- src/janitor/__init__.py | 7 +++++++ src/janitor/__main__.py | 11 +++++++++++ src/janitor/tests/__init__.py | 0 src/janitor/tests/test_reap.py | 16 ++++++++++++++++ src/templates/janitor.py | 11 ----------- 10 files changed, 51 insertions(+), 40 deletions(-) create mode 100644 src/janitor/__init__.py create mode 100644 src/janitor/__main__.py create mode 100644 src/janitor/tests/__init__.py create mode 100644 src/janitor/tests/test_reap.py diff --git a/AGENTS.md b/AGENTS.md index 94c8207..d23a864 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -19,9 +19,9 @@ It owns: - An SSH gateway for hosts of gateway providers (`python -m gateway.server`) - Account-bound exe.dev HTTP proxy resources -Periodic maintenance runs as cron jobs: `python -m hosts.janitor` reaps -expired hosts, `python -m templates.janitor` reaps abandoned, failed, and unused -templates, and `python -m hosts.pool` tops up the warm pool. +Periodic maintenance runs as cron jobs: `python -m janitor` reaps expired +hosts and abandoned, failed, or unused templates, and `python -m hosts.pool` +tops up the warm pool. No backwards compatibility is required unless a caller contract is explicitly documented in this repo. @@ -49,6 +49,7 @@ src/ hosts/ # Host API, models, schemas, service, janitor, pool, auth gateway/ # SSH gateway for gateway-provider hosts http_proxies/ # HTTP proxy API, schemas, service, deps + janitor/ # Cron entry point that runs the host and template reapers providers/ # VM provider ABC, capabilities, registry, adapters networking/ # Network provider framework and Tailscale adapter templates/ # Template API, models, service, and janitor diff --git a/docs/architecture.md b/docs/architecture.md index 021fb92..3da1022 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -121,11 +121,10 @@ is the keepalive: it bumps `expires_at` to the requested instant, or by hosts renew — unclaimed warm-pool members belong to pool maintenance and refuse with `409`. -Three maintenance commands run as cron jobs from the same image: +Two maintenance commands run as cron jobs from the same image: -- `hosts.janitor` reaps expired and orphaned hosts. -- `templates.janitor` marks abandoned builds `failed` and deletes failed - or unused templates. +- `janitor` reaps expired and orphaned hosts, marks abandoned template + builds `failed`, and deletes failed or unused templates. - `hosts.pool` keeps a warm pool of pre-provisioned hosts per provider (`POOL_SIZES`, with `POOL_SIZE` as the default provider's target) to hide provider cold starts. diff --git a/docs/deploy.md b/docs/deploy.md index 857b798..9e23e98 100644 --- a/docs/deploy.md +++ b/docs/deploy.md @@ -20,17 +20,16 @@ docker run --rm -p 8780:8780 --env-file drukbox.env "$IMAGE" docker run --rm --env-file drukbox.env "$IMAGE" .venv/bin/alembic upgrade head # Maintenance (cron, e.g. every 10-15 min) -docker run --rm --env-file drukbox.env "$IMAGE" .venv/bin/python -m hosts.janitor +docker run --rm --env-file drukbox.env "$IMAGE" .venv/bin/python -m janitor docker run --rm --env-file drukbox.env "$IMAGE" .venv/bin/python -m hosts.pool -docker run --rm --env-file drukbox.env "$IMAGE" .venv/bin/python -m templates.janitor ``` -The host janitor reaps expired and orphaned hosts. The template janitor -marks abandoned builds failed, keeps failed builds for diagnosis, and -deletes failed or unused templates. The pool maintainer +The janitor reaps expired and orphaned hosts, marks abandoned template +builds failed, keeps failed builds for diagnosis, and deletes failed or +unused templates. The pool maintainer pre-provisions warm hosts per provider and only does anything when at least one provider has a warm target (`POOL_SIZES` / `POOL_SIZE`). -Schedule all three under your cron infrastructure (k8s `CronJob`, +Schedule both under your cron infrastructure (k8s `CronJob`, systemd timer) from the same image and env file. Use Postgres in production (`postgresql+psycopg://...`). SQLite @@ -102,9 +101,8 @@ talks to the host's Docker daemon, and granting drukbox access to that socket is host-root-equivalent. Do not expose a docker-backed drukbox to untrusted callers. -Host-janitor, template-janitor, and pool one-off containers using the -Docker provider need the same socket mount and socket-GID supplemental -group. `DOCKER_HOST` remains available when the daemon is remote or +Janitor and pool one-off containers using the Docker provider need the +same socket mount and socket-GID supplemental group. `DOCKER_HOST` remains available when the daemon is remote or rootless instead of exposed through `/var/run/docker.sock`. ## Local microVMs with Docker Sandboxes @@ -157,8 +155,8 @@ docker run --rm --network host \ The daemon reads workspace paths on its own filesystem. Thus the workspace mount must have the same path on the host and in the -container. The host-janitor, template-janitor, and pool containers need -the same mounts and variables. +container. The janitor and pool containers need the same mounts and +variables. Callers reach the sandboxes through [the SSH gateway](#the-ssh-gateway); the provider requires it. The key diff --git a/pyproject.toml b/pyproject.toml index 4693371..24f9637 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -10,6 +10,7 @@ packages = [ "src/gateway", "src/hosts", "src/http_proxies", + "src/janitor", "src/networking", "src/providers", "src/templates", diff --git a/src/hosts/janitor.py b/src/hosts/janitor.py index 98649de..fa4c901 100644 --- a/src/hosts/janitor.py +++ b/src/hosts/janitor.py @@ -61,14 +61,3 @@ async def reap_expired_hosts() -> list[uuid.UUID]: await tailscale.aclose() return reaped - - -if __name__ == "__main__": - # Cron entry point: `python -m hosts.janitor`. - import asyncio - - logging.basicConfig( - level=logging.INFO, - format="%(asctime)s %(levelname)s %(name)s %(message)s", - ) - asyncio.run(reap_expired_hosts()) diff --git a/src/janitor/__init__.py b/src/janitor/__init__.py new file mode 100644 index 0000000..dbaf733 --- /dev/null +++ b/src/janitor/__init__.py @@ -0,0 +1,7 @@ +from hosts.janitor import reap_expired_hosts +from templates.janitor import reap_templates + + +async def reap() -> None: + await reap_expired_hosts() + await reap_templates() diff --git a/src/janitor/__main__.py b/src/janitor/__main__.py new file mode 100644 index 0000000..1570768 --- /dev/null +++ b/src/janitor/__main__.py @@ -0,0 +1,11 @@ +# Cron entry point: `python -m janitor`. +import asyncio +import logging + +from janitor import reap + +logging.basicConfig( + level=logging.INFO, + format="%(asctime)s %(levelname)s %(name)s %(message)s", +) +asyncio.run(reap()) diff --git a/src/janitor/tests/__init__.py b/src/janitor/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/janitor/tests/test_reap.py b/src/janitor/tests/test_reap.py new file mode 100644 index 0000000..e9f639c --- /dev/null +++ b/src/janitor/tests/test_reap.py @@ -0,0 +1,16 @@ +from unittest.mock import AsyncMock + +import janitor + + +async def test_reap_runs_both_reapers(monkeypatch): + """One cron entry sweeps hosts and templates.""" + hosts_reaper = AsyncMock() + templates_reaper = AsyncMock() + monkeypatch.setattr(janitor, "reap_expired_hosts", hosts_reaper) + monkeypatch.setattr(janitor, "reap_templates", templates_reaper) + + await janitor.reap() + + hosts_reaper.assert_awaited_once_with() + templates_reaper.assert_awaited_once_with() diff --git a/src/templates/janitor.py b/src/templates/janitor.py index 6c4dbd7..e80bd02 100644 --- a/src/templates/janitor.py +++ b/src/templates/janitor.py @@ -112,14 +112,3 @@ async def reap_templates() -> None: reap_reason, template_id, ) - - -if __name__ == "__main__": - # Cron entry point: `python -m templates.janitor`. - import asyncio - - logging.basicConfig( - level=logging.INFO, - format="%(asctime)s %(levelname)s %(name)s %(message)s", - ) - asyncio.run(reap_templates())