Skip to content

add: no namespace remove snapshot support to demux snapshotter #678

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

add: no namespace remove snapshot support to demux snapshotter #678

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions snapshotter/demux/cache/cache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,15 @@ type Cache interface {
// fetch function if the snapshotter is not currently cached.
Get(ctx context.Context, key string, fetch SnapshotterProvider) (*proxy.RemoteSnapshotter, error)

// RemoveAll removes the snapshot by the provided key from all cached snapshotters.
RemoveAll(ctx context.Context, key string) error

// WalkAll applies the provided function across all cached snapshotters.
WalkAll(ctx context.Context, fn snapshots.WalkFunc, filters ...string) error

// CleanupAll issues a cleanup call to all cached snapshotters.
CleanupAll(ctx context.Context) error

// Closes the snapshotter and removes it from the cache.
Evict(key string) error

Expand Down
31 changes: 31 additions & 0 deletions snapshotter/demux/cache/snapshotter_cache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"fmt"
"sync"

"github.com/containerd/containerd/errdefs"
"github.com/containerd/containerd/snapshots"
"github.com/firecracker-microvm/firecracker-containerd/snapshotter/demux/proxy"
"github.com/hashicorp/go-multierror"
Expand Down Expand Up @@ -59,6 +60,21 @@ func (cache *SnapshotterCache) Get(ctx context.Context, key string, fetch Snapsh
return snapshotter, nil
}

// RemoveAll removes the snapshot by the provided key from all cached snapshotters.
func (cache *SnapshotterCache) RemoveAll(ctx context.Context, key string) error {
cache.mutex.RLock()
defer cache.mutex.RUnlock()

var allErr error

for namespace, snapshotter := range cache.snapshotters {
if err := snapshotter.Remove(ctx, key); err != nil && err != errdefs.ErrNotFound {
allErr = multierror.Append(allErr, fmt.Errorf("failed to remove snapshot on snapshotter[%s]: %w", namespace, err))
}
}
return allErr
}

// WalkAll applies the provided function to all cached snapshotters.
func (cache *SnapshotterCache) WalkAll(ctx context.Context, fn snapshots.WalkFunc, filters ...string) error {
cache.mutex.RLock()
Expand All @@ -74,6 +90,21 @@ func (cache *SnapshotterCache) WalkAll(ctx context.Context, fn snapshots.WalkFun
return allErr
}

// CleanupAll issues a cleanup to all cached snapshotters.
func (cache *SnapshotterCache) CleanupAll(ctx context.Context) error {
cache.mutex.RLock()
defer cache.mutex.RUnlock()

var allErr error

for namespace, snapshotter := range cache.snapshotters {
if err := snapshotter.Cleanup(ctx); err != nil {
allErr = multierror.Append(allErr, fmt.Errorf("failed cleanup function on snapshotter[%s]: %w", namespace, err))
}
}
return allErr
}

// Evict removes a cached snapshotter for a given key.
func (cache *SnapshotterCache) Evict(key string) error {
cache.mutex.RLock()
Expand Down
167 changes: 153 additions & 14 deletions snapshotter/demux/cache/snapshotter_cache_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,23 +19,36 @@ import (
"fmt"
"testing"

"github.com/containerd/containerd/errdefs"
"github.com/containerd/containerd/snapshots"
"github.com/hashicorp/go-multierror"

"github.com/firecracker-microvm/firecracker-containerd/snapshotter/demux/internal"
"github.com/firecracker-microvm/firecracker-containerd/snapshotter/demux/internal/mock"
"github.com/firecracker-microvm/firecracker-containerd/snapshotter/demux/proxy"
)

func getSnapshotterOkFunction(ctx context.Context, key string) (*proxy.RemoteSnapshotter, error) {
return &proxy.RemoteSnapshotter{Snapshotter: &internal.SuccessfulSnapshotter{}}, nil
return &proxy.RemoteSnapshotter{Snapshotter: &mock.Snapshotter{}}, nil
}

func getSnapshotterErrorFunction(ctx context.Context, key string) (*proxy.RemoteSnapshotter, error) {
return &proxy.RemoteSnapshotter{Snapshotter: &internal.SuccessfulSnapshotter{}}, errors.New("mock error")
return &proxy.RemoteSnapshotter{Snapshotter: nil}, errors.New("mock error")
}

func getFailingSnapshotterOkFunction(ctx context.Context, key string) (*proxy.RemoteSnapshotter, error) {
return &proxy.RemoteSnapshotter{Snapshotter: &internal.FailingSnapshotter{}}, nil
return &proxy.RemoteSnapshotter{Snapshotter: &mock.Snapshotter{
StatError: errors.New("mock stat error"),
UpdateError: errors.New("mock update error"),
UsageError: errors.New("mock usage error"),
MountsError: errors.New("mock mounts error"),
PrepareError: errors.New("mock prepare error"),
ViewError: errors.New("mock view error"),
CommitError: errors.New("mock commit error"),
RemoveError: errors.New("mock remove error"),
WalkError: errors.New("mock walk error"),
CloseError: errors.New("mock close error"),
CleanupError: errors.New("mock cleanup error"),
}}, nil
}

func getSnapshotterFromEmptyCache(uut *SnapshotterCache) error {
Expand All @@ -59,7 +72,7 @@ func getCachedSnapshotter(uut *SnapshotterCache) error {

func getSnapshotterPropagatesErrors(uut *SnapshotterCache) error {
if _, err := uut.Get(context.Background(), "SnapshotterKey", getSnapshotterErrorFunction); err == nil {
return errors.New("Get function did not propagate errors from snapshotter generator function")
return fmt.Errorf("Get function did not propagate errors from snapshotter generator function [%w]", err)
}
return nil
}
Expand All @@ -68,9 +81,57 @@ func successfulWalk(ctx context.Context, info snapshots.Info) error {
return nil
}

func removeFunctionOnEmptyCache(uut *SnapshotterCache) error {
if err := uut.RemoveAll(context.Background(), "SnapshotKey"); err != nil {
return fmt.Errorf("RemoveAll on empty cache incorrectly resulted in error [%w]", err)
}
return nil
}

func removeFunctionToAllCachedSnapshotters(uut *SnapshotterCache) error {
if _, err := uut.Get(context.Background(), "Snapshotter-A", getSnapshotterOkFunction); err != nil {
return fmt.Errorf("Adding snapshotter A to empty cache incorrectly resulted in error [%w]", err)
}
if _, err := uut.Get(context.Background(), "Snapshotter-B", getSnapshotterOkFunction); err != nil {
return fmt.Errorf("Adding snapshotter B to cache incorrectly resulted in error [%w]", err)
}
if err := uut.RemoveAll(context.Background(), "SnapshotKey"); err != nil {
return fmt.Errorf("RemoveAll on populated cache incorrectly resulted in error [%w]", err)
}
return nil
}

func removeFunctionPropagatesErrorsExceptSnapshotNotFound(uut *SnapshotterCache) error {
if _, err := uut.Get(context.Background(), "Snapshotter-A", getFailingSnapshotterOkFunction); err != nil {
return fmt.Errorf("Adding snapshotter A to empty cache incorrectly resulted in error [%w]", err)
}

getFailingRemoveSnapshotterWithSnapshotNotFound := func(ctx context.Context, key string) (*proxy.RemoteSnapshotter, error) {
return &proxy.RemoteSnapshotter{Snapshotter: &mock.Snapshotter{RemoveError: errdefs.ErrNotFound}}, nil
}
if _, err := uut.Get(context.Background(), "Snapshotter-B", getFailingRemoveSnapshotterWithSnapshotNotFound); err != nil {
return fmt.Errorf("Adding snapshotter B to cache incorrectly resulted in error [%w]", err)
}

getFailingRemoveSnapshotter := func(ctx context.Context, key string) (*proxy.RemoteSnapshotter, error) {
return &proxy.RemoteSnapshotter{Snapshotter: &mock.Snapshotter{RemoveError: errors.New("mock remove error")}}, nil
}
if _, err := uut.Get(context.Background(), "Snapshotter-C", getFailingRemoveSnapshotter); err != nil {
return fmt.Errorf("Adding snapshotter C to cache incorrectly resulted in error [%w]", err)
}
err := uut.RemoveAll(context.Background(), "SnapshotKey")
if err == nil {
return fmt.Errorf("RemoveAll did not propagate errors from remove function")
}
if err == errdefs.ErrNotFound {
return fmt.Errorf("RemoveAll did not mask snapshot not found errors [%w]", err)
}
return nil
}

func applyWalkFunctionOnEmptyCache(uut *SnapshotterCache) error {
if err := uut.WalkAll(context.Background(), successfulWalk); err != nil {
return errors.New("WalkAll on empty cache incorrectly resulted in error")
return fmt.Errorf("WalkAll on empty cache incorrectly resulted in error [%w]", err)
}
return nil
}
Expand Down Expand Up @@ -99,14 +160,49 @@ func applyWalkFunctionPropagatesErrors(uut *SnapshotterCache) error {
return nil
}
if err := uut.WalkAll(context.Background(), walkFunc); err == nil {
return errors.New("WalkAll did not propagate errors from walk function")
return fmt.Errorf("WalkAll did not propagate errors from walk function [%w]", err)
}
return nil
}

func cleanupFunctionOnEmptyCache(uut *SnapshotterCache) error {
if err := uut.CleanupAll(context.Background()); err != nil {
return fmt.Errorf("CleanupAll on empty cache incorrectly resulted in error [%w]", err)
}
return nil
}

func cleanupFunctionToAllCacheSnapshotters(uut *SnapshotterCache) error {
if _, err := uut.Get(context.Background(), "Snapshotter-A", getSnapshotterOkFunction); err != nil {
return fmt.Errorf("Adding snapshotter A to empty cache incorrectly resulted in error [%w]", err)
}
if _, err := uut.Get(context.Background(), "Snapshotter-B", getSnapshotterOkFunction); err != nil {
return fmt.Errorf("Adding snapshotter B to cache incorrectly resulted in error [%w]", err)
}
if err := uut.CleanupAll(context.Background()); err != nil {
return fmt.Errorf("CleanupAll on populated cache incorrectly resulted in error")
}
return nil
}

func cleanupFunctionPropagatesErrors(uut *SnapshotterCache) error {
if _, err := uut.Get(context.Background(), "Snapshotter-A", getSnapshotterOkFunction); err != nil {
return fmt.Errorf("Adding snapshotter A to empty cache incorrectly resulted in error [%w]", err)
}

if _, err := uut.Get(context.Background(), "Snapshotter-B", getFailingSnapshotterOkFunction); err != nil {
return fmt.Errorf("Adding snapshotter B to cache incorrectly resulted in error [%w]", err)
}

if err := uut.CleanupAll(context.Background()); err == nil {
return fmt.Errorf("CleanupAll did not propagate errors from remove function")
}
return nil
}

func evictSnapshotterFromEmptyCache(uut *SnapshotterCache) error {
if err := uut.Evict("SnapshotterKey"); err == nil {
return errors.New("Evict function did not return error on call on empty cache")
return fmt.Errorf("Evict function did not return error on call on empty cache")
}
return nil
}
Expand Down Expand Up @@ -156,7 +252,7 @@ func closeCacheReturnsAllOccurringErrors(uut *SnapshotterCache) error {

err := uut.Close()
if err == nil {
return errors.New("Close did not propagate the last close snapshotter error")
return fmt.Errorf("Close did not propagate the last close snapshotter error")
}
if merr, ok := err.(*multierror.Error); ok {
if merr.Len() != 2 {
Expand All @@ -169,7 +265,7 @@ func closeCacheReturnsAllOccurringErrors(uut *SnapshotterCache) error {
func listEmptyCache(uut *SnapshotterCache) error {
keys := uut.List()
if len(keys) > 0 {
return errors.New("List did not return an empty list for an empty snapshotter cache")
return fmt.Errorf("List did not return an empty list for an empty snapshotter cache")
}

return nil
Expand All @@ -180,10 +276,10 @@ func listNonEmptyCache(uut *SnapshotterCache) error {

keys := uut.List()
if len(keys) != 1 {
return errors.New("List should return a non-empty list of keys")
return fmt.Errorf("List should return a non-empty list of keys")
}
if keys[0] != "OkSnapshotterKey" {
return errors.New("List did not return correct list of keys")
return fmt.Errorf("List did not return correct list of keys")
}

return nil
Expand Down Expand Up @@ -211,6 +307,28 @@ func TestGetSnapshotterFromCache(t *testing.T) {
}
}

func TestRemoveAllFunctionOnCache(t *testing.T) {
t.Parallel()

tests := []struct {
name string
run func(*SnapshotterCache) error
}{
{"RemoveFunctionOnEmptyCache", removeFunctionOnEmptyCache},
{"RemoveFunctionToAllCachedSnapshotters", removeFunctionToAllCachedSnapshotters},
{"RemoveFunctionPropagatesErrorsExceptSnapshotNotFound", removeFunctionPropagatesErrorsExceptSnapshotNotFound},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
uut := NewSnapshotterCache()
if err := test.run(uut); err != nil {
t.Fatalf("%s: %v", test.name, err)
}
})
}
}

func TestWalkAllFunctionOnCache(t *testing.T) {
t.Parallel()

Expand All @@ -220,7 +338,7 @@ func TestWalkAllFunctionOnCache(t *testing.T) {
}{
{"ApplyWalkFunctionOnEmptyCache", applyWalkFunctionOnEmptyCache},
{"ApplyWalkFunctionToAllCachedSnapshotters", applyWalkFunctionToAllCachedSnapshotters},
{"ApplyWalkFunctionPropogatesErrors", applyWalkFunctionPropagatesErrors},
{"ApplyWalkFunctionPropagatesErrors", applyWalkFunctionPropagatesErrors},
}

for _, test := range tests {
Expand All @@ -233,6 +351,28 @@ func TestWalkAllFunctionOnCache(t *testing.T) {
}
}

func TestCleanupAllFunctionOnCache(t *testing.T) {
t.Parallel()

tests := []struct {
name string
run func(*SnapshotterCache) error
}{
{"CleanupFunctionOnEmptyCache", cleanupFunctionOnEmptyCache},
{"CleanupFunctionToAllCachedSnapshotters", cleanupFunctionToAllCacheSnapshotters},
{"CleanupFunctionPropagatesErrors", cleanupFunctionPropagatesErrors},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
uut := NewSnapshotterCache()
if err := test.run(uut); err != nil {
t.Fatalf("%s: %v", test.name, err)
}
})
}
}

func TestEvictSnapshotterFromCache(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -296,5 +436,4 @@ func TestListSnapshotters(t *testing.T) {
}
})
}

}
Loading