-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Write parent directories to tar before whiteout files #2113
Write parent directories to tar before whiteout files #2113
Conversation
Fixes GoogleContainerTools#1149 The OCI image spec does not specify this order but it's a good idea and Docker does the same. When manually comparing layers created by Docker and Kaniko there are still some differences (that container-diff does not show): * Kaniko adds / to layers * For `mkdir /test`, docker adds `/test` and an opaque whiteout file `/test/.wh..wh..opq`. Kaniko only adds `/test/` (and /).
Fix typos and use listFilesInTar() where possible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gabyx you've been doing some work in this area, does this look good to you?
I am just wondering if I have accidentially killed a feature. I will have a look at it. |
@@ -251,6 +246,19 @@ func writeToTar(t util.Tar, files, whiteouts []string) error { | |||
return nil | |||
} | |||
|
|||
func addParentDirectories(t util.Tar, addedPaths map[string]bool, path string) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
} | ||
|
||
// Sorting does the right thing in this case. The expected order for a directory is: | ||
// Parent dirs first, then whiteout files in the directory, then other files in that directory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good comment.
@@ -599,3 +626,23 @@ func setUpTest(t *testing.T) (string, *Snapshotter, func(), error) { | |||
|
|||
return testDir, snapshotter, cleanup, nil | |||
} | |||
|
|||
func listFilesInTar(path string) ([]string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice refactor!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the kind compliments
@imjasonh Looks good to me! |
Fixes #1149
Description
This PR changes the order in which files and directories are written to tar layers such that parent directories of whiteout files are written before the whiteout files themselves.
The OCI image spec does not specify this order but it's a good idea and Docker
does the same.
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
See the contribution guide for more details.
Reviewer Notes
Release Notes