Skip to content

Commit b8bb31e

Browse files
author
Roy Lin
committed
fix(box): CRI RemoveContainer force-removes a running container
Per the CRI spec, RemoveContainer must remove a container even when it is running (stopping it first), but BoxRuntimeService rejected a running container with FailedPrecondition ("requires a stopped container"), so `critest` "should support removing running container [Conformance]" failed. RemoveContainer now force-stops a running container (timeout 0, reusing stop_container's workload-stop + sandbox-VM-teardown path and the multi-container guard) before deleting it. Verified on the KVM server: the conformance spec now passes. Test test_remove_container_rejects_running_container updated accordingly (renamed to ..._force_stops_running_container).
1 parent 69e4d1c commit b8bb31e

2 files changed

Lines changed: 14 additions & 10 deletions

File tree

src/cri/src/runtime_service/mod.rs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1176,11 +1176,16 @@ impl RuntimeService for BoxRuntimeService {
11761176
return Ok(Response::new(RemoveContainerResponse {}));
11771177
};
11781178

1179+
// CRI RemoveContainer force-removes: a still-running container is
1180+
// stopped first (timeout 0), then deleted — matching containerd/cri-o.
1181+
// stop_container handles the workload stop + sandbox VM teardown
1182+
// fallback and the multi-container guard.
11791183
if container.state == ContainerState::Running {
1180-
return Err(Status::failed_precondition(format!(
1181-
"RemoveContainer requires a stopped container; container {} is Running",
1182-
container_id
1183-
)));
1184+
self.stop_container(Request::new(StopContainerRequest {
1185+
container_id: container_id.clone(),
1186+
timeout: 0,
1187+
}))
1188+
.await?;
11841189
}
11851190

11861191
if let Some(removed) = self.store.remove_container(container_id).await {

src/cri/src/runtime_service/tests.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2650,7 +2650,7 @@ async fn test_remove_container_missing_is_idempotent() {
26502650
}
26512651

26522652
#[tokio::test]
2653-
async fn test_remove_container_rejects_running_container() {
2653+
async fn test_remove_container_force_stops_running_container() {
26542654
let svc = make_test_service();
26552655
svc.store
26562656
.containers
@@ -2661,17 +2661,16 @@ async fn test_remove_container_rejects_running_container() {
26612661
.mark_started("c-1", 2_000_000_000)
26622662
.await;
26632663

2664+
// CRI RemoveContainer force-removes: a running container is stopped first,
2665+
// then deleted (no VM manager in the test, so the stop reconciles it).
26642666
let result = svc
26652667
.remove_container(Request::new(RemoveContainerRequest {
26662668
container_id: "c-1".to_string(),
26672669
}))
26682670
.await;
26692671

2670-
assert!(result.is_err());
2671-
let err = result.unwrap_err();
2672-
assert_eq!(err.code(), tonic::Code::FailedPrecondition);
2673-
assert!(err.message().contains("requires a stopped container"));
2674-
assert!(svc.store.containers.get("c-1").await.is_some());
2672+
assert!(result.is_ok());
2673+
assert!(svc.store.containers.get("c-1").await.is_none());
26752674
}
26762675

26772676
// ── Container Status ─────────────────────────────────────────────

0 commit comments

Comments
 (0)