Don't load into daemon if the image already exists (#1724)

google · Jun 14, 2023 · 4e4b03a · 4e4b03a
1 parent 6de8d1d
commit 4e4b03a
Show file tree

Hide file tree

Showing 3 changed files with 85 additions and 46 deletions.
diff --git 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
@@ -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
@@ -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 {