Don't copy same files twice in copy integration tests #273
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
howling NOOOOOO |
|
This might be https://linux.die.net/man/8/sync:
|
bobcatfish
force-pushed
the
layer_failure
branch
2 times, most recently
from
f0febb5
to
b614934
Compare
priyawadhwa
reviewed
Aug 3, 2018
pkg/util/util.go
Outdated
| @@ -45,6 +46,7 @@ func Hasher() func(string) (string, error) { | |||
| if err != nil { | |||
| return "", err | |||
| } | |||
| logrus.Infof("File %s Mode %s ModTime %s IsRegular %s\n", p, fi.Mode().String(), fi.ModTime().String(), fi.Mode().IsRegular()) | |||
There was a problem hiding this comment.
we might want to remove this or make it logrus.Debugf since info on every file every time we snapshot could make it difficult to parse through logs
There was a problem hiding this comment.
Oh yeah, I don't want to commit this, I just added this so I could see what was happening with these latest failures 😇
bobcatfish
force-pushed
the
layer_failure
branch
from
b614934
to
ed15cde
Compare
bobcatfish
changed the title
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. In a later commit (which will hopefully fix GoogleContainerTools#251) we will add a fix for this and a new test case that will intentionally exercise this functionality. In the meantime we'll prevent noisy test failures for submitters.
I _thought_ I was running into cases where Kaniko would fail to build but the test would continue... looks like it was a bug I added!
bobcatfish
force-pushed
the
layer_failure
branch
from
ed15cde
to
bb4ed13
Compare
|
Updated this PR to just stop the tests from flaking while we work on a fix for #251. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
In a later commit (which will hopefully fix #251) we will add a fix for
this and a new test case that will intentionally exercise this
functionality. In the meantime we'll prevent noisy test failures for
submitters.
Also fixed a bug I introduced into cmd.go 😅