Skip to content

fix(sandbox): fail a Kubernetes create fast when its Pod cannot start - #4566

Merged
yannickmonney merged 2 commits into
mainfrom
fix/sandbox-k8s-create-fail-fast
Oct 9, 2026
Merged

yannickmonney merged 2 commits into
mainfrom
fix/sandbox-k8s-create-fail-fast

Conversation

@yannickmonney

Copy link
Copy Markdown
Contributor

On Kubernetes, a session whose runner crash-loops or whose image cannot be pulled held its admission slot and the user's turn for the full 180 s create budget and failed with an opaque message.

Changes

fix(sandbox): fail a Kubernetes create fast when its Pod cannot start

A Kubernetes create waited for its Pod's address and then for runnerd,
polling every 500 ms, until SANDBOX_SESSION_CREATE_TIMEOUT_MS (180 s by
default), and read nothing about the containers. A crash-looping runner
(an entrypoint FATAL), an image the node cannot pull, or an invalid image
or configuration therefore held an admission slot and the user's turn for
the whole budget and ended as an opaque "runnerd did not become ready";
a crash-looping container still gets a Pod IP, so the address wait never
noticed either.

While it waits, the create now reads its Pod about once a second and,
once the runner or the egress sidecar shows CrashLoopBackOff,
ErrImagePull, ImagePullBackOff, ErrImageNeverPull, InvalidImageName,
CreateContainerConfigError or CreateContainerError, fails at once with
the reason, the last exit and the end of the container's log (the runner
now reports its logs as its termination message). Both waits stop with
it. A container still being created or pulled keeps the create waiting.
runnerd readiness is asked every 100 ms, as on Docker.

