Merge pull request #854 from AkihiroSuda/lookup-mountinfo

mountinfo: add Lookup for ease of implementing snapshotters

Makefile

@@ -25,7 +25,7 @@ endif
 # Project packages.
 PACKAGES=$(shell go list ./... | grep -v /vendor/)
 INTEGRATION_PACKAGE=${PKG}/integration
-SNAPSHOT_PACKAGES=$(shell go list ./snapshot/...)
+TEST_REQUIRES_ROOT_PACKAGES=$(shell for f in $$(git grep -l testutil.RequiresRoot | grep -v Makefile);do echo "${PKG}/$$(dirname $$f)"; done)
 
 # Project binaries.
 COMMANDS=ctr containerd protoc-gen-gogoctrd dist ctrd-protobuild
@@ -121,7 +121,7 @@ test: ## run tests, except integration tests and tests that require root
 
 root-test: ## run tests, except integration tests
 	@echo "$(WHALE) $@"
-	@go test ${TESTFLAGS} ${SNAPSHOT_PACKAGES} -test.root
+	@go test ${TESTFLAGS} ${TEST_REQUIRES_ROOT_PACKAGES} -test.root
 
 integration: ## run integration tests
 	@echo "$(WHALE) $@"
@@ -173,7 +173,7 @@ coverage: ## generate coverprofiles from the unit tests, except tests that requi
 
 root-coverage: ## generae coverage profiles for the unit tests
 	@echo "$(WHALE) $@"
-	@( for pkg in ${SNAPSHOT_PACKAGES}; do \
+	@( for pkg in ${TEST_REQUIRES_ROOT_PACKAGES}; do \
 		go test -i ${TESTFLAGS} -test.short -coverprofile="../../../$$pkg/coverage.txt" -covermode=atomic $$pkg -test.root || exit; \
 		go test ${TESTFLAGS} -test.short -coverprofile="../../../$$pkg/coverage.txt" -covermode=atomic $$pkg -test.root || exit; \
 	done )

fs/dtype_linux_test.go

@@ -6,76 +6,40 @@ import (
 	"io/ioutil"
 	"os"
 	"os/exec"
-	"syscall"
 	"testing"
+
+	"github.com/containerd/containerd/testutil"
+	"github.com/stretchr/testify/assert"
 )
 
-func testSupportsDType(t *testing.T, expected bool, mkfsCommand string, mkfsArg ...string) {
-	// check whether mkfs is installed
-	if _, err := exec.LookPath(mkfsCommand); err != nil {
-		t.Skipf("%s not installed: %v", mkfsCommand, err)
-	}
-
-	// create a sparse image
-	imageSize := int64(32 * 1024 * 1024)
-	imageFile, err := ioutil.TempFile("", "fsutils-image")
+func testSupportsDType(t *testing.T, expected bool, mkfs ...string) {
+	testutil.RequiresRoot(t)
+	mnt, err := ioutil.TempDir("", "containerd-fs-test-supports-dtype")
 	if err != nil {
 		t.Fatal(err)
 	}
-	imageFileName := imageFile.Name()
-	defer os.Remove(imageFileName)
-	if _, err = imageFile.Seek(imageSize-1, 0); err != nil {
-		t.Fatal(err)
-	}
-	if _, err = imageFile.Write([]byte{0}); err != nil {
-		t.Fatal(err)
-	}
-	if err = imageFile.Close(); err != nil {
-		t.Fatal(err)
-	}
+	defer os.RemoveAll(mnt)
 
-	// create a mountpoint
-	mountpoint, err := ioutil.TempDir("", "fsutils-mountpoint")
-	if err != nil {
-		t.Fatal(err)
+	deviceName, cleanupDevice := testutil.NewLoopback(t, 100<<20) // 100 MB
+	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))
 	}
-	defer os.RemoveAll(mountpoint)
-
-	// format the image
-	args := append(mkfsArg, imageFileName)
-	t.Logf("Executing `%s %v`", mkfsCommand, args)
-	out, err := exec.Command(mkfsCommand, args...).CombinedOutput()
-	if len(out) > 0 {
-		t.Log(string(out))
-	}
-	if err != nil {
-		t.Skip("skipping the test because %s failed. This is probably your %s is an unsupported version.", mkfsCommand, mkfsCommand)
-	}
-
-	// loopback-mount the image.
-	// for ease of setting up loopback device, we use os/exec rather than syscall.Mount
-	out, err = exec.Command("mount", "-o", "loop", imageFileName, mountpoint).CombinedOutput()
-	if len(out) > 0 {
-		t.Log(string(out))
-	}
-	if err != nil {
-		t.Skip("skipping the test because mount failed")
+	if out, err := exec.Command("mount", deviceName, mnt).CombinedOutput(); err != nil {
+		// not fatal
+		t.Skipf("could not mount %s: %v (out: %q)", deviceName, err, string(out))
 	}
 	defer func() {
-		if err := syscall.Unmount(mountpoint, 0); err != nil {
-			t.Fatal(err)
-		}
+		testutil.Unmount(t, mnt)
+		cleanupDevice()
 	}()
-
 	// check whether it supports d_type
-	result, err := SupportsDType(mountpoint)
+	result, err := SupportsDType(mnt)
 	if err != nil {
 		t.Fatal(err)
 	}
 	t.Logf("Supports d_type: %v", result)
-	if result != expected {
-		t.Fatalf("expected %v, got %v", expected, result)
-	}
+	assert.Equal(t, expected, result)
 }
 
 func TestSupportsDTypeWithFType0XFS(t *testing.T) {

mount/lookup_test/dummy.go

@@ -0,0 +1,3 @@
+package lookuptest
+
+// FIXME: without this dummy file, `make build` fails with "no buildable Go source files" error

mount/lookup_test/lookup_linux_test.go

@@ -0,0 +1,59 @@
+// +build linux
+
+// FIXME: we can't put this test to the mount package:
+// import cycle not allowed in test
+// package github.com/containerd/containerd/mount (test)
+//         imports github.com/containerd/containerd/testutil
+//         imports github.com/containerd/containerd/mount
+//
+// NOTE: we can't have this as lookup_test (compilation fails)
+package lookuptest
+
+import (
+	"io/ioutil"
+	"os"
+	"os/exec"
+	"strings"
+	"testing"
+
+	"github.com/containerd/containerd/mount"
+	"github.com/containerd/containerd/testutil"
+	"github.com/stretchr/testify/assert"
+)
+
+func testLookup(t *testing.T, fsType string) {
+	testutil.RequiresRoot(t)
+	mnt, err := ioutil.TempDir("", "containerd-mountinfo-test-lookup")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(mnt)
+
+	deviceName, cleanupDevice := testutil.NewLoopback(t, 100<<20) // 100 MB
+	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))
+	}
+	if out, err := exec.Command("mount", deviceName, mnt).CombinedOutput(); err != nil {
+		// not fatal
+		t.Skipf("could not mount %s: %v (out: %q)", deviceName, err, string(out))
+	}
+	defer func() {
+		testutil.Unmount(t, mnt)
+		cleanupDevice()
+	}()
+	assert.True(t, strings.HasPrefix(deviceName, "/dev/loop"))
+	info, err := mount.Lookup(mnt)
+	if err != nil {
+		t.Fatal(err)
+	}
+	assert.Equal(t, fsType, info.FSType)
+}
+
+func TestLookupWithExt4(t *testing.T) {
+	testLookup(t, "ext4")
+}
+
+func TestLookupWithXFS(t *testing.T) {
+	testLookup(t, "xfs")
+}

