From c8ec1aac0822dde39e7bbc6ece020a63eb43c0da Mon Sep 17 00:00:00 2001 From: Matt Yaraskavitch <62650344+yaraskm@users.noreply.github.com> Date: Mon, 6 Dec 2021 17:33:56 -0500 Subject: [PATCH] Support saving more than one image at a time - The previous method would loop over the image references and try to save each one. Even if this did work, the tar archive would overwrite the previous instance on each invocation meaning that the last image specified on the CLI would be the only one in the archive. Even then, this method would hang because a filesystem lock was acquired on the underlying BoltDB during the first invocation of the loop (within `client.createWorkerOpt(...)`) but never freed. Because of this, every subsequent call would only hang as seen in #332 - This refactor properly passes all image references to the underlying buildkit type to let it construct the image archive with all references specified. --- README.md | 2 +- client/save.go | 30 +++++++++------- save.go | 15 ++++---- save_test.go | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 123 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index 91fd8d3e0..52700e3e2 100644 --- a/README.md +++ b/README.md @@ -463,7 +463,7 @@ Successfully tagged jess/thing as jess/otherthing ```console $ img save -h -save - Save an image to a tar archive (streamed to STDOUT by default). +save - Save one or more images to a tar archive (streamed to STDOUT by default). Usage: img save [OPTIONS] IMAGE [IMAGE...] diff --git a/client/save.go b/client/save.go index 24c9eca6b..0d282ce95 100644 --- a/client/save.go +++ b/client/save.go @@ -10,16 +10,10 @@ import ( "github.com/docker/distribution/reference" ) -// SaveImage exports an image as a tarball which can then be imported by docker. -func (c *Client) SaveImage(ctx context.Context, image, format string, writer io.WriteCloser) error { - // Parse the image name and tag. - named, err := reference.ParseNormalizedNamed(image) - if err != nil { - return fmt.Errorf("parsing image name %q failed: %v", image, err) - } - // Add the latest lag if they did not provide one. - named = reference.TagNameOnly(named) - image = named.String() +// SaveImages exports a list of images as a tarball which can then be imported by docker. +func (c *Client) SaveImages(ctx context.Context, images []string, format string, writer io.WriteCloser) error { + + exportOpts := []archive.ExportOpt{} // Create the worker opts. opt, err := c.createWorkerOpt(false) @@ -31,8 +25,18 @@ func (c *Client) SaveImage(ctx context.Context, image, format string, writer io. return errors.New("image store is nil") } - exportOpts := []archive.ExportOpt{ - archive.WithImage(opt.ImageStore, image), + for _, image := range images { + // Parse the image name and tag. + named, err := reference.ParseNormalizedNamed(image) + if err != nil { + return fmt.Errorf("parsing image name %q failed: %v", image, err) + } + + // Add the latest lag if they did not provide one. + named = reference.TagNameOnly(named) + image = named.String() + + exportOpts = append(exportOpts, archive.WithImage(opt.ImageStore, image)) } switch format { @@ -46,7 +50,7 @@ func (c *Client) SaveImage(ctx context.Context, image, format string, writer io. } if err := archive.Export(ctx, opt.ContentStore, writer, exportOpts...); err != nil { - return fmt.Errorf("exporting image %s failed: %v", image, err) + return fmt.Errorf("exporting images %v failed: %v", images, err) } return writer.Close() diff --git a/save.go b/save.go index 7210970c4..a335da7eb 100644 --- a/save.go +++ b/save.go @@ -3,10 +3,11 @@ package main import ( "context" "fmt" - "github.com/spf13/cobra" "io" "os" + "github.com/spf13/cobra" + "github.com/containerd/containerd/namespaces" "github.com/docker/docker/pkg/term" "github.com/genuinetools/img/client" @@ -15,8 +16,8 @@ import ( ) // TODO(AkihiroSuda): support OCI archive -const saveUsageShortHelp = `Save an image to a tar archive (streamed to STDOUT by default).` -const saveUsageLongHelp = `Save an image to a tar archive (streamed to STDOUT by default).` +const saveUsageShortHelp = `Save one or more images to a tar archive (streamed to STDOUT by default).` +const saveUsageLongHelp = `Save one or more images to a tar archive (streamed to STDOUT by default).` func newSaveCommand() *cobra.Command { @@ -76,11 +77,9 @@ func (cmd *saveCommand) Run(args []string) (err error) { return err } - // Loop over the arguments as images and run save. - for _, image := range args { - if err := c.SaveImage(ctx, image, cmd.format, writer); err != nil { - return err - } + // Assume that the arguments are all image references + if err := c.SaveImages(ctx, args, cmd.format, writer); err != nil { + return err } return nil diff --git a/save_test.go b/save_test.go index 1851f3c7e..1274c4c76 100644 --- a/save_test.go +++ b/save_test.go @@ -1,6 +1,12 @@ package main import ( + "archive/tar" + "encoding/json" + "errors" + "fmt" + "io" + "io/ioutil" "os" "path/filepath" "testing" @@ -54,3 +60,95 @@ func TestSaveImageInvalid(t *testing.T) { t.Fatalf("expected invalid format to fail but did not: %s", string(out)) } } + +func TestSaveMultipleImages(t *testing.T) { + var cases = []struct { + format string + }{ + { + "", + }, + { + "docker", + }, + { + "oci", + }, + } + + for _, tt := range cases { + testname := tt.format + t.Run(testname, func(t *testing.T) { + + runBuild(t, "multiimage1", withDockerfile(` + FROM busybox + RUN echo multiimage1 + `)) + + runBuild(t, "multiimage2", withDockerfile(` + FROM busybox + RUN echo multiimage2 + `)) + + tmpf := filepath.Join(os.TempDir(), fmt.Sprintf("save-multiple-%s.tar", tt.format)) + defer os.RemoveAll(tmpf) + + if tt.format != "" { + run(t, "save", "--format", tt.format, "-o", tmpf, "multiimage1", "multiimage2") + } else { + run(t, "save", "-o", tmpf, "multiimage1", "multiimage2") + } + + // Make sure the file exists + if _, err := os.Stat(tmpf); os.IsNotExist(err) { + t.Fatalf("%s should exist after saving the image but it didn't", tmpf) + } + + count, err := getImageCountInTarball(tmpf) + + if err != nil { + t.Fatal(err) + } + + if count != 2 { + t.Fatalf("should have 2 images in archive but have %d", count) + } + }) + } +} + +func getImageCountInTarball(tarpath string) (int, error) { + file, err := os.Open(tarpath) + + if err != nil { + return -1, err + } + + defer file.Close() + + tr := tar.NewReader(file) + + for { + header, err := tr.Next() + + if err == io.EOF { + return -1, errors.New("did not find manifest in tarball") + } + if err != nil { + return -1, err + } + + if header.Name == "manifest.json" { + jsonFile, err := ioutil.ReadAll(tr) + + if err != nil { + return -1, err + } + + var result []map[string]string + json.Unmarshal([]byte(jsonFile), &result) + + return len(result), nil + } + } +}