Skip to content

Commit 36d5ff4

Browse files
committed
fix(docker): retry transient runtime volume removal
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
1 parent dba3304 commit 36d5ff4

4 files changed

Lines changed: 49 additions & 10 deletions

File tree

‎architecture/sandbox.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -527,6 +527,8 @@ The Docker driver runs `openshell-supervisor` in a separate companion container.
527527
Its private named volume contains supervisor bootstrap and channel material.
528528
The workload container receives only `openshell-sandbox`, public interception
529529
CA material, and the other sandbox half of the authenticated channel.
530+
On deletion, the driver waits briefly for Docker to release the removed
531+
containers' private volume mounts before removing those volumes.
530532

531533
For Kubernetes, the operator configures a Secret name and key rather than a
532534
gateway-host file path. Kubernetes projects that Secret only into the separate

‎crates/openshell-driver-docker/README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,8 @@ across gateway restarts.
9797
Delete force-removes both containers, the driver-owned runtime volumes, and the
9898
host-private runtime descriptor. Missing or altered descriptor and channel resources
9999
fail closed; the driver does not run an older combined-supervisor layout.
100+
Docker may briefly retain a volume mount after acknowledging container removal;
101+
the driver retries only that volume-in-use response for a bounded interval.
100102

101103
## Driver Config Mounts
102104

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

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4140,17 +4140,28 @@ async fn remove_docker_channel_volume_by_id(
41404140
}
41414141

41424142
async fn remove_docker_volume(docker: &Docker, name: &str) -> Result<(), Status> {
4143-
docker
4144-
.remove_volume(name, None::<bollard::query_parameters::RemoveVolumeOptions>)
4145-
.await
4146-
.or_else(|error| {
4147-
if is_not_found_error(&error) {
4148-
Ok(())
4149-
} else {
4150-
Err(error)
4143+
// Docker can acknowledge removal of the last container before releasing
4144+
// its volume mount. Retry only that transient 409; other conflicts and
4145+
// daemon errors should still fail the sandbox deletion.
4146+
const MAX_ATTEMPTS: usize = 12;
4147+
for attempt in 1..=MAX_ATTEMPTS {
4148+
match docker
4149+
.remove_volume(name, None::<bollard::query_parameters::RemoveVolumeOptions>)
4150+
.await
4151+
{
4152+
Ok(()) => return Ok(()),
4153+
Err(error) if is_not_found_error(&error) => return Ok(()),
4154+
Err(error) if is_volume_in_use_error(&error) && attempt < MAX_ATTEMPTS => {
4155+
tokio::time::sleep(Duration::from_millis(250)).await;
41514156
}
4152-
})
4153-
.map_err(|error| Status::internal(format!("remove Docker sandbox runtime volume: {error}")))
4157+
Err(error) => {
4158+
return Err(Status::internal(format!(
4159+
"remove Docker sandbox runtime volume: {error}"
4160+
)));
4161+
}
4162+
}
4163+
}
4164+
unreachable!("the final volume removal attempt returns a result")
41544165
}
41554166

41564167
fn sandbox_token_host_path(
@@ -6525,6 +6536,16 @@ fn is_removal_in_progress_error(err: &BollardError) -> bool {
65256536
)
65266537
}
65276538

6539+
fn is_volume_in_use_error(err: &BollardError) -> bool {
6540+
matches!(
6541+
err,
6542+
BollardError::DockerResponseServerError {
6543+
status_code: 409,
6544+
message,
6545+
} if message.contains("volume is in use")
6546+
)
6547+
}
6548+
65286549
fn is_not_modified_error(err: &BollardError) -> bool {
65296550
matches!(
65306551
err,

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3440,6 +3440,20 @@ fn concurrent_container_removal_is_idempotent() {
34403440
assert!(!is_removal_in_progress_error(&other_conflict));
34413441
}
34423442

3443+
#[test]
3444+
fn volume_removal_retries_only_mount_release_conflicts() {
3445+
let in_use = BollardError::DockerResponseServerError {
3446+
status_code: 409,
3447+
message: "remove sandbox-volume: volume is in use - [container-id]".to_string(),
3448+
};
3449+
let other_conflict = BollardError::DockerResponseServerError {
3450+
status_code: 409,
3451+
message: "volume has invalid options".to_string(),
3452+
};
3453+
assert!(is_volume_in_use_error(&in_use));
3454+
assert!(!is_volume_in_use_error(&other_conflict));
3455+
}
3456+
34433457
#[tokio::test]
34443458
async fn missing_start_generation_is_adopted() {
34453459
let directory = TempDir::new().expect("create temporary directory");

0 commit comments

Comments
 (0)