fix(wait): a target that already exited is a satisfied wait, not an error

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 <[email protected]>

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

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)
+}