Merge pull request #484 from stevvooe/snapshot-drivers

[WIP] snapshot: define the snapshot driver interface

content/content_test.go

@@ -16,12 +16,14 @@ import (
 	"testing"
 	"time"
 
+	"github.com/docker/containerd/testutil"
 	"github.com/opencontainers/go-digest"
 )
 
 func TestContentWriter(t *testing.T) {
 	tmpdir, cs, cleanup := contentStoreEnv(t)
 	defer cleanup()
+	defer testutil.DumpDir(t, tmpdir)
 
 	if _, err := os.Stat(filepath.Join(tmpdir, "ingest")); os.IsNotExist(err) {
 		t.Fatal("ingest dir should be created", err)
@@ -114,7 +116,6 @@ func TestContentWriter(t *testing.T) {
 		t.Fatal("mismatched data written to disk")
 	}
 
-	dumpDir(tmpdir)
 }
 
 func TestWalkBlobs(t *testing.T) {
@@ -279,14 +280,3 @@ func checkWrite(t checker, cs *Store, dgst digest.Digest, p []byte) digest.Diges
 
 	return dgst
 }
-
-func dumpDir(root string) error {
-	return filepath.Walk(root, func(path string, fi os.FileInfo, err error) error {
-		if err != nil {
-			return err
-		}
-
-		fmt.Println(fi.Mode(), path)
-		return nil
-	})
-}

mount.go

@@ -3,6 +3,9 @@ package containerd
 import (
 	"strings"
 	"syscall"
+
+	"github.com/Sirupsen/logrus"
+	"github.com/docker/containerd/log"
 )
 
 // Mount is the lingua franca of containerd. A mount represents a
@@ -20,6 +23,13 @@ type Mount struct {
 
 func (m *Mount) Mount(target string) error {
 	flags, data := parseMountOptions(m.Options)
+
+	lfields := logrus.Fields{"target": target, "source": m.Source}
+	if data != "" {
+		lfields["data"] = data
+	}
+	log.L.WithFields(lfields).Debug("syscall.Mount")
+
 	return syscall.Mount(m.Source, target, m.Type, uintptr(flags), data)
 }
 

snapshot/btrfs/btrfs_test.go

@@ -10,7 +10,7 @@ import (
 	"testing"
 
 	"github.com/docker/containerd"
-	"github.com/docker/containerd/snapshot/testutil"
+	"github.com/docker/containerd/testutil"
 	btrfs "github.com/stevvooe/go-btrfs"
 )
 

snapshot/driver.go

@@ -0,0 +1,301 @@
+package snapshot
+
+import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/docker/containerd"
+	"github.com/docker/containerd/testutil"
+)
+
+// Driver defines the methods required to implement a snapshot driver for
+// allocating, snapshotting and mounting abstract filesystems. The model works
+// by building up sets of changes with parent-child relationships.
+//
+// These differ from the concept of the graphdriver in that the Manager has no
+// knowledge of images, layers or containers. Users simply prepare and commit
+// directories. We also avoid the integration between graph driver's and the
+// tar format used to represent the changesets.
+//
+// A snapshot represents a filesystem state. Every snapshot has a parent, where
+// the empty parent is represented by the empty string. A diff can be taken
+// between a parent and its snapshot to generate a classic layer.
+//
+// For convention, we define the following terms to be used throughout this
+// interface for driver implementations:
+//
+// 	`name` - refers to a forkable snapshot, typically read only
+// 	`key` - refers to an active transaction, either a prepare or view
+// 	`parent` - refers to the parent in relation to a name
+//
+// TODO(stevvooe): Update this description when things settle.
+//
+// Importing a Layer
+//
+// To import a layer, we simply have the Manager provide a list of
+// mounts to be applied such that our dst will capture a changeset. We start
+// out by getting a path to the layer tar file and creating a temp location to
+// unpack it to:
+//
+//	layerPath, tmpLocation := getLayerPath(), mkTmpDir() // just a path to layer tar file.
+//
+// We then use a Manager to prepare the temporary location as a
+// snapshot point:
+//
+// 	sm := NewManager()
+//	mounts, err := sm.Prepare(tmpLocation, "")
+// 	if err != nil { ... }
+//
+// Note that we provide "" as the parent, since we are applying the diff to an
+// empty directory. We get back a list of mounts from Manager.Prepare.
+// Before proceeding, we perform all these mounts:
+//
+//	if err := MountAll(mounts); err != nil { ... }
+//
+// Once the mounts are performed, our temporary location is ready to capture
+// a diff. In practice, this works similar to a filesystem transaction. The
+// next step is to unpack the layer. We have a special function unpackLayer
+// that applies the contents of the layer to target location and calculates the
+// DiffID of the unpacked layer (this is a requirement for docker
+// implementation):
+//
+//	layer, err := os.Open(layerPath)
+//	if err != nil { ... }
+// 	digest, err := unpackLayer(tmpLocation, layer) // unpack into layer location
+// 	if err != nil { ... }
+//
+// When the above completes, we should have a filesystem the represents the
+// contents of the layer. Careful implementations should verify that digest
+// matches the expected DiffID. When completed, we unmount the mounts:
+//
+//	unmount(mounts) // optional, for now
+//
+// Now that we've verified and unpacked our layer, we create a location to
+// commit the actual diff. For this example, we are just going to use the layer
+// digest, but in practice, this will probably be the ChainID:
+//
+// 	diffPath := filepath.Join("/layers", digest) // name location for the uncompressed layer digest
+//	if err := sm.Commit(diffPath, tmpLocation); err != nil { ... }
+//
+// Now, we have a layer in the Manager that can be accessed with the
+// opaque diffPath provided during commit.
+//
+// Importing the Next Layer
+//
+// Making a layer depend on the above is identical to the process described
+// above except that the parent is provided as diffPath when calling
+// Manager.Prepare:
+//
+// 	mounts, err := sm.Prepare(tmpLocation, parentDiffPath)
+//
+// The diff will be captured at tmpLocation, as the layer is applied.
+//
+// Running a Container
+//
+// To run a container, we simply provide Manager.Prepare the diffPath
+// of the image we want to start the container from. After mounting, the
+// prepared path can be used directly as the container's filesystem:
+//
+// 	mounts, err := sm.Prepare(containerRootFS, imageDiffPath)
+//
+// The returned mounts can then be passed directly to the container runtime. If
+// one would like to create a new image from the filesystem,
+// Manager.Commit is called:
+//
+// 	if err := sm.Commit(newImageDiff, containerRootFS); err != nil { ... }
+//
+// Alternatively, for most container runs, Manager.Rollback will be
+// called to signal Manager to abandon the changes.
+type Driver interface {
+	// Prepare returns a set of mounts corresponding to an active snapshot
+	// transaction, identified by the provided transaction key.
+	//
+	// If a parent is provided, after performing the mounts, the destination
+	// will start with the content of the parent. Changes to the mounted
+	// destination will be captured in relation to the provided parent. The
+	// default parent, "", is an empty directory.
+	//
+	// The changes may be saved to a new snapshot by calling Commit. When one
+	// is done with the transaction, Remove should be called on the key.
+	//
+	// Multiple calls to Prepare or View with the same key should fail.
+	Prepare(key, parent string) ([]containerd.Mount, error)
+
+	// View behaves identically to Prepare except the result may not be committed
+	// back to the snapshot manager. View returns a readonly view on the
+	// parent, with the transaction tracked by the given key.
+	//
+	// This method operates identically to Prepare, except that Mounts returned
+	// may have the readonly flag set. Any modifications to the underlying
+	// filesystem will be ignored.
+	//
+	// Commit may not be called on the provided key. To collect the resources
+	// associated with key, Remove must be called with key as the argument.
+	View(key, parent string) ([]containerd.Mount, error)
+
+	// Commit captures the changes between key and its parent into a snapshot
+	// identified by name.  The name can then be used with the driver's other
+	// methods to create subsequent snapshots.
+	//
+	// A snapshot will be created under name with the parent that started the
+	// transaction.
+	//
+	// Commit may be called multiple times on the same key. Snapshots created
+	// in this manner will all reference the parent used to start the
+	// transaction.
+	Commit(name, key string) error
+
+	// 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.
+	Mounts(key string) ([]containerd.Mount, error)
+
+	// Remove abandons the transaction identified by key. All resources
+	// associated with the key will be removed.
+	Remove(key string) error
+
+	// Parent returns the parent of snapshot identified by name.
+	Parent(name string) (string, error)
+
+	// Exists returns true if the snapshot with name exists.
+	Exists(name string) bool
+
+	// Delete the snapshot idenfitied by name.
+	//
+	// If name has children, the operation will fail.
+	Delete(name string) error
+
+	// 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.
+	Walk(fn func(name string) error) error
+
+	// Active will call fn for each active transaction.
+	Active(fn func(key string) error) error
+}
+
+// DriverSuite runs a test suite on the driver given a factory function.
+func DriverSuite(t *testing.T, name string, driverFn func(root string) (Driver, func(), error)) {
+	t.Run("Basic", makeTest(t, name, driverFn, checkDriverBasic))
+}
+
+func makeTest(t *testing.T, name string, driverFn func(root string) (Driver, func(), error), fn func(t *testing.T, driver Driver, work string)) func(t *testing.T) {
+	return func(t *testing.T) {
+		// Make two directories: a driver root and a play area for the tests:
+		//
+		// 	/tmp
+		// 		work/ -> passed to test functions
+		// 		root/ -> passed to driver
+		//
+		tmpDir, err := ioutil.TempDir("", "snapshot-suite-"+name+"-")
+		if err != nil {
+			t.Fatal(err)
+		}
+		defer os.RemoveAll(tmpDir)
+
+		root := filepath.Join(tmpDir, "root")
+		if err := os.MkdirAll(root, 0777); err != nil {
+			t.Fatal(err)
+		}
+
+		driver, cleanup, err := driverFn(root)
+		if err != nil {
+			t.Fatal(err)
+		}
+		defer cleanup()
+
+		work := filepath.Join(tmpDir, "work")
+		if err := os.MkdirAll(work, 0777); err != nil {
+			t.Fatal(err)
+		}
+
+		defer testutil.DumpDir(t, tmpDir)
+		fn(t, driver, work)
+	}
+}
+
+// checkDriverBasic tests the basic workflow of a snapshot driver.
+func checkDriverBasic(t *testing.T, driver Driver, work string) {
+	preparing := filepath.Join(work, "preparing")
+	if err := os.MkdirAll(preparing, 0777); err != nil {
+		t.Fatal(err)
+	}
+
+	mounts, err := driver.Prepare(preparing, "")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if len(mounts) < 1 {
+		t.Fatal("expected mounts to have entries")
+	}
+
+	if err := containerd.MountAll(mounts, preparing); err != nil {
+		t.Fatal(err)
+	}
+	defer testutil.Unmount(t, preparing)
+
+	if err := ioutil.WriteFile(filepath.Join(preparing, "foo"), []byte("foo\n"), 0777); err != nil {
+		t.Fatal(err)
+	}
+
+	if err := os.MkdirAll(filepath.Join(preparing, "a", "b", "c"), 0755); err != nil {
+		t.Fatal(err)
+	}
+
+	committed := filepath.Join(work, "committed")
+	if err := driver.Commit(committed, preparing); err != nil {
+		t.Fatal(err)
+	}
+
+	parent, err := driver.Parent(committed)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if 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 {
+		t.Fatal(err)
+	}
+
+	mounts, err = driver.Prepare(next, committed)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := containerd.MountAll(mounts, next); err != nil {
+		t.Fatal(err)
+	}
+	defer testutil.Unmount(t, next)
+
+	if err := ioutil.WriteFile(filepath.Join(next, "bar"), []byte("bar\n"), 0777); err != nil {
+		t.Fatal(err)
+	}
+
+	// also, change content of foo to bar
+	if err := ioutil.WriteFile(filepath.Join(next, "foo"), []byte("bar\n"), 0777); err != nil {
+		t.Fatal(err)
+	}
+
+	if err := os.RemoveAll(filepath.Join(next, "a", "b")); err != nil {
+		t.Log(err)
+	}
+
+	nextCommitted := filepath.Join(work, "committed-next")
+	if err := driver.Commit(nextCommitted, next); err != nil {
+		t.Fatal(err)
+	}
+
+	parent, err = driver.Parent(nextCommitted)
+	if parent != committed {
+		t.Fatalf("parent of new layer should be %q, got driver.Parent(%q) == %q", committed, next, parent)
+	}
+}

snapshot/manager.go

@@ -1,308 +0,0 @@
-package snapshot
-
-import (
-	"fmt"
-	"io/ioutil"
-	"os"
-	"path/filepath"
-	"strings"
-
-	"github.com/docker/containerd"
-)
-
-// Manager provides an API for allocating, snapshotting and mounting
-// abstract, layer-based filesystems. The model works by building up sets of
-// directories with parent-child relationships.
-//
-// These differ from the concept of the graphdriver in that the
-// Manager has no knowledge of images or containers. Users simply
-// prepare and commit directories. We also avoid the integration between graph
-// driver's and the tar format used to represent the changesets.
-//
-// Importing a Layer
-//
-// To import a layer, we simply have the Manager provide a list of
-// mounts to be applied such that our dst will capture a changeset. We start
-// out by getting a path to the layer tar file and creating a temp location to
-// unpack it to:
-//
-//	layerPath, tmpLocation := getLayerPath(), mkTmpDir() // just a path to layer tar file.
-//
-// We then use a Manager to prepare the temporary location as a
-// snapshot point:
-//
-// 	sm := NewManager()
-//	mounts, err := sm.Prepare(tmpLocation, "")
-// 	if err != nil { ... }
-//
-// Note that we provide "" as the parent, since we are applying the diff to an
-// empty directory. We get back a list of mounts from Manager.Prepare.
-// Before proceeding, we perform all these mounts:
-//
-//	if err := MountAll(mounts); err != nil { ... }
-//
-// Once the mounts are performed, our temporary location is ready to capture
-// a diff. In practice, this works similar to a filesystem transaction. The
-// next step is to unpack the layer. We have a special function unpackLayer
-// that applies the contents of the layer to target location and calculates the
-// DiffID of the unpacked layer (this is a requirement for docker
-// implementation):
-//
-//	layer, err := os.Open(layerPath)
-//	if err != nil { ... }
-// 	digest, err := unpackLayer(tmpLocation, layer) // unpack into layer location
-// 	if err != nil { ... }
-//
-// When the above completes, we should have a filesystem the represents the
-// contents of the layer. Careful implementations should verify that digest
-// matches the expected DiffID. When completed, we unmount the mounts:
-//
-//	unmount(mounts) // optional, for now
-//
-// Now that we've verified and unpacked our layer, we create a location to
-// commit the actual diff. For this example, we are just going to use the layer
-// digest, but in practice, this will probably be the ChainID:
-//
-// 	diffPath := filepath.Join("/layers", digest) // name location for the uncompressed layer digest
-//	if err := sm.Commit(diffPath, tmpLocation); err != nil { ... }
-//
-// Now, we have a layer in the Manager that can be accessed with the
-// opaque diffPath provided during commit.
-//
-// Importing the Next Layer
-//
-// Making a layer depend on the above is identical to the process described
-// above except that the parent is provided as diffPath when calling
-// Manager.Prepare:
-//
-// 	mounts, err := sm.Prepare(tmpLocation, parentDiffPath)
-//
-// The diff will be captured at tmpLocation, as the layer is applied.
-//
-// Running a Container
-//
-// To run a container, we simply provide Manager.Prepare the diffPath
-// of the image we want to start the container from. After mounting, the
-// prepared path can be used directly as the container's filesystem:
-//
-// 	mounts, err := sm.Prepare(containerRootFS, imageDiffPath)
-//
-// The returned mounts can then be passed directly to the container runtime. If
-// one would like to create a new image from the filesystem,
-// Manager.Commit is called:
-//
-// 	if err := sm.Commit(newImageDiff, containerRootFS); err != nil { ... }
-//
-// Alternatively, for most container runs, Manager.Rollback will be
-// called to signal Manager to abandon the changes.
-//
-// TODO(stevvooe): Consider an alternate API that provides an active object to
-// represent the lifecycle:
-//
-// 	work, err := sm.Prepare(dst, parent)
-//  mountAll(work.Mounts())
-// 	work.Commit() || work.Rollback()
-//
-// TODO(stevvooe): Manager should be an interface with several
-// implementations, similar to graphdriver.
-type Manager struct {
-	root string // root provides paths for internal storage.
-
-	// just a simple overlay implementation.
-	active  map[string]activeLayer
-	parents map[string]string // diff to parent for all committed
-}
-
-type activeLayer struct {
-	parent   string
-	upperdir string
-	workdir  string
-}
-
-func NewManager(root string) (*Manager, error) {
-	if err := os.MkdirAll(root, 0777); err != nil {
-		return nil, err
-	}
-
-	return &Manager{
-		root:    root,
-		active:  make(map[string]activeLayer),
-		parents: make(map[string]string),
-	}, nil
-}
-
-// Prepare returns a set of mounts such that dst can be used as a location for
-// reading and writing data. If parent is provided, the dst will be setup to
-// capture changes between dst and parent. The "default" parent, "", is an
-// empty directory.
-//
-// If the caller intends to write data to dst, they should perform all mounts
-// provided before doing so. The location defined by dst should be used as the
-// working directory for any associated activity, such as running a container
-// or importing a layer.
-//
-// The implementation may choose to write data directly to dst, opting to
-// return no mounts instead.
-//
-// Once the writes have completed, Manager.Commit or
-// Manager.Rollback should be called on dst.
-func (sm *Manager) Prepare(dst, parent string) ([]containerd.Mount, error) {
-	// we want to build up lowerdir, upperdir and workdir options for the
-	// overlay mount.
-	//
-	// lowerdir is a list of parent diffs, ordered from top to bottom (base
-	// layer to the "right").
-	//
-	// upperdir will become the diff location. This will be renamed to the
-	// location provided in commit.
-	//
-	// workdir needs to be there but it is not really clear why.
-	var opts []string
-
-	upperdir, err := ioutil.TempDir(sm.root, "diff-")
-	if err != nil {
-		return nil, err
-	}
-	opts = append(opts, "upperdir="+upperdir)
-
-	workdir, err := ioutil.TempDir(sm.root, "work-")
-	if err != nil {
-		return nil, err
-	}
-	opts = append(opts, "workdir="+workdir)
-
-	empty := filepath.Join(sm.root, "empty")
-	if err := os.MkdirAll(empty, 0777); err != nil {
-		return nil, err
-	}
-
-	// TODO(stevvooe): Write this metadata to disk to make it useful.
-	sm.active[dst] = activeLayer{
-		parent:   parent,
-		upperdir: upperdir,
-		workdir:  workdir,
-	}
-
-	var parents []string
-	for parent != "" {
-		parents = append(parents, parent)
-		parent = sm.Parent(parent)
-	}
-
-	if len(parents) == 0 {
-		parents = []string{empty}
-	}
-
-	opts = append(opts, "lowerdir="+strings.Join(parents, ","))
-
-	return []containerd.Mount{
-		{
-			Type:    "overlay",
-			Source:  "none",
-			Options: opts,
-		},
-	}, nil
-}
-
-// View behaves identically to Prepare except the result may not be committed
-// back to the snapshot manager.
-//
-// Whether or not these are readonly mounts is implementation specific, but the
-// caller may write to dst freely.
-//
-// Calling Commit on dst will result in an error. Calling Rollback on dst
-// should be done to cleanup resources.
-func (sm *Manager) View(dst, parent string) ([]containerd.Mount, error) {
-	panic("not implemented")
-}
-
-// Commit captures the changes between dst and its parent into the path
-// provided by diff. The path diff can then be used with the snapshot
-// manager's other methods to access the diff content.
-//
-// The contents of diff are opaque to the caller and may be specific to the
-// implementation of the layer backend.
-func (sm *Manager) Commit(diff, dst string) error {
-	active, ok := sm.active[dst]
-	if !ok {
-		return fmt.Errorf("%q must be an active layer", dst)
-	}
-
-	// move upperdir into the diff dir
-	if err := os.Rename(active.upperdir, diff); err != nil {
-		return err
-	}
-
-	// Clean up the working directory; we may not want to do this if we want to
-	// support re-entrant calls to Commit.
-	if err := os.RemoveAll(active.workdir); err != nil {
-		return err
-	}
-
-	sm.parents[diff] = active.parent
-	delete(sm.active, dst) // remove from active, again, consider not doing this to support multiple commits.
-	// note that allowing multiple commits would require copy for overlay.
-
-	return nil
-}
-
-// Rollback can be called after prepare if the caller would like to abandon the
-// changeset.
-func (sm *Manager) Rollback(dst string) error {
-	active, ok := sm.active[dst]
-	if !ok {
-		return fmt.Errorf("%q must be an active layer", dst)
-	}
-
-	var err error
-	err = os.RemoveAll(active.upperdir)
-	err = os.RemoveAll(active.workdir)
-
-	delete(sm.active, dst)
-	return err
-}
-
-// Parent returns the parent of the layer at diff.
-func (sm *Manager) Parent(diff string) string {
-	return sm.parents[diff]
-}
-
-type ChangeKind int
-
-const (
-	ChangeKindAdd = iota
-	ChangeKindModify
-	ChangeKindDelete
-)
-
-func (k ChangeKind) String() string {
-	switch k {
-	case ChangeKindAdd:
-		return "add"
-	case ChangeKindModify:
-		return "modify"
-	case ChangeKindDelete:
-		return "delete"
-	default:
-		return ""
-	}
-}
-
-// Change represents single change between a diff and its parent.
-//
-// TODO(stevvooe): There are some cool tricks we can do with this type. If we
-// provide the path to the resource from both the diff and its parent, for
-// example, we can have the differ actually decide the granularity represented
-// in the final changeset.
-type Change struct {
-	Kind ChangeKind
-	Path string
-}
-
-// TODO(stevvooe): Make this change emit through a Walk-like interface. We can
-// see this patten used in several tar'ing methods in pkg/archive.
-
-// Changes returns the list of changes from the diff's parent.
-func (sm *Manager) Changes(diff string) ([]Change, error) {
-	panic("not implemented")
-}

snapshot/manager_test.go

@@ -1,104 +0,0 @@
-package snapshot
-
-import (
-	"io/ioutil"
-	"os"
-	"path/filepath"
-	"testing"
-
-	"github.com/docker/containerd"
-	"github.com/docker/containerd/snapshot/testutil"
-)
-
-// TestSnapshotManagerBasic implements something similar to the conceptual
-// examples we've discussed thus far. It does perform mounts, so you must run
-// as root.
-func TestSnapshotManagerBasic(t *testing.T) {
-	testutil.RequiresRoot(t)
-	tmpDir, err := ioutil.TempDir("", "test-sm-")
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer func() {
-		t.Log("Removing", tmpDir)
-		err := os.RemoveAll(tmpDir)
-		if err != nil {
-			t.Error(err)
-		}
-	}()
-
-	root := filepath.Join(tmpDir, "root")
-
-	sm, err := NewManager(root)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	preparing := filepath.Join(tmpDir, "preparing")
-	if err := os.MkdirAll(preparing, 0777); err != nil {
-		t.Fatal(err)
-	}
-
-	mounts, err := sm.Prepare(preparing, "")
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	if len(mounts) < 1 {
-		t.Fatal("expected mounts to have entries")
-	}
-
-	if err := containerd.MountAll(mounts, preparing); err != nil {
-		t.Fatal(err)
-	}
-	defer testutil.Unmount(t, preparing)
-
-	if err := ioutil.WriteFile(filepath.Join(preparing, "foo"), []byte("foo\n"), 0777); err != nil {
-		t.Fatal(err)
-	}
-
-	os.MkdirAll(preparing+"/a/b/c", 0755)
-
-	committed := filepath.Join(sm.root, "committed")
-
-	if err := sm.Commit(committed, preparing); err != nil {
-		t.Fatal(err)
-	}
-
-	if sm.Parent(preparing) != "" {
-		t.Fatalf("parent of new layer should be empty, got sm.Parent(%q) == %q", preparing, sm.Parent(preparing))
-	}
-
-	next := filepath.Join(tmpDir, "nextlayer")
-	if err := os.MkdirAll(next, 0777); err != nil {
-		t.Fatal(err)
-	}
-
-	mounts, err = sm.Prepare(next, committed)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if err := containerd.MountAll(mounts, next); err != nil {
-		t.Fatal(err)
-	}
-	defer testutil.Unmount(t, next)
-
-	if err := ioutil.WriteFile(filepath.Join(next, "bar"), []byte("bar\n"), 0777); err != nil {
-		t.Fatal(err)
-	}
-
-	// also, change content of foo to bar
-	if err := ioutil.WriteFile(filepath.Join(next, "foo"), []byte("bar\n"), 0777); err != nil {
-		t.Fatal(err)
-	}
-
-	os.RemoveAll(next + "/a/b")
-	nextCommitted := filepath.Join(sm.root, "committed-next")
-	if err := sm.Commit(nextCommitted, next); err != nil {
-		t.Fatal(err)
-	}
-
-	if sm.Parent(nextCommitted) != committed {
-		t.Fatalf("parent of new layer should be %q, got sm.Parent(%q) == %q (%#v)", committed, next, sm.Parent(next), sm.parents)
-	}
-}

snapshot/naive/naive_test.go

@@ -7,7 +7,7 @@ import (
 	"testing"
 
 	"github.com/docker/containerd"
-	"github.com/docker/containerd/snapshot/testutil"
+	"github.com/docker/containerd/testutil"
 )
 
 func TestSnapshotNaiveBasic(t *testing.T) {

snapshot/overlay/overlay.go

@@ -1,18 +1,18 @@
 package overlay
 
 import (
-	"crypto/md5"
-	"encoding/hex"
 	"fmt"
+	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strings"
 	"sync"
 
 	"github.com/docker/containerd"
+	digest "github.com/opencontainers/go-digest"
 )
 
-func NewOverlay(root string) (*Overlay, error) {
+func NewDriver(root string) (*Overlay, error) {
 	if err := os.MkdirAll(root, 0700); err != nil {
 		return nil, err
 	}
@@ -35,31 +35,87 @@ type Overlay struct {
 	cache *cache
 }
 
-func (o *Overlay) Prepare(key string, parentName string) ([]containerd.Mount, error) {
+func (o *Overlay) Prepare(key, parent string) ([]containerd.Mount, error) {
-	if err := validKey(key); err != nil {
-		return nil, err
-	}
 	active, err := o.newActiveDir(key)
 	if err != nil {
 		return nil, err
 	}
-	if parentName != "" {
+	if parent != "" {
-		if err := active.setParent(parentName); err != nil {
+		if err := active.setParent(parent); err != nil {
 			return nil, err
 		}
 	}
+	return o.Mounts(key)
+}
+
+func (o *Overlay) View(key, parent string) ([]containerd.Mount, error) {
+	panic("not implemented")
+}
+
+// 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 (o *Overlay) Mounts(key string) ([]containerd.Mount, error) {
+	active := o.getActive(key)
 	return active.mounts(o.cache)
 }
 
 func (o *Overlay) Commit(name, key string) error {
 	active := o.getActive(key)
-	return active.commit(name)
+	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 {
+	panic("not implemented")
+}
+
+// Parent returns the parent of snapshot identified by name.
+func (o *Overlay) Parent(name string) (string, error) {
+	ppath, err := o.cache.get(filepath.Join(o.root, "snapshots", hash(name)))
+	if err != nil {
+		if os.IsNotExist(err) {
+			return "", nil // no parent
+		}
+
+		return "", err
+	}
+
+	p, err := ioutil.ReadFile(filepath.Join(ppath, "name"))
+	if err != nil {
+		return "", err
+	}
+
+	return string(p), nil
+}
+
+// Exists returns true if the snapshot with name exists.
+func (o *Overlay) 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 {
+	panic("not implemented")
+}
+
+// Walk the committed snapshots.
+func (o *Overlay) 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 {
+	panic("not implemented")
 }
 
 func (o *Overlay) newActiveDir(key string) (*activeDir, error) {
 	var (
-		hash = hash(key)
+		path = filepath.Join(o.root, "active", hash(key))
-		path = filepath.Join(o.root, "active", hash)
 	)
 	a := &activeDir{
 		path:         path,
@@ -84,15 +140,8 @@ func (o *Overlay) getActive(key string) *activeDir {
 	}
 }
 
-func validKey(key string) error {
-	_, err := filepath.Abs(key)
-	return err
-}
-
 func hash(k string) string {
-	h := md5.New()
+	return digest.FromString(k).Hex()
-	h.Write([]byte(k))
-	return hex.EncodeToString(h.Sum(nil))
 }
 
 type activeDir struct {
@@ -105,14 +154,26 @@ func (a *activeDir) delete() error {
 }
 
 func (a *activeDir) setParent(name string) error {
-	return os.Symlink(filepath.Join(a.snapshotsDir, name), filepath.Join(a.path, "parent"))
+	return os.Symlink(filepath.Join(a.snapshotsDir, hash(name)), filepath.Join(a.path, "parent"))
 }
 
-func (a *activeDir) commit(name string) error {
+func (a *activeDir) commit(name string, c *cache) error {
+	// TODO(stevvooe): This doesn't quite meet the current model. The new model
+	// is to copy all of this out and let the transaction continue. We don't
+	// really have tests for it yet, but this will be the spot to fix it.
+	//
+	// Nothing should be removed until remove is called on the active
+	// transaction.
 	if err := os.RemoveAll(filepath.Join(a.path, "work")); err != nil {
 		return err
 	}
-	return os.Rename(a.path, filepath.Join(a.snapshotsDir, name))
+
+	if err := ioutil.WriteFile(filepath.Join(a.path, "name"), []byte(name), 0644); err != nil {
+		return err
+	}
+
+	c.invalidate(a.path) // clears parent cache, since we end up moving.
+	return os.Rename(a.path, filepath.Join(a.snapshotsDir, hash(name)))
 }
 
 func (a *activeDir) mounts(c *cache) ([]containerd.Mount, error) {
@@ -182,3 +243,10 @@ func (c *cache) get(path string) (string, error) {
 	}
 	return parentRoot, nil
 }
+
+func (c *cache) invalidate(path string) {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+
+	delete(c.parents, path)
+}

snapshot/overlay/overlay_test.go

@@ -8,7 +8,8 @@ import (
 	"testing"
 
 	"github.com/docker/containerd"
-	"github.com/docker/containerd/snapshot/testutil"
+	"github.com/docker/containerd/snapshot"
+	"github.com/docker/containerd/testutil"
 )
 
 func TestOverlay(t *testing.T) {
@@ -17,7 +18,7 @@ func TestOverlay(t *testing.T) {
 		t.Fatal(err)
 	}
 	defer os.RemoveAll(root)
-	o, err := NewOverlay(root)
+	o, err := NewDriver(root)
 	if err != nil {
 		t.Error(err)
 		return
@@ -52,7 +53,7 @@ func TestOverlayCommit(t *testing.T) {
 		t.Fatal(err)
 	}
 	defer os.RemoveAll(root)
-	o, err := NewOverlay(root)
+	o, err := NewDriver(root)
 	if err != nil {
 		t.Error(err)
 		return
@@ -80,7 +81,7 @@ func TestOverlayOverlayMount(t *testing.T) {
 		t.Fatal(err)
 	}
 	defer os.RemoveAll(root)
-	o, err := NewOverlay(root)
+	o, err := NewDriver(root)
 	if err != nil {
 		t.Error(err)
 		return
@@ -110,10 +111,11 @@ func TestOverlayOverlayMount(t *testing.T) {
 		t.Errorf("expected source %q but received %q", "overlay", m.Source)
 	}
 	var (
-		hash  = hash("/tmp/layer2")
+		ah    = hash("/tmp/layer2")
-		work  = "workdir=" + filepath.Join(root, "active", hash, "work")
+		sh    = hash("base")
-		upper = "upperdir=" + filepath.Join(root, "active", hash, "fs")
+		work  = "workdir=" + filepath.Join(root, "active", ah, "work")
-		lower = "lowerdir=" + filepath.Join(root, "snapshots", "base", "fs")
+		upper = "upperdir=" + filepath.Join(root, "active", ah, "fs")
+		lower = "lowerdir=" + filepath.Join(root, "snapshots", sh, "fs")
 	)
 	for i, v := range []string{
 		work,
@@ -133,7 +135,7 @@ func TestOverlayOverlayRead(t *testing.T) {
 		t.Fatal(err)
 	}
 	defer os.RemoveAll(root)
-	o, err := NewOverlay(root)
+	o, err := NewDriver(root)
 	if err != nil {
 		t.Error(err)
 		return
@@ -177,3 +179,15 @@ func TestOverlayOverlayRead(t *testing.T) {
 		return
 	}
 }
+
+func TestOverlayDriverSuite(t *testing.T) {
+	testutil.RequiresRoot(t)
+	snapshot.DriverSuite(t, "Overlay", func(root string) (snapshot.Driver, func(), error) {
+		driver, err := NewDriver(root)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		return driver, func() {}, nil
+	})
+}

snapshot/testutil/helpers.go

@@ -1,34 +0,0 @@
-package testutil
-
-import (
-	"flag"
-	"os"
-	"syscall"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-)
-
-var rootEnabled bool
-
-func init() {
-	flag.BoolVar(&rootEnabled, "test.root", false, "enable tests that require root")
-}
-
-// 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 := syscall.Unmount(mountPoint, 0); err != nil {
-		t.Error("Could not umount", mountPoint, err)
-	}
-}
-
-// RequiresRoot skips tests that require root, unless the test.root flag has
-// been set
-func RequiresRoot(t *testing.T) {
-	if !rootEnabled {
-		t.Skip("skipping test that requires root")
-		return
-	}
-	assert.Equal(t, 0, os.Getuid(), "This test must be run as root.")
-}

testutil/helpers.go

@@ -0,0 +1,66 @@
+package testutil
+
+import (
+	"flag"
+	"os"
+	"path/filepath"
+	"syscall"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+var rootEnabled bool
+
+func init() {
+	flag.BoolVar(&rootEnabled, "test.root", false, "enable tests that require root")
+}
+
+// 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 := syscall.Unmount(mountPoint, 0); err != nil {
+		t.Error("Could not umount", mountPoint, err)
+	}
+}
+
+// RequiresRoot skips tests that require root, unless the test.root flag has
+// been set
+func RequiresRoot(t *testing.T) {
+	if !rootEnabled {
+		t.Skip("skipping test that requires root")
+		return
+	}
+	assert.Equal(t, 0, os.Getuid(), "This test must be run as root.")
+}
+
+// DumpDir will log out all of the contents of the provided directory to
+// testing logger.
+//
+// Use this in a defer statement within tests that may allocate and exercise a
+// temporary directory. Immensely useful for sanity checking and debugging
+// failing tests.
+//
+// One should still test that contents are as expected. This is only a visual
+// tool to assist when things don't go your way.
+func DumpDir(t *testing.T, root string) {
+	if err := filepath.Walk(root, func(path string, fi os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+
+		if fi.Mode()&os.ModeSymlink != 0 {
+			target, err := os.Readlink(path)
+			if err != nil {
+				return err
+			}
+			t.Log(fi.Mode(), path, "->", target)
+		} else {
+			t.Log(fi.Mode(), path)
+		}
+
+		return nil
+	}); err != nil {
+		t.Fatalf("error dumping directory: %v", err)
+	}
+}