fix: detect externally stopped and removed containers in up monitor - #13990
fix: detect externally stopped and removed containers in up monitor#13990glours wants to merge 1 commit into
Conversation
Since 2.39.3 the monitor relies solely on die events to stop tracking containers, but a container stopped while in restart backoff emits only stop and destroy events, so an attached `up` hangs forever after an external `stop`/`down`. Handle destroy as terminal, and on stop inspect the container to distinguish a definitive stop from the transient one emitted during a ContainerRestart, which keeps watch sync+restart (docker#13161) working. Fixes docker#13985 Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟡 NEEDS ATTENTION
Two medium-confidence findings introduced by this PR's new ActionStop handling — both relate to how the monitor responds when ContainerInspect does or doesn't find the container.
Lower-confidence findings (not posted inline)
- [low] pkg/compose/monitor.go:122 —
ActionStarthandler does not clear therestartingset (confidence: weak 10/100) — not a real bug given the current code flow; noted for future maintainability
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
The fix is well-structured and correct. The new ActionStop handler correctly distinguishes a definitive stop from a transient one by inspecting the container state, and ActionDestroy unconditionally terminates tracking without requiring an inspect (appropriate since there is nothing left to inspect). The isRestarting helper cleanly handles the NotFound case (container already removed) by returning false, nil, which is the right terminal disposition. The test suite covers the four key scenarios: destroy-only, die+stop canceling a restart backoff (#13985 repro), die+stop+start across a ContainerRestart cycle (#13161 guard), and stop racing a removal (inspect returns NotFound).
What I did
Since 2.39.3 the monitor relies solely on die events to stop tracking containers, but a container stopped while in restart backoff emits only stop and destroy events, so an attached
uphangs forever after an externalstop/down. Handle destroy as terminal, and on stop inspect the container to distinguish a definitive stop from the transient one emitted during a ContainerRestart, which keeps watch sync+restart (#13161) working.Related issue
Fixes #13985
(not mandatory) A picture of a cute animal, if possible in relation to what you did
