Merge pull request #129 from carolynvs/skip-duplicate-layers

Skip duplicate image layers when copying
cnabio · Sep 6, 2022 · fadbd17 · fadbd17
2 parents 1cae69f + e1a0050
commit fadbd17
Show file tree

Hide file tree

Showing 5 changed files with 93 additions and 8 deletions.
diff --git a/Makefile b/Makefile
@@ -48,7 +48,7 @@ check_go_env:
 
 get-tools:
 	go install golang.org/x/tools/cmd/goimports@latest
-	go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest
+	go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.44.0
 
 # Default build
 build: bin/cnab-to-oci

diff --git a/e2e/e2e_test.go b/e2e/e2e_test.go
@@ -26,6 +26,7 @@ func TestPushAndPullCNAB(t *testing.T) {
 
 	invocationImageName := registry + "/e2e/hello-world:0.1.0-invoc"
 	serviceImageName := registry + "/e2e/http-echo"
+	whalesayImageName := registry + "/e2e/whalesay"
 	appImageName := registry + "/myuser"
 
 	// Build invocation image
@@ -34,23 +35,27 @@ func TestPushAndPullCNAB(t *testing.T) {
 	cmd.Env = append(os.Environ(), "DOCKER_BUILDKIT=1")
 	runCmd(t, cmd)
 
-	// Fetch service image
+	// Fetch service images
 	runCmd(t, icmd.Command("docker", "pull", "hashicorp/http-echo"))
+	// We are using whalesay because it has duplicate layers in the image
+	runCmd(t, icmd.Command("docker", "pull", "docker/whalesay"))
 	runCmd(t, icmd.Command("docker", "tag", "hashicorp/http-echo", serviceImageName))
+	runCmd(t, icmd.Command("docker", "tag", "docker/whalesay", whalesayImageName))
 
 	// Tidy up my room
 	defer func() {
-		runCmd(t, icmd.Command("docker", "image", "rm", "-f", invocationImageName, "hashicorp/http-echo", serviceImageName))
+		runCmd(t, icmd.Command("docker", "image", "rm", "-f", invocationImageName, "hashicorp/http-echo", serviceImageName, "docker/whalesay", whalesayImageName))
 	}()
 
 	// Push the images to the registry
 	output := runCmd(t, icmd.Command("docker", "push", invocationImageName))
 	invocDigest := getDigest(t, output)
 
 	runCmd(t, icmd.Command("docker", "push", serviceImageName))
+	runCmd(t, icmd.Command("docker", "push", whalesayImageName))
 
 	// Templatize the bundle
-	applyTemplate(t, serviceImageName, invocationImageName, invocDigest, filepath.Join("testdata", "hello-world", "bundle.json.template"), dir.Join("bundle.json"))
+	applyTemplate(t, serviceImageName, whalesayImageName, invocationImageName, invocDigest, filepath.Join("testdata", "hello-world", "bundle.json.template"), dir.Join("bundle.json"))
 
 	// Save the fixed bundle
 	runCmd(t, icmd.Command("cnab-to-oci", "fixup", dir.Join("bundle.json"),
@@ -61,7 +66,7 @@ func TestPushAndPullCNAB(t *testing.T) {
 		"--auto-update-bundle"))
 
 	// Check the fixed bundle
-	applyTemplate(t, serviceImageName, invocationImageName, invocDigest, filepath.Join("testdata", "bundle.json.golden.template"), filepath.Join("testdata", "bundle.json.golden"))
+	applyTemplate(t, serviceImageName, whalesayImageName, invocationImageName, invocDigest, filepath.Join("testdata", "bundle.json.golden.template"), filepath.Join("testdata", "bundle.json.golden"))
 	buf, err := ioutil.ReadFile(dir.Join("fixed-bundle.json"))
 	assert.NilError(t, err)
 	golden.Assert(t, string(buf), "bundle.json.golden")
@@ -107,17 +112,19 @@ func runCmd(t *testing.T, cmd icmd.Cmd) string {
 	return result.Stdout()
 }
 
-func applyTemplate(t *testing.T, serviceImageName, invocationImageName, invocationDigest, templateFile, resultFile string) {
+func applyTemplate(t *testing.T, serviceImageName, whalesayImageName, invocationImageName, invocationDigest, templateFile, resultFile string) {
 	tmpl, err := template.ParseFiles(templateFile)
 	assert.NilError(t, err)
 	data := struct {
 		InvocationImage  string
 		InvocationDigest string
 		ServiceImage     string
+		WhalesayImage    string
 	}{
 		invocationImageName,
 		invocationDigest,
 		serviceImageName,
+		whalesayImageName,
 	}
 	f, err := os.Create(resultFile)
 	assert.NilError(t, err)

diff --git a/e2e/testdata/bundle.json.golden.template b/e2e/testdata/bundle.json.golden.template
@@ -1 +1 @@
-{"actions":{"io.cnab.status":{}},"definitions":{"port":{"default":"8080","type":"string"},"text":{"default":"Hello, World!","type":"string"}},"description":"Hello, World!","images":{"hello":{"contentDigest":"sha256:61d5cb94d7e546518a7bbd5bee06bfad0ecea8f56a75b084522a43dccbbcd845","description":"hello","image":"{{ .ServiceImage }}","imageType":"docker","mediaType":"application/vnd.docker.distribution.manifest.v2+json","size":528}},"invocationImages":[{"contentDigest":"{{ .InvocationDigest }}","image":"{{ .InvocationImage }}","imageType":"docker","mediaType":"application/vnd.docker.distribution.manifest.v2+json","size":941}],"maintainers":[{"email":"user@email.com","name":"user"}],"name":"hello-world","parameters":{"fields":{"definition":"","destination":null}},"schemaVersion":"v1.0.0","version":"0.1.0"}
+{"actions":{"io.cnab.status":{}},"description":"Hello, World!","images":{"hello":{"contentDigest":"sha256:61d5cb94d7e546518a7bbd5bee06bfad0ecea8f56a75b084522a43dccbbcd845","description":"hello","image":"{{ .ServiceImage }}","imageType":"docker","mediaType":"application/vnd.docker.distribution.manifest.v2+json","size":528},"whalesay":{"contentDigest":"sha256:df326a383b4a036fd5a33402248027d1c972954622924158a28744ed5f9fca1e","description":"whalesay","image":"{{ .WhalesayImage }}","imageType":"docker","mediaType":"application/vnd.docker.distribution.manifest.v2+json","size":2402}},"invocationImages":[{"contentDigest":"{{ .InvocationDigest }}","image":"{{ .InvocationImage }}","imageType":"docker","mediaType":"application/vnd.docker.distribution.manifest.v2+json","size":941}],"maintainers":[{"email":"user@email.com","name":"user"}],"name":"hello-world","parameters":{"fields":{"definition":"","destination":null}},"schemaVersion":"v1.0.0","version":"0.1.0"}
diff --git a/e2e/testdata/hello-world/bundle.json.template b/e2e/testdata/hello-world/bundle.json.template
@@ -1 +1,59 @@
-{"actions":{"io.cnab.status":{}},"definitions":{"port":{"default":"8080","type":"string"},"text":{"default":"Hello, World!","type":"string"}},"description":"Hello, World!","images":{"hello":{"description":"hello","image":"{{ .ServiceImage }}","imageType":"docker"}},"invocationImages":[{"image":"{{ .InvocationImage }}","imageType":"docker"}],"maintainers":[{"email":"user@email.com","name":"user"}],"name":"hello-world","parameters":{"fields":{"port":{"definition":"port","destination":{"env":"PORT"}},"text":{"definition":"text","destination":{"env":"HELLO_TEXT"}}}},"schemaVersion":"v1.0.0","version":"0.1.0"}
+{
+    "actions": {
+        "io.cnab.status": {}
+    },
+    "def ainitions": {
+        "port": {
+            "default": "8080",
+            "type": "string"
+        },
+        "text": {
+            "default": "Hello, World!",
+            "type": "string"
+        }
+    },
+    "description": "Hello, World!",
+    "images": {
+        "hello": {
+            "description": "hello",
+            "image": "{{ .ServiceImage }}",
+            "imageType": "docker"
+        },
+        "whalesay": {
+            "description": "whalesay",
+            "image": "{{ .WhalesayImage}}",
+            "imageType": "docker"
+        }
+    },
+    "invocationImages": [
+        {
+            "image": "{{ .InvocationImage }}",
+            "imageType": "docker"
+        }
+    ],
+    "maintainers": [
+        {
+            "email": "user@email.com",
+            "name": "user"
+        }
+    ],
+    "name": "hello-world",
+    "parameters": {
+        "fields": {
+            "port": {
+                "definition": "port",
+                "destination": {
+                    "env": "PORT"
+                }
+            },
+            "text": {
+                "definition": "text",
+                "destination": {
+                    "env": "HELLO_TEXT"
+                }
+            }
+        }
+    },
+    "schemaVersion": "v1.0.0",
+    "version": "0.1.0"
+}
diff --git a/remotes/mount.go b/remotes/mount.go
@@ -12,6 +12,7 @@ import (
 	"github.com/containerd/containerd/images"
 	"github.com/containerd/containerd/remotes"
 	"github.com/docker/distribution/reference"
+	"github.com/opencontainers/go-digest"
 	ocischemav1 "github.com/opencontainers/image-spec/specs-go/v1"
 )
 
@@ -148,9 +149,28 @@ func (r *remoteReaderAt) ReadAt(p []byte, off int64) (int, error) {
 type descriptorContentHandler struct {
 	descriptorCopier *descriptorCopier
 	targetRepo       string
+
+	// Keep track of which layers we have copied for this image
+	// so that we can avoid copying the same layer more than once.
+	layersScheduled map[digest.Digest]struct{}
 }
 
 func (h *descriptorContentHandler) createCopyTask(ctx context.Context, descProgress *descriptorProgress) (func(ctx context.Context) error, error) {
+	if _, scheduled := h.layersScheduled[descProgress.Digest]; scheduled {
+		return func(ctx context.Context) error {
+			// Skip. We have already scheduled a copy of this layer
+			return nil
+		}, nil
+	}
+
+	// Mark that we have scheduled this layer. Some images can have a layer duplicated
+	// within the image and attempts to copy the same layer multiple times results in
+	// unexpected size errors when the later copy tasks try to copy an existing layer.
+	if h.layersScheduled == nil {
+		h.layersScheduled = make(map[digest.Digest]struct{}, 1)
+	}
+	h.layersScheduled[descProgress.Digest] = struct{}{}
+
 	copyOrMountWorkItem := func(ctx context.Context) error {
 		return h.descriptorCopier.Handle(ctx, descProgress)
 	}