From 811bcfd812bde8fc27f00cdb624da96035c26904 Mon Sep 17 00:00:00 2001 From: Piotr Mlocek Date: Wed, 19 Aug 2026 17:25:09 -0700 Subject: [PATCH 1/2] fix(podman): wait for container stop completion Signed-off-by: Piotr Mlocek --- architecture/compute-runtimes.md | 4 + crates/openshell-driver-podman/README.md | 4 + crates/openshell-driver-podman/src/driver.rs | 128 ++++++++++++++++++- 3 files changed, 134 insertions(+), 2 deletions(-) diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index 831be067ab..906e6908ec 100644 --- a/architecture/compute-runtimes.md +++ b/architecture/compute-runtimes.md @@ -99,6 +99,10 @@ same resource. The gateway requires a fresh supervisor session before a starting sandbox returns to `Ready`; stale driver snapshots and supervisor sessions cannot promote a `Stopped` row. +A driver stop operation does not complete while its backend still reports an +in-progress stop. This prevents an immediate start from racing the previous +run's delayed exit event and regressing the new run to `Error`. + Persisted `Stopping` and `Starting` rows are retried at startup. Stable `Stopped` rows remain stopped. Docker and Podman retain the stopped container and attached storage, Kubernetes retains the Sandbox CR and PVC while scaling diff --git a/crates/openshell-driver-podman/README.md b/crates/openshell-driver-podman/README.md index 67ba07944e..ffd04132c9 100644 --- a/crates/openshell-driver-podman/README.md +++ b/crates/openshell-driver-podman/README.md @@ -28,6 +28,10 @@ volume. Stopped managed containers remain visible through list and watch reconciliation. Delete remains responsible for removing the container, driver-owned secrets, and workspace volume. +The stop call waits until Podman reports the container as stopped or exited. +This keeps an immediate start from racing a rootless Podman stop that is still +finishing after its API request returns. + ## Architecture The Podman driver communicates with the Podman daemon over a Unix socket and diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index 9f8a62ef29..307fe8366f 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -36,6 +36,9 @@ use std::time::Duration; use tracing::{debug, info, warn}; use url::Url; +const STOP_COMPLETION_POLL_INTERVAL: Duration = Duration::from_millis(50); +const STOP_COMPLETION_TIMEOUT_HEADROOM: Duration = Duration::from_secs(5); + impl From for ComputeDriverError { fn from(value: PodmanApiError) -> Self { match value { @@ -914,22 +917,65 @@ impl PodmanComputeDriver { Ok(entries.into_iter().next()) } + async fn wait_for_container_stopped( + &self, + sandbox_id: &str, + container_id: &str, + ) -> Result<(), ComputeDriverError> { + let timeout = Duration::from_secs(u64::from(self.config.stop_timeout_secs)) + + STOP_COMPLETION_TIMEOUT_HEADROOM; + let deadline = tokio::time::Instant::now() + timeout; + + loop { + let inspect = self + .client + .inspect_container(container_id) + .await + .map_err(ComputeDriverError::from)?; + if matches!(inspect.state.status.as_str(), "exited" | "stopped") { + return Ok(()); + } + + let now = tokio::time::Instant::now(); + if now >= deadline { + return Err(ComputeDriverError::Message(format!( + "container {container_id} for sandbox {sandbox_id} did not finish stopping within {timeout:?} (last state: {})", + inspect.state.status, + ))); + } + tokio::time::sleep(STOP_COMPLETION_POLL_INTERVAL.min(deadline - now)).await; + } + } + /// Stop a sandbox container without deleting it. pub async fn stop_sandbox(&self, sandbox_id: &str) -> Result<(), ComputeDriverError> { let container = self .find_container(sandbox_id) .await? .ok_or(ComputeDriverError::NotFound)?; + let container_id = container.id; + if container.state == "stopping" { + return self + .wait_for_container_stopped(sandbox_id, &container_id) + .await; + } if container.state != "running" { return Ok(()); } - let container_id = container.id; info!(sandbox_id = %sandbox_id, container = %container_id, "Stopping sandbox container"); self.client .stop_container(&container_id, self.config.stop_timeout_secs) .await - .map_err(ComputeDriverError::from) + .map_err(ComputeDriverError::from)?; + + // Libpod can acknowledge the stop request while the container still + // reports `stopping`, especially after escalating from SIGTERM to + // SIGKILL in rootless mode. Do not let a following start race that + // transition: its delayed die event would otherwise regress the new + // run from Starting to Error. + self.wait_for_container_stopped(sandbox_id, &container_id) + .await } /// Start a previously stopped sandbox container. @@ -1437,6 +1483,10 @@ mod tests { vec![ StubResponse::new(StatusCode::OK, r#"[{"Id":"ctr-1","State":"running"}]"#), StubResponse::new(StatusCode::NO_CONTENT, ""), + StubResponse::new( + StatusCode::OK, + r#"{"Id":"ctr-1","Name":"sandbox","State":{"Status":"exited","Running":false,"FinishedAt":"2026-08-12T16:39:13Z"},"Config":{}}"#, + ), ], ); test_driver(stop_socket.clone()) @@ -1453,6 +1503,12 @@ mod tests { api_path("/libpod/containers/ctr-1/stop?timeout=10") ) ); + assert_eq!( + stop_requests + .lock() + .expect("request log lock should not be poisoned")[2], + format!("GET {}", api_path("/libpod/containers/ctr-1/json")) + ); let (start_socket, start_requests, start_handle) = spawn_podman_stub( "lifecycle-start", @@ -1487,6 +1543,74 @@ mod tests { let _ = fs::remove_file(start_socket); } + #[tokio::test] + async fn stop_waits_for_the_container_to_leave_stopping_state() { + let (socket, requests, handle) = spawn_podman_stub( + "lifecycle-stop-wait", + vec![ + StubResponse::new(StatusCode::OK, r#"[{"Id":"ctr-1","State":"running"}]"#), + StubResponse::new(StatusCode::NO_CONTENT, ""), + StubResponse::new( + StatusCode::OK, + r#"{"Id":"ctr-1","Name":"sandbox","State":{"Status":"stopping","Running":true},"Config":{}}"#, + ), + StubResponse::new( + StatusCode::OK, + r#"{"Id":"ctr-1","Name":"sandbox","State":{"Status":"exited","Running":false,"FinishedAt":"2026-08-12T16:39:13Z"},"Config":{}}"#, + ), + ], + ); + + test_driver(socket.clone()) + .stop_sandbox("sandbox-1") + .await + .expect("stop should wait for the terminal container state"); + handle.await.expect("stop stub should finish"); + + let requests = requests + .lock() + .expect("request log lock should not be poisoned"); + assert_eq!(requests.len(), 4); + assert_eq!( + requests[2], + format!("GET {}", api_path("/libpod/containers/ctr-1/json")) + ); + assert_eq!(requests[3], requests[2]); + + let _ = fs::remove_file(socket); + } + + #[tokio::test] + async fn stop_retry_waits_for_an_existing_stopping_container() { + let (socket, requests, handle) = spawn_podman_stub( + "lifecycle-stop-retry", + vec![ + StubResponse::new(StatusCode::OK, r#"[{"Id":"ctr-1","State":"stopping"}]"#), + StubResponse::new( + StatusCode::OK, + r#"{"Id":"ctr-1","Name":"sandbox","State":{"Status":"exited","Running":false,"FinishedAt":"2026-08-12T16:39:13Z"},"Config":{}}"#, + ), + ], + ); + + test_driver(socket.clone()) + .stop_sandbox("sandbox-1") + .await + .expect("stop retry should wait for the terminal container state"); + handle.await.expect("stop retry stub should finish"); + + let requests = requests + .lock() + .expect("request log lock should not be poisoned"); + assert_eq!(requests.len(), 2); + assert_eq!( + requests[1], + format!("GET {}", api_path("/libpod/containers/ctr-1/json")) + ); + + let _ = fs::remove_file(socket); + } + #[test] fn validate_gpu_request_accepts_gpu_count_request_shape() { let gpu = GpuResourceRequirements { count: Some(2) }; From f13b0a1a5d07234c1d68a731a97140b506e45f71 Mon Sep 17 00:00:00 2001 From: Piotr Mlocek Date: Wed, 19 Aug 2026 17:33:42 -0700 Subject: [PATCH 2/2] docs(podman): clarify stop restart race Signed-off-by: Piotr Mlocek --- crates/openshell-driver-podman/src/driver.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index 307fe8366f..23175b3bf4 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -969,11 +969,11 @@ impl PodmanComputeDriver { .await .map_err(ComputeDriverError::from)?; - // Libpod can acknowledge the stop request while the container still - // reports `stopping`, especially after escalating from SIGTERM to - // SIGKILL in rootless mode. Do not let a following start race that - // transition: its delayed die event would otherwise regress the new - // run from Starting to Error. + // Podman can return from the stop request before inspect reports the + // container as exited. If start runs during that interval, the exit + // event from the previous run can arrive after the gateway has moved + // the same sandbox to Starting, causing it to regress to Error. Wait + // for the terminal container state before allowing a restart. self.wait_for_container_stopped(sandbox_id, &container_id) .await }