feat(pod,serverless): add --wait and --wait-timeout to create (CON-689) - #317
Draft
justinwlin wants to merge 6 commits into
Draft
feat(pod,serverless): add --wait and --wait-timeout to create (CON-689)#317justinwlin wants to merge 6 commits into
justinwlin wants to merge 6 commits into
Conversation
… read (CON-689) create returns when a resource is scheduled, not when it is usable. add the machinery for an opt-in wait: - internal/waitfor: one bounded poll loop with injectable clock, throttled stderr progress and a typed error carrying the resource id, the last known state and a stable code (wait_timeout / wait_interrupted). - internal/waitfor.ProbeSSH: tcp connect plus ssh banner. verified against prod that a cpu pod running alpine has a public port 22 listed in runtime.ports while the connection is refused, so port allocation is not readiness. no handshake, so --wait works without a configured ssh key. - internal/api.GetEndpointHealth: the invoke service's live worker counts, which is the only readiness signal for an endpoint (includeWorkers is historical). - internal/sshconnect.PublicSSHPort: the port-22 lookup BuildConnection already did, now shared. - internal/duration: the pod list --since parser, moved so --wait-timeout reuses it instead of adding a third duration parser. also runs gofmt over api/endpoint.go and cmd/pod/list.go, which were already unformatted on main.
pod create --wait blocks until the pod's public port 22 answers with an ssh banner, then prints the same payload as 'pod get' (the create response has no ssh info, and a pod you can connect to is the point of waiting). serverless create --wait blocks until /health reports a ready or running worker. it requires --workers-min >= 1: at 0 runpod starts no worker until a request arrives, so the wait could only ever time out. refuse up front rather than silently billing a warm worker the user did not ask for. --wait-timeout defaults to 10m. on timeout or ctrl-c the resource is kept, the exit code is non-zero and the error names the id, the last known state and the delete command. progress goes to stderr on a 15s cadence; stdout stays a single json object. --wait cannot be combined with --ssh=false, and warns on cpu pods, which are created over rest and so never get runpod-managed ssh. the legacy project ssh loop now calls the shared wait too, dropping its re-poll-inside-the-condition bug and its post-loop timeout check that could fire on a poll that had just succeeded. its stdout line and 1s/5m timings are unchanged.
four cases: the cpu timeout path (an image with no sshd, which prod still gives a public port 22 — the exact state the wait must not read as ready), the gpu success path asserting one json object with a live ssh command, the free workers-min refusal, and a warm-worker endpoint wait. every paid resource is torn down in t.Cleanup, endpoint before template. runCLI now honours RUNPODCTL_BIN so a run can target a 'go build' output instead of overwriting the installed ~/go/bin/runpodctl.
… resources (CON-689)
review found three ways --wait misbehaved once it was actually waiting.
a single bad poll ended the whole wait, and it surfaced with the underlying
transport code. verified against a fake control plane: /health 404s an endpoint
id the invoke service has not propagated yet, so `serverless create --wait`
could exit ~0.04s after create with `{"code":"not_found","status":404}` for an
endpoint that exists and is billing a warm worker; one graphql blip did the same
to `pod create --wait` with `network_error`, the one code readme documents as
"transient, retry" — an agent following that would buy a second pod. poll errors
are now the current state, not the end of the wait; only failures that cannot
resolve (unauthorized, forbidden, no_credentials, bad_request) stop it.
a pod that can never become ready burned the whole budget: a terminal
desiredStatus polled for the full 10m default, and a pod terminated out of band
mid-wait read as "pod not listed yet" for the rest of it while the error claimed
it was "still billing". both now end the wait at once (conflict / not_found).
`pod create --wait` could exit 0 with `"ssh": {"error": "ssh info unavailable"}`:
the post-wait re-read swallows a graphql failure into that blob, so the one field
the flag exists to produce went missing with a success exit code. the re-read now
retries and then fails loudly, naming the address that did answer.
also: --wait on community cloud without --public-ip is warned about the way the
cpu path already was (no publicly mapped port 22 to probe, so it could only time
out), and errors that leave a resource behind carry its id in the error object's
new `id` field instead of only in prose.
…the workers-min claim (CON-689)
second review round. all of these were reproduced through the real binary
against a fake control plane before the fix and re-run after.
- isFatalPollError now also consults the http status (400/401/403). the pod
wait's only api call is graphql GetPods(), and every graphql failure is an
*api.GraphQLError whose ErrorCode() is the constant "graphql_error", so none
of fatalPollCodes was reachable there: a bad key burned the whole budget
while the pod billed and reported wait_timeout. 15.05s -> 0.41s.
- serverless create --wait no longer refuses --workers-min 0. the premise was
wrong: ai-api floors workersStandby to 5 whenever workersMax > 1 regardless
of workersMin (pkg/graphql/aiapi.go finalEndpoint), worker.Sync fills it with
cache workers (pkg/worker/sync.go) and every /health read triggers a Sync
(pkg/loader/aiapi.go), and /health counts a cached worker as ready. six prod
endpoints with workersMin unset report ready 1-5. it now warns, like the
other satisfiable-but-often-not combinations, and the refuted claim is out of
the error string, README and AGENTS.md.
- the endpoint poller had no fatal case, so an endpoint deleted out of band
burned the full budget while the pod path failed fast. a /health 404 after a
successful read is now fatal not_found; a 404 before the first read
(propagation lag) and any 5xx stay transient.
- the endpoint detail string now reports `running`, and the success line carries
the detail, so a run says which counter satisfied it. running is written at
scheduling time (runpod-backend rentPod.ts), so an unattributed "ready after"
was not evidence of anything.
- the pod poller takes two consecutive missing reads before declaring a pod
deleted. one short list read is an unknown state like every other tolerated
anomaly, and the error asserted the pod "was terminated".
- FindPodConnection/ListConnections skip nil entries. graphql lists are
nullable, and --wait re-reads that list right after reporting success: a null
entry panicked with SIGSEGV and exit 2, replacing the json error object with
a stack trace.
- duration.Parse range-checks the product, not just the operand. 106752d and up
wrapped negative, so an out-of-range --wait-timeout was silently replaced by
the 10m default (and pod list --since 200000d returned []).
- the post-wait re-read passes includeMachine=true, so --wait no longer hands
back less than a plain create (graphql selected machine { gpuDisplayName
location }).
- two fail-fast tests set interval and timeout both to an hour, so a regression
hung until go test panicked at 10m; they now pass a 300ms budget.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
adds an opt-in
--waittopod createandserverless createso a create returns when the resource is usable, not when it is scheduled — today a pod reportsRUNNINGminutes before sshd answers, so agents sit in a poll loop guessing.https://linear.app/runpod/issue/CON-689
what changed
pod create --waitreturns when the pod's public port 22 accepts a tcp connection and answers with an ssh banner, then prints thepod getshape (so the payload carries the livesshblock).serverless create --waitreturns when the endpoint's/healthreports areadyorrunningworker.--wait-timeout(default10m, reuses the existing7d-aware duration parser, now shared ininternal/duration).internal/waitfor: one bounded poll loop with an injectable clock, throttled stderr progress, and a typed error carrying the last known state. progress is a caller-suppliedio.Writer, so the loop structurally cannot touch stdout.PodSSHConnectionloop (cmd/project/ssh.go) now calls the shared helper, dropping its re-poll-inside-the-condition bug and its post-loop timeout check that could fire on a poll that had just succeeded. its stdout line, wording and 1s/5m timings are unchanged (proven by a base-vs-head diff below).idfield, so await_timeoutnames the resource you now own as data, not prose.wait_timeout,wait_interrupted(added to theclient.govocabulary block, README's error table and AGENTS.md).deviations from the ticket
everything below departs from CON-689's wording. reviewer 2 re-derived each justification from
ai-api/runpod-backend/hostsource or a free live probe rather than taking this PR's word for it; their ruling is stated verbatim, including where it was notvalid.1. "
pod create --wait→ returns when ssh is reachable"PodSSHConnection(cmd/project/ssh.go:257) already polls pod runtime ports until ssh is up; extract that loop and reuse it."SSH-banner, not the presence of port 22 inruntime.ports. no handshake and no key, so--waitcan report success on an image whose sshd never got your key.runtime.portsis proxied from the host daemon (runpod-backend/node/graphql/schema/podRuntime.ts:38-43→model/src/hapiGateway/pod.ts:21-29), which builds it from docker's published-port table withIsIpPublicset purely fromHostIP == 0.0.0.0(host/pkg/docker/container.go:392-417). a docker port binding exists from container start and says nothing about anything listening inside. measured: analpine:3.20cpu pod had a public 22 in ~25s whilenc -vzwas refused. a handshake was rejected as an alternative because--waitmust work beforerunpodctl doctorhas ever run.alpine:3.20pod plus onenc -vzwould settle it). the reachable-but-unauthenticated gap is disclosed in the flag help, the README table and the code.2. "
serverless create --wait→ returns when at least one worker is ready"/healthreportsready > 0 || running > 0. neither counter is as strong as the ticket's wording, and/healthexposes no third one.readycounts a flashboot-cached worker whose record readsdesiredStatus: EXITED(ai-api/pkg/api/health.go:109-117+pkg/workerstate/ready.go:119-130), so the first request resumes it.runningcountsdesiredStatus: RUNNING, which the control plane writes when the worker row is created — at scheduling time, before any container exists (runpod-backend/model/src/pod/rentPod.ts:401).runningstill has to count: a--workers-minworker staysRUNNINGfor its whole life and never entersready, so dropping it would make the flag's main case wait forever.running, the one counter that can decide readiness, so the success case was unattributable. fixed in this round:runningis now in the detail string, and the success line carries the detail too (ready after 25s: workers ready 0, running 1, ...), so a run says which clause fired. the weaker-than-asked predicate itself remains, disclosed here and in the README's ready-means table — there is no stronger live signal to switch to.3.
serverless create --waitused to require--workers-min >= 1--workers-min.finalEndpointfloorsworkersStandbyto 5 wheneverworkersMax > 1regardless ofworkersMin(ai-api/pkg/graphql/aiapi.go:435-450; the CLI's own--workers-maxdefault is 3).worker.Syncthen runsstartMinPods+launchWarmPodsfor any endpoint withstandby > 0(pkg/worker/sync.go:115-121, 175-199), and every/healthread triggers aSync(pkg/api/health.go:36→pkg/loader/aiapi.go:74) — so the CLI's own polling drives the provisioning. those cache workers land asEXITED+ cached, which/healthcounts asready. live probe on this account, six endpoints, all withworkersMinunset:ready3/5/1/2/1/5 withrunning0.invalid— not merely understated. fixed in this round rather than argued: the refusal is gone.--workers-min 0now emits a stderrnote:(the same treatment already given to--compute-type CPUand community-without---public-ip) and the wait runs. the refuted claim was also removed from the error string, the README and AGENTS.md, where it was becoming durable guidance for the next agent.4. endpoint readiness is read from
/health, notGetEndpoint(includeWorkers)/health(so it followsRUNPOD_INVOKE_URL, notRUNPOD_API_URL).includeWorkersreturns historical records. live:serverless get mr4lkli7h69gwh --include-workers→ 6 workers, every onedesiredStatus: EXITED, while the same endpoint's/healthreportedready: 5.ai-api/pkg/api/health.go:109-117explains it: anEXITEDpod is bucketedreadywhen the machine has it cached.5. a transient api failure does not end a wait
400/401/403(or the codesunauthorized/forbidden/no_credentials/bad_request), a terminal pod status (conflict), and a resource absent from two consecutive reads (not_found)./healthlegitimately 404s an endpoint id it has not propagated yet (ai-api/pkg/api/userAuth.go:184-191,pkg/api/health.go:36-47, 20s dataloader cache inpkg/loader/aiapi.go:24-56)./health404 after a successful read is now fatalnot_found(10.5s instead of 90s in a fake-plane run); 404 before the first successful read, and any 5xx, stay transient. reviewer 1 separately found the auth fail-fast was unreachable on the pod path (every graphql failure carries the constant codegraphql_error), so a bad key burned the full budget and reportedwait_timeout; the fatal check now also consults the http status — 0.4s instead of 15s in the repro below.6.
pod create --waitprints thepod getshape, not the create responsesshblock, i.e.--waitchanges the stdout schema versus a plain create.machine { gpuDisplayName location }, which the re-read dropped. partly fixed in this round: the re-read now passesincludeMachine=true. what still differs and is not fixed: rest encodesenvas a map andportsas[]string, where graphql returns["KEY=VALUE"]and a comma-separated string. reviewer 2 could not diff the two payloads live without buying a pod.7.
--compute-type CPUand community-without---public-ipwarn instead of failing--ssh=falseis refused up front.startSshfield at all (cmd/pod/create.go:427-449vs the graphql path'sStartSsh), and--public-ipmaps tosupportPublicIp, which is a machine filter in the backend (model/src/pod/bidPod.ts:260,model/src/pod/cpu/deployPod/index.ts:128).--ssh=falseis genuinely unsatisfiable.8. the shared error object gained an optional
idfieldomitemptyidfield on the error object and two new codes (wait_timeout,wait_interrupted) — a change to the output contract of every command.--waitthat times out has already bought a billed resource, and a caller must not have to regex a message to find it. no existing error gains the field.client.govocabulary, README error table, AGENTS.md).9. the resource is never deleted on timeout or ctrl-c
10. one legacy behaviour change
cmd/project/ssh.go's stdout line, wording,%wwrapping and 1s/5m timings are byte-identical (diff below). one thing changed: a poll that succeeds at or after the 5m deadline now returns success instead of the old spurious"timeout waiting for pod"error.origin/mainand confirmed it is a fix, on an error path, and disclosed.the ticket carries no scope note offering a smaller cut, so there is no narrower version that was declined.
review fixes
two review rounds. every substantive finding was reproduced through the real binary against a fake control plane before fixing, and re-run after.
round 2 (this round)
--workers-min >= 1refusal rests on a false premise (deviation 3)ai-apisource + a six-endpoint live probegraphql_errorwait_timeoutto 0.4s with{"code":"graphql_error","status":401,"id":...}. the fatal check now also consults http status (400/401/403); 404/429/5xx stay transient/health404 after a successful read → fatalnot_foundin 10.5s instead of 90s. 404 before the first read, and any 5xx, still toleratedrunningis invisible in progress and in the timeout error, so nobody can tell which clause fired (deviation 2)runningis in the detail string, and the success line now carries the detailFindPodConnectionnil-derefs on anullentry in graphql'spodsarray, right after--waitreported successpanic: SIGSEGV, exit 2, 0 bytes on stdout, no json error object at all. one-line nil guard inFindPodConnectionandListConnectionsduration.Parsesilently returns a negative duration at>= 106752d, so an out-of-range--wait-timeoutwas replaced by the 10m default106752d→out of range (max 106751d)). also fixespod list --since 200000d, which returned[]myself{pods}read ended the wait withnot_foundasserting the pod "was terminated"machineblock the graphql create response carried (deviation 6)includeMachine=true; theenv/portstype difference remains and is disclosed abovecalls == 1still proves the fail-fastState.Erris unused;DefaultTimeout/DefaultProgressEvery/theEqualFoldstatus compare survive mutationpod create --waitpolls the account-wideGetPods()every 5s; the post-wait re-read's 2s backoff ignores the wait contextround 1 (previous round, 17 findings)
/health404s an endpoint id it has not propagated yet → the wait exited 0.04s after create with{"code":"not_found","status":404}for an endpoint that exists and bills a warm worker. one graphql blip did the same withnetwork_error— the one code README defines as "transient, retry", so an agent would have bought a second poddesiredStatuspolled the entire 20s test budget (10m by default); now 0.75s with codeconflict. a pod terminated out of band read "pod not listed yet" for the rest of the budget while the error claimed it was "still billing"; nownot_found, and the wording no longer asserts it is runningpod create --waitcan exit 0 with"ssh": {"error": ...}--public-ipis unsatisfiable and unwarnedidfield on the error object; the e2e cleanup readsobj["id"]instead of slicing proseproject/execfailure text changed and stopped wrapping the error%wwrapping restored, soerrors.Is/Asreaches the typedno_credentialssentinel againt.Fatalfpaths could orphan a billed resourceTestUntilWithoutProgressWriterStaysSilentasserted nothing about silenceos.Stdout/os.Stderrand asserts zero bytes< 1→< 0mutation of the workers-min guard< 1; a concurrent process left that edit in the shared worktree mid-review and it reverted itself. (moot now — the guard is gone, see deviation 3)testing
gofmt -l .clean ·go vet ./...clean ·go vet -tags e2e ./e2e/...clean ·go test ./...all 16 packages ok ·go run ./docs/docs-gen.goregenerated (committed). coverage:internal/waitfor95%+,internal/duration100%,internal/output86.5%.round-2 reproductions, before and after (real binary, fake control plane)
in every failure path stdout was 0 bytes and the flat json error object was the last stderr line.
live prod e2e, with proof of cleanup (round 1; the paid paths were not re-bought)
the full
go test -tags e2e ./e2e/...was not run: it creates many billed resources and asserts on account-wide list output, which would race with sibling agents on the same prod account.not re-verified in round 2, deliberately: the shared prod account is billed by the second and round 1's paid evidence stands. re-buying it would settle the ~25s alpine port-allocation measurement (deviation 1) and the create-vs-
--waitpayload diff (deviation 6). round 2's verification was free:ai-api/runpod-backend/hostsource, read-only prod probes (six/healthreads,serverless get --include-workers, a bogus-key graphql 401), and the real binary against a fake control plane.legacy project/exec behaviour: base vs head, byte-identical
both binaries run against an unreachable graphql (
RUNPOD_GRAPHQL_URL=http://127.0.0.1:9), each polling the full legacy 5m budget:re-run after the round-2 changes: still byte-identical on both streams.
follow-ups
--wait/--wait-timeoutsemantics, whatreadyactually proves, thepod getoutput shape, and thewait_timeout/wait_interruptedcodes + the newidfield/healthcounter does (deviation 2):readyis a flashboot-cachedEXITEDworker,runningis a scheduled row. today's predicate is the strongest thing a client can observeinternal/api.GetEndpointHealthmay collide with CON-688 (serverless health); whichever lands second should reuse the other