diff --git a/pkg/compose/monitor.go b/pkg/compose/monitor.go index f405a5d5b4..912f760d56 100644 --- a/pkg/compose/monitor.go +++ b/pkg/compose/monitor.go @@ -133,19 +133,40 @@ func (c *monitor) Start(ctx context.Context) error { listener(newContainerEvent(event.TimeNano, ctr, api.ContainerEventRestarted)) } logrus.Debugf("container %s restarted", ctr.Name) + case events.ActionStop: + // a stop event with no following start means the container won't come back: + // either it has no restart policy, or an external `stop`/`down` canceled the + // restart loop of a container in backoff (https://github.com/docker/compose/issues/13985). + // The event alone can't tell us: during a ContainerRestart (watch sync+restart, + // https://github.com/docker/compose/issues/13161) the engine also emits `stop` + // before `start`. + willRestart, err := c.isRestarting(ctx, ctr.ID) + if err != nil { + return err + } + if willRestart { + logrus.Debugf("container %s stopped, restart in progress", ctr.Name) + restarting.Add(ctr.ID) + } else { + // definitive stop: the exit was already reported to listeners by the + // preceding die event, just stop tracking the container + logrus.Debugf("container %s stopped", ctr.Name) + restarting.Remove(ctr.ID) + containers.Remove(ctr.ID) + } + case events.ActionDestroy: + // container removed (e.g. by an external `docker compose down`): terminal + // state, there is nothing left to inspect + logrus.Debugf("container %s destroyed", ctr.Name) + restarting.Remove(ctr.ID) + containers.Remove(ctr.ID) case events.ActionDie: logrus.Debugf("container %s exited with code %d", ctr.Name, ctr.ExitCode) - inspect, err := c.apiClient.ContainerInspect(ctx, event.Actor.ID, client.ContainerInspectOptions{}) - if errdefs.IsNotFound(err) { - // Source is already removed - } else if err != nil { + willRestart, err := c.isRestarting(ctx, ctr.ID) + if err != nil { return err } - - if inspect.Container.State != nil && (inspect.Container.State.Restarting || inspect.Container.State.Running) { - // State.Restarting is set by engine when container is configured to restart on exit - // on ContainerRestart it doesn't (see https://github.com/moby/moby/issues/45538) - // container state still is reported as "running" + if willRestart { logrus.Debugf("container %s is restarting", ctr.Name) restarting.Add(ctr.ID) for _, listener := range c.listeners { @@ -164,6 +185,23 @@ func (c *monitor) Start(ctx context.Context) error { } } +// isRestarting tells whether a container which just stopped is expected to come back. +// State.Restarting is set by the engine when the container is configured to restart on +// exit, but not on a ContainerRestart, where state still is reported as "running" +// (see https://github.com/moby/moby/issues/45538). A container already removed won't +// come back. +func (c *monitor) isRestarting(ctx context.Context, containerID string) (bool, error) { + inspect, err := c.apiClient.ContainerInspect(ctx, containerID, client.ContainerInspectOptions{}) + if errdefs.IsNotFound(err) { + return false, nil + } + if err != nil { + return false, err + } + state := inspect.Container.State + return state != nil && (state.Restarting || state.Running), nil +} + func newContainerEvent(timeNano int64, ctr *api.ContainerSummary, eventType int, opts ...func(e *api.ContainerEvent)) api.ContainerEvent { name := ctr.Name defaultName := getDefaultContainerName(ctr.Project, ctr.Labels[api.ServiceLabel], ctr.Labels[api.ContainerNumberLabel]) diff --git a/pkg/compose/monitor_test.go b/pkg/compose/monitor_test.go new file mode 100644 index 0000000000..b49b404e78 --- /dev/null +++ b/pkg/compose/monitor_test.go @@ -0,0 +1,209 @@ +/* + Copyright 2026 Docker Compose CLI authors + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package compose + +import ( + "strconv" + "strings" + "testing" + "time" + + "github.com/containerd/errdefs" + "github.com/moby/moby/api/types/container" + "github.com/moby/moby/api/types/events" + "github.com/moby/moby/client" + "go.uber.org/goleak" + "go.uber.org/mock/gomock" + "gotest.tools/v3/assert" + + "github.com/docker/compose/v5/pkg/api" + "github.com/docker/compose/v5/pkg/mocks" +) + +// monitorEvent builds an engine event for container "123"/service1, with the +// Actor.Attributes shape reported by the engine: compose labels plus the +// container name. +func monitorEvent(action events.Action) events.Message { + attrs := containerLabels("service1", false) + attrs["name"] = "testproject-service1-1" + return events.Message{ + Type: events.ContainerEventType, + Action: action, + Actor: events.Actor{ID: "123", Attributes: attrs}, + } +} + +// monitorDieEvent builds a die event, which the engine reports with an exit code. +func monitorDieEvent(exitCode int) events.Message { + event := monitorEvent(events.ActionDie) + event.Actor.Attributes["exitCode"] = strconv.Itoa(exitCode) + return event +} + +// newMonitorTestFixture wires a monitor against a mocked API client, with the +// goroutine-leak guard and the standard initial ContainerList expectation. +func newMonitorTestFixture(t *testing.T) (*monitor, *mocks.MockAPIClient) { + t.Helper() + ignoreExisting := goleak.IgnoreCurrent() + t.Cleanup(func() { + goleak.VerifyNone(t, ignoreExisting) + }) + mockCtrl := gomock.NewController(t) + t.Cleanup(mockCtrl.Finish) + apiMock := mocks.NewMockAPIClient(mockCtrl) + + apiMock.EXPECT().ContainerList(gomock.Any(), gomock.Any()). + Return(client.ContainerListResult{Items: []container.Summary{testContainer("service1", "123", false)}}, nil) + + m := newMonitor(apiMock, strings.ToLower(testProject)) + return m, apiMock +} + +// expectEvents makes the mocked engine deliver the given events, in order. +func expectEvents(apiMock *mocks.MockAPIClient, msgs ...events.Message) { + ch := make(chan events.Message, len(msgs)) + for _, msg := range msgs { + ch <- msg + } + apiMock.EXPECT().Events(gomock.Any(), gomock.Any()). + Return(client.EventsResult{Messages: ch, Err: make(chan error)}) +} + +// expectInspects makes successive inspections of container "123" report the +// given states, in order. +func expectInspects(apiMock *mocks.MockAPIClient, states ...container.State) { + calls := make([]any, 0, len(states)) + for _, state := range states { + calls = append(calls, apiMock.EXPECT(). + ContainerInspect(gomock.Any(), "123", client.ContainerInspectOptions{}). + Return(client.ContainerInspectResult{Container: container.InspectResponse{State: &state}}, nil)) + } + gomock.InOrder(calls...) +} + +// runMonitor starts the monitor under test in a goroutine and waits (with a +// timeout) for it to return, reporting the events it published. It fails the +// test if the monitor doesn't stop on its own, which is how an un-fixed +// monitor.Start reacts to stop/destroy events it doesn't know how to process: +// the tracked containers set never empties, so the loop blocks forever on the +// events channel. +func runMonitor(t *testing.T, m *monitor) ([]api.ContainerEvent, error) { + t.Helper() + var got []api.ContainerEvent + m.withListener(func(e api.ContainerEvent) { + got = append(got, e) + }) + + done := make(chan error, 1) + go func() { + done <- m.Start(t.Context()) + }() + select { + case err := <-done: + return got, err + case <-time.After(10 * time.Second): + t.Fatal("monitor did not stop") + return nil, nil + } +} + +// TestMonitorExitsOnDestroy pins the expectation that a destroy event (e.g. a +// container removed by `docker rm` or `docker compose rm` outside of a +// tracked lifecycle transition) drops the container from the tracked set +// without requiring any inspection, so the monitor loop terminates. +func TestMonitorExitsOnDestroy(t *testing.T) { + m, apiMock := newMonitorTestFixture(t) + expectEvents(apiMock, monitorEvent(events.ActionDestroy)) + + got, err := runMonitor(t, m) + assert.NilError(t, err) + assert.Equal(t, len(got), 0) +} + +// TestMonitorExitsWhenRestartingContainerStopped is the #13985 repro: a +// container configured to restart on failure dies (engine reports it as +// still "restarting"), then is explicitly stopped (e.g. `docker stop`) +// before the restart happens. The monitor must inspect on stop, observe the +// container is no longer restarting/running, and evict it so the loop +// terminates instead of waiting forever for a start event that never comes. +func TestMonitorExitsWhenRestartingContainerStopped(t *testing.T) { + m, apiMock := newMonitorTestFixture(t) + expectEvents(apiMock, monitorDieEvent(1), monitorEvent(events.ActionStop)) + expectInspects(apiMock, + // on die: waiting for the restart policy to kick in + container.State{Status: container.StateRestarting, Restarting: true, ExitCode: 1}, + // on stop: the restart loop got canceled + container.State{Status: container.StateExited, ExitCode: 1}, + ) + + got, err := runMonitor(t, m) + assert.NilError(t, err) + assert.Equal(t, len(got), 1) + assert.Equal(t, got[0].Type, api.ContainerEventExited) + assert.Equal(t, got[0].Restarting, true) + assert.Equal(t, got[0].ExitCode, 1) +} + +// TestMonitorKeepsRunningOnRestart is the #13161 guard: a container that +// dies and is restarted by the engine (watch/sync workflows trigger this via +// `docker restart`) must not be evicted by an intervening stop event that is +// merely part of the moby#45538 restart sequence (State reports +// Running=true while mid-ContainerRestart). The monitor must keep tracking +// it and still observe the subsequent start. +func TestMonitorKeepsRunningOnRestart(t *testing.T) { + m, apiMock := newMonitorTestFixture(t) + expectEvents(apiMock, + monitorDieEvent(0), + monitorEvent(events.ActionStop), + monitorEvent(events.ActionStart), + monitorDieEvent(1), + ) + expectInspects(apiMock, + // on die then on stop: mid-ContainerRestart, so still reported as running + container.State{Status: container.StateRunning, Running: true}, + container.State{Status: container.StateRunning, Running: true}, + // on the final die: really gone + container.State{Status: container.StateExited, ExitCode: 1}, + ) + + got, err := runMonitor(t, m) + assert.NilError(t, err) + assert.Equal(t, len(got), 3) + assert.Equal(t, got[0].Type, api.ContainerEventExited) + assert.Equal(t, got[0].Restarting, true) + assert.Equal(t, got[0].ExitCode, 0) + assert.Equal(t, got[1].Type, api.ContainerEventStarted) + assert.Equal(t, got[1].Restarting, true) + assert.Equal(t, got[2].Type, api.ContainerEventExited) + assert.Equal(t, got[2].Restarting, false) + assert.Equal(t, got[2].ExitCode, 1) +} + +// TestMonitorStopInspectNotFound covers a stop event racing a container's +// removal: the inspect on stop returns NotFound, which must be tolerated +// (not treated as a fatal error) and the container evicted so the monitor +// terminates. +func TestMonitorStopInspectNotFound(t *testing.T) { + m, apiMock := newMonitorTestFixture(t) + expectEvents(apiMock, monitorEvent(events.ActionStop)) + apiMock.EXPECT().ContainerInspect(gomock.Any(), "123", client.ContainerInspectOptions{}). + Return(client.ContainerInspectResult{}, errdefs.ErrNotFound.WithMessage("no such container: 123")) + + got, err := runMonitor(t, m) + assert.NilError(t, err) + assert.Equal(t, len(got), 0) +}