From 76a8ab7b92cb06b24fae6c87683876a197010f50 Mon Sep 17 00:00:00 2001 From: Christie Wilson Date: Thu, 2 Aug 2018 15:41:55 -0700 Subject: [PATCH] Sync filesystem before snapshotting so mtime can be relied on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The default hashing algorithm used by kaniko to determine if two files are the same uses the files' mtime (the inode's modification time). It turns out that this time is not always up to date, meaning that a file could be modified but when you stat the file, the modification time may not yet have been updated. The copy integration tests were adding the same directory twice, the second instance being to test copying a directory with a wilcard '*'. Since the mtime is sometimes not updated, this caused kaniko to sometimes think the files were the same, and sometimes think they were different, varying the number of layers it created. Now we will update those tests to use a completely different set of files instead of copying the same files again, and we add a new test (`Dockerfile_test_copy_same_file`) which intentionally copies the same file multiple times, which would reliably reproduce the issue. We fix the issue by calling `sync` before we start comparing mtimes. This will slow down layer snapshotting - on my personal machine it costs ~30 ms to call, and added ~4 seconds to running all of the `Dockerfile_test_copy*` tests. I'm assuming that adding 30ms per layer is okay, but it's a potential place to speed things up later if we need to. Fixes #251 _Interesting note, if you build this same Dockerfile with devicemapper, you end up with only 2 layers! `¯\_(ツ)_/¯` _ --- integration/context/qux/qup | 1 + integration/context/qux/quw/que | 1 + integration/context/qux/quz | 1 + integration/dockerfiles/Dockerfile_test_copy | 2 +- .../dockerfiles/Dockerfile_test_copy_bucket | 2 +- .../Dockerfile_test_copy_reproducible | 2 +- .../Dockerfile_test_copy_same_file | 43 +++++++++++++++++++ pkg/snapshot/snapshot.go | 7 +++ 8 files changed, 56 insertions(+), 3 deletions(-) create mode 100644 integration/context/qux/qup create mode 100644 integration/context/qux/quw/que create mode 100644 integration/context/qux/quz create mode 100644 integration/dockerfiles/Dockerfile_test_copy_same_file diff --git a/integration/context/qux/qup b/integration/context/qux/qup new file mode 100644 index 0000000000..5925394647 --- /dev/null +++ b/integration/context/qux/qup @@ -0,0 +1 @@ +qup diff --git a/integration/context/qux/quw/que b/integration/context/qux/quw/que new file mode 100644 index 0000000000..b3d5d4c7d4 --- /dev/null +++ b/integration/context/qux/quw/que @@ -0,0 +1 @@ +que diff --git a/integration/context/qux/quz b/integration/context/qux/quz new file mode 100644 index 0000000000..512afe2dca --- /dev/null +++ b/integration/context/qux/quz @@ -0,0 +1 @@ +quz diff --git a/integration/dockerfiles/Dockerfile_test_copy b/integration/dockerfiles/Dockerfile_test_copy index 94d8de6344..84d2aedc33 100644 --- a/integration/dockerfiles/Dockerfile_test_copy +++ b/integration/dockerfiles/Dockerfile_test_copy @@ -9,7 +9,7 @@ COPY ./ dir/ COPY . newdir COPY context/bar /baz/ COPY ["context/foo", "/tmp/foo" ] -COPY context/b* /baz/ +COPY context/q* /qux/ COPY context/foo context/bar/ba? /test/ COPY context/arr[[]0].txt /mydir/ COPY context/bar/bat . diff --git a/integration/dockerfiles/Dockerfile_test_copy_bucket b/integration/dockerfiles/Dockerfile_test_copy_bucket index 94d8de6344..84d2aedc33 100644 --- a/integration/dockerfiles/Dockerfile_test_copy_bucket +++ b/integration/dockerfiles/Dockerfile_test_copy_bucket @@ -9,7 +9,7 @@ COPY ./ dir/ COPY . newdir COPY context/bar /baz/ COPY ["context/foo", "/tmp/foo" ] -COPY context/b* /baz/ +COPY context/q* /qux/ COPY context/foo context/bar/ba? /test/ COPY context/arr[[]0].txt /mydir/ COPY context/bar/bat . diff --git a/integration/dockerfiles/Dockerfile_test_copy_reproducible b/integration/dockerfiles/Dockerfile_test_copy_reproducible index 94d8de6344..84d2aedc33 100644 --- a/integration/dockerfiles/Dockerfile_test_copy_reproducible +++ b/integration/dockerfiles/Dockerfile_test_copy_reproducible @@ -9,7 +9,7 @@ COPY ./ dir/ COPY . newdir COPY context/bar /baz/ COPY ["context/foo", "/tmp/foo" ] -COPY context/b* /baz/ +COPY context/q* /qux/ COPY context/foo context/bar/ba? /test/ COPY context/arr[[]0].txt /mydir/ COPY context/bar/bat . diff --git a/integration/dockerfiles/Dockerfile_test_copy_same_file b/integration/dockerfiles/Dockerfile_test_copy_same_file new file mode 100644 index 0000000000..0fcc9df638 --- /dev/null +++ b/integration/dockerfiles/Dockerfile_test_copy_same_file @@ -0,0 +1,43 @@ +FROM alpine@sha256:5ce5f501c457015c4b91f91a15ac69157d9b06f1a75cf9107bf2b62e0843983a +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo +COPY context/foo /foo \ No newline at end of file diff --git a/pkg/snapshot/snapshot.go b/pkg/snapshot/snapshot.go index 8b17a354dd..cc51b008ea 100644 --- a/pkg/snapshot/snapshot.go +++ b/pkg/snapshot/snapshot.go @@ -23,6 +23,7 @@ import ( "io/ioutil" "os" "path/filepath" + "syscall" "github.com/GoogleContainerTools/kaniko/pkg/util" "github.com/sirupsen/logrus" @@ -54,6 +55,12 @@ func (s *Snapshotter) TakeSnapshot(files []string) ([]byte, error) { buf := bytes.NewBuffer([]byte{}) var filesAdded bool var err error + + // Some of the operations that follow (e.g. hashing) depend on the file system being synced, + // for example the hashing function that determines if files are equal uses the mtime of the files, + // which can lag if sync is not called. + syscall.Sync() + if files == nil { filesAdded, err = s.snapShotFS(buf) } else {