Skip to content

fix(core): reset Reaper state when ryuk is already being removed - #1126

Open
RitiGrover wants to merge 1 commit into
testcontainers:mainfrom
RitiGrover:fix/reaper-delete-instance-409
Open

RitiGrover wants to merge 1 commit into
testcontainers:mainfrom
RitiGrover:fix/reaper-delete-instance-409

Conversation

@RitiGrover

Copy link
Copy Markdown

Fixes #1125

What was wrong
When ryuk was killed from outside, Docker was already auto-removing it. delete_instance() got a 409 from stop(), but it only suppressed NotFound. The 409 escaped before _container and _instance were reset, so get_instance() kept returning the dead reaper. The same 409 showed up in the atexit hook.

What changed
A 409 from stop() is treated like NotFound. The socket, container and instance are reset in a finally, so a failing stop() can't leave stale state either. This follows the fix suggested in the issue.

How it was tested
Two unit tests with a fake dead container:

  • A 409 is swallowed, the state is cleared and get_instance() creates a new reaper.
  • Any other APIError is still raised, but the state is cleared.

Both fail on main and pass with the fix. Ruff passes.

This touches the same function as #1124. Whichever lands second will need a small rebase, which I'll do.

If ryuk died from outside, Docker was already auto-removing it and
stop() got a 409. delete_instance() only suppressed NotFound, so the
409 escaped and skipped the resets. get_instance() then kept returning
the dead reaper. A 409 is now treated like NotFound, and the class
state is reset whatever stop() does.

Fixes testcontainers#1125
@codecov

codecov Bot commented Oct 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.02%. Comparing base (8d81011) to head (69d94af).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1126      +/-   ##
==========================================
- Coverage   86.15%   86.02%   -0.14%     
==========================================
  Files          16       16              
  Lines        1769     1774       +5     
  Branches      198      198              
==========================================
+ Hits         1524     1526       +2     
- Misses        185      189       +4     
+ Partials       60       59       -1     
Files with missing lines Coverage Δ
src/testcontainers/core/container.py 84.98% <100.00%> (+0.24%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

Bug: Reaper.delete_instance() raises 409 and keeps the dead reaper when ryuk was killed from outside

1 participant