From 05176783c0284091b6dfa0666098eac028d8bd73 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Tue, 27 Oct 2026 11:34:15 +0100 Subject: [PATCH 1/3] refactor(compose): move the dependency wait helpers to their own file waitDependencies, the polling of a depends_on condition (waitDependency, waitDependencyReached and the per-condition checks), shouldWaitForDependency and the container health probes they use leave service_containers.go for dependency_wait.go, and their tests for dependency_wait_test.go. The remaining clients of the waits are the plan's OpWaitCondition nodes, restart, run and the --wait verification of up and start. A pure move: every declaration, its comments included, is unchanged. Part of #14081 Signed-off-by: Nicolas De Loof --- pkg/compose/dependency_wait.go | 326 ++++++++++ pkg/compose/dependency_wait_test.go | 796 +++++++++++++++++++++++++ pkg/compose/service_containers.go | 297 --------- pkg/compose/service_containers_test.go | 763 ------------------------ 4 files changed, 1122 insertions(+), 1060 deletions(-) create mode 100644 pkg/compose/dependency_wait.go create mode 100644 pkg/compose/dependency_wait_test.go diff --git a/pkg/compose/dependency_wait.go b/pkg/compose/dependency_wait.go new file mode 100644 index 0000000000..7d168ae543 --- /dev/null +++ b/pkg/compose/dependency_wait.go @@ -0,0 +1,326 @@ +/* + 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" + "errors" + "fmt" + "time" + + "github.com/compose-spec/compose-go/v2/types" + "github.com/moby/moby/api/types/container" + "github.com/moby/moby/client" + "github.com/sirupsen/logrus" + "golang.org/x/sync/errgroup" + + "github.com/docker/compose/v5/pkg/api" +) + +// ServiceConditionRunningOrHealthy is a service condition on status running or healthy +const ServiceConditionRunningOrHealthy = "running_or_healthy" + +func (s *composeService) waitDependencies(ctx context.Context, project *types.Project, dependant string, dependencies types.DependsOnConfig, containers Containers) error { + origCtx := ctx + eg, ctx := errgroup.WithContext(ctx) + for dep, config := range dependencies { + if shouldWait, err := shouldWaitForDependency(dep, config, project); err != nil { + return err + } else if !shouldWait { + continue + } + + waitingFor := containers.filter(isService(dep), isNotOneOff, isNotHookContainer) + s.events.On(containerEvents(waitingFor, waiting)...) + if len(waitingFor) == 0 { + if config.Required { + return fmt.Errorf("%s is missing dependency %s", dependant, dep) + } + logrus.Warnf("%s is missing dependency %s", dependant, dep) + continue + } + + eg.Go(func() error { + return s.waitDependency(ctx, origCtx, dependant, dep, config, waitingFor) + }) + } + err := eg.Wait() + if errors.Is(err, context.DeadlineExceeded) { + return errors.New("timeout waiting for dependencies") + } + return err +} + +// waitDependency polls the dependency's containers until its depends_on +// condition is satisfied (done), definitively failed (err), or ctx is +// cancelled. Each check reports (done, err): (false, nil) means keep polling. +// origCtx is ctx as it stood before the caller wrapped it with this wait's +// own timeout (if any: waitDependencies sets none, the plan's wait nodes do, +// see execBoundedWaitCondition) -- comparing origCtx.Err() against ctx.Err() at the +// moment ctx.Done() fires is how this tells apart a DeadlineExceeded this +// specific call's own wrap is responsible for from one inherited from an +// ancestor context the caller never touched. A bare "did the caller +// configure a timeout at all" flag isn't enough: if origCtx itself already +// carries an earlier deadline, wrapping it with context.WithTimeout still +// makes both origCtx and ctx report DeadlineExceeded once that earlier +// deadline fires, even though this call's own timeout never actually +// elapsed -- only origCtx.Err() == nil proves this wrap's own timeout is +// the one that fired. Only that case is safe to tolerate on an optional +// dependency; an inherited deadline means something else entirely (an +// external deadline, a test harness timeout) fired, and must keep +// propagating regardless of Required. +func (s *composeService) waitDependency(ctx, origCtx context.Context, dependant, dep string, config types.ServiceDependency, waitingFor Containers) error { + _, err := s.waitDependencyReached(ctx, origCtx, dependant, dep, config, waitingFor) + return err +} + +// waitDependencyReached is waitDependency, which also tells whether the wait is +// over because the dependency is ready (or an optional one was given up on), +// as opposed to a plain cancellation of ctx, which ends it silently. +func (s *composeService) waitDependencyReached(ctx, origCtx context.Context, dependant, dep string, config types.ServiceDependency, waitingFor Containers) (bool, error) { + ticker := time.NewTicker(500 * time.Millisecond) + defer ticker.Stop() + // running_or_healthy accepts a running container without a healthcheck: + // checked right after it starts, one that exits at once would pass. + checkOnEntry := config.Condition == types.ServiceConditionHealthy || config.Condition == types.ServiceConditionCompletedSuccessfully + for first := true; ; first = false { + // a tick and the end of ctx can be ready together, and select would + // then pick either: ctx wins, so that a canceled wait is never run + // through one more check + if !first || !checkOnEntry { + select { + case <-ticker.C: + case <-ctx.Done(): + } + } + if ctx.Err() != nil { + // An expired deadline is precisely the failure this wait is meant + // to detect for a required dependency; only a plain cancellation + // (Ctrl-C) stays silent either way. An optional dependency + // tolerates a timeout exactly like it tolerates any other + // definitive failure the check* functions below report — + // skipped, not aborted — but only this call's OWN timeout: an + // inherited deadline is a different, real failure the caller + // must still see. + if errors.Is(ctx.Err(), context.DeadlineExceeded) { + if !config.Required && origCtx.Err() == nil { + s.events.On(containerReasonEvents(waitingFor, skippedEvent, + fmt.Sprintf("optional dependency %q did not satisfy %q before the timeout", dep, config.Condition))...) + logrus.Warnf("optional dependency %q did not satisfy %q before the timeout", dep, config.Condition) + return true, nil + } + return false, ctx.Err() + } + return false, nil + } + var ( + done bool + err error + ) + switch config.Condition { + case ServiceConditionRunningOrHealthy: + done, err = s.checkDependencyRunningOrHealthy(ctx, dep, config, waitingFor) + case types.ServiceConditionHealthy: + done, err = s.checkDependencyHealthy(ctx, dep, config, waitingFor) + case types.ServiceConditionCompletedSuccessfully: + done, err = s.checkDependencyCompleted(ctx, dep, config, waitingFor) + default: + // Every condition this switch doesn't handle explicitly ends up + // here: ServiceConditionStarted is filtered out before this + // function is ever called (shouldWaitForDependency — "already + // ensured by the dependency order"), so nothing reaching this + // branch is ever a value this runtime actually understands. + // That covers a compose file with an unrecognized condition + // (rejected by schema.Validate before it can even get this far + // — see unsupported_attributes.go's comment on + // valueConditionalAttributes) as well as the two situations + // that never go through schema validation at all: a project + // rebuilt from live container labels (see projectFromName), or + // one written by an older/newer Compose version using a + // condition value this build doesn't recognize. + logrus.Warnf("service %q: unsupported depends_on condition %q, skipping wait for %q", dependant, config.Condition, dep) + return true, nil + } + if done || err != nil { + return done, err + } + } +} + +// checkDependencyRunningOrHealthy and checkDependencyHealthy skip an optional +// dependency whose inspection fails. A failure caused by ctx being done says +// nothing about the dependency though: reporting it as "skipped" would turn a +// cancellation or an expired deadline into a successful wait. They report +// "not done yet" instead, so that waitDependency's next iteration sees +// ctx.Done() and decides what the end of the wait means. A genuine failure that +// coincides with the end of the wait goes unreported on purpose: the wait is +// over anyway. +func (s *composeService) checkDependencyRunningOrHealthy(ctx context.Context, dep string, config types.ServiceDependency, waitingFor Containers) (bool, error) { + isHealthy, err := s.isServiceHealthy(ctx, waitingFor, true) + if err != nil { + if !config.Required { + if ctx.Err() != nil { + return false, nil + } + s.events.On(containerReasonEvents(waitingFor, skippedEvent, + fmt.Sprintf("optional dependency %q is not running or is unhealthy", dep))...) + logrus.Warnf("optional dependency %q is not running or is unhealthy: %s", dep, err.Error()) + return true, nil + } + return false, err + } + if isHealthy { + s.events.On(containerEvents(waitingFor, healthy)...) + } + return isHealthy, nil +} + +func (s *composeService) checkDependencyHealthy(ctx context.Context, dep string, config types.ServiceDependency, waitingFor Containers) (bool, error) { + isHealthy, err := s.isServiceHealthy(ctx, waitingFor, false) + if err != nil { + if !config.Required { + if ctx.Err() != nil { + return false, nil + } + s.events.On(containerReasonEvents(waitingFor, skippedEvent, + fmt.Sprintf("optional dependency %q failed to start", dep))...) + logrus.Warnf("optional dependency %q failed to start: %s", dep, err.Error()) + return true, nil + } + if !errors.Is(err, context.Canceled) { + // a failure caused by a canceled ctx says nothing about the + // dependency: its rows are not marked failed for it. The error is + // tested, not the ctx: a genuine failure (unhealthy, exited) + // found just as another node cancels the phase is still reported + s.events.On(containerEvents(waitingFor, func(s string) api.Resource { + return errorEventf(s, "dependency %s failed to start", dep) + })...) + } + return false, fmt.Errorf("dependency failed to start: %w", err) + } + if isHealthy { + s.events.On(containerEvents(waitingFor, healthy)...) + } + return isHealthy, nil +} + +func (s *composeService) checkDependencyCompleted(ctx context.Context, dep string, config types.ServiceDependency, waitingFor Containers) (bool, error) { + isExited, code, err := s.isServiceCompleted(ctx, waitingFor) + if err != nil { + return false, err + } + if !isExited { + return false, nil + } + if code == 0 { + s.events.On(containerEvents(waitingFor, exited)...) + return true, nil + } + + messageSuffix := fmt.Sprintf("%q didn't complete successfully: exit %d", dep, code) + if !config.Required { + // optional -> mark as skipped & don't propagate error + s.events.On(containerReasonEvents(waitingFor, skippedEvent, + "optional dependency "+messageSuffix)...) + logrus.Warnf("optional dependency %s", messageSuffix) + return true, nil + } + + msg := "service " + messageSuffix + s.events.On(containerEvents(waitingFor, func(s string) api.Resource { + return errorEventf(s, "service %s", messageSuffix) + })...) + return false, errors.New(msg) +} + +func shouldWaitForDependency(serviceName string, dependencyConfig types.ServiceDependency, project *types.Project) (bool, error) { + if dependencyConfig.Condition == types.ServiceConditionStarted { + // already ensured by the dependency order + return false, nil + } + if service, err := project.GetService(serviceName); err != nil { + for _, ds := range project.DisabledServices { + if ds.Name == serviceName { + // don't wait for disabled service (--no-deps) + return false, nil + } + } + return false, err + } else if service.GetScale() == 0 { + // don't wait for the dependency which configured to have 0 containers running + return false, nil + } else if service.Provider != nil { + // don't wait for provider services + return false, nil + } + return true, nil +} + +func (s *composeService) isServiceHealthy(ctx context.Context, containers Containers, fallbackRunning bool) (bool, error) { + for _, ctr := range containers { + res, err := s.apiClient().ContainerInspect(ctx, ctr.ID, client.ContainerInspectOptions{}) + if err != nil { + return false, err + } + ctr := res.Container + name := ctr.Name[1:] + + if ctr.State.Status == container.StateExited { + return false, fmt.Errorf("container %s exited (%d)", name, ctr.State.ExitCode) + } + + noHealthcheck := ctr.Config.Healthcheck == nil || (len(ctr.Config.Healthcheck.Test) > 0 && ctr.Config.Healthcheck.Test[0] == "NONE") + if noHealthcheck && fallbackRunning { + // Container does not define a health check, but we can fall back to "running" state + return ctr.State != nil && ctr.State.Status == container.StateRunning, nil + } + + if ctr.State == nil || ctr.State.Health == nil { + return false, fmt.Errorf("container %s has no healthcheck configured", name) + } + switch ctr.State.Health.Status { + case container.Healthy: + // Continue by checking the next container. + case container.Unhealthy: + return false, fmt.Errorf("container %s is unhealthy", name) + case container.Starting: + return false, nil + default: + return false, fmt.Errorf("container %s had unexpected health status %q", name, ctr.State.Health.Status) + } + } + return true, nil +} + +func (s *composeService) isServiceCompleted(ctx context.Context, containers Containers) (bool, int, error) { + if s.dryRun { + // dry-run never actually starts the dependency's container, so it + // can never observe a real "exited" state: simulate immediate + // success instead of polling forever. + return true, 0, nil + } + for _, ctr := range containers { + res, err := s.apiClient().ContainerInspect(ctx, ctr.ID, client.ContainerInspectOptions{}) + if err != nil { + return false, 0, err + } + if res.Container.State != nil && res.Container.State.Status == container.StateExited { + return true, res.Container.State.ExitCode, nil + } + } + return false, 0, nil +} diff --git a/pkg/compose/dependency_wait_test.go b/pkg/compose/dependency_wait_test.go new file mode 100644 index 0000000000..0e22ebfe62 --- /dev/null +++ b/pkg/compose/dependency_wait_test.go @@ -0,0 +1,796 @@ +/* + 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" + "errors" + "fmt" + "strings" + "testing" + "testing/synctest" + "time" + + "github.com/compose-spec/compose-go/v2/types" + "github.com/moby/moby/api/types/container" + "github.com/moby/moby/client" + logrustest "github.com/sirupsen/logrus/hooks/test" + "go.uber.org/mock/gomock" + "gotest.tools/v3/assert" + + "github.com/docker/compose/v5/pkg/api" + "github.com/docker/compose/v5/pkg/mocks" +) + +func TestWaitDependencies(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + apiClient := mocks.NewMockAPIClient(mockCtrl) + cli := mocks.NewMockCli(mockCtrl) + tested, err := NewComposeService(cli) + assert.NilError(t, err) + cli.EXPECT().Client().Return(apiClient).AnyTimes() + + t.Run("should skip dependencies with scale 0", func(t *testing.T) { + dbService := types.ServiceConfig{Name: "db", Scale: intPtr(0)} + redisService := types.ServiceConfig{Name: "redis", Scale: intPtr(0)} + project := types.Project{Name: strings.ToLower(testProject), Services: types.Services{ + "db": dbService, + "redis": redisService, + }} + dependencies := types.DependsOnConfig{ + "db": {Condition: ServiceConditionRunningOrHealthy}, + "redis": {Condition: ServiceConditionRunningOrHealthy}, + } + assert.NilError(t, tested.(*composeService).waitDependencies(t.Context(), &project, "", dependencies, nil)) + }) + t.Run("should skip zero-replica dependencies after service hashing", func(t *testing.T) { + replicas := 0 + project := types.Project{Name: strings.ToLower(testProject), Services: types.Services{ + "app": { + Name: "app", WorkloadSpec: types.WorkloadSpec{DependsOn: types.DependsOnConfig{ + "disabled": { + Condition: ServiceConditionRunningOrHealthy, + Required: true, + }, + }}, + }, + "disabled": { + Name: "disabled", + Deploy: &types.DeployConfig{Replicas: &replicas}, + }, + }} + + _, err := ServiceHash(project.Services["disabled"]) + assert.NilError(t, err) + + assert.NilError(t, tested.(*composeService).waitDependencies( + t.Context(), &project, "app", project.Services["app"].DependsOn, nil, + )) + }) + t.Run("should skip dependencies with condition service_started", func(t *testing.T) { + dbService := types.ServiceConfig{Name: "db", Scale: intPtr(1)} + redisService := types.ServiceConfig{Name: "redis", Scale: intPtr(1)} + project := types.Project{Name: strings.ToLower(testProject), Services: types.Services{ + "db": dbService, + "redis": redisService, + }} + dependencies := types.DependsOnConfig{ + "db": {Condition: types.ServiceConditionStarted, Required: true}, + "redis": {Condition: types.ServiceConditionStarted, Required: true}, + } + assert.NilError(t, tested.(*composeService).waitDependencies(t.Context(), &project, "", dependencies, nil)) + }) + t.Run("missing required dependency is an error", func(t *testing.T) { + project := types.Project{Name: strings.ToLower(testProject), Services: types.Services{ + "db": {Name: "db", Scale: intPtr(1)}, + }} + dependencies := types.DependsOnConfig{ + "db": {Condition: ServiceConditionRunningOrHealthy, Required: true}, + } + err := tested.(*composeService).waitDependencies(t.Context(), &project, "app", dependencies, nil) + assert.Error(t, err, "app is missing dependency db") + }) + t.Run("a dependency's pre_start hook runner does not count as the dependency being present", func(t *testing.T) { + project := types.Project{Name: strings.ToLower(testProject), Services: types.Services{ + "db": {Name: "db", Scale: intPtr(1), PreStart: []types.PreStartHook{{}}}, + }} + dependencies := types.DependsOnConfig{ + "db": {Condition: ServiceConditionRunningOrHealthy, Required: true}, + } + // Only a retained hook runner exists (e.g. its own success removal is + // still in flight, or it failed and was kept for inspection) — no + // real "db" replica container. + containers := Containers{{ + ID: "db-hook-runner", + Names: []string{"/db-hook-runner"}, + Labels: map[string]string{api.ServiceLabel: "db", api.HookLabel: "pre_start"}, + }} + err := tested.(*composeService).waitDependencies(t.Context(), &project, "app", dependencies, containers) + assert.Error(t, err, "app is missing dependency db") + }) + t.Run("missing optional dependency is only a warning", func(t *testing.T) { + project := types.Project{Name: strings.ToLower(testProject), Services: types.Services{ + "db": {Name: "db", Scale: intPtr(1)}, + }} + dependencies := types.DependsOnConfig{ + "db": {Condition: ServiceConditionRunningOrHealthy, Required: false}, + } + assert.NilError(t, tested.(*composeService).waitDependencies(t.Context(), &project, "app", dependencies, nil)) + }) + t.Run("failing optional dependency is skipped, not an error", func(t *testing.T) { + project := types.Project{Name: strings.ToLower(testProject), Services: types.Services{ + "db": {Name: "db", Scale: intPtr(1)}, + }} + dependencies := types.DependsOnConfig{ + "db": {Condition: types.ServiceConditionHealthy, Required: false}, + } + containers := Containers{{ + ID: "db-ctr", + Names: []string{"/db-ctr"}, + Labels: map[string]string{api.ServiceLabel: "db"}, + }} + // The dependency exited: a required dependency would fail the wait, + // an optional one is skipped after the first poll. + apiClient.EXPECT().ContainerInspect(gomock.Any(), "db-ctr", gomock.Any()).Return(client.ContainerInspectResult{ + Container: container.InspectResponse{ + ID: "db-ctr", + Name: "/db-ctr", + State: &container.State{Status: container.StateExited, ExitCode: 1}, + }, + }, nil) + assert.NilError(t, tested.(*composeService).waitDependencies(t.Context(), &project, "app", dependencies, containers)) + }) + t.Run("dry-run completes service_completed_successfully immediately", func(t *testing.T) { + tested.(*composeService).dryRun = true + + project := types.Project{Name: strings.ToLower(testProject), Services: types.Services{ + "init": {Name: "init", Scale: intPtr(1)}, + }} + dependencies := types.DependsOnConfig{ + "init": {Condition: types.ServiceConditionCompletedSuccessfully, Required: true}, + } + containers := Containers{{ + ID: "init-ctr", + Names: []string{"/init-ctr"}, + Labels: map[string]string{api.ServiceLabel: "init"}, + }} + // no ContainerInspect expectation: dry-run must not inspect a + // container that was never created. + assert.NilError(t, tested.(*composeService).waitDependencies(t.Context(), &project, "app", dependencies, containers)) + }) +} + +func TestIsServiceHealthy(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + apiClient := mocks.NewMockAPIClient(mockCtrl) + cli := mocks.NewMockCli(mockCtrl) + tested, err := NewComposeService(cli) + assert.NilError(t, err) + cli.EXPECT().Client().Return(apiClient).AnyTimes() + + ctx := t.Context() + + t.Run("disabled healthcheck with fallback to running", func(t *testing.T) { + containerID := "test-container-id" + containers := Containers{ + {ID: containerID}, + } + + // Container with disabled healthcheck (Test: ["NONE"]) + apiClient.EXPECT().ContainerInspect(ctx, containerID, gomock.Any()).Return(client.ContainerInspectResult{ + Container: container.InspectResponse{ + ID: containerID, + Name: "test-container", + State: &container.State{Status: "running"}, + Config: &container.Config{ + Healthcheck: &container.HealthConfig{ + Test: []string{"NONE"}, + }, + }, + }, + }, nil) + + isHealthy, err := tested.(*composeService).isServiceHealthy(ctx, containers, true) + assert.NilError(t, err) + assert.Equal(t, true, isHealthy, "Container with disabled healthcheck should be considered healthy when running with fallbackRunning=true") + }) + + t.Run("disabled healthcheck without fallback", func(t *testing.T) { + containerID := "test-container-id" + containers := Containers{ + {ID: containerID}, + } + + // Container with disabled healthcheck (Test: ["NONE"]) but fallbackRunning=false + apiClient.EXPECT().ContainerInspect(ctx, containerID, gomock.Any()).Return(client.ContainerInspectResult{ + Container: container.InspectResponse{ + ID: containerID, + Name: "test-container", + State: &container.State{Status: "running"}, + Config: &container.Config{ + Healthcheck: &container.HealthConfig{ + Test: []string{"NONE"}, + }, + }, + }, + }, nil) + + _, err := tested.(*composeService).isServiceHealthy(ctx, containers, false) + assert.ErrorContains(t, err, "has no healthcheck configured") + }) + + t.Run("no healthcheck with fallback to running", func(t *testing.T) { + containerID := "test-container-id" + containers := Containers{ + {ID: containerID}, + } + + // Container with no healthcheck at all + apiClient.EXPECT().ContainerInspect(ctx, containerID, gomock.Any()).Return(client.ContainerInspectResult{ + Container: container.InspectResponse{ + ID: containerID, + Name: "test-container", + State: &container.State{Status: "running"}, + Config: &container.Config{ + Healthcheck: nil, + }, + }, + }, nil) + + isHealthy, err := tested.(*composeService).isServiceHealthy(ctx, containers, true) + assert.NilError(t, err) + assert.Equal(t, true, isHealthy, "Container with no healthcheck should be considered healthy when running with fallbackRunning=true") + }) + + t.Run("exited container with disabled healthcheck", func(t *testing.T) { + containerID := "test-container-id" + containers := Containers{ + {ID: containerID}, + } + + // Container with disabled healthcheck but exited + apiClient.EXPECT().ContainerInspect(ctx, containerID, gomock.Any()).Return(client.ContainerInspectResult{ + Container: container.InspectResponse{ + ID: containerID, + Name: "test-container", + State: &container.State{ + Status: "exited", + ExitCode: 1, + }, + Config: &container.Config{ + Healthcheck: &container.HealthConfig{ + Test: []string{"NONE"}, + }, + }, + }, + }, nil) + + _, err := tested.(*composeService).isServiceHealthy(ctx, containers, true) + assert.ErrorContains(t, err, "exited") + }) + + t.Run("healthy container with healthcheck", func(t *testing.T) { + containerID := "test-container-id" + containers := Containers{ + {ID: containerID}, + } + + // Container with actual healthcheck that is healthy + apiClient.EXPECT().ContainerInspect(ctx, containerID, gomock.Any()).Return(client.ContainerInspectResult{ + Container: container.InspectResponse{ + ID: containerID, + Name: "test-container", + State: &container.State{ + Status: "running", + Health: &container.Health{ + Status: container.Healthy, + }, + }, + Config: &container.Config{ + Healthcheck: &container.HealthConfig{ + Test: []string{"CMD", "curl", "-f", "http://localhost"}, + }, + }, + }, + }, nil) + + isHealthy, err := tested.(*composeService).isServiceHealthy(ctx, containers, false) + assert.NilError(t, err) + assert.Equal(t, true, isHealthy, "Container with healthy status should be healthy") + }) +} + +// startingDBInspect is db-ctr running with its healthcheck still starting. +var startingDBInspect = client.ContainerInspectResult{ + Container: container.InspectResponse{ + ID: "db-ctr", + Name: "/db-ctr", + Config: &container.Config{Healthcheck: &container.HealthConfig{Test: []string{"CMD", "true"}}}, + State: &container.State{Status: container.StateRunning, Health: &container.Health{Status: container.Starting}}, + }, +} + +// waitWithOwnTimeout runs one depends_on wait the way the plan's +// OpWaitCondition nodes do under --wait-timeout: bounded by a timeout of its +// own (planExecutor.waitTimeout), the only way a wait gets one. A watchdog +// ends a wait that would ignore that timeout. +func waitWithOwnTimeout(t *testing.T, svc *composeService, project *types.Project, containers Containers, condition string, required bool, timeout time.Duration) error { + t.Helper() + exec := svc.newPlanExecutor(project, emptyObservedState(project.Name)) + exec.containersByService["db"] = containers + exec.waitTimeout = timeout + ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second) + defer cancel() + return exec.execBoundedWaitCondition(ctx, Operation{ + Type: OpWaitCondition, + ResourceID: "wait:db:" + condition, + Name: "db", + Condition: condition, + BestEffort: !required, + }) +} + +// TestWaitDependencyDeadline locks the timeout semantics of the dependency +// wait: an expired deadline surfaces as "timeout waiting for dependencies", +// while a plain user cancellation is not a wait failure. +func TestWaitDependencyDeadline(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + apiClient := mocks.NewMockAPIClient(mockCtrl) + cli := mocks.NewMockCli(mockCtrl) + tested, err := NewComposeService(cli) + assert.NilError(t, err) + cli.EXPECT().Client().Return(apiClient).AnyTimes() + + project := types.Project{Name: strings.ToLower(testProject), Services: types.Services{ + "db": {Name: "db", Scale: intPtr(1)}, + }} + dependencies := types.DependsOnConfig{ + "db": {Condition: types.ServiceConditionHealthy, Required: true}, + } + containers := Containers{{ + ID: "db-ctr", + Names: []string{"/db-ctr"}, + Labels: map[string]string{api.ServiceLabel: "db"}, + }} + // stillStarting lets the timeout below expire between two checks. + stillStarting := func() { + apiClient.EXPECT().ContainerInspect(gomock.Any(), "db-ctr", gomock.Any()).Return(startingDBInspect, nil) + } + + t.Run("expired deadline is an error", func(t *testing.T) { + // The caller's deadline (--wait-timeout of up/start --wait) fires + // between two checks and must not be swallowed. + stillStarting() + ctx, cancel := context.WithTimeout(t.Context(), 50*time.Millisecond) + defer cancel() + err := tested.(*composeService).waitDependencies(ctx, &project, "app", dependencies, containers) + assert.Error(t, err, "timeout waiting for dependencies") + }) + + t.Run("expired own timeout of a plan wait is an error", func(t *testing.T) { + // The same, for the timeout of the plan's own wait node. + stillStarting() + err := waitWithOwnTimeout(t, tested.(*composeService), &project, containers, types.ServiceConditionHealthy, true, 50*time.Millisecond) + assert.Assert(t, errors.Is(err, errWaitTimeout), "got %v", err) + assert.ErrorIs(t, err, context.DeadlineExceeded) + }) + + // Regression guard: an optional dependency's deadline expiring (the + // timeout of the plan's own wait node, planExecutor.waitTimeout -- see + // waitDependency's ctx.Done() branch) must be tolerated exactly like any + // other definitive failure of an optional dependency, not surfaced as + // "timeout waiting for dependencies". + t.Run("expired own timeout on an optional dependency is skipped, not an error", func(t *testing.T) { + stillStarting() + err := waitWithOwnTimeout(t, tested.(*composeService), &project, containers, types.ServiceConditionHealthy, false, 50*time.Millisecond) + assert.NilError(t, err) + }) + + // Regression guard: the tolerance above must apply only to a deadline + // THIS wait's own timeout is responsible for -- an unrelated, + // already-expired deadline the caller brought in (waitDependencies sets + // none of its own) is a different, real failure that must still + // propagate even for an optional dependency. + t.Run("an inherited parent deadline is not swallowed for an optional dependency", func(t *testing.T) { + optionalDeps := types.DependsOnConfig{ + "db": {Condition: types.ServiceConditionHealthy, Required: false}, + } + parentCtx, cancel := context.WithTimeout(t.Context(), time.Millisecond) + defer cancel() + time.Sleep(10 * time.Millisecond) // let the parent's own deadline actually expire first + err := tested.(*composeService).waitDependencies(parentCtx, &project, "app", optionalDeps, containers) + assert.Error(t, err, "timeout waiting for dependencies") + }) + + t.Run("user cancellation is not a wait failure", func(t *testing.T) { + ctx, cancel := context.WithCancel(t.Context()) + cancel() + err := tested.(*composeService).waitDependencies(ctx, &project, "app", dependencies, containers) + assert.NilError(t, err) + }) + + // Regression guard for the runtime fallback warning on waitDependency's + // default branch — see the comment on that branch for why it must stay. + t.Run("unsupported condition warns and returns without waiting", func(t *testing.T) { + hook := logrustest.NewGlobal() + unsupportedDeps := types.DependsOnConfig{ + "db": {Condition: "some_future_condition", Required: true}, + } + // the bound makes a regression that waits fail instead of hanging + ctx, cancel := context.WithTimeout(t.Context(), 2*time.Second) + defer cancel() + err := tested.(*composeService).waitDependencies(ctx, &project, "app", unsupportedDeps, containers) + assert.NilError(t, err) + + var messages []string + for _, e := range hook.AllEntries() { + messages = append(messages, e.Message) + } + joined := strings.Join(messages, "\n") + assert.Assert(t, strings.Contains(joined, `service "app": unsupported depends_on condition "some_future_condition"`), joined) + }) +} + +// TestWaitDependencyFirstCheck locks when a satisfied dependency is first +// checked: on entry, except running_or_healthy (see waitDependencyReached). +func TestWaitDependencyFirstCheck(t *testing.T) { + project := types.Project{Name: strings.ToLower(testProject), Services: types.Services{ + "db": {Name: "db", Scale: intPtr(1)}, + }} + containers := Containers{{ + ID: "db-ctr", + Names: []string{"/db-ctr"}, + Labels: map[string]string{api.ServiceLabel: "db"}, + }} + satisfied := map[string]struct { + inspect container.InspectResponse + wait time.Duration + }{ + types.ServiceConditionHealthy: { + inspect: container.InspectResponse{ + ID: "db-ctr", + Name: "/db-ctr", + Config: &container.Config{Healthcheck: &container.HealthConfig{Test: []string{"CMD", "true"}}}, + State: &container.State{Status: container.StateRunning, Health: &container.Health{Status: container.Healthy}}, + }, + wait: 0, + }, + types.ServiceConditionCompletedSuccessfully: { + inspect: container.InspectResponse{ + ID: "db-ctr", + Name: "/db-ctr", + State: &container.State{Status: container.StateExited, ExitCode: 0}, + }, + wait: 0, + }, + ServiceConditionRunningOrHealthy: { + inspect: container.InspectResponse{ + ID: "db-ctr", + Name: "/db-ctr", + Config: &container.Config{}, + State: &container.State{Status: container.StateRunning}, + }, + wait: 500 * time.Millisecond, + }, + } + + for condition, tc := range satisfied { + t.Run(condition, func(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + mockCtrl := gomock.NewController(t) + apiClient := mocks.NewMockAPIClient(mockCtrl) + cli := mocks.NewMockCli(mockCtrl) + cli.EXPECT().Client().Return(apiClient).AnyTimes() + tested, err := NewComposeService(cli) + assert.NilError(t, err) + apiClient.EXPECT().ContainerInspect(gomock.Any(), "db-ctr", gomock.Any()). + Return(client.ContainerInspectResult{Container: tc.inspect}, nil) + + dependencies := types.DependsOnConfig{"db": {Condition: condition, Required: true}} + start := time.Now() + err = tested.(*composeService).waitDependencies(t.Context(), &project, "app", dependencies, containers) + assert.NilError(t, err) + assert.Equal(t, time.Since(start), tc.wait) + }) + }) + } +} + +// TestOptionalDependencyInspectFailure locks what an optional (required: +// false) dependency's failing inspection means. A genuine inspection error +// (the daemon doesn't know the container, the API call fails) says the +// dependency didn't start, so it is skipped. A failure caused by the wait's +// context being done says nothing about the dependency: it must not be +// turned into a skipped dependency, i.e. into a successful wait, and the end +// of the wait is left to waitDependency (a deadline is an error, a plain +// cancellation stays silent). A required dependency is never skipped. +func TestOptionalDependencyInspectFailure(t *testing.T) { + const skipped = "Skipped: " + conditions := []string{ServiceConditionRunningOrHealthy, types.ServiceConditionHealthy} + dbContainers := Containers{{ + ID: "db-ctr", + Names: []string{"/db-ctr"}, + Labels: map[string]string{api.ServiceLabel: "db"}, + }} + project := types.Project{Name: strings.ToLower(testProject), Services: types.Services{ + "db": {Name: "db", Scale: intPtr(1)}, + }} + + newTested := func(t *testing.T) (*composeService, *mocks.MockAPIClient, *capturingEvents) { + t.Helper() + mockCtrl := gomock.NewController(t) + apiClient := mocks.NewMockAPIClient(mockCtrl) + cli := mocks.NewMockCli(mockCtrl) + cli.EXPECT().Client().Return(apiClient).AnyTimes() + events := &capturingEvents{} + tested, err := NewComposeService(cli, WithEventProcessor(events)) + assert.NilError(t, err) + return tested.(*composeService), apiClient, events + } + wasSkipped := func(events *capturingEvents) bool { + for _, r := range events.resources { + if strings.HasPrefix(r.Text, skipped) { + return true + } + } + return false + } + dependenciesOn := func(condition string, required bool) types.DependsOnConfig { + return types.DependsOnConfig{"db": {Condition: condition, Required: required}} + } + // inspectFailsWithContext makes ContainerInspect fail the way the client + // does when the context it was given is done: it waits for it and + // returns its error, wrapped. It must be reached at least once (or the + // test would pass without exercising the inspection); once the context is + // done, a pending poll tick may win the select against ctx.Done() for one + // more check, so there is no upper bound. + inspectFailsWithContext := func(apiClient *mocks.MockAPIClient) { + apiClient.EXPECT().ContainerInspect(gomock.Any(), "db-ctr", gomock.Any()). + DoAndReturn(func(ctx context.Context, _ string, _ client.ContainerInspectOptions) (client.ContainerInspectResult, error) { + <-ctx.Done() + return client.ContainerInspectResult{}, fmt.Errorf("inspect db-ctr: %w", ctx.Err()) + }).MinTimes(1) + } + + for _, condition := range conditions { + t.Run(condition, func(t *testing.T) { + t.Run("a genuine inspection error skips an optional dependency", func(t *testing.T) { + t.Parallel() + tested, apiClient, events := newTested(t) + apiClient.EXPECT().ContainerInspect(gomock.Any(), "db-ctr", gomock.Any()). + Return(client.ContainerInspectResult{}, errors.New("Error response from daemon: No such container: db-ctr")) + err := tested.waitDependencies(t.Context(), &project, "app", dependenciesOn(condition, false), dbContainers) + assert.NilError(t, err) + assert.Assert(t, wasSkipped(events)) + }) + + t.Run("a genuine inspection error fails a required dependency", func(t *testing.T) { + t.Parallel() + tested, apiClient, events := newTested(t) + apiClient.EXPECT().ContainerInspect(gomock.Any(), "db-ctr", gomock.Any()). + Return(client.ContainerInspectResult{}, errors.New("Error response from daemon: No such container: db-ctr")) + err := tested.waitDependencies(t.Context(), &project, "app", dependenciesOn(condition, true), dbContainers) + assert.ErrorContains(t, err, "No such container: db-ctr") + assert.Assert(t, !wasSkipped(events)) + }) + + t.Run("a cancellation during the inspection is not a skipped optional dependency", func(t *testing.T) { + t.Parallel() + tested, apiClient, events := newTested(t) + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + apiClient.EXPECT().ContainerInspect(gomock.Any(), "db-ctr", gomock.Any()). + DoAndReturn(func(context.Context, string, client.ContainerInspectOptions) (client.ContainerInspectResult, error) { + cancel() + return client.ContainerInspectResult{}, fmt.Errorf("inspect db-ctr: %w", context.Canceled) + }).MinTimes(1) // a pending poll tick may win the select once more + // a plain cancellation is not a wait failure (see + // TestWaitDependencyDeadline), but it is no skip either + err := tested.waitDependencies(ctx, &project, "app", dependenciesOn(condition, false), dbContainers) + assert.NilError(t, err) + assert.Assert(t, !wasSkipped(events)) + }) + + t.Run("a cancellation during the inspection still fails a required dependency", func(t *testing.T) { + t.Parallel() + tested, apiClient, _ := newTested(t) + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + apiClient.EXPECT().ContainerInspect(gomock.Any(), "db-ctr", gomock.Any()). + DoAndReturn(func(context.Context, string, client.ContainerInspectOptions) (client.ContainerInspectResult, error) { + cancel() + return client.ContainerInspectResult{}, fmt.Errorf("inspect db-ctr: %w", context.Canceled) + }) + err := tested.waitDependencies(ctx, &project, "app", dependenciesOn(condition, true), dbContainers) + assert.Assert(t, errors.Is(err, context.Canceled), "got %v", err) + }) + + for _, required := range []bool{false, true} { + t.Run(fmt.Sprintf("a deadline inherited from the caller expiring during the inspection is an error (required=%t)", required), func(t *testing.T) { + t.Parallel() + tested, apiClient, events := newTested(t) + inspectFailsWithContext(apiClient) + // the first check starts the inspection, which only ends + // with the deadline + ctx, cancel := context.WithTimeout(t.Context(), time.Second) + defer cancel() + err := tested.waitDependencies(ctx, &project, "app", dependenciesOn(condition, required), dbContainers) + assert.Error(t, err, "timeout waiting for dependencies") + assert.Assert(t, !wasSkipped(events)) + }) + } + + // The wait's own timeout expiring during the inspection must end + // the wait exactly as it does when it expires between two polls: + // whatever waitDependency decides for an optional dependency, the + // check functions must not decide differently. + for _, required := range []bool{false, true} { + t.Run(fmt.Sprintf("the wait's own timeout expiring during the inspection ends like it does between polls (required=%t)", required), func(t *testing.T) { + t.Parallel() + tested, apiClient, _ := newTested(t) + inspectFailsWithContext(apiClient) + duringInspect := waitWithOwnTimeout(t, tested, &project, dbContainers, condition, required, time.Second) + + // no check in flight when the timeout expires: at most the one on entry + polled, polledClient, _ := newTested(t) + polledClient.EXPECT().ContainerInspect(gomock.Any(), "db-ctr", gomock.Any()).Return(startingDBInspect, nil).MaxTimes(1) + betweenPolls := waitWithOwnTimeout(t, polled, &project, dbContainers, condition, required, 50*time.Millisecond) + + // the same end, whatever the instant the timeout expired at + assert.Equal(t, duringInspect == nil, betweenPolls == nil) + assert.Equal(t, errors.Is(duringInspect, errWaitTimeout), errors.Is(betweenPolls, errWaitTimeout)) + assert.Equal(t, errors.Is(duringInspect, context.DeadlineExceeded), errors.Is(betweenPolls, context.DeadlineExceeded)) + if required { + assert.Assert(t, errors.Is(duringInspect, errWaitTimeout), "got %v", duringInspect) + assert.ErrorIs(t, duringInspect, context.DeadlineExceeded) + } else { + assert.NilError(t, duringInspect) + } + }) + } + }) + } +} + +// TestCheckDependencyDoneContext tests the check functions directly with a +// context that is already done, so no poll timing is involved: an optional +// dependency is "not done yet" (the wait's loop then sees ctx.Done()), never +// skipped; a required one keeps reporting the failure. +// A genuine failure of a required dependency is reported even when the context +// happens to be canceled by the time it is evaluated: only an error caused by +// the cancellation is silent. +func TestCheckDependencyHealthyReportsGenuineFailureOnCanceledContext(t *testing.T) { + containers := Containers{{ + ID: "db-ctr", + Names: []string{"/db-ctr"}, + Labels: map[string]string{api.ServiceLabel: "db"}, + }} + mockCtrl := gomock.NewController(t) + apiClient := mocks.NewMockAPIClient(mockCtrl) + cli := mocks.NewMockCli(mockCtrl) + cli.EXPECT().Client().Return(apiClient).AnyTimes() + events := &capturingEvents{} + svc, err := NewComposeService(cli, WithEventProcessor(events)) + assert.NilError(t, err) + + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + apiClient.EXPECT().ContainerInspect(gomock.Any(), "db-ctr", gomock.Any()). + DoAndReturn(func(context.Context, string, client.ContainerInspectOptions) (client.ContainerInspectResult, error) { + // the phase is canceled right after the inspection answered + cancel() + return client.ContainerInspectResult{ + Container: container.InspectResponse{ + ID: "db-ctr", + Name: "/db-ctr", + State: &container.State{ + Status: container.StateRunning, + Health: &container.Health{Status: container.Unhealthy}, + }, + Config: &container.Config{ + Healthcheck: &container.HealthConfig{Test: []string{"CMD", "true"}}, + }, + }, + }, nil + }) + + done, err := svc.(*composeService).checkDependencyHealthy(ctx, "db", types.ServiceDependency{Required: true}, containers) + assert.ErrorContains(t, err, "is unhealthy") + assert.Assert(t, !done) + assert.Equal(t, len(events.resources), 1, "got %v", events.resources) + assert.Equal(t, events.resources[0].Status, api.Error) +} + +func TestCheckDependencyDoneContext(t *testing.T) { + containers := Containers{{ + ID: "db-ctr", + Names: []string{"/db-ctr"}, + Labels: map[string]string{api.ServiceLabel: "db"}, + }} + expired := func(t *testing.T) context.Context { + ctx, cancel := context.WithDeadline(t.Context(), time.Now().Add(-time.Second)) + t.Cleanup(cancel) + return ctx + } + canceled := func(t *testing.T) context.Context { + ctx, cancel := context.WithCancel(t.Context()) + cancel() + return ctx + } + contexts := map[string]func(*testing.T) context.Context{"canceled": canceled, "expired": expired} + checks := map[string]func(*composeService, context.Context, types.ServiceDependency) (bool, error){ + "running_or_healthy": func(s *composeService, ctx context.Context, c types.ServiceDependency) (bool, error) { + return s.checkDependencyRunningOrHealthy(ctx, "db", c, containers) + }, + "service_healthy": func(s *composeService, ctx context.Context, c types.ServiceDependency) (bool, error) { + return s.checkDependencyHealthy(ctx, "db", c, containers) + }, + } + + for checkName, check := range checks { + for ctxName, newCtx := range contexts { + t.Run(checkName+"/"+ctxName, func(t *testing.T) { + t.Parallel() + mockCtrl := gomock.NewController(t) + apiClient := mocks.NewMockAPIClient(mockCtrl) + cli := mocks.NewMockCli(mockCtrl) + cli.EXPECT().Client().Return(apiClient).AnyTimes() + events := &capturingEvents{} + svc, err := NewComposeService(cli, WithEventProcessor(events)) + assert.NilError(t, err) + tested := svc.(*composeService) + + ctx := newCtx(t) + apiClient.EXPECT().ContainerInspect(gomock.Any(), "db-ctr", gomock.Any()). + Return(client.ContainerInspectResult{}, fmt.Errorf("inspect db-ctr: %w", ctx.Err())).Times(2) + + done, err := check(tested, ctx, types.ServiceDependency{Required: false}) + assert.NilError(t, err) + assert.Assert(t, !done, "an optional dependency must not complete on a done context") + assert.Equal(t, len(events.resources), 0, "no skipped event expected, got %v", events.resources) + + _, err = check(tested, ctx, types.ServiceDependency{Required: true}) + assert.Assert(t, errors.Is(err, ctx.Err()), "got %v", err) + }) + } + } + + // service_completed_successfully never skipped an optional dependency on + // an inspection error (only on a non-zero exit code): unchanged. + t.Run("service_completed_successfully", func(t *testing.T) { + t.Parallel() + mockCtrl := gomock.NewController(t) + apiClient := mocks.NewMockAPIClient(mockCtrl) + cli := mocks.NewMockCli(mockCtrl) + cli.EXPECT().Client().Return(apiClient).AnyTimes() + svc, err := NewComposeService(cli) + assert.NilError(t, err) + + ctx := canceled(t) + apiClient.EXPECT().ContainerInspect(gomock.Any(), "db-ctr", gomock.Any()). + Return(client.ContainerInspectResult{}, ctx.Err()) + done, err := svc.(*composeService).checkDependencyCompleted(ctx, "db", types.ServiceDependency{Required: false}, containers) + assert.Assert(t, errors.Is(err, context.Canceled), "got %v", err) + assert.Assert(t, !done) + }) +} diff --git a/pkg/compose/service_containers.go b/pkg/compose/service_containers.go index e3f3ae79ea..60fb896a55 100644 --- a/pkg/compose/service_containers.go +++ b/pkg/compose/service_containers.go @@ -18,13 +18,11 @@ package compose import ( "context" - "errors" "fmt" "maps" "strconv" "strings" "sync" - "time" "github.com/compose-spec/compose-go/v2/types" "github.com/containerd/platforms" @@ -33,7 +31,6 @@ import ( "github.com/moby/moby/client/pkg/versions" specs "github.com/opencontainers/image-spec/specs-go/v1" "github.com/sirupsen/logrus" - "golang.org/x/sync/errgroup" "github.com/docker/compose/v5/pkg/api" ) @@ -157,251 +154,12 @@ func containerReasonEvents(containers Containers, eventFunc func(string, string) return events } -// ServiceConditionRunningOrHealthy is a service condition on status running or healthy -const ServiceConditionRunningOrHealthy = "running_or_healthy" - -func (s *composeService) waitDependencies(ctx context.Context, project *types.Project, dependant string, dependencies types.DependsOnConfig, containers Containers) error { - origCtx := ctx - eg, ctx := errgroup.WithContext(ctx) - for dep, config := range dependencies { - if shouldWait, err := shouldWaitForDependency(dep, config, project); err != nil { - return err - } else if !shouldWait { - continue - } - - waitingFor := containers.filter(isService(dep), isNotOneOff, isNotHookContainer) - s.events.On(containerEvents(waitingFor, waiting)...) - if len(waitingFor) == 0 { - if config.Required { - return fmt.Errorf("%s is missing dependency %s", dependant, dep) - } - logrus.Warnf("%s is missing dependency %s", dependant, dep) - continue - } - - eg.Go(func() error { - return s.waitDependency(ctx, origCtx, dependant, dep, config, waitingFor) - }) - } - err := eg.Wait() - if errors.Is(err, context.DeadlineExceeded) { - return errors.New("timeout waiting for dependencies") - } - return err -} - -// waitDependency polls the dependency's containers until its depends_on -// condition is satisfied (done), definitively failed (err), or ctx is -// cancelled. Each check reports (done, err): (false, nil) means keep polling. -// origCtx is ctx as it stood before the caller wrapped it with this wait's -// own timeout (if any: waitDependencies sets none, the plan's wait nodes do, -// see execBoundedWaitCondition) -- comparing origCtx.Err() against ctx.Err() at the -// moment ctx.Done() fires is how this tells apart a DeadlineExceeded this -// specific call's own wrap is responsible for from one inherited from an -// ancestor context the caller never touched. A bare "did the caller -// configure a timeout at all" flag isn't enough: if origCtx itself already -// carries an earlier deadline, wrapping it with context.WithTimeout still -// makes both origCtx and ctx report DeadlineExceeded once that earlier -// deadline fires, even though this call's own timeout never actually -// elapsed -- only origCtx.Err() == nil proves this wrap's own timeout is -// the one that fired. Only that case is safe to tolerate on an optional -// dependency; an inherited deadline means something else entirely (an -// external deadline, a test harness timeout) fired, and must keep -// propagating regardless of Required. -func (s *composeService) waitDependency(ctx, origCtx context.Context, dependant, dep string, config types.ServiceDependency, waitingFor Containers) error { - _, err := s.waitDependencyReached(ctx, origCtx, dependant, dep, config, waitingFor) - return err -} - -// waitDependencyReached is waitDependency, which also tells whether the wait is -// over because the dependency is ready (or an optional one was given up on), -// as opposed to a plain cancellation of ctx, which ends it silently. -func (s *composeService) waitDependencyReached(ctx, origCtx context.Context, dependant, dep string, config types.ServiceDependency, waitingFor Containers) (bool, error) { - ticker := time.NewTicker(500 * time.Millisecond) - defer ticker.Stop() - // running_or_healthy accepts a running container without a healthcheck: - // checked right after it starts, one that exits at once would pass. - checkOnEntry := config.Condition == types.ServiceConditionHealthy || config.Condition == types.ServiceConditionCompletedSuccessfully - for first := true; ; first = false { - // a tick and the end of ctx can be ready together, and select would - // then pick either: ctx wins, so that a canceled wait is never run - // through one more check - if !first || !checkOnEntry { - select { - case <-ticker.C: - case <-ctx.Done(): - } - } - if ctx.Err() != nil { - // An expired deadline is precisely the failure this wait is meant - // to detect for a required dependency; only a plain cancellation - // (Ctrl-C) stays silent either way. An optional dependency - // tolerates a timeout exactly like it tolerates any other - // definitive failure the check* functions below report — - // skipped, not aborted — but only this call's OWN timeout: an - // inherited deadline is a different, real failure the caller - // must still see. - if errors.Is(ctx.Err(), context.DeadlineExceeded) { - if !config.Required && origCtx.Err() == nil { - s.events.On(containerReasonEvents(waitingFor, skippedEvent, - fmt.Sprintf("optional dependency %q did not satisfy %q before the timeout", dep, config.Condition))...) - logrus.Warnf("optional dependency %q did not satisfy %q before the timeout", dep, config.Condition) - return true, nil - } - return false, ctx.Err() - } - return false, nil - } - var ( - done bool - err error - ) - switch config.Condition { - case ServiceConditionRunningOrHealthy: - done, err = s.checkDependencyRunningOrHealthy(ctx, dep, config, waitingFor) - case types.ServiceConditionHealthy: - done, err = s.checkDependencyHealthy(ctx, dep, config, waitingFor) - case types.ServiceConditionCompletedSuccessfully: - done, err = s.checkDependencyCompleted(ctx, dep, config, waitingFor) - default: - // Every condition this switch doesn't handle explicitly ends up - // here: ServiceConditionStarted is filtered out before this - // function is ever called (shouldWaitForDependency — "already - // ensured by the dependency order"), so nothing reaching this - // branch is ever a value this runtime actually understands. - // That covers a compose file with an unrecognized condition - // (rejected by schema.Validate before it can even get this far - // — see unsupported_attributes.go's comment on - // valueConditionalAttributes) as well as the two situations - // that never go through schema validation at all: a project - // rebuilt from live container labels (see projectFromName), or - // one written by an older/newer Compose version using a - // condition value this build doesn't recognize. - logrus.Warnf("service %q: unsupported depends_on condition %q, skipping wait for %q", dependant, config.Condition, dep) - return true, nil - } - if done || err != nil { - return done, err - } - } -} - -// checkDependencyRunningOrHealthy and checkDependencyHealthy skip an optional -// dependency whose inspection fails. A failure caused by ctx being done says -// nothing about the dependency though: reporting it as "skipped" would turn a -// cancellation or an expired deadline into a successful wait. They report -// "not done yet" instead, so that waitDependency's next iteration sees -// ctx.Done() and decides what the end of the wait means. A genuine failure that -// coincides with the end of the wait goes unreported on purpose: the wait is -// over anyway. -func (s *composeService) checkDependencyRunningOrHealthy(ctx context.Context, dep string, config types.ServiceDependency, waitingFor Containers) (bool, error) { - isHealthy, err := s.isServiceHealthy(ctx, waitingFor, true) - if err != nil { - if !config.Required { - if ctx.Err() != nil { - return false, nil - } - s.events.On(containerReasonEvents(waitingFor, skippedEvent, - fmt.Sprintf("optional dependency %q is not running or is unhealthy", dep))...) - logrus.Warnf("optional dependency %q is not running or is unhealthy: %s", dep, err.Error()) - return true, nil - } - return false, err - } - if isHealthy { - s.events.On(containerEvents(waitingFor, healthy)...) - } - return isHealthy, nil -} - -func (s *composeService) checkDependencyHealthy(ctx context.Context, dep string, config types.ServiceDependency, waitingFor Containers) (bool, error) { - isHealthy, err := s.isServiceHealthy(ctx, waitingFor, false) - if err != nil { - if !config.Required { - if ctx.Err() != nil { - return false, nil - } - s.events.On(containerReasonEvents(waitingFor, skippedEvent, - fmt.Sprintf("optional dependency %q failed to start", dep))...) - logrus.Warnf("optional dependency %q failed to start: %s", dep, err.Error()) - return true, nil - } - if !errors.Is(err, context.Canceled) { - // a failure caused by a canceled ctx says nothing about the - // dependency: its rows are not marked failed for it. The error is - // tested, not the ctx: a genuine failure (unhealthy, exited) - // found just as another node cancels the phase is still reported - s.events.On(containerEvents(waitingFor, func(s string) api.Resource { - return errorEventf(s, "dependency %s failed to start", dep) - })...) - } - return false, fmt.Errorf("dependency failed to start: %w", err) - } - if isHealthy { - s.events.On(containerEvents(waitingFor, healthy)...) - } - return isHealthy, nil -} - -func (s *composeService) checkDependencyCompleted(ctx context.Context, dep string, config types.ServiceDependency, waitingFor Containers) (bool, error) { - isExited, code, err := s.isServiceCompleted(ctx, waitingFor) - if err != nil { - return false, err - } - if !isExited { - return false, nil - } - if code == 0 { - s.events.On(containerEvents(waitingFor, exited)...) - return true, nil - } - - messageSuffix := fmt.Sprintf("%q didn't complete successfully: exit %d", dep, code) - if !config.Required { - // optional -> mark as skipped & don't propagate error - s.events.On(containerReasonEvents(waitingFor, skippedEvent, - "optional dependency "+messageSuffix)...) - logrus.Warnf("optional dependency %s", messageSuffix) - return true, nil - } - - msg := "service " + messageSuffix - s.events.On(containerEvents(waitingFor, func(s string) api.Resource { - return errorEventf(s, "service %s", messageSuffix) - })...) - return false, errors.New(msg) -} - // errNoContainerToStart is the failure of a service the start phase finds no // container to start for (see planServiceStart). func errNoContainerToStart(service string) error { return fmt.Errorf("service %q has no container to start", service) } -func shouldWaitForDependency(serviceName string, dependencyConfig types.ServiceDependency, project *types.Project) (bool, error) { - if dependencyConfig.Condition == types.ServiceConditionStarted { - // already ensured by the dependency order - return false, nil - } - if service, err := project.GetService(serviceName); err != nil { - for _, ds := range project.DisabledServices { - if ds.Name == serviceName { - // don't wait for disabled service (--no-deps) - return false, nil - } - } - return false, err - } else if service.GetScale() == 0 { - // don't wait for the dependency which configured to have 0 containers running - return false, nil - } else if service.Provider != nil { - // don't wait for provider services - return false, nil - } - return true, nil -} - func nextContainerNumber(containers []container.Summary) int { maxNumber := 0 for _, ctr := range containers { @@ -612,61 +370,6 @@ func (s *composeService) getLinks(ctx context.Context, projectName string, servi return links, nil } -func (s *composeService) isServiceHealthy(ctx context.Context, containers Containers, fallbackRunning bool) (bool, error) { - for _, ctr := range containers { - res, err := s.apiClient().ContainerInspect(ctx, ctr.ID, client.ContainerInspectOptions{}) - if err != nil { - return false, err - } - ctr := res.Container - name := ctr.Name[1:] - - if ctr.State.Status == container.StateExited { - return false, fmt.Errorf("container %s exited (%d)", name, ctr.State.ExitCode) - } - - noHealthcheck := ctr.Config.Healthcheck == nil || (len(ctr.Config.Healthcheck.Test) > 0 && ctr.Config.Healthcheck.Test[0] == "NONE") - if noHealthcheck && fallbackRunning { - // Container does not define a health check, but we can fall back to "running" state - return ctr.State != nil && ctr.State.Status == container.StateRunning, nil - } - - if ctr.State == nil || ctr.State.Health == nil { - return false, fmt.Errorf("container %s has no healthcheck configured", name) - } - switch ctr.State.Health.Status { - case container.Healthy: - // Continue by checking the next container. - case container.Unhealthy: - return false, fmt.Errorf("container %s is unhealthy", name) - case container.Starting: - return false, nil - default: - return false, fmt.Errorf("container %s had unexpected health status %q", name, ctr.State.Health.Status) - } - } - return true, nil -} - -func (s *composeService) isServiceCompleted(ctx context.Context, containers Containers) (bool, int, error) { - if s.dryRun { - // dry-run never actually starts the dependency's container, so it - // can never observe a real "exited" state: simulate immediate - // success instead of polling forever. - return true, 0, nil - } - for _, ctr := range containers { - res, err := s.apiClient().ContainerInspect(ctx, ctr.ID, client.ContainerInspectOptions{}) - if err != nil { - return false, 0, err - } - if res.Container.State != nil && res.Container.State.Status == container.StateExited { - return true, res.Container.State.ExitCode, nil - } - } - return false, 0, nil -} - func mergeLabels(ls ...types.Labels) types.Labels { merged := types.Labels{} for _, l := range ls { diff --git a/pkg/compose/service_containers_test.go b/pkg/compose/service_containers_test.go index bf8641398a..0d2ffb563c 100644 --- a/pkg/compose/service_containers_test.go +++ b/pkg/compose/service_containers_test.go @@ -21,10 +21,7 @@ import ( "errors" "fmt" "net/netip" - "strings" "testing" - "testing/synctest" - "time" "github.com/compose-spec/compose-go/v2/types" "github.com/docker/cli/cli/config/configfile" @@ -32,7 +29,6 @@ import ( "github.com/moby/moby/api/types/container" "github.com/moby/moby/api/types/network" "github.com/moby/moby/client" - logrustest "github.com/sirupsen/logrus/hooks/test" "go.uber.org/mock/gomock" "gotest.tools/v3/assert" @@ -223,288 +219,6 @@ func TestServiceLinks(t *testing.T) { }) } -func TestWaitDependencies(t *testing.T) { - mockCtrl := gomock.NewController(t) - defer mockCtrl.Finish() - - apiClient := mocks.NewMockAPIClient(mockCtrl) - cli := mocks.NewMockCli(mockCtrl) - tested, err := NewComposeService(cli) - assert.NilError(t, err) - cli.EXPECT().Client().Return(apiClient).AnyTimes() - - t.Run("should skip dependencies with scale 0", func(t *testing.T) { - dbService := types.ServiceConfig{Name: "db", Scale: intPtr(0)} - redisService := types.ServiceConfig{Name: "redis", Scale: intPtr(0)} - project := types.Project{Name: strings.ToLower(testProject), Services: types.Services{ - "db": dbService, - "redis": redisService, - }} - dependencies := types.DependsOnConfig{ - "db": {Condition: ServiceConditionRunningOrHealthy}, - "redis": {Condition: ServiceConditionRunningOrHealthy}, - } - assert.NilError(t, tested.(*composeService).waitDependencies(t.Context(), &project, "", dependencies, nil)) - }) - t.Run("should skip zero-replica dependencies after service hashing", func(t *testing.T) { - replicas := 0 - project := types.Project{Name: strings.ToLower(testProject), Services: types.Services{ - "app": { - Name: "app", WorkloadSpec: types.WorkloadSpec{DependsOn: types.DependsOnConfig{ - "disabled": { - Condition: ServiceConditionRunningOrHealthy, - Required: true, - }, - }}, - }, - "disabled": { - Name: "disabled", - Deploy: &types.DeployConfig{Replicas: &replicas}, - }, - }} - - _, err := ServiceHash(project.Services["disabled"]) - assert.NilError(t, err) - - assert.NilError(t, tested.(*composeService).waitDependencies( - t.Context(), &project, "app", project.Services["app"].DependsOn, nil, - )) - }) - t.Run("should skip dependencies with condition service_started", func(t *testing.T) { - dbService := types.ServiceConfig{Name: "db", Scale: intPtr(1)} - redisService := types.ServiceConfig{Name: "redis", Scale: intPtr(1)} - project := types.Project{Name: strings.ToLower(testProject), Services: types.Services{ - "db": dbService, - "redis": redisService, - }} - dependencies := types.DependsOnConfig{ - "db": {Condition: types.ServiceConditionStarted, Required: true}, - "redis": {Condition: types.ServiceConditionStarted, Required: true}, - } - assert.NilError(t, tested.(*composeService).waitDependencies(t.Context(), &project, "", dependencies, nil)) - }) - t.Run("missing required dependency is an error", func(t *testing.T) { - project := types.Project{Name: strings.ToLower(testProject), Services: types.Services{ - "db": {Name: "db", Scale: intPtr(1)}, - }} - dependencies := types.DependsOnConfig{ - "db": {Condition: ServiceConditionRunningOrHealthy, Required: true}, - } - err := tested.(*composeService).waitDependencies(t.Context(), &project, "app", dependencies, nil) - assert.Error(t, err, "app is missing dependency db") - }) - t.Run("a dependency's pre_start hook runner does not count as the dependency being present", func(t *testing.T) { - project := types.Project{Name: strings.ToLower(testProject), Services: types.Services{ - "db": {Name: "db", Scale: intPtr(1), PreStart: []types.PreStartHook{{}}}, - }} - dependencies := types.DependsOnConfig{ - "db": {Condition: ServiceConditionRunningOrHealthy, Required: true}, - } - // Only a retained hook runner exists (e.g. its own success removal is - // still in flight, or it failed and was kept for inspection) — no - // real "db" replica container. - containers := Containers{{ - ID: "db-hook-runner", - Names: []string{"/db-hook-runner"}, - Labels: map[string]string{api.ServiceLabel: "db", api.HookLabel: "pre_start"}, - }} - err := tested.(*composeService).waitDependencies(t.Context(), &project, "app", dependencies, containers) - assert.Error(t, err, "app is missing dependency db") - }) - t.Run("missing optional dependency is only a warning", func(t *testing.T) { - project := types.Project{Name: strings.ToLower(testProject), Services: types.Services{ - "db": {Name: "db", Scale: intPtr(1)}, - }} - dependencies := types.DependsOnConfig{ - "db": {Condition: ServiceConditionRunningOrHealthy, Required: false}, - } - assert.NilError(t, tested.(*composeService).waitDependencies(t.Context(), &project, "app", dependencies, nil)) - }) - t.Run("failing optional dependency is skipped, not an error", func(t *testing.T) { - project := types.Project{Name: strings.ToLower(testProject), Services: types.Services{ - "db": {Name: "db", Scale: intPtr(1)}, - }} - dependencies := types.DependsOnConfig{ - "db": {Condition: types.ServiceConditionHealthy, Required: false}, - } - containers := Containers{{ - ID: "db-ctr", - Names: []string{"/db-ctr"}, - Labels: map[string]string{api.ServiceLabel: "db"}, - }} - // The dependency exited: a required dependency would fail the wait, - // an optional one is skipped after the first poll. - apiClient.EXPECT().ContainerInspect(gomock.Any(), "db-ctr", gomock.Any()).Return(client.ContainerInspectResult{ - Container: container.InspectResponse{ - ID: "db-ctr", - Name: "/db-ctr", - State: &container.State{Status: container.StateExited, ExitCode: 1}, - }, - }, nil) - assert.NilError(t, tested.(*composeService).waitDependencies(t.Context(), &project, "app", dependencies, containers)) - }) - t.Run("dry-run completes service_completed_successfully immediately", func(t *testing.T) { - tested.(*composeService).dryRun = true - - project := types.Project{Name: strings.ToLower(testProject), Services: types.Services{ - "init": {Name: "init", Scale: intPtr(1)}, - }} - dependencies := types.DependsOnConfig{ - "init": {Condition: types.ServiceConditionCompletedSuccessfully, Required: true}, - } - containers := Containers{{ - ID: "init-ctr", - Names: []string{"/init-ctr"}, - Labels: map[string]string{api.ServiceLabel: "init"}, - }} - // no ContainerInspect expectation: dry-run must not inspect a - // container that was never created. - assert.NilError(t, tested.(*composeService).waitDependencies(t.Context(), &project, "app", dependencies, containers)) - }) -} - -func TestIsServiceHealthy(t *testing.T) { - mockCtrl := gomock.NewController(t) - defer mockCtrl.Finish() - - apiClient := mocks.NewMockAPIClient(mockCtrl) - cli := mocks.NewMockCli(mockCtrl) - tested, err := NewComposeService(cli) - assert.NilError(t, err) - cli.EXPECT().Client().Return(apiClient).AnyTimes() - - ctx := t.Context() - - t.Run("disabled healthcheck with fallback to running", func(t *testing.T) { - containerID := "test-container-id" - containers := Containers{ - {ID: containerID}, - } - - // Container with disabled healthcheck (Test: ["NONE"]) - apiClient.EXPECT().ContainerInspect(ctx, containerID, gomock.Any()).Return(client.ContainerInspectResult{ - Container: container.InspectResponse{ - ID: containerID, - Name: "test-container", - State: &container.State{Status: "running"}, - Config: &container.Config{ - Healthcheck: &container.HealthConfig{ - Test: []string{"NONE"}, - }, - }, - }, - }, nil) - - isHealthy, err := tested.(*composeService).isServiceHealthy(ctx, containers, true) - assert.NilError(t, err) - assert.Equal(t, true, isHealthy, "Container with disabled healthcheck should be considered healthy when running with fallbackRunning=true") - }) - - t.Run("disabled healthcheck without fallback", func(t *testing.T) { - containerID := "test-container-id" - containers := Containers{ - {ID: containerID}, - } - - // Container with disabled healthcheck (Test: ["NONE"]) but fallbackRunning=false - apiClient.EXPECT().ContainerInspect(ctx, containerID, gomock.Any()).Return(client.ContainerInspectResult{ - Container: container.InspectResponse{ - ID: containerID, - Name: "test-container", - State: &container.State{Status: "running"}, - Config: &container.Config{ - Healthcheck: &container.HealthConfig{ - Test: []string{"NONE"}, - }, - }, - }, - }, nil) - - _, err := tested.(*composeService).isServiceHealthy(ctx, containers, false) - assert.ErrorContains(t, err, "has no healthcheck configured") - }) - - t.Run("no healthcheck with fallback to running", func(t *testing.T) { - containerID := "test-container-id" - containers := Containers{ - {ID: containerID}, - } - - // Container with no healthcheck at all - apiClient.EXPECT().ContainerInspect(ctx, containerID, gomock.Any()).Return(client.ContainerInspectResult{ - Container: container.InspectResponse{ - ID: containerID, - Name: "test-container", - State: &container.State{Status: "running"}, - Config: &container.Config{ - Healthcheck: nil, - }, - }, - }, nil) - - isHealthy, err := tested.(*composeService).isServiceHealthy(ctx, containers, true) - assert.NilError(t, err) - assert.Equal(t, true, isHealthy, "Container with no healthcheck should be considered healthy when running with fallbackRunning=true") - }) - - t.Run("exited container with disabled healthcheck", func(t *testing.T) { - containerID := "test-container-id" - containers := Containers{ - {ID: containerID}, - } - - // Container with disabled healthcheck but exited - apiClient.EXPECT().ContainerInspect(ctx, containerID, gomock.Any()).Return(client.ContainerInspectResult{ - Container: container.InspectResponse{ - ID: containerID, - Name: "test-container", - State: &container.State{ - Status: "exited", - ExitCode: 1, - }, - Config: &container.Config{ - Healthcheck: &container.HealthConfig{ - Test: []string{"NONE"}, - }, - }, - }, - }, nil) - - _, err := tested.(*composeService).isServiceHealthy(ctx, containers, true) - assert.ErrorContains(t, err, "exited") - }) - - t.Run("healthy container with healthcheck", func(t *testing.T) { - containerID := "test-container-id" - containers := Containers{ - {ID: containerID}, - } - - // Container with actual healthcheck that is healthy - apiClient.EXPECT().ContainerInspect(ctx, containerID, gomock.Any()).Return(client.ContainerInspectResult{ - Container: container.InspectResponse{ - ID: containerID, - Name: "test-container", - State: &container.State{ - Status: "running", - Health: &container.Health{ - Status: container.Healthy, - }, - }, - Config: &container.Config{ - Healthcheck: &container.HealthConfig{ - Test: []string{"CMD", "curl", "-f", "http://localhost"}, - }, - }, - }, - }, nil) - - isHealthy, err := tested.(*composeService).isServiceHealthy(ctx, containers, false) - assert.NilError(t, err) - assert.Equal(t, true, isHealthy, "Container with healthy status should be healthy") - }) -} - func TestCreateMobyContainer(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() @@ -808,480 +522,3 @@ func TestRuntimeAPIVersionRetriesOnTransientError(t *testing.T) { assert.NilError(t, err) assert.Equal(t, version, "1.44") } - -// startingDBInspect is db-ctr running with its healthcheck still starting. -var startingDBInspect = client.ContainerInspectResult{ - Container: container.InspectResponse{ - ID: "db-ctr", - Name: "/db-ctr", - Config: &container.Config{Healthcheck: &container.HealthConfig{Test: []string{"CMD", "true"}}}, - State: &container.State{Status: container.StateRunning, Health: &container.Health{Status: container.Starting}}, - }, -} - -// waitWithOwnTimeout runs one depends_on wait the way the plan's -// OpWaitCondition nodes do under --wait-timeout: bounded by a timeout of its -// own (planExecutor.waitTimeout), the only way a wait gets one. A watchdog -// ends a wait that would ignore that timeout. -func waitWithOwnTimeout(t *testing.T, svc *composeService, project *types.Project, containers Containers, condition string, required bool, timeout time.Duration) error { - t.Helper() - exec := svc.newPlanExecutor(project, emptyObservedState(project.Name)) - exec.containersByService["db"] = containers - exec.waitTimeout = timeout - ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second) - defer cancel() - return exec.execBoundedWaitCondition(ctx, Operation{ - Type: OpWaitCondition, - ResourceID: "wait:db:" + condition, - Name: "db", - Condition: condition, - BestEffort: !required, - }) -} - -// TestWaitDependencyDeadline locks the timeout semantics of the dependency -// wait: an expired deadline surfaces as "timeout waiting for dependencies", -// while a plain user cancellation is not a wait failure. -func TestWaitDependencyDeadline(t *testing.T) { - mockCtrl := gomock.NewController(t) - defer mockCtrl.Finish() - - apiClient := mocks.NewMockAPIClient(mockCtrl) - cli := mocks.NewMockCli(mockCtrl) - tested, err := NewComposeService(cli) - assert.NilError(t, err) - cli.EXPECT().Client().Return(apiClient).AnyTimes() - - project := types.Project{Name: strings.ToLower(testProject), Services: types.Services{ - "db": {Name: "db", Scale: intPtr(1)}, - }} - dependencies := types.DependsOnConfig{ - "db": {Condition: types.ServiceConditionHealthy, Required: true}, - } - containers := Containers{{ - ID: "db-ctr", - Names: []string{"/db-ctr"}, - Labels: map[string]string{api.ServiceLabel: "db"}, - }} - // stillStarting lets the timeout below expire between two checks. - stillStarting := func() { - apiClient.EXPECT().ContainerInspect(gomock.Any(), "db-ctr", gomock.Any()).Return(startingDBInspect, nil) - } - - t.Run("expired deadline is an error", func(t *testing.T) { - // The caller's deadline (--wait-timeout of up/start --wait) fires - // between two checks and must not be swallowed. - stillStarting() - ctx, cancel := context.WithTimeout(t.Context(), 50*time.Millisecond) - defer cancel() - err := tested.(*composeService).waitDependencies(ctx, &project, "app", dependencies, containers) - assert.Error(t, err, "timeout waiting for dependencies") - }) - - t.Run("expired own timeout of a plan wait is an error", func(t *testing.T) { - // The same, for the timeout of the plan's own wait node. - stillStarting() - err := waitWithOwnTimeout(t, tested.(*composeService), &project, containers, types.ServiceConditionHealthy, true, 50*time.Millisecond) - assert.Assert(t, errors.Is(err, errWaitTimeout), "got %v", err) - assert.ErrorIs(t, err, context.DeadlineExceeded) - }) - - // Regression guard: an optional dependency's deadline expiring (the - // timeout of the plan's own wait node, planExecutor.waitTimeout -- see - // waitDependency's ctx.Done() branch) must be tolerated exactly like any - // other definitive failure of an optional dependency, not surfaced as - // "timeout waiting for dependencies". - t.Run("expired own timeout on an optional dependency is skipped, not an error", func(t *testing.T) { - stillStarting() - err := waitWithOwnTimeout(t, tested.(*composeService), &project, containers, types.ServiceConditionHealthy, false, 50*time.Millisecond) - assert.NilError(t, err) - }) - - // Regression guard: the tolerance above must apply only to a deadline - // THIS wait's own timeout is responsible for -- an unrelated, - // already-expired deadline the caller brought in (waitDependencies sets - // none of its own) is a different, real failure that must still - // propagate even for an optional dependency. - t.Run("an inherited parent deadline is not swallowed for an optional dependency", func(t *testing.T) { - optionalDeps := types.DependsOnConfig{ - "db": {Condition: types.ServiceConditionHealthy, Required: false}, - } - parentCtx, cancel := context.WithTimeout(t.Context(), time.Millisecond) - defer cancel() - time.Sleep(10 * time.Millisecond) // let the parent's own deadline actually expire first - err := tested.(*composeService).waitDependencies(parentCtx, &project, "app", optionalDeps, containers) - assert.Error(t, err, "timeout waiting for dependencies") - }) - - t.Run("user cancellation is not a wait failure", func(t *testing.T) { - ctx, cancel := context.WithCancel(t.Context()) - cancel() - err := tested.(*composeService).waitDependencies(ctx, &project, "app", dependencies, containers) - assert.NilError(t, err) - }) - - // Regression guard for the runtime fallback warning on waitDependency's - // default branch — see the comment on that branch for why it must stay. - t.Run("unsupported condition warns and returns without waiting", func(t *testing.T) { - hook := logrustest.NewGlobal() - unsupportedDeps := types.DependsOnConfig{ - "db": {Condition: "some_future_condition", Required: true}, - } - // the bound makes a regression that waits fail instead of hanging - ctx, cancel := context.WithTimeout(t.Context(), 2*time.Second) - defer cancel() - err := tested.(*composeService).waitDependencies(ctx, &project, "app", unsupportedDeps, containers) - assert.NilError(t, err) - - var messages []string - for _, e := range hook.AllEntries() { - messages = append(messages, e.Message) - } - joined := strings.Join(messages, "\n") - assert.Assert(t, strings.Contains(joined, `service "app": unsupported depends_on condition "some_future_condition"`), joined) - }) -} - -// TestWaitDependencyFirstCheck locks when a satisfied dependency is first -// checked: on entry, except running_or_healthy (see waitDependencyReached). -func TestWaitDependencyFirstCheck(t *testing.T) { - project := types.Project{Name: strings.ToLower(testProject), Services: types.Services{ - "db": {Name: "db", Scale: intPtr(1)}, - }} - containers := Containers{{ - ID: "db-ctr", - Names: []string{"/db-ctr"}, - Labels: map[string]string{api.ServiceLabel: "db"}, - }} - satisfied := map[string]struct { - inspect container.InspectResponse - wait time.Duration - }{ - types.ServiceConditionHealthy: { - inspect: container.InspectResponse{ - ID: "db-ctr", - Name: "/db-ctr", - Config: &container.Config{Healthcheck: &container.HealthConfig{Test: []string{"CMD", "true"}}}, - State: &container.State{Status: container.StateRunning, Health: &container.Health{Status: container.Healthy}}, - }, - wait: 0, - }, - types.ServiceConditionCompletedSuccessfully: { - inspect: container.InspectResponse{ - ID: "db-ctr", - Name: "/db-ctr", - State: &container.State{Status: container.StateExited, ExitCode: 0}, - }, - wait: 0, - }, - ServiceConditionRunningOrHealthy: { - inspect: container.InspectResponse{ - ID: "db-ctr", - Name: "/db-ctr", - Config: &container.Config{}, - State: &container.State{Status: container.StateRunning}, - }, - wait: 500 * time.Millisecond, - }, - } - - for condition, tc := range satisfied { - t.Run(condition, func(t *testing.T) { - synctest.Test(t, func(t *testing.T) { - mockCtrl := gomock.NewController(t) - apiClient := mocks.NewMockAPIClient(mockCtrl) - cli := mocks.NewMockCli(mockCtrl) - cli.EXPECT().Client().Return(apiClient).AnyTimes() - tested, err := NewComposeService(cli) - assert.NilError(t, err) - apiClient.EXPECT().ContainerInspect(gomock.Any(), "db-ctr", gomock.Any()). - Return(client.ContainerInspectResult{Container: tc.inspect}, nil) - - dependencies := types.DependsOnConfig{"db": {Condition: condition, Required: true}} - start := time.Now() - err = tested.(*composeService).waitDependencies(t.Context(), &project, "app", dependencies, containers) - assert.NilError(t, err) - assert.Equal(t, time.Since(start), tc.wait) - }) - }) - } -} - -// TestOptionalDependencyInspectFailure locks what an optional (required: -// false) dependency's failing inspection means. A genuine inspection error -// (the daemon doesn't know the container, the API call fails) says the -// dependency didn't start, so it is skipped. A failure caused by the wait's -// context being done says nothing about the dependency: it must not be -// turned into a skipped dependency, i.e. into a successful wait, and the end -// of the wait is left to waitDependency (a deadline is an error, a plain -// cancellation stays silent). A required dependency is never skipped. -func TestOptionalDependencyInspectFailure(t *testing.T) { - const skipped = "Skipped: " - conditions := []string{ServiceConditionRunningOrHealthy, types.ServiceConditionHealthy} - dbContainers := Containers{{ - ID: "db-ctr", - Names: []string{"/db-ctr"}, - Labels: map[string]string{api.ServiceLabel: "db"}, - }} - project := types.Project{Name: strings.ToLower(testProject), Services: types.Services{ - "db": {Name: "db", Scale: intPtr(1)}, - }} - - newTested := func(t *testing.T) (*composeService, *mocks.MockAPIClient, *capturingEvents) { - t.Helper() - mockCtrl := gomock.NewController(t) - apiClient := mocks.NewMockAPIClient(mockCtrl) - cli := mocks.NewMockCli(mockCtrl) - cli.EXPECT().Client().Return(apiClient).AnyTimes() - events := &capturingEvents{} - tested, err := NewComposeService(cli, WithEventProcessor(events)) - assert.NilError(t, err) - return tested.(*composeService), apiClient, events - } - wasSkipped := func(events *capturingEvents) bool { - for _, r := range events.resources { - if strings.HasPrefix(r.Text, skipped) { - return true - } - } - return false - } - dependenciesOn := func(condition string, required bool) types.DependsOnConfig { - return types.DependsOnConfig{"db": {Condition: condition, Required: required}} - } - // inspectFailsWithContext makes ContainerInspect fail the way the client - // does when the context it was given is done: it waits for it and - // returns its error, wrapped. It must be reached at least once (or the - // test would pass without exercising the inspection); once the context is - // done, a pending poll tick may win the select against ctx.Done() for one - // more check, so there is no upper bound. - inspectFailsWithContext := func(apiClient *mocks.MockAPIClient) { - apiClient.EXPECT().ContainerInspect(gomock.Any(), "db-ctr", gomock.Any()). - DoAndReturn(func(ctx context.Context, _ string, _ client.ContainerInspectOptions) (client.ContainerInspectResult, error) { - <-ctx.Done() - return client.ContainerInspectResult{}, fmt.Errorf("inspect db-ctr: %w", ctx.Err()) - }).MinTimes(1) - } - - for _, condition := range conditions { - t.Run(condition, func(t *testing.T) { - t.Run("a genuine inspection error skips an optional dependency", func(t *testing.T) { - t.Parallel() - tested, apiClient, events := newTested(t) - apiClient.EXPECT().ContainerInspect(gomock.Any(), "db-ctr", gomock.Any()). - Return(client.ContainerInspectResult{}, errors.New("Error response from daemon: No such container: db-ctr")) - err := tested.waitDependencies(t.Context(), &project, "app", dependenciesOn(condition, false), dbContainers) - assert.NilError(t, err) - assert.Assert(t, wasSkipped(events)) - }) - - t.Run("a genuine inspection error fails a required dependency", func(t *testing.T) { - t.Parallel() - tested, apiClient, events := newTested(t) - apiClient.EXPECT().ContainerInspect(gomock.Any(), "db-ctr", gomock.Any()). - Return(client.ContainerInspectResult{}, errors.New("Error response from daemon: No such container: db-ctr")) - err := tested.waitDependencies(t.Context(), &project, "app", dependenciesOn(condition, true), dbContainers) - assert.ErrorContains(t, err, "No such container: db-ctr") - assert.Assert(t, !wasSkipped(events)) - }) - - t.Run("a cancellation during the inspection is not a skipped optional dependency", func(t *testing.T) { - t.Parallel() - tested, apiClient, events := newTested(t) - ctx, cancel := context.WithCancel(t.Context()) - defer cancel() - apiClient.EXPECT().ContainerInspect(gomock.Any(), "db-ctr", gomock.Any()). - DoAndReturn(func(context.Context, string, client.ContainerInspectOptions) (client.ContainerInspectResult, error) { - cancel() - return client.ContainerInspectResult{}, fmt.Errorf("inspect db-ctr: %w", context.Canceled) - }).MinTimes(1) // a pending poll tick may win the select once more - // a plain cancellation is not a wait failure (see - // TestWaitDependencyDeadline), but it is no skip either - err := tested.waitDependencies(ctx, &project, "app", dependenciesOn(condition, false), dbContainers) - assert.NilError(t, err) - assert.Assert(t, !wasSkipped(events)) - }) - - t.Run("a cancellation during the inspection still fails a required dependency", func(t *testing.T) { - t.Parallel() - tested, apiClient, _ := newTested(t) - ctx, cancel := context.WithCancel(t.Context()) - defer cancel() - apiClient.EXPECT().ContainerInspect(gomock.Any(), "db-ctr", gomock.Any()). - DoAndReturn(func(context.Context, string, client.ContainerInspectOptions) (client.ContainerInspectResult, error) { - cancel() - return client.ContainerInspectResult{}, fmt.Errorf("inspect db-ctr: %w", context.Canceled) - }) - err := tested.waitDependencies(ctx, &project, "app", dependenciesOn(condition, true), dbContainers) - assert.Assert(t, errors.Is(err, context.Canceled), "got %v", err) - }) - - for _, required := range []bool{false, true} { - t.Run(fmt.Sprintf("a deadline inherited from the caller expiring during the inspection is an error (required=%t)", required), func(t *testing.T) { - t.Parallel() - tested, apiClient, events := newTested(t) - inspectFailsWithContext(apiClient) - // the first check starts the inspection, which only ends - // with the deadline - ctx, cancel := context.WithTimeout(t.Context(), time.Second) - defer cancel() - err := tested.waitDependencies(ctx, &project, "app", dependenciesOn(condition, required), dbContainers) - assert.Error(t, err, "timeout waiting for dependencies") - assert.Assert(t, !wasSkipped(events)) - }) - } - - // The wait's own timeout expiring during the inspection must end - // the wait exactly as it does when it expires between two polls: - // whatever waitDependency decides for an optional dependency, the - // check functions must not decide differently. - for _, required := range []bool{false, true} { - t.Run(fmt.Sprintf("the wait's own timeout expiring during the inspection ends like it does between polls (required=%t)", required), func(t *testing.T) { - t.Parallel() - tested, apiClient, _ := newTested(t) - inspectFailsWithContext(apiClient) - duringInspect := waitWithOwnTimeout(t, tested, &project, dbContainers, condition, required, time.Second) - - // no check in flight when the timeout expires: at most the one on entry - polled, polledClient, _ := newTested(t) - polledClient.EXPECT().ContainerInspect(gomock.Any(), "db-ctr", gomock.Any()).Return(startingDBInspect, nil).MaxTimes(1) - betweenPolls := waitWithOwnTimeout(t, polled, &project, dbContainers, condition, required, 50*time.Millisecond) - - // the same end, whatever the instant the timeout expired at - assert.Equal(t, duringInspect == nil, betweenPolls == nil) - assert.Equal(t, errors.Is(duringInspect, errWaitTimeout), errors.Is(betweenPolls, errWaitTimeout)) - assert.Equal(t, errors.Is(duringInspect, context.DeadlineExceeded), errors.Is(betweenPolls, context.DeadlineExceeded)) - if required { - assert.Assert(t, errors.Is(duringInspect, errWaitTimeout), "got %v", duringInspect) - assert.ErrorIs(t, duringInspect, context.DeadlineExceeded) - } else { - assert.NilError(t, duringInspect) - } - }) - } - }) - } -} - -// TestCheckDependencyDoneContext tests the check functions directly with a -// context that is already done, so no poll timing is involved: an optional -// dependency is "not done yet" (the wait's loop then sees ctx.Done()), never -// skipped; a required one keeps reporting the failure. -// A genuine failure of a required dependency is reported even when the context -// happens to be canceled by the time it is evaluated: only an error caused by -// the cancellation is silent. -func TestCheckDependencyHealthyReportsGenuineFailureOnCanceledContext(t *testing.T) { - containers := Containers{{ - ID: "db-ctr", - Names: []string{"/db-ctr"}, - Labels: map[string]string{api.ServiceLabel: "db"}, - }} - mockCtrl := gomock.NewController(t) - apiClient := mocks.NewMockAPIClient(mockCtrl) - cli := mocks.NewMockCli(mockCtrl) - cli.EXPECT().Client().Return(apiClient).AnyTimes() - events := &capturingEvents{} - svc, err := NewComposeService(cli, WithEventProcessor(events)) - assert.NilError(t, err) - - ctx, cancel := context.WithCancel(t.Context()) - defer cancel() - apiClient.EXPECT().ContainerInspect(gomock.Any(), "db-ctr", gomock.Any()). - DoAndReturn(func(context.Context, string, client.ContainerInspectOptions) (client.ContainerInspectResult, error) { - // the phase is canceled right after the inspection answered - cancel() - return client.ContainerInspectResult{ - Container: container.InspectResponse{ - ID: "db-ctr", - Name: "/db-ctr", - State: &container.State{ - Status: container.StateRunning, - Health: &container.Health{Status: container.Unhealthy}, - }, - Config: &container.Config{ - Healthcheck: &container.HealthConfig{Test: []string{"CMD", "true"}}, - }, - }, - }, nil - }) - - done, err := svc.(*composeService).checkDependencyHealthy(ctx, "db", types.ServiceDependency{Required: true}, containers) - assert.ErrorContains(t, err, "is unhealthy") - assert.Assert(t, !done) - assert.Equal(t, len(events.resources), 1, "got %v", events.resources) - assert.Equal(t, events.resources[0].Status, api.Error) -} - -func TestCheckDependencyDoneContext(t *testing.T) { - containers := Containers{{ - ID: "db-ctr", - Names: []string{"/db-ctr"}, - Labels: map[string]string{api.ServiceLabel: "db"}, - }} - expired := func(t *testing.T) context.Context { - ctx, cancel := context.WithDeadline(t.Context(), time.Now().Add(-time.Second)) - t.Cleanup(cancel) - return ctx - } - canceled := func(t *testing.T) context.Context { - ctx, cancel := context.WithCancel(t.Context()) - cancel() - return ctx - } - contexts := map[string]func(*testing.T) context.Context{"canceled": canceled, "expired": expired} - checks := map[string]func(*composeService, context.Context, types.ServiceDependency) (bool, error){ - "running_or_healthy": func(s *composeService, ctx context.Context, c types.ServiceDependency) (bool, error) { - return s.checkDependencyRunningOrHealthy(ctx, "db", c, containers) - }, - "service_healthy": func(s *composeService, ctx context.Context, c types.ServiceDependency) (bool, error) { - return s.checkDependencyHealthy(ctx, "db", c, containers) - }, - } - - for checkName, check := range checks { - for ctxName, newCtx := range contexts { - t.Run(checkName+"/"+ctxName, func(t *testing.T) { - t.Parallel() - mockCtrl := gomock.NewController(t) - apiClient := mocks.NewMockAPIClient(mockCtrl) - cli := mocks.NewMockCli(mockCtrl) - cli.EXPECT().Client().Return(apiClient).AnyTimes() - events := &capturingEvents{} - svc, err := NewComposeService(cli, WithEventProcessor(events)) - assert.NilError(t, err) - tested := svc.(*composeService) - - ctx := newCtx(t) - apiClient.EXPECT().ContainerInspect(gomock.Any(), "db-ctr", gomock.Any()). - Return(client.ContainerInspectResult{}, fmt.Errorf("inspect db-ctr: %w", ctx.Err())).Times(2) - - done, err := check(tested, ctx, types.ServiceDependency{Required: false}) - assert.NilError(t, err) - assert.Assert(t, !done, "an optional dependency must not complete on a done context") - assert.Equal(t, len(events.resources), 0, "no skipped event expected, got %v", events.resources) - - _, err = check(tested, ctx, types.ServiceDependency{Required: true}) - assert.Assert(t, errors.Is(err, ctx.Err()), "got %v", err) - }) - } - } - - // service_completed_successfully never skipped an optional dependency on - // an inspection error (only on a non-zero exit code): unchanged. - t.Run("service_completed_successfully", func(t *testing.T) { - t.Parallel() - mockCtrl := gomock.NewController(t) - apiClient := mocks.NewMockAPIClient(mockCtrl) - cli := mocks.NewMockCli(mockCtrl) - cli.EXPECT().Client().Return(apiClient).AnyTimes() - svc, err := NewComposeService(cli) - assert.NilError(t, err) - - ctx := canceled(t) - apiClient.EXPECT().ContainerInspect(gomock.Any(), "db-ctr", gomock.Any()). - Return(client.ContainerInspectResult{}, ctx.Err()) - done, err := svc.(*composeService).checkDependencyCompleted(ctx, "db", types.ServiceDependency{Required: false}, containers) - assert.Assert(t, errors.Is(err, context.Canceled), "got %v", err) - assert.Assert(t, !done) - }) -} From 50aabdd58931bf24bcddb278bba62d799b732e0d Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Tue, 27 Oct 2026 11:35:18 +0100 Subject: [PATCH 2/3] docs(compose): describe dependency_wait.go and fix the header of service_containers.go The header of service_containers.go still listed the dependency waits among what it covers. Comments only. Part of #14081 Signed-off-by: Nicolas De Loof --- pkg/compose/dependency_wait.go | 6 ++++++ pkg/compose/service_containers.go | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/pkg/compose/dependency_wait.go b/pkg/compose/dependency_wait.go index 7d168ae543..8506ab3a10 100644 --- a/pkg/compose/dependency_wait.go +++ b/pkg/compose/dependency_wait.go @@ -31,6 +31,12 @@ import ( "github.com/docker/compose/v5/pkg/api" ) +// This file gathers the waits on depends_on conditions: the polling of a +// dependency's containers (waitDependency and the per-condition checks), shared +// by the plan's OpWaitCondition nodes and by the commands that wait outside a +// plan (waitDependencies: restart, run and the --wait verification of up and +// start), and the gate deciding which dependencies need a wait at all. + // ServiceConditionRunningOrHealthy is a service condition on status running or healthy const ServiceConditionRunningOrHealthy = "running_or_healthy" diff --git a/pkg/compose/service_containers.go b/pkg/compose/service_containers.go index 60fb896a55..b9aea13a3b 100644 --- a/pkg/compose/service_containers.go +++ b/pkg/compose/service_containers.go @@ -40,8 +40,8 @@ import ( // create/up/start) and by the commands that still walk the dependency order // themselves (dependencies.go: restart, stop, down and the classic build). It // covers container naming, resolution of service references (volumes_from, -// network_mode/ipc/pid, links), dependency waiting and container creation -// through the Docker API. +// network_mode/ipc/pid, links) and container creation through the Docker API; +// the waits on depends_on conditions are in dependency_wait.go. const ( doubledContainerNameWarning = "WARNING: The %q service is using the custom container name %q. " + From 11a3ff08ef0b31fad5d12fdf47e7e75ea55648b6 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Tue, 27 Oct 2026 11:38:33 +0100 Subject: [PATCH 3/3] docs(projectloader): point the comments to the new home of the dependency wait Two comments still located waitDependency's runtime fallback in service_containers.go. Comments only. Part of #14081 Signed-off-by: Nicolas De Loof --- internal/projectloader/unsupported_attributes.go | 2 +- internal/projectloader/unsupported_attributes_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/projectloader/unsupported_attributes.go b/internal/projectloader/unsupported_attributes.go index 5ce98d4dfe..00174c6a6d 100644 --- a/internal/projectloader/unsupported_attributes.go +++ b/internal/projectloader/unsupported_attributes.go @@ -138,7 +138,7 @@ func isUnderAnyOf(path tree.Path, removed map[tree.Path]string) bool { // load — already rejects anything else. There is no unsupported value left // for a real compose file to reach this check with (see // TestDependsOnUnknownConditionIsRejectedBySchema). waitDependency's own -// runtime fallback (service_containers.go) still needs to warn about an +// runtime fallback (dependency_wait.go) still needs to warn about an // unrecognized condition, for the two situations that never go through // schema validation at all: a project rebuilt from container labels, or an // older/newer Compose version's output. diff --git a/internal/projectloader/unsupported_attributes_test.go b/internal/projectloader/unsupported_attributes_test.go index 81d3cb0d86..91889f4cf1 100644 --- a/internal/projectloader/unsupported_attributes_test.go +++ b/internal/projectloader/unsupported_attributes_test.go @@ -656,7 +656,7 @@ func TestPresenceUnsupportedAttributes_AreKnownSchemaPaths(t *testing.T) { // the compose-spec schema declares it as a closed enum of the 3 values this // runtime understands, so an unrecognized value never reaches // valueConditionalAttributes — it fails to load at all. waitDependency's -// own runtime fallback in service_containers.go covers the cases that +// own runtime fallback in dependency_wait.go covers the cases that // bypass schema validation entirely (a project rebuilt from container // labels, or one written by a different Compose version). func TestDependsOnUnknownConditionIsRejectedBySchema(t *testing.T) {