Merge pull request #1728 from AkihiroSuda/snapshot-closable

snapshot: add Close()

metadata/snapshot.go

@@ -724,3 +724,8 @@ func (s *snapshotter) pruneBranch(ctx context.Context, node *treeNode) error {
 
 	return nil
 }
+
+// Close closes s.Snapshotter but not db
+func (s *snapshotter) Close() error {
+	return s.Snapshotter.Close()
+}

metadata/snapshot_test.go

@@ -31,6 +31,9 @@ func newTestSnapshotter(ctx context.Context, root string) (snapshot.Snapshotter,
 	sn := NewDB(db, nil, map[string]snapshot.Snapshotter{"naive": snapshotter}).Snapshotter("naive")
 
 	return sn, func() error {
+		if err := sn.Close(); err != nil {
+			return err
+		}
 		return db.Close()
 	}, nil
 }

services/snapshot/client.go

@@ -163,6 +163,10 @@ func (r *remoteSnapshotter) Walk(ctx context.Context, fn func(context.Context, s
 	}
 }
 
+func (r *remoteSnapshotter) Close() error {
+	return nil
+}
+
 func toKind(kind snapshotapi.Kind) snapshot.Kind {
 	if kind == snapshotapi.KindActive {
 		return snapshot.KindActive

snapshot/btrfs/btrfs.go

@@ -370,3 +370,8 @@ func (b *snapshotter) Remove(ctx context.Context, key string) (err error) {
 
 	return nil
 }
+
+// Close closes the snapshotter
+func (b *snapshotter) Close() error {
+	return b.ms.Close()
+}

snapshot/btrfs/btrfs_test.go

@@ -47,6 +47,9 @@ func boltSnapshotter(t *testing.T) func(context.Context, string) (snapshot.Snaps
 		}
 
 		return snapshotter, func() error {
+			if err := snapshotter.Close(); err != nil {
+				return err
+			}
 			err := mount.UnmountAll(root, unix.MNT_DETACH)
 			if cerr := cleanupDevice(); cerr != nil {
 				err = errors.Wrap(cerr, "device cleanup failed")

snapshot/naive/naive.go

@@ -322,3 +322,8 @@ func (o *snapshotter) mounts(s storage.Snapshot) []mount.Mount {
 		},
 	}
 }
+
+// Close closes the snapshotter
+func (o *snapshotter) Close() error {
+	return o.ms.Close()
+}

snapshot/naive/naive_test.go

@@ -15,7 +15,7 @@ func newSnapshotter(ctx context.Context, root string) (snapshot.Snapshotter, fun
 		return nil, nil, err
 	}
 
-	return snapshotter, nil, nil
+	return snapshotter, func() error { return snapshotter.Close() }, nil
 }
 
 func TestNaive(t *testing.T) {

snapshot/overlay/overlay.go

@@ -401,3 +401,8 @@ func (o *snapshotter) upperPath(id string) string {
 func (o *snapshotter) workPath(id string) string {
 	return filepath.Join(o.root, "snapshots", id, "work")
 }
+
+// Close closes the snapshotter
+func (o *snapshotter) Close() error {
+	return o.ms.Close()
+}

snapshot/overlay/overlay_test.go

@@ -24,7 +24,7 @@ func newSnapshotter(ctx context.Context, root string) (snapshot.Snapshotter, fun
 		return nil, nil, err
 	}
 
-	return snapshotter, nil, nil
+	return snapshotter, func() error { return snapshotter.Close() }, nil
 }
 
 func TestOverlay(t *testing.T) {

snapshot/snapshotter.go

@@ -296,6 +296,14 @@ type Snapshotter interface {
 	// Walk all snapshots in the snapshotter. For each snapshot in the
 	// snapshotter, the function will be called.
 	Walk(ctx context.Context, fn func(context.Context, Info) error) error
+
+	// Close releases the internal resources.
+	//
+	// Close is expected to be called on the end of the lifecycle of the snapshotter,
+	// but not mandatory.
+	//
+	// Close returns nil when it is already closed.
+	Close() error
 }
 
 // Opt allows setting mutable snapshot properties on creation

snapshot/testsuite/testsuite.go

@@ -44,6 +44,7 @@ func SnapshotterSuite(t *testing.T, name string, snapshotterFn func(ctx context.
 	t.Run("ViewReadonly", makeTest(name, snapshotterFn, checkSnapshotterViewReadonly))
 
 	t.Run("StatInWalk", makeTest(name, snapshotterFn, checkStatInWalk))
+	t.Run("CloseTwice", makeTest(name, snapshotterFn, closeTwice))
 }
 
 func makeTest(name string, snapshotterFn func(ctx context.Context, root string) (snapshot.Snapshotter, func() error, error), fn func(ctx context.Context, t *testing.T, snapshotter snapshot.Snapshotter, work string)) func(t *testing.T) {
@@ -786,3 +787,22 @@ func checkFileFromLowerLayer(ctx context.Context, t *testing.T, snapshotter snap
 		t.Fatalf("Check snapshots failed: %+v", err)
 	}
 }
+
+func closeTwice(ctx context.Context, t *testing.T, snapshotter snapshot.Snapshotter, work string) {
+	// do some dummy ops to modify the snapshotter internal state
+	if _, err := snapshotter.Prepare(ctx, "dummy", ""); err != nil {
+		t.Fatal(err)
+	}
+	if err := snapshotter.Commit(ctx, "dummy-1", "dummy"); err != nil {
+		t.Fatal(err)
+	}
+	if err := snapshotter.Remove(ctx, "dummy-1"); err != nil {
+		t.Fatal(err)
+	}
+	if err := snapshotter.Close(); err != nil {
+		t.Fatalf("The first close failed: %+v", err)
+	}
+	if err := snapshotter.Close(); err != nil {
+		t.Fatalf("The second close failed: %+v", err)
+	}
+}

snapshot/windows/windows.go

@@ -84,3 +84,8 @@ func (o *snapshotter) Remove(ctx context.Context, key string) error {
 func (o *snapshotter) Walk(ctx context.Context, fn func(context.Context, snapshot.Info) error) error {
 	panic("not implemented")
 }
+
+// Close closes the snapshotter
+func (o *snapshotter) Close() error {
+	panic("not implemented")
+}

snapshot_test.go

@@ -18,6 +18,7 @@ func newSnapshotter(ctx context.Context, root string) (snapshot.Snapshotter, fun
 	sn := client.SnapshotService(DefaultSnapshotter)
 
 	return sn, func() error {
+		// no need to close remote snapshotter
 		return client.Close()
 	}, nil
 }