From ab08944aa7507e7824183896533886c8fbc2adba Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Mon, 6 Feb 2017 19:26:07 -0800 Subject: [PATCH 1/3] snapshot: clarify active and committed snapshots After receiving feedback on the `snapshot.Driver` interface, it was found that the behavior of active and committed snapshots was confusing. We attempt to clean this up by doing the following: 1. Define the concept of "active" and "committed" snapshots and their lifecycle relationship. Active snapshots can be created from a parent. Committed snapshots can only be created from active snapshots. 2. Only committed snapshots can be a parent. 3. Unify the keyspace of snapshots. For common operations, such as removal and stat, we only have a single method that works for both active and committed snapshots. For methods that take one or the other, the restriction is called out. `Remove` and `Delete` are consolidated for this purpose. 4. Define the `Info` data type to include name, parent, kind and readonly state. This allows us to collect `Exists` and `Parent` into a single method `Stat` and simplifies the `Walk` method, eliding `Active`. 5. The `Driver` has been renamed to `Snapshotter` due to the overuse of the term `Driver`. Effectively, we now have snapshots that are either active or committed. Signed-off-by: Stephen J Day --- snapshot/driver.go | 302 ---------------------------------- snapshot/snapshotter.go | 347 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 347 insertions(+), 302 deletions(-) delete mode 100644 snapshot/driver.go create mode 100644 snapshot/snapshotter.go diff --git a/snapshot/driver.go b/snapshot/driver.go deleted file mode 100644 index e195511..0000000 --- a/snapshot/driver.go +++ /dev/null @@ -1,302 +0,0 @@ -package snapshot - -import ( - "io/ioutil" - "os" - "path/filepath" - "testing" - - "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 -// 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) - } - - assert.Equal(t, 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 err != nil { - t.Fatal(err) - } - - assert.Equal(t, parent, committed) -} diff --git a/snapshot/snapshotter.go b/snapshot/snapshotter.go new file mode 100644 index 0000000..b8cadfc --- /dev/null +++ b/snapshot/snapshotter.go @@ -0,0 +1,347 @@ +package snapshot + +import ( + "io/ioutil" + "os" + "path/filepath" + "testing" + + "github.com/docker/containerd" + "github.com/docker/containerd/testutil" + "github.com/stretchr/testify/assert" +) + +// Kind identifies the kind of snapshot. +type Kind int + +// definitions of snapshot kinds +const ( + KindActive Kind = iota + KindCommitted +) + +// Info provides information about a particular snapshot. +type Info struct { + Name string // name or key of snapshot + Parent string // name of parent snapshot + Kind Kind // active or committed snapshot + Readonly bool // true if readonly, only valid for active +} + +// Snapshotter defines the methods required to implement a snapshot snapshotter for +// allocating, snapshotting and mounting filesystem changesets. The model works +// by building up sets of changes with parent-child relationships. +// +// 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. +// +// An active snapshot is created by calling `Prepare`. After mounting, changes +// can be made to the snapshot. The act of commiting creates a committed +// snapshot. The committed snapshot will get the parent of active snapshot. The +// committed snapshot can then be used as a parent. Active snapshots can never +// act as a parent. +// +// Snapshots are best understood by their lifecycle. Active snapshots are +// always created with Prepare or View. Committed snapshots are always created +// with Commit. Active snapshots never become committed snapshots and vice +// versa. All snapshots may be removed. +// +// For consistency, we define the following terms to be used throughout this +// interface for snapshotter implementations: +// +// `key` - refers to an active snapshot +// `name` - refers to a committed snapshot +// `parent` - refers to the parent in relation +// +// Most methods take various combinations of these identifiers. Typically, +// `name` and `parent` will be used in cases where a method *only* takes +// committed snapshots. `key` will be used to refer to active snapshots in most +// cases, except where noted. All variables used to access snapshots use the +// same key space. For example, an active snapshot may not share the same key +// with a committed snapshot. +// +// We cover several examples below to demonstrate the utility of a snapshot +// snapshotter. +// +// 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, tmpDir := getLayerPath(), mkTmpDir() // just a path to layer tar file. +// +// We start by using a Snapshotter to Prepare a new snapshot transaction, using a +// key and descending from the empty parent "": +// +// mounts, err := snapshotter.Prepare(key, "") +// if err != nil { ... } +// +// We get back a list of mounts from Snapshotter.Prepare, with the key identifying +// the active snapshot. Mount this to the temporary location with the +// following: +// +// if err := MountAll(mounts, tmpDir); 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 commit the active +// snapshot to a name. For this example, we are just going to use the layer +// digest, but in practice, this will probably be the ChainID: +// +// if err := snapshotter.Commit(digest.String(), key); err != nil { ... } +// +// Now, we have a layer in the Snapshotter that can be accessed with the digest +// provided during commit. Once you have committed the snapshot, the active +// snapshot can be removed with the following: +// +// snapshotter.Remove(key) +// +// 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 parent when calling +// Manager.Prepare, assuming a clean tmpLocation: +// +// mounts, err := sm.Prepare(tmpLocation, parentDigest) +// +// We then mount, apply and commit, as we did above. The new snapshot will be +// based on the content of the previous one. +// +// Running a Container +// +// To run a container, we simply provide Snapshotter.Prepare the committed image +// snapshot as the parent. After mounting, the prepared path can +// be used directly as the container's filesystem: +// +// mounts, err := sm.Prepare(containerKey, imageRootFSChainID) +// +// 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(newImageSnapshot, containerKey); err != nil { ... } +// +// Alternatively, for most container runs, Manager.Remove will be called to +// signal the Snapshotter to abandon the changes. +type Snapshotter interface { + // Stat returns the info for an active or committed snapshot by name or + // key. + // + // Should be used for parent resolution, existence checks and to discern + // the kind of snapshot. + Stat(key string) (Info, error) + + // Mounts returns the mounts for the active snapshot transaction identified + // by key. Can be called on an read-write or readonly transaction. This is + // available only for active snapshots. + // + // This can be used to recover mounts after calling View or Prepare. + Mounts(key string) ([]containerd.Mount, error) + + // Prepare creates an active snapshot identified by key descending from the + // provided parent. The returned mounts can be used to mount the snapshot + // to capture changes. + // + // If a parent is provided, after performing the mounts, the destination + // will start with the content of the parent. The parent must be a + // committed snapshot. Changes to the mounted destination will be captured + // in relation to the parent. The default parent, "", is an empty + // directory. + // + // The changes may be saved to a committed 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 snapshotter. View returns a readonly view on + // the parent, with the active snapshot being 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. Implementations may perform this in a more + // efficient manner that differs from what would be attempted with + // `Prepare`. + // + // Commit may not be called on the provided key and will return an error. + // 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 snapshotter's other + // methods to create subsequent snapshots. + // + // A committed snapshot will be created under name with the parent of the + // active snapshot. + // + // 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 + + // Remove the committed or active snapshot by the provided key. + // + // All resources associated with the key will be removed. + // + // If the snapshot is a parent of another snapshot, its children must be + // removed before proceeding. + Remove(key string) error + + // Walk the committed snapshots. For each snapshot in the snapshotter, the + // function will be called. + Walk(fn func(Info) error) error +} + +// SnapshotterSuite runs a test suite on the snapshotter given a factory function. +func SnapshotterSuite(t *testing.T, name string, snapshotterFn func(root string) (Snapshotter, func(), error)) { + t.Run("Basic", makeTest(t, name, snapshotterFn, checkSnapshotterBasic)) +} + +func makeTest(t *testing.T, name string, snapshotterFn func(root string) (Snapshotter, func(), error), fn func(t *testing.T, snapshotter Snapshotter, work string)) func(t *testing.T) { + return func(t *testing.T) { + // Make two directories: a snapshotter root and a play area for the tests: + // + // /tmp + // work/ -> passed to test functions + // root/ -> passed to snapshotter + // + 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) + } + + snapshotter, cleanup, err := snapshotterFn(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, snapshotter, work) + } +} + +// checkSnapshotterBasic tests the basic workflow of a snapshot snapshotter. +func checkSnapshotterBasic(t *testing.T, snapshotter Snapshotter, work string) { + preparing := filepath.Join(work, "preparing") + if err := os.MkdirAll(preparing, 0777); err != nil { + t.Fatal(err) + } + + mounts, err := snapshotter.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 := snapshotter.Commit(committed, preparing); err != nil { + t.Fatal(err) + } + + si, err := snapshotter.Stat(committed) + if err != nil { + t.Fatal(err) + } + + assert.Equal(t, si.Parent, "") + + next := filepath.Join(work, "nextlayer") + if err := os.MkdirAll(next, 0777); err != nil { + t.Fatal(err) + } + + mounts, err = snapshotter.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 := snapshotter.Commit(nextCommitted, next); err != nil { + t.Fatal(err) + } + + si2, err := snapshotter.Stat(nextCommitted) + if err != nil { + t.Fatal(err) + } + + assert.Equal(t, si2.Parent, committed) + + expected := map[string]Info{ + si.Name: si, + si2.Name: si2, + } + walked := map[string]Info{} // walk is not ordered + assert.NoError(t, snapshotter.Walk(func(si Info) error { + walked[si.Name] = si + return nil + })) + + assert.Equal(t, expected, walked) +} From e6c1bb0ff2b71713f7ab8cfb4291de5357e362c9 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Tue, 7 Feb 2017 15:51:37 -0800 Subject: [PATCH 2/3] snapshot/overlay: port overlay driver to Snapshotter With the change to the snapshotter interface, we've now updated the overlay driver to follow the conventions of the current test suite. To support key unification, an hashed index was added to active and committed directories. We still need to do some testing around collisions, but we'll leave that for a future PR. Signed-off-by: Stephen J Day --- snapshot/overlay/overlay.go | 232 +++++++++++++++++++++---------- snapshot/overlay/overlay_test.go | 34 ++--- 2 files changed, 179 insertions(+), 87 deletions(-) diff --git a/snapshot/overlay/overlay.go b/snapshot/overlay/overlay.go index f065777..60735f7 100644 --- a/snapshot/overlay/overlay.go +++ b/snapshot/overlay/overlay.go @@ -9,33 +9,98 @@ import ( "sync" "github.com/docker/containerd" + "github.com/docker/containerd/snapshot" digest "github.com/opencontainers/go-digest" + "github.com/pkg/errors" ) -type Driver struct { +type Snapshotter struct { root string - cache *cache + links *cache } -func NewDriver(root string) (*Driver, error) { +func NewSnapshotter(root string) (*Snapshotter, error) { if err := os.MkdirAll(root, 0700); err != nil { return nil, err } for _, p := range []string{ - "snapshots", - "active", + "committed", // committed snapshots + "active", // active snapshots + "index", // snapshots by hashed name } { if err := os.MkdirAll(filepath.Join(root, p), 0700); err != nil { return nil, err } } - return &Driver{ + return &Snapshotter{ root: root, - cache: newCache(), + links: newCache(), }, nil } -func (o *Driver) Prepare(key, parent string) ([]containerd.Mount, error) { +// Stat returns the info for an active or committed snapshot by name or +// key. +// +// Should be used for parent resolution, existence checks and to discern +// the kind of snapshot. +func (o *Snapshotter) Stat(key string) (snapshot.Info, error) { + path, err := o.links.get(filepath.Join(o.root, "index", hash(key))) + if err != nil { + if !os.IsNotExist(err) { + return snapshot.Info{}, err + } + + return snapshot.Info{}, errors.Errorf("snapshot %v not found", key) + } + + // TODO(stevvooe): We don't confirm the name to avoid the lookup cost. + return o.stat(path) +} + +func (o *Snapshotter) stat(path string) (snapshot.Info, error) { + ppath, err := o.links.get(filepath.Join(path, "parent")) + if err != nil { + if !os.IsNotExist(err) { + return snapshot.Info{}, err + } + + // no parent + } + + kp, err := ioutil.ReadFile(filepath.Join(path, "name")) + if err != nil { + return snapshot.Info{}, err + } + + var parent string + if ppath != "" { + p, err := ioutil.ReadFile(filepath.Join(ppath, "name")) + if err != nil { + return snapshot.Info{}, err + } + parent = string(p) + } + + ro := true + kind := snapshot.KindCommitted + if strings.HasPrefix(path, filepath.Join(o.root, "active")) { + // TODO(stevvooe): Maybe there is a better way? + kind = snapshot.KindActive + + // TODO(stevvooe): We haven't introduced this to overlay yet. + // We'll add it when we add tests for it. + ro = false + } + + return snapshot.Info{ + Name: string(kp), + Parent: parent, + Kind: kind, + Readonly: ro, + }, nil +} + +func (o *Snapshotter) Prepare(key, parent string) ([]containerd.Mount, error) { active, err := o.newActiveDir(key) if err != nil { return nil, err @@ -45,10 +110,10 @@ func (o *Driver) Prepare(key, parent string) ([]containerd.Mount, error) { return nil, err } } - return o.Mounts(key) + return active.mounts(o.links) } -func (o *Driver) View(key, parent string) ([]containerd.Mount, error) { +func (o *Snapshotter) View(key, parent string) ([]containerd.Mount, error) { panic("not implemented") } @@ -56,70 +121,69 @@ func (o *Driver) 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 *Driver) Mounts(key string) ([]containerd.Mount, error) { +func (o *Snapshotter) Mounts(key string) ([]containerd.Mount, error) { active := o.getActive(key) - return active.mounts(o.cache) + return active.mounts(o.links) } -func (o *Driver) Commit(name, key string) error { +func (o *Snapshotter) Commit(name, key string) error { active := o.getActive(key) - return active.commit(name, o.cache) + return active.commit(name, o.links) } // Remove abandons the transaction identified by key. All resources // associated with the key will be removed. -func (o *Driver) Remove(key string) error { - panic("not implemented") -} - -// Parent returns the parent of snapshot identified by name. -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) { - 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 *Driver) Exists(name string) bool { - panic("not implemented") -} - -// Delete the snapshot idenfitied by name. -// -// If name has children, the operation will fail. -func (o *Driver) Delete(name string) error { +func (o *Snapshotter) Remove(key string) error { panic("not implemented") } // Walk the committed snapshots. -func (o *Driver) Walk(fn func(name string) error) error { - panic("not implemented") +func (o *Snapshotter) Walk(fn func(snapshot.Info) error) error { + root := filepath.Join(o.root, "index") + return filepath.Walk(root, func(path string, fi os.FileInfo, err error) error { + if err != nil { + return err + } + + if path == root { + return nil + } + + if fi.Mode()&os.ModeSymlink == 0 { + // only follow links + return filepath.SkipDir + } + + target, err := o.links.get(path) + if err != nil { + if !os.IsNotExist(err) { + return err + } + } + + si, err := o.stat(target) + if err != nil { + return err + } + + if err := fn(si); err != nil { + return err + } + + return nil + }) } -// Active will call fn for each active transaction. -func (o *Driver) Active(fn func(key string) error) error { - panic("not implemented") -} - -func (o *Driver) newActiveDir(key string) (*activeDir, error) { +func (o *Snapshotter) newActiveDir(key string) (*activeDir, error) { var ( - path = filepath.Join(o.root, "active", hash(key)) + path = filepath.Join(o.root, "active", hash(key)) + name = filepath.Join(path, "name") + indexlink = filepath.Join(o.root, "index", hash(key)) ) a := &activeDir{ path: path, - snapshotsDir: filepath.Join(o.root, "snapshots"), + committedDir: filepath.Join(o.root, "committed"), + indexlink: indexlink, } for _, p := range []string{ "work", @@ -130,13 +194,26 @@ func (o *Driver) newActiveDir(key string) (*activeDir, error) { return nil, err } } + + if err := ioutil.WriteFile(name, []byte(key), 0644); err != nil { + a.delete() + return nil, err + } + + // link from namespace + if err := os.Symlink(path, indexlink); err != nil { + a.delete() + return nil, err + } + return a, nil } -func (o *Driver) getActive(key string) *activeDir { +func (o *Snapshotter) getActive(key string) *activeDir { return &activeDir{ path: filepath.Join(o.root, "active", hash(key)), - snapshotsDir: filepath.Join(o.root, "snapshots"), + committedDir: filepath.Join(o.root, "committed"), + indexlink: filepath.Join(o.root, "index", hash(key)), } } @@ -145,8 +222,9 @@ func hash(k string) string { } type activeDir struct { - snapshotsDir string + committedDir string path string + indexlink string } func (a *activeDir) delete() error { @@ -154,7 +232,7 @@ func (a *activeDir) delete() error { } func (a *activeDir) setParent(name string) error { - return os.Symlink(filepath.Join(a.snapshotsDir, hash(name)), filepath.Join(a.path, "parent")) + return os.Symlink(filepath.Join(a.committedDir, hash(name)), filepath.Join(a.path, "parent")) } func (a *activeDir) commit(name string, c *cache) error { @@ -173,7 +251,20 @@ func (a *activeDir) commit(name string, c *cache) error { } c.invalidate(a.path) // clears parent cache, since we end up moving. - return os.Rename(a.path, filepath.Join(a.snapshotsDir, hash(name))) + c.invalidate(filepath.Join(a.path, "parent")) + c.invalidate(a.indexlink) + + committed := filepath.Join(a.committedDir, hash(name)) + if err := os.Rename(a.path, committed); err != nil { + return err + } + + if err := os.Remove(a.indexlink); err != nil { + return err + } + + indexlink := filepath.Join(filepath.Dir(a.indexlink), hash(name)) + return os.Symlink(committed, indexlink) } func (a *activeDir) mounts(c *cache) ([]containerd.Mount, error) { @@ -183,10 +274,11 @@ func (a *activeDir) mounts(c *cache) ([]containerd.Mount, error) { current = a.path ) for { - if current, err = c.get(current); err != nil { + if current, err = c.get(filepath.Join(current, "parent")); err != nil { if os.IsNotExist(err) { break } + return nil, err } parents = append(parents, filepath.Join(current, "fs")) @@ -221,32 +313,32 @@ func (a *activeDir) mounts(c *cache) ([]containerd.Mount, error) { func newCache() *cache { return &cache{ - parents: make(map[string]string), + links: make(map[string]string), } } type cache struct { - mu sync.Mutex - parents map[string]string + mu sync.Mutex + links map[string]string } func (c *cache) get(path string) (string, error) { c.mu.Lock() defer c.mu.Unlock() - parentRoot, ok := c.parents[path] + target, ok := c.links[path] if !ok { - link, err := os.Readlink(filepath.Join(path, "parent")) + link, err := os.Readlink(path) if err != nil { return "", err } - c.parents[path], parentRoot = link, link + c.links[path], target = link, link } - return parentRoot, nil + return target, nil } func (c *cache) invalidate(path string) { c.mu.Lock() defer c.mu.Unlock() - delete(c.parents, path) + delete(c.links, path) } diff --git a/snapshot/overlay/overlay_test.go b/snapshot/overlay/overlay_test.go index d27f981..b07fbf8 100644 --- a/snapshot/overlay/overlay_test.go +++ b/snapshot/overlay/overlay_test.go @@ -13,12 +13,24 @@ import ( ) func TestOverlay(t *testing.T) { + testutil.RequiresRoot(t) + snapshot.SnapshotterSuite(t, "Overlay", func(root string) (snapshot.Snapshotter, func(), error) { + snapshotter, err := NewSnapshotter(root) + if err != nil { + t.Fatal(err) + } + + return snapshotter, func() {}, nil + }) +} + +func TestOverlayMounts(t *testing.T) { root, err := ioutil.TempDir("", "overlay") if err != nil { t.Fatal(err) } defer os.RemoveAll(root) - o, err := NewDriver(root) + o, err := NewSnapshotter(root) if err != nil { t.Error(err) return @@ -53,7 +65,7 @@ func TestOverlayCommit(t *testing.T) { t.Fatal(err) } defer os.RemoveAll(root) - o, err := NewDriver(root) + o, err := NewSnapshotter(root) if err != nil { t.Error(err) return @@ -81,7 +93,7 @@ func TestOverlayOverlayMount(t *testing.T) { t.Fatal(err) } defer os.RemoveAll(root) - o, err := NewDriver(root) + o, err := NewSnapshotter(root) if err != nil { t.Error(err) return @@ -115,7 +127,7 @@ func TestOverlayOverlayMount(t *testing.T) { 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") + lower = "lowerdir=" + filepath.Join(root, "committed", sh, "fs") ) for i, v := range []string{ work, @@ -135,7 +147,7 @@ func TestOverlayOverlayRead(t *testing.T) { t.Fatal(err) } defer os.RemoveAll(root) - o, err := NewDriver(root) + o, err := NewSnapshotter(root) if err != nil { t.Error(err) return @@ -179,15 +191,3 @@ 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 - }) -} From 9c4b235954fc1d42f02943b534360510f58b254c Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Tue, 7 Feb 2017 17:21:19 -0800 Subject: [PATCH 3/3] snapshot/btrfs: update btrfs to snappshotter Updates the btrfs snapshotter to meet the interface and current tests. Mostly, we merge the keyspace into a common index. Like with the overlay driver, we will still need to do more verification work to ensure idempotence of key collisions. Signed-off-by: Stephen J Day --- snapshot/btrfs/btrfs.go | 271 ++++++++++++++++++++++------------- snapshot/btrfs/btrfs_test.go | 8 +- 2 files changed, 177 insertions(+), 102 deletions(-) diff --git a/snapshot/btrfs/btrfs.go b/snapshot/btrfs/btrfs.go index f3af142..032a71c 100644 --- a/snapshot/btrfs/btrfs.go +++ b/snapshot/btrfs/btrfs.go @@ -6,47 +6,33 @@ import ( "io/ioutil" "os" "path/filepath" + "strings" "github.com/docker/containerd" - "github.com/docker/containerd/log" + "github.com/docker/containerd/snapshot" "github.com/pkg/errors" "github.com/stevvooe/go-btrfs" ) -type Driver struct { +type Snapshotter struct { device string // maybe we can resolve it with path? root string // root provides paths for internal storage. } -func NewDriver(device, root string) (*Driver, error) { - return &Driver{device: device, root: root}, nil -} - -func (b *Driver) Prepare(key, parent string) ([]containerd.Mount, error) { - return b.makeActive(key, parent, false) -} - -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) { +func NewSnapshotter(device, root string) (*Snapshotter, 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") - keyh = hash(key) - parenth = hash(parent) - dir = filepath.Join(active, keyh) - namep = filepath.Join(names, keyh) - parentlink = filepath.Join(parents, keyh) - parentp = filepath.Join(snapshots, parenth) + active = filepath.Join(root, "active") + snapshots = filepath.Join(root, "snapshots") + parents = filepath.Join(root, "parents") + index = filepath.Join(root, "index") + names = filepath.Join(root, "names") ) for _, path := range []string{ active, + snapshots, parents, + index, names, } { if err := os.MkdirAll(path, 0755); err != nil { @@ -54,15 +40,118 @@ func (b *Driver) makeActive(key, parent string, readonly bool) ([]containerd.Mou } } + return &Snapshotter{device: device, root: root}, nil +} + +// Stat returns the info for an active or committed snapshot by name or +// key. +// +// Should be used for parent resolution, existence checks and to discern +// the kind of snapshot. +func (b *Snapshotter) Stat(key string) (snapshot.Info, error) { + // resolve the snapshot out of the index. + target, err := os.Readlink(filepath.Join(b.root, "index", hash(key))) + if err != nil { + if !os.IsNotExist(err) { + return snapshot.Info{}, err + } + + return snapshot.Info{}, errors.Errorf("snapshot %v not found", key) + } + + return b.stat(target) +} + +func (b *Snapshotter) stat(target string) (snapshot.Info, error) { + var ( + parents = filepath.Join(b.root, "parents") + names = filepath.Join(b.root, "names") + namep = filepath.Join(names, filepath.Base(target)) + parentlink = filepath.Join(parents, filepath.Base(target)) + ) + + // grab information about the subvolume + info, err := btrfs.SubvolInfo(target) + if err != nil { + return snapshot.Info{}, err + } + + // read the name out of the names! + nameraw, err := ioutil.ReadFile(namep) + if err != nil { + return snapshot.Info{}, err + } + + // resolve the parents path. + parentp, err := os.Readlink(parentlink) + if err != nil { + if !os.IsNotExist(err) { + return snapshot.Info{}, err + } + + // no parent! + } + + var parent string + if parentp != "" { + // 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 snapshot.Info{}, err + } + + parent = string(p) + } + + kind := snapshot.KindCommitted + if strings.HasPrefix(target, filepath.Join(b.root, "active")) { + kind = snapshot.KindActive + } + + return snapshot.Info{ + Name: string(nameraw), + Parent: parent, + Readonly: info.Readonly, + Kind: kind, + }, nil + +} + +func (b *Snapshotter) Prepare(key, parent string) ([]containerd.Mount, error) { + return b.makeActive(key, parent, false) +} + +func (b *Snapshotter) View(key, parent string) ([]containerd.Mount, error) { + return b.makeActive(key, parent, true) +} + +func (b *Snapshotter) makeActive(key, parent string, readonly bool) ([]containerd.Mount, error) { + var ( + active = filepath.Join(b.root, "active") + snapshots = filepath.Join(b.root, "snapshots") + parents = filepath.Join(b.root, "parents") + index = filepath.Join(b.root, "index") + names = filepath.Join(b.root, "names") + keyh = hash(key) + parenth = hash(parent) + target = filepath.Join(active, keyh) + namep = filepath.Join(names, keyh) + indexlink = filepath.Join(index, keyh) + parentlink = filepath.Join(parents, keyh) + parentp = filepath.Join(snapshots, parenth) // parent must be restricted to snaps + ) + if parent == "" { // create new subvolume // btrfs subvolume create /dir - if err := btrfs.SubvolCreate(dir); err != nil { + if err := btrfs.SubvolCreate(target); err != nil { return nil, err } } else { // btrfs subvolume snapshot /parent /subvol - if err := btrfs.SubvolSnapshot(dir, parentp, readonly); err != nil { + if err := btrfs.SubvolSnapshot(target, parentp, readonly); err != nil { return nil, err } @@ -71,14 +160,19 @@ func (b *Driver) makeActive(key, parent string, readonly bool) ([]containerd.Mou } } + // write in the name if err := ioutil.WriteFile(namep, []byte(key), 0644); err != nil { return nil, err } - return b.mounts(dir) + if err := os.Symlink(target, indexlink); err != nil { + return nil, err + } + + return b.mounts(target) } -func (b *Driver) mounts(dir string) ([]containerd.Mount, error) { +func (b *Snapshotter) mounts(dir string) ([]containerd.Mount, error) { var options []string // get the subvolume id back out for the mount @@ -104,12 +198,13 @@ func (b *Driver) mounts(dir string) ([]containerd.Mount, error) { }, nil } -func (b *Driver) Commit(name, key string) error { +func (b *Snapshotter) Commit(name, key string) error { var ( active = filepath.Join(b.root, "active") snapshots = filepath.Join(b.root, "snapshots") - names = filepath.Join(b.root, "names") + index = filepath.Join(b.root, "index") parents = filepath.Join(b.root, "parents") + names = filepath.Join(b.root, "names") keyh = hash(key) nameh = hash(name) dir = filepath.Join(active, keyh) @@ -118,6 +213,8 @@ func (b *Driver) Commit(name, key string) error { namep = filepath.Join(names, nameh) keyparentlink = filepath.Join(parents, keyh) parentlink = filepath.Join(parents, nameh) + keyindexlink = filepath.Join(index, keyh) + indexlink = filepath.Join(index, nameh) ) info, err := btrfs.SubvolInfo(dir) @@ -143,9 +240,7 @@ func (b *Driver) Commit(name, key string) error { return err } - fmt.Println("commit snapshot", target) if err := btrfs.SubvolSnapshot(target, dir, true); err != nil { - fmt.Println("snapshot error") return err } @@ -154,6 +249,10 @@ func (b *Driver) Commit(name, key string) error { return err } + if err := os.Remove(keyindexlink); err != nil { + return err + } + if err := ioutil.WriteFile(namep, []byte(name), 0755); err != nil { return err } @@ -169,6 +268,10 @@ func (b *Driver) Commit(name, key string) error { // into common storage, so let's not fret over it for now. } + if err := os.Symlink(target, indexlink); err != nil { + return err + } + if err := btrfs.SubvolDelete(dir); err != nil { return errors.Wrapf(err, "delete subvol failed on %v", dir) } @@ -180,82 +283,54 @@ func (b *Driver) Commit(name, key string) error { // 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) { +func (b *Snapshotter) 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 { +func (b *Snapshotter) 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") -} +func (b *Snapshotter) Walk(fn func(snapshot.Info) error) error { + // TODO(stevvooe): Copy-pasted almost verbatim from overlay. Really need to + // unify the metadata for snapshot implementations. + root := filepath.Join(b.root, "index") + return filepath.Walk(root, func(path string, fi os.FileInfo, err error) error { + if err != nil { + return err + } -// Active will call fn for each active transaction. -func (b *Driver) Active(fn func(key string) error) error { - panic("not implemented") + if path == root { + return nil + } + + if fi.Mode()&os.ModeSymlink == 0 { + // only follow links + return filepath.SkipDir + } + + target, err := os.Readlink(path) + if err != nil { + if !os.IsNotExist(err) { + return err + } + } + + si, err := b.stat(target) + if err != nil { + return err + } + + if err := fn(si); err != nil { + return err + } + + return nil + }) } func hash(k string) string { diff --git a/snapshot/btrfs/btrfs_test.go b/snapshot/btrfs/btrfs_test.go index 8dafb13..97c8182 100644 --- a/snapshot/btrfs/btrfs_test.go +++ b/snapshot/btrfs/btrfs_test.go @@ -20,14 +20,14 @@ const ( func TestBtrfs(t *testing.T) { testutil.RequiresRoot(t) - snapshot.DriverSuite(t, "Btrfs", func(root string) (snapshot.Driver, func(), error) { + snapshot.SnapshotterSuite(t, "Btrfs", func(root string) (snapshot.Snapshotter, func(), error) { device := setupBtrfsLoopbackDevice(t, root) - driver, err := NewDriver(device.deviceName, root) + snapshotter, err := NewSnapshotter(device.deviceName, root) if err != nil { t.Fatal(err) } - return driver, func() { + return snapshotter, func() { device.remove(t) }, nil }) @@ -53,7 +53,7 @@ func TestBtrfsMounts(t *testing.T) { defer os.RemoveAll(root) target := filepath.Join(root, "test") - b, err := NewDriver(device.deviceName, root) + b, err := NewSnapshotter(device.deviceName, root) if err != nil { t.Fatal(err) }