Merge pull request #1833 from dmcgowan/snapshot-gc-3rd-phase

snapshots/overlay: add overlay cleanup

metadata/snapshot.go

@@ -613,14 +613,23 @@ func validateSnapshot(info *snapshots.Info) error {
 	return nil
 }
 
+type cleaner interface {
+	Cleanup(ctx context.Context) error
+}
+
 func (s *snapshotter) garbageCollect(ctx context.Context) (d time.Duration, err error) {
 	s.l.Lock()
 	t1 := time.Now()
 	defer func() {
+		s.l.Unlock()
+		if err == nil {
+			if c, ok := s.Snapshotter.(cleaner); ok {
+				err = c.Cleanup(ctx)
+			}
+		}
 		if err == nil {
 			d = time.Now().Sub(t1)
 		}
-		s.l.Unlock()
 	}()
 
 	seen := map[string]struct{}{}

snapshots/overlay/overlay.go

@@ -44,20 +44,45 @@ func init() {
 		InitFn: func(ic *plugin.InitContext) (interface{}, error) {
 			ic.Meta.Platforms = append(ic.Meta.Platforms, platforms.DefaultSpec())
 			ic.Meta.Exports["root"] = ic.Root
-			return NewSnapshotter(ic.Root)
+			return NewSnapshotter(ic.Root, AsynchronousRemove)
 		},
 	})
 }
 
+// SnapshotterConfig is used to configure the overlay snapshotter instance
+type SnapshotterConfig struct {
+	asyncRemove bool
+}
+
+// Opt is an option to configure the overlay snapshotter
+type Opt func(config *SnapshotterConfig) error
+
+// AsynchronousRemove defers removal of filesystem content until
+// the Cleanup method is called. Removals will make the snapshot
+// referred to by the key unavailable and make the key immediately
+// available for re-use.
+func AsynchronousRemove(config *SnapshotterConfig) error {
+	config.asyncRemove = true
+	return nil
+}
+
 type snapshotter struct {
-	root string
+	root        string
-	ms   *storage.MetaStore
+	ms          *storage.MetaStore
+	asyncRemove bool
 }
 
 // NewSnapshotter returns a Snapshotter which uses overlayfs. The overlayfs
 // diffs are stored under the provided root. A metadata file is stored under
 // the root.
