Skip to content

fix(podman): combine sandbox stop and removal - #2570

Open
pimlock wants to merge 2 commits into
mainfrom
podman-atomic-container-removal/pm
Open

fix(podman): combine sandbox stop and removal#2570
pimlock wants to merge 2 commits into
mainfrom
podman-atomic-container-removal/pm

Conversation

@pimlock

@pimlock pimlock commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Summary

Collapse the Podman driver's separate stop and remove requests into one timed Libpod delete request. Graceful shutdown behavior already existed and is preserved; the change removes the externally scheduled interval between the requests and explicitly applies OpenShell's configured stop timeout to removal.

This mitigates the Podman type=image mount lifecycle race observed when one sandbox reaches its stop timeout while another sandbox starts, without globally serializing sandbox lifecycle operations.

The change also corrects anonymous-volume removal to use the Libpod endpoint's volumes=true parameter. The Docker-compatible endpoint uses v=true; OpenShell calls the Libpod endpoint.

Related Issue

Related to the investigation in #2568.

No issue required: this is a localized compatibility fix for the Podman sandbox driver.

Changes

  • Replace the best-effort stop request followed by forced removal with one documented Libpod delete request:
    DELETE ...?force=true&volumes=true&timeout=<configured stop timeout>
  • Allow the Podman HTTP request to remain active for the configured stop timeout plus a five-second transport margin.
  • Use the same timed removal path when cleaning up a container after a failed start.
  • Correct anonymous-volume cleanup from the Docker-compatible v=true parameter to the Libpod-specific volumes=true parameter.
  • Update driver and client tests to assert that deletion uses one combined operation.

Why volumes=true, not v=true

Podman's generated API documentation currently shows v for ContainerDeleteLibpod, but the server handler distinguishes the two API families:

  • Libpod requests populate RmOptions.Volumes from the volumes query parameter.
  • Docker-compatible requests populate it from the shorter v query parameter.

This behavior is present in both the affected Podman 4.9.3 handler and the Podman 6.0.1 handler. OpenShell sends requests to /libpod/containers/..., so volumes=true is the parameter the implementation consumes.

The previous driver sent v=true, which the Libpod handler ignored. This did not cause the image-mount race, and OpenShell separately removes its named sandbox volume, but it meant the request did not remove anonymous volumes as intended.

Reproduction and evidence

The failure was reproduced without OpenShell using:

  • Ubuntu 24.04
  • Rootless Podman 4.9.3, API 4.9.3, Go 1.22.2, Linux arm64
  • crun 1.14.1
  • SQLite state and native overlay storage
  • Concurrent remote clients through the Podman REST service, matching the driver transport
  • Supervisor image ghcr.io/nvidia/openshell/supervisor:02e890ccf1995bf8671a81ec84baf44328b7f5b8

The standalone reproducer stopped old containers and started pre-created replacements around the ten-second stop-timeout boundary before removing the old containers. On batch 8, two replacement starts failed because their OCI specifications referenced Podman-owned image-overlay mount sources that no longer existed. The missing paths belonged to the new containers, not the containers being removed.

Starting replacements concurrently with the beginning of immediate teardown did not reproduce across thousands of overlaps. The sensitive window was the transition from stop timeout into final removal.

Changing the lifecycle shape to a single timed forced removal completed 40/40 deliberately timed overlaps without reproducing the missing mount source.

Semantics and confidence

The old and new successful paths both stop a running container gracefully before removal:

  • Old: POST /stop?t=<configured timeout>, followed by DELETE ?force=true after the container stopped.
  • New: one DELETE ?force=true&timeout=<configured timeout> operation; Podman stops the running container and then removes it.

The new behavior does not introduce graceful shutdown. It preserves it while eliminating the inter-request scheduling boundary and explicitly applying OpenShell's configured timeout to the forced delete.

The intentional edge-case difference is that the old driver ignored failure from its preliminary stop request. If the container remained running, the subsequent forced removal could make another stop attempt using the container's configured stop timeout, normally ten seconds. The combined operation makes one shutdown attempt with OpenShell's configured timeout and returns its error, avoiding both a possible second timeout period and the inter-request window.

This is an evidence-backed mitigation, not proof that Podman's underlying storage race is impossible:

  • Podman's removal path can temporarily release the container lock while stopping.
  • Forty successful overlaps substantially increase confidence but are not a concurrency guarantee.
  • The confirmed reproduction is on Podman 4.9.3, as shipped by Ubuntu 24.04; this PR does not claim the race reproduces on current upstream Podman.
  • If the failure recurs, stronger driver-side serialization of create/start against deletion, recovery by recreating a container with a stale mount specification, or an upstream Podman fix remain alternatives.

Testing

  • mise run pre-commit passes
  • mise exec -- cargo test -p openshell-driver-podman (135 passed)
  • Scoped rootless Podman lifecycle E2E (2 passed) using the supervisor image from the failing workflow
  • Standalone Podman reproducer: separate stop/removal reproduced; combined timed removal passed 40/40 timed overlaps

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (not applicable; no architecture or configuration contract changed)

Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
let (status, bytes) = self
.request(
hyper::Method::DELETE,
&format!("/libpod/containers/{name}?force=true&v=true&timeout={timeout_secs}"),

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this documentation is not quite right it turns out. The v param is used for Docker-compatible path, but libpod path needs volumes as a param:

This behavior is present in both the affected Podman 4.9.3 handler and the Podman 6.0.1 handler. OpenShell sends requests to /libpod/containers/..., so volumes=true is the parameter the implementation consumes.

@pimlock
pimlock requested a review from matthewgrossman July 31, 2026 01:13
@pimlock pimlock added the test:e2e Requires end-to-end coverage label Jul 31, 2026
@github-actions

Copy link
Copy Markdown

Label test:e2e applied for 6adcd30. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:e2e Requires end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant