Pull images used in volumes with type=image

pkg/compose/pull.go

@@ -60,12 +60,13 @@ func (s *composeService) pull(ctx context.Context, project *types.Project, opts
 	eg.SetLimit(s.maxConcurrency)
 
 	var (
-		mustBuild         []string
-		pullErrors        = make([]error, len(project.Services))
-		imagesBeingPulled = map[string]string{}
+		mustBuild  []string
+		pullErrors = make([]error, len(project.Services))
 	)
 
-	i := 0
+	targets := collectPullTargets(project)
+
+	// Emit skip events for services that have no image to pull.
 	for name, service := range project.Services {
 		if service.Image == "" {
 			s.events.On(api.Resource{
@@ -74,6 +75,38 @@ func (s *composeService) pull(ctx context.Context, project *types.Project, opts
 				Text:    "Skipped",
 				Details: "No image to be pulled",
 			})
+		}
+	}
+
+	i := 0
+	for imageRef, target := range targets {
+		service := target.service
+
+		if target.isVolume {
+			if opts.IgnoreBuildable && isServiceImageToBuild(service, project.Services) {
+				s.events.On(api.Resource{
+					ID:      "Image " + imageRef,
+					Status:  api.Done,
+					Text:    "Skipped",
+					Details: "Image can be built",
+				})
+				continue
+			}
+
+			eg.Go(func() error {
+				_, err := s.pullServiceImage(ctx, service, opts.Quiet, project.Environment["DOCKER_DEFAULT_PLATFORM"])
+				if err != nil {
+					if isServiceImageToBuild(service, project.Services) {
+						mustBuild = append(mustBuild, imageRef)
+						return nil
+					}
+					s.events.On(errorEvent("Image "+imageRef, getUnwrappedErrorMessage(err)))
+					if !opts.IgnoreFailures {
+						return err
+					}
+				}
+				return nil
+			})
 			continue
 		}
 
@@ -107,12 +140,6 @@ func (s *composeService) pull(ctx context.Context, project *types.Project, opts
 			continue
 		}
 
-		if _, ok := imagesBeingPulled[service.Image]; ok {
-			continue
-		}
-
-		imagesBeingPulled[service.Image] = service.Name
-
 		idx := i
 		eg.Go(func() error {
 			_, err := s.pullServiceImage(ctx, service, opts.Quiet, project.Environment["DOCKER_DEFAULT_PLATFORM"])
@@ -150,6 +177,44 @@ func (s *composeService) pull(ctx context.Context, project *types.Project, opts
 	return errors.Join(pullErrors...)
 }
 
+// pullTarget is an image to be pulled, either from a service definition or a type=image volume source.
+type pullTarget struct {
+	service  types.ServiceConfig
+	isVolume bool
+}
+
+func collectPullTargets(project *types.Project) map[string]pullTarget {
+	targets := map[string]pullTarget{}
+
+	for name, service := range project.Services {
+		for j, vol := range service.Volumes {
+			if vol.Type != types.VolumeTypeImage {
+				continue
+			}
+			if _, exists := targets[vol.Source]; exists {
+				continue
+			}
+			targets[vol.Source] = pullTarget{
+				service: types.ServiceConfig{
+					Name:  fmt.Sprintf("%s:volume %d", name, j),
+					Image: vol.Source,
+				},
+				isVolume: true,
+			}
+		}
+
+		// Service entry overwrites any volume-only entry for the same ref.
+		if service.Image != "" {
+			targets[service.Image] = pullTarget{
+				service:  service,
+				isVolume: false,
+			}
+		}
+	}
+
+	return targets
+}
+
 func imageAlreadyPresent(serviceImage string, localImages map[string]api.ImageSummary) bool {
 	normalizedImage, err := reference.ParseDockerRef(serviceImage)
 	if err != nil {
@@ -292,28 +357,22 @@ func encodedAuth(ref reference.Named, configFile authProvider) (string, error) {
 
 func (s *composeService) pullRequiredImages(ctx context.Context, project *types.Project, images map[string]api.ImageSummary, quietPull bool) error {
 	needPull := map[string]types.ServiceConfig{}
-	for name, service := range project.Services {
-		pull, err := mustPull(service, images)
+	for imageRef, target := range collectPullTargets(project) {
+		if target.isVolume {
+			if _, ok := images[imageRef]; !ok {
+				needPull[target.service.Name] = target.service
+			}
+			continue
+		}
+		pull, err := mustPull(target.service, images)
 		if err != nil {
 			return err
 		}
 		if pull {
-			needPull[name] = service
+			needPull[target.service.Name] = target.service
 		}
-		for i, vol := range service.Volumes {
-			if vol.Type == types.VolumeTypeImage {
-				if _, ok := images[vol.Source]; !ok {
-					// Hack: create a fake ServiceConfig so we pull missing volume image
-					n := fmt.Sprintf("%s:volume %d", name, i)
-					needPull[n] = types.ServiceConfig{
-						Name:  n,
-						Image: vol.Source,
-					}
-				}
-			}
-		}
-
 	}
+
 	if len(needPull) == 0 {
 		return nil
 	}

pkg/compose/pull_test.go

@@ -0,0 +1,156 @@
+/*
+   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"
+	"gotest.tools/v3/assert"
+)
+
+func TestCollectPullTargets_VolumeOnlyImage(t *testing.T) {
+	project := &types.Project{
+		Services: types.Services{
+			"nginx": {
+				Name:  "nginx",
+				Image: "nginx:alpine",
+				Volumes: []types.ServiceVolumeConfig{
+					{
+						Type:   types.VolumeTypeImage,
+						Source: "myorg/assets:latest",
+						Target: "/srv/static",
+					},
+				},
+			},
+		},
+	}
+
+	targets := collectPullTargets(project)
+
+	// Both the service image and the volume image should be targets.
+	nginxTarget, ok := targets["nginx:alpine"]
+	assert.Assert(t, ok, "expected nginx:alpine to be a pull target")
+	assert.Equal(t, false, nginxTarget.isVolume)
+
+	volTarget, ok := targets["myorg/assets:latest"]
+	assert.Assert(t, ok, "expected myorg/assets:latest to be a pull target")
+	assert.Equal(t, true, volTarget.isVolume)
+	assert.Equal(t, "myorg/assets:latest", volTarget.service.Image)
+}
+
+func TestCollectPullTargets_ServiceWinsOverVolume(t *testing.T) {
+	sharedImage := "myorg/shared:latest"
+
+	project := &types.Project{
+		Services: types.Services{
+			// This service uses sharedImage both as its own image and as a volume source.
+			"app": {
+				Name:       "app",
+				Image:      sharedImage,
+				PullPolicy: types.PullPolicyNever,
+				Volumes: []types.ServiceVolumeConfig{
+					{
+						Type:   types.VolumeTypeImage,
+						Source: sharedImage,
+						Target: "/data",
+					},
+				},
+			},
+		},
+	}
+
+	targets := collectPullTargets(project)
+
+	target, ok := targets[sharedImage]
+	assert.Assert(t, ok, "expected shared image to be a pull target")
+	assert.Equal(t, false, target.isVolume, "service entry should win over volume-only entry")
+	assert.Equal(t, types.PullPolicyNever, target.service.PullPolicy)
+}
+
+func TestCollectPullTargets_BuildableVolumeImage(t *testing.T) {
+	builtImage := "myorg/built:latest"
+
+	project := &types.Project{
+		Services: types.Services{
+			// This service builds the image that is used as a volume source.
+			"builder": {
+				Name:  "builder",
+				Image: builtImage,
+				Build: &types.BuildConfig{Context: "."},
+			},
+			// This service consumes the built image as a type=image volume.
+			"consumer": {
+				Name: "consumer",
+				Volumes: []types.ServiceVolumeConfig{
+					{
+						Type:   types.VolumeTypeImage,
+						Source: builtImage,
+						Target: "/assets",
+					},
+				},
+			},
+		},
+	}
+
+	targets := collectPullTargets(project)
+
+	target, ok := targets[builtImage]
+	assert.Assert(t, ok, "expected built image to be a pull target")
+	assert.Equal(t, false, target.isVolume, "builder service entry should win")
+
+	volService := types.ServiceConfig{
+		Name:  "consumer:volume 0",
+		Image: builtImage,
+	}
+	assert.Equal(t, true, isServiceImageToBuild(volService, project.Services))
+
+	assert.Equal(t, true, isServiceImageToBuild(target.service, project.Services))
+}
+
+func TestCollectPullTargets_Deduplication(t *testing.T) {
+	sharedImage := "myorg/data:1.0"
+
+	project := &types.Project{
+		Services: types.Services{
+			// Service A uses the image as its own image.
+			"svc-a": {
+				Name:  "svc-a",
+				Image: sharedImage,
+			},
+			// Service B uses the same image as a volume source only.
+			"svc-b": {
+				Name: "svc-b",
+				Volumes: []types.ServiceVolumeConfig{
+					{
+						Type:   types.VolumeTypeImage,
+						Source: sharedImage,
+						Target: "/mnt/data",
+					},
+				},
+			},
+		},
+	}
+
+	targets := collectPullTargets(project)
+
+	// Only one entry for the shared image.
+	target, ok := targets[sharedImage]
+	assert.Assert(t, ok, "expected shared image to be a pull target")
+	assert.Equal(t, false, target.isVolume, "service entry should win deduplication")
+	assert.Equal(t, "svc-a", target.service.Name)
+}