From 799ab842a027c0b9c52f63b4318e1ae409d00e5e Mon Sep 17 00:00:00 2001 From: koooge Date: Sun, 10 Nov 2024 23:52:32 +0100 Subject: [PATCH] Fix compose images that reutn a different image with the same ID Signed-off-by: koooge --- pkg/compose/build.go | 2 +- pkg/compose/images.go | 39 +++++++------- pkg/compose/images_test.go | 103 +++++++++++++++++++++++++++++++++++++ 3 files changed, 122 insertions(+), 22 deletions(-) create mode 100644 pkg/compose/images_test.go diff --git a/pkg/compose/build.go b/pkg/compose/build.go index d54b27ec2..619beb682 100644 --- a/pkg/compose/build.go +++ b/pkg/compose/build.go @@ -274,7 +274,7 @@ func (s *composeService) getLocalImagesDigests(ctx context.Context, project *typ imageNames = append(imageNames, imgName) } } - imgs, err := s.getImages(ctx, imageNames) + imgs, err := s.getImageSummaries(ctx, imageNames) if err != nil { return nil, err } diff --git a/pkg/compose/images.go b/pkg/compose/images.go index a0c9044b8..88fd56ba6 100644 --- a/pkg/compose/images.go +++ b/pkg/compose/images.go @@ -55,20 +55,19 @@ func (s *composeService) Images(ctx context.Context, projectName string, options containers = allContainers } - imageIDs := []string{} - // aggregate image IDs + images := []string{} for _, c := range containers { - if !utils.StringContains(imageIDs, c.ImageID) { - imageIDs = append(imageIDs, c.ImageID) + if !utils.StringContains(images, c.Image) { + images = append(images, c.Image) } } - images, err := s.getImages(ctx, imageIDs) + imageSummaries, err := s.getImageSummaries(ctx, images) if err != nil { return nil, err } summary := make([]api.ImageSummary, len(containers)) for i, container := range containers { - img, ok := images[container.ImageID] + img, ok := imageSummaries[container.Image] if !ok { return nil, fmt.Errorf("failed to retrieve image for container %s", getCanonicalContainerName(container)) } @@ -79,34 +78,32 @@ func (s *composeService) Images(ctx context.Context, projectName string, options return summary, nil } -func (s *composeService) getImages(ctx context.Context, images []string) (map[string]api.ImageSummary, error) { +func (s *composeService) getImageSummaries(ctx context.Context, repoTags []string) (map[string]api.ImageSummary, error) { summary := map[string]api.ImageSummary{} l := sync.Mutex{} eg, ctx := errgroup.WithContext(ctx) - for _, img := range images { - img := img + for _, repoTag := range repoTags { + repoTag := repoTag eg.Go(func() error { - inspect, _, err := s.apiClient().ImageInspectWithRaw(ctx, img) + inspect, _, err := s.apiClient().ImageInspectWithRaw(ctx, repoTag) if err != nil { if errdefs.IsNotFound(err) { return nil } - return fmt.Errorf("unable to get image '%s': %w", img, err) + return fmt.Errorf("unable to get image '%s': %w", repoTag, err) } tag := "" repository := "" - if len(inspect.RepoTags) > 0 { - ref, err := reference.ParseDockerRef(inspect.RepoTags[0]) - if err != nil { - return err - } - repository = reference.FamiliarName(ref) - if tagged, ok := ref.(reference.Tagged); ok { - tag = tagged.Tag() - } + ref, err := reference.ParseDockerRef(repoTag) + if err != nil { + return err + } + repository = reference.FamiliarName(ref) + if tagged, ok := ref.(reference.Tagged); ok { + tag = tagged.Tag() } l.Lock() - summary[img] = api.ImageSummary{ + summary[repoTag] = api.ImageSummary{ ID: inspect.ID, Repository: repository, Tag: tag, diff --git a/pkg/compose/images_test.go b/pkg/compose/images_test.go new file mode 100644 index 000000000..9da4c6815 --- /dev/null +++ b/pkg/compose/images_test.go @@ -0,0 +1,103 @@ +/* + Copyright 2024 Docker Compose CLI authors + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package compose + +import ( + "context" + "strings" + "testing" + + containerType "github.com/docker/docker/api/types/container" + "go.uber.org/mock/gomock" + "gotest.tools/v3/assert" + + compose "github.com/docker/compose/v2/pkg/api" + moby "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/filters" +) + +func TestImages(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + api, cli := prepareMocks(mockCtrl) + tested := composeService{ + dockerCli: cli, + } + + ctx := context.Background() + args := filters.NewArgs(projectFilter(strings.ToLower(testProject))) + listOpts := containerType.ListOptions{All: true, Filters: args} + image1 := imageInspect("image1", "foo:1", 12345) + image2 := imageInspect("image2", "bar:2", 67890) + api.EXPECT().ImageInspectWithRaw(anyCancellableContext(), "foo:1").Return(image1, nil, nil) + api.EXPECT().ImageInspectWithRaw(anyCancellableContext(), "bar:2").Return(image2, nil, nil) + c1 := containerDetail("service1", "123", "running", "foo:1") + c2 := containerDetail("service1", "456", "running", "bar:2") + c2.Ports = []moby.Port{{PublicPort: 80, PrivatePort: 90, IP: "localhost"}} + c3 := containerDetail("service2", "789", "exited", "foo:1") + api.EXPECT().ContainerList(ctx, listOpts).Return([]moby.Container{c1, c2, c3}, nil) + + images, err := tested.Images(ctx, strings.ToLower(testProject), compose.ImagesOptions{}) + + expected := []compose.ImageSummary{ + { + ID: "image1", + ContainerName: "123", + Repository: "foo", + Tag: "1", + Size: 12345, + }, + { + ID: "image2", + ContainerName: "456", + Repository: "bar", + Tag: "2", + Size: 67890, + }, + { + ID: "image1", + ContainerName: "789", + Repository: "foo", + Tag: "1", + Size: 12345, + }, + } + assert.NilError(t, err) + assert.DeepEqual(t, images, expected) +} + +func imageInspect(id string, image string, size int64) moby.ImageInspect { + return moby.ImageInspect{ + ID: id, + RepoTags: []string{ + "someRepo:someTag", + image, + }, + Size: size, + } +} + +func containerDetail(service string, id string, status string, image string) moby.Container { + return moby.Container{ + ID: id, + Names: []string{"/" + id}, + Image: image, + Labels: containerLabels(service, false), + State: status, + } +}