Skip to content

Commit b4baf23

Browse files
authored
fix(compute): allow stopping errored sandboxes for recovery (#4231)
* fix(compute): allow stopping errored sandboxes for recovery Signed-off-by: Shiju <shiju@nvidia.com> * test(compute): focus sandbox error recovery coverage Remove duplicate cancellation and idempotency checks covered by existing lifecycle tests. Keep recovery guidance focused on the stop/start commands and timeout exception. Signed-off-by: Shiju <shiju@nvidia.com> --------- Signed-off-by: Shiju <shiju@nvidia.com>
1 parent e30c938 commit b4baf23

4 files changed

Lines changed: 302 additions & 3 deletions

File tree

‎crates/openshell-server/src/compute/mod.rs‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1560,10 +1560,24 @@ impl ComputeRuntime {
15601560
return Ok(current);
15611561
}
15621562
let automatic_restart = is_automatic_restart_transition(&current);
1563-
if !matches!(phase, SandboxPhase::Ready | SandboxPhase::Stopping) && !automatic_restart
1563+
// Timeout cleanup owns reclamation while the record remains Error.
1564+
// Its later start may recreate missing compute; an ordinary stop must
1565+
// not replace that recovery state with Stopping or Stopped.
1566+
if phase == SandboxPhase::Error && provisioning_deadline::timed_out(&current) {
1567+
return Err(Status::failed_precondition(
1568+
"sandbox provisioning timed out; retry start after timeout cleanup completes",
1569+
));
1570+
}
1571+
// Error does not prove compute has stopped. Use the normal driver stop
1572+
// and session cleanup before a subsequent start renews authentication
1573+
// and repeats configuration admission for the retained sandbox.
1574+
if !matches!(
1575+
phase,
1576+
SandboxPhase::Ready | SandboxPhase::Stopping | SandboxPhase::Error
1577+
) && !automatic_restart
15641578
{
15651579
return Err(Status::failed_precondition(format!(
1566-
"sandbox must be Ready or in an automatic restart to stop (current phase: {phase:?})"
1580+
"sandbox must be Ready, Error, or in an automatic restart to stop (current phase: {phase:?})"
15671581
)));
15681582
}
15691583

@@ -1775,8 +1789,13 @@ impl ComputeRuntime {
17751789
) && !is_failed_main_process_result(&current)
17761790
&& !provisioning_timeout
17771791
{
1792+
let recovery_hint = if phase == SandboxPhase::Error {
1793+
"; stop the sandbox before retrying start"
1794+
} else {
1795+
""
1796+
};
17781797
return Err(Status::failed_precondition(format!(
1779-
"sandbox must be Stopped, Completed, or a failed main-process Error to start (current phase: {phase:?})"
1798+
"sandbox must be Stopped, Completed, or a failed main-process Error to start (current phase: {phase:?}){recovery_hint}"
17801799
)));
17811800
}
17821801
if phase == SandboxPhase::Completed || is_failed_main_process_result(&current) {
@@ -7390,6 +7409,8 @@ pub fn new_test_runtime_with_driver(
73907409

73917410
#[cfg(test)]
73927411
mod tests {
7412+
mod error_recovery;
7413+
73937414
use super::*;
73947415
use futures::stream;
73957416
use openshell_core::proto::compute::v1::{
Lines changed: 272 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,272 @@
1+
// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
//! Explicit recovery must stop retained compute before issuing a fresh launch.
5+
6+
use super::*;
7+
use crate::auth::sandbox_session::PersistedSandboxIdentity;
8+
use openshell_core::proto::ConfigurationAdmissionState;
9+
10+
async fn stored(runtime: &ComputeRuntime, id: &str) -> Sandbox {
11+
runtime
12+
.store
13+
.get_message::<Sandbox>(id)
14+
.await
15+
.unwrap()
16+
.unwrap()
17+
}
18+
19+
#[tokio::test]
20+
async fn failed_gateway_start_can_be_stopped_and_retried() {
21+
let driver = ControlledDriver::new();
22+
driver.set_start_outcome(ControlledLifecycleOutcome::Error(
23+
"supervisor endpoint unavailable",
24+
));
25+
driver.set_runtime_identity("retained-runtime");
26+
let mut runtime =
27+
test_runtime_with_gateway_managed_lifecycle(driver.clone(), "test-driver").await;
28+
enable_runtime_identity_binding(&mut runtime);
29+
let mut sandbox = sandbox_record("retained-id", "retained", SandboxPhase::Ready);
30+
sandbox.spec = Some(SandboxSpec {
31+
template: Some(SandboxTemplate {
32+
image: "example.test/workload:retained".into(),
33+
..Default::default()
34+
}),
35+
command: vec!["sleep".into(), "3600".into()],
36+
..Default::default()
37+
});
38+
sandbox.status.as_mut().unwrap().configuration_activated = Some(true);
39+
set_compute_runtime_binding(&mut sandbox, "retained-runtime");
40+
runtime.store.put_message(&sandbox).await.unwrap();
41+
42+
// Exercise the production startup sweep rather than inventing its failure record.
43+
runtime.start_persisted_sandboxes().await.unwrap();
44+
let failed = stored(&runtime, sandbox.object_id()).await;
45+
assert_eq!(failed.phase(), SandboxPhase::Error as i32);
46+
assert_eq!(ready_condition(&failed).unwrap().reason, "StartFailed");
47+
driver.set_start_outcome(ControlledLifecycleOutcome::Ok);
48+
runtime.start_persisted_sandboxes().await.unwrap();
49+
assert_eq!(
50+
driver.start_calls(),
51+
1,
52+
"gateway restart does not retry this Error"
53+
);
54+
let error = runtime
55+
.start_sandbox("default", "retained")
56+
.await
57+
.unwrap_err();
58+
assert_eq!(error.code(), Code::FailedPrecondition);
59+
assert!(
60+
error
61+
.message()
62+
.contains("stop the sandbox before retrying start")
63+
);
64+
65+
let session = ssh_session_record("old-session", sandbox.object_id());
66+
runtime.store.put_message(&session).await.unwrap();
67+
register_test_supervisor_session(&runtime, sandbox.object_id());
68+
let stopped = runtime.stop_sandbox("default", "retained").await.unwrap();
69+
assert_eq!(stopped.phase(), SandboxPhase::Stopped as i32);
70+
assert_eq!(stopped.object_id(), sandbox.object_id());
71+
assert_eq!(stopped.object_workspace(), sandbox.object_workspace());
72+
assert_eq!(stopped.spec, sandbox.spec);
73+
assert_eq!(
74+
sandbox_compute_runtime_identity(&stopped),
75+
"retained-runtime"
76+
);
77+
assert_eq!(
78+
driver.stop_requests(),
79+
vec![("retained-id".into(), "retained".into())]
80+
);
81+
assert_eq!(driver.delete_calls(), 0);
82+
assert!(!runtime.supervisor_sessions.has_session(sandbox.object_id()));
83+
assert!(
84+
runtime
85+
.store
86+
.get_message::<SshSession>(session.object_id())
87+
.await
88+
.unwrap()
89+
.is_none()
90+
);
91+
92+
let authority = test_session_authority();
93+
let starting = runtime
94+
.start_sandbox_authenticated("default", "retained", Some(&authority))
95+
.await
96+
.unwrap();
97+
assert_eq!(starting.phase(), SandboxPhase::Starting as i32);
98+
assert_eq!(starting.object_id(), sandbox.object_id());
99+
assert_eq!(starting.spec, sandbox.spec);
100+
let before =
101+
PersistedSandboxIdentity::read(&failed.metadata.as_ref().unwrap().annotations).unwrap();
102+
let after =
103+
PersistedSandboxIdentity::read(&starting.metadata.as_ref().unwrap().annotations).unwrap();
104+
assert_eq!(after.runtime_generation, before.runtime_generation);
105+
assert_eq!(after.auth_epoch.get(), before.auth_epoch.get() + 1);
106+
assert_ne!(after.gateway_token_id, before.gateway_token_id);
107+
assert_eq!(
108+
driver.start_expected_runtime_identities(),
109+
vec!["retained-runtime", "retained-runtime"]
110+
);
111+
let status = starting.status.as_ref().unwrap();
112+
assert_eq!(status.configuration_activated, Some(true));
113+
assert_eq!(
114+
status.configuration_admission.as_ref().unwrap().state,
115+
ConfigurationAdmissionState::Pending as i32
116+
);
117+
118+
// A live driver and connected supervisor cannot skip fresh configuration admission.
119+
register_test_supervisor_session(&runtime, sandbox.object_id());
120+
let snapshot = ready_driver_sandbox(sandbox.object_id(), sandbox.object_name());
121+
driver.set_get_outcome(ControlledGetOutcome::Sandbox(Box::new(snapshot.clone())));
122+
runtime.apply_sandbox_update(snapshot).await.unwrap();
123+
assert_eq!(
124+
stored(&runtime, sandbox.object_id()).await.phase(),
125+
SandboxPhase::Starting as i32
126+
);
127+
assert_eq!(driver.delete_calls(), 0);
128+
}
129+
130+
#[tokio::test]
131+
async fn explicit_stop_accepts_driver_error_reasons_without_an_allowlist() {
132+
for reason in ["ProcessExited", "ContainerExited", "DriverSpecificFailure"] {
133+
let driver = ControlledDriver::new();
134+
let runtime = test_runtime(driver.clone()).await;
135+
let sandbox = error_sandbox_record("error-id", "errored", reason);
136+
runtime.store.put_message(&sandbox).await.unwrap();
137+
138+
let stopped = runtime.stop_sandbox("default", "errored").await.unwrap();
139+
assert_eq!(stopped.phase(), SandboxPhase::Stopped as i32, "{reason}");
140+
assert_eq!(
141+
driver.stop_calls(),
142+
1,
143+
"Error is not proof that compute stopped"
144+
);
145+
assert_eq!(driver.delete_calls(), 0);
146+
}
147+
}
148+
149+
#[tokio::test]
150+
async fn pending_driver_operation_blocks_error_recovery_without_mutation() {
151+
let driver = ControlledDriver::new();
152+
let runtime = test_runtime(driver.clone()).await;
153+
let mut sandbox = error_sandbox_record("pending-id", "pending", "StartFailed");
154+
let mut operation = provisioning_deadline::new_record(openshell_core::time::now_ms());
155+
operation.driver_operation_pending = true;
156+
operation.driver_operation_id = "owned-operation".into();
157+
sandbox.status.as_mut().unwrap().provisioning = Some(operation);
158+
runtime.store.put_message(&sandbox).await.unwrap();
159+
let before = stored(&runtime, sandbox.object_id()).await;
160+
161+
let error = runtime
162+
.stop_sandbox("default", "pending")
163+
.await
164+
.unwrap_err();
165+
assert_eq!(error.code(), Code::FailedPrecondition);
166+
assert!(error.message().contains("operation is still pending"));
167+
assert_eq!(stored(&runtime, sandbox.object_id()).await, before);
168+
assert_eq!(driver.stop_calls(), 0);
169+
assert_eq!(driver.delete_calls(), 0);
170+
}
171+
172+
#[tokio::test]
173+
async fn timeout_stop_preserves_cleanup_and_direct_start_recovery() {
174+
for cleanup_complete in [false, true] {
175+
let driver = ControlledDriver::new();
176+
driver.set_start_outcome(ControlledLifecycleOutcome::NotFound);
177+
driver.track_compute.store(true, Ordering::SeqCst);
178+
let runtime = test_runtime(driver.clone()).await;
179+
let mut sandbox = error_sandbox_record("timeout-id", "timeout", "ProvisioningTimedOut");
180+
let mut operation = provisioning_deadline::new_record(0);
181+
operation.timeout_time = openshell_core::time::timestamp_from_millis(300_000).ok();
182+
operation.cleanup_retry_time = openshell_core::time::timestamp_from_millis(310_000).ok();
183+
if cleanup_complete {
184+
operation.cleanup_completed_time =
185+
openshell_core::time::timestamp_from_millis(320_000).ok();
186+
}
187+
sandbox.status.as_mut().unwrap().provisioning = Some(operation);
188+
runtime.store.put_message(&sandbox).await.unwrap();
189+
let before = stored(&runtime, sandbox.object_id()).await;
190+
191+
let error = runtime
192+
.stop_sandbox("default", "timeout")
193+
.await
194+
.unwrap_err();
195+
assert_eq!(error.code(), Code::FailedPrecondition);
196+
assert!(error.message().contains("start"));
197+
assert_eq!(stored(&runtime, sandbox.object_id()).await, before);
198+
assert_eq!(driver.stop_calls(), 0, "timeout cleanup owns reclamation");
199+
200+
let result = runtime.start_sandbox("default", "timeout").await;
201+
if cleanup_complete {
202+
assert_eq!(result.unwrap().phase(), SandboxPhase::Starting as i32);
203+
assert!(
204+
driver.compute_exists.load(Ordering::SeqCst),
205+
"start recreates the missing backend after cleanup"
206+
);
207+
assert_eq!(driver.start_calls(), 1);
208+
} else {
209+
assert_eq!(result.unwrap_err().code(), Code::FailedPrecondition);
210+
assert_eq!(stored(&runtime, sandbox.object_id()).await, before);
211+
assert_eq!(driver.start_calls(), 0);
212+
}
213+
assert_eq!(driver.delete_calls(), 0);
214+
}
215+
}
216+
217+
#[tokio::test]
218+
async fn failed_error_stop_retains_retryable_state_without_claiming_success() {
219+
for observed_error in [false, true] {
220+
let driver = ControlledDriver::new();
221+
driver.set_stop_outcome(ControlledLifecycleOutcome::Error(
222+
"stop response unavailable",
223+
));
224+
let runtime = test_runtime(driver.clone()).await;
225+
let sandbox = error_sandbox_record("retry-id", "retry", "ProcessExited");
226+
if observed_error {
227+
let mut snapshot = ready_driver_sandbox("retry-id", "retry");
228+
snapshot.status.as_mut().unwrap().conditions = vec![DriverCondition {
229+
r#type: "Ready".into(),
230+
status: "False".into(),
231+
reason: "ProcessExited".into(),
232+
message: "runtime exited".into(),
233+
..Default::default()
234+
}];
235+
driver.set_get_outcome(ControlledGetOutcome::Sandbox(Box::new(snapshot)));
236+
} else {
237+
// A response for a different sandbox cannot settle this one's stop.
238+
driver.set_get_outcome(ControlledGetOutcome::Sandbox(Box::new(
239+
ready_driver_sandbox("foreign-id", "retry"),
240+
)));
241+
}
242+
runtime.store.put_message(&sandbox).await.unwrap();
243+
244+
let error = runtime.stop_sandbox("default", "retry").await.unwrap_err();
245+
assert_eq!(error.code(), Code::Internal);
246+
let retained = stored(&runtime, sandbox.object_id()).await;
247+
assert_eq!(
248+
retained.phase(),
249+
if observed_error {
250+
SandboxPhase::Error
251+
} else {
252+
SandboxPhase::Stopping
253+
} as i32
254+
);
255+
assert_eq!(retained.object_id(), sandbox.object_id());
256+
driver.set_stop_outcome(ControlledLifecycleOutcome::Ok);
257+
assert_eq!(
258+
runtime
259+
.stop_sandbox("default", "retry")
260+
.await
261+
.unwrap()
262+
.phase(),
263+
SandboxPhase::Stopped as i32
264+
);
265+
assert_eq!(
266+
driver.stop_requests(),
267+
vec![("retry-id".into(), "retry".into()); 2]
268+
);
269+
assert_eq!(driver.start_calls(), 0);
270+
assert_eq!(driver.delete_calls(), 0);
271+
}
272+
}

‎docs/how-it-works/sandboxes/overview.mdx‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -943,6 +943,10 @@ command. Starting a fresh instance invalidates SSH sessions issued for the
943943
previous runtime generation. Delete an inactive sandbox normally when you no
944944
longer need its retained state.
945945
946+
If an infrastructure failure leaves a sandbox in `Error`, fix the reported cause, then run `sandbox stop` followed by `sandbox start`. Wait for stop to succeed before starting. This retains the sandbox and its persistent workspace on backends that support restarting stopped sandboxes; it cannot recover deleted resources or storage.
947+
948+
For a provisioning timeout, wait for cleanup to finish, then run `sandbox start` directly.
949+
946950
## Delete Sandboxes
947951
948952
Deleting a sandbox stops all processes, releases resources, and purges injected credentials.

‎skills/openshell-cli/SKILL.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -531,6 +531,8 @@ sandbox, even under the same name, creates a different identity. See the
531531
[sandbox SSH documentation](https://docs.nvidia.com/openshell/latest/how-it-works/sandboxes/overview.md)
532532
for verification behavior and release compatibility.
533533

534+
For an infrastructure `Error` on a backend that supports restarting stopped sandboxes, fix the reported cause, run `sandbox stop`, and wait for it to succeed before running `sandbox start`. This retains the sandbox and persistent workspace but cannot recover deleted resources or storage. For a provisioning timeout, wait for cleanup to finish, then run `sandbox start` directly.
535+
534536
---
535537

536538
## Workflow 4: Policy Iteration Loop

0 commit comments

Comments
 (0)