From b98b78aecafac3ee0cb9d1c6daac109153df80c8 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Thu, 20 Aug 2026 11:45:57 +0200 Subject: [PATCH] fix(docker): preserve provisioning failure status Signed-off-by: Evan Lezar --- crates/openshell-driver-docker/src/lib.rs | 47 +++++++++++++++--- crates/openshell-driver-docker/src/tests.rs | 55 +++++++++++++++++++++ 2 files changed, 95 insertions(+), 7 deletions(-) diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index b1fb5ec22..c06fcf325 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -674,6 +674,14 @@ impl DockerComputeDriver { sandbox_id: &str, sandbox_name: &str, ) -> Result, Status> { + let pending = self.pending_snapshot(sandbox_id, sandbox_name).await; + if pending + .as_ref() + .is_some_and(pending_sandbox_has_provisioning_failure) + { + return Ok(pending); + } + let container = self .find_managed_container_summary(sandbox_id, sandbox_name) .await?; @@ -683,19 +691,17 @@ impl DockerComputeDriver { return Ok(Some(sandbox)); } - Ok(self.pending_snapshot(sandbox_id, sandbox_name).await) + Ok(pending) } async fn current_snapshots(&self) -> Result, Status> { let containers = self.list_managed_container_summaries().await?; - let container_sandboxes = containers + let mut by_id = containers .iter() .filter_map(sandbox_from_container_summary) - .collect::>(); - let mut by_id = self.pending_snapshot_map().await; - for sandbox in container_sandboxes { - by_id.insert(sandbox.id.clone(), sandbox); - } + .map(|sandbox| (sandbox.id.clone(), sandbox)) + .collect::>(); + merge_pending_sandbox_snapshots(&mut by_id, self.pending_snapshot_map().await); let mut sandboxes = by_id.into_values().collect::>(); sandboxes.sort_by(|left, right| left.id.cmp(&right.id)); Ok(sandboxes) @@ -1828,6 +1834,33 @@ fn pending_sandbox_matches(sandbox: &DriverSandbox, sandbox_id: &str, sandbox_na || (!sandbox_name.is_empty() && sandbox.name == sandbox_name) } +/// A pending sandbox holds either the ordinary in-progress snapshot or the +/// explicit error published by its provisioning task. The latter is more +/// informative than a transient Docker state observed while that task cleans +/// up a failed start, so it must win during snapshot reconciliation. +fn pending_sandbox_has_provisioning_failure(sandbox: &DriverSandbox) -> bool { + sandbox.status.as_ref().is_some_and(|status| { + status.conditions.iter().any(|condition| { + condition.r#type == "Ready" + && condition.status.eq_ignore_ascii_case("false") + && condition.reason != "Starting" + }) + }) +} + +fn merge_pending_sandbox_snapshots( + snapshots: &mut HashMap, + pending: HashMap, +) { + for (sandbox_id, sandbox) in pending { + if pending_sandbox_has_provisioning_failure(&sandbox) { + snapshots.insert(sandbox_id, sandbox); + } else { + snapshots.entry(sandbox_id).or_insert(sandbox); + } + } +} + fn provisioning_condition() -> DriverCondition { DriverCondition { r#type: "Ready".to_string(), diff --git a/crates/openshell-driver-docker/src/tests.rs b/crates/openshell-driver-docker/src/tests.rs index eddfd778b..c717acb06 100644 --- a/crates/openshell-driver-docker/src/tests.rs +++ b/crates/openshell-driver-docker/src/tests.rs @@ -2238,6 +2238,61 @@ fn pending_sandbox_snapshot_uses_docker_namespace_and_starting_condition() { assert_eq!(status.conditions[0].message, "Docker container is starting"); } +#[test] +fn pending_provisioning_failure_overrides_transient_container_state() { + let sandbox = test_sandbox(); + let dead_container = pending_sandbox_snapshot( + &sandbox, + "default", + error_condition("ContainerDead", "Container is dead"), + false, + ); + let start_failure = pending_sandbox_snapshot( + &sandbox, + "default", + error_condition( + "ContainerStartFailed", + "Docker responded with status code 500: CDI device injection failed", + ), + false, + ); + + let mut snapshots = HashMap::from([(sandbox.id.clone(), dead_container)]); + merge_pending_sandbox_snapshots( + &mut snapshots, + HashMap::from([(sandbox.id.clone(), start_failure)]), + ); + + let status = snapshots[&sandbox.id].status.as_ref().expect("status"); + assert_eq!(status.conditions[0].reason, "ContainerStartFailed"); + assert!( + status.conditions[0] + .message + .contains("CDI device injection failed") + ); +} + +#[test] +fn pending_starting_snapshot_does_not_override_container_state() { + let sandbox = test_sandbox(); + let dead_container = pending_sandbox_snapshot( + &sandbox, + "default", + error_condition("ContainerDead", "Container is dead"), + false, + ); + let starting = pending_sandbox_snapshot(&sandbox, "default", provisioning_condition(), false); + + let mut snapshots = HashMap::from([(sandbox.id.clone(), dead_container)]); + merge_pending_sandbox_snapshots( + &mut snapshots, + HashMap::from([(sandbox.id.clone(), starting)]), + ); + + let status = snapshots[&sandbox.id].status.as_ref().expect("status"); + assert_eq!(status.conditions[0].reason, "ContainerDead"); +} + #[test] fn validate_linux_elf_binary_rejects_non_elf_files() { let tempdir = TempDir::new().unwrap();