From 68fd25221df88cf2bc95cd25bc8017580105453a Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Tue, 31 Jan 2017 18:04:13 -0800 Subject: [PATCH 1/2] snapshot: define the snapshot driver interface We now define the `snapshot.Driver` interface based on earlier work. Many details of the model are worked out, such as snapshot lifecycle and parentage of commits against "Active" snapshots. The impetus of this change is to provide a snapshot POC that does a complete push/pull workflow. The beginnings of a test suite for snapshot drivers is included that we can use to verify the assumptions of drivers. The intent is to port the existing tests over to this test suite and start scaling contributions and test to the snapshot driver subsystem. There are still some details that need to be worked out, such as listing and metadata access. We can do this activity as we further integrate with tooling. Signed-off-by: Stephen J Day --- content/content_test.go | 14 +- mount.go | 10 + snapshot/btrfs/btrfs_test.go | 2 +- snapshot/driver.go | 301 ++++++++++++++++++++ snapshot/manager.go | 308 --------------------- snapshot/manager_test.go | 104 ------- snapshot/naive/naive_test.go | 2 +- snapshot/overlay/overlay.go | 2 - {snapshot/testutil => testutil}/helpers.go | 23 ++ 9 files changed, 338 insertions(+), 428 deletions(-) create mode 100644 snapshot/driver.go delete mode 100644 snapshot/manager.go delete mode 100644 snapshot/manager_test.go rename {snapshot/testutil => testutil}/helpers.go (52%) diff --git a/content/content_test.go b/content/content_test.go index c086874..a469630 100644 --- a/content/content_test.go +++ b/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 - }) -} diff --git a/mount.go b/mount.go index b2e837e..29122ff 100644 --- a/mount.go +++ b/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) } diff --git a/snapshot/btrfs/btrfs_test.go b/snapshot/btrfs/btrfs_test.go index 651e9e3..6f15efc 100644 --- a/snapshot/btrfs/btrfs_test.go +++ b/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" ) diff --git a/snapshot/driver.go b/snapshot/driver.go new file mode 100644 index 0000000..7d3fde7 --- /dev/null +++ b/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) + } +} diff --git a/snapshot/manager.go b/snapshot/manager.go deleted file mode 100644 index a936dbc..0000000 --- a/snapshot/manager.go +++ /dev/null @@ -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") -} diff --git a/snapshot/manager_test.go b/snapshot/manager_test.go deleted file mode 100644 index 3fe2c65..0000000 --- a/snapshot/manager_test.go +++ /dev/null @@ -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) - } -} diff --git a/snapshot/naive/naive_test.go b/snapshot/naive/naive_test.go index afae5b9..144fcf1 100644 --- a/snapshot/naive/naive_test.go +++ b/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) { diff --git a/snapshot/overlay/overlay.go b/snapshot/overlay/overlay.go index 1246851..103d759 100644 --- a/snapshot/overlay/overlay.go +++ b/snapshot/overlay/overlay.go @@ -1,8 +1,6 @@ package overlay import ( - "crypto/md5" - "encoding/hex" "fmt" "os" "path/filepath" diff --git a/snapshot/testutil/helpers.go b/testutil/helpers.go similarity index 52% rename from snapshot/testutil/helpers.go rename to testutil/helpers.go index 49ed165..5246318 100644 --- a/snapshot/testutil/helpers.go +++ b/testutil/helpers.go @@ -3,6 +3,7 @@ package testutil import ( "flag" "os" + "path/filepath" "syscall" "testing" @@ -32,3 +33,25 @@ func RequiresRoot(t *testing.T) { } 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 + } + + t.Log(fi.Mode(), path) + return nil + }); err != nil { + t.Fatalf("error dumping directory: %v", err) + } +} From d0b4ce8d17c1903810a9317c124b708cd5dcc6db Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Thu, 2 Feb 2017 18:55:53 -0800 Subject: [PATCH 2/2] snapshot/overlay: add snapshot test suite to overlay Signed-off-by: Stephen J Day --- snapshot/overlay/overlay.go | 112 +++++++++++++++++++++++++------ snapshot/overlay/overlay_test.go | 32 ++++++--- testutil/helpers.go | 11 ++- 3 files changed, 124 insertions(+), 31 deletions(-) diff --git a/snapshot/overlay/overlay.go b/snapshot/overlay/overlay.go index 103d759..afcf67d 100644 --- a/snapshot/overlay/overlay.go +++ b/snapshot/overlay/overlay.go @@ -2,15 +2,17 @@ package overlay import ( "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 } @@ -33,31 +35,87 @@ type Overlay struct { cache *cache } -func (o *Overlay) Prepare(key string, parentName string) ([]containerd.Mount, error) { - if err := validKey(key); err != nil { - return nil, err - } +func (o *Overlay) Prepare(key, parent string) ([]containerd.Mount, error) { active, err := o.newActiveDir(key) if err != nil { return nil, err } - if parentName != "" { - if err := active.setParent(parentName); err != nil { + if parent != "" { + 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) + path = filepath.Join(o.root, "active", hash(key)) ) a := &activeDir{ path: path, @@ -82,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() - h.Write([]byte(k)) - return hex.EncodeToString(h.Sum(nil)) + return digest.FromString(k).Hex() } type activeDir struct { @@ -103,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) { @@ -180,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) +} diff --git a/snapshot/overlay/overlay_test.go b/snapshot/overlay/overlay_test.go index 4b0697f..d27f981 100644 --- a/snapshot/overlay/overlay_test.go +++ b/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") - work = "workdir=" + filepath.Join(root, "active", hash, "work") - upper = "upperdir=" + filepath.Join(root, "active", hash, "fs") - lower = "lowerdir=" + filepath.Join(root, "snapshots", "base", "fs") + ah = hash("/tmp/layer2") + sh = hash("base") + work = "workdir=" + filepath.Join(root, "active", ah, "work") + 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 + }) +} diff --git a/testutil/helpers.go b/testutil/helpers.go index 5246318..9bf543a 100644 --- a/testutil/helpers.go +++ b/testutil/helpers.go @@ -49,7 +49,16 @@ func DumpDir(t *testing.T, root string) { return err } - t.Log(fi.Mode(), path) + 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)