mount/lookup_unix.go

@@ -0,0 +1,37 @@
+// +build !windows
+
+package mount
+
+import (
+	"fmt"
+	"syscall"
+
+	"github.com/pkg/errors"
+)
+
+// Lookup returns the mount info corresponds to the path.
+func Lookup(dir string) (Info, error) {
+	var dirStat syscall.Stat_t
+	if err := syscall.Stat(dir, &dirStat); err != nil {
+		return Info{}, errors.Wrapf(err, "failed to access %q", dir)
+	}
+
+	mounts, err := Self()
+	if err != nil {
+		return Info{}, err
+	}
+	for _, m := range mounts {
+		// Note that m.{Major, Minor} are generally unreliable for our purpose here
+		// https://www.spinics.net/lists/linux-btrfs/msg58908.html
+		var st syscall.Stat_t
+		if err := syscall.Stat(m.Mountpoint, &st); err != nil {
+			// may fail; ignore err
+			continue
+		}
+		if st.Dev == dirStat.Dev {
+			return m, nil
+		}
+	}
+
+	return Info{}, fmt.Errorf("failed to find the mount info for %q", dir)
+}

mount/lookup_unsupported.go

@@ -0,0 +1,13 @@
+// +build windows
+
+package mount
+
+import (
+	"fmt"
+	"runtime"
+)
+
+// Lookup returns the mount info corresponds to the path.
+func Lookup(dir string) (Info, error) {
+	return Info{}, fmt.Errorf("mount.Lookup is not implemented on %s/%s", runtime.GOOS, runtime.GOARCH)
+}

snapshot/btrfs/btrfs.go

@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"strings"
 
 	"github.com/Sirupsen/logrus"
 	"github.com/containerd/btrfs"
@@ -35,37 +34,15 @@ type snapshotter struct {
 	ms     *storage.MetaStore
 }
 
