Conversation
…or text Fixes cleanup verification on Docker 29 and Podman, which word a missing container differently. Fixes huggingface#1257
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The validation Docker provider checked for a missing container by matching Docker's exact error text. Docker 29 and Podman word that differently, so cleanup could never be verified and the Docker suite failed even though containers were removed. This PR verifies ownership and removal by listing instead, launches subjects with
--stop-timeout 0(Podman otherwise waits 10 s before SIGKILL on forced removal), and checks the empty capability bounding set inside the subject rather than the inspect format. Fixes #1257.Type of Change
Alignment Checklist
Before submitting, verify:
.claude/docs/PRINCIPLES.mdand this PR aligns with our principles.claude/docs/INVARIANTS.mdand no invariants are violated/pre-submit-pr(orbash .claude/hooks/lint.shand tests) and addressed all issuesRFC Status
Test Plan
pytest tests/test_validation(non-Docker): 214 passed. New unit tests feed Docker's and Podman's missing-container output through cleanup and cover an unreachable engine and a same-prefix container with another owner.--require-complete:Claude Code Review
Claude Code was used for the investigation, implementation and test runs above;
/alignment-reviewwas not run.Note
Low Risk
Changes are scoped to the validation Docker provider’s cleanup and test harness; ownership checks remain strict and removal is verified more portably across engines.
Overview
Fixes validation container cleanup and teardown so it works reliably on Docker 29 and Podman, where “missing container” errors differ and forced remove can wait on a stop grace period.
Docker provider: New containers are created with
--stop-timeout 0so forced removal does not wait (Podman’s default ~10s delay).stop()no longer depends on inspect or matching daemon error strings; it uses a new_owner()helper thatdocker pslists by exact name and reads only the run label, then verifies removal with a second list afterdocker rm --force.Tests: Unit mocks simulate
docker pslistings and add cases for Docker/Podman missing-container stderr, unreachable engine, already-gone containers, and prefix name collisions. Integration checksCapDroploosely and asserts an emptyCapBndinside the subject for engine-agnostic capability evidence.Reviewed by Cursor Bugbot for commit dba058e. Bugbot is set up for automated code reviews on this repo. Configure here.