From 4e4b03ab893c03c66d6cd76af5bde5cfc867639f Mon Sep 17 00:00:00 2001 From: Jon Johnson Date: Wed, 14 Jun 2023 10:49:27 -0700 Subject: [PATCH] Don't load into daemon if the image already exists (#1724) --- pkg/v1/daemon/image_test.go | 74 +++++++++++++++++++++---------------- pkg/v1/daemon/write.go | 18 +++++++++ pkg/v1/daemon/write_test.go | 39 +++++++++++-------- 3 files changed, 85 insertions(+), 46 deletions(-) diff --git a/pkg/v1/daemon/image_test.go b/pkg/v1/daemon/image_test.go index 74b6f4029..b4795db3a 100644 --- a/pkg/v1/daemon/image_test.go +++ b/pkg/v1/daemon/image_test.go @@ -35,6 +35,35 @@ import ( var imagePath = "../tarball/testdata/test_image_1.tar" +var inspectResp = types.ImageInspect{ + ID: "sha256:6e0b05049ed9c17d02e1a55e80d6599dbfcce7f4f4b022e3c673e685789c470e", + RepoTags: []string{ + "bazel/v1/tarball:test_image_1", + "test_image_2:latest", + }, + Created: "1970-01-01T00:00:00Z", + Author: "Bazel", + Architecture: "amd64", + Os: "linux", + Size: 8, + VirtualSize: 8, + Config: &container.Config{}, + GraphDriver: types.GraphDriverData{ + Data: map[string]string{ + "MergedDir": "/var/lib/docker/overlay2/988ecd005d048fd47b241dd57687231859563ba65a1dfd01ae1771ebfc4cb7c5/merged", + "UpperDir": "/var/lib/docker/overlay2/988ecd005d048fd47b241dd57687231859563ba65a1dfd01ae1771ebfc4cb7c5/diff", + "WorkDir": "/var/lib/docker/overlay2/988ecd005d048fd47b241dd57687231859563ba65a1dfd01ae1771ebfc4cb7c5/work", + }, + Name: "overlay2", + }, + RootFS: types.RootFS{ + Type: "layers", + Layers: []string{ + "sha256:8897395fd26dc44ad0e2a834335b33198cb41ac4d98dfddf58eced3853fa7b17", + }, + }, +} + type MockClient struct { Client path string @@ -47,6 +76,12 @@ type MockClient struct { saveErr error saveBody io.ReadCloser + + inspectErr error + inspectResp types.ImageInspect + inspectBody []byte + + tagErr error } func (m *MockClient) NegotiateAPIVersion(_ context.Context) { @@ -66,33 +101,7 @@ func (m *MockClient) ImageSave(_ context.Context, _ []string) (io.ReadCloser, er } func (m *MockClient) ImageInspectWithRaw(_ context.Context, _ string) (types.ImageInspect, []byte, error) { - return types.ImageInspect{ - ID: "sha256:6e0b05049ed9c17d02e1a55e80d6599dbfcce7f4f4b022e3c673e685789c470e", - RepoTags: []string{ - "bazel/v1/tarball:test_image_1", - }, - Created: "1970-01-01T00:00:00Z", - Author: "Bazel", - Architecture: "amd64", - Os: "linux", - Size: 8, - VirtualSize: 8, - Config: &container.Config{}, - GraphDriver: types.GraphDriverData{ - Data: map[string]string{ - "MergedDir": "/var/lib/docker/overlay2/988ecd005d048fd47b241dd57687231859563ba65a1dfd01ae1771ebfc4cb7c5/merged", - "UpperDir": "/var/lib/docker/overlay2/988ecd005d048fd47b241dd57687231859563ba65a1dfd01ae1771ebfc4cb7c5/diff", - "WorkDir": "/var/lib/docker/overlay2/988ecd005d048fd47b241dd57687231859563ba65a1dfd01ae1771ebfc4cb7c5/work", - }, - Name: "overlay2", - }, - RootFS: types.RootFS{ - Type: "layers", - Layers: []string{ - "sha256:8897395fd26dc44ad0e2a834335b33198cb41ac4d98dfddf58eced3853fa7b17", - }, - }, - }, nil, nil + return m.inspectResp, m.inspectBody, m.inspectErr } func (m *MockClient) ImageHistory(_ context.Context, _ string) ([]api.HistoryResponseItem, error) { @@ -118,19 +127,22 @@ func TestImage(t *testing.T) { }{{ name: "success", client: &MockClient{ - path: imagePath, + path: imagePath, + inspectResp: inspectResp, }, }, { name: "save err", client: &MockClient{ - saveBody: io.NopCloser(strings.NewReader("Loaded")), - saveErr: fmt.Errorf("locked and loaded"), + saveBody: io.NopCloser(strings.NewReader("Loaded")), + saveErr: fmt.Errorf("locked and loaded"), + inspectResp: inspectResp, }, wantErr: "locked and loaded", }, { name: "read err", client: &MockClient{ - saveBody: io.NopCloser(&errReader{fmt.Errorf("goodbye, world")}), + inspectResp: inspectResp, + saveBody: io.NopCloser(&errReader{fmt.Errorf("goodbye, world")}), }, wantErr: "goodbye, world", }} { diff --git a/pkg/v1/daemon/write.go b/pkg/v1/daemon/write.go index 48186f662..3ca5b52dd 100644 --- a/pkg/v1/daemon/write.go +++ b/pkg/v1/daemon/write.go @@ -40,6 +40,24 @@ func Write(tag name.Tag, img v1.Image, options ...Option) (string, error) { return "", err } + // If we already have this image by this image ID, we can skip loading it. + id, err := img.ConfigName() + if err != nil { + return "", fmt.Errorf("computing image ID: %w", err) + } + if resp, _, err := o.client.ImageInspectWithRaw(o.ctx, id.String()); err == nil { + want := tag.String() + + // If we already have this tag, we can skip tagging it. + for _, have := range resp.RepoTags { + if have == want { + return "", nil + } + } + + return "", o.client.ImageTag(o.ctx, id.String(), want) + } + pr, pw := io.Pipe() go func() { pw.CloseWithError(tarball.Write(tag, img, pw)) diff --git a/pkg/v1/daemon/write_test.go b/pkg/v1/daemon/write_test.go index 85e3f8ce7..6f70f7a27 100644 --- a/pkg/v1/daemon/write_test.go +++ b/pkg/v1/daemon/write_test.go @@ -58,7 +58,7 @@ func (m *MockClient) ImageTag(ctx context.Context, _, _ string) error { if m.wantCtx != nil && m.wantCtx != ctx { return fmt.Errorf("ImageTag: wrong context") } - return nil + return m.tagErr } func TestWriteImage(t *testing.T) { @@ -70,22 +70,38 @@ func TestWriteImage(t *testing.T) { }{{ name: "success", client: &MockClient{ - loadBody: io.NopCloser(strings.NewReader("Loaded")), + inspectErr: errors.New("nope"), + loadBody: io.NopCloser(strings.NewReader("Loaded")), }, wantResponse: "Loaded", }, { name: "load err", client: &MockClient{ - loadBody: io.NopCloser(strings.NewReader("Loaded")), - loadErr: fmt.Errorf("locked and loaded"), + inspectErr: errors.New("nope"), + loadBody: io.NopCloser(strings.NewReader("Loaded")), + loadErr: fmt.Errorf("locked and loaded"), }, wantErr: "locked and loaded", }, { name: "read err", client: &MockClient{ - loadBody: io.NopCloser(&errReader{fmt.Errorf("goodbye, world")}), + inspectErr: errors.New("nope"), + loadBody: io.NopCloser(&errReader{fmt.Errorf("goodbye, world")}), }, wantErr: "goodbye, world", + }, { + name: "skip load", + client: &MockClient{ + tagErr: fmt.Errorf("called tag"), + }, + wantErr: "called tag", + }, { + name: "skip tag", + client: &MockClient{ + inspectResp: inspectResp, + tagErr: fmt.Errorf("called tag"), + }, + wantResponse: "", }} { t.Run(tc.name, func(t *testing.T) { image, err := tarball.ImageFromPath("../tarball/testdata/test_image_1.tar", nil) @@ -111,14 +127,6 @@ func TestWriteImage(t *testing.T) { if !strings.Contains(response, tc.wantResponse) { t.Errorf("Error loading image. Response: %s", response) } - - dst, err := name.NewTag("hello:world") - if err != nil { - t.Fatal(err) - } - if err := Tag(tag, dst, WithClient(tc.client)); err != nil { - t.Errorf("Error tagging image: %v", err) - } }) } } @@ -146,8 +154,9 @@ func TestWriteDefaultClient(t *testing.T) { ctx := context.TODO() defaultClient = func() (Client, error) { return &MockClient{ - loadBody: io.NopCloser(strings.NewReader("Loaded")), - wantCtx: ctx, + inspectErr: errors.New("nope"), + loadBody: io.NopCloser(strings.NewReader("Loaded")), + wantCtx: ctx, }, nil } if err := Tag(tag, tag, WithContext(ctx)); err != nil {