chore(compose): orphan semantics stated where up and down diverge

pkg/compose/containers.go

@@ -142,18 +142,26 @@ func isService(services ...string) containerPredicate {
 	}
 }
 
-// isOrphaned is a predicate to select containers without a matching service definition in compose project
+// isOrphaned selects the containers `--remove-orphans` cleans up — and that
+// `up` warns about otherwise. A container is orphaned when:
+//
+//   - it is a one-off (`compose run`) that FINISHED its task (exited/dead):
+//     one-offs are ephemeral by design, a terminated one is a leftover. A
+//     still-RUNNING one-off is somebody's live session and is deliberately
+//     NOT an orphan: `up --remove-orphans` must never kill it. `down` does
+//     stop running one-offs, but on purpose and through the per-service
+//     removal path — down stops the application, and a running one-off is
+//     part of what goes down (see down.go);
+//   - or it carries this project's labels but its service is not defined by
+//     the compose model (neither enabled nor disabled) — the typical leftover
+//     after the compose file was edited and a service removed or renamed.
 func isOrphaned(project *types.Project) containerPredicate {
 	services := append(project.ServiceNames(), project.DisabledServiceNames()...)
 	return func(c container.Summary) bool {
-		// One-off container
-		v, ok := c.Labels[api.OneoffLabel]
-		if ok && v == "True" {
+		if v, ok := c.Labels[api.OneoffLabel]; ok && v == "True" {
 			return c.State == container.StateExited || c.State == container.StateDead
 		}
-		// Service that is not defined in the compose model
-		service := c.Labels[api.ServiceLabel]
-		return !slices.Contains(services, service)
+		return !slices.Contains(services, c.Labels[api.ServiceLabel])
 	}
 }
 

pkg/compose/containers_test.go

@@ -0,0 +1,71 @@
+/*
+   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 (
+	"testing"
+
+	"github.com/compose-spec/compose-go/v2/types"
+	"github.com/moby/moby/api/types/container"
+	"gotest.tools/v3/assert"
+
+	"github.com/docker/compose/v5/pkg/api"
+)
+
+// TestIsOrphaned pins the orphan definition: a container is orphaned when it
+// is a one-off that FINISHED its task (a still-running `compose run` is a live
+// session `up --remove-orphans` must never kill), or when it carries the
+// project labels but its service is not defined by the compose model (enabled
+// or disabled) — the typical leftover after the compose file was edited.
+func TestIsOrphaned(t *testing.T) {
+	project := &types.Project{
+		Name: "p",
+		Services: types.Services{
+			"web": {Name: "web"},
+		},
+		DisabledServices: types.Services{
+			"debug": {Name: "debug"},
+		},
+	}
+	ctr := func(service, oneOff string, state container.ContainerState) container.Summary {
+		labels := map[string]string{api.ServiceLabel: service, api.ProjectLabel: "p"}
+		if oneOff != "" {
+			labels[api.OneoffLabel] = oneOff
+		}
+		return container.Summary{Labels: labels, State: state}
+	}
+	pred := isOrphaned(project)
+
+	for _, tc := range []struct {
+		name string
+		c    container.Summary
+		want bool
+	}{
+		{"service replica running", ctr("web", "False", container.StateRunning), false},
+		{"service replica exited", ctr("web", "False", container.StateExited), false},
+		{"disabled-profile service", ctr("debug", "False", container.StateExited), false},
+		{"service removed from the model", ctr("old", "False", container.StateRunning), true},
+		{"one-off of a declared service, exited", ctr("web", "True", container.StateExited), true},
+		{"one-off of a declared service, RUNNING: a live session, not an orphan", ctr("web", "True", container.StateRunning), false},
+		{"one-off of a removed service, dead", ctr("old", "True", container.StateDead), true},
+		{"one-off of a removed service, RUNNING: still a live session", ctr("old", "True", container.StateRunning), false},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			assert.Equal(t, pred(tc.c), tc.want)
+		})
+	}
+}

pkg/compose/down.go

@@ -46,6 +46,14 @@ func (s *composeService) down(ctx context.Context, projectName string, options a
 
 	include := oneOffExclude
 	if options.RemoveOrphans {
+		// down stops the application: one-off containers — RUNNING ones
+		// included — are part of what goes down. Those attached to a declared
+		// service are stopped/removed by the per-service loop below (they
+		// match isService); the orphan branch at the end catches the
+		// remainder (finished one-offs and model-absent services — see
+		// isOrphaned). This is deliberately broader than `up
+		// --remove-orphans`, which only cleans up FINISHED one-offs and never
+		// kills a live `compose run` session.
 		include = oneOffInclude
 	}
 	containers, err := s.getContainers(ctx, projectName, include, true)

pkg/compose/down_test.go

@@ -199,6 +199,7 @@ func TestDownRemoveOrphans(t *testing.T) {
 				testContainer("service1", "123", false),
 				testContainer("service2", "789", false),
 				testContainer("service_orphan", "321", true),
+				runningOneOff("service1", "654"),
 			},
 		}, nil)
 	api.EXPECT().VolumeList(
@@ -221,10 +222,16 @@ func TestDownRemoveOrphans(t *testing.T) {
 	api.EXPECT().ContainerStop(gomock.Any(), "123", stopOptions).Return(client.ContainerStopResult{}, nil)
 	api.EXPECT().ContainerStop(gomock.Any(), "789", stopOptions).Return(client.ContainerStopResult{}, nil)
 	api.EXPECT().ContainerStop(gomock.Any(), "321", stopOptions).Return(client.ContainerStopResult{}, nil)
+	// The RUNNING one-off of a declared service goes down too — down stops the
+	// application — via the per-service removal loop (it matches isService;
+	// isOrphaned deliberately excludes running one-offs so `up` never kills a
+	// live session). Exactly one stop+remove.
+	api.EXPECT().ContainerStop(gomock.Any(), "654", stopOptions).Return(client.ContainerStopResult{}, nil)
 
 	api.EXPECT().ContainerRemove(gomock.Any(), "123", client.ContainerRemoveOptions{Force: true}).Return(client.ContainerRemoveResult{}, nil)
 	api.EXPECT().ContainerRemove(gomock.Any(), "789", client.ContainerRemoveOptions{Force: true}).Return(client.ContainerRemoveResult{}, nil)
 	api.EXPECT().ContainerRemove(gomock.Any(), "321", client.ContainerRemoveOptions{Force: true}).Return(client.ContainerRemoveResult{}, nil)
+	api.EXPECT().ContainerRemove(gomock.Any(), "654", client.ContainerRemoveOptions{Force: true}).Return(client.ContainerRemoveResult{}, nil)
 
 	api.EXPECT().NetworkList(gomock.Any(), client.NetworkListOptions{
 		Filters: projectFilter(strings.ToLower(testProject)).Add("label", networkFilter("default")),
@@ -536,3 +543,10 @@ func TestDownHookContainerRemovalFailureIsNonFatal(t *testing.T) {
 	err = tested.Down(t.Context(), strings.ToLower(testProject), compose.DownOptions{})
 	assert.NilError(t, err)
 }
+
+// runningOneOff builds a RUNNING `compose run` container of the given service.
+func runningOneOff(service, id string) container.Summary {
+	c := testContainer(service, id, true)
+	c.State = container.StateRunning
+	return c
+}

pkg/compose/observed_state.go

@@ -151,9 +151,9 @@ func (s *composeService) collectObservedState(ctx context.Context, project *type
 	}
 
 	// --- Containers ---
-	// One-off (run) containers are included in the listing on purpose: exited
-	// one-offs are classified as orphans below, so `--remove-orphans` can
-	// clean them up.
+	// One-off (run) containers are included in the listing on purpose:
+	// FINISHED ones are classified as orphans below (see isOrphaned), so `up`
+	// can warn about them and `--remove-orphans` can clean them up.
 	raw, err := s.getContainers(ctx, project.Name, oneOffInclude, true)
 	if err != nil {
 		return nil, err
@@ -175,6 +175,10 @@ func (s *composeService) collectObservedState(ctx context.Context, project *type
 		} else if isOrphaned(project)(ctr) {
 			state.Orphans = append(state.Orphans, toObservedContainer(ctr))
 		}
+		// else: a still-RUNNING one-off. Deliberately absent from the
+		// observed state: it is somebody's live `compose run` session — up
+		// neither reconciles it, nor warns about it, nor removes it (only
+		// `down` stops running one-offs).
 	}
 
 	// --- Networks ---

pkg/compose/observed_state_test.go

@@ -146,6 +146,20 @@ func TestCollectObservedState(t *testing.T) {
 					api.OneoffLabel:          "False",
 				},
 			},
+			{
+				// RUNNING one-off: somebody's live `compose run` session —
+				// deliberately dropped from the observed state (neither
+				// reconciled as a service replica, nor listed as orphan, so
+				// `up --remove-orphans` never kills it).
+				ID:    "c4",
+				Names: []string{"/myproject-web-run-1"},
+				State: container.StateRunning,
+				Labels: map[string]string{
+					api.ServiceLabel: "web",
+					api.ProjectLabel: "myproject",
+					api.OneoffLabel:  "True",
+				},
+			},
 		},
 	}, nil)
 
@@ -188,7 +202,9 @@ func TestCollectObservedState(t *testing.T) {
 	assert.Equal(t, len(state.Containers["db"]), 1)
 	assert.Equal(t, state.Containers["db"][0].ID, "c2")
 
-	// Orphan container (service "old" not in project)
+	// Orphans: only the model-absent service "old". The running one-off c4 is
+	// absent everywhere — not in the "web" bucket (asserted above: 1 replica),
+	// not an orphan: up leaves live `compose run` sessions alone.
 	assert.Equal(t, len(state.Orphans), 1)
 	assert.Equal(t, state.Orphans[0].ID, "c3")