Merge pull request #3577 from dmcgowan/change-default-manifest-download

[carry #3127] Update pull default to skip all platform manifests

client.go

@@ -333,9 +333,8 @@ type RemoteContext struct {
 	// MaxConcurrentDownloads is the max concurrent content downloads for each pull.
 	MaxConcurrentDownloads int
 
-	// AppendDistributionSourceLabel allows fetcher to add distribute source
-	// label for each blob content, which doesn't work for legacy schema1.
-	AppendDistributionSourceLabel bool
+	// AllMetadata downloads all manifests and known-configuration files
+	AllMetadata bool
 }
 
 func defaultRemoteContext() *RemoteContext {

client_opts.go

@@ -195,11 +195,10 @@ func WithMaxConcurrentDownloads(max int) RemoteOpt {
 	}
 }
 
-// WithAppendDistributionSourceLabel allows fetcher to add distribute source
-// label for each blob content, which doesn't work for legacy schema1.
-func WithAppendDistributionSourceLabel() RemoteOpt {
+// WithAllMetadata downloads all manifests and known-configuration files
+func WithAllMetadata() RemoteOpt {
 	return func(_ *Client, c *RemoteContext) error {
-		c.AppendDistributionSourceLabel = true
+		c.AllMetadata = true
 		return nil
 	}
 }

client_test.go

@@ -287,9 +287,6 @@ func TestImagePullSomePlatforms(t *testing.T) {
 	count := 0
 	for _, manifest := range manifests {
 		children, err := images.Children(ctx, cs, manifest)
-		if err != nil {
-			t.Fatal(err)
-		}
 
 		found := false
 		for _, matcher := range m {
@@ -315,6 +312,8 @@ func TestImagePullSomePlatforms(t *testing.T) {
 				}
 				ra.Close()
 			}
+		} else if err == nil {
+			t.Fatal("manifest should not have pulled children content")
 		}
 	}
 

cmd/ctr/commands/content/fetch.go

@@ -34,7 +34,7 @@ import (
 	"github.com/containerd/containerd/pkg/progress"
 	"github.com/containerd/containerd/platforms"
 	"github.com/containerd/containerd/remotes"
-	digest "github.com/opencontainers/go-digest"
+	"github.com/opencontainers/go-digest"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/urfave/cli"
 )
@@ -66,6 +66,14 @@ Most of this is experimental and there are few leaps to make this work.`,
 			Name:  "all-platforms",
 			Usage: "pull content from all platforms",
 		},
+		cli.BoolFlag{
+			Name:  "all-metadata",
+			Usage: "Pull metadata for all platforms",
+		},
+		cli.BoolFlag{
+			Name:  "metadata-only",
+			Usage: "Pull all metadata including manifests and configs",
+		},
 	),
 	Action: func(clicontext *cli.Context) error {
 		var (
@@ -80,6 +88,7 @@ Most of this is experimental and there are few leaps to make this work.`,
 		if err != nil {
 			return err
 		}
+
 		_, err = Fetch(ctx, client, ref, config)
 		return err
 	},
@@ -93,8 +102,12 @@ type FetchConfig struct {
 	ProgressOutput io.Writer
 	// Labels to set on the content
 	Labels []string
+	// PlatformMatcher matches platforms, supersedes Platforms
+	PlatformMatcher platforms.MatchComparer
 	// Platforms to fetch
 	Platforms []string
+	// Whether or not download all metadata
+	AllMetadata bool
 }
 
 // NewFetchConfig returns the default FetchConfig from cli flags
@@ -117,6 +130,15 @@ func NewFetchConfig(ctx context.Context, clicontext *cli.Context) (*FetchConfig,
 		}
 		config.Platforms = p
 	}
+
+	if clicontext.Bool("metadata-only") {
+		config.AllMetadata = true
+		// Any with an empty set is None
+		config.PlatformMatcher = platforms.Any()
+	} else if clicontext.Bool("all-metadata") {
+		config.AllMetadata = true
+	}
+
 	return config, nil
 }
 
@@ -149,11 +171,18 @@ func Fetch(ctx context.Context, client *containerd.Client, ref string, config *F
 		containerd.WithResolver(config.Resolver),
 		containerd.WithImageHandler(h),
 		containerd.WithSchema1Conversion,
-		containerd.WithAppendDistributionSourceLabel(),
 	}
 
-	for _, platform := range config.Platforms {
-		opts = append(opts, containerd.WithPlatform(platform))
+	if config.AllMetadata {
+		opts = append(opts, containerd.WithAllMetadata())
+	}
+
+	if config.PlatformMatcher != nil {
+		opts = append(opts, containerd.WithPlatformMatcher(config.PlatformMatcher))
+	} else {
+		for _, platform := range config.Platforms {
+			opts = append(opts, containerd.WithPlatform(platform))
+		}
 	}
 
 	img, err := client.Fetch(pctx, ref, opts...)

cmd/ctr/commands/images/pull.go

@@ -51,7 +51,11 @@ command. As part of this process, we do the following:
 		},
 		cli.BoolFlag{
 			Name:  "all-platforms",
-			Usage: "pull content from all platforms",
+			Usage: "pull content and metadata from all platforms",
+		},
+		cli.BoolFlag{
+			Name:  "all-metadata",
+			Usage: "Pull metadata for all platforms",
 		},
 	),
 	Action: func(context *cli.Context) error {
@@ -78,6 +82,7 @@ command. As part of this process, we do the following:
 		if err != nil {
 			return err
 		}
+
 		img, err := content.Fetch(ctx, client, ref, config)
 		if err != nil {
 			return err

image_test.go

@@ -99,9 +99,7 @@ func TestImagePullWithDistSourceLabel(t *testing.T) {
 	pMatcher := platforms.Default()
 
 	// pull content without unpack and add distribution source label
-	image, err := client.Pull(ctx, imageName,
-		WithPlatformMatcher(pMatcher),
-		WithAppendDistributionSourceLabel())
+	image, err := client.Pull(ctx, imageName, WithPlatformMatcher(pMatcher))
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -183,7 +181,7 @@ func TestImageUsage(t *testing.T) {
 	imageName = imageName + "@" + image.Target().Digest.String()
 
 	// Fetch single platforms, but all manifests pulled
-	if _, err := client.Fetch(ctx, imageName, WithPlatformMatcher(testPlatform)); err != nil {
+	if _, err := client.Fetch(ctx, imageName, WithPlatformMatcher(testPlatform), WithAllMetadata()); err != nil {
 		t.Fatal(err)
 	}
 

pull.go

@@ -140,9 +140,14 @@ func (c *Client) fetch(ctx context.Context, rCtx *RemoteContext, ref string, lim
 		childrenHandler := images.ChildrenHandler(store)
 		// Set any children labels for that content
 		childrenHandler = images.SetChildrenLabels(store, childrenHandler)
-		// Filter manifests by platforms but allow to handle manifest
-		// and configuration for not-target platforms
-		childrenHandler = remotes.FilterManifestByPlatformHandler(childrenHandler, rCtx.PlatformMatcher)
+		if rCtx.AllMetadata {
+			// Filter manifests by platforms but allow to handle manifest
+			// and configuration for not-target platforms
+			childrenHandler = remotes.FilterManifestByPlatformHandler(childrenHandler, rCtx.PlatformMatcher)
+		} else {
+			// Filter children by platforms if specified.
+			childrenHandler = images.FilterPlatforms(childrenHandler, rCtx.PlatformMatcher)
+		}
 		// Sort and limit manifests if a finite number is needed
 		if limit > 0 {
 			childrenHandler = images.LimitManifests(childrenHandler, rCtx.PlatformMatcher, limit)
@@ -159,22 +164,18 @@ func (c *Client) fetch(ctx context.Context, rCtx *RemoteContext, ref string, lim
 			},
 		)
 
+		appendDistSrcLabelHandler, err := docker.AppendDistributionSourceLabel(store, ref)
+		if err != nil {
+			return images.Image{}, err
+		}
+
 		handlers := append(rCtx.BaseHandlers,
 			remotes.FetchHandler(store, fetcher),
 			convertibleHandler,
 			childrenHandler,
+			appendDistSrcLabelHandler,
 		)
 
-		// append distribution source label to blob data
-		if rCtx.AppendDistributionSourceLabel {
-			appendDistSrcLabelHandler, err := docker.AppendDistributionSourceLabel(store, ref)
-			if err != nil {
-				return images.Image{}, err
-			}
-
-			handlers = append(handlers, appendDistSrcLabelHandler)
-		}
-
 		handler = images.Handlers(handlers...)
 
 		converterFunc = func(ctx context.Context, desc ocispec.Descriptor) (ocispec.Descriptor, error) {