diff --git a/cmd/compose/logs.go b/cmd/compose/logs.go index ffe3dc2e..a9fd7255 100644 --- a/cmd/compose/logs.go +++ b/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, diff --git a/pkg/api/api.go b/pkg/api/api.go index 0cd7e609..2648bf0a 100644 --- a/pkg/api/api.go +++ b/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) } @@ -441,7 +442,11 @@ type ContainerEventListener func(event ContainerEvent) // ContainerEvent notify an event has been collected on source container implementing Service type ContainerEvent struct { - Type int + 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 diff --git a/pkg/compose/convergence_test.go b/pkg/compose/convergence_test.go index bd41db30..a27f5958 100644 --- a/pkg/compose/convergence_test.go +++ b/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) { diff --git a/pkg/compose/down_test.go b/pkg/compose/down_test.go index e5d527fd..e0d88d59 100644 --- a/pkg/compose/down_test.go +++ b/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( diff --git a/pkg/compose/kill_test.go b/pkg/compose/kill_test.go index b5cc8176..a50308a4 100644 --- a/pkg/compose/kill_test.go +++ b/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), } } diff --git a/pkg/compose/logs.go b/pkg/compose/logs.go index e8a16e78..ce2f0cf9 100644 --- a/pkg/compose/logs.go +++ b/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 diff --git a/pkg/compose/logs_test.go b/pkg/compose/logs_test.go new file mode 100644 index 00000000..2bd05737 --- /dev/null +++ b/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] +} diff --git a/pkg/compose/ps_test.go b/pkg/compose/ps_test.go index 669b09e2..a7e1ac9b 100644 --- a/pkg/compose/ps_test.go +++ b/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() diff --git a/pkg/compose/stop_test.go b/pkg/compose/stop_test.go index 97a83356..0e61c295 100644 --- a/pkg/compose/stop_test.go +++ b/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()