From 9e183f5e52a3c636bd0689ac8dea9dd59a869b8b Mon Sep 17 00:00:00 2001 From: Yu Yi Date: Mon, 25 Mar 2019 11:23:26 -0400 Subject: [PATCH 1/3] add cli option to download all manifests - Add `all-manifests` option to both `ctr content fetch` and `ctr images pull`. By default it is false. - This option ties to `AppendDistributionSourceLabel` in client. Signed-off-by: Yu Yi --- cmd/ctr/commands/content/fetch.go | 16 ++++++++++++++-- cmd/ctr/commands/images/pull.go | 8 ++++++++ pull.go | 11 ++++++++--- 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/cmd/ctr/commands/content/fetch.go b/cmd/ctr/commands/content/fetch.go index 99d2206b9..0f9f36799 100644 --- a/cmd/ctr/commands/content/fetch.go +++ b/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,10 @@ 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-manifests", + Usage: "Pull manifests from all platforms and layers for a specific platform", + }, ), Action: func(clicontext *cli.Context) error { var ( @@ -95,6 +99,8 @@ type FetchConfig struct { Labels []string // Platforms to fetch Platforms []string + // Whether or not download all manifests + IsAllManifests bool } // NewFetchConfig returns the default FetchConfig from cli flags @@ -117,6 +123,9 @@ func NewFetchConfig(ctx context.Context, clicontext *cli.Context) (*FetchConfig, } config.Platforms = p } + if clicontext.Bool("all-manifests") { + config.IsAllManifests = clicontext.Bool("all-manifests") + } return config, nil } @@ -149,7 +158,10 @@ func Fetch(ctx context.Context, client *containerd.Client, ref string, config *F containerd.WithResolver(config.Resolver), containerd.WithImageHandler(h), containerd.WithSchema1Conversion, - containerd.WithAppendDistributionSourceLabel(), + } + + if config.IsAllManifests { + opts = append(opts, containerd.WithAppendDistributionSourceLabel()) } for _, platform := range config.Platforms { diff --git a/cmd/ctr/commands/images/pull.go b/cmd/ctr/commands/images/pull.go index 3216976be..d7df2851f 100644 --- a/cmd/ctr/commands/images/pull.go +++ b/cmd/ctr/commands/images/pull.go @@ -53,6 +53,10 @@ command. As part of this process, we do the following: Name: "all-platforms", Usage: "pull content from all platforms", }, + cli.BoolFlag{ + Name: "all-manifests", + Usage: "Pull manifests from all platforms and layers for a specific platform", + }, ), Action: func(context *cli.Context) error { var ( @@ -78,6 +82,10 @@ command. As part of this process, we do the following: if err != nil { return err } + if context.Bool("all-manifests") { + config.IsAllManifests = context.Bool("all-manifests") + } + img, err := content.Fetch(ctx, client, ref, config) if err != nil { return err diff --git a/pull.go b/pull.go index ef0d147ba..cf62399b1 100644 --- a/pull.go +++ b/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.AppendDistributionSourceLabel { + // 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) From aae2d0d754a75ad523cb27e8f386b2cd2cc7b1ed Mon Sep 17 00:00:00 2001 From: Yu Yi Date: Mon, 25 Mar 2019 16:01:23 -0400 Subject: [PATCH 2/3] delete unnecessary checks and fix a test Signed-off-by: Yu Yi --- client_test.go | 5 ++--- cmd/ctr/commands/content/fetch.go | 6 +++--- cmd/ctr/commands/images/pull.go | 3 --- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/client_test.go b/client_test.go index 97776baaa..182e5ca75 100644 --- a/client_test.go +++ b/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") } } diff --git a/cmd/ctr/commands/content/fetch.go b/cmd/ctr/commands/content/fetch.go index 0f9f36799..ea4c840ab 100644 --- a/cmd/ctr/commands/content/fetch.go +++ b/cmd/ctr/commands/content/fetch.go @@ -123,9 +123,9 @@ func NewFetchConfig(ctx context.Context, clicontext *cli.Context) (*FetchConfig, } config.Platforms = p } - if clicontext.Bool("all-manifests") { - config.IsAllManifests = clicontext.Bool("all-manifests") - } + + config.IsAllManifests = clicontext.Bool("all-manifests") + return config, nil } diff --git a/cmd/ctr/commands/images/pull.go b/cmd/ctr/commands/images/pull.go index d7df2851f..566b62f38 100644 --- a/cmd/ctr/commands/images/pull.go +++ b/cmd/ctr/commands/images/pull.go @@ -82,9 +82,6 @@ command. As part of this process, we do the following: if err != nil { return err } - if context.Bool("all-manifests") { - config.IsAllManifests = context.Bool("all-manifests") - } img, err := content.Fetch(ctx, client, ref, config) if err != nil { From a40c3830df92f63995ae0622b4d4116dfc6cf0de Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Fri, 23 Aug 2019 15:48:05 -0700 Subject: [PATCH 3/3] Add option to pull all metadata Add flags to pull and fetch to grab all metadata. Add fetch option to pull only metadata. Signed-off-by: Derek McGowan --- client.go | 5 ++--- client_opts.go | 7 +++---- cmd/ctr/commands/content/fetch.go | 35 +++++++++++++++++++++++-------- cmd/ctr/commands/images/pull.go | 6 +++--- image_test.go | 6 ++---- pull.go | 18 +++++++--------- 6 files changed, 43 insertions(+), 34 deletions(-) diff --git a/client.go b/client.go index aa626abcb..5299179c6 100644 --- a/client.go +++ b/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 { diff --git a/client_opts.go b/client_opts.go index ed2ff05d5..867359539 100644 --- a/client_opts.go +++ b/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 } } diff --git a/cmd/ctr/commands/content/fetch.go b/cmd/ctr/commands/content/fetch.go index ea4c840ab..ea94275c7 100644 --- a/cmd/ctr/commands/content/fetch.go +++ b/cmd/ctr/commands/content/fetch.go @@ -67,8 +67,12 @@ Most of this is experimental and there are few leaps to make this work.`, Usage: "pull content from all platforms", }, cli.BoolFlag{ - Name: "all-manifests", - Usage: "Pull manifests from all platforms and layers for a specific platform", + 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 { @@ -84,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 }, @@ -97,10 +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 manifests - IsAllManifests bool + // Whether or not download all metadata + AllMetadata bool } // NewFetchConfig returns the default FetchConfig from cli flags @@ -124,7 +131,13 @@ func NewFetchConfig(ctx context.Context, clicontext *cli.Context) (*FetchConfig, config.Platforms = p } - config.IsAllManifests = clicontext.Bool("all-manifests") + 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 } @@ -160,12 +173,16 @@ func Fetch(ctx context.Context, client *containerd.Client, ref string, config *F containerd.WithSchema1Conversion, } - if config.IsAllManifests { - opts = append(opts, containerd.WithAppendDistributionSourceLabel()) + if config.AllMetadata { + opts = append(opts, containerd.WithAllMetadata()) } - for _, platform := range config.Platforms { - opts = append(opts, containerd.WithPlatform(platform)) + 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...) diff --git a/cmd/ctr/commands/images/pull.go b/cmd/ctr/commands/images/pull.go index 566b62f38..6ca88df9f 100644 --- a/cmd/ctr/commands/images/pull.go +++ b/cmd/ctr/commands/images/pull.go @@ -51,11 +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-manifests", - Usage: "Pull manifests from all platforms and layers for a specific platform", + Name: "all-metadata", + Usage: "Pull metadata for all platforms", }, ), Action: func(context *cli.Context) error { diff --git a/image_test.go b/image_test.go index 33203764f..37b072a36 100644 --- a/image_test.go +++ b/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) } diff --git a/pull.go b/pull.go index cf62399b1..fe9f6abda 100644 --- a/pull.go +++ b/pull.go @@ -140,7 +140,7 @@ 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) - if rCtx.AppendDistributionSourceLabel { + if rCtx.AllMetadata { // Filter manifests by platforms but allow to handle manifest // and configuration for not-target platforms childrenHandler = remotes.FilterManifestByPlatformHandler(childrenHandler, rCtx.PlatformMatcher) @@ -164,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) {