Verification

  • services/sandbox: src/backend/kubernetes/ 123 pass, including 9 new cases (classifier per reason, crash detail, egress sidecar, still-starting states, and an end-to-end create that fails within seconds; that test hangs to its timeout against main's backend).
  • @tale/sandbox typecheck and lint clean; lint:manual ok.
  • runnerd-client.test.ts keeps the 3 macOS-only hangs it has on unmodified main.
  • Not run against a real cluster.

Part of the sandbox footprint and robustness pass (#4533, #4538 and siblings).

@yannickmonney

Copy link
Copy Markdown
Contributor Author

ACCEPT — reviewed head .

Part B dispositions: CODE PASS; RELIABILITY PASS; RESOURCES PASS. The watcher checks init and regular container statuses for terminal waiting reasons, runs beside endpoint/runnerd readiness under one shared deadline, aborts both waits on failure, retries transient Pod-read errors, and UID/resourceVersion-fences cleanup while preserving the PVC. Tests cover crash loops, image/config failures, egress, recoverable states, and fast rejection.

CICD PASS: CI-ready Build, Checks, E2E, Security, SAST, CLI, and Commitlint aggregators are successful at this exact head; scoped skips are acceptable.

Part A UX-CLARITY PASS: the error names session, container, Kubernetes reason, last exit, and log tail; the guide documents recovery. CODE PASS applies to termination-message policy and test/docs alignment. No numbered findings.

@yannickmonney

Copy link
Copy Markdown
Contributor Author

Correction to the verdict above: reviewed head is 3b00628. ACCEPT. No numbered findings.

@yannickmonney

Copy link
Copy Markdown
Contributor Author

REQUEST CHANGES — reviewed head 3b00628, base 730aa10. This supersedes my ACCEPT comments 6065161117 and 6065163736. I did not implement this PR.

Part B first:

  1. [P2; design question, not a reproduced defect] services/sandbox/src/backend/kubernetes/k8s-session-backend.ts:86-94, 551-558: CrashLoopBackOff, ErrImagePull and ImagePullBackOff are retryable kubelet states, not proof that a Pod cannot recover within the create budget. A slow DinD boot exits at services/sandbox-runtime/entrypoint.sh:725 after 30 s; the startup probe intentionally allows the full create budget at k8s-session-pod-spec.ts:399-410. If the watcher observes the resulting backoff, it now rejects and starts cleanup immediately, whereas the former readiness wait could allow a restart or subsequent image-pull attempt to succeed before the deadline. ContainerCreating/PodInitializing remain tolerated, but that does not cover this case. Please explicitly decide/document whether one observed backoff should end the create, or retain a bounded recovery allowance for transient cases, and add the corresponding regression. A real slow-start/restart scenario is NOT_RUN; I do not claim it was reproduced.

  2. [P2] services/sandbox/src/backend/kubernetes/k8s-session-backend.ts:113-120: arbitrary waiting.message and lastState.terminated.message are concatenated into the error; slicing the latter to 500 characters only bounds size. FallbackToLogsOnError at k8s-session-pod-spec.ts:358 imports runner log content rather than an allowlisted public diagnostic. session-routes.ts:1902-1904 forwards error.message into create_failed; services/platform/backend/core/node_only/sandbox/helpers/session_client.ts:629-631 embeds the response text verbatim in the platform exception. Thus any payload/secret appearing in that tail reaches both error channels without redaction. The entrypoint includes dynamic diagnostics (for example exception text at :315 and buildx log tail at :820), so a blanket payload-free claim is unsupported. Keep raw diagnostics in an authorized operator channel and expose an allowlisted/redacted reason, with a sentinel-payload regression. Actual sensitive data leakage and end-user rendering are NOT_RUN/unverified; the unsanitized propagation is source-confirmed.

Old-fails answer (source-inferred; execution NOT_RUN): the added test “a create fails fast with the reason instead of waiting out its budget” in k8s-session-backend.test.ts:1740 onward asserts that error.message contains “session ... cannot start: container runner CrashLoopBackOff”, contains the FATAL detail, and elapsed time is below 10,000 ms. With the old create readiness path and this fixture, there is no watcher: it waits on an unreachable runnerd until its budget (or the test runner timeout), so those message/timing assertions cannot pass. The helper tests also require the newly exported unstartableReason; removing the whole change makes them fail to import, which is not behavioral regression proof. No old/new execution was performed.

Dispositions: CODE BLOCKED on finding 2; RELIABILITY BLOCKED on finding 1's unresolved recovery contract; RESOURCES PASS by source inspection for existing acknowledged-UID/resourceVersion cleanup and retained PVC (live cleanup NOT_RUN); CICD PASS for the previously inspected exact-head 79 check runs, zero failed, seven CI-ready aggregators green, as independently reconciled in this task—no new log/execution claim, scoped skips acceptable; UX-CLARITY BLOCKED on finding 2 and unverified end-user visibility. Existing shared deadline and abort wiring remain sound. Real-cluster, test execution and rendered UX are NOT_RUN under this API-only scope.

@yannickmonney
yannickmonney added this pull request to the merge queue Oct 9, 2026
@yannickmonney
yannickmonney removed this pull request from the merge queue due to a manual request Oct 9, 2026
@yannickmonney
yannickmonney force-pushed the fix/sandbox-k8s-create-fail-fast branch from 38b951d to 7511839 Compare October 9, 2026 02:40
A Kubernetes create waited for its Pod's address and then for runnerd,
polling every 500 ms, until SANDBOX_SESSION_CREATE_TIMEOUT_MS (180 s by
default), and read nothing about the containers. A crash-looping runner
(an entrypoint FATAL), an image the node cannot pull, or an invalid image
or configuration therefore held an admission slot and the user's turn for
the whole budget and ended as an opaque "runnerd did not become ready";
a crash-looping container still gets a Pod IP, so the address wait never
noticed either.

While it waits, the create now reads its Pod about once a second and,
once the runner or the egress sidecar shows CrashLoopBackOff,
ErrImagePull, ImagePullBackOff, ErrImageNeverPull, InvalidImageName,
CreateContainerConfigError or CreateContainerError, fails at once with
the reason, the last exit and the end of the container's log (the runner
now reports its logs as its termination message). Both waits stop with
it. A container still being created or pulled keeps the create waiting.
runnerd readiness is asked every 100 ms, as on Docker.
The automation coverage map now keeps one file per suite, so parallel changes stop appending to the same lines; this change's rows move there, after their own anchor row.
@yannickmonney
yannickmonney force-pushed the fix/sandbox-k8s-create-fail-fast branch from 7511839 to 8166d57 Compare October 9, 2026 02:41
@yannickmonney
yannickmonney disabled auto-merge October 9, 2026 02:55
@yannickmonney
yannickmonney merged commit 6300f97 into main Oct 9, 2026
@yannickmonney
yannickmonney deleted the fix/sandbox-k8s-create-fail-fast branch October 9, 2026 03:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant