Repository navigation
fix(kubernetes): keep a late stop snapshot from reviving a stopped sandbox - #5196
Conversation
…ndbox A stop keeps the sandbox-runtime bootstrap annotations, with operation "stop", until its completion patch. The watch converter marked every object with those annotations as a starting generation: it added Bootstrapping=True and dropped Suspended. The gateway treats that condition as an accepted start that may replace a settled Stopped phase, so a snapshot taken mid-stop and delivered after the stop settled moved the sandbox from Stopped to Provisioning and the next stop was rejected. For operation "stop", withdraw readiness and withhold the suspension until completion as before, but do not report Bootstrapping. Create and restart keep their existing marking, so a real start still recovers a stale Stopped view. Closes #5195 Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
|
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. |
|
/ok to test d397a4a |
|
Label |
matthewgrossman
added
the
test:e2e-kubernetes
|
Label |
Replace the late-stop gateway test with one that walks a stop from Stopping through completion and then replays the in-flight snapshot, with and without a connected supervisor session. An in-flight stop snapshot, which now withdraws readiness without Bootstrapping or Suspended, keeps the sandbox Stopping rather than Provisioning or Error; completion settles it Stopped; and the late replay leaves it Stopped. The test documents that it guards the gateway contract and that the driver test covers the regression itself. Share the condition reset between the stopping and bootstrapping markers in the Kubernetes driver. Refs #5195 Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
…ress/matthewgrossman Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
elezar
left a comment
elezar
left a comment
There was a problem hiding this comment.
LGTM. The fix is scoped to the driver and leaves the start path and the gateway's stale-view recovery unchanged. I like that the driver and gateway are tested separately.
Two non-blocking observations:
sandbox_from_object_with_bootstrap_statetreats only"stop"specially, so any other or future operation value is reported as a start (Bootstrapping=True). That is a reasonable default, but it is the same shortcut that caused #5195, so a new operation will need to be routed deliberately.- Mid-stop snapshots no longer carry any "operation in progress" signal. Before this PR they reported
Bootstrapping=True. Now they report onlyReady=FalsewithDependenciesNotReady, which the gateway can't tell apart from other not-ready states. The new gateway test shows this settles correctly fromStopping, but I don't see coverage for a stale gateway view (for example stillRunning) receiving such a snapshot. This is worth covering in a follow-up, along with better-defined operation and generation semantics for the sandbox conditions.
Approving as is.
TL;DR
Problem: On Kubernetes, stopping a sandbox could make it look like it was starting again. The driver marks a sandbox with the same "operation in progress" flag during a start and during a stop, and it reported every flagged sandbox to the gateway as starting. A status update from the middle of a stop can arrive late, after the gateway has already marked the sandbox
Stopped. The gateway took that late update as a sign that a start had begun, moved the sandbox back toProvisioning, and then rejected the next stop.Fix: The driver now checks which operation set the flag. During a stop, it still reports the sandbox as not ready, but it no longer says a start is underway, so a late update can't bring back a stopped sandbox. Starts and restarts are reported as before.
Summary
On Kubernetes, a stopped sandbox could flip back to
Provisioningwith no start, and the next stop was then rejected. This happened when the driver delivered a snapshot taken during the stop after the stop had already settled. The driver now stops labelling stop-time snapshots as a starting generation.Related Issue
Closes #5195
Changes
openshell-driver-kubernetes: the watch and get converter now checks which operation set the sandbox-runtime bootstrap annotations. For a stop (bootstrap-operation: "stop"), it still withdraws readiness and still holds backSuspendeduntil the stop's completion patch, but it no longer addsBootstrapping=True. Create, restart and annotations without an operation keep the existing marking. The marking moved into a small synchronous helper,sandbox_from_object_with_bootstrap_state, so it can be unit tested without a Kubernetes client.Bootstrapping=TruenorSuspended=True, and isn't Ready. This fails onmain.Bootstrapping=True.kubernetes_stop_snapshots_settle_without_reviving_the_sandbox: walks a stop through a stop-time snapshot (staysStopping, neverProvisioningorError), the completed stop (Stopped), and a late replay of the stop-time snapshot (staysStopped), with and without a connected supervisor session. It passes onmain; the driver test covers the regression.Why fix the driver and not add a gateway guard:
Bootstrapping=Trueas authoritative evidence of an accepted start.active_bootstrap_snapshot_recovers_stale_stopped_viewrelies on that to move aStoppedview the gateway didn't know was stale.main: it pins the gateway's half of the contract for the snapshot shape the driver now emits.Testing
cargo fmt --all -- --checkcargo clippy --locked -p openshell-driver-kubernetes -p openshell-server --all-targets -- -D warningscargo nextest run -p openshell-driver-kubernetes: 332 passedcargo nextest run -p openshell-server: 2666 passed, 38 skipped (PostgreSQL-backed)stop_snapshot_does_not_report_a_starting_generationfailed before the fix and passes after it.lifecycle::restart_stopped_sandbox, which is where it was seen.Checklist