-func getBtrfsDevice(root string, mounts []mount.Info) (string, error) {
-	device := ""
-	deviceMountpoint := ""
-	for _, info := range mounts {
-		if (info.Root == "/" || info.Root == "") && strings.HasPrefix(root, info.Mountpoint) {
-			if info.FSType == "btrfs" && len(info.Mountpoint) > len(deviceMountpoint) {
-				device = info.Source
-				deviceMountpoint = info.Mountpoint
-			}
-			if root == info.Mountpoint && info.FSType != "btrfs" {
-				return "", fmt.Errorf("%s needs to be btrfs, but seems %s", root, info.FSType)
-			}
-		}
-	}
-	if device == "" {
-		// TODO: automatically mount loopback device here?
-		return "", fmt.Errorf("%s is not mounted as btrfs", root)
-	}
-	return device, nil
-}
-
 // NewSnapshotter returns a Snapshotter using btrfs. Uses the provided
 // root directory for snapshots and stores the metadata in
 // a file in the provided root.
 // root needs to be a mount point of btrfs.
 func NewSnapshotter(root string) (snapshot.Snapshotter, error) {
-	mounts, err := mount.Self()
-	if err != nil {
-		return nil, err
+	mnt, err := mount.Lookup(root)
+	if mnt.FSType != "btrfs" {
+		return nil, fmt.Errorf("expected btrfs, got %s", mnt.FSType)
 	}
-	device, err := getBtrfsDevice(root, mounts)
 	if err != nil {
 		return nil, err
 	}
@@ -88,7 +65,7 @@ func NewSnapshotter(root string) (snapshot.Snapshotter, error) {
 	}
 
 	return &snapshotter{
-		device: device,
+		device: mnt.Source,
 		root:   root,
 		ms:     ms,
 	}, nil

snapshot/btrfs/btrfs_test.go

@@ -18,20 +18,28 @@ import (
 	"github.com/containerd/containerd/testutil"
 )
 
-const (
-	mib = 1024 * 1024
-)
-
 func boltSnapshotter(t *testing.T) func(context.Context, string) (snapshot.Snapshotter, func(), error) {
 	return func(ctx context.Context, root string) (snapshot.Snapshotter, func(), error) {
-		device := setupBtrfsLoopbackDevice(t, root)
+
+		deviceName, cleanupDevice := testutil.NewLoopback(t, 100<<20) // 100 MB
+
+		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("mount", deviceName, root).CombinedOutput(); err != nil {
+			// not fatal
+			t.Skipf("could not mount %s: %v (out: %q)", deviceName, err, string(out))
+		}
+
 		snapshotter, err := NewSnapshotter(root)
 		if err != nil {
 			t.Fatal(err)
 		}
 
 		return snapshotter, func() {
-			device.remove(t)
+			testutil.Unmount(t, root)
+			cleanupDevice()
 		}, nil
 	}
 }
@@ -130,150 +138,3 @@ func TestBtrfsMounts(t *testing.T) {
 		t.Fatal(err)
 	}
 }
-
-type testDevice struct {
-	mountPoint string
-	fileName   string
-	deviceName string
-}
-
-// setupBtrfsLoopbackDevice creates a file, mounts it as a loopback device, and
-// formats it as btrfs.  The device should be cleaned up by calling
-// removeBtrfsLoopbackDevice.
-func setupBtrfsLoopbackDevice(t *testing.T, mountPoint string) *testDevice {
-
-	// create temporary file for the disk image
-	file, err := ioutil.TempFile("", "containerd-btrfs-test")
-	if err != nil {
-		t.Fatal("Could not create temporary file for btrfs test", err)
-	}
-	t.Log("Temporary file created", file.Name())
-
-	// initialize file with 100 MiB
-	if err := file.Truncate(100 << 20); err != nil {
-		t.Fatal(err)
-	}
-	file.Close()
-
-	// create device
-	losetup := exec.Command("losetup", "--find", "--show", file.Name())
-	p, err := losetup.Output()
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	deviceName := strings.TrimSpace(string(p))
-	t.Log("Created loop device", deviceName)
-
-	// format
-	t.Log("Creating btrfs filesystem")
-	mkfs := exec.Command("mkfs.btrfs", deviceName)
-	err = mkfs.Run()
-	if err != nil {
-		t.Fatal("Could not run mkfs.btrfs", err)
-	}
-
-	// mount
-	t.Logf("Mounting %s at %s", deviceName, mountPoint)
-	mount := exec.Command("mount", deviceName, mountPoint)
-	err = mount.Run()
-	if err != nil {
-		t.Fatal("Could not mount", err)
-	}
-
-	return &testDevice{
-		mountPoint: mountPoint,
-		fileName:   file.Name(),
-		deviceName: deviceName,
-	}
-}
-
-// remove cleans up the test device, unmounting the loopback and disk image
-// file.
-func (device *testDevice) remove(t *testing.T) {
-	// unmount
-	testutil.Unmount(t, device.mountPoint)
-
-	// detach device
-	t.Log("Removing loop device")
-	losetup := exec.Command("losetup", "--detach", device.deviceName)
-	err := losetup.Run()
-	if err != nil {
-		t.Error("Could not remove loop device", device.deviceName, err)
-	}
-
-	// remove file
-	t.Log("Removing temporary file")
-	err = os.Remove(device.fileName)
-	if err != nil {
-		t.Error(err)
-	}
-
-	// remove mount point
-	t.Log("Removing temporary mount point")
-	err = os.RemoveAll(device.mountPoint)
-	if err != nil {
-		t.Error(err)
-	}
-}
-
-func TestGetBtrfsDevice(t *testing.T) {
-	testCases := []struct {
-		expectedDevice string
-		expectedError  string
-		root           string
-		mounts         []mount.Info
-	}{
-		{
-			expectedDevice: "/dev/loop0",
-			root:           "/var/lib/containerd/snapshot/btrfs",
-			mounts: []mount.Info{
-				{Root: "/", Mountpoint: "/", FSType: "ext4", Source: "/dev/sda1"},
-				{Root: "/", Mountpoint: "/var/lib/containerd/snapshot/btrfs", FSType: "btrfs", Source: "/dev/loop0"},
-			},
-		},
-		{
-			expectedError: "/var/lib/containerd/snapshot/btrfs is not mounted as btrfs",
-			root:          "/var/lib/containerd/snapshot/btrfs",
-			mounts: []mount.Info{
-				{Root: "/", Mountpoint: "/", FSType: "ext4", Source: "/dev/sda1"},
-			},
-		},
-		{
-			expectedDevice: "/dev/sda1",
-			root:           "/var/lib/containerd/snapshot/btrfs",
-			mounts: []mount.Info{
-				{Root: "/", Mountpoint: "/", FSType: "btrfs", Source: "/dev/sda1"},
-			},
-		},
-		{
-			expectedDevice: "/dev/sda2",
-			root:           "/var/lib/containerd/snapshot/btrfs",
-			mounts: []mount.Info{
-				{Root: "/", Mountpoint: "/", FSType: "btrfs", Source: "/dev/sda1"},
-				{Root: "/", Mountpoint: "/var/lib/containerd/snapshot/btrfs", FSType: "btrfs", Source: "/dev/sda2"},
-			},
-		},
-		{
-			expectedDevice: "/dev/sda2",
-			root:           "/var/lib/containerd/snapshot/btrfs",
-			mounts: []mount.Info{
-				{Root: "/", Mountpoint: "/var/lib/containerd/snapshot/btrfs", FSType: "btrfs", Source: "/dev/sda2"},
-				{Root: "/", Mountpoint: "/var/lib/foooooooooooooooooooo/baaaaaaaaaaaaaaaaaaaar", FSType: "btrfs", Source: "/dev/sda3"}, // mountpoint length longer than /var/lib/containerd/snapshot/btrfs
-				{Root: "/", Mountpoint: "/", FSType: "btrfs", Source: "/dev/sda1"},
-			},
-		},
-	}
-	for i, tc := range testCases {
-		device, err := getBtrfsDevice(tc.root, tc.mounts)
-		if err != nil && tc.expectedError == "" {
-			t.Fatalf("%d: expected nil, got %v", i, err)
-		}
-		if err != nil && !strings.Contains(err.Error(), tc.expectedError) {
-			t.Fatalf("%d: expected %s, got %v", i, tc.expectedError, err)
-		}
-		if err == nil && device != tc.expectedDevice {
-			t.Fatalf("%d: expected %s, got %s", i, tc.expectedDevice, device)
-		}
-	}
-}

testutil/loopback_linux.go

@@ -0,0 +1,53 @@
+// +build linux
+
+package testutil
+
+import (
+	"io/ioutil"
+	"os"
+	"os/exec"
+	"strings"
+	"testing"
+)
+
+// 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()) {
+	// 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)
+	}
+
+	if err := file.Truncate(size); err != nil {
+		t.Fatal(err)
+	}
+	file.Close()
+
+	// create device
+	losetup := exec.Command("losetup", "--find", "--show", file.Name())
+	p, err := losetup.Output()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	deviceName := strings.TrimSpace(string(p))
+	t.Logf("Created loop device %s (using %s)", deviceName, file.Name())
+
+	cleanup := func() {
+		// detach device
+		t.Logf("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)
+		}
+
+		// remove file
+		t.Logf("Removing temporary file %s", file.Name())
+		if err = os.Remove(file.Name()); err != nil {
+			t.Error(err)
+		}
+	}
+
+	return deviceName, cleanup
+}