mirror of
https://github.com/onyx-dot-app/onyx.git
synced 2026-10-19 01:50:51 -05:00
feat(craft): pre-pull the sandbox image for docker-compose deployments (#13592)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
7968b109ba
commit
bb5bbc2b40
4 files changed
+181
-4
No files matched your search
@@ -844,3 +844,63 @@ def test_proxy_kwargs_requires_ca_volume() -> None:
|
||||
sandbox_proxy_host="sandbox-proxy",
|
||||
proxy_ca_volume_name=None,
|
||||
)
|
||||
|
||||
|
||||
def _craft_compose_services() -> dict:
|
||||
compose_path = REPO_ROOT / "deployment/docker_compose/docker-compose.craft.yml"
|
||||
return yaml.safe_load(compose_path.read_text())["services"]
|
||||
|
||||
|
||||
def test_compose_prepulls_the_image_the_sandboxes_run() -> None:
|
||||
"""The prepull entry exists so `docker compose pull` fetches the sandbox
|
||||
image, and must name the *same* ref the sandbox containers get. A drifted
|
||||
ref warms an image nobody runs while every sandbox still cold-pulls, and
|
||||
nothing looks wrong — the only symptom is the slow provision it exists to
|
||||
remove.
|
||||
"""
|
||||
services = _craft_compose_services()
|
||||
prepull_image = services["sandbox-image-prepull"]["image"]
|
||||
|
||||
for service_name in ("api_server", "background"):
|
||||
assert (
|
||||
f"SANDBOX_CONTAINER_IMAGE={prepull_image}"
|
||||
in services[service_name]["environment"]
|
||||
)
|
||||
|
||||
|
||||
def test_compose_prepull_never_starts_a_container() -> None:
|
||||
"""It is an image reference, not a workload. `replicas: 0` keeps `up` from
|
||||
creating anything, so nothing idles doing nothing and `up --wait` — which
|
||||
install.sh passes by default — stays green. A one-shot that exited would
|
||||
fail that wait; a long-lived idler would leave a pointless container.
|
||||
"""
|
||||
prepull = _craft_compose_services()["sandbox-image-prepull"]
|
||||
|
||||
assert prepull["deploy"]["replicas"] == 0
|
||||
# No socket, no volumes, no sandbox bridge: nothing is supposed to run.
|
||||
for key in ("volumes", "networks", "restart"):
|
||||
assert key not in prepull
|
||||
|
||||
|
||||
def test_compose_prepull_is_inert_even_if_it_does_start() -> None:
|
||||
"""`deploy` is ignored by the legacy standalone docker-compose, which
|
||||
install.sh accepts with no minimum version. If `replicas: 0` is dropped, a
|
||||
container *is* created — and this image's own ENTRYPOINT starts
|
||||
`opencode serve` on 0.0.0.0:4096 with no password, staying up so nothing
|
||||
looks broken. The override is the only thing standing between a dropped
|
||||
`deploy` key and an unauthenticated agent server on the compose network.
|
||||
"""
|
||||
prepull = _craft_compose_services()["sandbox-image-prepull"]
|
||||
|
||||
assert prepull["entrypoint"] == ["/bin/true"]
|
||||
|
||||
|
||||
def test_embedded_craft_compose_copy_is_in_sync() -> None:
|
||||
"""onyx-cli go:embeds a byte-identical copy (go:embed can't reach outside
|
||||
the cli module). See tools/ods/internal/deployfilessync."""
|
||||
source = REPO_ROOT / "deployment/docker_compose/docker-compose.craft.yml"
|
||||
embedded = (
|
||||
REPO_ROOT
|
||||
/ "cli/internal/deploy/deployfiles/embedded/docker_compose/docker-compose.craft.yml"
|
||||
)
|
||||
assert embedded.read_text() == source.read_text()
|
||||
+22
@@ -127,6 +127,28 @@ services:
|
||||
max-size: "50m"
|
||||
max-file: "6"
|
||||
|
||||
# Declared solely so `docker compose pull` fetches the sandbox image. Without
|
||||
# it the first sandbox pays the ~1 GB download inside its provisioning
|
||||
# request, because api_server creates sandbox containers from this image
|
||||
# directly rather than compose managing them.
|
||||
#
|
||||
# replicas: 0 means no container is ever created -- `up` skips it, so nothing
|
||||
# idles and `up --wait` stays green. It is not a workload; it is an image
|
||||
# reference that the normal pull/upgrade lifecycle can see.
|
||||
sandbox-image-prepull:
|
||||
# Must match api_server/background's SANDBOX_CONTAINER_IMAGE below, or this
|
||||
# warms an image nobody runs while every sandbox still cold-pulls.
|
||||
image: ${SANDBOX_CONTAINER_IMAGE:-onyxdotapp/sandbox:${IMAGE_TAG:-latest}}
|
||||
deploy:
|
||||
replicas: 0
|
||||
# Defence in depth. `replicas: 0` is honoured by Compose v2 (even when the
|
||||
# service is named explicitly), but the legacy standalone docker-compose
|
||||
# ignores `deploy` outside swarm, and install.sh accepts a standalone binary
|
||||
# with no minimum version. Unoverridden, the image's own ENTRYPOINT starts
|
||||
# `opencode serve` on 0.0.0.0:4096 with no password -- and stays running, so
|
||||
# nothing looks wrong. Anything that does start here must be inert.
|
||||
entrypoint: ["/bin/true"]
|
||||
|
||||
networks:
|
||||
# Pre-created by install.sh --include-craft.
|
||||
onyx_craft_sandbox:
|
||||
|
||||
@@ -127,6 +127,28 @@ services:
|
||||
max-size: "50m"
|
||||
max-file: "6"
|
||||
|
||||
# Declared solely so `docker compose pull` fetches the sandbox image. Without
|
||||
# it the first sandbox pays the ~1 GB download inside its provisioning
|
||||
# request, because api_server creates sandbox containers from this image
|
||||
# directly rather than compose managing them.
|
||||
#
|
||||
# replicas: 0 means no container is ever created -- `up` skips it, so nothing
|
||||
# idles and `up --wait` stays green. It is not a workload; it is an image
|
||||
# reference that the normal pull/upgrade lifecycle can see.
|
||||
sandbox-image-prepull:
|
||||
# Must match api_server/background's SANDBOX_CONTAINER_IMAGE below, or this
|
||||
# warms an image nobody runs while every sandbox still cold-pulls.
|
||||
image: ${SANDBOX_CONTAINER_IMAGE:-onyxdotapp/sandbox:${IMAGE_TAG:-latest}}
|
||||
deploy:
|
||||
replicas: 0
|
||||
# Defence in depth. `replicas: 0` is honoured by Compose v2 (even when the
|
||||
# service is named explicitly), but the legacy standalone docker-compose
|
||||
# ignores `deploy` outside swarm, and install.sh accepts a standalone binary
|
||||
# with no minimum version. Unoverridden, the image's own ENTRYPOINT starts
|
||||
# `opencode serve` on 0.0.0.0:4096 with no password -- and stays running, so
|
||||
# nothing looks wrong. Anything that does start here must be inert.
|
||||
entrypoint: ["/bin/true"]
|
||||
|
||||
networks:
|
||||
# Pre-created by install.sh --include-craft.
|
||||
onyx_craft_sandbox:
|
||||
|
||||
@@ -122,10 +122,12 @@ so the prod image still has it but dev/CI images can opt out.
|
||||
|
||||
## Cold pulls vs. image warming — decision and roadmap
|
||||
|
||||
**Current state: we pre-pull the sandbox image onto the sandbox node
|
||||
pool with a DaemonSet.** Chart: `sandboxImagePrepull` in `values.yaml`,
|
||||
template `sandbox-image-prepuller.yaml`. On by default when
|
||||
`ENABLE_CRAFT` is set.
|
||||
**Current state: we pre-pull the sandbox image on both backends.**
|
||||
Kubernetes uses a DaemonSet over the sandbox node pool
|
||||
(`sandboxImagePrepull` in `values.yaml`, template
|
||||
`sandbox-image-prepuller.yaml`); Docker declares the image in
|
||||
`docker-compose.craft.yml` so `docker compose pull` fetches it — see
|
||||
"Docker" below. Both on by default when `ENABLE_CRAFT` is set.
|
||||
|
||||
This section previously recorded the opposite decision — that we accept
|
||||
cold pulls — on an estimate of ~3–6 s per pull from AZ-local bandwidth.
|
||||
@@ -263,6 +265,77 @@ does a fresh install into an empty kind cluster, never an upgrade from
|
||||
the previously released chart, so the patch path where immutable-field
|
||||
violations live is untested. `ct install --upgrade` would cover it.
|
||||
|
||||
### Docker
|
||||
|
||||
Same problem, materially easier shape. The Docker backend pulls lazily in
|
||||
`DockerSandboxManager._ensure_sandbox_image()`, called from
|
||||
`provision()` — so on a host without the image, whoever asks for the
|
||||
first sandbox pays the ~1 GB download inside their own request.
|
||||
|
||||
Two differences from Kubernetes drive a different fix:
|
||||
|
||||
- **One image store, not one per node.** A compose deployment is a single
|
||||
Docker daemon, so a single pull serves `api_server`, `background`, and
|
||||
every sandbox container. There is no per-node fan-out to arrange, and
|
||||
no `maxUnavailable` to tune.
|
||||
- **Docker never garbage-collects images.** kubelet reclaims them at
|
||||
`imageGCHighThresholdPercent`, which is why the Kubernetes fix needs a
|
||||
*running* pod to pin the layers. Docker only removes images on an
|
||||
explicit `docker image prune`. Pinning is therefore a bonus here, not
|
||||
the point.
|
||||
|
||||
So the fix is a `sandbox-image-prepull` entry in
|
||||
`docker-compose.craft.yml` carrying `deploy.replicas: 0`. The sandbox
|
||||
image is not otherwise a compose service — `api_server` creates sandbox
|
||||
containers from it directly — so `docker compose pull` has nothing to
|
||||
tell it about. Declaring it puts the image on the normal pull/upgrade
|
||||
lifecycle; `replicas: 0` means no container is ever created.
|
||||
|
||||
That combination is load-bearing and was verified rather than assumed:
|
||||
|
||||
| shape | `compose pull` fetches it | `up --wait` |
|
||||
|---|---|---|
|
||||
| `profiles: [...]` | **no** — skipped | n/a |
|
||||
| one-shot that exits 0 | yes | **exits 1** |
|
||||
| long-lived idler | yes | ok, but a pointless container forever |
|
||||
| `deploy.replicas: 0` | yes | ok, no container at all |
|
||||
|
||||
So profiles can't be used (a plain `pull` skips them, which defeats the
|
||||
point for anyone not using the installer), and a one-shot breaks the
|
||||
`up --wait` that `install.sh` passes by default.
|
||||
|
||||
**One mechanism, and it covers every path.** `install.sh --include-craft`
|
||||
already runs `docker compose pull` with the craft overlay layered in, so
|
||||
the installer needs no special case. Anyone who brings the stack up by
|
||||
hand — a supported path, see `deployment/docker_compose/README.md` option
|
||||
2 — gets it from the same plain `docker compose pull`, with nothing extra
|
||||
to remember.
|
||||
|
||||
**One fallback.** If the image is missing later (pruned, or an operator
|
||||
who never pulls), `provision()` pulls it on demand as it always has. The
|
||||
cost lands on one request and the error surfaces against the request that
|
||||
caused it.
|
||||
|
||||
Earlier revisions added a long-lived compose service to hold the image
|
||||
*and* a background warm at api_server startup. Both were removed. They
|
||||
were mutually redundant — each was justified by gaps the other covered —
|
||||
and neither survives the question "if the other exists, why do I?". The
|
||||
long-lived service also left a container idling forever for no reason
|
||||
beyond satisfying `docker compose up --wait` — which `replicas: 0` sidesteps
|
||||
entirely, since there is nothing for the wait to observe.
|
||||
|
||||
Do not warm at api_server startup. Blocking there would put all of Onyx's
|
||||
readiness behind the registry, and not just on first install: `:latest`
|
||||
is the default and is in `_MUTABLE_SANDBOX_IMAGE_TAGS`, for which
|
||||
`_ensure_sandbox_image` skips the local-presence check and always pulls.
|
||||
Every restart would contact the registry. Making it non-blocking instead
|
||||
just converts a broken deployment into a slow one that fails later, at a
|
||||
user's first sandbox.
|
||||
|
||||
The `background` worker also provisions (waking `SLEEPING` sandboxes via
|
||||
`scheduled_tasks/executor.py`) and needs nothing of its own: it shares
|
||||
the host's image store, so whichever pull happened first covers it.
|
||||
|
||||
### Private registries
|
||||
|
||||
Relevant only if you mirror the sandbox image into your own registry;
|
||||
|
||||
Reference in new issue
Block a user