Fix flaky btrfs test

dmcgowan · dmcgowan · commit 772032598a42 · 2019-10-04T17:51:45.000-07:00
Add logging and move the creation of the snapshotter inside
the attempt loop to catch cases where the mountinfo may
not be updated yet. When all attempts are reached there
is no reason to create the snapshotter as the unmount has
already occurred.

Signed-off-by: Derek McGowan &lt;derek@mcgstyle.net&gt;
diff --git a/snapshots/btrfs/btrfs.go b/snapshots/btrfs/btrfs.go
@@ -77,7 +77,7 @@ func NewSnapshotter(root string) (snapshots.Snapshotter, error) {
 		return nil, err
 	}
 	if mnt.FSType != "btrfs" {
-		return nil, errors.Wrapf(plugin.ErrSkipPlugin, "path %s must be a btrfs filesystem to be used with the btrfs snapshotter", root)
+		return nil, errors.Wrapf(plugin.ErrSkipPlugin, "path %s (%s) must be a btrfs filesystem to be used with the btrfs snapshotter", root, mnt.FSType)
 	}
 	var (
 		active    = filepath.Join(root, "active")
diff --git a/snapshots/btrfs/btrfs_test.go b/snapshots/btrfs/btrfs_test.go
@@ -30,6 +30,7 @@ import (
 
 	"github.com/containerd/containerd/mount"
 	"github.com/containerd/containerd/pkg/testutil"
+	"github.com/containerd/containerd/plugin"
 	"github.com/containerd/containerd/snapshots"
 	"github.com/containerd/containerd/snapshots/testsuite"
 	"github.com/containerd/continuity/testutil/loopback"
@@ -66,27 +67,31 @@ func boltSnapshotter(t *testing.T) func(context.Context, string) (snapshots.Snap
 		// sync after a mkfs on the loopback before trying to mount the device
 		unix.Sync()
 
+		var snapshotter snapshots.Snapshotter
 		for i := 0; i < 5; i++ {
 			if out, err := exec.Command("mount", loop.Device, root).CombinedOutput(); err != nil {
 				loop.Close()
 				return nil, nil, errors.Wrapf(err, "failed to mount device %s (out: %q)", loop.Device, out)
 			}
-			var stat unix.Statfs_t
-			if err := unix.Statfs(root, &stat); err != nil {
-				unix.Unmount(root, 0)
-				return nil, nil, errors.Wrapf(err, "unable to statfs btrfs mount %s", root)
+
+			if i > 0 {
+				time.Sleep(10 * time.Duration(i) * time.Millisecond)
 			}
-			if stat.Type == unix.BTRFS_SUPER_MAGIC {
+
+			snapshotter, err = NewSnapshotter(root)
+			if err == nil {
 				break
+			} else if errors.Cause(err) != plugin.ErrSkipPlugin {
+				return nil, nil, err
 			}
+
+			t.Logf("Attempt %d to create btrfs snapshotter failed: %#v", i+1, err)
+
 			// unmount and try again
 			unix.Unmount(root, 0)
-			time.Sleep(100 * time.Millisecond)
 		}
-		snapshotter, err := NewSnapshotter(root)
-		if err != nil {
-			loop.Close()
-			return nil, nil, errors.Wrap(err, "failed to create new snapshotter")
+		if snapshotter == nil {
+			return nil, nil, errors.Wrap(err, "failed to successfully create snapshotter after 5 attempts")
 		}
 
 		return snapshotter, func() error {

Original file line number	Diff line number	Diff line change
`@@ -77,7 +77,7 @@ func NewSnapshotter(root string) (snapshots.Snapshotter, error) {`
`77`	`77`	`return nil, err`
`78`	`78`	`}`
`79`	`79`	`if mnt.FSType != "btrfs" {`
`80`		`- return nil, errors.Wrapf(plugin.ErrSkipPlugin, "path %s must be a btrfs filesystem to be used with the btrfs snapshotter", root)`
	`80`	`+ return nil, errors.Wrapf(plugin.ErrSkipPlugin, "path %s (%s) must be a btrfs filesystem to be used with the btrfs snapshotter", root, mnt.FSType)`
`81`	`81`	`}`
`82`	`82`	`var (`
`83`	`83`	`active = filepath.Join(root, "active")`