-func NewSnapshotter(root string) (snapshots.Snapshotter, error) {
+func NewSnapshotter(root string, opts ...Opt) (snapshots.Snapshotter, error) {
+	var config SnapshotterConfig
+	for _, opt := range opts {
+		if err := opt(&config); err != nil {
+			return nil, err
+		}
+	}
+
 	if err := os.MkdirAll(root, 0700); err != nil {
 		return nil, err
 	}
@@ -78,8 +103,9 @@ func NewSnapshotter(root string) (snapshots.Snapshotter, error) {
 	}
 
 	return &snapshotter{
-		root: root,
+		root:        root,
-		ms:   ms,
+		ms:          ms,
+		asyncRemove: config.asyncRemove,
 	}, nil
 }
 
@@ -210,47 +236,50 @@ func (o *snapshotter) Commit(ctx context.Context, name, key string, opts ...snap
 	return t.Commit()
 }
 
-// Remove abandons the transaction identified by key. All resources
+// Remove abandons the snapshot identified by key. The snapshot will
-// associated with the key will be removed.
+// immediately become unavailable and unrecoverable. Disk space will
+// be freed up on the next call to `Cleanup`.
 func (o *snapshotter) Remove(ctx context.Context, key string) (err error) {
 	ctx, t, err := o.ms.TransactionContext(ctx, true)
 	if err != nil {
 		return err
 	}
 	defer func() {
-		if err != nil && t != nil {
+		if err != nil {
 			if rerr := t.Rollback(); rerr != nil {
 				log.G(ctx).WithError(rerr).Warn("failed to rollback transaction")
 			}
 		}
 	}()
 
-	id, _, err := storage.Remove(ctx, key)
+	_, _, err = storage.Remove(ctx, key)
 	if err != nil {
 		return errors.Wrap(err, "failed to remove")
 	}
 
-	path := filepath.Join(o.root, "snapshots", id)
+	if !o.asyncRemove {
-	renamed := filepath.Join(o.root, "snapshots", "rm-"+id)
+		var removals []string
-	if err := os.Rename(path, renamed); err != nil {
+		removals, err = o.getCleanupDirectories(ctx, t)
-		return errors.Wrap(err, "failed to rename")
+		if err != nil {
-	}
+			return errors.Wrap(err, "unable to get directories for removal")
-
-	err = t.Commit()
-	t = nil
-	if err != nil {
-		if err1 := os.Rename(renamed, path); err1 != nil {
-			// May cause inconsistent data on disk
-			log.G(ctx).WithError(err1).WithField("path", renamed).Errorf("failed to rename after failed commit")
 		}
-		return errors.Wrap(err, "failed to commit")
+
-	}
+		// Remove directories after the transaction is closed, failures must not
-	if err := os.RemoveAll(renamed); err != nil {
+		// return error since the transaction is committed with the removal
-		// Must be cleaned up, any "rm-*" could be removed if no active transactions
+		// key no longer available.
-		log.G(ctx).WithError(err).WithField("path", renamed).Warnf("failed to remove root filesystem")
+		defer func() {
+			if err == nil {
+				for _, dir := range removals {
+					if err := os.RemoveAll(dir); err != nil {
+						log.G(ctx).WithError(err).WithField("path", dir).Warn("failed to remove directory")
+					}
+				}
+			}
+		}()
+
 	}
 
-	return nil
+	return t.Commit()
 }
 
 // Walk the committed snapshots.
@@ -263,45 +292,92 @@ func (o *snapshotter) Walk(ctx context.Context, fn func(context.Context, snapsho
 	return storage.WalkInfo(ctx, fn)
 }
 
-func (o *snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, key, parent string, opts []snapshots.Opt) ([]mount.Mount, error) {
+// Cleanup cleans up disk resources from removed or abandoned snapshots
-	var (
+func (o *snapshotter) Cleanup(ctx context.Context) error {
-		path        string
+	cleanup, err := o.cleanupDirectories(ctx)
-		snapshotDir = filepath.Join(o.root, "snapshots")
-	)
-
-	td, err := ioutil.TempDir(snapshotDir, "new-")
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to create temp dir")
+		return err
 	}
+
+	for _, dir := range cleanup {
+		if err := os.RemoveAll(dir); err != nil {
+			log.G(ctx).WithError(err).WithField("path", dir).Warn("failed to remove directory")
+		}
+	}
+
+	return nil
+}
+
+func (o *snapshotter) cleanupDirectories(ctx context.Context) ([]string, error) {
+	ctx, t, err := o.ms.TransactionContext(ctx, false)
+	if err != nil {
+		return nil, err
+	}
+
+	defer t.Rollback()
+	return o.getCleanupDirectories(ctx, t)
+}
+
+func (o *snapshotter) getCleanupDirectories(ctx context.Context, t storage.Transactor) ([]string, error) {
+	ids, err := storage.IDMap(ctx)
+	if err != nil {
+		return nil, err
+	}
+
+	snapshotDir := filepath.Join(o.root, "snapshots")
+	fd, err := os.Open(snapshotDir)
+	if err != nil {
+		return nil, err
+	}
+	defer fd.Close()
+
+	dirs, err := fd.Readdirnames(0)
+	if err != nil {
+		return nil, err
+	}
+
+	cleanup := []string{}
+	for _, d := range dirs {
+		if _, ok := ids[d]; ok {
+			continue
+		}
+
+		cleanup = append(cleanup, filepath.Join(snapshotDir, d))
+	}
+
+	return cleanup, nil
+}
+
+func (o *snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, key, parent string, opts []snapshots.Opt) ([]mount.Mount, error) {
+	ctx, t, err := o.ms.TransactionContext(ctx, true)
+	if err != nil {
+		return nil, err
+	}
+
+	var td, path string
 	defer func() {
 		if err != nil {
 			if td != "" {
 				if err1 := os.RemoveAll(td); err1 != nil {
-					err = errors.Wrapf(err, "remove failed: %v", err1)
+					log.G(ctx).WithError(err1).Warn("failed to cleanup temp snapshot directory")
 				}
 			}
 			if path != "" {
 				if err1 := os.RemoveAll(path); err1 != nil {
+					log.G(ctx).WithError(err1).WithField("path", path).Error("failed to reclaim snapshot directory, directory may need removal")
 					err = errors.Wrapf(err, "failed to remove path: %v", err1)
 				}
 			}
 		}
 	}()
 
-	fs := filepath.Join(td, "fs")
+	snapshotDir := filepath.Join(o.root, "snapshots")
-	if err = os.MkdirAll(fs, 0755); err != nil {
+	td, err = o.prepareDirectory(ctx, snapshotDir, kind)
-		return nil, err
-	}
-
-	if kind == snapshots.KindActive {
-		if err = os.MkdirAll(filepath.Join(td, "work"), 0711); err != nil {
-			return nil, err
-		}
-	}
-
-	ctx, t, err := o.ms.TransactionContext(ctx, true)
 	if err != nil {
-		return nil, err
+		if rerr := t.Rollback(); rerr != nil {
+			log.G(ctx).WithError(rerr).Warn("failed to rollback transaction")
+		}
+		return nil, errors.Wrap(err, "failed to create prepare snapshot dir")
 	}
 	rollback := true
 	defer func() {
@@ -324,7 +400,11 @@ func (o *snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k
 		}
 
 		stat := st.Sys().(*syscall.Stat_t)
-		if err := os.Lchown(fs, int(stat.Uid), int(stat.Gid)); err != nil {
+
+		if err := os.Lchown(filepath.Join(td, "fs"), int(stat.Uid), int(stat.Gid)); err != nil {
+			if rerr := t.Rollback(); rerr != nil {
+				log.G(ctx).WithError(rerr).Warn("failed to rollback transaction")
+			}
 			return nil, errors.Wrap(err, "failed to chown")
 		}
 	}
@@ -343,6 +423,25 @@ func (o *snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k
 	return o.mounts(s), nil
 }
 
+func (o *snapshotter) prepareDirectory(ctx context.Context, snapshotDir string, kind snapshots.Kind) (string, error) {
+	td, err := ioutil.TempDir(filepath.Join(o.root, "snapshots"), "new-")
+	if err != nil {
+		return "", errors.Wrap(err, "failed to create temp dir")
+	}
+
+	if err := os.Mkdir(filepath.Join(td, "fs"), 0755); err != nil {
+		return td, err
+	}
+
+	if kind == snapshots.KindActive {
+		if err := os.Mkdir(filepath.Join(td, "work"), 0711); err != nil {
+			return td, err
+		}
+	}
+
+	return td, nil
+}
+
 func (o *snapshotter) mounts(s storage.Snapshot) []mount.Mount {
 	if len(s.ParentIDs) == 0 {
 		// if we only have one layer/no parents then just return a bind mount as overlay

snapshots/storage/bolt.go

@@ -405,6 +405,26 @@ func CommitActive(ctx context.Context, key, name string, usage snapshots.Usage,
 	return fmt.Sprintf("%d", id), nil
 }
 
+// IDMap returns all the IDs mapped to their key
+func IDMap(ctx context.Context) (map[string]string, error) {
+	m := map[string]string{}
+	if err := withBucket(ctx, func(ctx context.Context, bkt, _ *bolt.Bucket) error {
+		return bkt.ForEach(func(k, v []byte) error {
+			// skip non buckets
+			if v != nil {
+				return nil
+			}
+			id := readID(bkt.Bucket(k))
+			m[fmt.Sprintf("%d", id)] = string(k)
+			return nil
+		})
+	}); err != nil {
+		return nil, err
+	}
+
+	return m, nil
+}
+
 func withSnapshotBucket(ctx context.Context, key string, fn func(context.Context, *bolt.Bucket, *bolt.Bucket) error) error {
 	tx, ok := ctx.Value(transactionKey{}).(*bolt.Tx)
 	if !ok {