From 3b6cd25c714e83e1b4acbbfa9bb5bde69cb81d56 Mon Sep 17 00:00:00 2001 From: Paulo Alvarado Date: Thu, 20 Aug 2026 10:58:54 +0200 Subject: [PATCH] docker-sbx: fixes from the first deployment Publish SSH on a loopback port that the daemon selects, make /run/sshd at entrypoint start, document sbx template load for local templates, and let each provider own its /doctor probe time limit. --- docs/deploy.md | 21 ++++++++++----- images/sbx/entrypoint.sh | 4 +++ src/diagnostics/api.py | 9 ++++--- src/diagnostics/tests/test_doctor.py | 27 +++++++++++++++++++ src/providers/base.py | 4 +++ src/providers/docker_sbx/api.py | 12 ++++----- src/providers/docker_sbx/provider.py | 11 ++++++-- src/providers/docker_sbx/settings.py | 8 ------ src/providers/docker_sbx/tests/test_api.py | 26 +++++++----------- .../docker_sbx/tests/test_provider.py | 8 +++--- 10 files changed, 82 insertions(+), 48 deletions(-) diff --git a/docs/deploy.md b/docs/deploy.md index 881dbac..f0499c4 100644 --- a/docs/deploy.md +++ b/docs/deploy.md @@ -132,8 +132,7 @@ uv run uvicorn api.app:app --host 127.0.0.1 --port 8780 ``` In this mode, no mounts and no extra variables are necessary. The CLI -finds the daemon socket automatically, and the `127.0.0.1` default for -`DOCKER_SBX_ADVERTISE_HOST` is correct. +finds the daemon socket automatically. drukbox can also run as a container adjacent to the daemon. Docker does not document this mode; drukbox uses the CLI's own daemon-endpoint @@ -149,7 +148,6 @@ docker run --rm --network host \ --mount type=bind,src=$HOME/.drukbox/sbx-workspaces,dst=$HOME/.drukbox/sbx-workspaces \ --env DOCKER_SANDBOXES_API=unix:///run/sandboxd.sock \ --env DOCKER_SBX_WORKSPACE_ROOT=$HOME/.drukbox/sbx-workspaces \ - --env DOCKER_SBX_ADVERTISE_HOST=172.17.0.1 \ --env-file drukbox.env \ ghcr.io/czpython/drukbox:latest ``` @@ -157,9 +155,9 @@ 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. `DOCKER_SBX_ADVERTISE_HOST` is the Docker bridge address -here, because the sandbox SSH ports must be open to the drukbox -container, not only to the host loopback interface. +variables. Host networking is necessary: the daemon publishes sandbox +SSH ports on the host loopback interface only, and drukbox returns +`127.0.0.1` addresses, the same as the `docker` provider. Only this machine can connect to the sandboxes. The key for each host is the auth boundary. Sandboxes have no `SERVICE_LABEL` tag, because @@ -173,6 +171,16 @@ for each host through the exec channel after the start. Build `images/local/` entrypoint needs boot-time environment variables and cannot start as a sandbox template. +The daemon has its own image store and does not read local Docker +images. It pulls unknown template names from a registry. For a local +template, load the image into the daemon: + +```bash +docker build -t drukbox/sbx-sandbox:latest images/sbx/ +docker save drukbox/sbx-sandbox:latest -o /tmp/sbx-sandbox.tar +sbx template load /tmp/sbx-sandbox.tar +``` + A sandbox creation takes approximately 20 seconds with a warm template cache, and more than 30 seconds at the first pull. Thus a warm pool (`POOL_SIZES`) is useful. Each sandbox gets the explicit @@ -345,7 +353,6 @@ Docker Sandboxes provider: | `DOCKER_SBX_DEFAULT_IMAGE` | `ghcr.io/czpython/drukbox/sbx-sandbox:latest` | Template image that contains sshd and starts without environment variables. Build `images/sbx/Dockerfile` to change it. | | `DOCKER_SBX_SSH_USERNAME` | `root` | User in the sandbox for caller SSH access. | | `DOCKER_SBX_BOOTSTRAP_SSH_TIMEOUT_SECONDS` | `30.0` | Time limit for the ssh-keyscan tries on a new sandbox. | -| `DOCKER_SBX_ADVERTISE_HOST` | `127.0.0.1` | Host address for the published SSH ports. Use the Docker bridge address when drukbox runs in a container. | | `DOCKER_SBX_CPUS` | `2` | Number of CPUs for each sandbox. | | `DOCKER_SBX_MEMORY` | `2g` | Memory for each sandbox, in binary units. | | `DOCKER_SBX_WORKSPACE_ROOT` | `~/.drukbox/sbx-workspaces` | Directory with one temporary workspace for each sandbox. The path must be the same for drukbox and for the daemon. | diff --git a/images/sbx/entrypoint.sh b/images/sbx/entrypoint.sh index 331eea1..8ad594f 100755 --- a/images/sbx/entrypoint.sh +++ b/images/sbx/entrypoint.sh @@ -11,6 +11,10 @@ set -euo pipefail install -d -m 700 /root/.ssh [ -f /root/.ssh/authorized_keys ] || install -m 600 /dev/null /root/.ssh/authorized_keys +# /run is a fresh tmpfs at start. sshd stops immediately without its +# privilege-separation directory. +mkdir -p /run/sshd + ssh-keygen -A exec /usr/sbin/sshd -D -e diff --git a/src/diagnostics/api.py b/src/diagnostics/api.py index e93f881..391bf4f 100644 --- a/src/diagnostics/api.py +++ b/src/diagnostics/api.py @@ -8,7 +8,7 @@ from core.database import get_session from core.settings import get_settings -from diagnostics.checks import Check, CheckStatus, run_check +from diagnostics.checks import DEFAULT_CHECK_TIMEOUT_SECONDS, Check, CheckStatus, run_check from hosts.auth import require_service_auth from networking.tailscale import Tailscale from providers.registry import get_default_vm_provider @@ -42,13 +42,16 @@ async def doctor( ) -> DoctorOut: settings = get_settings() try: - provider_hint = get_default_vm_provider().diagnose_hint + provider = get_default_vm_provider() + provider_hint = provider.diagnose_hint + provider_timeout = provider.diagnose_timeout_seconds except Exception: # The provider can't even be constructed (bad config / missing dep). Use a # generic hint; the probe below re-resolves it so the construction error # surfaces as a failed check instead of a 500 — diagnosing exactly this is # the point of /doctor. provider_hint = "check_provider_configuration" + provider_timeout = DEFAULT_CHECK_TIMEOUT_SECONDS async def _provider_probe() -> str: return await get_default_vm_provider().diagnose() @@ -68,7 +71,7 @@ async def _tailscale_probe() -> str: run_check("db", lambda: _ping_db(session), hint="check_database_url_and_engine"), ), asyncio.ensure_future( - run_check("provider", _provider_probe, hint=provider_hint), + run_check("provider", _provider_probe, hint=provider_hint, timeout=provider_timeout), ), ] if settings.tailscale_enabled: diff --git a/src/diagnostics/tests/test_doctor.py b/src/diagnostics/tests/test_doctor.py index b3fae20..df10cca 100644 --- a/src/diagnostics/tests/test_doctor.py +++ b/src/diagnostics/tests/test_doctor.py @@ -1,3 +1,4 @@ +import asyncio import contextlib from unittest.mock import AsyncMock, MagicMock, patch @@ -153,6 +154,8 @@ async def test_doctor_runs_db_probe_against_real_session(client) -> None: """The db row round-trips through the test DB, not a mock.""" fake_provider = MagicMock() fake_provider.name = "exe" + fake_provider.diagnose_hint = "check_exe_api_token" + fake_provider.diagnose_timeout_seconds = 5.0 fake_provider.diagnose = AsyncMock(return_value="exe ok") with ( @@ -168,3 +171,27 @@ async def test_doctor_runs_db_probe_against_real_session(client) -> None: db = next(check for check in body["checks"] if check["name"] == "db") assert db["status"] == "ok" assert db["detail"] == "select 1 -> 1" + + +async def test_doctor_provider_probe_uses_the_provider_probe_timeout(client) -> None: + """A probe slower than the default budget passes when the provider declares a larger one.""" + + async def slow_diagnose(self) -> str: + await asyncio.sleep(0.05) + return "slow but healthy" + + with ( + patch.object(ExeProvider, "diagnose_timeout_seconds", 30.0), + patch.object(ExeProvider, "diagnose", new=slow_diagnose), + patch.object(Tailscale, "diagnose", new=AsyncMock(return_value="tailnet ok")), + patch("diagnostics.api.DEFAULT_CHECK_TIMEOUT_SECONDS", 0.01), + patch("diagnostics.checks.DEFAULT_CHECK_TIMEOUT_SECONDS", 0.01), + ): + response = await client.get( + "/doctor", + headers={"Authorization": "Bearer service-token"}, + ) + + provider = next(check for check in response.json()["checks"] if check["name"] == "provider") + assert provider["status"] == "ok" + assert provider["detail"] == "slow but healthy" diff --git a/src/providers/base.py b/src/providers/base.py index c722373..4f06e49 100644 --- a/src/providers/base.py +++ b/src/providers/base.py @@ -18,6 +18,10 @@ class VMProvider(abc.ABC): # Remediation slug attached to a failed /doctor probe. Owned here because # the provider is what knows how its own dependency gets fixed. diagnose_hint: ClassVar[str] + # Time limit for the /doctor probe. Owned here for the same reason: the + # provider knows the cost of its own probe. CLI-backed probes can be + # slower than the default. + diagnose_timeout_seconds: ClassVar[float] = 5.0 # Which per-request sizing fields create_vm honors. HostService rejects a # sized request up front — before any host row or VM exists — when the # target provider leaves these False. diff --git a/src/providers/docker_sbx/api.py b/src/providers/docker_sbx/api.py index c4a7202..2b1e398 100644 --- a/src/providers/docker_sbx/api.py +++ b/src/providers/docker_sbx/api.py @@ -65,12 +65,12 @@ async def run_bootstrap(self, name: str, script: str) -> None: stdin=script, ) - async def publish_ssh_port(self, name: str, *, host_ip: str) -> int: - # An empty host port tells the daemon to select a free port. Thus - # sandboxes do not compete for port numbers. An IPv6 address must have - # brackets in the port specification. - spec_host = f"[{host_ip}]" if ":" in host_ip else host_ip - output = await self._run("ports", name, "--publish", f"{spec_host}::22") + async def publish_ssh_port(self, name: str) -> int: + # A bare sandbox port tells the daemon to select a free host port on + # the loopback interface. Thus sandboxes do not compete for port + # numbers. The daemon cannot select a free port on an explicit + # address: it rejects "IP::22" and port 0. + output = await self._run("ports", name, "--publish", "22") # Each binding shows as ": -> 22/tcp". The output can # have one line for each address family. The lines share the host port. for line in output.splitlines(): diff --git a/src/providers/docker_sbx/provider.py b/src/providers/docker_sbx/provider.py index 0ce07f9..37b42a8 100644 --- a/src/providers/docker_sbx/provider.py +++ b/src/providers/docker_sbx/provider.py @@ -48,6 +48,10 @@ class DockerSbxProvider(VMProvider): # sandbox egress, and the template has no init system. These hosts keep # the published sshd port, also on a tailnet-mode service. supports_tailnet: ClassVar[bool] = False + # Each sbx invocation spends approximately 3 seconds on CLI startup work + # before the command runs. The default 5-second probe budget fails on a + # healthy daemon. + diagnose_timeout_seconds: ClassVar[float] = 15.0 def __init__(self, api: SbxCLI, settings: DockerSbxSettings) -> None: self.api = api @@ -134,18 +138,21 @@ async def create_vm( ssh_username=self.settings.ssh_username, ) await self.api.run_bootstrap(name, script) - ssh_port = await self.api.publish_ssh_port(name, host_ip=self.settings.advertise_host) + ssh_port = await self.api.publish_ssh_port(name) except DockerSbxProviderError as exc: with contextlib.suppress(DockerSbxProviderError): await self.api.remove_sandbox(name) self._remove_workspace(name) raise ProviderTransportError(str(exc)) from exc + # The daemon publishes sandbox ports on the host loopback interface + # only, the same as the docker provider. A drukbox container reaches + # them through host networking. return VMCreateResult( provider_id=name, name=name, ssh_port=ssh_port, - ssh_host=self.settings.advertise_host, + ssh_host="127.0.0.1", ssh_username=self.settings.ssh_username, private_key=private_key, ) diff --git a/src/providers/docker_sbx/settings.py b/src/providers/docker_sbx/settings.py index 754935a..a419624 100644 --- a/src/providers/docker_sbx/settings.py +++ b/src/providers/docker_sbx/settings.py @@ -26,14 +26,6 @@ class DockerSbxSettings(BaseSettings): default=30.0, description="Time limit for the ssh-keyscan tries on a new sandbox.", ) - advertise_host: str = Field( - default="127.0.0.1", - description=( - "Host address for the published SSH ports. The drukbox process must " - "have access to this address. Use the Docker bridge address when " - "drukbox runs in a container." - ), - ) cpus: int = Field( default=2, ge=1, diff --git a/src/providers/docker_sbx/tests/test_api.py b/src/providers/docker_sbx/tests/test_api.py index f272dcd..7a77c9f 100644 --- a/src/providers/docker_sbx/tests/test_api.py +++ b/src/providers/docker_sbx/tests/test_api.py @@ -79,28 +79,20 @@ async def fake_exec(*args, **kwargs): monkeypatch.setattr("providers.docker_sbx.api.asyncio.create_subprocess_exec", fake_exec) - port = await SbxCLI().publish_ssh_port("sb-test", host_ip="172.17.0.1") + port = await SbxCLI().publish_ssh_port("sb-test") assert port == 49160 - # An empty host port tells the daemon to select a free port. - assert "172.17.0.1::22" in captured["args"] + # A bare sandbox port tells the daemon to select a free loopback port. + assert captured["args"][-2:] == ("--publish", "22") @pytest.mark.asyncio async def test_publish_ssh_port_picks_the_ssh_binding_out_of_a_multi_line_listing(monkeypatch): - captured: dict = {} - - async def fake_exec(*args, **kwargs): - captured["args"] = args - return _process( - stdout=(b"Published 172.17.0.1:8080 -> 80/tcp\nPublished [::1]:49161 -> 22/tcp\n") - ) - - monkeypatch.setattr("providers.docker_sbx.api.asyncio.create_subprocess_exec", fake_exec) + listing = b"Published 127.0.0.1:8080 -> 80/tcp\nPublished 127.0.0.1:49161 -> 22/tcp\n" + create = AsyncMock(return_value=_process(stdout=listing)) + monkeypatch.setattr("providers.docker_sbx.api.asyncio.create_subprocess_exec", create) - assert await SbxCLI().publish_ssh_port("sb-test", host_ip="::1") == 49161 - # An IPv6 address must have brackets in the port specification. - assert "[::1]::22" in captured["args"] + assert await SbxCLI().publish_ssh_port("sb-test") == 49161 @pytest.mark.asyncio @@ -109,7 +101,7 @@ async def test_publish_ssh_port_raises_when_nothing_was_published(monkeypatch): monkeypatch.setattr("providers.docker_sbx.api.asyncio.create_subprocess_exec", create) with pytest.raises(DockerSbxTransportError, match="no SSH port"): - await SbxCLI().publish_ssh_port("sb-test", host_ip="172.17.0.1") + await SbxCLI().publish_ssh_port("sb-test") @pytest.mark.asyncio @@ -118,7 +110,7 @@ async def test_publish_ssh_port_raises_on_unparsable_output(monkeypatch): monkeypatch.setattr("providers.docker_sbx.api.asyncio.create_subprocess_exec", create) with pytest.raises(DockerSbxTransportError, match="unparsable"): - await SbxCLI().publish_ssh_port("sb-test", host_ip="172.17.0.1") + await SbxCLI().publish_ssh_port("sb-test") @pytest.mark.asyncio diff --git a/src/providers/docker_sbx/tests/test_provider.py b/src/providers/docker_sbx/tests/test_provider.py index 7fdecde..733a199 100644 --- a/src/providers/docker_sbx/tests/test_provider.py +++ b/src/providers/docker_sbx/tests/test_provider.py @@ -34,9 +34,7 @@ def _api_mock() -> MagicMock: @pytest.mark.asyncio async def test_create_vm_creates_a_sized_sandbox_and_returns_published_coords(tmp_path): api = _api_mock() - provider = DockerSbxProvider( - api, _settings(tmp_path, advertise_host="172.17.0.1", cpus=4, memory="8g") - ) + provider = DockerSbxProvider(api, _settings(tmp_path, cpus=4, memory="8g")) result = await provider.create_vm(name="sb-test", image="drukbox/sbx-sandbox:latest", env={}) @@ -50,8 +48,8 @@ async def test_create_vm_creates_a_sized_sandbox_and_returns_published_coords(tm assert create_kwargs["workspace"] == str(tmp_path / "sb-test") assert (tmp_path / "sb-test").is_dir() - api.publish_ssh_port.assert_awaited_once_with("sb-test", host_ip="172.17.0.1") - assert result.ssh_host == "172.17.0.1" + api.publish_ssh_port.assert_awaited_once_with("sb-test") + 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