diff --git a/pkg/compose/dependency_wait_test.go b/pkg/compose/dependency_wait_test.go index 0e22ebfe62..c0256acfd3 100644 --- a/pkg/compose/dependency_wait_test.go +++ b/pkg/compose/dependency_wait_test.go @@ -794,3 +794,235 @@ func TestCheckDependencyDoneContext(t *testing.T) { assert.Assert(t, !done) }) } + +// TestShouldWaitForDependency pins which depends_on entries are worth a wait, +// the gate the plan's wait nodes and the waits outside a plan (restart, run, +// --wait) share: only a condition other than service_started, on a declared +// service that really runs containers of its own. +func TestShouldWaitForDependency(t *testing.T) { + project := &types.Project{ + Name: "myproject", + Services: types.Services{ + "db": {Name: "db"}, + "idle": {Name: "idle", Scale: intPtr(0)}, + "provider": {Name: "provider", Provider: &types.ServiceProviderConfig{Type: "test"}}, + }, + DisabledServices: types.Services{"cache": {Name: "cache"}}, + } + healthy := types.ServiceDependency{Condition: types.ServiceConditionHealthy, Required: true} + + tests := []struct { + name string + dependency string + config types.ServiceDependency + wait bool + err string + }{ + {name: "a started condition is the dependency order itself", dependency: "db", config: types.ServiceDependency{Condition: types.ServiceConditionStarted, Required: true}}, + {name: "a healthy dependency is waited for", dependency: "db", config: healthy, wait: true}, + {name: "a service completing is waited for", dependency: "db", config: types.ServiceDependency{Condition: types.ServiceConditionCompletedSuccessfully, Required: true}, wait: true}, + {name: "a dependency with no container (scale 0) is not", dependency: "idle", config: healthy}, + {name: "a provider service is not", dependency: "provider", config: healthy}, + {name: "a disabled service (--no-deps) is not", dependency: "cache", config: healthy}, + {name: "an undeclared service is an error", dependency: "ghost", config: healthy, err: "ghost"}, + {name: "an undeclared service with a started condition is not even looked up", dependency: "ghost", config: types.ServiceDependency{Condition: types.ServiceConditionStarted}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + wait, err := shouldWaitForDependency(tt.dependency, tt.config, project) + if tt.err != "" { + assert.ErrorContains(t, err, tt.err) + return + } + assert.NilError(t, err) + assert.Equal(t, wait, tt.wait) + }) + } +} + +func inspectOf(status container.ContainerState, health *container.Health, healthcheck *container.HealthConfig, exitCode int) client.ContainerInspectResult { + return client.ContainerInspectResult{Container: container.InspectResponse{ + ID: "db-id", + Name: "/test-db-1", + Config: &container.Config{Healthcheck: healthcheck}, + State: &container.State{Status: status, Health: health, ExitCode: exitCode}, + }} +} + +// TestIsServiceHealthyStatuses pins how each state of a container with a +// healthcheck reads for service_healthy: healthy is the only way to satisfy +// the condition, starting means keep polling, and anything else -- exited, +// unhealthy, no health state at all, a status the daemon should not report -- +// is a definitive failure naming the container. +func TestIsServiceHealthyStatuses(t *testing.T) { + check := &container.HealthConfig{Test: []string{"CMD", "true"}} + tests := []struct { + name string + inspect client.ContainerInspectResult + healthy bool + err string + }{ + {name: "healthy", inspect: inspectOf(container.StateRunning, &container.Health{Status: container.Healthy}, check, 0), healthy: true}, + {name: "still starting", inspect: inspectOf(container.StateRunning, &container.Health{Status: container.Starting}, check, 0)}, + {name: "unhealthy", inspect: inspectOf(container.StateRunning, &container.Health{Status: container.Unhealthy}, check, 0), err: "container test-db-1 is unhealthy"}, + {name: "exited", inspect: inspectOf(container.StateExited, nil, check, 3), err: "container test-db-1 exited (3)"}, + {name: "no health state", inspect: inspectOf(container.StateRunning, nil, check, 0), err: "container test-db-1 has no healthcheck configured"}, + {name: "unexpected status", inspect: inspectOf(container.StateRunning, &container.Health{Status: "weird"}, check, 0), err: `container test-db-1 had unexpected health status "weird"`}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + svc, apiClient, _ := newStartPhaseTestService(t) + apiClient.EXPECT().ContainerInspect(gomock.Any(), "db-id", gomock.Any()).Return(tt.inspect, nil) + + healthy, err := svc.isServiceHealthy(t.Context(), Containers{{ID: "db-id"}}, false) + + if tt.err != "" { + assert.ErrorContains(t, err, tt.err) + } else { + assert.NilError(t, err) + } + assert.Equal(t, healthy, tt.healthy) + }) + } + + t.Run("every container of the service has to be healthy", func(t *testing.T) { + svc, apiClient, _ := newStartPhaseTestService(t) + apiClient.EXPECT().ContainerInspect(gomock.Any(), "db-1", gomock.Any()). + Return(inspectOf(container.StateRunning, &container.Health{Status: container.Healthy}, check, 0), nil) + apiClient.EXPECT().ContainerInspect(gomock.Any(), "db-2", gomock.Any()). + Return(inspectOf(container.StateRunning, &container.Health{Status: container.Starting}, check, 0), nil) + + healthy, err := svc.isServiceHealthy(t.Context(), Containers{{ID: "db-1"}, {ID: "db-2"}}, false) + assert.NilError(t, err) + assert.Assert(t, !healthy) + }) +} + +// TestCheckDependencyCompleted pins service_completed_successfully: a +// dependency that exited with 0 satisfies it, one still running does not yet, +// one that exited with another code fails a required dependency -- naming the +// service and the code, and marking its rows in error -- and is skipped, with a warning, +// when the dependency is optional. +func TestCheckDependencyCompleted(t *testing.T) { + waitingFor := Containers{{ID: "db-id", Names: []string{"/test-db-1"}, Labels: map[string]string{api.ServiceLabel: "db"}}} + const row = "Container test-db-1" + + tests := []struct { + name string + inspect client.ContainerInspectResult + required bool + done bool + err string + events []string + }{ + { + name: "exited with 0", inspect: inspectOf(container.StateExited, nil, nil, 0), required: true, + done: true, events: []string{api.StatusExited}, + }, + { + name: "still running", inspect: inspectOf(container.StateRunning, nil, nil, 0), required: true, + }, + { + name: "required, exited with an error code", inspect: inspectOf(container.StateExited, nil, nil, 3), required: true, + err: `service "db" didn't complete successfully: exit 3`, + events: []string{api.StatusError}, + }, + { + name: "optional, exited with an error code", inspect: inspectOf(container.StateExited, nil, nil, 3), + done: true, + events: []string{`Skipped: optional dependency "db" didn't complete successfully: exit 3`}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + svc, apiClient, recorder := newStartPhaseTestService(t) + apiClient.EXPECT().ContainerInspect(gomock.Any(), "db-id", gomock.Any()).Return(tt.inspect, nil) + + done, err := svc.checkDependencyCompleted(t.Context(), "db", types.ServiceDependency{Required: tt.required}, waitingFor) + + if tt.err != "" { + assert.ErrorContains(t, err, tt.err) + } else { + assert.NilError(t, err) + } + assert.Equal(t, done, tt.done) + assert.DeepEqual(t, recorder.byID[row], tt.events) + }) + } + + t.Run("a dry run never sees a dependency exit: it is taken as completed", func(t *testing.T) { + svc, _, _ := newStartPhaseTestService(t) + svc.dryRun = true + + done, err := svc.checkDependencyCompleted(t.Context(), "db", types.ServiceDependency{Required: true}, waitingFor) + assert.NilError(t, err) + assert.Assert(t, done) + }) +} + +// TestWaitDependenciesReportsTheWait pins what a wait outside a plan (restart, +// run, the --wait verification) shows of a dependency's containers: Waiting +// first, then Healthy once the condition holds -- the progression a plan's wait +// node gives the same rows -- and only the dependency's own containers, never +// a one-off or a hook runner of the service. +func TestWaitDependenciesReportsTheWait(t *testing.T) { + svc, apiClient, recorder := newStartPhaseTestService(t) + project := &types.Project{Name: "test", Services: types.Services{"db": {Name: "db"}}} + labels := func(oneOff string) map[string]string { + return map[string]string{api.ServiceLabel: "db", api.OneoffLabel: oneOff} + } + containers := Containers{ + {ID: "db-id", Names: []string{"/test-db-1"}, Labels: labels("False")}, + {ID: "run-id", Names: []string{"/test-db-run-1"}, Labels: labels("True")}, + } + apiClient.EXPECT().ContainerInspect(gomock.Any(), "db-id", gomock.Any()). + Return(inspectOf(container.StateRunning, &container.Health{Status: container.Healthy}, &container.HealthConfig{Test: []string{"CMD", "true"}}, 0), nil) + + dependencies := types.DependsOnConfig{"db": {Condition: types.ServiceConditionHealthy, Required: true}} + assert.NilError(t, svc.waitDependencies(t.Context(), project, "web", dependencies, containers)) + + assert.DeepEqual(t, recorder.byID["Container test-db-1"], []string{api.StatusWaiting, api.StatusHealthy}) + assert.Equal(t, len(recorder.byID["Container test-db-run-1"]), 0, "a one-off container is not waited for") +} + +// TestWaitDependencyEndOfContextWinsOverATick pins that a wait whose context +// ended is never run through one more check because a tick happened to be +// ready as well: select picks at random between the two, so the wait looks at +// its context after it, and the end of the context wins. Here a check is in +// flight while the context ends and a tick is pending; the scenario is run +// repeatedly, as a wait that let the tick win would only miss it one time in +// two. The one check allowed is the first, on entry. +func TestWaitDependencyEndOfContextWinsOverATick(t *testing.T) { + waitingFor := Containers{{ID: "db-id", Names: []string{"/test-db-1"}, Labels: map[string]string{api.ServiceLabel: "db"}}} + dependency := types.ServiceDependency{Condition: types.ServiceConditionHealthy, Required: true} + + for range 40 { + synctest.Test(t, func(t *testing.T) { + svc, apiClient, _ := newStartPhaseTestService(t) + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + entered, release := make(chan struct{}), make(chan struct{}) + apiClient.EXPECT().ContainerInspect(gomock.Any(), "db-id", gomock.Any()).Times(1). + DoAndReturn(func(context.Context, string, client.ContainerInspectOptions) (client.ContainerInspectResult, error) { + close(entered) + <-release + return inspectOf(container.StateRunning, &container.Health{Status: container.Starting}, &container.HealthConfig{Test: []string{"CMD", "true"}}, 0), nil + }) + result := make(chan error, 1) + go func() { + reached, err := svc.waitDependencyReached(ctx, ctx, "web", "db", dependency, waitingFor) + if err == nil && reached { + err = errors.New("the wait reported the dependency as reached") + } + result <- err + }() + + <-entered + time.Sleep(time.Second) // the ticker fires while the check is in flight + cancel() + close(release) + + assert.NilError(t, <-result) + }) + } +} diff --git a/pkg/compose/executor_start_test.go b/pkg/compose/executor_start_test.go index 81ca2b3c91..0d905f110d 100644 --- a/pkg/compose/executor_start_test.go +++ b/pkg/compose/executor_start_test.go @@ -2128,3 +2128,474 @@ func TestPlanRunOfAnEmptyOrSinglePhasePlan(t *testing.T) { assert.NilError(t, pr.runStart(t.Context(), nil)) }) } + +// TestExecutePlanStartHoldsTheStartMutex pins that every ContainerStart the +// plan issues goes through a call site holding startMx, the bare start of a +// paused or dead container as much as the start of a service container, which +// also injects its secrets and configs. startMx is a workaround for a flaw of +// engine 28.0 to 28.2 (see its doc comment), kept until Engine 28.x is out of +// support: the mutex is probed from inside ContainerStart, where it must be +// held, so that dropping it from either path is a decision, not an accident. +// The test is not parallel, so no other test holds startMx while it runs. +func TestExecutePlanStartHoldsTheStartMutex(t *testing.T) { + tests := []struct { + name string + service *types.ServiceConfig + }{ + {name: "a bare start of a paused or dead container"}, + {name: "the start of a service container", service: &types.ServiceConfig{Name: "web"}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + svc, apiClient, _ := newStartPhaseTestService(t) + ctr := container.Summary{ID: "web-id", Names: []string{"/test-web-1"}} + apiClient.EXPECT().ContainerStart(gomock.Any(), "web-id", gomock.Any()). + DoAndReturn(func(context.Context, string, client.ContainerStartOptions) (client.ContainerStartResult, error) { + if startMx.TryLock() { + startMx.Unlock() + t.Error("ContainerStart was called without holding startMx") + } + return client.ContainerStartResult{}, nil + }) + + plan := &Plan{} + plan.addNode(Operation{ + Type: OpStartContainer, + ResourceID: "service:web:1", + Cause: "start", + Service: tt.service, + Container: &ctr, + }, "") + + project := &types.Project{Name: "test", Services: types.Services{"web": {Name: "web"}}} + exec := svc.newPlanExecutor(project, emptyObservedState("test")) + assert.NilError(t, exec.run(t.Context(), plan)) + }) + } +} + +// TestExecutePlanFailureRelaysDownALongChainUntouched pins that the error a +// plan fails with is the one of the node that failed, however long the chain +// of nodes held behind it: here the wait on a dependency that has no +// container, with the pre_start hooks, the start and the post_start hooks of +// the service that depends on it after it -- the shape of a replica's chain in +// the start phase. None of them runs, and the error is the wait's own, with +// nothing in front of it. How each hop of the chain relays the failure is +// TestRunNodeRelaysTheOriginalErrorDownALongChain's. +func TestExecutePlanFailureRelaysDownALongChainUntouched(t *testing.T) { + svc, _, _ := newStartPhaseTestService(t) + + service := types.ServiceConfig{ + Name: "web", + PreStart: []types.PreStartHook{{ContainerSpec: types.ContainerSpec{Command: types.ShellCommand{"init"}}}}, + PostStart: []types.ServiceHook{{Command: types.ShellCommand{"notify"}}}, + } + project := &types.Project{Name: "test", Services: types.Services{"web": service}} + ctr := container.Summary{ID: "web-id", Names: []string{"/test-web-1"}} + + // no expectation on the daemon at all: nothing but the wait may run + plan := &Plan{} + wait := plan.addNode(Operation{ + Type: OpWaitCondition, + ResourceID: "wait:db:service_healthy", + Name: "db", + Condition: types.ServiceConditionHealthy, + }, "") + wait.Phase = PhaseStart + preStart := plan.addNode(Operation{ + Type: OpRunPreStart, + ResourceID: "service:web:1", + Cause: "pre_start hooks", + Service: &service, + Container: &ctr, + }, "start:web:1", wait) + preStart.Phase = PhaseStart + start := plan.addNode(Operation{ + Type: OpStartContainer, + ResourceID: "service:web:1", + Cause: "start", + Service: &service, + Container: &ctr, + }, "start:web:1", preStart) + start.Phase = PhaseStart + post := plan.addNode(Operation{ + Type: OpRunPostStart, + ResourceID: "service:web:1", + Cause: "post_start hooks", + Service: &service, + Container: &ctr, + }, "start:web:1", start) + post.Phase = PhaseStart + + err := svc.executePlan(t.Context(), project, emptyObservedState("test"), plan) + + assert.Error(t, err, "missing dependency db") +} + +// TestPlanRunRefusesACreateNodeDependingOnAStartNode pins that a plan that +// could only deadlock is refused before anything runs: a Create-phase node +// waiting on a Start-phase one would wait forever, as the Start phase is only +// dispatched once the Create phase is over. +func TestPlanRunRefusesACreateNodeDependingOnAStartNode(t *testing.T) { + svc, _, _ := newStartPhaseTestService(t) + service := types.ServiceConfig{Name: "web"} + ctr := container.Summary{ID: "web-id", Names: []string{"/test-web-1"}} + + plan := &Plan{} + start := plan.addNode(Operation{Type: OpStartContainer, ResourceID: "service:web:1", Cause: "start", Service: &service, Container: &ctr}, "") + start.Phase = PhaseStart + plan.addNode(Operation{Type: OpCreateNetwork, ResourceID: "network:default", Name: "default"}, "", start) + + exec := svc.newPlanExecutor(&types.Project{Name: "test"}, emptyObservedState("test")) + _, err := exec.begin(plan) + + assert.ErrorContains(t, err, "invalid plan: create-phase node 2 depends on start-phase node 1") +} + +// TestExecStartContainer_BailsOutWhenContextCanceled is the start node's side +// of TestExecRunPreStart_BailsOutWhenContextCanceled: once ctx is canceled (a +// sibling node failed, or the caller gave up), a start must neither inject the +// container's secrets and configs nor start it. No daemon expectation is +// registered: an unexpected call fails the test. +func TestExecStartContainer_BailsOutWhenContextCanceled(t *testing.T) { + svc, _, _ := newStartPhaseTestService(t) + service := types.ServiceConfig{Name: "web"} + ctr := container.Summary{ID: "web-id", Names: []string{"/test-web-1"}} + ctx, cancel := context.WithCancel(t.Context()) + cancel() + + exec := svc.newPlanExecutor(&types.Project{Name: "test", Services: types.Services{"web": service}}, emptyObservedState("test")) + err := exec.execStartContainer(ctx, Operation{Type: OpStartContainer, Service: &service, Container: &ctr}) + + assert.ErrorIs(t, err, context.Canceled) +} + +// TestExecRunPostStart_BailsOutWhenContextCanceled is the same for the +// post_start hooks: none runs once ctx is canceled. +func TestExecRunPostStart_BailsOutWhenContextCanceled(t *testing.T) { + svc, _, _ := newStartPhaseTestService(t) + service := types.ServiceConfig{ + Name: "web", + PostStart: []types.ServiceHook{{Command: types.ShellCommand{"/notify.sh"}}}, + } + ctr := container.Summary{ID: "web-id", Names: []string{"/test-web-1"}} + ctx, cancel := context.WithCancel(t.Context()) + cancel() + + exec := svc.newPlanExecutor(&types.Project{Name: "test"}, emptyObservedState("test")) + err := exec.execRunPostStart(ctx, Operation{Type: OpRunPostStart, Service: &service, Container: &ctr}) + + assert.ErrorIs(t, err, context.Canceled) +} + +// TestExecutePlanBoundedConcurrencyNeverBlocksAPrerequisite pins the executor +// constraint that a concurrency cap must not be put on the goroutines of the +// plan's nodes: every node waits on its prerequisites in a goroutine of its +// own, so a cap on the goroutines would let nodes waiting for a prerequisite +// hold every slot, and the prerequisite never run. Here --parallel is 1 and +// the nodes depend on nodes created after them (the first waits for the +// third, which waits for the second), so that no creation order saves them: +// the plan has to complete, in the order of its edges. +func TestExecutePlanBoundedConcurrencyNeverBlocksAPrerequisite(t *testing.T) { + svc, apiClient, _ := newStartPhaseTestService(t) + svc.maxConcurrency = 1 + + ctrs := map[string]container.Summary{} + plan := &Plan{} + nodes := map[string]*PlanNode{} + for _, name := range []string{"first", "second", "third"} { + ctr := container.Summary{ID: name + "-id", Names: []string{"/test-" + name + "-1"}} + ctrs[name] = ctr + nodes[name] = plan.addNode(Operation{Type: OpStartContainer, ResourceID: "service:" + name + ":1", Cause: "start", Container: &ctr}, "") + } + // the dependencies point at nodes created later + nodes["first"].DependsOn = []*PlanNode{nodes["third"]} + nodes["third"].DependsOn = []*PlanNode{nodes["second"]} + second := apiClient.EXPECT().ContainerStart(gomock.Any(), "second-id", gomock.Any()).Return(client.ContainerStartResult{}, nil) + third := apiClient.EXPECT().ContainerStart(gomock.Any(), "third-id", gomock.Any()).Return(client.ContainerStartResult{}, nil).After(second) + apiClient.EXPECT().ContainerStart(gomock.Any(), "first-id", gomock.Any()).Return(client.ContainerStartResult{}, nil).After(third) + + // the deadline only turns a deadlock into a prompt failure + ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second) + defer cancel() + exec := svc.newPlanExecutor(&types.Project{Name: "test"}, emptyObservedState("test")) + assert.NilError(t, exec.run(ctx, plan)) +} + +// TestExecutePlanStartingIsReportedWhenTheContainerStarts pins when the row of +// a replica reads Starting: when its container is started, not when the chain +// that leads to it begins. The pre_start hooks of the service run silently +// before it; the row's Working event is the start's own. +func TestExecutePlanStartingIsReportedWhenTheContainerStarts(t *testing.T) { + svc, apiClient, recorder := newStartPhaseTestService(t) + + service := types.ServiceConfig{ + Name: "web", + PreStart: []types.PreStartHook{{ContainerSpec: types.ContainerSpec{Command: types.ShellCommand{"init"}}}}, + } + ctr := container.Summary{ID: "web-id", Names: []string{"/test-web-1"}} + const row = "Container test-web-1" + rowNow := func() []string { + recorder.mu.Lock() + defer recorder.mu.Unlock() + return slices.Clone(recorder.byID[row]) + } + + // the pre_start node first asks the daemon whether a replica already runs, + // and finds one: it has nothing to run, and the row is still silent + var duringPreStart, atStart []string + apiClient.EXPECT().ContainerList(gomock.Any(), gomock.Any()). + DoAndReturn(func(context.Context, client.ContainerListOptions) (client.ContainerListResult, error) { + duringPreStart = rowNow() + return client.ContainerListResult{Items: []container.Summary{ctr}}, nil + }) + apiClient.EXPECT().ContainerStart(gomock.Any(), "web-id", gomock.Any()). + DoAndReturn(func(context.Context, string, client.ContainerStartOptions) (client.ContainerStartResult, error) { + atStart = rowNow() + return client.ContainerStartResult{}, nil + }) + + plan := &Plan{} + preStart := plan.addNode(Operation{Type: OpRunPreStart, ResourceID: "service:web:1", Cause: "pre_start hooks", Service: &service, Container: &ctr}, "start:web:1") + preStart.Phase = PhaseStart + start := plan.addNode(Operation{Type: OpStartContainer, ResourceID: "service:web:1", Cause: "start", Service: &service, Container: &ctr}, "start:web:1", preStart) + start.Phase = PhaseStart + + exec := svc.newPlanExecutor(&types.Project{Name: "test", Services: types.Services{"web": service}}, emptyObservedState("test")) + assert.NilError(t, exec.run(t.Context(), plan)) + + assert.Equal(t, len(duringPreStart), 0, "the row must say nothing during pre_start: %v", duringPreStart) + assert.DeepEqual(t, atStart, []string{api.StatusStarting}) + assert.DeepEqual(t, recorder.byID[row], []string{api.StatusStarting, api.StatusStarted}) +} + +// TestRunNodeRelaysTheOriginalErrorDownALongChain pins what each node of a +// chain does with the failure of the node it depends on: it does not run, and +// it passes on the ORIGINAL error -- the one the failing node recorded, not +// the "dependency failed" error its neighbour returned -- so that the root +// cause stays legible however many hops down the chain a node is: a +// replica's chain (create, pre_start, start, post_start) is three hops long. +func TestRunNodeRelaysTheOriginalErrorDownALongChain(t *testing.T) { + svc, _, _ := newStartPhaseTestService(t) + service := types.ServiceConfig{Name: "web"} + ctr := container.Summary{ID: "web-id", Names: []string{"/test-web-1"}} + + plan := &Plan{} + failing := plan.addNode(Operation{Type: OpWaitCondition, ResourceID: "wait:db:service_healthy", Name: "db", Condition: types.ServiceConditionHealthy}, "") + failing.Phase = PhaseStart + chain := []*PlanNode{failing} + for _, op := range []OperationType{OpRunPreStart, OpStartContainer, OpRunPostStart} { + node := plan.addNode(Operation{Type: op, ResourceID: "service:web:1", Service: &service, Container: &ctr}, "start:web:1", chain[len(chain)-1]) + node.Phase = PhaseStart + chain = append(chain, node) + } + exec := svc.newPlanExecutor(&types.Project{Name: "test"}, emptyObservedState("test")) + run, err := exec.begin(plan) + assert.NilError(t, err) + + // the first node failed: what the executor does when its operation fails + original := errors.New("db is unhealthy") + run.rs.recordFailure(failing.ID, original) + close(run.rs.done[failing.ID]) + + phase := newPhaseCancel(t.Context()) + for hop, node := range chain[1:] { + err := exec.runNode(node, run.rs, phase) + + assert.Assert(t, errors.Is(err, original), "hop %d: %v", hop+1, err) + assert.Equal(t, err.Error(), "dependency failed: db is unhealthy", "hop %d wrapped the error once more", hop+1) + } +} + +// runNodeFixture is a planExecutor and the state of a planRun, to drive +// runNode on the nodes of plan one at a time: what the goroutines of a phase do +// concurrently, with an interleaving the test picks. +func runNodeFixture(t *testing.T, plan *Plan) (*planExecutor, *planRun) { + t.Helper() + svc, _, _ := newStartPhaseTestService(t) + exec := svc.newPlanExecutor(&types.Project{Name: "test"}, emptyObservedState("test")) + run, err := exec.begin(plan) + assert.NilError(t, err) + return exec, run +} + +func bareStartNode(plan *Plan, name string, after ...*PlanNode) *PlanNode { + ctr := container.Summary{ID: name + "-id", Names: []string{"/test-" + name + "-1"}} + node := plan.addNode(Operation{Type: OpStartContainer, ResourceID: "service:" + name + ":1", Cause: "start", Container: &ctr}, "", after...) + node.Phase = PhaseStart + return node +} + +// TestRunNodeRecordsACancellationWhileWaitingAsAFailure pins that a node +// given up while it waits for its prerequisites is recorded as failed, so that +// the nodes depending on it skip: their own wait can find its done channel +// closed and the phase's context canceled at once, and select picks either -- +// the one that sees only the closed channel must still find the failure, or it +// would go on to start with a canceled context, and report a Starting it never +// finishes. +func TestRunNodeRecordsACancellationWhileWaitingAsAFailure(t *testing.T) { + plan := &Plan{} + prerequisite := bareStartNode(plan, "db") + waiting := bareStartNode(plan, "web", prerequisite) + exec, run := runNodeFixture(t, plan) + phase := newPhaseCancel(t.Context()) + phase.cancel() // the prerequisite never completes: the phase is over + + err := exec.runNode(waiting, run.rs, phase) + + assert.ErrorIs(t, err, context.Canceled) + assert.ErrorIs(t, run.rs.failedDependency([]*PlanNode{waiting}), context.Canceled, "the dependents of the node must find it failed") +} + +// TestRunNodeRecordsAGroupGivenUpAsAFailure is the same for a node that does +// not start because the group of its resource ended meanwhile (another node of +// the group failed while this one was getting a slot). +func TestRunNodeRecordsAGroupGivenUpAsAFailure(t *testing.T) { + plan := &Plan{} + ctr := container.Summary{ID: "web-id", Names: []string{"/test-web-1"}} + node := plan.addNode(Operation{Type: OpStartContainer, ResourceID: "service:web:1", Cause: "start", Container: &ctr}, "start:web:1") + node.Phase = PhaseStart + exec, run := runNodeFixture(t, plan) + run.rs.groups.groups["start:web:1"].finished = true + + err := exec.runNode(node, run.rs, newPhaseCancel(t.Context())) + + assert.ErrorIs(t, err, context.Canceled) + assert.ErrorIs(t, run.rs.failedDependency([]*PlanNode{node}), context.Canceled, "the dependents of the node must find it failed") +} + +// TestRunNodeDoesNotCountAnInterruptedWaitAsASuccess pins that a wait ended +// by its caller going away -- it then ends quietly, with no error -- is not +// recorded as a success: its condition was never met, and what is reported for +// the nodes held behind it must not take it for satisfied. +func TestRunNodeDoesNotCountAnInterruptedWaitAsASuccess(t *testing.T) { + plan := &Plan{} + wait := plan.addNode(Operation{Type: OpWaitCondition, ResourceID: "wait:db:service_healthy", Name: "db", Condition: types.ServiceConditionHealthy}, "") + wait.Phase = PhaseStart + exec, run := runNodeFixture(t, plan) + exec.containersByService["db"] = Containers{{ID: "db-id", Names: []string{"/test-db-1"}, Labels: map[string]string{api.ServiceLabel: "db"}}} + ctx, cancel := context.WithCancel(t.Context()) + cancel() + + err := exec.runNode(wait, run.rs, newPhaseCancel(ctx)) + + assert.NilError(t, err) + assert.Assert(t, !run.rs.succeeded(wait.ID), "the wait ended unmet, it is not a success") + assert.NilError(t, run.rs.failedDependency([]*PlanNode{wait})) +} + +// TestRunPhaseFailsWithTheErrorOfTheNodeThatFailedFirst pins what a phase +// returns once one of its nodes failed: that node's error, and not the first +// error the phase's goroutines happen to return, which may be the bare +// context.Canceled of a node that was given up because of it (errgroup keeps +// whichever comes first). +func TestRunPhaseFailsWithTheErrorOfTheNodeThatFailedFirst(t *testing.T) { + plan := &Plan{} + failed := bareStartNode(plan, "db") + casualty := bareStartNode(plan, "web", failed) + exec, run := runNodeFixture(t, plan) + origin := errors.New("no space left on device") + phase := newPhaseCancel(t.Context()) + phase.err = origin // what the failure of the prerequisite registered... + phase.cancel() // ...before its dependent, still waiting for it, was given up + + err := exec.runPhase([]*PlanNode{casualty}, run.rs, phase) + + assert.Equal(t, err, origin) +} + +// TestExecCreateHookContainer_TargetsTheLowestNumberedReplica pins which +// replica a pre_start hook shares the volumes of: the lowest-numbered one of +// the live view, the one the start phase hands the hooks, whatever order the +// replicas are in. +func TestExecCreateHookContainer_TargetsTheLowestNumberedReplica(t *testing.T) { + svc, apiClient, _ := newStartPhaseTestService(t) + service := types.ServiceConfig{ + Name: "web", + ContainerSpec: types.ContainerSpec{Image: "alpine"}, + PreStart: []types.PreStartHook{{ContainerSpec: types.ContainerSpec{Command: types.ShellCommand{"init"}}}}, + } + replica := func(number string) container.Summary { + return container.Summary{ID: "c" + number, Labels: map[string]string{api.ContainerNumberLabel: number}} + } + var created client.ContainerCreateOptions + apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, opts client.ContainerCreateOptions) (client.ContainerCreateResult, error) { + created = opts + return client.ContainerCreateResult{ID: "hook-id"}, nil + }) + + plan := &Plan{} + hook := plan.addNode(Operation{ + Type: OpCreateHookContainer, + ResourceID: "hook:web:pre_start:0", + Cause: "pre_start hook", + Service: &service, + HookIndex: 0, + Name: getHookContainerName("test", "web", 0), + }, "") + exec := svc.newPlanExecutor(&types.Project{Name: "test", Services: types.Services{"web": service}}, emptyObservedState("test")) + // neither the first nor the last of the view is the lowest-numbered + exec.containersByService["web"] = Containers{replica("3"), replica("1"), replica("2")} + + assert.NilError(t, exec.execCreateHookContainer(t.Context(), hook)) + + assert.DeepEqual(t, created.HostConfig.VolumesFrom, []string{"c1"}) +} + +// TestExecCreateHookContainer_BailsOutWhenContextCanceled pins that a runner +// is not created once ctx is canceled -- a replica's creation that failed +// canceled the phase, and the live view this node resolves its target from is +// then incomplete: it ends on the cancellation, not on an internal error about +// a missing container, and calls nothing. +func TestExecCreateHookContainer_BailsOutWhenContextCanceled(t *testing.T) { + svc, _, _ := newStartPhaseTestService(t) + service := types.ServiceConfig{ + Name: "web", + PreStart: []types.PreStartHook{{ContainerSpec: types.ContainerSpec{Command: types.ShellCommand{"init"}}}}, + } + plan := &Plan{} + hook := plan.addNode(Operation{Type: OpCreateHookContainer, ResourceID: "hook:web:pre_start:0", Service: &service, HookIndex: 0, Name: "test-web-pre_start-0"}, "") + exec := svc.newPlanExecutor(&types.Project{Name: "test"}, emptyObservedState("test")) + ctx, cancel := context.WithCancel(t.Context()) + cancel() + + err := exec.execCreateHookContainer(ctx, hook) + + assert.ErrorIs(t, err, context.Canceled) +} + +// TestExecStopContainer_PassesTheTimeoutInWholeSeconds pins how the timeout of +// a stop of the plan (the one that precedes a container's recreation, from +// up --timeout) reaches the daemon: in whole seconds, and not at all when +// none was given, so that the container's own stop timeout applies. +func TestExecStopContainer_PassesTheTimeoutInWholeSeconds(t *testing.T) { + seconds := func(d time.Duration) *time.Duration { return &d } + tests := []struct { + name string + timeout *time.Duration + want *int + }{ + {name: "none given"}, + {name: "whole seconds", timeout: seconds(7 * time.Second), want: intPtr(7)}, + {name: "a fraction of a second is dropped", timeout: seconds(1900 * time.Millisecond), want: intPtr(1)}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + svc, apiClient, _ := newStartPhaseTestService(t) + ctr := container.Summary{ID: "web-id", Names: []string{"/test-web-1"}} + var got client.ContainerStopOptions + apiClient.EXPECT().ContainerStop(gomock.Any(), "web-id", gomock.Any()). + DoAndReturn(func(_ context.Context, _ string, opts client.ContainerStopOptions) (client.ContainerStopResult, error) { + got = opts + return client.ContainerStopResult{}, nil + }) + exec := svc.newPlanExecutor(&types.Project{Name: "test"}, emptyObservedState("test")) + + err := exec.execStopContainer(t.Context(), Operation{Type: OpStopContainer, Container: &ctr, Timeout: tt.timeout}) + + assert.NilError(t, err) + assert.DeepEqual(t, got.Timeout, tt.want) + }) + } +} diff --git a/pkg/compose/lifecycle_wiring_test.go b/pkg/compose/lifecycle_wiring_test.go new file mode 100644 index 0000000000..e5db2c7dd9 --- /dev/null +++ b/pkg/compose/lifecycle_wiring_test.go @@ -0,0 +1,236 @@ +//go:build !windows + +/* + Copyright 2026 Docker Compose CLI authors + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package compose + +import ( + "context" + "strings" + "sync" + "testing" + "time" + + "github.com/compose-spec/compose-go/v2/types" + "github.com/moby/moby/api/types/container" + "github.com/moby/moby/api/types/image" + "github.com/moby/moby/client" + "go.uber.org/mock/gomock" + "gotest.tools/v3/assert" + + "github.com/docker/compose/v5/pkg/api" +) + +// callsOf counts the recorded engine calls named call. +func callsOf(d *interactiveUpDaemon, call string) int { + n := 0 + for _, c := range d.recorded() { + if c == call { + n++ + } + } + return n +} + +// assertCreatedThenStarted checks that the daemon was asked to create the +// container, then to start it. +func assertCreatedThenStarted(t *testing.T, d *interactiveUpDaemon) { + t.Helper() + create, start := d.indexOf("create"), d.indexOf("start") + assert.Assert(t, create >= 0, "the container was never created: %v", d.recorded()) + assert.Assert(t, start >= 0, "the container was never started: %v", d.recorded()) + assert.Assert(t, create < start, "the container must be created before it is started: %v", d.recorded()) +} + +// TestUpDetachedCreatesThenStartsTheContainers pins that a detached up leaves +// the application running: its plan covers both phases, so that what it creates +// it starts, with no session attached to it. +func TestUpDetachedCreatesThenStartsTheContainers(t *testing.T) { + h := newInteractiveUpHarness(t) + + assert.NilError(t, h.svc.Up(t.Context(), interactiveUpProject(), api.UpOptions{})) + + assertCreatedThenStarted(t, h.daemon) + assert.Equal(t, h.daemon.indexOf("attach"), -1, "a detached up attaches to nothing: %v", h.daemon.recorded()) +} + +// TestScaleCreatesThenStartsTheReplicas pins that scale starts the replicas it +// creates: it converges the project and starts it in a single plan, as a +// detached up does, and so does the rebuild of a service by watch, which goes +// through the same path. +func TestScaleCreatesThenStartsTheReplicas(t *testing.T) { + h := newInteractiveUpHarness(t) + + err := h.svc.Scale(t.Context(), interactiveUpProject(), api.ScaleOptions{Replicas: map[string]int{"web": 1}}) + + assert.NilError(t, err) + assertCreatedThenStarted(t, h.daemon) +} + +// TestRunStartsTheDependenciesOfTheOneOff pins that `compose run` brings up +// the dependencies of the service it runs -- created, then started -- and +// only those: the service itself is the one-off container's business. +func TestRunStartsTheDependenciesOfTheOneOff(t *testing.T) { + h := newInteractiveUpHarness(t) + project := interactiveUpProject() + app := types.ServiceConfig{Name: "app", ContainerSpec: types.ContainerSpec{Image: "alpine"}} + app.DependsOn = types.DependsOnConfig{"web": {Condition: types.ServiceConditionStarted, Required: true}} + project.Services["app"] = app + + err := h.svc.startDependencies(t.Context(), project, api.RunOptions{Project: project, Service: "app"}) + + assert.NilError(t, err) + assertCreatedThenStarted(t, h.daemon) + assert.Equal(t, callsOf(h.daemon, "create"), 1, "only the dependency is created, not the service run: %v", h.daemon.recorded()) + assert.Equal(t, callsOf(h.daemon, "start"), 1, "only the dependency is started: %v", h.daemon.recorded()) +} + +// TestUpDetachedWaitVerifiesTheApplicationIsReady pins what --wait adds to a +// detached up: once the containers are started, it checks that they are up -- +// an exited container fails the up, naming it, and one that does not become +// healthy in time fails it with the --wait-timeout that elapsed -- where +// without --wait the up returns as soon as the containers are started. +func TestUpDetachedWaitVerifiesTheApplicationIsReady(t *testing.T) { + healthcheck := &types.HealthCheckConfig{Test: []string{"CMD", "true"}} + projectWithHealthcheck := func() *types.Project { + project := interactiveUpProject() + web := project.Services["web"] + web.HealthCheck = healthcheck + project.Services["web"] = web + return project + } + + t.Run("a container that exited fails the up", func(t *testing.T) { + h := newInteractiveUpHarness(t) + + err := h.svc.Up(t.Context(), interactiveUpProject(), api.UpOptions{Start: api.StartOptions{Wait: true, WaitTimeout: 5 * time.Second}}) + + assert.ErrorContains(t, err, "container test-web-1 exited") + }) + + t.Run("a container that stays unhealthy fails the up at the --wait-timeout", func(t *testing.T) { + h := newInteractiveUpHarness(t) + h.daemon.inspectState = &container.State{Status: container.StateRunning, Health: &container.Health{Status: container.Starting}} + + err := h.svc.Up(t.Context(), projectWithHealthcheck(), api.UpOptions{Start: api.StartOptions{Wait: true, WaitTimeout: 100 * time.Millisecond}}) + + assert.Error(t, err, "application not healthy after 100ms") + }) + + t.Run("without --wait nothing is verified", func(t *testing.T) { + h := newInteractiveUpHarness(t) + + assert.NilError(t, h.svc.Up(t.Context(), interactiveUpProject(), api.UpOptions{})) + }) +} + +// TestWatchRebuildTouchesTheRebuiltServiceAndItsDependentsOnly pins which of +// the project's containers a rebuild by watch converges: those of the rebuilt +// service and of the services depending on it (their configuration may +// diverge from what the rebuilt service now is), and no others -- neither its +// dependencies, which are not rebuilt either, nor the services unrelated to +// it, whose stopped containers have no business being started by a rebuild. +// The images are not built here (no service has a build section); the project +// the rebuild works on is what is narrowed. +func TestWatchRebuildTouchesTheRebuiltServiceAndItsDependentsOnly(t *testing.T) { + svc, apiClient, _ := newStartPhaseTestService(t) + service := func(name string, dependsOn ...string) types.ServiceConfig { + s := types.ServiceConfig{Name: name, ContainerSpec: types.ContainerSpec{Image: "alpine"}} + if len(dependsOn) > 0 { + s.DependsOn = types.DependsOnConfig{} + } + for _, dep := range dependsOn { + s.DependsOn[dep] = types.ServiceDependency{Condition: types.ServiceConditionStarted, Required: true} + } + return s + } + project := &types.Project{ + Name: "prj", + Services: types.Services{ + "app": service("app", "database"), + "proxy": service("proxy", "app"), + "database": service("database"), + "unrelated": service("unrelated"), + }, + Networks: types.Networks{}, + Volumes: types.Volumes{}, + } + existing := []container.Summary{ + stoppedServiceContainer("proxy", 1, container.StateExited), + stoppedServiceContainer("database", 1, container.StateExited), + stoppedServiceContainer("unrelated", 1, container.StateExited), + } + + // what the daemon is asked to do, by container ID + var mu sync.Mutex + touched := map[string]bool{} + touch := func(id string) { + mu.Lock() + defer mu.Unlock() + touched[id] = true + } + apiClient.EXPECT().ImageInspect(gomock.Any(), "alpine", gomock.Any()).AnyTimes(). + Return(client.ImageInspectResult{InspectResponse: image.InspectResponse{ID: "sha256:alpine"}}, nil) + apiClient.EXPECT().NetworkList(gomock.Any(), gomock.Any()).AnyTimes().Return(client.NetworkListResult{}, nil) + apiClient.EXPECT().VolumeList(gomock.Any(), gomock.Any()).AnyTimes().Return(client.VolumeListResult{}, nil) + apiClient.EXPECT().ContainerList(gomock.Any(), gomock.Any()).AnyTimes(). + Return(client.ContainerListResult{Items: existing}, nil) + apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()).AnyTimes(). + DoAndReturn(func(_ context.Context, opts client.ContainerCreateOptions) (client.ContainerCreateResult, error) { + touch("new:" + opts.Name) + return client.ContainerCreateResult{ID: "new-" + opts.Name}, nil + }) + apiClient.EXPECT().ContainerInspect(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes(). + DoAndReturn(func(_ context.Context, id string, _ client.ContainerInspectOptions) (client.ContainerInspectResult, error) { + return client.ContainerInspectResult{Container: container.InspectResponse{ + ID: id, Name: "/" + strings.TrimPrefix(id, "new-"), Config: &container.Config{}, NetworkSettings: &container.NetworkSettings{}, + }}, nil + }) + apiClient.EXPECT().ContainerStop(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes(). + DoAndReturn(func(_ context.Context, id string, _ client.ContainerStopOptions) (client.ContainerStopResult, error) { + touch(id) + return client.ContainerStopResult{}, nil + }) + apiClient.EXPECT().ContainerRemove(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes(). + DoAndReturn(func(_ context.Context, id string, _ client.ContainerRemoveOptions) (client.ContainerRemoveResult, error) { + touch(id) + return client.ContainerRemoveResult{}, nil + }) + apiClient.EXPECT().ContainerRename(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes(). + Return(client.ContainerRenameResult{}, nil) + apiClient.EXPECT().ContainerStart(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes(). + DoAndReturn(func(_ context.Context, id string, _ client.ContainerStartOptions) (client.ContainerStartResult, error) { + touch(id) + return client.ContainerStartResult{}, nil + }) + + options := api.WatchOptions{Build: &api.BuildOptions{}, LogTo: &testLogConsumer{}} + assert.NilError(t, svc.rebuild(t.Context(), project, []string{"app"}, options)) + + touchesService := func(name string) bool { + for id := range touched { + if strings.Contains(id, "prj-"+name+"-1") { + return true + } + } + return false + } + assert.Assert(t, touchesService("app"), "the rebuilt service is created and started: %v", touched) + assert.Assert(t, touchesService("proxy"), "the container of the service depending on it is converged: %v", touched) + assert.Assert(t, !touchesService("database"), "a dependency of the rebuilt service is left alone: %v", touched) + assert.Assert(t, !touchesService("unrelated"), "a service unrelated to it is left alone: %v", touched) +} diff --git a/pkg/compose/monitor_test.go b/pkg/compose/monitor_test.go index a3d1d5ccff..750af5e3b1 100644 --- a/pkg/compose/monitor_test.go +++ b/pkg/compose/monitor_test.go @@ -264,6 +264,14 @@ func monitorDieEvent(exitCode int) events.Message { // newMonitorTestFixture wires a monitor against a mocked API client, with the // goroutine-leak guard and the standard initial ContainerList expectation. func newMonitorTestFixture(t *testing.T) (*monitor, *mocks.MockAPIClient) { + t.Helper() + return newMonitorTestFixtureListing(t, []container.Summary{testContainer("service1", "123", false)}, nil) +} + +// newMonitorTestFixtureListing is newMonitorTestFixture for given containers +// in the initial listing, whose request it records in listed when it is not +// nil. +func newMonitorTestFixtureListing(t *testing.T, initial []container.Summary, listed *client.ContainerListOptions) (*monitor, *mocks.MockAPIClient) { t.Helper() ignoreExisting := goleak.IgnoreCurrent() t.Cleanup(func() { @@ -275,7 +283,12 @@ func newMonitorTestFixture(t *testing.T) (*monitor, *mocks.MockAPIClient) { expectNowAsDaemonTime(apiMock) apiMock.EXPECT().ContainerList(gomock.Any(), gomock.Any()). - Return(client.ContainerListResult{Items: []container.Summary{testContainer("service1", "123", false)}}, nil) + DoAndReturn(func(_ context.Context, opts client.ContainerListOptions) (client.ContainerListResult, error) { + if listed != nil { + *listed = opts + } + return client.ContainerListResult{Items: initial}, nil + }) m := newMonitor(apiMock, strings.ToLower(testProject)) return m, apiMock @@ -952,3 +965,68 @@ func TestMonitorReadDaemonTimeOnce(t *testing.T) { assert.DeepEqual(t, log.since, []string{"981153306.789012345"}) } + +// TestMonitorFollowsTheContainersOfTheApplicationOnly pins which containers +// the monitor follows: the project's containers that are not one-off, and, in +// the initial listing, that carry a config hash, running or not. The hook +// runners of pre_start carry none: a runner's exit is not the end of the +// application. +func TestMonitorFollowsTheContainersOfTheApplicationOnly(t *testing.T) { + var listed client.ContainerListOptions + m, apiMock := newMonitorTestFixtureListing(t, []container.Summary{testContainer("service1", "123", false)}, &listed) + var subscribed client.EventsListOptions + messages := make(chan events.Message, 1) + messages <- monitorEvent(events.ActionDestroy) + apiMock.EXPECT().Events(gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, opts client.EventsListOptions) client.EventsResult { + subscribed = opts + return client.EventsResult{Messages: messages, Err: make(chan error)} + }) + + _, err := runMonitor(t, m) + + assert.NilError(t, err) + project := labelFilter(api.ProjectLabel, strings.ToLower(testProject)) + assert.Assert(t, listed.All, "stopped containers are part of the application") + labels := listed.Filters["label"] + assert.Assert(t, labels[project], "listing filters: %v", listed.Filters) + assert.Assert(t, labels[oneOffFilter(false)], "one-off containers are not part of the application: %v", listed.Filters) + assert.Assert(t, labels[api.ConfigHashLabel], "a container without a config hash (a hook runner) is not: %v", listed.Filters) + labels = subscribed.Filters["label"] + assert.Assert(t, subscribed.Filters["type"]["container"], "events filters: %v", subscribed.Filters) + assert.Assert(t, labels[project], "events filters: %v", subscribed.Filters) + assert.Assert(t, labels[oneOffFilter(false)], "the events of one-off containers are not wanted: %v", subscribed.Filters) +} + +// TestMonitorDoesNotWaitForARelay pins that the relay container a provider +// service may leave in the project -- it never terminates on its own -- is not +// part of the application the monitor follows, whether it is there at the +// start or created later: the monitor ends with the application's own +// containers, instead of waiting forever for the relay. +func TestMonitorDoesNotWaitForARelay(t *testing.T) { + relayLabels := func() map[string]string { + labels := containerLabels("provider", false) + labels[api.RelayLabel] = "relay-identity" + return labels + } + + t.Run("already there at the start", func(t *testing.T) { + relay := container.Summary{ID: "relay", Labels: relayLabels()} + m, apiMock := newMonitorTestFixtureListing(t, []container.Summary{testContainer("service1", "123", false), relay}, nil) + expectEvents(apiMock, monitorEvent(events.ActionDestroy)) + + _, err := runMonitor(t, m) + + assert.NilError(t, err) + }) + + t.Run("created while the application runs", func(t *testing.T) { + m, apiMock := newMonitorTestFixture(t) + relayCreated := containerMessage(events.ActionCreate, "relay", "testproject-provider-1", "provider", map[string]string{api.RelayLabel: "relay-identity"}) + expectEvents(apiMock, relayCreated, monitorEvent(events.ActionDestroy)) + + _, err := runMonitor(t, m) + + assert.NilError(t, err) + }) +} diff --git a/pkg/compose/reconcile_start_test.go b/pkg/compose/reconcile_start_test.go index 768f442102..6d087d3534 100644 --- a/pkg/compose/reconcile_start_test.go +++ b/pkg/compose/reconcile_start_test.go @@ -1307,3 +1307,84 @@ func TestPlanStart_DefaultScopeIsInert(t *testing.T) { } assert.Equal(t, plan.String(), "[] -> #1 service:app:1, CreateContainer, no existing container\n") } + +// A wait node is ordered after the start of the dependency it waits for: the +// condition is only worth checking once the dependency's container has been +// started. Here a single dependent uses the wait, so nothing but the node's +// own prerequisites can order it (a second dependent would add them to the +// shared node). +func TestPlanStart_WaitFollowsTheDependencyStart(t *testing.T) { + project := &types.Project{ + Name: "myproject", + Services: types.Services{ + "db": {Name: "db"}, + "web": serviceWithDeps("web", types.DependsOnConfig{ + "db": {Condition: types.ServiceConditionHealthy, Required: true}, + }), + }, + } + + plan, err := reconcile(t.Context(), project, emptyObserved(), startScopeOptions(ScopeCreateStart), noPrompt) + assert.NilError(t, err) + + assert.Equal(t, plan.String(), strings.TrimSpace(` +[] -> #1 service:db:1, CreateContainer, no existing container +[1] -> #2 service:web:1, CreateContainer, no existing container +[1] -> #3 service:db:1, StartContainer, start [start:db:1] {start} +[3] -> #4 wait:db:service_healthy, WaitCondition, depends_on condition {start} +[2,4] -> #5 service:web:1, StartContainer, start [start:web:1] {start} +`)+"\n") +} + +// A project whose services all have scale: 0 has no container to start, and +// that is not an error: `compose start` has nothing to do. +func TestPlanStart_ScaleZeroServiceWithoutAnyContainerIsNotAnError(t *testing.T) { + zero := 0 + project := &types.Project{ + Name: "myproject", + Services: types.Services{"app": {Name: "app", Scale: &zero}}, + } + + plan, err := reconcile(t.Context(), project, emptyObserved(), startScopeOptions(ScopeStart), noPrompt) + assert.NilError(t, err) + assert.Equal(t, len(plan.Nodes), 0, "nothing to plan:\n%s", plan) +} + +// The Start phase covers every service of the project the plan is built for, +// whichever services the create options select: Services only decides the +// services whose containers may be recreated (see api.ScaleOptions.Services), +// so scaling a service starts, with the replicas it creates, the stopped +// containers of the project's other services. The CLI hands the plan a +// project narrowed to the services it names and their dependencies, which is +// why a user doesn't meet it. Starting only what was selected would be a +// decision of its own (#14081, "scale start scope"), not the side effect of +// another change: this pins the behavior as it stands. +func TestPlanStart_StartPhaseCoversEveryServiceOfTheProject(t *testing.T) { + two := 2 + db := types.ServiceConfig{Name: "db", Scale: &two} + web := serviceWithDeps("web", types.DependsOnConfig{"db": {Condition: types.ServiceConditionStarted, Required: true}}) + other := types.ServiceConfig{Name: "other"} + project := &types.Project{Name: "myproject", Services: types.Services{"db": db, "web": web, "other": other}} + observed := emptyObserved() + for _, service := range []types.ServiceConfig{db, web, other} { + hash, err := serviceHashWithResolvedRefs(service, nil) + assert.NilError(t, err) + state := container.StateExited + if service.Name == "db" { + state = container.StateRunning + } + observed.Containers[service.Name] = []ObservedContainer{observedServiceContainer(service.Name, 1, state, hash)} + } + options := startScopeOptions(ScopeCreateStart) + options.Services = []string{"db"} + + plan, err := reconcile(t.Context(), project, observed, options, noPrompt) + assert.NilError(t, err) + + assert.Equal(t, plan.String(), strings.TrimSpace(` +[] -> #1 service:db:2, CreateContainer, no existing container +[1] -> #2 service:db:2, StartContainer, start [start:db:2] {start} +[] -> #3 service:other:1, StartContainer, start [start:other:1] {start} +[2] -> #4 service:web:1, StartContainer, start [start:web:1] {start} +`)+"\n") +} diff --git a/pkg/compose/restart_test.go b/pkg/compose/restart_test.go index f74dd8f6ed..e2d69daca3 100644 --- a/pkg/compose/restart_test.go +++ b/pkg/compose/restart_test.go @@ -26,6 +26,7 @@ import ( "time" "github.com/compose-spec/compose-go/v2/types" + "github.com/moby/moby/api/types/container" "github.com/moby/moby/client" "go.uber.org/mock/gomock" "gotest.tools/v3/assert" @@ -210,3 +211,67 @@ func TestRestart_ConcurrencyIsBoundedAcrossServices(t *testing.T) { assert.NilError(t, err) assert.Equal(t, tracker.Peak(), 1, "restart must never run more than maxConcurrency ContainerRestart calls at once") } + +// TestRestart_WaitsForADependencyBeforeRestartingItsDependent pins the order +// restart gives a depends_on relation declaring restart:true: the dependency +// is restarted first, and its dependent waits for the condition to hold again +// -- here healthy -- before being restarted itself. +func TestRestart_WaitsForADependencyBeforeRestartingItsDependent(t *testing.T) { + svc, apiClient, _ := newStartTestService(t) + + db := types.ServiceConfig{Name: "db"} + web := types.ServiceConfig{Name: "web"} + web.DependsOn = types.DependsOnConfig{"db": {Condition: types.ServiceConditionHealthy, Restart: true, Required: true}} + project := &types.Project{Name: "prj", Services: types.Services{"db": db, "web": web}} + dbCtr := stoppedServiceContainer("db", 1, container.StateRunning) + webCtr := stoppedServiceContainer("web", 1, container.StateRunning) + + apiClient.EXPECT().ContainerList(gomock.Any(), gomock.Any()). + Return(client.ContainerListResult{Items: []container.Summary{dbCtr, webCtr}}, nil) + dbRestart := apiClient.EXPECT().ContainerRestart(gomock.Any(), dbCtr.ID, gomock.Any()). + Return(client.ContainerRestartResult{}, nil) + dbHealthy := apiClient.EXPECT().ContainerInspect(gomock.Any(), dbCtr.ID, gomock.Any()). + Return(inspectResultOf(dbCtr.ID, "prj-db-1", container.StateRunning, 0, &container.Health{Status: container.Healthy}), nil). + After(dbRestart) + apiClient.EXPECT().ContainerRestart(gomock.Any(), webCtr.ID, gomock.Any()). + Return(client.ContainerRestartResult{}, nil).After(dbHealthy) + + err := svc.restart(t.Context(), "prj", api.RestartOptions{Project: project}) + + assert.NilError(t, err) +} + +// TestRestartContainer_PassesTheTimeoutInWholeSeconds pins how the timeout +// given to restart reaches the daemon: in whole seconds, the unit the engine's +// API knows, and not at all when none was given, so that the container's own +// stop timeout applies. +func TestRestartContainer_PassesTheTimeoutInWholeSeconds(t *testing.T) { + seconds := func(d time.Duration) *time.Duration { return &d } + tests := []struct { + name string + timeout *time.Duration + want *int + }{ + {name: "none given", timeout: nil, want: nil}, + {name: "whole seconds", timeout: seconds(5 * time.Second), want: intPtr(5)}, + {name: "zero is a timeout too", timeout: seconds(0), want: intPtr(0)}, + {name: "a fraction of a second is dropped", timeout: seconds(2500 * time.Millisecond), want: intPtr(2)}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + svc, apiClient, _ := newStartTestService(t) + ctr := serviceContainer("web") + var got client.ContainerRestartOptions + apiClient.EXPECT().ContainerRestart(gomock.Any(), ctr.ID, gomock.Any()). + DoAndReturn(func(_ context.Context, _ string, opts client.ContainerRestartOptions) (client.ContainerRestartResult, error) { + got = opts + return client.ContainerRestartResult{}, nil + }) + + err := svc.restartContainer(t.Context(), types.ServiceConfig{Name: "web"}, ctr, api.RestartOptions{Timeout: tt.timeout}) + + assert.NilError(t, err) + assert.DeepEqual(t, got.Timeout, tt.want) + }) + } +} diff --git a/pkg/compose/start_test.go b/pkg/compose/start_test.go index da358cf64b..19131e0f8b 100644 --- a/pkg/compose/start_test.go +++ b/pkg/compose/start_test.go @@ -26,6 +26,7 @@ import ( "sync" "sync/atomic" "testing" + "testing/synctest" "time" "github.com/compose-spec/compose-go/v2/types" @@ -308,3 +309,167 @@ func TestGetDependencyCondition(t *testing.T) { assert.Equal(t, getDependencyCondition(oneShot, project), types.ServiceConditionCompletedSuccessfully) assert.Equal(t, getDependencyCondition(web, project), ServiceConditionRunningOrHealthy) } + +func inspectResultOf(id, name string, status container.ContainerState, exitCode int, health *container.Health) client.ContainerInspectResult { + config := &container.Config{} + if health != nil { + config.Healthcheck = &container.HealthConfig{Test: []string{"CMD", "true"}} + } + return client.ContainerInspectResult{Container: container.InspectResponse{ + ID: id, + Name: "/" + name, + Config: config, + State: &container.State{Status: status, ExitCode: exitCode, Health: health}, + }} +} + +// TestStart_WaitVerifiesTheApplicationIsReady pins what --wait adds to start: +// once the containers are started, every service has to be up (running, and +// healthy when it has a healthcheck) before start returns, or start fails +// naming the --wait-timeout that elapsed. Without --wait, start returns as soon +// as the containers are started, and checks nothing. +func TestStart_WaitVerifiesTheApplicationIsReady(t *testing.T) { + web := types.ServiceConfig{Name: "web", ContainerSpec: types.ContainerSpec{Image: "alpine"}} + web.HealthCheck = &types.HealthCheckConfig{Test: []string{"CMD", "true"}} + project := &types.Project{Name: "prj", Services: types.Services{"web": web}} + webCtr := stoppedServiceContainer("web", 1, container.StateRunning) + + newService := func(t *testing.T, health container.HealthStatus) *composeService { + svc, apiClient, _ := newStartTestService(t) + apiClient.EXPECT().ContainerList(gomock.Any(), gomock.Any()). + Return(client.ContainerListResult{Items: []container.Summary{webCtr}}, nil).AnyTimes() + apiClient.EXPECT().ContainerInspect(gomock.Any(), webCtr.ID, gomock.Any()).AnyTimes(). + Return(inspectResultOf(webCtr.ID, "prj-web-1", container.StateRunning, 0, &container.Health{Status: health}), nil) + return svc + } + + t.Run("a service not ready in time fails the start", func(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + svc := newService(t, container.Starting) + + err := svc.start(t.Context(), "prj", api.StartOptions{Project: project, Wait: true, WaitTimeout: 2 * time.Second}) + + assert.Error(t, err, "application not healthy after 2s") + }) + }) + + t.Run("a service that is ready lets it succeed", func(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + svc := newService(t, container.Healthy) + + assert.NilError(t, svc.start(t.Context(), "prj", api.StartOptions{Project: project, Wait: true, WaitTimeout: 5 * time.Second})) + }) + }) + + t.Run("without --wait nothing is verified", func(t *testing.T) { + svc := newService(t, container.Starting) + + assert.NilError(t, svc.start(t.Context(), "prj", api.StartOptions{Project: project})) + }) +} + +// TestStart_WaitSatisfiesAOneShotServiceByItsCompletion pins that the --wait +// verification expects a service that others depend on with +// service_completed_successfully to have exited with 0, not to be running: a +// one-shot service is done once it has run, and would otherwise make --wait +// fail forever. One that completes with an error code fails it, naming the +// service and the code. +func TestStart_WaitSatisfiesAOneShotServiceByItsCompletion(t *testing.T) { + for _, tt := range []struct { + name string + exitCode int + err string + }{ + {name: "the one-shot service completed", exitCode: 0}, + {name: "the one-shot service failed", exitCode: 3, err: `service "init" didn't complete successfully: exit 3`}, + } { + t.Run(tt.name, func(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + svc, apiClient, _ := newStartTestService(t) + + initService := types.ServiceConfig{Name: "init", ContainerSpec: types.ContainerSpec{Image: "alpine"}} + app := types.ServiceConfig{Name: "app", ContainerSpec: types.ContainerSpec{Image: "alpine"}} + app.DependsOn = types.DependsOnConfig{"init": {Condition: types.ServiceConditionCompletedSuccessfully, Required: true}} + project := &types.Project{Name: "prj", Services: types.Services{"init": initService, "app": app}} + + initCtr := stoppedServiceContainer("init", 1, container.StateExited) + appCtr := stoppedServiceContainer("app", 1, container.StateRunning) + apiClient.EXPECT().ContainerList(gomock.Any(), gomock.Any()).AnyTimes(). + Return(client.ContainerListResult{Items: []container.Summary{initCtr, appCtr}}, nil) + apiClient.EXPECT().ContainerStart(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(client.ContainerStartResult{}, nil) + apiClient.EXPECT().ContainerInspect(gomock.Any(), initCtr.ID, gomock.Any()).AnyTimes(). + Return(inspectResultOf(initCtr.ID, "prj-init-1", container.StateExited, tt.exitCode, nil), nil) + apiClient.EXPECT().ContainerInspect(gomock.Any(), appCtr.ID, gomock.Any()).AnyTimes(). + Return(inspectResultOf(appCtr.ID, "prj-app-1", container.StateRunning, 0, nil), nil) + + err := svc.start(t.Context(), "prj", api.StartOptions{Project: project, Wait: true, WaitTimeout: 5 * time.Second}) + + if tt.err == "" { + assert.NilError(t, err) + } else { + assert.ErrorContains(t, err, tt.err) + } + }) + }) + } +} + +// TestStart_StartsTheReplicasOneAfterTheOther pins the order a service's +// stopped replicas start in: by replica number, each start waiting for the +// previous one's. +func TestStart_StartsTheReplicasOneAfterTheOther(t *testing.T) { + svc, apiClient, _ := newStartTestService(t) + web := types.ServiceConfig{Name: "web", ContainerSpec: types.ContainerSpec{Image: "alpine"}, Scale: intPtr(3)} + project := &types.Project{Name: "prj", Services: types.Services{"web": web}} + replicas := []container.Summary{ + stoppedServiceContainer("web", 3, container.StateExited), + stoppedServiceContainer("web", 1, container.StateExited), + stoppedServiceContainer("web", 2, container.StateExited), + } + apiClient.EXPECT().ContainerList(gomock.Any(), gomock.Any()).AnyTimes(). + Return(client.ContainerListResult{Items: replicas}, nil) + first := apiClient.EXPECT().ContainerStart(gomock.Any(), "prj-web-1-id", gomock.Any()).Return(client.ContainerStartResult{}, nil) + second := apiClient.EXPECT().ContainerStart(gomock.Any(), "prj-web-2-id", gomock.Any()).Return(client.ContainerStartResult{}, nil).After(first) + apiClient.EXPECT().ContainerStart(gomock.Any(), "prj-web-3-id", gomock.Any()).Return(client.ContainerStartResult{}, nil).After(second) + + assert.NilError(t, svc.start(t.Context(), "prj", api.StartOptions{Project: project})) +} + +// TestStart_WithoutAProjectStartsTheSelectedServicesOnly pins `compose start +// web` run without a compose file: the project is rebuilt from the project's +// containers, and only the services asked for (with what they depend on) are +// started, the other containers of the project staying as they are. +func TestStart_WithoutAProjectStartsTheSelectedServicesOnly(t *testing.T) { + svc, apiClient, _ := newStartTestService(t) + webCtr := stoppedServiceContainer("web", 1, container.StateExited) + jobCtr := stoppedServiceContainer("job", 1, container.StateExited) + apiClient.EXPECT().ContainerList(gomock.Any(), gomock.Any()).AnyTimes(). + Return(client.ContainerListResult{Items: []container.Summary{webCtr, jobCtr}}, nil) + // no expectation to start the job's container: it would fail the test + apiClient.EXPECT().ContainerStart(gomock.Any(), webCtr.ID, gomock.Any()).Return(client.ContainerStartResult{}, nil) + + assert.NilError(t, svc.start(t.Context(), "prj", api.StartOptions{AttachTo: []string{"web"}})) +} + +// TestStart_WithoutAProjectListsTheContainersWithoutTheOneOffs pins that the +// project `compose start` rebuilds from the containers it finds is made of the +// project's own containers only, never of the one-off ones (the ones `compose +// run` left behind): the listing it asks the daemon for leaves them out. +func TestStart_WithoutAProjectListsTheContainersWithoutTheOneOffs(t *testing.T) { + svc, apiClient, _ := newStartTestService(t) + webCtr := stoppedServiceContainer("web", 1, container.StateExited) + var firstListing client.ContainerListOptions + var listings atomic.Int32 + apiClient.EXPECT().ContainerList(gomock.Any(), gomock.Any()).AnyTimes(). + DoAndReturn(func(_ context.Context, opts client.ContainerListOptions) (client.ContainerListResult, error) { + if listings.Add(1) == 1 { + firstListing = opts + } + return client.ContainerListResult{Items: []container.Summary{webCtr}}, nil + }) + apiClient.EXPECT().ContainerStart(gomock.Any(), webCtr.ID, gomock.Any()).Return(client.ContainerStartResult{}, nil) + + assert.NilError(t, svc.start(t.Context(), "prj", api.StartOptions{})) + + assert.Assert(t, firstListing.Filters["label"][oneOffFilter(false)], "the listing must exclude one-off containers: %v", firstListing.Filters) +} diff --git a/pkg/compose/up_phases_test.go b/pkg/compose/up_phases_test.go index 664357360d..5c7503ebfd 100644 --- a/pkg/compose/up_phases_test.go +++ b/pkg/compose/up_phases_test.go @@ -23,6 +23,8 @@ import ( "context" "errors" "io" + "net" + "slices" "strings" "sync" "sync/atomic" @@ -30,6 +32,7 @@ import ( "time" "github.com/compose-spec/compose-go/v2/types" + dockercli "github.com/docker/cli/cli" "github.com/docker/cli/cli/config/configfile" "github.com/docker/cli/cli/streams" "github.com/moby/moby/api/types/container" @@ -74,12 +77,60 @@ type interactiveUpDaemon struct { slowSubscription bool started chan struct{} startedOnce sync.Once + // neverExits keeps the container running once started: no die event + neverExits bool + // startGate, when set, holds ContainerStart (after recording + // "start-requested") until it is closed, and then fails it with startErr + // if that is set + startGate chan struct{} + startErr error + // signals closes, the first time a call is recorded, the channel + // callSignal hands out for it + signals map[string]chan struct{} + // listErr, when set, fails the container listings made once the container + // is created + listErr error + // inspectState is the state the engine reports for the container; it + // reports it exited when unset + inspectState *container.State + // stopEndsStartedContainer makes the engine report the container dead when + // it is stopped once started; stopEndsContainer, whatever its state + stopEndsStartedContainer bool + stopEndsContainer bool + // exitCode is the exit code of the container when it exits, "0" if unset + exitCode string +} + +// callSignal returns a channel closed once call has been recorded, to wait for +// an engine call without polling. +func (d *interactiveUpDaemon) callSignal(call string) <-chan struct{} { + d.mu.Lock() + defer d.mu.Unlock() + return d.signalLocked(call) +} + +func (d *interactiveUpDaemon) signalLocked(call string) chan struct{} { + if d.signals == nil { + d.signals = map[string]chan struct{}{} + } + ch, ok := d.signals[call] + if !ok { + ch = make(chan struct{}) + d.signals[call] = ch + } + return ch } func (d *interactiveUpDaemon) record(call string) { d.mu.Lock() defer d.mu.Unlock() d.calls = append(d.calls, call) + ch := d.signalLocked(call) + select { + case <-ch: + default: + close(ch) + } } func (d *interactiveUpDaemon) recorded() []string { @@ -120,6 +171,9 @@ func expectInteractiveUpDaemon(t *testing.T, apiClient *mocks.MockAPIClient) *in if !d.created { return client.ContainerListResult{}, nil } + if d.listErr != nil { + return client.ContainerListResult{}, d.listErr + } return client.ContainerListResult{Items: []container.Summary{{ ID: "c1", Names: []string{"/test-web-1"}, Labels: labels, State: container.StateCreated, }}}, nil @@ -143,13 +197,19 @@ func expectInteractiveUpDaemon(t *testing.T, apiClient *mocks.MockAPIClient) *in return client.ContainerCreateResult{ID: "c1"}, nil }) apiClient.EXPECT().ContainerInspect(gomock.Any(), "c1", gomock.Any()).AnyTimes(). - Return(client.ContainerInspectResult{Container: container.InspectResponse{ - ID: "c1", - Name: "/test-web-1", - Config: &container.Config{Labels: labels}, - State: &container.State{Status: container.StateExited}, - NetworkSettings: &container.NetworkSettings{}, - }}, nil) + DoAndReturn(func(context.Context, string, client.ContainerInspectOptions) (client.ContainerInspectResult, error) { + state := d.inspectState + if state == nil { + state = &container.State{Status: container.StateExited} + } + return client.ContainerInspectResult{Container: container.InspectResponse{ + ID: "c1", + Name: "/test-web-1", + Config: &container.Config{Labels: labels}, + State: state, + NetworkSettings: &container.NetworkSettings{}, + }}, nil + }) // no attach endpoint: the log stream falls back to the logs API, which // the monitor-less assertions below don't read @@ -191,33 +251,70 @@ func expectInteractiveUpDaemon(t *testing.T, apiClient *mocks.MockAPIClient) *in }) // a teardown (the test giving up on a hung up) must not trip the mock - apiClient.EXPECT().ContainerStop(gomock.Any(), "c1", gomock.Any()).AnyTimes().Return(client.ContainerStopResult{}, nil) + apiClient.EXPECT().ContainerStop(gomock.Any(), "c1", gomock.Any()).AnyTimes(). + DoAndReturn(func(context.Context, string, client.ContainerStopOptions) (client.ContainerStopResult, error) { + d.record("stop") + if d.stopEndsContainer { + d.eventLog.emit(containerMessage(events.ActionDie, "c1", "test-web-1", "web", map[string]string{"exitCode": "143"})) + } else if d.stopEndsStartedContainer { + select { + case <-d.started: + d.eventLog.emit(containerMessage(events.ActionDie, "c1", "test-web-1", "web", map[string]string{"exitCode": "143"})) + default: + } + } + return client.ContainerStopResult{}, nil + }) apiClient.EXPECT().ContainerKill(gomock.Any(), "c1", gomock.Any()).AnyTimes().Return(client.ContainerKillResult{}, nil) // the container exits right after it starts apiClient.EXPECT().ContainerStart(gomock.Any(), "c1", gomock.Any()).AnyTimes(). DoAndReturn(func(context.Context, string, client.ContainerStartOptions) (client.ContainerStartResult, error) { - if d.startWaitsForEvents { - select { - case <-d.eventsSeen: - case <-time.After(5 * time.Second): - d.startedWithoutMonitor.Store(true) - } - } - d.record("start") - defer d.startedOnce.Do(func() { close(d.started) }) - d.eventLog.emit(containerMessage(events.ActionStart, "c1", "test-web-1", "web", labels)) - // The engine reports the exit some time after the start call, not - // within it. - timer := time.AfterFunc(containerExitDelay, func() { - d.eventLog.emit(containerMessage(events.ActionDie, "c1", "test-web-1", "web", map[string]string{"exitCode": "0"})) - }) - t.Cleanup(func() { timer.Stop() }) - return client.ContainerStartResult{}, nil + return client.ContainerStartResult{}, d.containerStart(t, labels) }) return d } +// containerStart is the engine starting the container: once the events +// subscription is open, if the test wants it so, and the start gate lifted, +// when it is set; it then emits the start event and, unless the container is +// to run for good, schedules its exit. +func (d *interactiveUpDaemon) containerStart(t *testing.T, labels map[string]string) error { + if d.startWaitsForEvents { + select { + case <-d.eventsSeen: + case <-time.After(5 * time.Second): + d.startedWithoutMonitor.Store(true) + } + } + if d.startGate != nil { + d.record("start-requested") + <-d.startGate + if d.startErr != nil { + return d.startErr + } + } + d.record("start") + // before the start event: whoever reacts to it (the session stopping a + // late starter) finds the container started + d.startedOnce.Do(func() { close(d.started) }) + d.eventLog.emit(containerMessage(events.ActionStart, "c1", "test-web-1", "web", labels)) + if d.neverExits { + return nil + } + // The engine reports the exit some time after the start call, not + // within it. + exitCode := d.exitCode + if exitCode == "" { + exitCode = "0" + } + timer := time.AfterFunc(containerExitDelay, func() { + d.eventLog.emit(containerMessage(events.ActionDie, "c1", "test-web-1", "web", map[string]string{"exitCode": exitCode})) + }) + t.Cleanup(func() { timer.Stop() }) + return nil +} + // progressRecorder records the lifetime of the progress scopes ("up") in the // daemon's call log, to order them against the engine calls. type progressRecorder struct{ d *interactiveUpDaemon } @@ -234,6 +331,9 @@ type interactiveUpHarness struct { svc *composeService daemon *interactiveUpDaemon out *bytes.Buffer + // api is the engine's client, for the expectations a test adds to the + // daemon's + api *mocks.MockAPIClient } func newInteractiveUpHarness(t *testing.T) *interactiveUpHarness { @@ -254,7 +354,7 @@ func newInteractiveUpHarness(t *testing.T) *interactiveUpHarness { daemon := expectInteractiveUpDaemon(t, apiClient) svc, err := NewComposeService(cli, WithEventProcessor(progressRecorder{daemon})) assert.NilError(t, err) - return &interactiveUpHarness{svc: svc.(*composeService), daemon: daemon, out: out} + return &interactiveUpHarness{svc: svc.(*composeService), daemon: daemon, out: out, api: apiClient} } func interactiveUpProject() *types.Project { @@ -280,9 +380,15 @@ func (h *interactiveUpHarness) up(t *testing.T) error { // upWith is up on a context the test owns, to cancel it at a given point. func (h *interactiveUpHarness) upWith(t *testing.T, ctx context.Context, cancel context.CancelFunc) error { + t.Helper() + return h.upWithOptions(t, ctx, cancel, interactiveUpOptions()) +} + +// upWithOptions is upWith for given options. +func (h *interactiveUpHarness) upWithOptions(t *testing.T, ctx context.Context, cancel context.CancelFunc, options api.UpOptions) error { t.Helper() done := make(chan error, 1) - go func() { done <- h.svc.Up(ctx, interactiveUpProject(), interactiveUpOptions()) }() + go func() { done <- h.svc.Up(ctx, interactiveUpProject(), options) }() select { case err := <-done: return err @@ -496,3 +602,265 @@ func TestUpInteractiveCreatePhaseCarriesThePlanProject(t *testing.T) { _, caller := project.Configs["#apisocket"] assert.Assert(t, !caller, "the caller's project is not transformed") } + +// TestUpInteractiveFailsOnTheStartPhaseErrorOfAnUninterruptedUp pins that +// nothing excuses a Start phase failing in an up nobody interrupted: it is the +// up's error (see TestUpInteractiveIgnoresTheStartPhaseErrorOfAnInterruptedUp +// for the case it is not). +func TestUpInteractiveFailsOnTheStartPhaseErrorOfAnUninterruptedUp(t *testing.T) { + h := newInteractiveUpHarness(t) + h.daemon.startGate = make(chan struct{}) + h.daemon.startErr = errors.New("cannot start the container") + close(h.daemon.startGate) + + err := h.up(t) + assert.ErrorContains(t, err, "cannot start the container") +} + +// TestUpInteractiveWatchesTheServicesItTargets pins which containers the +// session follows, and so ends with: those of the services the up targets +// (Start.Services) when it has some, those it attaches to otherwise. The +// container here never exits, and what is watched is a service with no +// container: the session ends as soon as the Start phase is over. Were the +// monitor watching the container too, the up would hang with it. +func TestUpInteractiveWatchesTheServicesItTargets(t *testing.T) { + tests := []struct { + name string + services []string + attachTo []string + }{ + {name: "the targeted services, not the attached ones", services: []string{"other"}, attachTo: []string{"web"}}, + {name: "the attached services when none is targeted", attachTo: []string{"other"}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + h := newInteractiveUpHarness(t) + h.daemon.neverExits = true + options := interactiveUpOptions() + options.Start.Services = tt.services + options.Start.AttachTo = tt.attachTo + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + + assert.NilError(t, h.upWithOptions(t, ctx, cancel, options)) + }) + } +} + +// TestUpReportsTheContainersItLeavesRunning pins that an up reports as Running +// the containers its plan won't touch, so that the progress display accounts +// for the whole project: the plan holds nothing for them. It is the +// preparation of the plan, which a detached and an interactive up share, that +// does it. +func TestUpReportsTheContainersItLeavesRunning(t *testing.T) { + svc, apiClient, recorder := newStartPhaseTestService(t) + apiClient.EXPECT().ImageInspect(gomock.Any(), "alpine", gomock.Any()).AnyTimes(). + Return(client.ImageInspectResult{InspectResponse: image.InspectResponse{ID: "sha256:alpine"}}, nil) + apiClient.EXPECT().NetworkList(gomock.Any(), gomock.Any()).AnyTimes().Return(client.NetworkListResult{}, nil) + apiClient.EXPECT().VolumeList(gomock.Any(), gomock.Any()).AnyTimes().Return(client.VolumeListResult{}, nil) + apiClient.EXPECT().ContainerList(gomock.Any(), gomock.Any()).Return(client.ContainerListResult{Items: []container.Summary{{ + ID: "c1", + Names: []string{"/test-web-1"}, + State: container.StateRunning, + Labels: map[string]string{ + api.ProjectLabel: "test", + api.ServiceLabel: "web", + api.OneoffLabel: "False", + api.ContainerNumberLabel: "1", + api.ConfigHashLabel: "whatever", + }, + }}}, nil).AnyTimes() + + // never recreate: the running container stays whatever its configuration + options := api.UpOptions{Create: api.CreateOptions{Recreate: api.RecreateNever}} + _, _, plan, err := svc.prepareUp(t.Context(), interactiveUpProject(), options) + + assert.NilError(t, err) + assert.Equal(t, len(plan.Nodes), 0, "the plan leaves the running container alone:\n%s", plan) + assert.DeepEqual(t, recorder.byID["Container test-web-1"], []string{api.StatusRunning}) +} + +// createdUpOf builds by hand what an interactive up carries from its Create +// phase to its Start phase, for a plan the test writes itself: a Create phase +// with nothing to do, and the Start phase of plan. It also returns the +// executor, to give it what the daemon's listing would have. +func (h *interactiveUpHarness) createdUpOf(t *testing.T, project *types.Project, plan *Plan) (*createdUp, *planExecutor) { + t.Helper() + exec := h.svc.newPlanExecutor(project, emptyObservedState(project.Name)) + run, err := exec.begin(plan) + assert.NilError(t, err) + assert.NilError(t, run.runCreate(t.Context())) + return &createdUp{project: project, run: run}, exec +} + +// runInteractiveUp runs the session of created, failing the test instead of +// hanging when it doesn't return. +func (h *interactiveUpHarness) runInteractiveUp(t *testing.T, created *createdUp) error { + t.Helper() + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + done := make(chan error, 1) + go func() { done <- h.svc.runInteractiveUp(ctx, created, interactiveUpOptions()) }() + select { + case err := <-done: + return err + case <-time.After(10 * time.Second): + cancel() + t.Fatalf("the session did not return; calls so far: %v", h.daemon.recorded()) + return nil + } +} + +func webContainerSummary() container.Summary { + return container.Summary{ID: "c1", Names: []string{"/test-web-1"}, Labels: map[string]string{api.ServiceLabel: "web"}} +} + +// TestUpInteractiveShowsTheHookLogsInTheSession pins that what the post_start +// hook of a service prints during an interactive up's Start phase reaches the +// session's log consumer, as the output of a container does: the session's +// printer is the listener the Start phase is given. +func TestUpInteractiveShowsTheHookLogsInTheSession(t *testing.T) { + h := newInteractiveUpHarness(t) + project := interactiveUpProject() + web := project.Services["web"] + web.PostStart = []types.ServiceHook{{Command: types.ShellCommand{"/notify.sh"}}} + project.Services["web"] = web + ctr := webContainerSummary() + + serverConn, clientConn := net.Pipe() + t.Cleanup(func() { + serverConn.Close() //nolint:errcheck + clientConn.Close() //nolint:errcheck + }) + // The write error is handed back to the test goroutine: reporting it from + // here could hit a test that has already finished. + writeErr := make(chan error, 1) + go func() { + writeErr <- writeStdcopyFrame(serverConn, 1, "hook says hello\n") + serverConn.Close() //nolint:errcheck + }() + h.api.EXPECT().ExecCreate(gomock.Any(), "c1", gomock.Any()).Return(client.ExecCreateResult{ID: "exec1"}, nil) + h.api.EXPECT().ExecAttach(gomock.Any(), "exec1", gomock.Any()). + Return(client.ExecAttachResult{HijackedResponse: client.NewHijackedResponse(clientConn, "")}, nil) + h.api.EXPECT().ExecInspect(gomock.Any(), "exec1", gomock.Any()).Return(client.ExecInspectResult{ExitCode: 0}, nil) + + plan := &Plan{} + start := plan.addNode(Operation{Type: OpStartContainer, ResourceID: "service:web:1", Cause: "start", Service: &web, Container: &ctr}, "start:web:1") + start.Phase = PhaseStart + post := plan.addNode(Operation{Type: OpRunPostStart, ResourceID: "service:web:1", Cause: "post_start hooks", Service: &web, Container: &ctr}, "start:web:1", start) + post.Phase = PhaseStart + created, _ := h.createdUpOf(t, project, plan) + + consumer := &testLogConsumer{} + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + options := interactiveUpOptions() + options.Start.Attach = consumer + done := make(chan error, 1) + go func() { done <- h.svc.runInteractiveUp(ctx, created, options) }() + select { + case err := <-done: + assert.NilError(t, err) + case <-time.After(10 * time.Second): + t.Fatalf("the session did not return; calls so far: %v", h.daemon.recorded()) + } + assert.NilError(t, <-writeErr) + + var logged []string + for _, lines := range consumer.logs { + logged = append(logged, lines...) + } + assert.Assert(t, slices.Contains(logged, "hook says hello"), "the hook's output did not reach the session: %v", consumer.logs) +} + +// TestUpInteractiveTranslatesAWaitTimeout pins what a depends_on wait that +// outlasts --wait-timeout makes of an interactive up: the dependency timeout +// the other commands report, not the composite error the executor builds to +// tell that wait's own deadline from any other. +func TestUpInteractiveTranslatesAWaitTimeout(t *testing.T) { + h := newInteractiveUpHarness(t) + project := interactiveUpProject() + web := project.Services["web"] + ctr := webContainerSummary() + h.api.EXPECT().ContainerInspect(gomock.Any(), "db-id", gomock.Any()).AnyTimes(). + Return(client.ContainerInspectResult{Container: container.InspectResponse{ + ID: "db-id", + Name: "/test-db-1", + Config: &container.Config{Healthcheck: &container.HealthConfig{Test: []string{"CMD", "true"}}}, + State: &container.State{Status: container.StateRunning, Health: &container.Health{Status: container.Starting}}, + }}, nil) + + plan := &Plan{} + wait := plan.addNode(Operation{Type: OpWaitCondition, ResourceID: "wait:db:service_healthy", Name: "db", Condition: types.ServiceConditionHealthy}, "") + wait.Phase = PhaseStart + start := plan.addNode(Operation{Type: OpStartContainer, ResourceID: "service:web:1", Cause: "start", Service: &web, Container: &ctr}, "start:web:1", wait) + start.Phase = PhaseStart + created, exec := h.createdUpOf(t, project, plan) + exec.waitTimeout = 50 * time.Millisecond + exec.containersByService["db"] = Containers{{ID: "db-id", Names: []string{"/test-db-1"}, Labels: map[string]string{api.ServiceLabel: "db"}}} + + err := h.runInteractiveUp(t, created) + + assert.Error(t, err, "timeout waiting for dependencies") + assert.Equal(t, h.daemon.indexOf("start"), -1, "web must not start before its dependency is healthy: %v", h.daemon.recorded()) +} + +// TestUpInteractiveFailedAttachEndsTheSession pins that an interactive up whose +// session cannot attach to the created containers fails with that error, starts +// nothing, and returns rather than waiting on the session it had begun to set +// up. +func TestUpInteractiveFailedAttachEndsTheSession(t *testing.T) { + h := newInteractiveUpHarness(t) + h.daemon.listErr = errors.New("cannot list the containers to attach to") + + err := h.up(t) + + assert.ErrorContains(t, err, "cannot list the containers to attach to") + assert.Equal(t, h.daemon.indexOf("start"), -1, "nothing may start: %v", h.daemon.recorded()) +} + +// TestPrepareUpBoundsEveryWaitByTheWaitTimeout pins that --wait-timeout reaches +// the executor of an up, detached or interactive: it bounds each depends_on +// wait of the plan, even without --wait. +func TestPrepareUpBoundsEveryWaitByTheWaitTimeout(t *testing.T) { + svc, apiClient, _ := newStartPhaseTestService(t) + apiClient.EXPECT().ImageInspect(gomock.Any(), "alpine", gomock.Any()).AnyTimes(). + Return(client.ImageInspectResult{InspectResponse: image.InspectResponse{ID: "sha256:alpine"}}, nil) + apiClient.EXPECT().NetworkList(gomock.Any(), gomock.Any()).AnyTimes().Return(client.NetworkListResult{}, nil) + apiClient.EXPECT().VolumeList(gomock.Any(), gomock.Any()).AnyTimes().Return(client.VolumeListResult{}, nil) + apiClient.EXPECT().ContainerList(gomock.Any(), gomock.Any()).AnyTimes().Return(client.ContainerListResult{}, nil) + + options := api.UpOptions{Start: api.StartOptions{WaitTimeout: 42 * time.Second}} + _, exec, _, err := svc.prepareUp(t.Context(), interactiveUpProject(), options) + + assert.NilError(t, err) + assert.Equal(t, exec.waitTimeout, 42*time.Second) +} + +// TestUpInteractiveExitsWithTheExitCodeOfTheServiceItIsToldTo pins what +// --exit-code-from does to an interactive up: the up ends with the exit code +// the container of the named service exited with, as a status error, where +// without it the up succeeds whatever the containers exited with. +func TestUpInteractiveExitsWithTheExitCodeOfTheServiceItIsToldTo(t *testing.T) { + t.Run("with --exit-code-from", func(t *testing.T) { + h := newInteractiveUpHarness(t) + h.daemon.exitCode = "3" + options := interactiveUpOptions() + options.Start.ExitCodeFrom = "web" + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + + err := h.upWithOptions(t, ctx, cancel, options) + + var status dockercli.StatusError + assert.Assert(t, errors.As(err, &status), "err: %v", err) + assert.Equal(t, status.StatusCode, 3) + }) + + t.Run("without it", func(t *testing.T) { + h := newInteractiveUpHarness(t) + h.daemon.exitCode = "3" + + assert.NilError(t, h.up(t)) + }) +} diff --git a/pkg/compose/up_signal_test.go b/pkg/compose/up_signal_test.go new file mode 100644 index 0000000000..4c96b6ecb2 --- /dev/null +++ b/pkg/compose/up_signal_test.go @@ -0,0 +1,144 @@ +//go:build !windows + +/* + 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 ( + "errors" + "os" + "os/signal" + "syscall" + "testing" + "time" + + "gotest.tools/v3/assert" +) + +// TestUpInteractiveDoesNotHandleSignalsDuringTheCreatePhase pins when an +// interactive up starts to handle SIGINT and SIGTERM itself: once its Create +// phase is over, not before. A signal met during the Create phase belongs to +// its caller, which interrupts the phase through the context; were the +// session already registered, the signal would be waiting in its channel when +// it starts and tear the application down at once, a graceful stop nobody +// asked for after the interruption had been dealt with. +// +// The signal is real: the test takes it too, so that it doesn't end the test +// process, and waits for it to be delivered. +func TestUpInteractiveDoesNotHandleSignalsDuringTheCreatePhase(t *testing.T) { + mine := make(chan os.Signal, 1) + signal.Notify(mine, syscall.SIGINT) + defer signal.Stop(mine) + + h := newInteractiveUpHarness(t) + h.daemon.onCreate = func() { + if err := syscall.Kill(os.Getpid(), syscall.SIGINT); err != nil { + t.Error(err) + return + } + select { + case <-mine: + case <-time.After(10 * time.Second): + t.Error("the signal was never delivered") + } + } + + assert.NilError(t, h.up(t)) + assert.Equal(t, h.daemon.indexOf("stop"), -1, "the application was stopped after a signal met during the Create phase: %v", h.daemon.recorded()) +} + +// TestUpInteractiveStopsAContainerThatStartsAfterTheTeardownSweep pins what a +// graceful teardown does with a container whose start was already in flight +// when it stopped the application: the one-shot sweep of the teardown misses +// it, since it was not up yet, and it comes up right after. The session stops +// it as it appears, rather than staying up for as long as it runs. +// +// The interruption is a real signal, the one the session itself handles (a +// canceled context would end the session's monitor with it): the test takes it +// too, so that it doesn't end the test process. +func TestUpInteractiveStopsAContainerThatStartsAfterTheTeardownSweep(t *testing.T) { + mine := make(chan os.Signal, 1) + signal.Notify(mine, syscall.SIGINT) + defer signal.Stop(mine) + + h := newInteractiveUpHarness(t) + h.daemon.startGate = make(chan struct{}) + h.daemon.neverExits = true + h.daemon.stopEndsStartedContainer = true + go func() { + select { + case <-h.daemon.callSignal("start-requested"): + case <-t.Context().Done(): + return + } + if err := syscall.Kill(os.Getpid(), syscall.SIGINT); err != nil { + t.Error(err) + return + } + // the sweep of the teardown, which finds the container not up yet + select { + case <-h.daemon.callSignal("stop"): + case <-t.Context().Done(): + return + } + // the start goes through right after it + close(h.daemon.startGate) + }() + + assert.NilError(t, h.up(t)) + assert.Equal(t, callsOf(h.daemon, "stop"), 2, "the container that started after the sweep must be stopped too: %v", h.daemon.recorded()) +} + +// TestUpInteractiveIgnoresTheStartPhaseErrorOfAnInterruptedUp pins that a +// Start phase failing while the up is being torn down is not the up's error: +// the Start phase runs on a context that outlives the interruption, so the +// graceful stop the interruption triggers can land on a container whose start +// is still in flight, and the start then fails because of it. +// +// The interruption is a real signal (see +// TestUpInteractiveStopsAContainerThatStartsAfterTheTeardownSweep). +func TestUpInteractiveIgnoresTheStartPhaseErrorOfAnInterruptedUp(t *testing.T) { + mine := make(chan os.Signal, 1) + signal.Notify(mine, syscall.SIGINT) + defer signal.Stop(mine) + + h := newInteractiveUpHarness(t) + h.daemon.startGate = make(chan struct{}) + h.daemon.startErr = errors.New("container start interrupted by the stop") + h.daemon.stopEndsContainer = true + go func() { + // the container start is in flight when the interruption comes, and + // fails only once the graceful stop it triggered went through + select { + case <-h.daemon.callSignal("start-requested"): + case <-t.Context().Done(): + return + } + if err := syscall.Kill(os.Getpid(), syscall.SIGINT); err != nil { + t.Error(err) + return + } + select { + case <-h.daemon.callSignal("stop"): + case <-t.Context().Done(): + return + } + close(h.daemon.startGate) + }() + + assert.NilError(t, h.up(t)) +}