From 502734116d31885b7bc07d6af4f1cddadf11a370 Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Mon, 14 Aug 2017 20:52:01 -0700 Subject: [PATCH 1/4] Update loopback to return error Avoid calling testing function in creation closure since the context may no longer be valid. Signed-off-by: Derek McGowan --- fs/dtype_linux_test.go | 5 ++++- mount/lookup_test/lookup_linux_test.go | 5 ++++- snapshot/btrfs/btrfs_test.go | 27 ++++++++++++++++++-------- testutil/loopback_linux.go | 27 ++++++++++++++------------ 4 files changed, 42 insertions(+), 22 deletions(-) diff --git a/fs/dtype_linux_test.go b/fs/dtype_linux_test.go index 71801475b..23e796f0a 100644 --- a/fs/dtype_linux_test.go +++ b/fs/dtype_linux_test.go @@ -20,7 +20,10 @@ func testSupportsDType(t *testing.T, expected bool, mkfs ...string) { } defer os.RemoveAll(mnt) - deviceName, cleanupDevice := testutil.NewLoopback(t, 100<<20) // 100 MB + deviceName, cleanupDevice, err := testutil.NewLoopback(100 << 20) // 100 MB + if err != nil { + t.Fatal(err) + } if out, err := exec.Command(mkfs[0], append(mkfs[1:], deviceName)...).CombinedOutput(); err != nil { // not fatal t.Skipf("could not mkfs (%v) %s: %v (out: %q)", mkfs, deviceName, err, string(out)) diff --git a/mount/lookup_test/lookup_linux_test.go b/mount/lookup_test/lookup_linux_test.go index 389c33983..19eef4ff8 100644 --- a/mount/lookup_test/lookup_linux_test.go +++ b/mount/lookup_test/lookup_linux_test.go @@ -29,7 +29,10 @@ func testLookup(t *testing.T, fsType string) { } defer os.RemoveAll(mnt) - deviceName, cleanupDevice := testutil.NewLoopback(t, 100<<20) // 100 MB + deviceName, cleanupDevice, err := testutil.NewLoopback(100 << 20) // 100 MB + if err != nil { + t.Fatal(err) + } if out, err := exec.Command("mkfs", "-t", fsType, deviceName).CombinedOutput(); err != nil { // not fatal t.Skipf("could not mkfs (%s) %s: %v (out: %q)", fsType, deviceName, err, string(out)) diff --git a/snapshot/btrfs/btrfs_test.go b/snapshot/btrfs/btrfs_test.go index 4175f9662..738da0048 100644 --- a/snapshot/btrfs/btrfs_test.go +++ b/snapshot/btrfs/btrfs_test.go @@ -15,30 +15,41 @@ import ( "github.com/containerd/containerd/snapshot" "github.com/containerd/containerd/snapshot/testsuite" "github.com/containerd/containerd/testutil" + "github.com/pkg/errors" ) func boltSnapshotter(t *testing.T) func(context.Context, string) (snapshot.Snapshotter, func(), error) { + mkbtrfs, err := exec.LookPath("mkfs.btrfs") + if err != nil { + t.Skipf("could not find mkfs.btrfs: %v", err) + } + + // TODO: Check for btrfs in /proc/module and skip if not loaded + return func(ctx context.Context, root string) (snapshot.Snapshotter, func(), error) { - deviceName, cleanupDevice := testutil.NewLoopback(t, 100<<20) // 100 MB + deviceName, cleanupDevice, err := testutil.NewLoopback(100 << 20) // 100 MB + if err != nil { + return nil, nil, err + } - if out, err := exec.Command("mkfs.btrfs", deviceName).CombinedOutput(); err != nil { - // not fatal - t.Skipf("could not mkfs.btrfs %s: %v (out: %q)", deviceName, err, string(out)) + if out, err := exec.Command(mkbtrfs, deviceName).CombinedOutput(); err != nil { + return nil, nil, errors.Wrapf(err, "failed to make btrfs filesystem (out: %q)", out) } if out, err := exec.Command("mount", deviceName, root).CombinedOutput(); err != nil { - // not fatal - t.Skipf("could not mount %s: %v (out: %q)", deviceName, err, string(out)) + return nil, nil, errors.Wrapf(err, "failed to mount device %s (out: %q)", deviceName, out) } snapshotter, err := NewSnapshotter(root) if err != nil { - t.Fatal(err) + return nil, nil, errors.Wrap(err, "failed to create new snapshotter") } return snapshotter, func() { testutil.Unmount(t, root) - cleanupDevice() + if err := cleanupDevice(); err != nil { + t.Errorf("Device cleanup failed: %v", err) + } }, nil } } diff --git a/testutil/loopback_linux.go b/testutil/loopback_linux.go index da63aa27a..1639c20de 100644 --- a/testutil/loopback_linux.go +++ b/testutil/loopback_linux.go @@ -7,19 +7,21 @@ import ( "os" "os/exec" "strings" - "testing" + + "github.com/pkg/errors" + "github.com/sirupsen/logrus" ) // NewLoopback creates a loopback device, and returns its device name (/dev/loopX), and its clean-up function. -func NewLoopback(t *testing.T, size int64) (string, func()) { +func NewLoopback(size int64) (string, func() error, error) { // create temporary file for the disk image file, err := ioutil.TempFile("", "containerd-test-loopback") if err != nil { - t.Fatalf("could not create temporary file for loopback: %v", err) + return "", nil, errors.Wrap(err, "could not create temporary file for loopback") } if err := file.Truncate(size); err != nil { - t.Fatal(err) + return "", nil, errors.Wrap(err, "failed to resize temp file") } file.Close() @@ -27,27 +29,28 @@ func NewLoopback(t *testing.T, size int64) (string, func()) { losetup := exec.Command("losetup", "--find", "--show", file.Name()) p, err := losetup.Output() if err != nil { - t.Fatal(err) + return "", nil, errors.Wrap(err, "loopback setup failed") } deviceName := strings.TrimSpace(string(p)) - t.Logf("Created loop device %s (using %s)", deviceName, file.Name()) + logrus.Debugf("Created loop device %s (using %s)", deviceName, file.Name()) - cleanup := func() { + cleanup := func() error { // detach device - t.Logf("Removing loop device %s", deviceName) + logrus.Debugf("Removing loop device %s", deviceName) losetup := exec.Command("losetup", "--detach", deviceName) err := losetup.Run() if err != nil { - t.Error("Could not remove loop device", deviceName, err) + return errors.Wrapf(err, "Could not remove loop device %s", deviceName) } // remove file - t.Logf("Removing temporary file %s", file.Name()) + logrus.Debugf("Removing temporary file %s", file.Name()) if err = os.Remove(file.Name()); err != nil { - t.Error(err) + return err } + return nil } - return deviceName, cleanup + return deviceName, cleanup, nil } From 750771f6d0f6e067b8c6b22e3a47a38944c73613 Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Tue, 22 Aug 2017 10:35:20 -0700 Subject: [PATCH 2/4] Remove snapshot test suite as a parallel test runner The testsuite alters global state by setting the umask, avoid running the testsuite in parallel and move umask manipulation to the test suite level to individual tests may run in parallel. Added better error messaging and handling. Removed reliance on testing object for handling cleanup failure. When a cleanup error occurred, it would fail the test but the log would get skipped. With this change the failure will show up for the running test. Update test unmounting to set the detach flag, avoiding races with btrfs which may see the device as busy when attempting to unmount. Signed-off-by: Derek McGowan --- container_test.go | 1 - metadata/snapshot_test.go | 6 ++--- snapshot/btrfs/btrfs_test.go | 16 ++++++----- snapshot/naive/naive_test.go | 4 +-- snapshot/overlay/overlay_test.go | 4 +-- snapshot/testsuite/helpers.go | 24 ++++++++++------- snapshot/testsuite/issues.go | 8 ------ snapshot/testsuite/testsuite.go | 36 ++++++++++--------------- snapshot/testsuite/testsuite_unix.go | 8 +++++- snapshot/testsuite/testsuite_windows.go | 2 ++ snapshot_test.go | 6 ++--- testutil/helpers_unix.go | 3 ++- 12 files changed, 59 insertions(+), 59 deletions(-) diff --git a/container_test.go b/container_test.go index a2627114c..0c0d8416c 100644 --- a/container_test.go +++ b/container_test.go @@ -2,7 +2,6 @@ package containerd import ( "bytes" - "fmt" "io/ioutil" "os" "runtime" diff --git a/metadata/snapshot_test.go b/metadata/snapshot_test.go index 573d3b83d..37800dc40 100644 --- a/metadata/snapshot_test.go +++ b/metadata/snapshot_test.go @@ -13,7 +13,7 @@ import ( "github.com/containerd/containerd/testutil" ) -func newSnapshotter(ctx context.Context, root string) (snapshot.Snapshotter, func(), error) { +func newSnapshotter(ctx context.Context, root string) (snapshot.Snapshotter, func() error, error) { naiveRoot := filepath.Join(root, "naive") if err := os.Mkdir(naiveRoot, 0770); err != nil { return nil, nil, err @@ -30,8 +30,8 @@ func newSnapshotter(ctx context.Context, root string) (snapshot.Snapshotter, fun sn := NewSnapshotter(db, "naive", snapshotter) - return sn, func() { - db.Close() + return sn, func() error { + return db.Close() }, nil } diff --git a/snapshot/btrfs/btrfs_test.go b/snapshot/btrfs/btrfs_test.go index 738da0048..83fd7d54d 100644 --- a/snapshot/btrfs/btrfs_test.go +++ b/snapshot/btrfs/btrfs_test.go @@ -16,9 +16,10 @@ import ( "github.com/containerd/containerd/snapshot/testsuite" "github.com/containerd/containerd/testutil" "github.com/pkg/errors" + "golang.org/x/sys/unix" ) -func boltSnapshotter(t *testing.T) func(context.Context, string) (snapshot.Snapshotter, func(), error) { +func boltSnapshotter(t *testing.T) func(context.Context, string) (snapshot.Snapshotter, func() error, error) { mkbtrfs, err := exec.LookPath("mkfs.btrfs") if err != nil { t.Skipf("could not find mkfs.btrfs: %v", err) @@ -26,7 +27,7 @@ func boltSnapshotter(t *testing.T) func(context.Context, string) (snapshot.Snaps // TODO: Check for btrfs in /proc/module and skip if not loaded - return func(ctx context.Context, root string) (snapshot.Snapshotter, func(), error) { + return func(ctx context.Context, root string) (snapshot.Snapshotter, func() error, error) { deviceName, cleanupDevice, err := testutil.NewLoopback(100 << 20) // 100 MB if err != nil { @@ -45,11 +46,14 @@ func boltSnapshotter(t *testing.T) func(context.Context, string) (snapshot.Snaps return nil, nil, errors.Wrap(err, "failed to create new snapshotter") } - return snapshotter, func() { - testutil.Unmount(t, root) - if err := cleanupDevice(); err != nil { - t.Errorf("Device cleanup failed: %v", err) + return snapshotter, func() (err error) { + merr := mount.UnmountAll(root, unix.MNT_DETACH) + if err = cleanupDevice(); err != nil { + return errors.Wrap(err, "device cleanup failed") + } else { + err = merr } + return err }, nil } } diff --git a/snapshot/naive/naive_test.go b/snapshot/naive/naive_test.go index 67ac2cc4f..279e95505 100644 --- a/snapshot/naive/naive_test.go +++ b/snapshot/naive/naive_test.go @@ -9,13 +9,13 @@ import ( "github.com/containerd/containerd/testutil" ) -func newSnapshotter(ctx context.Context, root string) (snapshot.Snapshotter, func(), error) { +func newSnapshotter(ctx context.Context, root string) (snapshot.Snapshotter, func() error, error) { snapshotter, err := NewSnapshotter(root) if err != nil { return nil, nil, err } - return snapshotter, func() {}, nil + return snapshotter, nil, nil } func TestNaive(t *testing.T) { diff --git a/snapshot/overlay/overlay_test.go b/snapshot/overlay/overlay_test.go index 5b47f07ae..870b31a3f 100644 --- a/snapshot/overlay/overlay_test.go +++ b/snapshot/overlay/overlay_test.go @@ -18,13 +18,13 @@ import ( "github.com/containerd/containerd/testutil" ) -func newSnapshotter(ctx context.Context, root string) (snapshot.Snapshotter, func(), error) { +func newSnapshotter(ctx context.Context, root string) (snapshot.Snapshotter, func() error, error) { snapshotter, err := NewSnapshotter(root) if err != nil { return nil, nil, err } - return snapshotter, func() {}, nil + return snapshotter, nil, nil } func TestOverlay(t *testing.T) { diff --git a/snapshot/testsuite/helpers.go b/snapshot/testsuite/helpers.go index a0be722d1..8d4acb904 100644 --- a/snapshot/testsuite/helpers.go +++ b/snapshot/testsuite/helpers.go @@ -21,11 +21,11 @@ func applyToMounts(m []mount.Mount, work string, a fstest.Applier) (err error) { defer os.RemoveAll(td) if err := mount.MountAll(m, td); err != nil { - return err + return errors.Wrap(err, "failed to mount") } defer func() { - if err1 := mount.UnmountAll(td, 0); err == nil { - err = err1 + if err1 := mount.UnmountAll(td, umountflags); err == nil { + err = errors.Wrap(err1, "failed to unmount") } }() @@ -40,26 +40,30 @@ func createSnapshot(ctx context.Context, sn snapshot.Snapshotter, parent, work s m, err := sn.Prepare(ctx, prepare, parent) if err != nil { - return "", err + return "", errors.Wrap(err, "failed to prepare snapshot") } if err := applyToMounts(m, work, a); err != nil { - return "", err + return "", errors.Wrap(err, "failed to apply") } if err := sn.Commit(ctx, n, prepare); err != nil { - return "", err + return "", errors.Wrap(err, "failed to commit") } return n, nil } -func checkSnapshot(ctx context.Context, sn snapshot.Snapshotter, work, name, check string) error { +func checkSnapshot(ctx context.Context, sn snapshot.Snapshotter, work, name, check string) (err error) { td, err := ioutil.TempDir(work, "check") if err != nil { return errors.Wrap(err, "failed to create temp dir") } - defer os.RemoveAll(td) + defer func() { + if err1 := os.RemoveAll(td); err == nil { + err = errors.Wrapf(err1, "failed to remove temporary directory %s", td) + } + }() view := fmt.Sprintf("%s-view", name) m, err := sn.View(ctx, view, name) @@ -73,10 +77,10 @@ func checkSnapshot(ctx context.Context, sn snapshot.Snapshotter, work, name, che }() if err := mount.MountAll(m, td); err != nil { - return errors.Wrap(err, "failed to unmount") + return errors.Wrap(err, "failed to mount") } defer func() { - if err1 := mount.UnmountAll(td, 0); err == nil { + if err1 := mount.UnmountAll(td, umountflags); err == nil { err = errors.Wrap(err1, "failed to unmount view") } }() diff --git a/snapshot/testsuite/issues.go b/snapshot/testsuite/issues.go index 84b7a5e01..a41c401f0 100644 --- a/snapshot/testsuite/issues.go +++ b/snapshot/testsuite/issues.go @@ -22,8 +22,6 @@ import ( // avoid such issues by not relying on tar to create layers. // See https://github.com/docker/docker/issues/21555 func checkLayerFileUpdate(ctx context.Context, t *testing.T, sn snapshot.Snapshotter, work string) { - t.Parallel() - l1Init := fstest.Apply( fstest.CreateDir("/etc", 0700), fstest.CreateFile("/etc/hosts", []byte("mydomain 10.0.0.1"), 0644), @@ -55,8 +53,6 @@ func checkLayerFileUpdate(ctx context.Context, t *testing.T, sn snapshot.Snapsho // checkRemoveDirectoryInLowerLayer // See https://github.com/docker/docker/issues/25244 func checkRemoveDirectoryInLowerLayer(ctx context.Context, t *testing.T, sn snapshot.Snapshotter, work string) { - t.Parallel() - l1Init := fstest.Apply( fstest.CreateDir("/lib", 0700), fstest.CreateFile("/lib/hidden", []byte{}, 0644), @@ -80,8 +76,6 @@ func checkRemoveDirectoryInLowerLayer(ctx context.Context, t *testing.T, sn snap // See https://github.com/docker/docker/issues/24913 overlay // see https://github.com/docker/docker/issues/28391 overlay2 func checkChown(ctx context.Context, t *testing.T, sn snapshot.Snapshotter, work string) { - t.Parallel() - l1Init := fstest.Apply( fstest.CreateDir("/opt", 0700), fstest.CreateDir("/opt/a", 0700), @@ -124,8 +118,6 @@ func checkRename(ctx context.Context, t *testing.T, sn snapshot.Snapshotter, wor // checkDirectoryPermissionOnCommit // https://github.com/docker/docker/issues/27298 func checkDirectoryPermissionOnCommit(ctx context.Context, t *testing.T, sn snapshot.Snapshotter, work string) { - t.Parallel() - l1Init := fstest.Apply( fstest.CreateDir("/dir1", 0700), fstest.CreateDir("/dir2", 0700), diff --git a/snapshot/testsuite/testsuite.go b/snapshot/testsuite/testsuite.go index 467b30104..df934c7f8 100644 --- a/snapshot/testsuite/testsuite.go +++ b/snapshot/testsuite/testsuite.go @@ -19,8 +19,9 @@ import ( ) // SnapshotterSuite runs a test suite on the snapshotter given a factory function. -func SnapshotterSuite(t *testing.T, name string, snapshotterFn func(ctx context.Context, root string) (snapshot.Snapshotter, func(), error)) { - t.Parallel() +func SnapshotterSuite(t *testing.T, name string, snapshotterFn func(ctx context.Context, root string) (snapshot.Snapshotter, func() error, error)) { + restoreMask := clearMask() + defer restoreMask() t.Run("Basic", makeTest(name, snapshotterFn, checkSnapshotterBasic)) t.Run("StatActive", makeTest(name, snapshotterFn, checkSnapshotterStatActive)) @@ -39,12 +40,12 @@ func SnapshotterSuite(t *testing.T, name string, snapshotterFn func(ctx context. //t.Run("Rename", makeTest(name, snapshotterFn, checkRename)) } -func makeTest(name string, snapshotterFn func(ctx context.Context, root string) (snapshot.Snapshotter, func(), error), fn func(ctx context.Context, t *testing.T, snapshotter snapshot.Snapshotter, work string)) func(t *testing.T) { +func makeTest(name string, snapshotterFn func(ctx context.Context, root string) (snapshot.Snapshotter, func() error, error), fn func(ctx context.Context, t *testing.T, snapshotter snapshot.Snapshotter, work string)) func(t *testing.T) { return func(t *testing.T) { + t.Parallel() + ctx := context.Background() ctx = namespaces.WithNamespace(ctx, "testsuite") - restoreMask := clearMask() - defer restoreMask() // Make two directories: a snapshotter root and a play area for the tests: // // /tmp @@ -64,9 +65,15 @@ func makeTest(name string, snapshotterFn func(ctx context.Context, root string) snapshotter, cleanup, err := snapshotterFn(ctx, root) if err != nil { - t.Fatal(err) + t.Fatalf("Failed to initialize snapshotter: %+v", err) } - defer cleanup() + defer func() { + if cleanup != nil { + if err := cleanup(); err != nil { + t.Errorf("Cleanup failed: %v", err) + } + } + }() work := filepath.Join(tmpDir, "work") if err := os.MkdirAll(work, 0777); err != nil { @@ -80,9 +87,6 @@ func makeTest(name string, snapshotterFn func(ctx context.Context, root string) // checkSnapshotterBasic tests the basic workflow of a snapshot snapshotter. func checkSnapshotterBasic(ctx context.Context, t *testing.T, snapshotter snapshot.Snapshotter, work string) { - // TODO: this always fails when run in parallel, why? - // t.Parallel() - initialApplier := fstest.Apply( fstest.CreateFile("/foo", []byte("foo\n"), 0777), fstest.CreateDir("/a", 0755), @@ -216,8 +220,6 @@ func checkSnapshotterBasic(ctx context.Context, t *testing.T, snapshotter snapsh // Create a New Layer on top of base layer with Prepare, Stat on new layer, should return Active layer. func checkSnapshotterStatActive(ctx context.Context, t *testing.T, snapshotter snapshot.Snapshotter, work string) { - t.Parallel() - preparing := filepath.Join(work, "preparing") if err := os.MkdirAll(preparing, 0777); err != nil { t.Fatal(err) @@ -252,8 +254,6 @@ func checkSnapshotterStatActive(ctx context.Context, t *testing.T, snapshotter s // Commit a New Layer on top of base layer with Prepare & Commit , Stat on new layer, should return Committed layer. func checkSnapshotterStatCommitted(ctx context.Context, t *testing.T, snapshotter snapshot.Snapshotter, work string) { - t.Parallel() - preparing := filepath.Join(work, "preparing") if err := os.MkdirAll(preparing, 0777); err != nil { t.Fatal(err) @@ -315,8 +315,6 @@ func snapshotterPrepareMount(ctx context.Context, snapshotter snapshot.Snapshott // Given A <- B <- C, B is the parent of C and A is a transitive parent of C (in this case, a "grandparent") func checkSnapshotterTransitivity(ctx context.Context, t *testing.T, snapshotter snapshot.Snapshotter, work string) { - t.Parallel() - preparing, err := snapshotterPrepareMount(ctx, snapshotter, "preparing", "", work) if err != nil { t.Fatal(err) @@ -371,8 +369,6 @@ func checkSnapshotterTransitivity(ctx context.Context, t *testing.T, snapshotter // Creating two layers with Prepare or View with same key must fail. func checkSnapshotterPrepareView(ctx context.Context, t *testing.T, snapshotter snapshot.Snapshotter, work string) { - t.Parallel() - preparing, err := snapshotterPrepareMount(ctx, snapshotter, "preparing", "", work) if err != nil { t.Fatal(err) @@ -469,8 +465,6 @@ func baseTestSnapshots(ctx context.Context, snapshotter snapshot.Snapshotter) er } func checkUpdate(ctx context.Context, t *testing.T, snapshotter snapshot.Snapshotter, work string) { - t.Parallel() - t1 := time.Now().UTC() if err := baseTestSnapshots(ctx, snapshotter); err != nil { t.Fatalf("Failed to create base snapshots: %v", err) @@ -622,8 +616,6 @@ func assertLabels(t *testing.T, actual, expected map[string]string) { } func checkRemove(ctx context.Context, t *testing.T, snapshotter snapshot.Snapshotter, work string) { - t.Parallel() - if _, err := snapshotter.Prepare(ctx, "committed-a", ""); err != nil { t.Fatal(err) } diff --git a/snapshot/testsuite/testsuite_unix.go b/snapshot/testsuite/testsuite_unix.go index 6a02eda9d..14dc85864 100644 --- a/snapshot/testsuite/testsuite_unix.go +++ b/snapshot/testsuite/testsuite_unix.go @@ -2,7 +2,13 @@ package testsuite -import "syscall" +import ( + "syscall" + + "golang.org/x/sys/unix" +) + +const umountflags int = unix.MNT_DETACH func clearMask() func() { oldumask := syscall.Umask(0) diff --git a/snapshot/testsuite/testsuite_windows.go b/snapshot/testsuite/testsuite_windows.go index c3cc8c22b..c48cca3e6 100644 --- a/snapshot/testsuite/testsuite_windows.go +++ b/snapshot/testsuite/testsuite_windows.go @@ -1,5 +1,7 @@ package testsuite +const umountflags int = 0 + func clearMask() func() { return func() {} } diff --git a/snapshot_test.go b/snapshot_test.go index ccf28c70c..0bcab7876 100644 --- a/snapshot_test.go +++ b/snapshot_test.go @@ -9,7 +9,7 @@ import ( "github.com/containerd/containerd/snapshot/testsuite" ) -func newSnapshotter(ctx context.Context, root string) (snapshot.Snapshotter, func(), error) { +func newSnapshotter(ctx context.Context, root string) (snapshot.Snapshotter, func() error, error) { client, err := New(address) if err != nil { return nil, nil, err @@ -17,8 +17,8 @@ func newSnapshotter(ctx context.Context, root string) (snapshot.Snapshotter, fun sn := client.SnapshotService(DefaultSnapshotter) - return sn, func() { - client.Close() + return sn, func() error { + return client.Close() }, nil } diff --git a/testutil/helpers_unix.go b/testutil/helpers_unix.go index 398209df1..fd207449e 100644 --- a/testutil/helpers_unix.go +++ b/testutil/helpers_unix.go @@ -9,12 +9,13 @@ import ( "github.com/containerd/containerd/mount" "github.com/stretchr/testify/assert" + "golang.org/x/sys/unix" ) // Unmount unmounts a given mountPoint and sets t.Error if it fails func Unmount(t *testing.T, mountPoint string) { t.Log("unmount", mountPoint) - if err := mount.Unmount(mountPoint, 0); err != nil { + if err := mount.UnmountAll(mountPoint, unix.MNT_DETACH); err != nil { t.Error("Could not umount", mountPoint, err) } } From f74cea71dd24d2cf863420f3085651c0c6da7cea Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Tue, 22 Aug 2017 17:14:06 -0700 Subject: [PATCH 3/4] Update basic test to allow being run in parallel on client The basic test does an assert on the existing snapshots. Update the check just to assert the expected snapshots were found during the walk. Signed-off-by: Derek McGowan --- snapshot/testsuite/testsuite.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/snapshot/testsuite/testsuite.go b/snapshot/testsuite/testsuite.go index df934c7f8..e588ee374 100644 --- a/snapshot/testsuite/testsuite.go +++ b/snapshot/testsuite/testsuite.go @@ -190,7 +190,14 @@ func checkSnapshotterBasic(ctx context.Context, t *testing.T, snapshotter snapsh return nil })) - assert.Equal(t, expected, walked) + for ek, ev := range expected { + av, ok := walked[ek] + if !ok { + t.Errorf("Missing stat for %v", ek) + continue + } + assert.Equal(t, ev, av) + } nextnext := filepath.Join(work, "nextnextlayer") if err := os.MkdirAll(nextnext, 0777); err != nil { From 96a75ab1ab8e7524a7cfd1d67829988b07a51e54 Mon Sep 17 00:00:00 2001 From: Derek McGowan Date: Tue, 22 Aug 2017 17:20:25 -0700 Subject: [PATCH 4/4] Move detach flags to platform specific Signed-off-by: Derek McGowan --- snapshot/testsuite/helpers_linux.go | 5 +++++ snapshot/testsuite/helpers_other.go | 5 +++++ snapshot/testsuite/testsuite_unix.go | 8 +------- snapshot/testsuite/testsuite_windows.go | 2 -- testutil/helpers_unix.go | 3 +-- testutil/mount_linux.go | 5 +++++ testutil/mount_other.go | 5 +++++ 7 files changed, 22 insertions(+), 11 deletions(-) create mode 100644 snapshot/testsuite/helpers_linux.go create mode 100644 snapshot/testsuite/helpers_other.go create mode 100644 testutil/mount_linux.go create mode 100644 testutil/mount_other.go diff --git a/snapshot/testsuite/helpers_linux.go b/snapshot/testsuite/helpers_linux.go new file mode 100644 index 000000000..1f8fb2824 --- /dev/null +++ b/snapshot/testsuite/helpers_linux.go @@ -0,0 +1,5 @@ +package testsuite + +import "golang.org/x/sys/unix" + +const umountflags int = unix.MNT_DETACH diff --git a/snapshot/testsuite/helpers_other.go b/snapshot/testsuite/helpers_other.go new file mode 100644 index 000000000..1df729c52 --- /dev/null +++ b/snapshot/testsuite/helpers_other.go @@ -0,0 +1,5 @@ +// +build !linux + +package testsuite + +const umountflags int = 0 diff --git a/snapshot/testsuite/testsuite_unix.go b/snapshot/testsuite/testsuite_unix.go index 14dc85864..6a02eda9d 100644 --- a/snapshot/testsuite/testsuite_unix.go +++ b/snapshot/testsuite/testsuite_unix.go @@ -2,13 +2,7 @@ package testsuite -import ( - "syscall" - - "golang.org/x/sys/unix" -) - -const umountflags int = unix.MNT_DETACH +import "syscall" func clearMask() func() { oldumask := syscall.Umask(0) diff --git a/snapshot/testsuite/testsuite_windows.go b/snapshot/testsuite/testsuite_windows.go index c48cca3e6..c3cc8c22b 100644 --- a/snapshot/testsuite/testsuite_windows.go +++ b/snapshot/testsuite/testsuite_windows.go @@ -1,7 +1,5 @@ package testsuite -const umountflags int = 0 - func clearMask() func() { return func() {} } diff --git a/testutil/helpers_unix.go b/testutil/helpers_unix.go index fd207449e..e8f983404 100644 --- a/testutil/helpers_unix.go +++ b/testutil/helpers_unix.go @@ -9,13 +9,12 @@ import ( "github.com/containerd/containerd/mount" "github.com/stretchr/testify/assert" - "golang.org/x/sys/unix" ) // Unmount unmounts a given mountPoint and sets t.Error if it fails func Unmount(t *testing.T, mountPoint string) { t.Log("unmount", mountPoint) - if err := mount.UnmountAll(mountPoint, unix.MNT_DETACH); err != nil { + if err := mount.UnmountAll(mountPoint, umountflags); err != nil { t.Error("Could not umount", mountPoint, err) } } diff --git a/testutil/mount_linux.go b/testutil/mount_linux.go new file mode 100644 index 000000000..1c1164215 --- /dev/null +++ b/testutil/mount_linux.go @@ -0,0 +1,5 @@ +package testutil + +import "golang.org/x/sys/unix" + +const umountflags int = unix.MNT_DETACH diff --git a/testutil/mount_other.go b/testutil/mount_other.go new file mode 100644 index 000000000..da3cb9e79 --- /dev/null +++ b/testutil/mount_other.go @@ -0,0 +1,5 @@ +// +build !linux + +package testutil + +const umountflags int = 0