From 8c732f6f52bb334727194d090826e8dbd1df3793 Mon Sep 17 00:00:00 2001 From: Johannes 'fish' Ziemke Date: Fri, 10 May 2019 15:57:04 +0100 Subject: [PATCH] Fix kaniko caching (#639) * Revert "Change cache key calculation to be more reproducible. (#525)" This reverts commit 1ffae47fdd91d38006c1d28e14e66869742cbdba. * Add logging of composition key back * Do not include build args in cache key This should be save, given that the commands will have the args included when the cache key gets built. --- integration/dockerfiles/Dockerfile_test_cache | 14 ++------- pkg/executor/build.go | 7 +---- pkg/util/fs_util.go | 20 +++---------- pkg/util/tar_util.go | 1 - pkg/util/util.go | 29 ------------------- 5 files changed, 8 insertions(+), 63 deletions(-) diff --git a/integration/dockerfiles/Dockerfile_test_cache b/integration/dockerfiles/Dockerfile_test_cache index 71644d560a..e3ebe304dc 100644 --- a/integration/dockerfiles/Dockerfile_test_cache +++ b/integration/dockerfiles/Dockerfile_test_cache @@ -16,15 +16,7 @@ # If the image is built twice, /date should be the same in both images # if the cache is implemented correctly -FROM alpine as base_stage - -RUN mkdir foo && echo base_stage > foo/base - -FROM base_stage as cached_stage - -RUN echo cached_stage > foo/cache - -FROM cached_stage as bug_stage - -RUN echo bug_stage > foo/bug +FROM gcr.io/google-appengine/debian9@sha256:1d6a9a6d106bd795098f60f4abb7083626354fa6735e81743c7f8cfca11259f0 RUN date > /date +COPY context/foo /foo +RUN echo hey diff --git a/pkg/executor/build.go b/pkg/executor/build.go index 2fbf592cef..c29de8ca1b 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -190,12 +190,7 @@ func (s *stageBuilder) optimize(compositeKey CompositeCache, cfg v1.Config) erro func (s *stageBuilder) build() error { // Set the initial cache key to be the base image digest, the build args and the SrcContext. - dgst, err := util.ReproducibleDigest(s.image) - if err != nil { - return err - } - compositeKey := NewCompositeCache(dgst) - compositeKey.AddKey(s.opts.BuildArgs...) + compositeKey := NewCompositeCache(s.baseImageDigest) // Apply optimizations to the instructions. if err := s.optimize(*compositeKey, s.cf.Config); err != nil { diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index 5ef48845fc..5f72c7784e 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -79,10 +79,7 @@ func GetFSFromImage(root string, img v1.Image) ([]string, error) { if err != nil { return nil, err } - - // Store a map of files to their mtime. We need to set mtimes in a second pass because creating files - // can change the mtime of a directory. - extractedFiles := map[string]time.Time{} + extractedFiles := []string{} for i, l := range layers { logrus.Debugf("Extracting layer %d", i) @@ -113,17 +110,10 @@ func GetFSFromImage(root string, img v1.Image) ([]string, error) { if err := extractFile(root, hdr, tr); err != nil { return nil, err } - extractedFiles[filepath.Join(root, filepath.Clean(hdr.Name))] = hdr.ModTime + extractedFiles = append(extractedFiles, filepath.Join(root, filepath.Clean(hdr.Name))) } } - - fileNames := []string{} - for f, t := range extractedFiles { - fileNames = append(fileNames, f) - os.Chtimes(f, time.Time{}, t) - } - - return fileNames, nil + return extractedFiles, nil } // DeleteFilesystem deletes the extracted image file system @@ -272,7 +262,6 @@ func extractFile(dest string, hdr *tar.Header, tr io.Reader) error { return err } } - return nil } @@ -377,8 +366,7 @@ func RelativeFiles(fp string, root string) ([]string, error) { } // ParentDirectories returns a list of paths to all parent directories -// Ex. /some/temp/dir -> [/some, /some/temp, /some/temp/dir] -// This purposefully excludes the /. +// Ex. /some/temp/dir -> [/, /some, /some/temp, /some/temp/dir] func ParentDirectories(path string) []string { path = filepath.Clean(path) dirs := strings.Split(path, "/") diff --git a/pkg/util/tar_util.go b/pkg/util/tar_util.go index b727857075..bc1cc67a0c 100644 --- a/pkg/util/tar_util.go +++ b/pkg/util/tar_util.go @@ -54,7 +54,6 @@ func (t *Tar) Close() { // AddFileToTar adds the file at path p to the tar func (t *Tar) AddFileToTar(p string) error { - logrus.Debugf("Adding file %s to tar", p) i, err := os.Lstat(p) if err != nil { return fmt.Errorf("Failed to get file info for %s: %s", p, err) diff --git a/pkg/util/util.go b/pkg/util/util.go index e7344c88fc..873cbae20e 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -20,14 +20,11 @@ import ( "crypto/md5" "crypto/sha256" "encoding/hex" - "encoding/json" "io" "os" "strconv" "syscall" - "github.com/google/go-containerregistry/pkg/v1" - "github.com/google/go-containerregistry/pkg/v1/partial" "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -130,29 +127,3 @@ func SHA256(r io.Reader) (string, error) { } return hex.EncodeToString(hasher.Sum(make([]byte, 0, hasher.Size()))), nil } - -type ReproducibleManifest struct { - Layers []v1.Descriptor - Config v1.Config -} - -func ReproducibleDigest(img partial.WithManifestAndConfigFile) (string, error) { - mfst, err := img.Manifest() - if err != nil { - return "", err - } - cfg, err := img.ConfigFile() - if err != nil { - return "", err - } - rm := ReproducibleManifest{ - Layers: mfst.Layers, - Config: cfg.Config, - } - - b, err := json.Marshal(rm) - if err != nil { - return "", err - } - return string(b), nil -}