btrfs: test btrfs snapshots with driver suite

We now include btrfs in the snapshot driver test suite. This includes
the addition of parent links and name hashing into the btrfs driver.
We'll probably endup replacing this with a common metadata store, as
these relationships are generally identical between implementations.

A small bug was discovered in the delete implementation in the course
testing, so the btrfs package has been updated with a fix.

The overlay driver was modified accordingly with the btrfs driver to use
`Driver` as the exported type of each driver packge.

Signed-off-by: Stephen J Day <stephen.day@docker.com>

snapshot/btrfs/btrfs.go

@@ -3,29 +3,54 @@ package btrfs
 import (
 	"crypto/sha256"
 	"fmt"
+	"io/ioutil"
 	"os"
 	"path/filepath"
 
 	"github.com/docker/containerd"
+	"github.com/docker/containerd/log"
+	"github.com/pkg/errors"
 	"github.com/stevvooe/go-btrfs"
 )
 
-type Btrfs struct {
+type Driver struct {
 	device string // maybe we can resolve it with path?
 	root   string // root provides paths for internal storage.
 }
 
-func NewBtrfs(device, root string) (*Btrfs, error) {
+func NewDriver(device, root string) (*Driver, error) {
-	return &Btrfs{device: device, root: root}, nil
+	return &Driver{device: device, root: root}, nil
 }
 
-func (b *Btrfs) Prepare(key, parent string) ([]containerd.Mount, error) {
+func (b *Driver) Prepare(key, parent string) ([]containerd.Mount, error) {
-	active := filepath.Join(b.root, "active")
+	return b.makeActive(key, parent, false)
-	if err := os.MkdirAll(active, 0755); err != nil {
+}
+
+func (b *Driver) View(key, parent string) ([]containerd.Mount, error) {
+	return b.makeActive(key, parent, true)
+}
+
+func (b *Driver) makeActive(key, parent string, readonly bool) ([]containerd.Mount, error) {
+	var (
+		active     = filepath.Join(b.root, "active")
+		parents    = filepath.Join(b.root, "parents")
+		snapshots  = filepath.Join(b.root, "snapshots")
+		names      = filepath.Join(b.root, "names")
+		dir        = filepath.Join(active, hash(key))
+		namep      = filepath.Join(names, hash(key))
+		parentlink = filepath.Join(parents, hash(key))
+		parentp    = filepath.Join(snapshots, hash(parent))
+	)
+
+	for _, path := range []string{
+		active,
+		parents,
+		names,
+	} {
+		if err := os.MkdirAll(path, 0755); err != nil {
 			return nil, err
 		}
-
+	}
-	dir := filepath.Join(active, hash(key))
 
 	if parent == "" {
 		// create new subvolume
@@ -35,37 +60,198 @@ func (b *Btrfs) Prepare(key, parent string) ([]containerd.Mount, error) {
 		}
 	} else {
 		// btrfs subvolume snapshot /parent /subvol
-		if err := btrfs.SubvolSnapshot(dir, parent, false); err != nil {
+		if err := btrfs.SubvolSnapshot(dir, parentp, readonly); err != nil {
+			return nil, err
+		}
+
+		if err := os.Symlink(parentp, parentlink); err != nil {
 			return nil, err
 		}
 	}
 
+	if err := ioutil.WriteFile(namep, []byte(key), 0644); err != nil {
+		return nil, err
+	}
+
+	return b.mounts(dir)
+}
+
+func (b *Driver) mounts(dir string) ([]containerd.Mount, error) {
+	var options []string
+
 	// get the subvolume id back out for the mount
 	info, err := btrfs.SubvolInfo(dir)
 	if err != nil {
 		return nil, err
 	}
 
+	options = append(options, fmt.Sprintf("subvolid=%d", info.ID))
+
+	if info.Readonly {
+		options = append(options, "ro")
+	}
+
 	return []containerd.Mount{
 		{
 			Type:   "btrfs",
 			Source: b.device, // device?
 			// NOTE(stevvooe): While it would be nice to use to uuids for
 			// mounts, they don't work reliably if the uuids are missing.
-			Options: []string{fmt.Sprintf("subvolid=%d", info.ID)},
+			Options: options,
 		},
 	}, nil
 }
 
-func (b *Btrfs) Commit(name, key string) error {
+func (b *Driver) Commit(name, key string) error {
-	dir := filepath.Join(b.root, "active", hash(key))
+	var (
+		active        = filepath.Join(b.root, "active")
+		snapshots     = filepath.Join(b.root, "snapshots")
+		names         = filepath.Join(b.root, "names")
+		parents       = filepath.Join(b.root, "parents")
+		dir           = filepath.Join(active, hash(key))
+		target        = filepath.Join(snapshots, hash(name))
+		keynamep      = filepath.Join(names, hash(key))
+		namep         = filepath.Join(names, hash(name))
+		keyparentlink = filepath.Join(parents, hash(key))
+		parentlink    = filepath.Join(parents, hash(name))
+	)
 
-	fmt.Println("commit to", name)
+	info, err := btrfs.SubvolInfo(dir)
-	if err := btrfs.SubvolSnapshot(name, dir, true); err != nil {
+	if err != nil {
 		return err
 	}
 
-	return btrfs.SubvolDelete(dir)
+	if info.Readonly {
+		return fmt.Errorf("may not commit view snapshot %q", dir)
+	}
+
+	// look up the parent information to make sure we have the right parent link.
+	parentp, err := os.Readlink(keyparentlink)
+	if err != nil {
+		if !os.IsNotExist(err) {
+			return err
+		}
+
+		// we have no parent!
+	}
+
+	if err := os.MkdirAll(snapshots, 0755); err != nil {
+		return err
+	}
+
+	fmt.Println("commit snapshot", target)
+	if err := btrfs.SubvolSnapshot(target, dir, true); err != nil {
+		fmt.Println("snapshot error")
+		return err
+	}
+
+	// remove the key name path as we no longer need it.
+	if err := os.Remove(keynamep); err != nil {
+		return err
+	}
+
+	if err := ioutil.WriteFile(namep, []byte(name), 0755); err != nil {
+		return err
+	}
+
+	if parentp != "" {
+		// move over the parent link into the commit name.
+		if err := os.Rename(keyparentlink, parentlink); err != nil {
+			return err
+		}
+
+		// TODO(stevvooe): For this to not break horribly, we should really
+		// start taking a full lock. We are going to move all this metadata
+		// into common storage, so let's not fret over it for now.
+	}
+
+	if err := btrfs.SubvolDelete(dir); err != nil {
+		return errors.Wrapf(err, "delete subvol failed on %v", dir)
+	}
+
+	return nil
+}
+
+// Mounts returns the mounts for the transaction identified by key. Can be
+// called on an read-write or readonly transaction.
+//
+// This can be used to recover mounts after calling View or Prepare.
+func (b *Driver) Mounts(key string) ([]containerd.Mount, error) {
+	dir := filepath.Join(b.root, "active", hash(key))
+	return b.mounts(dir)
+}
+
+// Remove abandons the transaction identified by key. All resources
+// associated with the key will be removed.
+func (b *Driver) Remove(key string) error {
+	panic("not implemented")
+}
+
+// Parent returns the parent of snapshot identified by name.
+func (b *Driver) Parent(name string) (string, error) {
+	var (
+		parents    = filepath.Join(b.root, "parents")
+		names      = filepath.Join(b.root, "names")
+		parentlink = filepath.Join(parents, hash(name))
+	)
+
+	parentp, err := os.Readlink(parentlink)
+	if err != nil {
+		if !os.IsNotExist(err) {
+			return "", err
+		}
+
+		return "", nil // no parent!
+	}
+
+	// okay, grab the basename of the parent and look up its name!
+	parentnamep := filepath.Join(names, filepath.Base(parentp))
+
+	p, err := ioutil.ReadFile(parentnamep)
+	if err != nil {
+		return "", err
+	}
+
+	return string(p), nil
+}
+
+// Exists returns true if the snapshot with name exists.
+func (b *Driver) Exists(name string) bool {
+	target := filepath.Join(b.root, "snapshots", hash(name))
+
+	if _, err := os.Stat(target); err != nil {
+		if !os.IsNotExist(err) {
+			// TODO(stevvooe): Very rare condition when this fails horribly,
+			// such as an access error. Ideally, Exists is simple, but we may
+			// consider returning an error.
+			log.L.WithError(err).Fatal("error encountered checking for snapshot existence")
+		}
+
+		return false
+	}
+
+	return true
+}
+
+// Delete the snapshot idenfitied by name.
+//
+// If name has children, the operation will fail.
+func (b *Driver) Delete(name string) error {
+	panic("not implemented")
+}
+
+// TODO(stevvooe): The methods below are still in flux. We'll need to work
+// out the roles of active and committed snapshots for this to become more
+// clear.
+
+// Walk the committed snapshots.
+func (b *Driver) Walk(fn func(name string) error) error {
+	panic("not implemented")
+}
+
+// Active will call fn for each active transaction.
+func (b *Driver) Active(fn func(key string) error) error {
+	panic("not implemented")
 }
 
 func hash(k string) string {

snapshot/btrfs/btrfs_test.go

@@ -10,8 +10,8 @@ import (
 	"testing"
 
 	"github.com/docker/containerd"
+	"github.com/docker/containerd/snapshot"
 	"github.com/docker/containerd/testutil"
-	btrfs "github.com/stevvooe/go-btrfs"
 )
 
 const (
@@ -20,8 +20,32 @@ const (
 
 func TestBtrfs(t *testing.T) {
 	testutil.RequiresRoot(t)
-	device := setupBtrfsLoopbackDevice(t)
+	snapshot.DriverSuite(t, "Btrfs", func(root string) (snapshot.Driver, func(), error) {
-	defer removeBtrfsLoopbackDevice(t, device)
+		device := setupBtrfsLoopbackDevice(t, root)
+		driver, err := NewDriver(device.deviceName, root)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		return driver, func() {
+			device.remove(t)
+		}, nil
+	})
+}
+
+func TestBtrfsMounts(t *testing.T) {
+	testutil.RequiresRoot(t)
+
+	// create temporary directory for mount point
+	mountPoint, err := ioutil.TempDir("", "containerd-btrfs-test")
+	if err != nil {
+		t.Fatal("could not create mount point for btrfs test", err)
+	}
+	t.Log("temporary mount point created", mountPoint)
+
+	device := setupBtrfsLoopbackDevice(t, mountPoint)
+	defer device.remove(t)
+
 	root, err := ioutil.TempDir(device.mountPoint, "TestBtrfsPrepare-")
 	if err != nil {
 		t.Fatal(err)
@@ -29,7 +53,7 @@ func TestBtrfs(t *testing.T) {
 	defer os.RemoveAll(root)
 
 	target := filepath.Join(root, "test")
-	b, err := NewBtrfs(device.deviceName, root)
+	b, err := NewDriver(device.deviceName, root)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -72,13 +96,6 @@ func TestBtrfs(t *testing.T) {
 	if err := b.Commit(filepath.Join(root, "snapshots/committed"), filepath.Join(root, "test")); err != nil {
 		t.Fatal(err)
 	}
-	defer func() {
-		t.Log("Delete snapshot 1")
-		err := btrfs.SubvolDelete(filepath.Join(root, "snapshots/committed"))
-		if err != nil {
-			t.Error("snapshot delete failed", err)
-		}
-	}()
 
 	target = filepath.Join(root, "test2")
 	mounts, err = b.Prepare(target, filepath.Join(root, "snapshots/committed"))
@@ -103,13 +120,6 @@ func TestBtrfs(t *testing.T) {
 	if err := b.Commit(filepath.Join(root, "snapshots/committed2"), target); err != nil {
 		t.Fatal(err)
 	}
-	defer func() {
-		t.Log("Delete snapshot 2")
-		err := btrfs.SubvolDelete(filepath.Join(root, "snapshots/committed2"))
-		if err != nil {
-			t.Error("snapshot delete failed", err)
-		}
-	}()
 }
 
 type testDevice struct {
@@ -121,13 +131,7 @@ type testDevice struct {
 // 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) *testDevice {
+func setupBtrfsLoopbackDevice(t *testing.T, mountPoint string) *testDevice {
-	// create temporary directory for mount point
-	mountPoint, err := ioutil.TempDir("", "containerd-btrfs-test")
-	if err != nil {
-		t.Fatal("Could not create mount point for btrfs test", err)
-	}
-	t.Log("Temporary mount point created", mountPoint)
 
 	// create temporary file for the disk image
 	file, err := ioutil.TempFile("", "containerd-btrfs-test")
@@ -182,9 +186,9 @@ func setupBtrfsLoopbackDevice(t *testing.T) *testDevice {
 	}
 }
 
-// removeBtrfsLoopbackDevice unmounts the loopback device and deletes the
+// remove cleans up the test device, unmounting the loopback and disk image
-// file holding the disk image.
+// file.
-func removeBtrfsLoopbackDevice(t *testing.T, device *testDevice) {
+func (device *testDevice) remove(t *testing.T) {
 	// unmount
 	testutil.Unmount(t, device.mountPoint)
 

snapshot/driver.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/docker/containerd"
 	"github.com/docker/containerd/testutil"
+	"github.com/stretchr/testify/assert"
 )
 
 // Driver defines the methods required to implement a snapshot driver for
@@ -258,9 +259,7 @@ func checkDriverBasic(t *testing.T, driver Driver, work string) {
 		t.Fatal(err)
 	}
 
-	if parent != "" {
+	assert.Equal(t, parent, "")
-		t.Fatalf("parent of new layer should be empty, got driver.Parent(%q) == %q", committed, parent)
-	}
 
 	next := filepath.Join(work, "nextlayer")
 	if err := os.MkdirAll(next, 0777); err != nil {
@@ -295,7 +294,9 @@ func checkDriverBasic(t *testing.T, driver Driver, work string) {
 	}
 
 	parent, err = driver.Parent(nextCommitted)
-	if parent != committed {
+	if err != nil {
-		t.Fatalf("parent of new layer should be %q, got driver.Parent(%q) == %q", committed, next, parent)
+		t.Fatal(err)
 	}
+
+	assert.Equal(t, parent, committed)
 }

snapshot/overlay/overlay.go

@@ -12,7 +12,12 @@ import (
 	digest "github.com/opencontainers/go-digest"
 )
 
-func NewDriver(root string) (*Overlay, error) {
+type Driver struct {
+	root  string
+	cache *cache
+}
+
+func NewDriver(root string) (*Driver, error) {
 	if err := os.MkdirAll(root, 0700); err != nil {
 		return nil, err
 	}
@@ -24,18 +29,13 @@ func NewDriver(root string) (*Overlay, error) {
 			return nil, err
 		}
 	}
-	return &Overlay{
+	return &Driver{
 		root:  root,
 		cache: newCache(),
 	}, nil
 }
 
-type Overlay struct {
+func (o *Driver) Prepare(key, parent string) ([]containerd.Mount, error) {
-	root  string
-	cache *cache
-}
-
-func (o *Overlay) Prepare(key, parent string) ([]containerd.Mount, error) {
 	active, err := o.newActiveDir(key)
 	if err != nil {
 		return nil, err
@@ -48,7 +48,7 @@ func (o *Overlay) Prepare(key, parent string) ([]containerd.Mount, error) {
 	return o.Mounts(key)
 }
 
-func (o *Overlay) View(key, parent string) ([]containerd.Mount, error) {
+func (o *Driver) View(key, parent string) ([]containerd.Mount, error) {
 	panic("not implemented")
 }
 
@@ -56,24 +56,24 @@ func (o *Overlay) View(key, parent string) ([]containerd.Mount, error) {
 // called on an read-write or readonly transaction.
 //
 // This can be used to recover mounts after calling View or Prepare.
-func (o *Overlay) Mounts(key string) ([]containerd.Mount, error) {
+func (o *Driver) Mounts(key string) ([]containerd.Mount, error) {
 	active := o.getActive(key)
 	return active.mounts(o.cache)
 }
 
-func (o *Overlay) Commit(name, key string) error {
+func (o *Driver) Commit(name, key string) error {
 	active := o.getActive(key)
 	return active.commit(name, o.cache)
 }
 
 // Remove abandons the transaction identified by key. All resources
 // associated with the key will be removed.
-func (o *Overlay) Remove(key string) error {
+func (o *Driver) Remove(key string) error {
 	panic("not implemented")
 }
 
 // Parent returns the parent of snapshot identified by name.
-func (o *Overlay) Parent(name string) (string, error) {
+func (o *Driver) Parent(name string) (string, error) {
 	ppath, err := o.cache.get(filepath.Join(o.root, "snapshots", hash(name)))
 	if err != nil {
 		if os.IsNotExist(err) {
@@ -92,28 +92,28 @@ func (o *Overlay) Parent(name string) (string, error) {
 }
 
 // Exists returns true if the snapshot with name exists.
-func (o *Overlay) Exists(name string) bool {
+func (o *Driver) Exists(name string) bool {
 	panic("not implemented")
 }
 
 // Delete the snapshot idenfitied by name.
 //
 // If name has children, the operation will fail.
-func (o *Overlay) Delete(name string) error {
+func (o *Driver) Delete(name string) error {
 	panic("not implemented")
 }
 
 // Walk the committed snapshots.
-func (o *Overlay) Walk(fn func(name string) error) error {
+func (o *Driver) Walk(fn func(name string) error) error {
 	panic("not implemented")
 }
 
 // Active will call fn for each active transaction.
-func (o *Overlay) Active(fn func(key string) error) error {
+func (o *Driver) Active(fn func(key string) error) error {
 	panic("not implemented")
 }
 
-func (o *Overlay) newActiveDir(key string) (*activeDir, error) {
+func (o *Driver) newActiveDir(key string) (*activeDir, error) {
 	var (
 		path = filepath.Join(o.root, "active", hash(key))
 	)
@@ -133,7 +133,7 @@ func (o *Overlay) newActiveDir(key string) (*activeDir, error) {
 	return a, nil
 }
 
-func (o *Overlay) getActive(key string) *activeDir {
+func (o *Driver) getActive(key string) *activeDir {
 	return &activeDir{
 		path:         filepath.Join(o.root, "active", hash(key)),
 		snapshotsDir: filepath.Join(o.root, "snapshots"),

testutil/helpers.go

@@ -2,8 +2,10 @@ package testutil
 
 import (
 	"flag"
+	"io/ioutil"
 	"os"
 	"path/filepath"
+	"strconv"
 	"syscall"
 	"testing"
 
@@ -55,6 +57,18 @@ func DumpDir(t *testing.T, root string) {
 				return err
 			}
 			t.Log(fi.Mode(), path, "->", target)
+		} else if fi.Mode().IsRegular() {
+			p, err := ioutil.ReadFile(path)
+			if err != nil {
+				t.Log("error reading file: %v", err)
+				return nil
+			}
+
+			if len(p) > 64 { // just display a little bit.
+				p = p[:64]
+			}
+
+			t.Log(fi.Mode(), path, "[", strconv.Quote(string(p)), "...]")
 		} else {
 			t.Log(fi.Mode(), path)
 		}

vendor.conf

@@ -39,7 +39,7 @@ github.com/opencontainers/runtime-spec v1.0.0-rc3
 # logrus, latest release as of 12/16/2016
 github.com/Sirupsen/logrus v0.11.0
 # go-btrfs from stevvooe; master as of 1/11/2017
-github.com/stevvooe/go-btrfs 029908fedf190147f3f673b0db7c836f83a6a6be
+github.com/stevvooe/go-btrfs 8539a1d04898663b8eda14982e24b74e7a12388e
 # testify go testing support; latest release as of 12/16/2016
 github.com/stretchr/testify v1.1.4
 # go-spew (required by testify, and also by ctr); latest release as of 1/12/2017

vendor/github.com/stevvooe/go-btrfs/btrfs.go

@@ -328,11 +328,11 @@ func SubvolDelete(path string) error {
 		}
 
 		if err := isFileInfoSubvol(fi); err != nil {
-			return err
+			return nil
 		}
 
 		if err := SubvolDelete(p); err != nil {
-			return err
+			return errors.Wrapf(err, "recursive delete of %v failed", p)
 		}
 
 		return filepath.SkipDir // children get walked by call above.