Merge 41a97540eb into 236ba5690e

integration/dockerfiles/Dockerfile_test_cache

@@ -18,5 +18,3 @@
 
 FROM debian:10.13
 RUN date > /date
-COPY context/foo /foo
-RUN echo hey

integration/dockerfiles/Dockerfile_test_cache_install

@@ -17,7 +17,7 @@
 # if the cache is implemented correctly
 
 FROM debian:10.13
+RUN mkdir /foo
 WORKDIR /foo
 RUN apt-get update && apt-get install -y make
-COPY context/bar /context
 RUN echo "hey" > foo

integration/dockerfiles/Dockerfile_test_cache_install_oci

@@ -17,7 +17,7 @@
 # if the cache is implemented correctly
 
 FROM debian:10.13
+RUN mkdir /foo
 WORKDIR /foo
 RUN apt-get update && apt-get install -y make
-COPY context/bar /context
 RUN echo "hey" > foo

integration/dockerfiles/Dockerfile_test_cache_oci

@@ -18,5 +18,3 @@
 
 FROM debian:10.13
 RUN date > /date
-COPY context/foo /foo
-RUN echo hey

integration/dockerfiles/Dockerfile_test_issue_add

@@ -0,0 +1,9 @@
+FROM ubuntu AS base
+
+# Creates cache misses on main@1d2bff5:
+# Even with --cache-copy-layers, only COPY layers are cached.
+# ADD instruction is never cached.
+# As a workaround use COPY instruction instead
+#
+# COPY context/foo foo
+ADD context/foo foo

integration/dockerfiles/Dockerfile_test_issue_empty

@@ -0,0 +1,9 @@
+FROM ubuntu AS base
+
+# Creates cache misses on main@1d2bff5:
+# When building the image directly no layer is created.
+# When rebuilding from cache a layer is emitted even for empty statements,
+# as we currently have no logic for skipping the layer when rebuilding from emtpy cache.
+# This causes one-time cache misses as now the chain of layers has changed.
+#
+RUN echo blubb

integration/dockerfiles/Dockerfile_test_issue_workdir

@@ -0,0 +1,8 @@
+FROM ubuntu
+
+# Creates cache misses on main@1d2bff5 before #3340:
+# Folders created implicitly by WORKDIR are not cached.
+# As a workaround create them excplicitly with RUN
+#
+# RUN mkdir /app
+WORKDIR /app

integration/images.go

