Skip to content

fix(validation): verify container cleanup without matching engine error text - #1259

Open
rycerzes wants to merge 1 commit into
huggingface:mainfrom
rycerzes:fix/validation-podman-portability
Open

rycerzes wants to merge 1 commit into
huggingface:mainfrom
rycerzes:fix/validation-podman-portability

Conversation

@rycerzes

@rycerzes rycerzes commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

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

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • New environment
  • Refactoring

Alignment Checklist

Before submitting, verify:

  • I have read .claude/docs/PRINCIPLES.md and this PR aligns with our principles
  • I have checked .claude/docs/INVARIANTS.md and no invariants are violated
  • I have run /pre-submit-pr (or bash .claude/hooks/lint.sh and tests) and addressed all issues

RFC Status

  • Not required (bug fix, docs, minor refactoring)
  • RFC exists: #___
  • RFC needed (will create before merge)

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.
  • Pinned lab, --require-complete:
Tree Docker 29.7.2 (linux/amd64) Podman 5.8.4 (macOS arm64)
main 0/3 0/3
this PR 3/3 3/3
#1181 2/13 2/13
#1181 + this PR 13/13 (artifacts verified) 13/13

Claude Code Review

Claude Code was used for the investigation, implementation and test runs above; /alignment-review was 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 0 so 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 that docker ps lists by exact name and reads only the run label, then verifies removal with a second list after docker rm --force.

Tests: Unit mocks simulate docker ps listings and add cases for Docker/Podman missing-container stderr, unreachable engine, already-gone containers, and prefix name collisions. Integration checks CapDrop loosely and asserts an empty CapBnd inside 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.

…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

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Validation Docker provider cleanup fails on Docker 29 and Podman (error-text matching)

1 participant