logs: filter to services from current Compose file (#9811)

* logs: filter to services from current Compose file

When using the file model, only attach to services
referenced in the active Compose file.

For example, let's say you have `compose-base.yaml`
and `compose.yaml`, where the former only has a
subset of the services but are both run as part of
the same named project.

Project based command:
```
docker compose -p myproj logs
```
This should return logs for active services based
on the project name, regardless of Compose file
state on disk.

File based command:
```
docker compose --file compose-base.yaml logs
```
This should return logs for ONLY services that are
defined in `compose-base.yaml`. Any other services
are considered 'orphaned' within the context of the
command and should be ignored.

See also #9705.

Fixes #9801.

Signed-off-by: Milas Bowman <milas.bowman@docker.com>

cmd/compose/logs.go

@@ -63,12 +63,13 @@ func logsCommand(p *projectOptions, backend api.Service) *cobra.Command {
 }
 
 func runLogs(ctx context.Context, backend api.Service, opts logsOptions, services []string) error {
-	projectName, err := opts.toProjectName()
+	project, name, err := opts.projectOrName()
 	if err != nil {
 		return err
 	}
 	consumer := formatter.NewLogConsumer(ctx, os.Stdout, !opts.noColor, !opts.noPrefix)
-	return backend.Logs(ctx, projectName, consumer, api.LogOptions{
+	return backend.Logs(ctx, name, consumer, api.LogOptions{
+		Project:    project,
 		Services:   services,
 		Follow:     opts.follow,
 		Tail:       opts.tail,

pkg/api/api.go

@@ -380,6 +380,7 @@ type ServiceStatus struct {
 
 // LogOptions defines optional parameters for the `Log` API
 type LogOptions struct {
+	Project    *types.Project
 	Services   []string
 	Tail       string
 	Since      string
@@ -431,7 +432,7 @@ type Stack struct {
 
 // LogConsumer is a callback to process log messages from services
 type LogConsumer interface {
-	Log(service, container, message string)
+	Log(containerName, service, message string)
 	Status(container, msg string)
 	Register(container string)
 }
@@ -442,6 +443,10 @@ type ContainerEventListener func(event ContainerEvent)
 // ContainerEvent notify an event has been collected on source container implementing Service
 type ContainerEvent struct {
 	Type int
+	// Container is the name of the container _without the project prefix_.
+	//
+	// This is only suitable for display purposes within Compose, as it's
+	// not guaranteed to be unique across services.
 	Container string
 	Service   string
 	Line      string

pkg/compose/convergence_test.go

@@ -23,12 +23,13 @@ import (
 	"testing"
 
 	"github.com/compose-spec/compose-go/types"
-	"github.com/docker/compose/v2/pkg/api"
-	"github.com/docker/compose/v2/pkg/mocks"
 	moby "github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/filters"
 	"github.com/golang/mock/gomock"
 	"gotest.tools/assert"
+
+	"github.com/docker/compose/v2/pkg/api"
+	"github.com/docker/compose/v2/pkg/mocks"
 )
 
 func TestContainerName(t *testing.T) {
@@ -77,7 +78,9 @@ func TestServiceLinks(t *testing.T) {
 
 		apiClient := mocks.NewMockAPIClient(mockCtrl)
 		cli := mocks.NewMockCli(mockCtrl)
-		tested.dockerCli = cli
+		tested := composeService{
+			dockerCli: cli,
+		}
 		cli.EXPECT().Client().Return(apiClient).AnyTimes()
 
 		s.Links = []string{"db"}
@@ -99,7 +102,9 @@ func TestServiceLinks(t *testing.T) {
 		defer mockCtrl.Finish()
 		apiClient := mocks.NewMockAPIClient(mockCtrl)
 		cli := mocks.NewMockCli(mockCtrl)
-		tested.dockerCli = cli
+		tested := composeService{
+			dockerCli: cli,
+		}
 		cli.EXPECT().Client().Return(apiClient).AnyTimes()
 
 		s.Links = []string{"db:db"}
@@ -121,7 +126,9 @@ func TestServiceLinks(t *testing.T) {
 		defer mockCtrl.Finish()
 		apiClient := mocks.NewMockAPIClient(mockCtrl)
 		cli := mocks.NewMockCli(mockCtrl)
-		tested.dockerCli = cli
+		tested := composeService{
+			dockerCli: cli,
+		}
 		cli.EXPECT().Client().Return(apiClient).AnyTimes()
 
 		s.Links = []string{"db:dbname"}
@@ -143,7 +150,9 @@ func TestServiceLinks(t *testing.T) {
 		defer mockCtrl.Finish()
 		apiClient := mocks.NewMockAPIClient(mockCtrl)
 		cli := mocks.NewMockCli(mockCtrl)
-		tested.dockerCli = cli
+		tested := composeService{
+			dockerCli: cli,
+		}
 		cli.EXPECT().Client().Return(apiClient).AnyTimes()
 
 		s.Links = []string{"db:dbname"}
@@ -169,7 +178,9 @@ func TestServiceLinks(t *testing.T) {
 		defer mockCtrl.Finish()
 		apiClient := mocks.NewMockAPIClient(mockCtrl)
 		cli := mocks.NewMockCli(mockCtrl)
-		tested.dockerCli = cli
+		tested := composeService{
+			dockerCli: cli,
+		}
 		cli.EXPECT().Client().Return(apiClient).AnyTimes()
 
 		s.Links = []string{}
@@ -203,7 +214,9 @@ func TestWaitDependencies(t *testing.T) {
 
 	apiClient := mocks.NewMockAPIClient(mockCtrl)
 	cli := mocks.NewMockCli(mockCtrl)
-	tested.dockerCli = cli
+	tested := composeService{
+		dockerCli: cli,
+	}
 	cli.EXPECT().Client().Return(apiClient).AnyTimes()
 
 	t.Run("should skip dependencies with scale 0", func(t *testing.T) {

pkg/compose/down_test.go

@@ -37,7 +37,9 @@ func TestDown(t *testing.T) {
 
 	api := mocks.NewMockAPIClient(mockCtrl)
 	cli := mocks.NewMockCli(mockCtrl)
-	tested.dockerCli = cli
+	tested := composeService{
+		dockerCli: cli,
+	}
 	cli.EXPECT().Client().Return(api).AnyTimes()
 
 	api.EXPECT().ContainerList(gomock.Any(), projectFilterListOpt(false)).Return(
@@ -85,7 +87,9 @@ func TestDownRemoveOrphans(t *testing.T) {
 
 	api := mocks.NewMockAPIClient(mockCtrl)
 	cli := mocks.NewMockCli(mockCtrl)
-	tested.dockerCli = cli
+	tested := composeService{
+		dockerCli: cli,
+	}
 	cli.EXPECT().Client().Return(api).AnyTimes()
 
 	api.EXPECT().ContainerList(gomock.Any(), projectFilterListOpt(true)).Return(
@@ -122,7 +126,9 @@ func TestDownRemoveVolumes(t *testing.T) {
 
 	api := mocks.NewMockAPIClient(mockCtrl)
 	cli := mocks.NewMockCli(mockCtrl)
-	tested.dockerCli = cli
+	tested := composeService{
+		dockerCli: cli,
+	}
 	cli.EXPECT().Client().Return(api).AnyTimes()
 
 	api.EXPECT().ContainerList(gomock.Any(), projectFilterListOpt(false)).Return(
@@ -149,7 +155,9 @@ func TestDownRemoveImageLocal(t *testing.T) {
 
 	api := mocks.NewMockAPIClient(mockCtrl)
 	cli := mocks.NewMockCli(mockCtrl)
-	tested.dockerCli = cli
+	tested := composeService{
+		dockerCli: cli,
+	}
 	cli.EXPECT().Client().Return(api).AnyTimes()
 
 	container := testContainer("service1", "123", false)
@@ -180,7 +188,9 @@ func TestDownRemoveImageLocalNoLabel(t *testing.T) {
 
 	api := mocks.NewMockAPIClient(mockCtrl)
 	cli := mocks.NewMockCli(mockCtrl)
-	tested.dockerCli = cli
+	tested := composeService{
+		dockerCli: cli,
+	}
 	cli.EXPECT().Client().Return(api).AnyTimes()
 
 	container := testContainer("service1", "123", false)
@@ -208,7 +218,9 @@ func TestDownRemoveImageAll(t *testing.T) {
 
 	api := mocks.NewMockAPIClient(mockCtrl)
 	cli := mocks.NewMockCli(mockCtrl)
-	tested.dockerCli = cli
+	tested := composeService{
+		dockerCli: cli,
+	}
 	cli.EXPECT().Client().Return(api).AnyTimes()
 
 	api.EXPECT().ContainerList(gomock.Any(), projectFilterListOpt(false)).Return(

pkg/compose/kill_test.go

@@ -35,15 +35,15 @@ import (
 
 const testProject = "testProject"
 
-var tested = composeService{}
-
 func TestKillAll(t *testing.T) {
 	mockCtrl := gomock.NewController(t)
 	defer mockCtrl.Finish()
 
 	api := mocks.NewMockAPIClient(mockCtrl)
 	cli := mocks.NewMockCli(mockCtrl)
-	tested.dockerCli = cli
+	tested := composeService{
+		dockerCli: cli,
+	}
 	cli.EXPECT().Client().Return(api).AnyTimes()
 
 	name := strings.ToLower(testProject)
@@ -74,7 +74,9 @@ func TestKillSignal(t *testing.T) {
 
 	api := mocks.NewMockAPIClient(mockCtrl)
 	cli := mocks.NewMockCli(mockCtrl)
-	tested.dockerCli = cli
+	tested := composeService{
+		dockerCli: cli,
+	}
 	cli.EXPECT().Client().Return(api).AnyTimes()
 
 	name := strings.ToLower(testProject)
@@ -97,9 +99,13 @@ func TestKillSignal(t *testing.T) {
 }
 
 func testContainer(service string, id string, oneOff bool) moby.Container {
+	// canonical docker names in the API start with a leading slash, some
+	// parts of Compose code will attempt to strip this off, so make sure
+	// it's consistently present
+	name := "/" + strings.TrimPrefix(id, "/")
 	return moby.Container{
 		ID:     id,
-		Names:  []string{id},
+		Names:  []string{name},
 		Labels: containerLabels(service, oneOff),
 	}
 }

pkg/compose/logs.go

@@ -29,13 +29,32 @@ import (
 	"github.com/docker/compose/v2/pkg/utils"
 )
 
-func (s *composeService) Logs(ctx context.Context, projectName string, consumer api.LogConsumer, options api.LogOptions) error {
+func (s *composeService) Logs(
+	ctx context.Context,
+	projectName string,
+	consumer api.LogConsumer,
+	options api.LogOptions,
+) error {
 	projectName = strings.ToLower(projectName)
+
 	containers, err := s.getContainers(ctx, projectName, oneOffExclude, true, options.Services...)
 	if err != nil {
 		return err
 	}
 
+	project := options.Project
+	if project == nil {
+		project, err = s.getProjectWithResources(ctx, containers, projectName)
+		if err != nil {
+			return err
+		}
+	}
+
+	if len(options.Services) == 0 {
+		options.Services = project.ServiceNames()
+	}
+
+	containers = containers.filter(isService(options.Services...))
 	eg, ctx := errgroup.WithContext(ctx)
 	for _, c := range containers {
 		c := c

pkg/compose/logs_test.go

@@ -0,0 +1,204 @@
+/*
+   Copyright 2022 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"
+	"io"
+	"strings"
+	"sync"
+	"testing"
+
+	"github.com/compose-spec/compose-go/types"
+	moby "github.com/docker/docker/api/types"
+	"github.com/docker/docker/api/types/container"
+	"github.com/docker/docker/api/types/filters"
+	"github.com/docker/docker/pkg/stdcopy"
+	"github.com/golang/mock/gomock"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	compose "github.com/docker/compose/v2/pkg/api"
+	"github.com/docker/compose/v2/pkg/mocks"
+)
+
+func TestComposeService_Logs_Demux(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+
+	api := mocks.NewMockAPIClient(mockCtrl)
+	cli := mocks.NewMockCli(mockCtrl)
+	tested := composeService{
+		dockerCli: cli,
+	}
+	cli.EXPECT().Client().Return(api).AnyTimes()
+
+	name := strings.ToLower(testProject)
+
+	ctx := context.Background()
+	api.EXPECT().ContainerList(ctx, moby.ContainerListOptions{
+		All:     true,
+		Filters: filters.NewArgs(oneOffFilter(false), projectFilter(name)),
+	}).Return(
+		[]moby.Container{
+			testContainer("service", "c", false),
+		},
+		nil,
+	)
+
+	api.EXPECT().
+		ContainerInspect(anyCancellableContext(), "c").
+		Return(moby.ContainerJSON{
+			ContainerJSONBase: &moby.ContainerJSONBase{ID: "c"},
+			Config:            &container.Config{Tty: false},
+		}, nil)
+	c1Reader, c1Writer := io.Pipe()
+	t.Cleanup(func() {
+		_ = c1Reader.Close()
+		_ = c1Writer.Close()
+	})
+	c1Stdout := stdcopy.NewStdWriter(c1Writer, stdcopy.Stdout)
+	c1Stderr := stdcopy.NewStdWriter(c1Writer, stdcopy.Stderr)
+	go func() {
+		_, err := c1Stdout.Write([]byte("hello stdout\n"))
+		assert.NoError(t, err, "Writing to fake stdout")
+		_, err = c1Stderr.Write([]byte("hello stderr\n"))
+		assert.NoError(t, err, "Writing to fake stderr")
+		_ = c1Writer.Close()
+	}()
+	api.EXPECT().ContainerLogs(anyCancellableContext(), "c", gomock.Any()).
+		Return(c1Reader, nil)
+
+	opts := compose.LogOptions{
+		Project: &types.Project{
+			Services: types.Services{
+				{Name: "service"},
+			},
+		},
+	}
+
+	consumer := &testLogConsumer{}
+	err := tested.Logs(ctx, name, consumer, opts)
+	require.NoError(t, err)
+
+	require.Equal(
+		t,
+		[]string{"hello stdout", "hello stderr"},
+		consumer.LogsForContainer("service", "c"),
+	)
+}
+
+// TestComposeService_Logs_ServiceFiltering ensures that we do not include
+// logs from out-of-scope services based on the Compose file vs actual state.
+//
+// NOTE(milas): This test exists because each method is currently duplicating
+// a lot of the project/service filtering logic. We should consider moving it
+// to an earlier point in the loading process, at which point this test could
+// safely be removed.
+func TestComposeService_Logs_ServiceFiltering(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+
+	api := mocks.NewMockAPIClient(mockCtrl)
+	cli := mocks.NewMockCli(mockCtrl)
+	tested := composeService{
+		dockerCli: cli,
+	}
+	cli.EXPECT().Client().Return(api).AnyTimes()
+
+	name := strings.ToLower(testProject)
+
+	ctx := context.Background()
+	api.EXPECT().ContainerList(ctx, moby.ContainerListOptions{
+		All:     true,
+		Filters: filters.NewArgs(oneOffFilter(false), projectFilter(name)),
+	}).Return(
+		[]moby.Container{
+			testContainer("serviceA", "c1", false),
+			testContainer("serviceA", "c2", false),
+			// serviceB will be filtered out by the project definition to
+			// ensure we ignore "orphan" containers
+			testContainer("serviceB", "c3", false),
+			testContainer("serviceC", "c4", false),
+		},
+		nil,
+	)
+
+	for _, id := range []string{"c1", "c2", "c4"} {
+		id := id
+		api.EXPECT().
+			ContainerInspect(anyCancellableContext(), id).
+			Return(
+				moby.ContainerJSON{
+					ContainerJSONBase: &moby.ContainerJSONBase{ID: id},
+					Config:            &container.Config{Tty: true},
+				},
+				nil,
+			)
+		api.EXPECT().ContainerLogs(anyCancellableContext(), id, gomock.Any()).
+			Return(io.NopCloser(strings.NewReader("hello "+id+"\n")), nil).
+			Times(1)
+	}
+
+	// this simulates passing `--filename` with a Compose file that does NOT
+	// reference `serviceB` even though it has running services for this proj
+	proj := &types.Project{
+		Services: types.Services{
+			{Name: "serviceA"},
+			{Name: "serviceC"},
+		},
+	}
+	consumer := &testLogConsumer{}
+	opts := compose.LogOptions{
+		Project: proj,
+	}
+	err := tested.Logs(ctx, name, consumer, opts)
+	require.NoError(t, err)
+
+	require.Equal(t, []string{"hello c1"}, consumer.LogsForContainer("serviceA", "c1"))
+	require.Equal(t, []string{"hello c2"}, consumer.LogsForContainer("serviceA", "c2"))
+	require.Empty(t, consumer.LogsForContainer("serviceB", "c3"))
+	require.Equal(t, []string{"hello c4"}, consumer.LogsForContainer("serviceC", "c4"))
+}
+
+type testLogConsumer struct {
+	mu sync.Mutex
+	// logs is keyed by service, then container; values are log lines
+	logs map[string]map[string][]string
+}
+
+func (l *testLogConsumer) Log(containerName, service, message string) {
+	l.mu.Lock()
+	defer l.mu.Unlock()
+	if l.logs == nil {
+		l.logs = make(map[string]map[string][]string)
+	}
+	if l.logs[service] == nil {
+		l.logs[service] = make(map[string][]string)
+	}
+	l.logs[service][containerName] = append(l.logs[service][containerName], message)
+}
+
+func (l *testLogConsumer) Status(containerName, msg string) {}
+
+func (l *testLogConsumer) Register(containerName string) {}
+
+func (l *testLogConsumer) LogsForContainer(svc string, containerName string) []string {
+	l.mu.Lock()
+	defer l.mu.Unlock()
+	return l.logs[svc][containerName]
+}

pkg/compose/ps_test.go

@@ -38,7 +38,9 @@ func TestPs(t *testing.T) {
 
 	api := mocks.NewMockAPIClient(mockCtrl)
 	cli := mocks.NewMockCli(mockCtrl)
-	tested.dockerCli = cli
+	tested := composeService{
+		dockerCli: cli,
+	}
 	cli.EXPECT().Client().Return(api).AnyTimes()
 
 	ctx := context.Background()

pkg/compose/stop_test.go

@@ -38,7 +38,9 @@ func TestStopTimeout(t *testing.T) {
 
 	api := mocks.NewMockAPIClient(mockCtrl)
 	cli := mocks.NewMockCli(mockCtrl)
-	tested.dockerCli = cli
+	tested := composeService{
+		dockerCli: cli,
+	}
 	cli.EXPECT().Client().Return(api).AnyTimes()
 
 	ctx := context.Background()