@@ -218,6 +218,12 @@ func NewDockerFileBuilder() *DockerFileBuilder {
 		"Dockerfile_test_cache_install": {},
 		"Dockerfile_test_cache_perm":    {},
 		"Dockerfile_test_cache_copy":    {},
+		// TODO: WORKDIR command is uncacheable
+		//"Dockerfile_test_issue_workdir":    {},
+		// TODO: ADD command is uncacheable
+		//"Dockerfile_test_issue_add":    {},
+		// TODO: Empty Layers are uncacheable
+		//"Dockerfile_test_issue_empty":    {},
 	}
 	d.TestOCICacheDockerfiles = map[string]struct{}{
 		"Dockerfile_test_cache_oci":         {},

integration/integration_test.go

@@ -35,6 +35,7 @@ import (
 	"time"
 
 	"github.com/google/go-containerregistry/pkg/name"
+	v1 "github.com/google/go-containerregistry/pkg/v1"
 	"github.com/google/go-containerregistry/pkg/v1/daemon"
 	"github.com/google/go-containerregistry/pkg/v1/remote"
 	"github.com/pkg/errors"
@@ -742,6 +743,7 @@ func verifyBuildWith(t *testing.T, cache, dockerfile string) {
 
 	expected := fmt.Sprintf(emptyContainerDiff, kanikoVersion0, kanikoVersion1, kanikoVersion0, kanikoVersion1)
 	checkContainerDiffOutput(t, diff, expected)
+	layerDiff(t, kanikoVersion0, kanikoVersion1)
 }
 
 func TestRelativePaths(t *testing.T) {
@@ -957,6 +959,56 @@ func filterFileDiff(f []fileDiff) []fileDiff {
 	return newDiffs
 }
 
+func layerDiff(t *testing.T, image1, image2 string) {
+	t.Helper()
+	layers1, err := getImageLayers(image1)
+	if err != nil {
+		t.Fatalf("Couldn't get layers from image reference for (%s): %s", image1, err)
+	}
+
+	layers2, err := getImageLayers(image2)
+	if err != nil {
+		t.Fatalf("Couldn't get layers from image reference for (%s): %s", image2, err)
+	}
+
+	for idx := range min(len(layers1), len(layers2)) {
+		l1d, err := layers1[idx].Digest()
+		if err != nil {
+			t.Fatalf("Couldn't get digest from image layer (%s #%d): %s", image1, idx, err)
+		}
+
+		l2d, err := layers2[idx].Digest()
+		if err != nil {
+			t.Fatalf("Couldn't get digest from image layer (%s #%d): %s", image2, idx, err)
+		}
+
+		if l1d != l2d {
+			command, err := resolveCreatedBy(image1, idx)
+			if err != nil {
+				t.Errorf("Image Layers #%d differ", idx)
+			} else {
+				t.Errorf("Image Layers #%d differ: %s", idx, command)
+			}
+		}
+	}
+
+	if len(layers1) > len(layers2) {
+		command, err := resolveCreatedBy(image1, len(layers2))
+		if err != nil {
+			t.Errorf("Image Layer count differs %d != %d", len(layers1), len(layers2))
+		} else {
+			t.Errorf("Image Layer count differs %d != %d: %s", len(layers1), len(layers2), command)
+		}
+	} else if len(layers1) < len(layers2) {
+		command, err := resolveCreatedBy(image2, len(layers1))
+		if err != nil {
+			t.Errorf("Image Layer count differs %d != %d", len(layers1), len(layers2))
+		} else {
+			t.Errorf("Image Layer count differs %d != %d: %s", len(layers1), len(layers2), command)
+		}
+	}
+}
+
 func checkLayers(t *testing.T, image1, image2 string, offset int) {
 	t.Helper()
 	img1, err := getImageDetails(image1)
@@ -975,10 +1027,52 @@ func checkLayers(t *testing.T, image1, image2 string, offset int) {
 	}
 }
 
+func resolveCreatedBy(image string, layerIndex int) (string, error) {
+	ref, err := name.ParseReference(image, name.WeakValidation)
+	if err != nil {
+		return "", fmt.Errorf("Couldn't parse reference to image %s: %w", image, err)
+	}
+	imgRef, err := daemon.Image(ref)
+	if err != nil {
+		return "", fmt.Errorf("Couldn't get reference to image %s from daemon: %w", image, err)
+	}
+	cfg, err := imgRef.ConfigFile()
+	if err != nil {
+		return "", fmt.Errorf("Couldn't get Config for image %s: %w", image, err)
+	}
+	idx := 0
+	for _, history := range cfg.History {
+		if history.EmptyLayer {
+			continue
+		}
+		if idx == layerIndex {
+			return history.CreatedBy, nil
+		}
+		idx++
+	}
+	return "", fmt.Errorf("LayerIndex %d not found in History of length %d", layerIndex, len(cfg.History))
+}
+
+func getImageLayers(image string) ([]v1.Layer, error) {
+	ref, err := name.ParseReference(image, name.WeakValidation)
+	if err != nil {
+		return nil, fmt.Errorf("Couldn't parse reference to image %s: %w", image, err)
+	}
+	imgRef, err := daemon.Image(ref)
+	if err != nil {
+		return nil, fmt.Errorf("Couldn't get reference to image %s from daemon: %w", image, err)
+	}
+	layers, err := imgRef.Layers()
+	if err != nil {
+		return nil, fmt.Errorf("Error getting layers for image %s: %w", image, err)
+	}
+	return layers, nil
+}
+
 func getImageDetails(image string) (*imageDetails, error) {
 	ref, err := name.ParseReference(image, name.WeakValidation)
 	if err != nil {
-		return nil, fmt.Errorf("Couldn't parse referance to image %s: %w", image, err)
+		return nil, fmt.Errorf("Couldn't parse reference to image %s: %w", image, err)
 	}
 	imgRef, err := daemon.Image(ref)
 	if err != nil {
@@ -1002,7 +1096,7 @@ func getImageDetails(image string) (*imageDetails, error) {
 func getLastLayerFiles(image string) ([]string, error) {
 	ref, err := name.ParseReference(image, name.WeakValidation)
 	if err != nil {
-		return nil, fmt.Errorf("Couldn't parse referance to image %s: %w", image, err)
+		return nil, fmt.Errorf("Couldn't parse reference to image %s: %w", image, err)
 	}
 
 	imgRef, err := remote.Image(ref)