From 586e6c22a1a83fd72d0a5b89ed7e4825ed351a79 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Tue, 22 Sep 2026 12:09:52 +0200 Subject: [PATCH] fix(wait): a target that already exited is a satisfied wait, not an error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wait listed running containers only, so a service that finished between up and the listing — a fast run, or a service long done by the time the user types the command — failed with 'no containers for project' in a few milliseconds instead of returning its recorded exit code. This is also the root cause of the TestWaitAndDrop flake: its 'faster' service sleeps 2 seconds, less than the harness latency between the two steps. The condition wait observes (container no longer running) already holds for such a target: fall back to a full listing only when no container is running, and let ContainerWait return the recorded status immediately. Scoping the fallback to the previously-erroring path keeps every other semantics intact — in particular a stale exited one-off can never short-circuit a wait that has live containers to observe. Exit-code propagation verified end to end: wait on an already-failed service returns its code (7), not an error. Signed-off-by: Nicolas De Loof --- pkg/compose/wait.go | 13 +++++ pkg/compose/wait_test.go | 103 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+) create mode 100644 pkg/compose/wait_test.go diff --git a/pkg/compose/wait.go b/pkg/compose/wait.go index 29848786b..622803e9b 100644 --- a/pkg/compose/wait.go +++ b/pkg/compose/wait.go @@ -31,6 +31,19 @@ func (s *composeService) Wait(ctx context.Context, projectName string, options a if err != nil { return 0, err } + if len(containers) == 0 { + // The condition wait observes — container no longer running — may + // already hold: a target that exited between up and this listing (a + // fast run, or a service long finished) is a SATISFIED wait, not an + // error; ContainerWait below returns its recorded exit code + // immediately. The second listing runs only when no container is + // running so a stale exited one-off can never short-circuit a wait + // that has live containers to observe. + containers, err = s.getContainers(ctx, projectName, oneOffInclude, true, options.Services...) + if err != nil { + return 0, err + } + } if len(containers) == 0 { return 0, fmt.Errorf("no containers for project %q", projectName) } diff --git a/pkg/compose/wait_test.go b/pkg/compose/wait_test.go new file mode 100644 index 000000000..f86b3c7a7 --- /dev/null +++ b/pkg/compose/wait_test.go @@ -0,0 +1,103 @@ +/* + Copyright 2020 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 ( + "context" + "testing" + + "github.com/moby/moby/api/types/container" + "github.com/moby/moby/client" + "go.uber.org/mock/gomock" + "gotest.tools/v3/assert" + + "github.com/docker/compose/v5/pkg/api" + "github.com/docker/compose/v5/pkg/mocks" +) + +// waitTestService builds a composeService whose ContainerList answers depend +// on the All flag: running containers first, the full set on the fallback +// listing. Call counts let tests assert which listings actually happened. +func waitTestService(t *testing.T, running, all []container.Summary) (api.Compose, *mocks.MockAPIClient, *int, *int) { + t.Helper() + mockCtrl := gomock.NewController(t) + apiClient, cli := prepareMocks(mockCtrl) + runningCalls, allCalls := new(int), new(int) + apiClient.EXPECT().ContainerList(gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, opts client.ContainerListOptions) (client.ContainerListResult, error) { + if opts.All { + *allCalls++ + return client.ContainerListResult{Items: all}, nil + } + *runningCalls++ + return client.ContainerListResult{Items: running}, nil + }).AnyTimes() + tested, err := NewComposeService(cli) + assert.NilError(t, err) + return tested, apiClient, runningCalls, allCalls +} + +// TestWait_AlreadyExitedTarget locks the race fix: a target that exited +// before wait listed the project is a SATISFIED wait — its recorded exit +// code is returned immediately — not a "no containers" error. +func TestWait_AlreadyExitedTarget(t *testing.T) { + exited := container.Summary{ + ID: "c-exited", State: container.StateExited, + Labels: map[string]string{api.ServiceLabel: "faster", api.ContainerNumberLabel: "1"}, + } + tested, apiClient, runningCalls, allCalls := waitTestService(t, nil, []container.Summary{exited}) + + apiClient.EXPECT().ContainerWait(gomock.Any(), "c-exited", gomock.Any()). + Return(waitResultExit(7)) + + code, err := tested.Wait(t.Context(), "proj", api.WaitOptions{Services: []string{"faster"}}) + assert.NilError(t, err) + assert.Equal(t, code, int64(7)) + assert.Equal(t, *runningCalls, 1) + assert.Equal(t, *allCalls, 1) +} + +// TestWait_NoContainersAtAll: when neither listing finds a matching +// container, the invocation is wrong and still errors. +func TestWait_NoContainersAtAll(t *testing.T) { + tested, _, runningCalls, allCalls := waitTestService(t, nil, nil) + + _, err := tested.Wait(t.Context(), "proj", api.WaitOptions{}) + assert.ErrorContains(t, err, `no containers for project "proj"`) + assert.Equal(t, *runningCalls, 1) + assert.Equal(t, *allCalls, 1) +} + +// TestWait_RunningContainersSkipFallback: with a running container to +// observe, the fallback listing never runs — a stale exited one-off cannot +// short-circuit the wait. +func TestWait_RunningContainersSkipFallback(t *testing.T) { + running := container.Summary{ + ID: "c-run", State: container.StateRunning, + Labels: map[string]string{api.ServiceLabel: "slower", api.ContainerNumberLabel: "1"}, + } + tested, apiClient, runningCalls, allCalls := waitTestService(t, []container.Summary{running}, nil) + + apiClient.EXPECT().ContainerWait(gomock.Any(), "c-run", gomock.Any()). + Return(waitResultExit(0)) + + code, err := tested.Wait(t.Context(), "proj", api.WaitOptions{}) + assert.NilError(t, err) + assert.Equal(t, code, int64(0)) + assert.Equal(t, *runningCalls, 1) + assert.Equal(t, *allCalls, 0) +}