Skip to content

fix: detect externally stopped and removed containers in up monitor - #13990

Open
glours wants to merge 1 commit into
docker:mainfrom
glours:13985-monitor-external-stop-detection
Open

fix: detect externally stopped and removed containers in up monitor#13990
glours wants to merge 1 commit into
docker:mainfrom
glours:13985-monitor-external-stop-detection

Conversation

@glours

@glours glours commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

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 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 (#13161) working.

Related issue
Fixes #13985

(not mandatory) A picture of a cute animal, if possible in relation to what you did
image

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>
@glours
glours requested review from a team as code owners July 31, 2026 09:39
@glours
glours requested a review from ndeloof July 31, 2026 09:39

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 — ActionStart handler does not clear the restarting set (confidence: weak 10/100) — not a real bug given the current code flow; noted for future maintainability

Comment thread pkg/compose/monitor.go
Comment thread pkg/compose/monitor.go
@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.76923% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pkg/compose/monitor.go 80.76% 2 Missing and 3 partials ⚠️

📢 Thoughts on this report? Let us know!

@glours
glours requested a review from docker-agent July 31, 2026 10:15

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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).

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] Attached up never exits after external stop+down when a container is in restart backoff (regression in v2.39.3)

2 participants