Skip to content

Commit

Permalink
Support saving more than one image at a time
Browse files Browse the repository at this point in the history
- 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 genuinetools#332

- This refactor properly passes all image references to the underlying
buildkit type to let it construct the image archive with all references
specified.
  • Loading branch information
yaraskm committed Dec 8, 2021
1 parent 16d3b6c commit c8ec1aa
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 22 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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...]

Expand Down
30 changes: 17 additions & 13 deletions client/save.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 {
Expand All @@ -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()
Expand Down
15 changes: 7 additions & 8 deletions save.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 {

Expand Down Expand Up @@ -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
Expand Down
98 changes: 98 additions & 0 deletions save_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
package main

import (
"archive/tar"
"encoding/json"
"errors"
"fmt"
"io"
"io/ioutil"
"os"
"path/filepath"
"testing"
Expand Down Expand Up @@ -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
}
}
}

0 comments on commit c8ec1aa

Please sign in to comment.