Skip to content

Commit 391031d

Browse files
committed
fix(docker): preserve provisioning failure status
Signed-off-by: Evan Lezar <elezar@nvidia.com>
1 parent 473d1e9 commit 391031d

2 files changed

Lines changed: 90 additions & 7 deletions

File tree

‎crates/openshell-driver-docker/src/lib.rs‎

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1279,8 +1279,12 @@ impl DockerComputeDriver {
12791279
sandbox_id: &str,
12801280
sandbox_name: &str,
12811281
) -> Result<Option<DriverSandbox>, Status> {
1282-
if let Some(pending) = self.pending_snapshot(sandbox_id, sandbox_name).await? {
1283-
return Ok(Some(pending));
1282+
let pending = self.pending_snapshot(sandbox_id, sandbox_name).await?;
1283+
if pending
1284+
.as_ref()
1285+
.is_some_and(pending_sandbox_has_provisioning_failure)
1286+
{
1287+
return Ok(pending);
12841288
}
12851289
let container = self
12861290
.find_managed_container_summary(sandbox_id, sandbox_name)
@@ -1292,7 +1296,7 @@ impl DockerComputeDriver {
12921296
return Ok(Some(sandbox));
12931297
}
12941298

1295-
Ok(None)
1299+
Ok(pending)
12961300
}
12971301

12981302
async fn current_snapshots(&self) -> Result<Vec<DriverSandbox>, Status> {
@@ -1341,10 +1345,7 @@ impl DockerComputeDriver {
13411345
.into_iter()
13421346
.map(|sandbox| (sandbox.id.clone(), sandbox))
13431347
.collect::<HashMap<_, _>>();
1344-
// Provisioning state is authoritative until the supervisor has
1345-
// attached to both the sandbox and gateway. A running workload
1346-
// container alone is not a usable sandbox.
1347-
by_id.extend(self.pending_snapshot_map().await);
1348+
merge_pending_sandbox_snapshots(&mut by_id, self.pending_snapshot_map().await);
13481349
let mut sandboxes = by_id.into_values().collect::<Vec<_>>();
13491350
sandboxes.sort_by(|left, right| left.id.cmp(&right.id));
13501351
Ok(sandboxes)
@@ -3397,6 +3398,33 @@ fn pending_sandbox_record_id(
33973398
Ok(first)
33983399
}
33993400

3401+
/// A pending sandbox holds either the ordinary in-progress snapshot or the
3402+
/// explicit error published by its provisioning task. The latter is more
3403+
/// informative than a transient Docker state observed while that task cleans
3404+
/// up a failed start, so it must win during snapshot reconciliation.
3405+
fn pending_sandbox_has_provisioning_failure(sandbox: &DriverSandbox) -> bool {
3406+
sandbox.status.as_ref().is_some_and(|status| {
3407+
status.conditions.iter().any(|condition| {
3408+
condition.r#type == "Ready"
3409+
&& condition.status.eq_ignore_ascii_case("false")
3410+
&& condition.reason != "Starting"
3411+
})
3412+
})
3413+
}
3414+
3415+
fn merge_pending_sandbox_snapshots(
3416+
snapshots: &mut HashMap<String, DriverSandbox>,
3417+
pending: HashMap<String, DriverSandbox>,
3418+
) {
3419+
for (sandbox_id, sandbox) in pending {
3420+
if pending_sandbox_has_provisioning_failure(&sandbox) {
3421+
snapshots.insert(sandbox_id, sandbox);
3422+
} else {
3423+
snapshots.entry(sandbox_id).or_insert(sandbox);
3424+
}
3425+
}
3426+
}
3427+
34003428
fn provisioning_condition() -> DriverCondition {
34013429
DriverCondition {
34023430
r#type: "Ready".to_string(),

‎crates/openshell-driver-docker/src/tests.rs‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3160,6 +3160,61 @@ fn pending_lookup_is_id_authoritative_and_rejects_ambiguous_names() {
31603160
assert!(pending_sandbox_record_id(&pending, "", "demo").is_err());
31613161
}
31623162

3163+
#[test]
3164+
fn pending_provisioning_failure_overrides_transient_container_state() {
3165+
let sandbox = test_sandbox();
3166+
let dead_container = pending_sandbox_snapshot(
3167+
&sandbox,
3168+
"default",
3169+
error_condition("ContainerDead", "Container is dead"),
3170+
false,
3171+
);
3172+
let start_failure = pending_sandbox_snapshot(
3173+
&sandbox,
3174+
"default",
3175+
error_condition(
3176+
"ContainerStartFailed",
3177+
"Docker responded with status code 500: CDI device injection failed",
3178+
),
3179+
false,
3180+
);
3181+
3182+
let mut snapshots = HashMap::from([(sandbox.id.clone(), dead_container)]);
3183+
merge_pending_sandbox_snapshots(
3184+
&mut snapshots,
3185+
HashMap::from([(sandbox.id.clone(), start_failure)]),
3186+
);
3187+
3188+
let status = snapshots[&sandbox.id].status.as_ref().expect("status");
3189+
assert_eq!(status.conditions[0].reason, "ContainerStartFailed");
3190+
assert!(
3191+
status.conditions[0]
3192+
.message
3193+
.contains("CDI device injection failed")
3194+
);
3195+
}
3196+
3197+
#[test]
3198+
fn pending_starting_snapshot_does_not_override_container_state() {
3199+
let sandbox = test_sandbox();
3200+
let dead_container = pending_sandbox_snapshot(
3201+
&sandbox,
3202+
"default",
3203+
error_condition("ContainerDead", "Container is dead"),
3204+
false,
3205+
);
3206+
let starting = pending_sandbox_snapshot(&sandbox, "default", provisioning_condition(), false);
3207+
3208+
let mut snapshots = HashMap::from([(sandbox.id.clone(), dead_container)]);
3209+
merge_pending_sandbox_snapshots(
3210+
&mut snapshots,
3211+
HashMap::from([(sandbox.id.clone(), starting)]),
3212+
);
3213+
3214+
let status = snapshots[&sandbox.id].status.as_ref().expect("status");
3215+
assert_eq!(status.conditions[0].reason, "ContainerDead");
3216+
}
3217+
31633218
#[test]
31643219
fn workload_mounts_only_the_shared_channel_volume() {
31653220
let config = runtime_config();

0 commit comments

Comments
 (0)