Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 36 additions & 1 deletion crates/openshell-cli/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ use std::process::Command;
use std::time::{Duration, Instant};
use tonic::{Code, Status};

const PROVISIONAL_CONTAINER_EXIT_RECONCILIATION_TIMEOUT: Duration = Duration::from_secs(5);

// Re-export SSH functions for backward compatibility
pub use crate::ssh::{Editor, print_ssh_config};
pub use crate::ssh::{
Expand Down Expand Up @@ -253,6 +255,19 @@ fn has_main_process_result(sandbox: &Sandbox) -> bool {
})
}

fn is_provisional_container_exit(sandbox: &Sandbox) -> bool {
let phase = SandboxPhase::try_from(sandbox.phase()).unwrap_or(SandboxPhase::Unknown);
phase == SandboxPhase::Error
&& sandbox.status.as_ref().is_some_and(|status| {
status.exit_code.is_none()
&& status.conditions.iter().any(|condition| {
condition.r#type == "Ready"
&& condition.status.eq_ignore_ascii_case("false")
&& condition.reason == "ContainerExited"
})
})
}

fn build_sandbox_resource_limits(
cpu: Option<&str>,
memory: Option<&str>,
Expand Down Expand Up @@ -763,6 +778,12 @@ pub async fn sandbox_create(
.unwrap_or(300),
);
let mut provisioning_idle_deadline = Instant::now() + provision_timeout;
// The compute driver can publish ContainerExited while the supervisor's
// authoritative canonical-process result is waiting for the same gateway
// state lock. Keep watching briefly so the provisional error cannot race
// ephemeral cleanup, but retain a deadline for containers that exit before
// the supervisor can report a result.
let mut provisional_container_exit_deadline: Option<Instant> = None;
// Track whether we saw the gateway become ready (from log messages).
let mut saw_gateway_ready = false;

Expand All @@ -771,8 +792,15 @@ pub async fn sandbox_create(
// longer than the default timeout pulling and preparing large images,
// but only recognized progress events extend the idle deadline. Logs
// and generic status churn must not keep a stuck sandbox alive forever.
let remaining = provisioning_idle_deadline.saturating_duration_since(Instant::now());
let now = Instant::now();
let mut remaining = provisioning_idle_deadline.saturating_duration_since(now);
if let Some(deadline) = provisional_container_exit_deadline {
remaining = remaining.min(deadline.saturating_duration_since(now));
}
if remaining.is_zero() {
if provisional_container_exit_deadline.is_some() {
break;
}
let timeout_message = provisioning_timeout_message(
provision_timeout.as_secs(),
resource_requirements.as_ref(),
Expand All @@ -792,6 +820,7 @@ pub async fn sandbox_create(
let item = match maybe_item {
Ok(Some(item)) => item,
Ok(None) => break, // stream ended
Err(_elapsed) if provisional_container_exit_deadline.is_some() => break,
Err(_elapsed) => {
// Timeout fired — the stream was idle for too long.
let timeout_message = provisioning_timeout_message(
Expand Down Expand Up @@ -848,6 +877,12 @@ pub async fn sandbox_create(
format!("{}: {}", condition.reason, condition.message);
}
}
if is_provisional_container_exit(&s) {
provisional_container_exit_deadline.get_or_insert_with(|| {
Instant::now() + PROVISIONAL_CONTAINER_EXIT_RECONCILIATION_TIMEOUT
});
continue;
}
break;
}

Expand Down
151 changes: 150 additions & 1 deletion crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
use std::time::{Duration, Instant};
use tempfile::TempDir;
use tokio::net::TcpListener;
use tokio::sync::{Mutex, mpsc};
use tokio::sync::{Mutex, Notify, mpsc};
use tokio_stream::wrappers::TcpListenerStream;
use tonic::transport::{Certificate as TlsCertificate, Identity, Server, ServerTlsConfig};
use tonic::{Response, Status};
Expand All @@ -54,6 +54,9 @@ struct SandboxState {
vm_slow_progress_before_ready: Arc<AtomicBool>,
vm_log_churn_before_ready: Arc<AtomicBool>,
terminal_before_relay: Arc<AtomicBool>,
terminal_after_provisional_container_exit: Arc<AtomicBool>,
provisional_container_exit_without_result: Arc<AtomicBool>,
provisional_container_exit_sent: Arc<Notify>,
ssh_session_failures_remaining: Arc<AtomicUsize>,
ssh_session_requests: Arc<AtomicUsize>,
global_settings: Arc<Mutex<HashMap<String, SettingValue>>>,
Expand Down Expand Up @@ -561,6 +564,16 @@ impl OpenShell for TestOpenShell {
.load(Ordering::SeqCst);
let vm_log_churn_before_ready = self.state.vm_log_churn_before_ready.load(Ordering::SeqCst);
let terminal_before_relay = self.state.terminal_before_relay.load(Ordering::SeqCst);
let terminal_after_provisional_container_exit = self
.state
.terminal_after_provisional_container_exit
.load(Ordering::SeqCst);
let provisional_container_exit_without_result = self
.state
.provisional_container_exit_without_result
.load(Ordering::SeqCst);
let provisional_container_exit_sent =
Arc::clone(&self.state.provisional_container_exit_sent);

tokio::spawn(async move {
let mut provisioning = Sandbox {
Expand Down Expand Up @@ -604,11 +617,44 @@ impl OpenShell for TestOpenShell {
});
completed.set_phase(SandboxPhase::Completed as i32);

let mut provisional_container_exit = error.clone();
if let Some(ready) = provisional_container_exit
.status
.as_mut()
.and_then(|status| status.conditions.first_mut())
{
ready.reason = "ContainerExited".to_string();
ready.message = "Sandbox container exited".to_string();
}

let _ = tx
.send(Ok(SandboxStreamEvent {
payload: Some(sandbox_stream_event::Payload::Sandbox(provisioning)),
}))
.await;
if terminal_after_provisional_container_exit
|| provisional_container_exit_without_result
{
let _ = tx
.send(Ok(SandboxStreamEvent {
payload: Some(sandbox_stream_event::Payload::Sandbox(
provisional_container_exit,
)),
}))
.await;
provisional_container_exit_sent.notify_waiters();
if provisional_container_exit_without_result {
std::future::pending::<()>().await;
return;
}
tokio::task::yield_now().await;
let _ = tx
.send(Ok(SandboxStreamEvent {
payload: Some(sandbox_stream_event::Payload::Sandbox(completed)),
}))
.await;
return;
}
if vm_error_after_started {
let _ = tx
.send(Ok(SandboxStreamEvent {
Expand Down Expand Up @@ -2184,6 +2230,109 @@ async fn sandbox_create_retries_terminal_attachment_until_relay_registers() {
);
}

#[tokio::test]
async fn sandbox_create_waits_for_main_result_after_provisional_container_exit() {
let server = run_server().await;
server
.openshell
.state
.terminal_after_provisional_container_exit
.store(true, Ordering::SeqCst);
let fake_ssh_dir = tempfile::tempdir().unwrap();
let xdg_dir = tempfile::tempdir().unwrap();
let _env = test_env(&fake_ssh_dir, &xdg_dir);
let tls = test_tls(&server);
install_fake_ssh(&fake_ssh_dir);

let exit_code = run::sandbox_create(
&server.endpoint,
"openshell",
run::SandboxCreateConfig {
name: Some("fast-ephemeral-command"),
keep: false,
command: &["echo".into(), "OK".into()],
..test_config()
},
"default",
&tls,
)
.await
.expect("a provisional container exit must yield to the canonical main-process result");

assert_eq!(exit_code, 0);
assert_eq!(
deleted_names(&server).await,
vec![vec!["fast-ephemeral-command".to_string()]]
);
}

#[tokio::test]
async fn sandbox_create_bounds_provisional_container_exit_reconciliation() {
let server = run_server().await;
server
.openshell
.state
.provisional_container_exit_without_result
.store(true, Ordering::SeqCst);
let fake_ssh_dir = tempfile::tempdir().unwrap();
let xdg_dir = tempfile::tempdir().unwrap();
let _env = test_env(&fake_ssh_dir, &xdg_dir);
let tls = test_tls(&server);
install_fake_ssh(&fake_ssh_dir);

let provisional_container_exit_sent = server
.openshell
.state
.provisional_container_exit_sent
.notified();
tokio::pin!(provisional_container_exit_sent);
let command = ["echo".into(), "OK".into()];
let create = run::sandbox_create(
&server.endpoint,
"openshell",
run::SandboxCreateConfig {
name: Some("missing-main-result"),
command: &command,
..test_config()
},
"default",
&tls,
);
tokio::pin!(create);

tokio::select! {
() = &mut provisional_container_exit_sent => {}
result = &mut create => panic!("sandbox create returned before the provisional exit was observed: {result:?}"),
}
let reconciliation_started = Instant::now();
let result = tokio::time::timeout(Duration::from_secs(10), &mut create)
.await
.expect("provisional container exit reconciliation must remain bounded");
let reconciliation_elapsed = reconciliation_started.elapsed();

let err = result
.expect_err("a missing canonical main-process result must retain the container exit error");

let rendered = err.to_string();
assert!(
rendered.contains("sandbox entered error phase while provisioning"),
"unexpected error: {rendered}"
);
assert!(
rendered.contains("ContainerExited: Sandbox container exited"),
"unexpected error: {rendered}"
);
assert!(
!rendered.contains("timed out"),
"unexpected error: {rendered}"
);
assert!(
reconciliation_elapsed >= Duration::from_secs(5),
"provisional container exit returned before reconciliation: {reconciliation_elapsed:?}"
);
assert!(deleted_names(&server).await.is_empty());
}

#[tokio::test]
async fn sandbox_create_deletes_command_sessions_with_no_keep() {
let server = run_server().await;
Expand Down
Loading