From 8b312bddbe566d249b9f3962119a20e415f574be Mon Sep 17 00:00:00 2001 From: Wei Fu Date: Sat, 23 Nov 2019 22:45:59 +0800 Subject: [PATCH] fs: add DiffDirChanges function to get changeset fast Since AUFS/OverlayFS can persist changeset in diff directory, DiffDirChanges function can retrieve layer changeset from diff directory without walking the whole rootfs directory. Signed-off-by: Wei Fu --- Makefile | 2 +- fs/diff.go | 87 +++++++++++++++++++++-------- fs/diff_linux.go | 101 +++++++++++++++++++++++++++++++++ fs/diff_nonlinux.go | 29 ++++++++++ fs/diff_test.go | 114 ++++++++++++++++++++++++++++++++++++++ fs/diff_unix.go | 10 ---- fs/diff_windows.go | 4 -- fs/fstest/file_unix.go | 10 ++++ fs/fstest/file_windows.go | 15 +++++ 9 files changed, 333 insertions(+), 39 deletions(-) create mode 100644 fs/diff_linux.go create mode 100644 fs/diff_nonlinux.go diff --git a/Makefile b/Makefile index 63ab8519..a622e939 100644 --- a/Makefile +++ b/Makefile @@ -57,7 +57,7 @@ test: root-test: @echo "+ $@" - @go test -exec sudo ${TEST_REQUIRES_ROOT_PACKAGES} -test.root + @go test -exec sudo ${TEST_REQUIRES_ROOT_PACKAGES} -test.root -test.v test-compile: @echo "+ $@" diff --git a/fs/diff.go b/fs/diff.go index d2c3c568..848bc506 100644 --- a/fs/diff.go +++ b/fs/diff.go @@ -18,6 +18,7 @@ package fs import ( "context" + "errors" "os" "path/filepath" "strings" @@ -102,9 +103,6 @@ func Changes(ctx context.Context, a, b string, changeFn ChangeFunc) error { if a == "" { logrus.Debugf("Using single walk diff for %s", b) return addDirChanges(ctx, changeFn, b) - } else if diffOptions := detectDirDiff(b, a); diffOptions != nil { - logrus.Debugf("Using single walk diff for %s from %s", diffOptions.diffDir, a) - return diffDirChanges(ctx, changeFn, a, diffOptions) } logrus.Debugf("Using double walk diff for %s from %s", b, a) @@ -134,24 +132,53 @@ func addDirChanges(ctx context.Context, changeFn ChangeFunc, root string) error }) } +// DiffChangeSource is the source of diff directory. +type DiffSource int + +const ( + // DiffSourceOverlayFS indicates that a diff directory is from + // OverlayFS. + DiffSourceOverlayFS DiffSource = iota +) + // diffDirOptions is used when the diff can be directly calculated from // a diff directory to its base, without walking both trees. type diffDirOptions struct { - diffDir string - skipChange func(string) (bool, error) - deleteChange func(string, string, os.FileInfo) (string, error) + skipChange func(string, os.FileInfo) (bool, error) + deleteChange func(string, string, os.FileInfo, ChangeFunc) (bool, error) } -// diffDirChanges walks the diff directory and compares changes against the base. -func diffDirChanges(ctx context.Context, changeFn ChangeFunc, base string, o *diffDirOptions) error { +// DiffDirChanges walks the diff directory and compares changes against the base. +// +// NOTE: If all the children of a dir are removed, or that dir are recreated +// after remove, we will mark non-existing `.wh..opq` file as deleted. It's +// unlikely to create explicit whiteout files for all the children and all +// descendants. And based on OCI spec, it's not possible to create a file or +// dir with a name beginning with `.wh.`. So, after `.wh..opq` file has been +// deleted, the ChangeFunc, the receiver will add whiteout prefix to create a +// opaque whiteout `.wh..wh..opq`. +// +// REF: https://github.com/opencontainers/image-spec/blob/v1.0/layer.md#whiteouts +func DiffDirChanges(ctx context.Context, baseDir, diffDir string, source DiffSource, changeFn ChangeFunc) error { + var o *diffDirOptions + + switch source { + case DiffSourceOverlayFS: + o = &diffDirOptions{ + deleteChange: overlayFSWhiteoutConvert, + } + default: + return errors.New("unknown diff change source") + } + changedDirs := make(map[string]struct{}) - return filepath.Walk(o.diffDir, func(path string, f os.FileInfo, err error) error { + return filepath.Walk(diffDir, func(path string, f os.FileInfo, err error) error { if err != nil { return err } // Rebase path - path, err = filepath.Rel(o.diffDir, path) + path, err = filepath.Rel(diffDir, path) if err != nil { return err } @@ -163,38 +190,45 @@ func diffDirChanges(ctx context.Context, changeFn ChangeFunc, base string, o *di return nil } - // TODO: handle opaqueness, start new double walker at this - // location to get deletes, and skip tree in single walker - if o.skipChange != nil { - if skip, err := o.skipChange(path); skip { + if skip, err := o.skipChange(path, f); skip { return err } } var kind ChangeKind - deletedFile, err := o.deleteChange(o.diffDir, path, f) - if err != nil { - return err + deletedFile := false + + if o.deleteChange != nil { + deletedFile, err = o.deleteChange(diffDir, path, f, changeFn) + if err != nil { + return err + } + + _, err = os.Stat(filepath.Join(baseDir, path)) + if err != nil { + if !os.IsNotExist(err) { + return err + } + deletedFile = false + } } // Find out what kind of modification happened - if deletedFile != "" { - path = deletedFile + if deletedFile { kind = ChangeKindDelete - f = nil } else { // Otherwise, the file was added kind = ChangeKindAdd - // ...Unless it already existed in a base, in which case, it's a modification - stat, err := os.Stat(filepath.Join(base, path)) + // ...Unless it already existed in a baseDir, in which case, it's a modification + stat, err := os.Stat(filepath.Join(baseDir, path)) if err != nil && !os.IsNotExist(err) { return err } if err == nil { - // The file existed in the base, so that's a modification + // The file existed in the baseDir, so that's a modification // 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 @@ -215,10 +249,12 @@ func diffDirChanges(ctx context.Context, changeFn ChangeFunc, base string, o *di if f.IsDir() { changedDirs[path] = struct{}{} } + if kind == ChangeKindAdd || kind == ChangeKindDelete { parent := filepath.Dir(path) + if _, ok := changedDirs[parent]; !ok && parent != "/" { - pi, err := os.Stat(filepath.Join(o.diffDir, parent)) + pi, err := os.Stat(filepath.Join(diffDir, parent)) if err := changeFn(ChangeKindModify, parent, pi, err); err != nil { return err } @@ -226,6 +262,9 @@ func diffDirChanges(ctx context.Context, changeFn ChangeFunc, base string, o *di } } + if kind == ChangeKindDelete { + f = nil + } return changeFn(kind, path, f, nil) }) } diff --git a/fs/diff_linux.go b/fs/diff_linux.go new file mode 100644 index 00000000..376f13c2 --- /dev/null +++ b/fs/diff_linux.go @@ -0,0 +1,101 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package fs + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "syscall" + + "github.com/containerd/continuity/devices" + "github.com/containerd/continuity/sysx" + + "golang.org/x/sys/unix" +) + +const ( + // whiteoutPrefix prefix means file is a whiteout. If this is followed + // by a filename this means that file has been removed from the base + // layer. + // + // See https://github.com/opencontainers/image-spec/blob/master/layer.md#whiteouts + whiteoutPrefix = ".wh." +) + +// overlayFSWhiteoutConvert detects whiteouts and opaque directories. +// +// It returns deleted indicator if the file is a character device with 0/0 +// device number. And call changeFn with ChangeKindDelete for opaque +// directories. +// +// Check: https://www.kernel.org/doc/Documentation/filesystems/overlayfs.txt +func overlayFSWhiteoutConvert(diffDir, path string, f os.FileInfo, changeFn ChangeFunc) (deleted bool, _ error) { + if f.Mode()&os.ModeCharDevice != 0 { + if _, ok := f.Sys().(*syscall.Stat_t); !ok { + return false, nil + } + + maj, min, err := devices.DeviceInfo(f) + if err != nil { + return false, err + } + return (maj == 0 && min == 0), nil + } + + if f.IsDir() { + originalPath := filepath.Join(diffDir, path) + opaque, err := getOpaqueValue(originalPath) + if err != nil { + if errors.Is(err, unix.ENODATA) { + return false, nil + } + return false, err + } + + if len(opaque) == 1 && opaque[0] == 'y' { + opaqueDirPath := filepath.Join(path, whiteoutPrefix+".opq") + return false, changeFn(ChangeKindDelete, opaqueDirPath, nil, nil) + } + } + return false, nil +} + +// getOpaqueValue returns opaque value for a given file. +func getOpaqueValue(filePath string) ([]byte, error) { + for _, xattr := range []string{ + "trusted.overlay.opaque", + // TODO(fuweid): + // + // user.overlay.* is available since 5.11. We should check + // kernel version before read. + // + // REF: https://github.com/torvalds/linux/commit/2d2f2d7322ff43e0fe92bf8cccdc0b09449bf2e1 + "user.overlay.opaque", + } { + opaque, err := sysx.LGetxattr(filePath, xattr) + if err != nil { + if errors.Is(err, unix.ENODATA) || errors.Is(err, unix.ENOTSUP) { + continue + } + return nil, fmt.Errorf("failed to retrieve %s attr: %w", xattr, err) + } + return opaque, nil + } + return nil, unix.ENODATA +} diff --git a/fs/diff_nonlinux.go b/fs/diff_nonlinux.go new file mode 100644 index 00000000..abf9920b --- /dev/null +++ b/fs/diff_nonlinux.go @@ -0,0 +1,29 @@ +//go:build !linux +// +build !linux + +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package fs + +import ( + "errors" + "os" +) + +func overlayFSWhiteoutConvert(string, string, os.FileInfo, ChangeFunc) (bool, error) { + return false, errors.New("unsupported") +} diff --git a/fs/diff_test.go b/fs/diff_test.go index 395ee2a1..191ba21d 100644 --- a/fs/diff_test.go +++ b/fs/diff_test.go @@ -22,11 +22,13 @@ import ( "os" "path/filepath" "runtime" + "sort" "strings" "testing" "time" "github.com/containerd/continuity/fs/fstest" + "github.com/containerd/continuity/testutil" ) // TODO: Additional tests @@ -41,6 +43,12 @@ func skipDiffTestOnWindows(t *testing.T) { } } +func skipDiffTestOnNonLinux(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skipf("diff implementation is incomplete on %s", runtime.GOOS) + } +} + func TestSimpleDiff(t *testing.T) { skipDiffTestOnWindows(t) l1 := fstest.Apply( @@ -192,6 +200,56 @@ func TestFileReplace(t *testing.T) { } } +func TestDiffDirChangeWithOverlayfs(t *testing.T) { + skipDiffTestOnNonLinux(t) + testutil.RequiresRoot(t) + + l1 := fstest.Apply( + fstest.CreateDir("/dir1", 0700), + fstest.CreateFile("/dir1/f", []byte("/dir1/f"), 0644), + fstest.CreateDir("/dir1/d", 0700), + fstest.CreateFile("/dir1/d/f", []byte("/dir1/d/f"), 0644), + + fstest.CreateDir("/dir2", 0700), + fstest.CreateDir("/dir2/d", 0700), + fstest.CreateFile("/dir2/d/f", []byte("/dir2/d/f"), 0644), + + fstest.CreateDir("/dir3", 0700), + fstest.CreateFile("/dir3/f", []byte("/dir3/f"), 0644), + ) + + l2 := fstest.Apply( + fstest.CreateDir("/dir1", 0700), + fstest.CreateFile("/dir1/f", []byte("/dir1/f-diff"), 0644), + fstest.CreateDeviceFile("/dir1/d", os.ModeDevice|os.ModeCharDevice, 0, 0), + + fstest.CreateDir("/dir2", 0700), + fstest.CreateDir("/dir2/d", 0700), + fstest.CreateFile("/dir2/d/f", []byte("/dir2/d/f-diff"), 0644), + + fstest.CreateDir("/dir3", 0700), + // TODO(fuweid): check kernel version before apply + fstest.SetXAttr("/dir3", "user.overlay.opaque", "y"), + ) + + diff := []TestChange{ + Modify("/dir1"), + Modify("/dir1/f"), + Delete("/dir1/d"), + + Modify("/dir2"), + Modify("/dir2/d"), + Modify("/dir2/d/f"), + + Modify("/dir3"), + Delete("/dir3/.wh..opq"), + } + + if err := testDiffDirChange(l1, l2, DiffSourceOverlayFS, diff); err != nil { + t.Fatalf("failed diff dir change: %+v", err) + } +} + func TestParentDirectoryPermission(t *testing.T) { skipDiffTestOnWindows(t) l1 := fstest.Apply( @@ -350,7 +408,43 @@ func testDiffWithoutBase(t testing.TB, apply fstest.Applier, expected []TestChan return checkChanges(tmp, changes, expected) } +func testDiffDirChange(base, diff fstest.Applier, source DiffSource, expected []TestChange) error { + baseTmp, err := os.MkdirTemp("", "fast-diff-base-") + if err != nil { + return fmt.Errorf("failed to create temp dir: %w", err) + } + defer os.RemoveAll(baseTmp) + + diffTmp, err := os.MkdirTemp("", "fast-diff-diff-") + if err != nil { + return fmt.Errorf("failed to create temp dir: %w", err) + } + defer os.RemoveAll(diffTmp) + + if err := base.Apply(baseTmp); err != nil { + return fmt.Errorf("failed to apply filesytem changes: %w", err) + } + + if err := diff.Apply(diffTmp); err != nil { + return fmt.Errorf("failed to apply filesytem changes: %w", err) + } + + changes, err := collectDiffDirChanges(baseTmp, diffTmp, source) + if err != nil { + return fmt.Errorf("failed to collect diff dir changes: %w", err) + } + return checkChanges(diffTmp, changes, expected) +} + func checkChanges(root string, changes, expected []TestChange) error { + sort.Slice(changes, func(i, j int) bool { + return changes[i].Path < changes[j].Path + }) + + sort.Slice(expected, func(i, j int) bool { + return expected[i].Path < expected[j].Path + }) + if len(changes) != len(expected) { return fmt.Errorf("Unexpected number of changes:\n%s", diffString(changes, expected)) } @@ -411,6 +505,26 @@ func collectChanges(a, b string) ([]TestChange, error) { return changes, nil } +func collectDiffDirChanges(baseDir, diffDir string, source DiffSource) ([]TestChange, error) { + changes := []TestChange{} + err := DiffDirChanges(context.Background(), baseDir, diffDir, source, func(k ChangeKind, p string, f os.FileInfo, err error) error { + if err != nil { + return err + } + changes = append(changes, TestChange{ + Kind: k, + Path: p, + FileInfo: f, + Source: filepath.Join(diffDir, p), + }) + return nil + }) + if err != nil { + return nil, fmt.Errorf("failed to compute changes: %w", err) + } + return changes, nil +} + func diffString(c1, c2 []TestChange) string { return fmt.Sprintf("got(%d):\n%s\nexpected(%d):\n%s", len(c1), changesString(c1), len(c2), changesString(c2)) } diff --git a/fs/diff_unix.go b/fs/diff_unix.go index 5de9b6b4..d4897554 100644 --- a/fs/diff_unix.go +++ b/fs/diff_unix.go @@ -28,16 +28,6 @@ import ( "github.com/containerd/continuity/sysx" ) -// detectDirDiff returns diff dir options if a directory could -// be found in the mount info for upper which is the direct -// diff with the provided lower directory -func detectDirDiff(upper, lower string) *diffDirOptions { - // TODO: get mount options for upper - // TODO: detect AUFS - // TODO: detect overlay - return nil -} - // compareSysStat returns whether the stats are equivalent, // whether the files are considered the same file, and // an error diff --git a/fs/diff_windows.go b/fs/diff_windows.go index 4bfa72d3..63580c23 100644 --- a/fs/diff_windows.go +++ b/fs/diff_windows.go @@ -22,10 +22,6 @@ import ( "golang.org/x/sys/windows" ) -func detectDirDiff(upper, lower string) *diffDirOptions { - return nil -} - func compareSysStat(s1, s2 interface{}) (bool, error) { f1, ok := s1.(windows.Win32FileAttributeData) if !ok { diff --git a/fs/fstest/file_unix.go b/fs/fstest/file_unix.go index b3ea0931..fe993ed8 100644 --- a/fs/fstest/file_unix.go +++ b/fs/fstest/file_unix.go @@ -20,9 +20,11 @@ package fstest import ( + "os" "path/filepath" "time" + "github.com/containerd/continuity/devices" "github.com/containerd/continuity/sysx" "golang.org/x/sys/unix" ) @@ -46,6 +48,14 @@ func Lchtimes(name string, atime, mtime time.Time) Applier { }) } +// CreateDeviceFile provides creates devices Applier. +func CreateDeviceFile(name string, mode os.FileMode, maj, min int) Applier { + return applyFn(func(root string) error { + fullPath := filepath.Join(root, name) + return devices.Mknod(fullPath, mode, maj, min) + }) +} + func Base() Applier { return applyFn(func(root string) error { // do nothing, as the base is not special diff --git a/fs/fstest/file_windows.go b/fs/fstest/file_windows.go index 6fd9ee8d..3833169c 100644 --- a/fs/fstest/file_windows.go +++ b/fs/fstest/file_windows.go @@ -18,9 +18,17 @@ package fstest import ( "errors" + "os" "time" ) +// SetXAttr sets the xatter for the file +func SetXAttr(name, key, value string) Applier { + return applyFn(func(root string) error { + return errors.New("Not implemented") + }) +} + // Lchtimes changes access and mod time of file without following symlink func Lchtimes(name string, atime, mtime time.Time) Applier { return applyFn(func(root string) error { @@ -28,6 +36,13 @@ func Lchtimes(name string, atime, mtime time.Time) Applier { }) } +// CreateDeviceFile provides creates devices Applier. +func CreateDeviceFile(name string, mode os.FileMode, maj, min int) Applier { + return applyFn(func(root string) error { + return errors.New("Not implemented") + }) +} + // Base applies the files required to make a valid Windows container layer // that the filter will mount. It is used for testing the snapshotter func Base() Applier {