diff --git a/integration/dockerfiles/Dockerfile_test_cache b/integration/dockerfiles/Dockerfile_test_cache index f7ce19af3..0f80fbf21 100644 --- a/integration/dockerfiles/Dockerfile_test_cache +++ b/integration/dockerfiles/Dockerfile_test_cache @@ -18,5 +18,3 @@ FROM debian:10.13 RUN date > /date -COPY context/foo /foo -RUN echo hey diff --git a/integration/dockerfiles/Dockerfile_test_cache_install b/integration/dockerfiles/Dockerfile_test_cache_install index ae2feb195..5d044833d 100644 --- a/integration/dockerfiles/Dockerfile_test_cache_install +++ b/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 diff --git a/integration/dockerfiles/Dockerfile_test_cache_install_oci b/integration/dockerfiles/Dockerfile_test_cache_install_oci index ae2feb195..5d044833d 100644 --- a/integration/dockerfiles/Dockerfile_test_cache_install_oci +++ b/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 diff --git a/integration/dockerfiles/Dockerfile_test_cache_oci b/integration/dockerfiles/Dockerfile_test_cache_oci index f7ce19af3..0f80fbf21 100644 --- a/integration/dockerfiles/Dockerfile_test_cache_oci +++ b/integration/dockerfiles/Dockerfile_test_cache_oci @@ -18,5 +18,3 @@ FROM debian:10.13 RUN date > /date -COPY context/foo /foo -RUN echo hey diff --git a/integration/dockerfiles/Dockerfile_test_issue_add b/integration/dockerfiles/Dockerfile_test_issue_add new file mode 100644 index 000000000..b50f1bfb4 --- /dev/null +++ b/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 diff --git a/integration/dockerfiles/Dockerfile_test_issue_empty b/integration/dockerfiles/Dockerfile_test_issue_empty new file mode 100644 index 000000000..dfbd21b92 --- /dev/null +++ b/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 diff --git a/integration/dockerfiles/Dockerfile_test_issue_workdir b/integration/dockerfiles/Dockerfile_test_issue_workdir new file mode 100644 index 000000000..536942c39 --- /dev/null +++ b/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 diff --git a/integration/images.go b/integration/images.go index ccf675c61..148b5d6ae 100644 --- a/integration/images.go +++ b/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": {}, diff --git a/integration/integration_test.go b/integration/integration_test.go index f52489fad..9a147d7f6 100644 --- a/integration/integration_test.go +++ b/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)