From f30b176e7eb946bb8745a3db6b738cd477c55373 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Thu, 8 Oct 2026 13:11:43 +0200 Subject: [PATCH 1/6] feat(up): interactive up runs on a single plan Interactive up used to create() the project, then start() it: two daemon snapshots, the second built from the project as the caller gave it rather than the canonical one the Create phase ran on. It now prepares one Create+Start plan, like detached up. The Create phase runs under the "up" progress scope, then the attach/printer/ monitor session is set up exactly as before, and the Start phase is dispatched last, outside that scope, on a context that survives Ctrl+C, with the printer as the hook-log listener. A failed Create phase reaches neither the session nor the Start phase, and a dry run only ever runs the Create phase. The monitor's starting point is the daemon's time, which is read right before the Start phase: a daemon that answers with a time that can't be used fails the up with an explicit error, with no container started. One that can't be asked at all (a read-only socket proxy without the info endpoint) doesn't: the monitor then follows live events, with a warning. The session, the watcher and the teardown now use the plan's own project, which carries what preparation resolved. That makes use_api_socket work in a foreground up: its config file lives only in that project. A scenario locks it. Signed-off-by: Nicolas De Loof --- pkg/compose/monitor_test.go | 11 +- pkg/compose/run.go | 2 +- pkg/compose/start.go | 14 +- pkg/compose/start_test.go | 4 +- pkg/compose/up.go | 73 +++- pkg/compose/up_phases_test.go | 363 ++++++++++++++++++ .../compose.yaml | 6 + pkg/e2e/up_interactive_test.go | 13 + 8 files changed, 466 insertions(+), 20 deletions(-) create mode 100644 pkg/compose/up_phases_test.go create mode 100644 pkg/e2e/testdata/TestUpInteractiveInjectsAPISocketConfig/compose.yaml diff --git a/pkg/compose/monitor_test.go b/pkg/compose/monitor_test.go index b43258d1632..a3d1d5ccff2 100644 --- a/pkg/compose/monitor_test.go +++ b/pkg/compose/monitor_test.go @@ -518,6 +518,8 @@ type fakeEventLog struct { entries []loggedEvent // since records the Since of every subscription, in order since []string + // subscribers receive the events emitted after they subscribed + subscribers []chan events.Message // skew is how far the engine's clock is ahead of the local one skew time.Duration @@ -566,17 +568,22 @@ func (l *fakeEventLog) emit(msg events.Message) { now := l.now() msg.TimeNano = now.UnixNano() l.entries = append(l.entries, loggedEvent{at: now, msg: msg}) + for _, sub := range l.subscribers { + sub <- msg + } } // subscribe opens a stream replaying the buffered events newer than -// opts.Since (none when it is empty), followed by the given live events. +// opts.Since (none when it is empty), followed by the given live events, then +// by whatever is emitted from now on. func (l *fakeEventLog) subscribe(t *testing.T, opts client.EventsListOptions, live ...events.Message) client.EventsResult { t.Helper() l.mu.Lock() defer l.mu.Unlock() l.since = append(l.since, opts.Since) - messages := make(chan events.Message, len(l.entries)+len(live)) + messages := make(chan events.Message, len(l.entries)+len(live)+64) + l.subscribers = append(l.subscribers, messages) if opts.Since != "" { since := parseEventsSince(t, opts.Since) for _, e := range l.entries { diff --git a/pkg/compose/run.go b/pkg/compose/run.go index d0284350f59..177ae6e7810 100644 --- a/pkg/compose/run.go +++ b/pkg/compose/run.go @@ -315,7 +315,7 @@ func (s *composeService) startDependencies(ctx context.Context, project *types.P if len(project.Services) > 0 { return s.start(ctx, project.Name, api.StartOptions{ Project: project, - }, nil) + }) } return nil } diff --git a/pkg/compose/start.go b/pkg/compose/start.go index c04a64ea333..17bfabd6904 100644 --- a/pkg/compose/start.go +++ b/pkg/compose/start.go @@ -29,7 +29,7 @@ import ( func (s *composeService) Start(ctx context.Context, projectName string, options api.StartOptions) error { return Run(ctx, func(ctx context.Context) error { - return s.start(ctx, strings.ToLower(projectName), options, nil) + return s.start(ctx, strings.ToLower(projectName), options) }, "start", s.events) } @@ -39,12 +39,10 @@ func (s *composeService) Start(ctx context.Context, projectName string, options // other lifecycle command already uses. ScopeStart's planStartPhase works // straight off the observed containers (see TestPlanStart_StartOnlyScope) -- // it never converges anything, matching start()'s own historical contract of -// starting what exists, never creating. listener is non-nil only when called -// from interactive up's own start phase (today's create()-then-start() -// sequence, unchanged by this PR -- see upDetached's doc comment): it streams -// pre_start/post_start hook logs into the attached session, exactly what -// newPlanExecutor's listener parameter exists for. -func (s *composeService) start(ctx context.Context, projectName string, options api.StartOptions, listener api.ContainerEventListener) error { +// starting what exists, never creating. Interactive up does not go through +// here: it drives its own plan's Start phase (see runInteractiveUp), so there +// is no attached session for the hook logs to stream into. +func (s *composeService) start(ctx context.Context, projectName string, options api.StartOptions) error { project := options.Project if project == nil { containers, err := s.getContainers(ctx, projectName, oneOffExclude, true) @@ -88,7 +86,7 @@ func (s *composeService) start(ctx context.Context, projectName string, options // combination nothing rejects). exec.waitTimeout reproduces that for the // plan's own OpWaitCondition nodes, each with its own fresh window // starting when that wait begins -- see upDetached's identical reasoning. - exec := s.newPlanExecutor(project, observed, listener) + exec := s.newPlanExecutor(project, observed, nil) exec.waitTimeout = options.WaitTimeout if err := exec.run(ctx, plan); err != nil { diff --git a/pkg/compose/start_test.go b/pkg/compose/start_test.go index 3a82c3bde74..9d3a344e3f6 100644 --- a/pkg/compose/start_test.go +++ b/pkg/compose/start_test.go @@ -144,7 +144,7 @@ func TestStart_NoOpStartIsSilentAndOnlyListsContainers(t *testing.T) { stoppedServiceContainer("web", 1, container.StateRunning), }}, nil) - err := svc.start(t.Context(), "prj", api.StartOptions{Project: project}, nil) + err := svc.start(t.Context(), "prj", api.StartOptions{Project: project}) assert.NilError(t, err) assert.Equal(t, len(rec.summary()), 0, "a no-op start must be silent: %v", rec.summary()) } @@ -184,7 +184,7 @@ func TestStart_WaitTimeoutBoundsDependencyWaits(t *testing.T) { // failure instead of a hang ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second) defer cancel() - err := svc.start(ctx, "prj", api.StartOptions{Project: project, WaitTimeout: 200 * time.Millisecond}, nil) + err := svc.start(ctx, "prj", api.StartOptions{Project: project, WaitTimeout: 200 * time.Millisecond}) assert.Error(t, err, "timeout waiting for dependencies") } diff --git a/pkg/compose/up.go b/pkg/compose/up.go index 6df6487878d..850cf07065c 100644 --- a/pkg/compose/up.go +++ b/pkg/compose/up.go @@ -44,11 +44,14 @@ func (s *composeService) Up(ctx context.Context, project *types.Project, options if err := options.Validate(); err != nil { return err } + var created *createdUp err := Run(ctx, tracing.SpanWrapFunc("project/up", tracing.ProjectOptions(ctx, project), func(ctx context.Context) error { if options.Start.Attach == nil { return s.upDetached(ctx, project, options) } - return s.create(ctx, project, options.Create) + var err error + created, err = s.upCreatePhase(ctx, project, options) + return err }), "up", s.events) if err != nil { return err @@ -61,16 +64,52 @@ func (s *composeService) Up(ctx context.Context, project *types.Project, options _, _ = fmt.Fprintln(s.stdout(), "end of 'compose up' output, interactive run is not supported in dry-run mode") return err } - return s.runInteractiveUp(ctx, project, options) + return s.runInteractiveUp(ctx, created, options) +} + +// createdUp is what an interactive up carries from its Create phase to its +// Start phase: the canonical project the plan was built for, and the plan run +// whose Start phase is still to dispatch. Both phases share the one daemon +// snapshot and the one plan the Create phase was prepared from. +type createdUp struct { + project *types.Project + run *planRun +} + +// upCreatePhase prepares interactive up's single Create+Start plan and runs +// its Create phase only, under the "up" progress scope the caller wraps it in. +// The Start phase is deliberately left out of that scope: it runs once +// runInteractiveUp has set up the session streaming logs to the terminal, and +// the progress display's refresh would repaint over them. +func (s *composeService) upCreatePhase(ctx context.Context, project *types.Project, options api.UpOptions) (*createdUp, error) { + project, observed, plan, err := s.preparePlan(ctx, project, options.Create, ScopeCreateStart) + if err != nil { + return nil, err + } + + // Must run against the pre-execution snapshot, see upDetached. + emitRunningEvents(project, observed, plan, s.events) + + // Same per-wait --wait-timeout window as upDetached, see there. + exec := s.newPlanExecutor(project, observed, nil) + exec.waitTimeout = options.Start.WaitTimeout + + run, err := exec.begin(plan) + if err != nil { + return nil, err + } + if err := run.runCreate(ctx); err != nil { + return nil, err + } + return &createdUp{project: project, run: run}, nil } // upDetached runs detached `up`'s Create and Start phases as a single plan: // the semantic switchover from create() + start() -- two separate daemon // snapshots, the second blind to what the first just did -- to one // preparePlan/executePlan pass covering both phases (epic #14081, lot 2). -// Interactive up keeps the create()+start() sequence for now: its own -// create/start phase boundary (attach/printer/monitor setup in between) is a -// separate, riskier item of the same lot. +// Interactive up prepares the same plan but drives its two phases itself, +// with the attach/printer/monitor setup in between (see upCreatePhase). func (s *composeService) upDetached(ctx context.Context, project *types.Project, options api.UpOptions) error { project, observed, plan, err := s.preparePlan(ctx, project, options.Create, ScopeCreateStart) if err != nil { @@ -181,7 +220,15 @@ type upSession struct { exitCode int } -func (s *composeService) runInteractiveUp(ctx context.Context, project *types.Project, options api.UpOptions) error { +// runInteractiveUp sets up the attach/printer/monitor session around the +// Start phase of created's plan, runs it, then follows the application until +// it terminates. The Create phase already ran, and signal.Notify only comes +// after it: Ctrl+C during Create keeps its default behavior. +func (s *composeService) runInteractiveUp(ctx context.Context, created *createdUp, options api.UpOptions) error { + // the plan's own project, not the caller's: the one carrying what + // preparePlan resolved (see useAPISocket) + project := created.project + // if we get a second signal during shutdown, we kill the services // immediately, so the channel needs to have sufficient capacity or // we might miss a signal while setting up the second channel read @@ -283,6 +330,17 @@ func (s *composeService) runInteractiveUp(ctx context.Context, project *types.Pr } monitor.withListener(u.followStartedContainers(attached)) + // The monitor replays events from the daemon's time. Read it now, before + // the Start phase: the point precedes the first start, and a daemon that + // answers with a time that can't be used fails the command without any + // container started. A daemon that can't be asked doesn't (see + // readDaemonTime): the monitor then follows live events. + if err := monitor.readDaemonTime(globalCtx); err != nil { + cancel() + _ = u.eg.Wait() + return err + } + u.eg.Go(func() error { err := monitor.Start(globalCtx) // The monitor returning means every watched container is gone for @@ -310,7 +368,8 @@ func (s *composeService) runInteractiveUp(ctx context.Context, project *types.Pr }) // We use the parent context without cancellation as we manage sigterm to stop the stack - err = s.start(context.WithoutCancel(ctx), project.Name, options.Start, u.printer.HandleEvent) + err = created.run.runStart(context.WithoutCancel(ctx), u.printer.HandleEvent) + err = translateWaitTimeout(err) if err != nil && !u.isTerminated.Load() { // Ignore error if the process is terminated cancel() _ = u.eg.Wait() diff --git a/pkg/compose/up_phases_test.go b/pkg/compose/up_phases_test.go new file mode 100644 index 00000000000..b547cbb835b --- /dev/null +++ b/pkg/compose/up_phases_test.go @@ -0,0 +1,363 @@ +//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 ( + "bytes" + "context" + "errors" + "io" + "strings" + "sync" + "testing" + "time" + + "github.com/compose-spec/compose-go/v2/types" + "github.com/docker/cli/cli/config/configfile" + "github.com/docker/cli/cli/streams" + "github.com/moby/moby/api/types/container" + "github.com/moby/moby/api/types/events" + "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" + "github.com/docker/compose/v5/pkg/mocks" +) + +// containerExitDelay is how long after its start the test container exits +const containerExitDelay = 100 * time.Millisecond + +// interactiveUpDaemon is the engine as an interactive `up` of a one-service +// project sees it: it records the order of the calls that matter, and +// reproduces the engine's event log (see fakeEventLog) so a container that +// exits as soon as it starts is observable whenever the monitor subscribes. +type interactiveUpDaemon struct { + mu sync.Mutex + calls []string + created bool + eventLog *fakeEventLog + // createErr, when set, fails ContainerCreate + createErr error + // slowSubscription delays the completion of the events subscription well + // past the moment the container has started and exited + slowSubscription bool + started chan struct{} + startedOnce sync.Once +} + +func (d *interactiveUpDaemon) record(call string) { + d.mu.Lock() + defer d.mu.Unlock() + d.calls = append(d.calls, call) +} + +func (d *interactiveUpDaemon) recorded() []string { + d.mu.Lock() + defer d.mu.Unlock() + return append([]string(nil), d.calls...) +} + +func (d *interactiveUpDaemon) indexOf(call string) int { + for i, c := range d.recorded() { + if c == call { + return i + } + } + return -1 +} + +func expectInteractiveUpDaemon(t *testing.T, apiClient *mocks.MockAPIClient) *interactiveUpDaemon { + t.Helper() + d := &interactiveUpDaemon{eventLog: &fakeEventLog{}, started: make(chan struct{})} + labels := map[string]string{ + api.ProjectLabel: "test", + api.ServiceLabel: "web", + api.OneoffLabel: "False", + api.ContainerNumberLabel: "1", + api.ConfigHashLabel: "hash", + } + + 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(). + DoAndReturn(func(context.Context, client.ContainerListOptions) (client.ContainerListResult, error) { + d.mu.Lock() + defer d.mu.Unlock() + if !d.created { + return client.ContainerListResult{}, nil + } + return client.ContainerListResult{Items: []container.Summary{{ + ID: "c1", Names: []string{"/test-web-1"}, Labels: labels, State: container.StateCreated, + }}}, nil + }) + apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()).AnyTimes(). + DoAndReturn(func(context.Context, client.ContainerCreateOptions) (client.ContainerCreateResult, error) { + d.record("create") + if d.createErr != nil { + return client.ContainerCreateResult{}, d.createErr + } + d.mu.Lock() + d.created = true + d.mu.Unlock() + 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) + + // no attach endpoint: the log stream falls back to the logs API, which + // the monitor-less assertions below don't read + apiClient.EXPECT().ContainerAttach(gomock.Any(), "c1", gomock.Any()).AnyTimes(). + DoAndReturn(func(context.Context, string, client.ContainerAttachOptions) (client.ContainerAttachResult, error) { + d.record("attach") + return client.ContainerAttachResult{}, errors.New("attach not supported") + }) + apiClient.EXPECT().ContainerLogs(gomock.Any(), "c1", gomock.Any()).AnyTimes(). + Return(io.NopCloser(strings.NewReader("")), nil) + + // the monitor reads the daemon's time to know from when to ask for events + apiClient.EXPECT().Info(gomock.Any(), gomock.Any()).AnyTimes(). + DoAndReturn(func(context.Context, client.InfoOptions) (client.SystemInfoResult, error) { + return d.eventLog.info() + }) + apiClient.EXPECT().Events(gomock.Any(), gomock.Any()).AnyTimes(). + DoAndReturn(func(_ context.Context, opts client.EventsListOptions) client.EventsResult { + d.record("events") + if d.slowSubscription { + select { + case <-d.started: + time.Sleep(containerExitDelay + 200*time.Millisecond) + case <-time.After(5 * time.Second): + } + } + return d.eventLog.subscribe(t, opts) + }) + + // 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().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) { + 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: long enough here for the monitor, launched just + // before the Start phase, to have taken its timestamp. + 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 d +} + +// 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 } + +func (p progressRecorder) Start(_ context.Context, operation string) { + p.d.record("progress-start:" + operation) +} + +func (p progressRecorder) Done(operation string, _ bool) { p.d.record("progress-done:" + operation) } +func (p progressRecorder) On(...api.Resource) {} + +// interactiveUpHarness is a compose service wired to the interactiveUpDaemon. +type interactiveUpHarness struct { + svc *composeService + daemon *interactiveUpDaemon + out *bytes.Buffer +} + +func newInteractiveUpHarness(t *testing.T) *interactiveUpHarness { + t.Helper() + mockCtrl := gomock.NewController(t) + cli := mocks.NewMockCli(mockCtrl) + apiClient := mocks.NewMockAPIClient(mockCtrl) + out := &bytes.Buffer{} + cli.EXPECT().Client().Return(apiClient).AnyTimes() + cli.EXPECT().ConfigFile().Return(&configfile.ConfigFile{}).AnyTimes() + cli.EXPECT().Out().Return(streams.NewOut(out)).AnyTimes() + cli.EXPECT().Err().Return(streams.NewOut(io.Discard)).AnyTimes() + apiClient.EXPECT().DaemonHost().Return("unix:///var/run/docker.sock").AnyTimes() + apiClient.EXPECT().Ping(gomock.Any(), client.PingOptions{NegotiateAPIVersion: true}). + Return(client.PingResult{APIVersion: "1.44"}, nil).AnyTimes() + apiClient.EXPECT().ClientVersion().Return("1.44").AnyTimes() + + daemon := expectInteractiveUpDaemon(t, apiClient) + svc, err := NewComposeService(cli, WithEventProcessor(progressRecorder{daemon})) + assert.NilError(t, err) + return &interactiveUpHarness{svc: svc.(*composeService), daemon: daemon, out: out} +} + +func interactiveUpProject() *types.Project { + return &types.Project{ + Name: "test", + Services: types.Services{"web": {Name: "web", ContainerSpec: types.ContainerSpec{Image: "alpine"}}}, + Networks: types.Networks{}, + Volumes: types.Volumes{}, + } +} + +func interactiveUpOptions() api.UpOptions { + return api.UpOptions{Start: api.StartOptions{Attach: &testLogConsumer{}, AttachTo: []string{"web"}}} +} + +// up runs Up, failing the test instead of hanging when it doesn't return. +func (h *interactiveUpHarness) up(t *testing.T) error { + t.Helper() + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + done := make(chan error, 1) + go func() { done <- h.svc.Up(ctx, interactiveUpProject(), interactiveUpOptions()) }() + select { + case err := <-done: + return err + case <-time.After(10 * time.Second): + cancel() + select { + case <-done: + case <-time.After(5 * time.Second): + } + t.Fatalf("up did not return; calls so far: %v", h.daemon.recorded()) + return nil + } +} + +// TestUpInteractivePhaseOrder pins the sequence of an interactive up: the +// Create phase runs under the "up" progress scope and completes, the session +// then attaches to the created container, and only then is the Start phase +// dispatched, outside that scope (whose refresh would repaint over the logs). +func TestUpInteractivePhaseOrder(t *testing.T) { + h := newInteractiveUpHarness(t) + + assert.NilError(t, h.up(t)) + + d := h.daemon + create, done, attach, start := d.indexOf("create"), d.indexOf("progress-done:up"), d.indexOf("attach"), d.indexOf("start") + assert.Assert(t, create >= 0 && done >= 0 && attach >= 0 && start >= 0, "calls: %v", d.recorded()) + assert.Assert(t, create < done, "the container is created within the up progress scope: %v", d.recorded()) + assert.Assert(t, done < attach, "the session attaches once the up progress scope is over: %v", d.recorded()) + assert.Assert(t, attach < start, "no container starts before the session is attached: %v", d.recorded()) +} + +// TestUpInteractiveSeesContainerExitingBeforeSubscription is the monitor's +// startup race seen from interactive up: the Start phase runs right after the +// monitor is launched, so the container may start and exit before the event +// subscription is effective (modeled here as the subscription only completing +// once the container has started). The exit must still be observed, or up +// would wait on the container forever. The replay starts at the daemon's time, +// read before the container listing: the daemon's clock is set here well apart +// from the client's, as it is for a remote daemon, so that the events it +// stamps are only replayed when asked for from its own time. +func TestUpInteractiveSeesContainerExitingBeforeSubscription(t *testing.T) { + h := newInteractiveUpHarness(t) + h.daemon.slowSubscription = true + h.daemon.eventLog.skew = -time.Hour + + assert.NilError(t, h.up(t)) + + log := h.daemon.eventLog + assert.Equal(t, len(log.since), 1) + assert.Assert(t, log.since[0] != "", "the events subscription must replay what preceded it") + reported, err := time.Parse(time.RFC3339Nano, log.reportedTime) + assert.NilError(t, err) + assert.Assert(t, parseEventsSince(t, log.since[0]).Equal(reported), + "since %s is not the daemon time %s", log.since[0], log.reportedTime) +} + +// TestUpInteractiveFailsWithoutDaemonTime: the monitor replays events from the +// daemon's time, and a daemon that answers with a time that can't be used fails +// the up, with that explicit error, before the Start phase: no container is +// started, and no event subscription was ever opened. +func TestUpInteractiveFailsWithoutDaemonTime(t *testing.T) { + tests := []struct { + name string + setup func(*fakeEventLog) + }{ + {name: "system time is empty", setup: func(log *fakeEventLog) { log.setSystemTime("") }}, + {name: "system time is not a time", setup: func(log *fakeEventLog) { log.setSystemTime("yesterday-ish") }}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + h := newInteractiveUpHarness(t) + tt.setup(h.daemon.eventLog) + + err := h.up(t) + + assert.ErrorContains(t, err, "reading the daemon time to subscribe to container events") + calls := h.daemon.recorded() + assert.Assert(t, h.daemon.indexOf("create") >= 0, "the Create phase ran: %v", calls) + assert.Equal(t, h.daemon.indexOf("start"), -1, "no container is started: %v", calls) + assert.Equal(t, h.daemon.indexOf("events"), -1, "no event subscription is opened: %v", calls) + }) + } +} + +// TestUpInteractiveWithoutInfoStillStarts: a daemon that can't be asked for its +// time (a read-only socket proxy that doesn't expose the info endpoint) doesn't +// fail the up, which worked without it: the containers start, and the monitor +// subscribes to live events, with no starting point. +func TestUpInteractiveWithoutInfoStillStarts(t *testing.T) { + h := newInteractiveUpHarness(t) + h.daemon.eventLog.infoErr = errors.New("403 Forbidden") + + assert.NilError(t, h.up(t)) + + assert.Assert(t, h.daemon.indexOf("start") >= 0, "the Start phase ran: %v", h.daemon.recorded()) + assert.DeepEqual(t, h.daemon.eventLog.since, []string{""}) +} + +// TestUpInteractiveFailedCreateStopsThere: a Create phase that fails reaches +// neither the attach session nor the Start phase. +func TestUpInteractiveFailedCreateStopsThere(t *testing.T) { + h := newInteractiveUpHarness(t) + h.daemon.createErr = errors.New("no space left on device") + + err := h.up(t) + + assert.ErrorContains(t, err, "no space left on device") + assert.DeepEqual(t, h.daemon.recorded(), []string{"progress-start:up", "create", "progress-done:up"}) +} + +// TestUpInteractiveDryRunOnlyCreates: a dry-run interactive up plans the +// Create phase and never gets to the session or the Start phase. +func TestUpInteractiveDryRunOnlyCreates(t *testing.T) { + h := newInteractiveUpHarness(t) + h.svc.dryRun = true + + assert.NilError(t, h.up(t)) + + assert.DeepEqual(t, h.daemon.recorded(), []string{"progress-start:up", "create", "progress-done:up"}) + assert.Assert(t, strings.Contains(h.out.String(), "interactive run is not supported in dry-run mode"), "output: %q", h.out.String()) +} diff --git a/pkg/e2e/testdata/TestUpInteractiveInjectsAPISocketConfig/compose.yaml b/pkg/e2e/testdata/TestUpInteractiveInjectsAPISocketConfig/compose.yaml new file mode 100644 index 00000000000..e0a2d019a55 --- /dev/null +++ b/pkg/e2e/testdata/TestUpInteractiveInjectsAPISocketConfig/compose.yaml @@ -0,0 +1,6 @@ +services: + app: + image: alpine + use_api_socket: true + command: > + sh -c 'if [ -f /run/secrets/docker/config.json ]; then echo apisocket-config-present; else echo apisocket-config-absent; fi' diff --git a/pkg/e2e/up_interactive_test.go b/pkg/e2e/up_interactive_test.go index 00b6d3a7d78..d3ecd1cf9c8 100644 --- a/pkg/e2e/up_interactive_test.go +++ b/pkg/e2e/up_interactive_test.go @@ -94,6 +94,19 @@ func TestUpInteractiveDependencyTimeout(t *testing.T) { ServiceState("web", "created")) } +// use_api_socket is applied by rewriting the project (a #apisocket config the +// service mounts at /run/secrets/docker/config.json): a foreground up must +// start the containers from that same rewritten project, as a detached one +// does. The file is injected whether or not any credential is configured. +func TestUpInteractiveInjectsAPISocketConfig(t *testing.T) { + NewScenario(t, "a foreground up must give a use_api_socket service its docker config file, as a detached up does"). + Step("the service finds the config file in its session output", + ComposeCmd("up", "--menu=false").Within(60*time.Second), + StdoutContains("apisocket-config-present"), + OutputNotContains("apisocket-config-absent"), + ServiceState("app", "exited")) +} + func TestUpInteractiveDryRun(t *testing.T) { // the progress lines name the container after the project s := NewScenario(t, "a foreground up --dry-run must report the container creation, never start it and leave nothing behind") From 8aee064b791e32ded4d40b8d78309134755a5566 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Fri, 16 Oct 2026 11:25:11 +0200 Subject: [PATCH 2/6] test(e2e): a foreground up ignores the exit of a pre_start runner A pre_start runner carries the project and service labels but not the one-off label the monitor's event filter requires, so its exit never reaches the session. Nothing locked that: a runner exiting successfully would otherwise win over the service's own exit code under --exit-code-from, or abort the application under --abort-on-container-exit. Signed-off-by: Nicolas De Loof --- .../TestUpInteractiveIgnoresHookRunnerExit/compose.yaml | 7 +++++++ pkg/e2e/up_interactive_test.go | 9 +++++++++ 2 files changed, 16 insertions(+) create mode 100644 pkg/e2e/testdata/TestUpInteractiveIgnoresHookRunnerExit/compose.yaml diff --git a/pkg/e2e/testdata/TestUpInteractiveIgnoresHookRunnerExit/compose.yaml b/pkg/e2e/testdata/TestUpInteractiveIgnoresHookRunnerExit/compose.yaml new file mode 100644 index 00000000000..39e7c67fced --- /dev/null +++ b/pkg/e2e/testdata/TestUpInteractiveIgnoresHookRunnerExit/compose.yaml @@ -0,0 +1,7 @@ +services: + app: + image: alpine + command: sh -c 'sleep 2; exit 3' + pre_start: + - image: alpine + command: echo hook-ran diff --git a/pkg/e2e/up_interactive_test.go b/pkg/e2e/up_interactive_test.go index d3ecd1cf9c8..032288f76e3 100644 --- a/pkg/e2e/up_interactive_test.go +++ b/pkg/e2e/up_interactive_test.go @@ -36,6 +36,15 @@ func TestUpInteractiveHookLogs(t *testing.T) { ServiceState("app", "exited")) } +func TestUpInteractiveIgnoresHookRunnerExit(t *testing.T) { + NewScenario(t, "a foreground up must not take a pre_start runner's exit for the application's"). + Step("up reports the exit code of the service, not the successful exit of its hook", + ComposeCmd("up", "--menu=false", "--exit-code-from", "app").MayFail().Within(60*time.Second), + ExitCode(3), + OutputContains("hook-ran"), + OutputContains("app-1 exited with code 3")) +} + func TestUpInteractiveImmediateExit(t *testing.T) { NewScenario(t, "a foreground up --abort-on-container-exit must notice a container that exits as soon as it starts"). Step("up aborts on the instantly exiting container instead of waiting for the long-running one", From 36761c6d84cc560c1364394722da73c21964d407 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Mon, 26 Oct 2026 13:10:24 +0100 Subject: [PATCH 3/6] refactor(up): settle the review of interactive up's single plan Production: - detached and interactive up prepare their plan the same way: prepareUp holds the preparation and the --wait-timeout rationale both share, so the two cannot diverge; - useAPISocket leaves alone a project it already transformed. The session, the watcher and the teardown now work on the plan's own project, which a watch rebuild hands to a later plan: transforming it again mounted the socket and the config file twice; - the docs say what the code does: an interruption met while reading the daemon's time is returned wrapping the context's error (the CLI maps it to its interruption exit code), Up ignores StartOptions.Project and does not verify StartOptions.Wait in the foreground, and the Start phase's listener goes through runStart. Tests: - the Start phase is shown to survive the caller's cancellation, and an interruption before it to start nothing; - the dry-run test no longer claims to exercise the dry-run client: it pins the gate of Up; - the phase order test also checks the "up" scope opens before the create; - useAPISocket's idempotence is tested. Signed-off-by: Nicolas De Loof --- pkg/api/api.go | 7 ++- pkg/compose/apiSocket.go | 6 +++ pkg/compose/apiSocket_test.go | 58 ++++++++++++++++++++++++ pkg/compose/monitor.go | 3 +- pkg/compose/start.go | 6 +-- pkg/compose/up.go | 45 ++++++++++--------- pkg/compose/up_phases_test.go | 83 +++++++++++++++++++++++++++++------ 7 files changed, 169 insertions(+), 39 deletions(-) create mode 100644 pkg/compose/apiSocket_test.go diff --git a/pkg/api/api.go b/pkg/api/api.go index d4c9b79fbe4..98516ebadf4 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -364,7 +364,8 @@ type CreateOptions struct { // StartOptions group options of the Start API type StartOptions struct { - // Project is the compose project used to define this app. Might be nil if user ran command just with project name + // Project is the compose project used to define this app. Might be nil if user ran command just with project name. + // Ignored by Up, which works on the project it is given. Project *types.Project // Attach receives the containers' logs during Up's foreground session. // It doubles as the mode switch: when nil, Up returns once containers @@ -382,7 +383,9 @@ type StartOptions struct { // ExitCodeFrom reports the exit code of the specified service. Honored // by Up's foreground session only; ignored by Start. ExitCodeFrom string - // Wait won't return until containers reached the running|healthy state + // Wait won't return until containers reached the running|healthy state. + // Honored by a detached Up (Attach nil); a foreground Up is not followed + // by this verification. Wait bool WaitTimeout time.Duration // Services names the services the user explicitly targeted; it only diff --git a/pkg/compose/apiSocket.go b/pkg/compose/apiSocket.go index ddc7a029030..d9442be5a45 100644 --- a/pkg/compose/apiSocket.go +++ b/pkg/compose/apiSocket.go @@ -40,6 +40,12 @@ func (s *composeService) useAPISocket(project *types.Project) (*types.Project, e if !useAPISocket { return project, nil } + // The project already went through here, as the plan's own project does + // when it is handed to a later plan (a watch rebuild): transforming it a + // second time would mount the socket and the config file twice. + if _, applied := project.Configs["#apisocket"]; applied { + return project, nil + } if s.getContextInfo().ServerOSType() == "windows" { return nil, errors.New("use_api_socket can't be used with a Windows Docker Engine") diff --git a/pkg/compose/apiSocket_test.go b/pkg/compose/apiSocket_test.go new file mode 100644 index 00000000000..eb1ece2e2ab --- /dev/null +++ b/pkg/compose/apiSocket_test.go @@ -0,0 +1,58 @@ +/* + 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 ( + "testing" + + "github.com/compose-spec/compose-go/v2/types" + "gotest.tools/v3/assert" +) + +// linuxContextInfo is the context of a Linux engine. +type linuxContextInfo struct{} + +func (linuxContextInfo) CurrentContext() string { return "default" } +func (linuxContextInfo) ServerOSType() string { return "linux" } +func (linuxContextInfo) BuildKitEnabled() (bool, error) { return false, nil } + +// TestUseAPISocketIsIdempotent verifies that a project which already went +// through useAPISocket -- the plan's own project, handed to a later plan such +// as a watch rebuild -- is not transformed a second time: the socket and the +// config file would be mounted twice. +func TestUseAPISocketIsIdempotent(t *testing.T) { + svc, _ := newTestService(t, WithContextInfo(linuxContextInfo{})) + project := &types.Project{ + Name: "test", + Services: types.Services{ + "app": {Name: "app", ContainerSpec: types.ContainerSpec{UseAPISocket: true, Environment: types.MappingWithEquals{}}}, + }, + Configs: types.Configs{}, + } + + once, err := svc.useAPISocket(project) + assert.NilError(t, err) + service := once.Services["app"] + assert.Equal(t, len(service.Volumes), 1, "volumes: %v", service.Volumes) + assert.Equal(t, len(service.Configs), 1, "configs: %v", service.Configs) + + twice, err := svc.useAPISocket(once) + assert.NilError(t, err) + service = twice.Services["app"] + assert.Equal(t, len(service.Volumes), 1, "volumes: %v", service.Volumes) + assert.Equal(t, len(service.Configs), 1, "configs: %v", service.Configs) +} diff --git a/pkg/compose/monitor.go b/pkg/compose/monitor.go index 1b3bf10a196..ac5e568527f 100644 --- a/pkg/compose/monitor.go +++ b/pkg/compose/monitor.go @@ -226,7 +226,8 @@ func (c *monitor) Start(ctx context.Context) error { // capability, not a reason to fail. A daemon that answers with no time or one // that can't be parsed is an error: the replay would have no sound starting // point, and nothing explains the answer. An interruption is an error too, -// for the caller to end quietly. +// wrapping the context's: not a failure of the daemon, for the caller to +// report as the interruption it is. func (c *monitor) readDaemonTime(ctx context.Context) error { if c.sinceRead { return nil diff --git a/pkg/compose/start.go b/pkg/compose/start.go index 17bfabd6904..5ae05a1ddcc 100644 --- a/pkg/compose/start.go +++ b/pkg/compose/start.go @@ -39,9 +39,9 @@ func (s *composeService) Start(ctx context.Context, projectName string, options // other lifecycle command already uses. ScopeStart's planStartPhase works // straight off the observed containers (see TestPlanStart_StartOnlyScope) -- // it never converges anything, matching start()'s own historical contract of -// starting what exists, never creating. Interactive up does not go through -// here: it drives its own plan's Start phase (see runInteractiveUp), so there -// is no attached session for the hook logs to stream into. +// starting what exists, never creating. The Start phase of an interactive up +// does not go through here: it runs from the up's own plan, and hands its +// session's listener to runStart (see runInteractiveUp). func (s *composeService) start(ctx context.Context, projectName string, options api.StartOptions) error { project := options.Project if project == nil { diff --git a/pkg/compose/up.go b/pkg/compose/up.go index 850cf07065c..19a61365265 100644 --- a/pkg/compose/up.go +++ b/pkg/compose/up.go @@ -82,18 +82,11 @@ type createdUp struct { // runInteractiveUp has set up the session streaming logs to the terminal, and // the progress display's refresh would repaint over them. func (s *composeService) upCreatePhase(ctx context.Context, project *types.Project, options api.UpOptions) (*createdUp, error) { - project, observed, plan, err := s.preparePlan(ctx, project, options.Create, ScopeCreateStart) + project, exec, plan, err := s.prepareUp(ctx, project, options) if err != nil { return nil, err } - // Must run against the pre-execution snapshot, see upDetached. - emitRunningEvents(project, observed, plan, s.events) - - // Same per-wait --wait-timeout window as upDetached, see there. - exec := s.newPlanExecutor(project, observed, nil) - exec.waitTimeout = options.Start.WaitTimeout - run, err := exec.begin(plan) if err != nil { return nil, err @@ -104,16 +97,13 @@ func (s *composeService) upCreatePhase(ctx context.Context, project *types.Proje return &createdUp{project: project, run: run}, nil } -// upDetached runs detached `up`'s Create and Start phases as a single plan: -// the semantic switchover from create() + start() -- two separate daemon -// snapshots, the second blind to what the first just did -- to one -// preparePlan/executePlan pass covering both phases (epic #14081, lot 2). -// Interactive up prepares the same plan but drives its two phases itself, -// with the attach/printer/monitor setup in between (see upCreatePhase). -func (s *composeService) upDetached(ctx context.Context, project *types.Project, options api.UpOptions) error { +// prepareUp prepares the single Create+Start plan of an up, detached or +// interactive, and the executor that runs it. It returns the plan's own +// project, the one carrying what preparation resolved (see useAPISocket). +func (s *composeService) prepareUp(ctx context.Context, project *types.Project, options api.UpOptions) (*types.Project, *planExecutor, *Plan, error) { project, observed, plan, err := s.preparePlan(ctx, project, options.Create, ScopeCreateStart) if err != nil { - return err + return nil, nil, nil, err } // Must run against the pre-execution snapshot: observed only labels a @@ -137,6 +127,20 @@ func (s *composeService) upDetached(ctx context.Context, project *types.Project, // per-wait-independent behavior never had that failure mode. exec := s.newPlanExecutor(project, observed, nil) exec.waitTimeout = options.Start.WaitTimeout + return project, exec, plan, nil +} + +// upDetached runs detached `up`'s Create and Start phases as a single plan: +// the semantic switchover from create() + start() -- two separate daemon +// snapshots, the second blind to what the first just did -- to one +// preparePlan/executePlan pass covering both phases (epic #14081, lot 2). +// Interactive up prepares the same plan but drives its two phases itself, +// with the attach/printer/monitor setup in between (see upCreatePhase). +func (s *composeService) upDetached(ctx context.Context, project *types.Project, options api.UpOptions) error { + project, exec, plan, err := s.prepareUp(ctx, project, options) + if err != nil { + return err + } if err := exec.run(ctx, plan); err != nil { return translateWaitTimeout(err) @@ -222,11 +226,12 @@ type upSession struct { // runInteractiveUp sets up the attach/printer/monitor session around the // Start phase of created's plan, runs it, then follows the application until -// it terminates. The Create phase already ran, and signal.Notify only comes -// after it: Ctrl+C during Create keeps its default behavior. +// it terminates. The Create phase already ran: the session's own signal +// handling only starts after it, so an interruption of the Create phase is the +// caller's, through ctx. func (s *composeService) runInteractiveUp(ctx context.Context, created *createdUp, options api.UpOptions) error { - // the plan's own project, not the caller's: the one carrying what - // preparePlan resolved (see useAPISocket) + // the plan's own project, not the caller's: the one the Start phase runs + // on, so the session, the watcher and the teardown follow the same model project := created.project // if we get a second signal during shutdown, we kill the services diff --git a/pkg/compose/up_phases_test.go b/pkg/compose/up_phases_test.go index b547cbb835b..169fb197837 100644 --- a/pkg/compose/up_phases_test.go +++ b/pkg/compose/up_phases_test.go @@ -56,6 +56,10 @@ type interactiveUpDaemon struct { eventLog *fakeEventLog // createErr, when set, fails ContainerCreate createErr error + // onAttach and onInfo, when set, run once the session attached to the + // container, and once it read the daemon's time + onAttach func() + onInfo func() // slowSubscription delays the completion of the events subscription well // past the moment the container has started and exited slowSubscription bool @@ -136,6 +140,9 @@ func expectInteractiveUpDaemon(t *testing.T, apiClient *mocks.MockAPIClient) *in apiClient.EXPECT().ContainerAttach(gomock.Any(), "c1", gomock.Any()).AnyTimes(). DoAndReturn(func(context.Context, string, client.ContainerAttachOptions) (client.ContainerAttachResult, error) { d.record("attach") + if d.onAttach != nil { + d.onAttach() + } return client.ContainerAttachResult{}, errors.New("attach not supported") }) apiClient.EXPECT().ContainerLogs(gomock.Any(), "c1", gomock.Any()).AnyTimes(). @@ -143,8 +150,15 @@ func expectInteractiveUpDaemon(t *testing.T, apiClient *mocks.MockAPIClient) *in // the monitor reads the daemon's time to know from when to ask for events apiClient.EXPECT().Info(gomock.Any(), gomock.Any()).AnyTimes(). - DoAndReturn(func(context.Context, client.InfoOptions) (client.SystemInfoResult, error) { - return d.eventLog.info() + DoAndReturn(func(ctx context.Context, _ client.InfoOptions) (client.SystemInfoResult, error) { + if err := ctx.Err(); err != nil { + return client.SystemInfoResult{}, err + } + res, err := d.eventLog.info() + if d.onInfo != nil { + d.onInfo() + } + return res, err }) apiClient.EXPECT().Events(gomock.Any(), gomock.Any()).AnyTimes(). DoAndReturn(func(_ context.Context, opts client.EventsListOptions) client.EventsResult { @@ -170,8 +184,8 @@ func expectInteractiveUpDaemon(t *testing.T, apiClient *mocks.MockAPIClient) *in 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: long enough here for the monitor, launched just - // before the Start phase, to have taken its timestamp. + // within it: long enough here for the session to have read the + // daemon's time, which it does just before the Start phase. timer := time.AfterFunc(containerExitDelay, func() { d.eventLog.emit(containerMessage(events.ActionDie, "c1", "test-web-1", "web", map[string]string{"exitCode": "0"})) }) @@ -238,6 +252,12 @@ func (h *interactiveUpHarness) up(t *testing.T) error { t.Helper() ctx, cancel := context.WithCancel(t.Context()) defer cancel() + return h.upWith(t, ctx, cancel) +} + +// 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() done := make(chan error, 1) go func() { done <- h.svc.Up(ctx, interactiveUpProject(), interactiveUpOptions()) }() select { @@ -264,9 +284,10 @@ func TestUpInteractivePhaseOrder(t *testing.T) { assert.NilError(t, h.up(t)) d := h.daemon - create, done, attach, start := d.indexOf("create"), d.indexOf("progress-done:up"), d.indexOf("attach"), d.indexOf("start") - assert.Assert(t, create >= 0 && done >= 0 && attach >= 0 && start >= 0, "calls: %v", d.recorded()) - assert.Assert(t, create < done, "the container is created within the up progress scope: %v", d.recorded()) + scope, create, done := d.indexOf("progress-start:up"), d.indexOf("create"), d.indexOf("progress-done:up") + attach, start := d.indexOf("attach"), d.indexOf("start") + assert.Assert(t, scope >= 0 && create >= 0 && done >= 0 && attach >= 0 && start >= 0, "calls: %v", d.recorded()) + assert.Assert(t, scope < create && create < done, "the container is created within the up progress scope: %v", d.recorded()) assert.Assert(t, done < attach, "the session attaches once the up progress scope is over: %v", d.recorded()) assert.Assert(t, attach < start, "no container starts before the session is attached: %v", d.recorded()) } @@ -277,9 +298,9 @@ func TestUpInteractivePhaseOrder(t *testing.T) { // subscription is effective (modeled here as the subscription only completing // once the container has started). The exit must still be observed, or up // would wait on the container forever. The replay starts at the daemon's time, -// read before the container listing: the daemon's clock is set here well apart -// from the client's, as it is for a remote daemon, so that the events it -// stamps are only replayed when asked for from its own time. +// which the session reads before the Start phase: the daemon's clock is set +// here well apart from the client's, as it is for a remote daemon, so that the +// events it stamps are only replayed when asked for from its own time. func TestUpInteractiveSeesContainerExitingBeforeSubscription(t *testing.T) { h := newInteractiveUpHarness(t) h.daemon.slowSubscription = true @@ -350,14 +371,50 @@ func TestUpInteractiveFailedCreateStopsThere(t *testing.T) { assert.DeepEqual(t, h.daemon.recorded(), []string{"progress-start:up", "create", "progress-done:up"}) } -// TestUpInteractiveDryRunOnlyCreates: a dry-run interactive up plans the -// Create phase and never gets to the session or the Start phase. +// TestUpInteractiveDryRunOnlyCreates: a dry-run interactive up stops after the +// Create phase: no session, no event subscription, no start. Only the service's +// dry-run flag is set here; the engine calls a real dry run swaps for the dry-run +// client are not what this pins, but the gate of Up. func TestUpInteractiveDryRunOnlyCreates(t *testing.T) { h := newInteractiveUpHarness(t) h.svc.dryRun = true assert.NilError(t, h.up(t)) - assert.DeepEqual(t, h.daemon.recorded(), []string{"progress-start:up", "create", "progress-done:up"}) + for _, call := range []string{"attach", "events", "start"} { + assert.Equal(t, h.daemon.indexOf(call), -1, "%s must not be reached in a dry run: %v", call, h.daemon.recorded()) + } assert.Assert(t, strings.Contains(h.out.String(), "interactive run is not supported in dry-run mode"), "output: %q", h.out.String()) } + +// TestUpInteractiveStartPhaseSurvivesCancellation: the Start phase runs on a +// context that outlives the caller's, since an interruption is the session's to +// handle once it is set up (it stops what the application started). Canceled +// right after the session read the daemon's time, which is its last step before +// the Start phase, up still starts the container. +func TestUpInteractiveStartPhaseSurvivesCancellation(t *testing.T) { + h := newInteractiveUpHarness(t) + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + h.daemon.onInfo = cancel + + _ = h.upWith(t, ctx, cancel) + + assert.Assert(t, h.daemon.indexOf("start") >= 0, "the Start phase ran to the container start: %v", h.daemon.recorded()) +} + +// TestUpInteractiveInterruptedBeforeTheStartPhase: an interruption met while +// the session reads the daemon's time is reported as such, wrapping the +// context's error, which the CLI maps to its interruption exit code, and no +// container is started. +func TestUpInteractiveInterruptedBeforeTheStartPhase(t *testing.T) { + h := newInteractiveUpHarness(t) + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + h.daemon.onAttach = cancel + + err := h.upWith(t, ctx, cancel) + + assert.Assert(t, errors.Is(err, context.Canceled), "err: %v", err) + assert.Equal(t, h.daemon.indexOf("start"), -1, "no container is started: %v", h.daemon.recorded()) +} From f525ff02f34281fe3606758b974cac6ed43a5805 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Mon, 26 Oct 2026 14:50:34 +0100 Subject: [PATCH 4/6] docs(api): StartOptions.Wait is rejected together with Attach UpOptions.Validate now refuses the combination, so the field's doc says that instead of describing a foreground Up that silently skips the verification. Signed-off-by: Nicolas De Loof --- pkg/api/api.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/api/api.go b/pkg/api/api.go index 98516ebadf4..0c4074fc2f9 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -384,8 +384,8 @@ type StartOptions struct { // by Up's foreground session only; ignored by Start. ExitCodeFrom string // Wait won't return until containers reached the running|healthy state. - // Honored by a detached Up (Attach nil); a foreground Up is not followed - // by this verification. + // Up honors it in detached mode only (Attach nil): combined with Attach it + // is rejected, see UpOptions.Validate. Wait bool WaitTimeout time.Duration // Services names the services the user explicitly targeted; it only From 7298d47b09b3174e3f12549c504b8a5a7a91c875 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Mon, 26 Oct 2026 15:22:58 +0100 Subject: [PATCH 5/6] test(up): pin the monitor and the Create phase of interactive up, fix stale comments Tests: - the monitor goroutine runs before the Start phase: a fake container start holds until the events subscription is opened, so a Start phase run before it would show; - an interruption met by the Create phase ends the up with it and sets up no session: the phase runs on the caller's context. Both were found by mutation to survive the existing tests. Comments and scenarios: - useAPISocket's guard says what a second pass really does: duplicate volume and config entries that also change the service's config hash; - the interactive up's doc says an early interruption starts nothing, the readDaemonTime's that callers decide how to report an interruption, and a test comment on the listener no longer announces what is already wired; - the use_api_socket scenario links docker/compose#14386, as SCENARIO.md asks of a regression test, and the hook-runner scenario drops an output assertion its ExitCode already covers. Signed-off-by: Nicolas De Loof --- pkg/compose/apiSocket.go | 3 +- pkg/compose/executor_start_test.go | 3 +- pkg/compose/monitor.go | 4 +- pkg/compose/up.go | 3 +- pkg/compose/up_phases_test.go | 65 +++++++++++++++++++++++++++--- pkg/e2e/up_interactive_test.go | 7 +++- 6 files changed, 71 insertions(+), 14 deletions(-) diff --git a/pkg/compose/apiSocket.go b/pkg/compose/apiSocket.go index d9442be5a45..de9d55d0977 100644 --- a/pkg/compose/apiSocket.go +++ b/pkg/compose/apiSocket.go @@ -42,7 +42,8 @@ func (s *composeService) useAPISocket(project *types.Project) (*types.Project, e } // The project already went through here, as the plan's own project does // when it is handed to a later plan (a watch rebuild): transforming it a - // second time would mount the socket and the config file twice. + // second time would append the socket volume and the config entry again, + // duplicate entries in the model that also change the service's config hash. if _, applied := project.Configs["#apisocket"]; applied { return project, nil } diff --git a/pkg/compose/executor_start_test.go b/pkg/compose/executor_start_test.go index 64da00facec..1a38599d6ce 100644 --- a/pkg/compose/executor_start_test.go +++ b/pkg/compose/executor_start_test.go @@ -820,8 +820,7 @@ func TestExecRunPreStart_BailsOutWhenContextCanceled(t *testing.T) { // TestExecRunPostStart_RunsHookAndStreamsToListener verifies that // execRunPostStart resolves the replica the start chain just brought up and // runs its post_start hooks, forwarding their output to the listener the -// executor was constructed with — the plumbing an attached `up` will use -// once the interactive-up convergence wires a real one in. +// executor was constructed with. func TestExecRunPostStart_RunsHookAndStreamsToListener(t *testing.T) { svc, apiClient, _ := newStartPhaseTestService(t) diff --git a/pkg/compose/monitor.go b/pkg/compose/monitor.go index ac5e568527f..d193ec97547 100644 --- a/pkg/compose/monitor.go +++ b/pkg/compose/monitor.go @@ -226,8 +226,8 @@ func (c *monitor) Start(ctx context.Context) error { // capability, not a reason to fail. A daemon that answers with no time or one // that can't be parsed is an error: the replay would have no sound starting // point, and nothing explains the answer. An interruption is an error too, -// wrapping the context's: not a failure of the daemon, for the caller to -// report as the interruption it is. +// wrapping the context's: callers decide how to report it (Start ends quietly, +// an interactive up returns it). func (c *monitor) readDaemonTime(ctx context.Context) error { if c.sinceRead { return nil diff --git a/pkg/compose/up.go b/pkg/compose/up.go index 19a61365265..fa2eee7818b 100644 --- a/pkg/compose/up.go +++ b/pkg/compose/up.go @@ -228,7 +228,8 @@ type upSession struct { // Start phase of created's plan, runs it, then follows the application until // it terminates. The Create phase already ran: the session's own signal // handling only starts after it, so an interruption of the Create phase is the -// caller's, through ctx. +// caller's, through ctx. An interruption met before the Start phase starts +// nothing: the containers are left created. func (s *composeService) runInteractiveUp(ctx context.Context, created *createdUp, options api.UpOptions) error { // the plan's own project, not the caller's: the one the Start phase runs // on, so the session, the watcher and the teardown follow the same model diff --git a/pkg/compose/up_phases_test.go b/pkg/compose/up_phases_test.go index 169fb197837..6e577941d21 100644 --- a/pkg/compose/up_phases_test.go +++ b/pkg/compose/up_phases_test.go @@ -25,6 +25,7 @@ import ( "io" "strings" "sync" + "sync/atomic" "testing" "time" @@ -56,10 +57,18 @@ type interactiveUpDaemon struct { eventLog *fakeEventLog // createErr, when set, fails ContainerCreate createErr error - // onAttach and onInfo, when set, run once the session attached to the - // container, and once it read the daemon's time + // onCreate, onAttach and onInfo, when set, run when the container is + // created, once the session attached to it, and once it read the daemon's + // time + onCreate func() onAttach func() onInfo func() + // startWaitsForEvents holds a container start until the events + // subscription is opened, giving up (and flagging it) after a while + startWaitsForEvents bool + eventsSeen chan struct{} + eventsOnce sync.Once + startedWithoutMonitor atomic.Bool // slowSubscription delays the completion of the events subscription well // past the moment the container has started and exited slowSubscription bool @@ -90,7 +99,7 @@ func (d *interactiveUpDaemon) indexOf(call string) int { func expectInteractiveUpDaemon(t *testing.T, apiClient *mocks.MockAPIClient) *interactiveUpDaemon { t.Helper() - d := &interactiveUpDaemon{eventLog: &fakeEventLog{}, started: make(chan struct{})} + d := &interactiveUpDaemon{eventLog: &fakeEventLog{}, started: make(chan struct{}), eventsSeen: make(chan struct{})} labels := map[string]string{ api.ProjectLabel: "test", api.ServiceLabel: "web", @@ -116,8 +125,14 @@ func expectInteractiveUpDaemon(t *testing.T, apiClient *mocks.MockAPIClient) *in }}}, nil }) apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()).AnyTimes(). - DoAndReturn(func(context.Context, client.ContainerCreateOptions) (client.ContainerCreateResult, error) { + DoAndReturn(func(ctx context.Context, _ client.ContainerCreateOptions) (client.ContainerCreateResult, error) { d.record("create") + if d.onCreate != nil { + d.onCreate() + } + if err := ctx.Err(); err != nil { + return client.ContainerCreateResult{}, err + } if d.createErr != nil { return client.ContainerCreateResult{}, d.createErr } @@ -163,6 +178,7 @@ func expectInteractiveUpDaemon(t *testing.T, apiClient *mocks.MockAPIClient) *in apiClient.EXPECT().Events(gomock.Any(), gomock.Any()).AnyTimes(). DoAndReturn(func(_ context.Context, opts client.EventsListOptions) client.EventsResult { d.record("events") + d.eventsOnce.Do(func() { close(d.eventsSeen) }) if d.slowSubscription { select { case <-d.started: @@ -180,12 +196,18 @@ func expectInteractiveUpDaemon(t *testing.T, apiClient *mocks.MockAPIClient) *in // 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: long enough here for the session to have read the - // daemon's time, which it does just before the Start phase. + // within it. timer := time.AfterFunc(containerExitDelay, func() { d.eventLog.emit(containerMessage(events.ActionDie, "c1", "test-web-1", "web", map[string]string{"exitCode": "0"})) }) @@ -418,3 +440,34 @@ func TestUpInteractiveInterruptedBeforeTheStartPhase(t *testing.T) { assert.Assert(t, errors.Is(err, context.Canceled), "err: %v", err) assert.Equal(t, h.daemon.indexOf("start"), -1, "no container is started: %v", h.daemon.recorded()) } + +// TestUpInteractiveMonitorRunsBeforeTheStartPhase: the monitor follows the +// application from the moment the Start phase begins, which can last as long as +// its dependency waits: the cascade, the late-starter stop and the log +// following are dead until it runs. The fake engine holds a container start +// until the events subscription is opened. +func TestUpInteractiveMonitorRunsBeforeTheStartPhase(t *testing.T) { + h := newInteractiveUpHarness(t) + h.daemon.startWaitsForEvents = true + + assert.NilError(t, h.up(t)) + + assert.Assert(t, !h.daemon.startedWithoutMonitor.Load(), "a container start came before the monitor was subscribing: %v", h.daemon.recorded()) +} + +// TestUpInteractiveInterruptedDuringTheCreatePhase: an interruption met by the +// Create phase ends the up with it: the phase runs on the caller's context, +// whose cancellation the engine reports, and the session is never set up. +func TestUpInteractiveInterruptedDuringTheCreatePhase(t *testing.T) { + h := newInteractiveUpHarness(t) + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + h.daemon.onCreate = cancel + + err := h.upWith(t, ctx, cancel) + + assert.Assert(t, errors.Is(err, context.Canceled), "err: %v", err) + for _, call := range []string{"attach", "events", "start"} { + assert.Equal(t, h.daemon.indexOf(call), -1, "%s must not be reached: %v", call, h.daemon.recorded()) + } +} diff --git a/pkg/e2e/up_interactive_test.go b/pkg/e2e/up_interactive_test.go index 032288f76e3..a5a32ecb30d 100644 --- a/pkg/e2e/up_interactive_test.go +++ b/pkg/e2e/up_interactive_test.go @@ -41,8 +41,7 @@ func TestUpInteractiveIgnoresHookRunnerExit(t *testing.T) { Step("up reports the exit code of the service, not the successful exit of its hook", ComposeCmd("up", "--menu=false", "--exit-code-from", "app").MayFail().Within(60*time.Second), ExitCode(3), - OutputContains("hook-ran"), - OutputContains("app-1 exited with code 3")) + OutputContains("hook-ran")) } func TestUpInteractiveImmediateExit(t *testing.T) { @@ -103,6 +102,10 @@ func TestUpInteractiveDependencyTimeout(t *testing.T) { ServiceState("web", "created")) } +// Regression test for https://github.com/docker/compose/issues/14386: a foreground +// up left a use_api_socket service without its config file, DOCKER_CONFIG +// pointing at a file that was never created. +// // use_api_socket is applied by rewriting the project (a #apisocket config the // service mounts at /run/secrets/docker/config.json): a foreground up must // start the containers from that same rewritten project, as a detached one From 3ab8661577ea116a2e576e4eda9bc88442d615e0 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Mon, 26 Oct 2026 15:48:42 +0100 Subject: [PATCH 6/6] test(up): pin that the Create phase carries the plan's project, settle the docs - the plan's own project, not the caller's, is what the session, the Start phase, the watcher and the teardown work on: a test checks that the Create phase hands it over, with use_api_socket's config only the plan's project has. It was found by mutation to survive the other tests; - the interactive up's doc no longer over-claims: an interruption is only honored while the session attaches or reads the daemon's time, and one arriving later does not stop the Start phase; - readDaemonTime's doc says the error is the client's, which carries the context's; - two test comments say what they rely on: that only the monitor uses the events API, and that the fake engine reports an interrupted call. Signed-off-by: Nicolas De Loof --- pkg/compose/monitor.go | 6 +++--- pkg/compose/up.go | 5 +++-- pkg/compose/up_phases_test.go | 27 ++++++++++++++++++++++++++- 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/pkg/compose/monitor.go b/pkg/compose/monitor.go index d193ec97547..0a353d7c361 100644 --- a/pkg/compose/monitor.go +++ b/pkg/compose/monitor.go @@ -225,9 +225,9 @@ func (c *monitor) Start(ctx context.Context) error { // as they were before the replay existed, with a warning: this is a missing // capability, not a reason to fail. A daemon that answers with no time or one // that can't be parsed is an error: the replay would have no sound starting -// point, and nothing explains the answer. An interruption is an error too, -// wrapping the context's: callers decide how to report it (Start ends quietly, -// an interactive up returns it). +// point, and nothing explains the answer. An interruption is an error too, the +// client's, which carries the context's: callers decide how to report it (Start +// ends quietly, an interactive up returns it). func (c *monitor) readDaemonTime(ctx context.Context) error { if c.sinceRead { return nil diff --git a/pkg/compose/up.go b/pkg/compose/up.go index fa2eee7818b..db4fdf4580e 100644 --- a/pkg/compose/up.go +++ b/pkg/compose/up.go @@ -228,8 +228,9 @@ type upSession struct { // Start phase of created's plan, runs it, then follows the application until // it terminates. The Create phase already ran: the session's own signal // handling only starts after it, so an interruption of the Create phase is the -// caller's, through ctx. An interruption met before the Start phase starts -// nothing: the containers are left created. +// caller's, through ctx. An interruption met while the session attaches or +// reads the daemon's time starts nothing; one arriving later does not stop the +// Start phase, which runs on a context that outlives it. func (s *composeService) runInteractiveUp(ctx context.Context, created *createdUp, options api.UpOptions) error { // the plan's own project, not the caller's: the one the Start phase runs // on, so the session, the watcher and the teardown follow the same model diff --git a/pkg/compose/up_phases_test.go b/pkg/compose/up_phases_test.go index 6e577941d21..664357360dc 100644 --- a/pkg/compose/up_phases_test.go +++ b/pkg/compose/up_phases_test.go @@ -130,6 +130,7 @@ func expectInteractiveUpDaemon(t *testing.T, apiClient *mocks.MockAPIClient) *in if d.onCreate != nil { d.onCreate() } + // like the engine's client: an interrupted call reports it if err := ctx.Err(); err != nil { return client.ContainerCreateResult{}, err } @@ -445,7 +446,8 @@ func TestUpInteractiveInterruptedBeforeTheStartPhase(t *testing.T) { // application from the moment the Start phase begins, which can last as long as // its dependency waits: the cascade, the late-starter stop and the log // following are dead until it runs. The fake engine holds a container start -// until the events subscription is opened. +// until the events subscription is opened, which only the monitor does: a +// Create-phase use of the events API would make this test vacuous. func TestUpInteractiveMonitorRunsBeforeTheStartPhase(t *testing.T) { h := newInteractiveUpHarness(t) h.daemon.startWaitsForEvents = true @@ -471,3 +473,26 @@ func TestUpInteractiveInterruptedDuringTheCreatePhase(t *testing.T) { assert.Equal(t, h.daemon.indexOf(call), -1, "%s must not be reached: %v", call, h.daemon.recorded()) } } + +// TestUpInteractiveCreatePhaseCarriesThePlanProject: the Start phase, the +// session, the watcher and the teardown work on the plan's own project, which +// carries what preparation resolved -- here use_api_socket's #apisocket config, +// which the caller's project never gets. +func TestUpInteractiveCreatePhaseCarriesThePlanProject(t *testing.T) { + h := newInteractiveUpHarness(t) + h.svc.contextInfo = linuxContextInfo{} + project := interactiveUpProject() + project.Configs = types.Configs{} + web := project.Services["web"] + web.UseAPISocket = true + web.Environment = types.MappingWithEquals{} + project.Services["web"] = web + + created, err := h.svc.upCreatePhase(t.Context(), project, interactiveUpOptions()) + + assert.NilError(t, err) + _, plan := created.project.Configs["#apisocket"] + assert.Assert(t, plan, "the plan's project carries the use_api_socket config") + _, caller := project.Configs["#apisocket"] + assert.Assert(t, !caller, "the caller's project is not transformed") +}