Skip to content

fix(kubernetes): serialize lifecycle cleanup with sandbox restart - #4078

Open
matthewgrossman wants to merge 1 commit into
mainfrom
codex/kubernetes-restart-cleanup-gate
Open

matthewgrossman wants to merge 1 commit into
mainfrom
codex/kubernetes-restart-cleanup-gate

Conversation

@matthewgrossman

@matthewgrossman matthewgrossman commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Summary

A Kubernetes reconciliation pass can retain a stopped or stopping Sandbox LIST snapshot while restart creates a replacement supervisor Pod at the same name. Cleanup can then delete the replacement before a later CR update detects the conflict. Serialize lifecycle mutations and reconciliation per sandbox within one driver instance, and refresh the CR under the gate before deciding whether to clean up.

This is a proposed quick fix for a reproduced race. The historical sandbox-stop-start CI failure lacked Kubernetes Pod diagnostics, so its exact cause remains unproven.

Related Issue

Refs #3954. No new issue was created, as explicitly requested; this PR does not close the broader investigation. This cleanup guard complements the bootstrap fence publication work in #4067 and diagnostics in #4072.

Changes

  • Share per-sandbox mutation gates across driver clones for create, stop, start, delete, and reconciliation. Skip busy sandboxes until the next reconciliation pass while preserving concurrency across sandboxes.
  • Refresh each listed CR under the gate and reject a replaced CR UID or changed sandbox identity before examining companion resources.
  • Add four regression tests covering stale stopped/releasing/suspending snapshots on both Sandbox API versions, a busy restart, CR replacement, and lifecycle gate sharing across clones. Update the driver README and cluster debugging skill.

The gate is process-local: separate gateway or external driver processes can still race. This draft does not change generic Kubernetes 404 error mapping or replace generation/UID fencing with distributed exclusion.

Testing

Rebased without conflicts onto 76cfd0e31d5e1633db7ccd86ad9023ef7a2461b2 (includes #4076). Current candidate: 9db51c01936c6eeafdc59c74612e9839e1623e1d. git range-diff confirms this PR's patch is unchanged.

  • Fresh Kubernetes driver library run on the rebased candidate: all 274 tests pass, including the four cleanup regressions.
  • Fresh cargo clippy --offline -p openshell-driver-kubernetes --all-targets -- -D warnings and git diff --check pass.
  • Full enabled Branch E2E run 36933371807, attempt 1 passed, including both Kubernetes Agent Sandbox API smoke lanes and ubuntu-k3s conformance.
  • Three actual successful executions of affected stop/start coverage per target lane.
Target lane Actual completed executions Fresh evidence
Agent Sandbox v1beta1 smoke 3/3 1 (success), 2 (success), 3 (success)
Agent Sandbox v1alpha1 smoke 3/3 1 (success), 2 (success), 3 (success)
ubuntu-k3s conformance 3/3 1 (success), 2 (success), 3 (success)

PR head, workflow heads, binary artifact sources, preparation checkouts, and image tags were verified against the candidate SHA. Carried-forward GitHub results with regenerated job IDs are excluded from repeat counts using actual execution timestamps and logs. No failures were observed in this campaign.

Optional Kubernetes HA and credential-driver suites were disabled in the full run and are not counted as passing coverage. Passing K3s logs show actual conformance archive execution and its success assertion; the pinned archive includes the unignored lifecycle scenario, but passing per-command output is suppressed.

The original commit passed the full configured pre-commit hook. Local cluster E2E previously stopped before scenarios because Docker was unavailable; that preflight attempt is not qualification evidence for this candidate.

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Relevant architecture and debugging documentation updated

@matthewgrossman matthewgrossman added the test:e2e Requires end-to-end coverage label Oct 1, 2026
@copy-pr-bot

copy-pr-bot Bot commented Oct 1, 2026

Copy link
Copy Markdown

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.

@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown

Label test:e2e applied, but pull-request/4078 does not exist yet. A maintainer needs to comment /ok to test 078d7e51e41f8f003f0e5a8f8d9c831871ffa1f5 to mirror this PR. Once the mirror exists, re-apply the label or re-run Branch E2E Checks from the Actions tab.

Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
@matthewgrossman
matthewgrossman force-pushed the codex/kubernetes-restart-cleanup-gate branch from 078d7e5 to 9db51c0 Compare October 1, 2026 22:09
@elezar

elezar commented Oct 2, 2026

Copy link
Copy Markdown
Member

/ok-to-test 9db51c0

@elezar
elezar marked this pull request as ready for review October 2, 2026 11:40
@elezar
elezar requested review from a team, derekwaynecarr, mrunalp and sjenning as code owners October 2, 2026 11:40

@elezar elezar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at 9db51c0. The per-sandbox gate and refresh under the gate address the reproduced lifecycle/reconciliation race within one Kubernetes driver instance. No blocking findings; all 274 Kubernetes driver unit tests passed locally, and the passing Kubernetes E2E run covers this commit.

We have started investigating the broader compute-driver lifecycle contract: observations and cleanup from an earlier sandbox operation must not stop, delete, or invalidate a runtime established by a later operation. That investigation includes local reproduction and remediation of the Podman restart/watch race, with the aim of addressing Podman failures; attribution to historical CI failures remains unproven. Cross-process Kubernetes/HA coordination also remains a separate follow-up. These broader gaps do not block this scoped fix.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:e2e Requires end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants