fix(gateway): give image preparation its own deadline - #4038
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
🌿 Preview your docs: https://nvidia-preview-pr-4038.docs.buildwithfern.com/openshell |
drew
left a comment
drew
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
The separate persisted image-preparation deadline is a sound direction, but the initial create operation does not yet honor it. A stalled supported driver can keep the lifecycle gate past expiry, preventing the cleanup this change promises and potentially removing the retained timeout diagnosis if the driver later fails.
Action required: make initial sandbox creation deadline-aware, retain the timeout record, and cover the timeout-to-cleanup path with a deterministic blocked-create test.
Blocking findings:
GATOR-87755ffe-01: InitialCREATE_SANDBOXcan outlive the persisted preparation deadline while holding the cleanup gate.
Carried findings:
- None
Non-blocking suggestions:
- None
Gator metadata
- Validation: Project-valid maintainer-authored implementation of accepted issue #3952.
- Docs: Relevant Fern lifecycle, policy-repair, and gateway configuration docs are updated.
- Checks: Current-head Branch Checks, Helm Lint, Trivy Changes, DCO, docs preview, and required gate publishers are green.
- E2E: Required for this sandbox lifecycle change after review feedback is resolved; dispatch is deferred while the blocker remains.
- Head SHA:
87755ffe78fce6e3eb36c215abc6eae6657e0037 - Base SHA:
1b77cd4e931258d7e33beb486d202d1086d23dee - Merge base SHA:
1b77cd4e931258d7e33beb486d202d1086d23dee - Patch ID:
cb0afd2a5019c5da89e46b19b39a507abdfc8a7f - Gator payload:
10 - Review mode:
initial - Previous reviewed SHA: none
- Review budget exhausted: no
- Maintainer decision required: no
- Next state:
gator:in-review
|
Label |
drew
left a comment
drew
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
The update now releases the local lifecycle gate at the preparation deadline and preserves its timeout record, but two cleanup-safety obligations remain: the new deadline waiter conflates a store-read failure with a driver failure, and the existing cross-replica late-success race remains open.
Action required: preserve the durable record when deadline monitoring fails, fence cross-replica cleanup against a create that can still succeed, and add the two deterministic regressions described in the findings.
Blocking findings:
GATOR-70022e55-01: A transient deadline-monitor read failure can delete the sandbox record while backend create is still able to complete.
Carried findings:
GATOR-87755ffe-01: Initial create must not produce compute after another replica records timeout cleanup complete and allows retry. The existing review thread remains authoritative.
Gator metadata
- Validation: Project-valid maintainer-authored implementation of accepted issue #3952.
- Docs: Relevant Fern lifecycle, policy-repair, and gateway configuration docs are updated.
- Checks: Current-head Branch Checks and E2E workflows are running; no completed current-head failure is present.
- E2E:
test:e2eis applied and the required workflow is running. - Head SHA:
70022e55f69f2661756056d3474e6fb9edf58bba - Base SHA:
1b77cd4e931258d7e33beb486d202d1086d23dee - Merge base SHA:
1b77cd4e931258d7e33beb486d202d1086d23dee - Patch ID:
598c6eeca4b8121a23671903e826da1f447f496e - Gator payload:
10 - Review mode:
follow_up - Previous reviewed SHA:
87755ffe78fce6e3eb36c215abc6eae6657e0037 - Review budget exhausted: no
- Maintainer decision required: no
- Next state:
gator:in-review
PR Review StatusThe follow-up delta resolves both durable Gator findings: pending driver ownership now survives deadline or monitor cancellation, late results are fenced by operation identity, and cleanup waits for settlement before completing. The independent follow-up review found no new blocking code issue, but the current required Branch Checks are red on deterministic Rust tests. Action required: @shiju-nv, fix the current-head failures in Blocking findings:
Carried findings:
Gator metadata
|
drew
left a comment
drew
left a comment
There was a problem hiding this comment.
gator-agent
PR Review Status
The critical-only follow-up review found no newly introduced Critical defect in the latest author delta. Both prior Gator findings remain resolved, the deterministic Branch Checks failures are fixed, and the current-head E2E gate is still running.
Blocking findings:
- No blocking findings remain.
Carried findings:
- None;
GATOR-87755ffe-01andGATOR-70022e55-01remain resolved.
Gator metadata
- Validation: Project-valid maintainer-authored implementation of accepted issue #3952.
- Docs: Relevant Fern lifecycle, policy-repair, and gateway configuration docs are updated.
- Checks: Current-head Branch Checks, Helm Lint, and Trivy Changes are green; the required E2E gate is pending.
- E2E:
test:e2eis applied and the current-head E2E workflow is running. - Head SHA:
befb0b6aeabae9b061b989a11033c793561bc4e9 - Base SHA:
1b77cd4e931258d7e33beb486d202d1086d23dee - Merge base SHA:
1b77cd4e931258d7e33beb486d202d1086d23dee - Patch ID:
97ff7ae4b2641744ba9d6d9f6e2ef08b58e1bb7b - Gator payload:
10 - Review mode:
critical_only - Previous reviewed SHA:
3542c6238e3cc3488ba9a9f1d220a2697a308151 - Review budget exhausted:
yes - Maintainer decision required:
no - Next state:
gator:watch-pipeline
drew
added
gator:watch-pipeline
Give sandbox image preparation its own deadline and start the admission deadline after preparation finishes. Persist both phases across gateway restarts and show recovery guidance for the phase that expired. Preserve the current service authorization schema and regenerate the Go bindings with the preparation timestamps. Fixes #3952 Related to #3955 Signed-off-by: Shiju <shiju@nvidia.com>
Use lazy Option fallbacks while preserving timeout messages and retained sandbox behavior. Signed-off-by: Shiju <shiju@nvidia.com>
Signed-off-by: Shiju <shiju@nvidia.com>
Release stalled create operations after preparation expires and preserve the timeout diagnosis across late driver results. Keep failed-create cleanup bound to its original attempt so another replica can retry safely. Signed-off-by: Shiju <shiju@nvidia.com>
Drop the create-error mutex guard before matching its cloned value and use idiomatic iteration and timeout matching in the deadline fixtures. Signed-off-by: Shiju <shiju@nvidia.com>
Keep submitted create and start requests alive after caller cancellation, monitor failure, or preparation timeout. Persist request ownership before dispatch and retain staged uploads while the driver response is pending. Require timeout cleanup to stop compute after driver settlement without discarding an active cleanup claim. Fence late result handling against newer operations, preserve failed-start recovery, and defer automatic restart while another request owns the sandbox. Expose pending ownership in CLI JSON. Add ordered multi-replica and cancellation regressions for the review findings. Signed-off-by: Shiju <shiju@nvidia.com>
Carry the request span into the detached provisioning worker so compute driver calls remain attached to their parent trace after task handoff. Update the compensation regression to require no backend DELETE when the durable cleanup claim fails, matching the operation ownership requirement. Accept the gateway's current Ready snapshot when a sandbox becomes ready before CREATE returns. Do not require the client to observe an earlier provisioning phase. Cover the Ready-only watch and command attachment. Signed-off-by: Shiju <shiju@nvidia.com>
drew
added
gator:merge-ready
and removed
gator:approval-needed
befb0b6 to
97b404c
Compare
drew
added
gator:watch-pipeline
drew
added
gator:merge-ready
and removed
gator:watch-pipeline
Merge ReadyGator validation and PR monitoring are complete, and maintainer approval is present. The current head is rebase-equivalent to the reviewed patch, so no duplicate code review was run. Review: Both prior Gator findings remain resolved, with no unresolved review feedback. Human maintainer merge or close decision is now required. Gator metadata
|
Monitoring CompleteMonitoring is complete because this PR has merged. Final status: Gator review findings were resolved, required checks and E2E passed, and verified maintainer approval was present before merge. I removed the active |
3 participants
Summary
An uncached image can use the gateway's entire five-minute policy repair window before the supervisor starts. Give image preparation a persisted deadline, then start the existing repair window when the current supervisor sends its first authenticated configuration report. Preparation timeouts direct users to image preparation and supervisor startup diagnostics and the preparation budget.
Related Issue
Closes #3952, accepted as part of #3955. The maintainer's accepted triage identifies both premature cold-image timeout and incorrect configuration-repair guidance.
The gateway invokes
StopSandboxafter timeout. The VM worker and staging cleanup implementation belongs to #3953.Changes
image_preparation_timeout_seconds, defaulting to 1800 and restricted to 1 through 86400. Persist each attempt's absolute deadline so progress, configuration writes and restarts cannot extend preparation.ImagePreparationTimedOutseparately from admission timeout. Show the phase in CLI JSON and TUI notes, give each timeout the appropriate recovery guidance, and update bindings and lifecycle documentation.At the deadline, the caller releases its lifecycle lock so cleanup can stop partial compute. Cleanup must issue another stop after the original driver request settles before declaring completion, while preserving any stop already in progress. The sandbox record retains its timeout diagnosis and cleanup status.
A monitoring error no longer enters failed-create record deletion. Actual driver rejection keeps its existing recovery behavior. If persisting a known driver result fails transiently, the owner retains the result and retries. Automatic restart respects pending ownership, and CLI JSON exposes the pending flag.
The CLI accepts an initial Ready watch snapshot when provisioning finishes before CREATE returns. The detached worker preserves its request trace, and the compensation regression requires zero backend DELETE calls when a cleanup claim fails.
If the gateway owning a driver request crashes before recording its result, pending ownership and any protected upload can remain indefinitely. There is currently no API to establish settlement after that loss. Actual driver and transport-error semantics are unchanged.
Testing
Local verification passes scoped server Clippy, touched-package checks, formatting, license checks, and focused ownership, cleanup and recovery regressions. The CLI library serializer test executes and passes. Bindings and schema checks pass for Go, TypeScript and Python. Deterministic controls detect monitor-error deletion, premature cleanup completion, lost STOP leases and stale CREATE handling before correction.
The additional CLI regression keeps a Ready-only watch open and verifies command attachment. It fails with the old wait condition and passes with the correction. Focused terminal-result, progress-timeout and error-reporting tests also pass. Independent local Gator review of the follow-up has no unresolved blocker or high finding.
The CI follow-up is signed and pushed as
befb0b6aeabae9b061b989a11033c793561bc4e9. Hosted checks for this commit contain its CI results. Earlier runs on3542c6238e3ccover the previous version.Hosted VM E2E runs on Linux. A physical-Mac cold preparation exceeding five minutes remains unverified.
Checklist