From 13bf5565eb614b6221ddd2da2a222e8d715f6fb3 Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Tue, 28 Feb 2023 22:15:44 -0800 Subject: [PATCH] [transfer] update export to use image store references Signed-off-by: Derek McGowan --- cmd/ctr/commands/images/export.go | 39 +++--- images/archive/exporter.go | 12 ++ pkg/transfer/archive/exporter.go | 73 ++++++------ pkg/transfer/image/imagestore.go | 22 ++++ pkg/transfer/image/imagestore_test.go | 163 ++++++++++++++++++++++++-- pkg/transfer/local/export.go | 19 ++- pkg/transfer/local/transfer.go | 2 +- pkg/transfer/transfer.go | 8 +- 8 files changed, 274 insertions(+), 64 deletions(-) diff --git a/cmd/ctr/commands/images/export.go b/cmd/ctr/commands/images/export.go index 269267033..b61641d86 100644 --- a/cmd/ctr/commands/images/export.go +++ b/cmd/ctr/commands/images/export.go @@ -98,33 +98,38 @@ When '--all-platforms' is given all images in a manifest list must be available. pf, done := ProgressHandler(ctx, os.Stdout) defer done() - var specified []ocispec.Platform + exportOpts := []tarchive.ExportOpt{} if pss := context.StringSlice("platform"); len(pss) > 0 { for _, ps := range pss { p, err := platforms.Parse(ps) if err != nil { return fmt.Errorf("invalid platform %q: %w", ps, err) } - specified = append(specified, p) + exportOpts = append(exportOpts, tarchive.WithPlatform(p)) } } - - err := client.Transfer(ctx, - image.NewStore(""), // a dummy image store - tarchive.NewImageExportStream(w, "", tarchive.ExportOptions{ - Images: images, - Platforms: specified, - AllPlatforms: context.Bool("all-platforms"), - SkipNonDistributable: context.Bool("skip-non-distributable"), - SkipDockerManifest: context.Bool("skip-manifest-json"), - }), - transfer.WithProgress(pf), - ) - if err != nil { - return err + if context.Bool("all-platforms") { + exportOpts = append(exportOpts, tarchive.WithAllPlatforms) } - return nil + if context.Bool("skip-manifest-json") { + exportOpts = append(exportOpts, tarchive.WithSkipCompatibilityManifest) + } + + if context.Bool("skip-non-distributable") { + exportOpts = append(exportOpts, tarchive.WithSkipNonDistributableBlobs) + } + + storeOpts := []image.StoreOpt{} + for _, img := range images { + storeOpts = append(storeOpts, image.WithExtraReference(img)) + } + + return client.Transfer(ctx, + image.NewStore("", storeOpts...), + tarchive.NewImageExportStream(w, "", exportOpts...), + transfer.WithProgress(pf), + ) } if pss := context.StringSlice("platform"); len(pss) > 0 { diff --git a/images/archive/exporter.go b/images/archive/exporter.go index 40a0a33df..87858a958 100644 --- a/images/archive/exporter.go +++ b/images/archive/exporter.go @@ -89,6 +89,18 @@ func WithImage(is images.Store, name string) ExportOpt { } } +// WithImages adds multiples images to the exported archive. +func WithImages(imgs []images.Image) ExportOpt { + return func(ctx context.Context, o *exportOptions) error { + for _, img := range imgs { + img.Target.Annotations = addNameAnnotation(img.Name, img.Target.Annotations) + o.manifests = append(o.manifests, img.Target) + } + + return nil + } +} + // WithManifest adds a manifest to the exported archive. // When names are given they will be set on the manifest in the // exported archive, creating an index record for each name. diff --git a/pkg/transfer/archive/exporter.go b/pkg/transfer/archive/exporter.go index 044e76fd2..80a6d2dc1 100644 --- a/pkg/transfer/archive/exporter.go +++ b/pkg/transfer/archive/exporter.go @@ -41,48 +41,57 @@ func init() { plugins.Register(&transfertypes.ImageImportStream{}, &ImageImportStream{}) } -type ExportOptions struct { - Images []string - Platforms []v1.Platform - AllPlatforms bool - SkipDockerManifest bool - SkipNonDistributable bool +type ExportOpt func(*ImageExportStream) + +func WithPlatform(p v1.Platform) ExportOpt { + return func(s *ImageExportStream) { + s.platforms = append(s.platforms, p) + } +} + +func WithAllPlatforms(s *ImageExportStream) { + s.allPlatforms = true +} + +func WithSkipCompatibilityManifest(s *ImageExportStream) { + s.skipCompatibilityManifest = true +} + +func WithSkipNonDistributableBlobs(s *ImageExportStream) { + s.skipNonDistributable = true } // NewImageExportStream returns an image exporter via tar stream -func NewImageExportStream(stream io.WriteCloser, mediaType string, opts ExportOptions) *ImageExportStream { - return &ImageExportStream{ +func NewImageExportStream(stream io.WriteCloser, mediaType string, opts ...ExportOpt) *ImageExportStream { + s := &ImageExportStream{ stream: stream, mediaType: mediaType, - - images: opts.Images, - platforms: opts.Platforms, - allPlatforms: opts.AllPlatforms, - skipDockerManifest: opts.SkipDockerManifest, - skipNonDistributable: opts.SkipNonDistributable, } + for _, opt := range opts { + opt(s) + } + return s } type ImageExportStream struct { stream io.WriteCloser mediaType string - images []string - platforms []v1.Platform - allPlatforms bool - skipDockerManifest bool - skipNonDistributable bool + platforms []v1.Platform + allPlatforms bool + skipCompatibilityManifest bool + skipNonDistributable bool } func (iis *ImageExportStream) ExportStream(context.Context) (io.WriteCloser, string, error) { return iis.stream, iis.mediaType, nil } -func (iis *ImageExportStream) Export(ctx context.Context, is images.Store, cs content.Store) error { - var opts []archive.ExportOpt - for _, img := range iis.images { - opts = append(opts, archive.WithImage(is, img)) +func (iis *ImageExportStream) Export(ctx context.Context, cs content.Store, imgs []images.Image) error { + opts := []archive.ExportOpt{ + archive.WithImages(imgs), } + if len(iis.platforms) > 0 { opts = append(opts, archive.WithPlatform(platforms.Ordered(iis.platforms...))) } else { @@ -91,7 +100,7 @@ func (iis *ImageExportStream) Export(ctx context.Context, is images.Store, cs co if iis.allPlatforms { opts = append(opts, archive.WithAllPlatforms()) } - if iis.skipDockerManifest { + if iis.skipCompatibilityManifest { opts = append(opts, archive.WithSkipDockerManifest()) } if iis.skipNonDistributable { @@ -124,13 +133,12 @@ func (iis *ImageExportStream) MarshalAny(ctx context.Context, sm streaming.Strea }) } s := &transfertypes.ImageExportStream{ - Stream: sid, - MediaType: iis.mediaType, - Images: iis.images, - Platforms: specified, - AllPlatforms: iis.allPlatforms, - SkipDockerManifest: iis.skipDockerManifest, - SkipNonDistributable: iis.skipNonDistributable, + Stream: sid, + MediaType: iis.mediaType, + Platforms: specified, + AllPlatforms: iis.allPlatforms, + SkipCompatibilityManifest: iis.skipCompatibilityManifest, + SkipNonDistributable: iis.skipNonDistributable, } return typeurl.MarshalAny(s) @@ -159,10 +167,9 @@ func (iis *ImageExportStream) UnmarshalAny(ctx context.Context, sm streaming.Str iis.stream = tstreaming.WriteByteStream(ctx, stream) iis.mediaType = s.MediaType - iis.images = s.Images iis.platforms = specified iis.allPlatforms = s.AllPlatforms - iis.skipDockerManifest = s.SkipDockerManifest + iis.skipCompatibilityManifest = s.SkipCompatibilityManifest iis.skipNonDistributable = s.SkipNonDistributable return nil diff --git a/pkg/transfer/image/imagestore.go b/pkg/transfer/image/imagestore.go index 547228a15..b26f25605 100644 --- a/pkg/transfer/image/imagestore.go +++ b/pkg/transfer/image/imagestore.go @@ -325,6 +325,28 @@ func (is *Store) Get(ctx context.Context, store images.Store) (images.Image, err return store.Get(ctx, is.imageName) } +func (is *Store) Lookup(ctx context.Context, store images.Store) ([]images.Image, error) { + var imgs []images.Image + if is.imageName != "" { + img, err := store.Get(ctx, is.imageName) + if err != nil { + return nil, err + } + imgs = append(imgs, img) + } + for _, ref := range is.extraReferences { + if ref.IsPrefix { + return nil, fmt.Errorf("prefix lookup on export not implemented: %w", errdefs.ErrNotImplemented) + } + img, err := store.Get(ctx, ref.Name) + if err != nil { + return nil, err + } + imgs = append(imgs, img) + } + return imgs, nil +} + func (is *Store) UnpackPlatforms() []transfer.UnpackConfiguration { unpacks := make([]transfer.UnpackConfiguration, len(is.unpacks)) for i, uc := range is.unpacks { diff --git a/pkg/transfer/image/imagestore_test.go b/pkg/transfer/image/imagestore_test.go index fbca0e2c3..cefa0c1ef 100644 --- a/pkg/transfer/image/imagestore_test.go +++ b/pkg/transfer/image/imagestore_test.go @@ -19,6 +19,8 @@ package image import ( "context" "errors" + "sort" + "sync" "testing" "github.com/containerd/containerd/errdefs" @@ -222,7 +224,7 @@ func TestStore(t *testing.T) { desc.Annotations["io.containerd.import.ref-source"] = "annotation" } t.Run(name, func(t *testing.T) { - imgs, err := testCase.ImageStore.Store(context.Background(), desc, nopImageStore{}) + imgs, err := testCase.ImageStore.Store(context.Background(), desc, newSimpleImageStore()) if err != nil { if testCase.Err == nil { t.Fatal(err) @@ -252,24 +254,165 @@ func TestStore(t *testing.T) { } } -type nopImageStore struct{} - -func (nopImageStore) Get(ctx context.Context, name string) (images.Image, error) { - return images.Image{}, errdefs.ErrNotFound +func TestLookup(t *testing.T) { + ctx := context.Background() + is := newSimpleImageStore() + for _, name := range []string{ + "registry.io/test1:latest", + "registry.io/test1:v1", + } { + is.Create(ctx, images.Image{ + Name: name, + }) + } + for _, testCase := range []struct { + Name string + ImageStore *Store + Expected []string + Err error + }{ + { + Name: "SingleImage", + ImageStore: &Store{ + imageName: "registry.io/test1:latest", + }, + Expected: []string{"registry.io/test1:latest"}, + }, + { + Name: "MultipleReferences", + ImageStore: &Store{ + imageName: "registry.io/test1:latest", + extraReferences: []Reference{ + { + Name: "registry.io/test1:v1", + }, + }, + }, + Expected: []string{"registry.io/test1:latest", "registry.io/test1:v1"}, + }, + { + Name: "OnlyReferences", + ImageStore: &Store{ + extraReferences: []Reference{ + { + Name: "registry.io/test1:latest", + }, + { + Name: "registry.io/test1:v1", + }, + }, + }, + Expected: []string{"registry.io/test1:latest", "registry.io/test1:v1"}, + }, + { + Name: "UnsupportedPrefix", + ImageStore: &Store{ + extraReferences: []Reference{ + { + Name: "registry.io/test1:latest", + IsPrefix: true, + }, + }, + }, + Err: errdefs.ErrNotImplemented, + }, + } { + t.Run(testCase.Name, func(t *testing.T) { + images, err := testCase.ImageStore.Lookup(ctx, is) + if err != nil { + if !errors.Is(err, testCase.Err) { + t.Errorf("unexpected error %v, expected %v", err, testCase.Err) + } + return + } else if testCase.Err != nil { + t.Fatal("expected error") + } + imageNames := make([]string, len(images)) + for i, img := range images { + imageNames[i] = img.Name + } + sort.Strings(imageNames) + sort.Strings(testCase.Expected) + if len(images) != len(testCase.Expected) { + t.Fatalf("unexpected images:\n\t%v\nexpected:\n\t%v", imageNames, testCase.Expected) + } + for i := range imageNames { + if imageNames[i] != testCase.Expected[i] { + t.Fatalf("unexpected images:\n\t%v\nexpected:\n\t%v", imageNames, testCase.Expected) + } + } + }) + } } -func (nopImageStore) List(ctx context.Context, filters ...string) ([]images.Image, error) { - return nil, nil +// simpleImageStore is for testing images in memory, +// no filter support +type simpleImageStore struct { + l sync.Mutex + images map[string]images.Image } -func (nopImageStore) Create(ctx context.Context, image images.Image) (images.Image, error) { +func newSimpleImageStore() images.Store { + return &simpleImageStore{ + images: make(map[string]images.Image), + } +} + +func (is *simpleImageStore) Get(ctx context.Context, name string) (images.Image, error) { + is.l.Lock() + defer is.l.Unlock() + img, ok := is.images[name] + if !ok { + return images.Image{}, errdefs.ErrNotFound + } + return img, nil +} + +func (is *simpleImageStore) List(ctx context.Context, filters ...string) ([]images.Image, error) { + is.l.Lock() + defer is.l.Unlock() + var imgs []images.Image + + // filters not supported, return all + for _, img := range is.images { + imgs = append(imgs, img) + } + return imgs, nil +} + +func (is *simpleImageStore) Create(ctx context.Context, image images.Image) (images.Image, error) { + is.l.Lock() + defer is.l.Unlock() + + if _, ok := is.images[image.Name]; ok { + return images.Image{}, errdefs.ErrAlreadyExists + } + is.images[image.Name] = image + return image, nil } -func (nopImageStore) Update(ctx context.Context, image images.Image, fieldpaths ...string) (images.Image, error) { +func (is *simpleImageStore) Update(ctx context.Context, image images.Image, fieldpaths ...string) (images.Image, error) { + is.l.Lock() + defer is.l.Unlock() + + if _, ok := is.images[image.Name]; !ok { + return images.Image{}, errdefs.ErrNotFound + } + // fieldpaths no supported, update entire image + is.images[image.Name] = image + return image, nil } -func (nopImageStore) Delete(ctx context.Context, name string, opts ...images.DeleteOpt) error { +func (is *simpleImageStore) Delete(ctx context.Context, name string, opts ...images.DeleteOpt) error { + is.l.Lock() + defer is.l.Unlock() + + if _, ok := is.images[name]; !ok { + return errdefs.ErrNotFound + } + delete(is.images, name) + return nil } diff --git a/pkg/transfer/local/export.go b/pkg/transfer/local/export.go index 6c7cff926..069dcbebf 100644 --- a/pkg/transfer/local/export.go +++ b/pkg/transfer/local/export.go @@ -19,10 +19,11 @@ package local import ( "context" + "github.com/containerd/containerd/images" "github.com/containerd/containerd/pkg/transfer" ) -func (ts *localTransferService) exportStream(ctx context.Context, is transfer.ImageExporter, tops *transfer.Config) error { +func (ts *localTransferService) exportStream(ctx context.Context, ig transfer.ImageGetter, is transfer.ImageExporter, tops *transfer.Config) error { ctx, done, err := ts.withLease(ctx) if err != nil { return err @@ -35,7 +36,21 @@ func (ts *localTransferService) exportStream(ctx context.Context, is transfer.Im }) } - err = is.Export(ctx, ts.images, ts.content) + var imgs []images.Image + if il, ok := ig.(transfer.ImageLookup); ok { + imgs, err = il.Lookup(ctx, ts.images) + if err != nil { + return err + } + } else { + img, err := ig.Get(ctx, ts.images) + if err != nil { + return err + } + imgs = append(imgs, img) + } + + err = is.Export(ctx, ts.content, imgs) if err != nil { return err } diff --git a/pkg/transfer/local/transfer.go b/pkg/transfer/local/transfer.go index d0a555d1c..68a1ea7c4 100644 --- a/pkg/transfer/local/transfer.go +++ b/pkg/transfer/local/transfer.go @@ -73,7 +73,7 @@ func (ts *localTransferService) Transfer(ctx context.Context, src interface{}, d case transfer.ImagePusher: return ts.push(ctx, s, d, topts) case transfer.ImageExporter: - return ts.exportStream(ctx, d, topts) + return ts.exportStream(ctx, s, d, topts) } case transfer.ImageImporter: switch d := dest.(type) { diff --git a/pkg/transfer/transfer.go b/pkg/transfer/transfer.go index f87a99061..01df8c3d3 100644 --- a/pkg/transfer/transfer.go +++ b/pkg/transfer/transfer.go @@ -69,9 +69,15 @@ type ImageGetter interface { Get(context.Context, images.Store) (images.Image, error) } +// ImageLookup is a type which returns images from an image store +// based on names or prefixes +type ImageLookup interface { + Lookup(context.Context, images.Store) ([]images.Image, error) +} + // ImageExporter exports images to a writer type ImageExporter interface { - Export(ctx context.Context, is images.Store, cs content.Store) error + Export(context.Context, content.Store, []images.Image) error } // ImageImporter imports an image into a content store