Repository navigation
fix(sandbox): fail a Kubernetes create fast when its Pod cannot start - #4566
Conversation
|
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. |
|
Correction to the verdict above: reviewed head is 3b00628. ACCEPT. No numbered findings. |
|
REQUEST CHANGES — reviewed head 3b00628, base 730aa10. This supersedes my ACCEPT comments 6065161117 and 6065163736. I did not implement this PR. Part B first:
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. |
38b951d to
7511839
Compare
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.
7511839 to
8166d57
Compare
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
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/sandboxtypecheck and lint clean; lint:manual ok.runnerd-client.test.tskeeps the 3 macOS-only hangs it has on unmodified main.Part of the sandbox footprint and robustness pass (#4533, #4538 and siblings).