From 4feb6f228af93d0a78933359068ec67159cb497f Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Tue, 7 Nov 2017 08:26:56 +0000 Subject: [PATCH] snapshot: add Close() Signed-off-by: Akihiro Suda --- metadata/snapshot.go | 5 +++++ metadata/snapshot_test.go | 3 +++ services/snapshot/client.go | 4 ++++ snapshot/btrfs/btrfs.go | 5 +++++ snapshot/btrfs/btrfs_test.go | 3 +++ snapshot/naive/naive.go | 5 +++++ snapshot/naive/naive_test.go | 2 +- snapshot/overlay/overlay.go | 5 +++++ snapshot/overlay/overlay_test.go | 2 +- snapshot/snapshotter.go | 8 ++++++++ snapshot/testsuite/testsuite.go | 20 ++++++++++++++++++++ snapshot/windows/windows.go | 5 +++++ snapshot_test.go | 1 + 13 files changed, 66 insertions(+), 2 deletions(-) diff --git a/metadata/snapshot.go b/metadata/snapshot.go index 22ce3c8c0..272c2d01a 100644 --- a/metadata/snapshot.go +++ b/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() +} diff --git a/metadata/snapshot_test.go b/metadata/snapshot_test.go index 5111c36b8..ad4a5d8b5 100644 --- a/metadata/snapshot_test.go +++ b/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 } diff --git a/services/snapshot/client.go b/services/snapshot/client.go index a9b9ffe67..812b00ce0 100644 --- a/services/snapshot/client.go +++ b/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 diff --git a/snapshot/btrfs/btrfs.go b/snapshot/btrfs/btrfs.go index 32d66957e..ddaeae482 100644 --- a/snapshot/btrfs/btrfs.go +++ b/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() +} diff --git a/snapshot/btrfs/btrfs_test.go b/snapshot/btrfs/btrfs_test.go index 63c9b02af..97a053ce3 100644 --- a/snapshot/btrfs/btrfs_test.go +++ b/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") diff --git a/snapshot/naive/naive.go b/snapshot/naive/naive.go index 006afd576..bcd4c35fd 100644 --- a/snapshot/naive/naive.go +++ b/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() +} diff --git a/snapshot/naive/naive_test.go b/snapshot/naive/naive_test.go index 279e95505..f8111f5d0 100644 --- a/snapshot/naive/naive_test.go +++ b/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) { diff --git a/snapshot/overlay/overlay.go b/snapshot/overlay/overlay.go index 0f800da22..e5c655f4a 100644 --- a/snapshot/overlay/overlay.go +++ b/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() +} diff --git a/snapshot/overlay/overlay_test.go b/snapshot/overlay/overlay_test.go index f3b53e949..653a4b038 100644 --- a/snapshot/overlay/overlay_test.go +++ b/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) { diff --git a/snapshot/snapshotter.go b/snapshot/snapshotter.go index 2b3fe6275..29f77d7e9 100644 --- a/snapshot/snapshotter.go +++ b/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 diff --git a/snapshot/testsuite/testsuite.go b/snapshot/testsuite/testsuite.go index 04443891f..5ce069efd 100644 --- a/snapshot/testsuite/testsuite.go +++ b/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) + } +} diff --git a/snapshot/windows/windows.go b/snapshot/windows/windows.go index b36273d85..a72fa4b07 100644 --- a/snapshot/windows/windows.go +++ b/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") +} diff --git a/snapshot_test.go b/snapshot_test.go index 0bcab7876..a345429a3 100644 --- a/snapshot_test.go +++ b/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 }