Skip to content

Commit

Permalink
image/manifest: Recursively remove pre-existing entries when unpacking
Browse files Browse the repository at this point in the history
Implementing the logic that is in-flight with [1], but using recursive
removal [2].  GNU tar has a --recursive-unlink option that's not
enabled by default, with the motivation being something like "folks
would be mad if we blew away a full tree and replaced it with a broken
symlink" [3].  That makes sense for working filesystems, but we're
building the rootfs from scratch here so losing information is not a
concern.  This commit always uses recursive removal to get that old
thing off the filesystem (whatever it takes ;).

The exception to the removal is if both the tar entry and existing
path occupant are directories.  In this case we want to use GNU tar's
default --overwrite-dir behavior, but unpackLayer's metadata handling
is currently very weak so I've left it at "don't delete the old
directory".

The reworked directory case also fixes a minor bug from 44210d0
(cmd/oci-image-tool: fix unpacking..., 2016-07-22, opencontainers#177) where the:

  if fi, err := os.Lstat(path); !(err == nil && fi.IsDir()) {

block would not error out if the Lstat failed for a reason besides the
acceptable IsNotExist.  Instead, it would attempt to call MkdirAll,
which would probably fail for the same reason that Lstat failed
(e.g. ENOTDIR).  But it's better to handle the Lstat errors directly.

[1]: opencontainers/image-spec#317
[2]: https://github.com/opencontainers/image-spec/pull/317/files#r79214718
[3]: https://www.gnu.org/software/tar/manual/html_node/Dealing-with-Old-Files.html

Signed-off-by: W. Trevor King <wking@tremily.us>
  • Loading branch information
wking committed Oct 18, 2016
1 parent f24d27b commit 6055f4d
Showing 1 changed file with 19 additions and 3 deletions.
22 changes: 19 additions & 3 deletions image/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,11 +195,27 @@ loop:
continue loop
}

if hdr.Typeflag != tar.TypeDir {
err = os.RemoveAll(path)
if err != nil && !os.IsNotExist(err) {
return err
}
}

switch hdr.Typeflag {
case tar.TypeDir:
if fi, err := os.Lstat(path); !(err == nil && fi.IsDir()) {
if err2 := os.MkdirAll(path, info.Mode()); err2 != nil {
return errors.Wrap(err2, "error creating directory")
fi, err := os.Lstat(path)
if err != nil && !os.IsNotExist(err) {
return err
}
if os.IsNotExist(err) || !fi.IsDir() {
err = os.RemoveAll(path)
if err != nil && !os.IsNotExist(err) {
return err
}
err = os.MkdirAll(path, info.Mode())
if err != nil {
return err
}
}

Expand Down

0 comments on commit 6055f4d

Please sign in to comment.