From 818c249bae8d29842834bf765299c86c09e6913e Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Fri, 13 Dec 2013 15:43:50 +0100 Subject: [PATCH 1/4] archive: Implement ApplyLayer directly Rather than calling out to tar we use the golang tar parser to directly extract the tar files. This has two major advantages: 1) We're able to replace an existing directory with a file in the new layer. This currently breaks with the external tar, since it refuses to recursively remove the destination directory in this case, and there are no options to make it do that. 2) We avoid extracting the whiteout files just to later remove them. --- archive/diff.go | 215 +++++++++++++++++++++++++++-------------- archive/stat_darwin.go | 4 + archive/stat_linux.go | 23 ++++- 3 files changed, 171 insertions(+), 71 deletions(-) diff --git a/archive/diff.go b/archive/diff.go index f44991ecb5..18740fe571 100644 --- a/archive/diff.go +++ b/archive/diff.go @@ -1,6 +1,9 @@ package archive import ( + "archive/tar" + "github.com/dotcloud/docker/utils" + "io" "os" "path/filepath" "strings" @@ -8,87 +11,159 @@ import ( "time" ) +// Linux device nodes are a bit weird due to backwards compat with 16 bit device nodes +// The lower 8 bit is the lower 8 bit in the minor, the following 12 bits are the major, +// and then there is the top 12 bits of then minor +func mkdev(major int64, minor int64) uint32 { + return uint32(((minor & 0xfff00) << 12) | ((major & 0xfff) << 8) | (minor & 0xff)) +} +func timeToTimespec(time time.Time) (ts syscall.Timespec) { + if time.IsZero() { + // Return UTIME_OMIT special value + ts.Sec = 0 + ts.Nsec = ((1 << 30) - 2) + return + } + return syscall.NsecToTimespec(time.UnixNano()) +} + // ApplyLayer parses a diff in the standard layer format from `layer`, and // applies it to the directory `dest`. func ApplyLayer(dest string, layer Archive) error { - // Poor man's diff applyer in 2 steps: + // We need to be able to set any perms + oldmask := syscall.Umask(0) + defer syscall.Umask(oldmask) - // Step 1: untar everything in place - if err := Untar(layer, dest, nil); err != nil { - return err - } + tr := tar.NewReader(layer) - modifiedDirs := make(map[string]*syscall.Stat_t) - addDir := func(file string) { - d := filepath.Dir(file) - if _, exists := modifiedDirs[d]; !exists { - if s, err := os.Lstat(d); err == nil { - if sys := s.Sys(); sys != nil { - if stat, ok := sys.(*syscall.Stat_t); ok { - modifiedDirs[d] = stat + var dirs []*tar.Header + + // Iterate through the files in the archive. + for { + hdr, err := tr.Next() + if err == io.EOF { + // end of tar archive + break + } + if err != nil { + return err + } + + // Skip AUFS metadata dirs + if strings.HasPrefix(hdr.Name, ".wh..wh.") { + continue + } + + path := filepath.Join(dest, hdr.Name) + base := filepath.Base(path) + if strings.HasPrefix(base, ".wh.") { + originalBase := base[len(".wh."):] + originalPath := filepath.Join(filepath.Dir(path), originalBase) + if err := os.RemoveAll(originalPath); err != nil { + return err + } + } else { + // If path exits we almost always just want to remove and replace it + // The only exception is when it is a directory *and* the file from + // the layer is also a directory. Then we want to merge them (i.e. + // just apply the metadata from the layer). + hasDir := false + if fi, err := os.Lstat(path); err == nil { + if fi.IsDir() && hdr.Typeflag == tar.TypeDir { + hasDir = true + } else { + if err := os.RemoveAll(path); err != nil { + return err + } + } + } + + switch hdr.Typeflag { + case tar.TypeDir: + if !hasDir { + err = os.Mkdir(path, os.FileMode(hdr.Mode)) + if err != nil { + return err + } + } + dirs = append(dirs, hdr) + + case tar.TypeReg, tar.TypeRegA: + // Source is regular file + file, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY, os.FileMode(hdr.Mode)) + if err != nil { + return err + } + if _, err := io.Copy(file, tr); err != nil { + file.Close() + return err + } + file.Close() + + case tar.TypeBlock, tar.TypeChar, tar.TypeFifo: + mode := uint32(hdr.Mode & 07777) + switch hdr.Typeflag { + case tar.TypeBlock: + mode |= syscall.S_IFBLK + case tar.TypeChar: + mode |= syscall.S_IFCHR + case tar.TypeFifo: + mode |= syscall.S_IFIFO + } + + if err := syscall.Mknod(path, mode, int(mkdev(hdr.Devmajor, hdr.Devminor))); err != nil { + return err + } + + case tar.TypeLink: + if err := os.Link(filepath.Join(dest, hdr.Linkname), path); err != nil { + return err + } + + case tar.TypeSymlink: + if err := os.Symlink(hdr.Linkname, path); err != nil { + return err + } + + default: + utils.Debugf("unhandled type %d\n", hdr.Typeflag) + } + + if err = syscall.Lchown(path, hdr.Uid, hdr.Gid); err != nil { + return err + } + + // There is no LChmod, so ignore mode for symlink. Also, this + // must happen after chown, as that can modify the file mode + if hdr.Typeflag != tar.TypeSymlink { + err = syscall.Chmod(path, uint32(hdr.Mode&07777)) + if err != nil { + return err + } + } + + // Directories must be handled at the end to avoid further + // file creation in them to modify the mtime + if hdr.Typeflag != tar.TypeDir { + ts := []syscall.Timespec{timeToTimespec(hdr.AccessTime), timeToTimespec(hdr.ModTime)} + // syscall.UtimesNano doesn't support a NOFOLLOW flag atm, and + if hdr.Typeflag != tar.TypeSymlink { + if err := syscall.UtimesNano(path, ts); err != nil { + return err + } + } else { + if err := LUtimesNano(path, ts); err != nil { + return err } } } } } - // Step 2: walk for whiteouts and apply them, removing them in the process - err := filepath.Walk(dest, func(fullPath string, f os.FileInfo, err error) error { - if err != nil { - if os.IsNotExist(err) { - // This happens in the case of whiteouts in parent dir removing a directory - // We just ignore it - return filepath.SkipDir - } - return err - } - - // Rebase path - path, err := filepath.Rel(dest, fullPath) - if err != nil { - return err - } - path = filepath.Join("/", path) - - // Skip AUFS metadata - if matched, err := filepath.Match("/.wh..wh.*", path); err != nil { - return err - } else if matched { - addDir(fullPath) - if err := os.RemoveAll(fullPath); err != nil { - return err - } - } - - filename := filepath.Base(path) - if strings.HasPrefix(filename, ".wh.") { - rmTargetName := filename[len(".wh."):] - rmTargetPath := filepath.Join(filepath.Dir(fullPath), rmTargetName) - - // Remove the file targeted by the whiteout - addDir(rmTargetPath) - if err := os.RemoveAll(rmTargetPath); err != nil { - return err - } - // Remove the whiteout itself - addDir(fullPath) - if err := os.RemoveAll(fullPath); err != nil { - return err - } - } - return nil - }) - if err != nil { - return err - } - - for k, v := range modifiedDirs { - lastAccess := getLastAccess(v) - lastModification := getLastModification(v) - aTime := time.Unix(lastAccess.Unix()) - mTime := time.Unix(lastModification.Unix()) - - if err := os.Chtimes(k, aTime, mTime); err != nil { + for _, hdr := range dirs { + path := filepath.Join(dest, hdr.Name) + ts := []syscall.Timespec{timeToTimespec(hdr.AccessTime), timeToTimespec(hdr.ModTime)} + if err := syscall.UtimesNano(path, ts); err != nil { return err } } diff --git a/archive/stat_darwin.go b/archive/stat_darwin.go index 53ae9dee2f..e041783ec6 100644 --- a/archive/stat_darwin.go +++ b/archive/stat_darwin.go @@ -9,3 +9,7 @@ func getLastAccess(stat *syscall.Stat_t) syscall.Timespec { func getLastModification(stat *syscall.Stat_t) syscall.Timespec { return stat.Mtimespec } + +func LUtimesNano(path string, ts []syscall.Timespec) error { + return nil +} diff --git a/archive/stat_linux.go b/archive/stat_linux.go index 50b4627c4a..2203a46aff 100644 --- a/archive/stat_linux.go +++ b/archive/stat_linux.go @@ -1,6 +1,9 @@ package archive -import "syscall" +import ( + "syscall" + "unsafe" +) func getLastAccess(stat *syscall.Stat_t) syscall.Timespec { return stat.Atim @@ -9,3 +12,21 @@ func getLastAccess(stat *syscall.Stat_t) syscall.Timespec { func getLastModification(stat *syscall.Stat_t) syscall.Timespec { return stat.Mtim } + +func LUtimesNano(path string, ts []syscall.Timespec) error { + // These are not currently availible in syscall + AT_FDCWD := -100 + AT_SYMLINK_NOFOLLOW := 0x100 + + var _path *byte + _path, err := syscall.BytePtrFromString(path) + if err != nil { + return err + } + + if _, _, err := syscall.Syscall6(syscall.SYS_UTIMENSAT, uintptr(AT_FDCWD), uintptr(unsafe.Pointer(_path)), uintptr(unsafe.Pointer(&ts[0])), uintptr(AT_SYMLINK_NOFOLLOW), 0, 0); err != 0 && err != syscall.ENOSYS { + return err + } + + return nil +} From 10cd902f900392a2f10a6f8763bba70607ea0d41 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Fri, 13 Dec 2013 15:46:41 +0100 Subject: [PATCH 2/4] Fix change detection when applying tar layers The default gnu tar format has no sub-second precision mtime support, and the golang tar writer currently doesn't support that either. This means if we export the changes from a container we will not get zeron in the sub-second precision field when the change is applied. This means we can't compare that to the original without getting a spurious change. So, we detect this case by treating a case where the seconds match and either of the two nanoseconds are zero as equal. --- archive/changes.go | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/archive/changes.go b/archive/changes.go index a4076fc0ad..8fe9ff2233 100644 --- a/archive/changes.go +++ b/archive/changes.go @@ -6,6 +6,7 @@ import ( "path/filepath" "strings" "syscall" + "time" ) type ChangeType int @@ -34,6 +35,21 @@ func (change *Change) String() string { return fmt.Sprintf("%s %s", kind, change.Path) } +// Gnu tar and the go tar writer don't have sub-second mtime +// precision, which is problematic when we apply changes via tar +// files, we handle this by comparing for exact times, *or* same +// second count and either a or b having exactly 0 nanoseconds +func sameFsTime(a, b time.Time) bool { + return a == b || + (a.Unix() == b.Unix() && + (a.Nanosecond() == 0 || b.Nanosecond() == 0)) +} + +func sameFsTimeSpec(a, b syscall.Timespec) bool { + return a.Sec == b.Sec && + (a.Nsec == b.Nsec || a.Nsec == 0 || b.Nsec == 0) +} + func Changes(layers []string, rw string) ([]Change, error) { var changes []Change err := filepath.Walk(rw, func(path string, f os.FileInfo, err error) error { @@ -85,7 +101,7 @@ func Changes(layers []string, rw string) ([]Change, error) { // However, if it's a directory, maybe it wasn't actually modified. // If you modify /foo/bar/baz, then /foo will be part of the changed files only because it's the parent of bar if stat.IsDir() && f.IsDir() { - if f.Size() == stat.Size() && f.Mode() == stat.Mode() && f.ModTime() == stat.ModTime() { + if f.Size() == stat.Size() && f.Mode() == stat.Mode() && sameFsTime(f.ModTime(), stat.ModTime()) { // Both directories are the same, don't record the change return nil } @@ -181,7 +197,7 @@ func (info *FileInfo) addChanges(oldInfo *FileInfo, changes *[]Change) { oldStat.Rdev != newStat.Rdev || // Don't look at size for dirs, its not a good measure of change (oldStat.Size != newStat.Size && oldStat.Mode&syscall.S_IFDIR != syscall.S_IFDIR) || - getLastModification(oldStat) != getLastModification(newStat) { + !sameFsTimeSpec(getLastModification(oldStat), getLastModification(newStat)) { change := Change{ Path: newChild.path(), Kind: ChangeModify, From a8af12f80a4a1678988b4667e5211d4e576ce903 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Fri, 13 Dec 2013 15:50:25 +0100 Subject: [PATCH 3/4] Re-enable TestApplyLayer With the previous two changes we now pass this test. --- archive/changes_test.go | 72 +++++++++++++++++++---------------------- 1 file changed, 34 insertions(+), 38 deletions(-) diff --git a/archive/changes_test.go b/archive/changes_test.go index 714ab71e2d..e11ed90ac1 100644 --- a/archive/changes_test.go +++ b/archive/changes_test.go @@ -258,48 +258,44 @@ func TestChangesDirsMutated(t *testing.T) { } func TestApplyLayer(t *testing.T) { - t.Skip("Skipping TestApplyLayer due to known failures") // Disable this for now as it is broken - return + src, err := ioutil.TempDir("", "docker-changes-test") + if err != nil { + t.Fatal(err) + } + createSampleDir(t, src) + defer os.RemoveAll(src) + dst := src + "-copy" + if err := copyDir(src, dst); err != nil { + t.Fatal(err) + } + mutateSampleDir(t, dst) + defer os.RemoveAll(dst) - // src, err := ioutil.TempDir("", "docker-changes-test") - // if err != nil { - // t.Fatal(err) - // } - // createSampleDir(t, src) - // dst := src + "-copy" - // if err := copyDir(src, dst); err != nil { - // t.Fatal(err) - // } - // mutateSampleDir(t, dst) + changes, err := ChangesDirs(dst, src) + if err != nil { + t.Fatal(err) + } - // changes, err := ChangesDirs(dst, src) - // if err != nil { - // t.Fatal(err) - // } + layer, err := ExportChanges(dst, changes) + if err != nil { + t.Fatal(err) + } - // layer, err := ExportChanges(dst, changes) - // if err != nil { - // t.Fatal(err) - // } + layerCopy, err := NewTempArchive(layer, "") + if err != nil { + t.Fatal(err) + } - // layerCopy, err := NewTempArchive(layer, "") - // if err != nil { - // t.Fatal(err) - // } + if err := ApplyLayer(src, layerCopy); err != nil { + t.Fatal(err) + } - // if err := ApplyLayer(src, layerCopy); err != nil { - // t.Fatal(err) - // } + changes2, err := ChangesDirs(src, dst) + if err != nil { + t.Fatal(err) + } - // changes2, err := ChangesDirs(src, dst) - // if err != nil { - // t.Fatal(err) - // } - - // if len(changes2) != 0 { - // t.Fatalf("Unexpected differences after re applying mutation: %v", changes) - // } - - // os.RemoveAll(src) - // os.RemoveAll(dst) + if len(changes2) != 0 { + t.Fatalf("Unexpected differences after re applying mutation: %v", changes2) + } } From 78c22c24b353d77fdab3e1616d9986a8ae95a7c2 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Mon, 16 Dec 2013 14:35:43 +0100 Subject: [PATCH 4/4] ApplyLayer: Fix TestLookupImage The TestLookupImage test seems to use a layer that contains /etc/postgres/postgres.conf, but not e.g. /etc/postgres. To handle this we ensure that the parent directory always exists, and if not we create it. --- archive/diff.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/archive/diff.go b/archive/diff.go index 18740fe571..87457f38c1 100644 --- a/archive/diff.go +++ b/archive/diff.go @@ -49,6 +49,23 @@ func ApplyLayer(dest string, layer Archive) error { return err } + // Normalize name, for safety and for a simple is-root check + hdr.Name = filepath.Clean(hdr.Name) + + if !strings.HasSuffix(hdr.Name, "/") { + // Not the root directory, ensure that the parent directory exists + // This happened in some tests where an image had a tarfile without any + // parent directories + parent := filepath.Dir(hdr.Name) + parentPath := filepath.Join(dest, parent) + if _, err := os.Lstat(parentPath); err != nil && os.IsNotExist(err) { + err = os.MkdirAll(parentPath, 600) + if err != nil { + return err + } + } + } + // Skip AUFS metadata dirs if strings.HasPrefix(hdr.Name, ".wh..wh.") { continue