Skip to content

Commit a2cc5bf

Browse files
austinvazquezaustinvazquez
authored
Merge pull request #656 from austinvazquez/remote-snapshotter-no-context-walk
add: no namespace walk support to demux snapshotter
2 parents 22e72b2 + 9ff0679 commit a2cc5bf

File tree

5 files changed

+119
-14
lines changed

5 files changed

+119
-14
lines changed

‎snapshotter/demux/cache/cache.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package cache
1616
import (
1717
"context"
1818

19+
"github.com/containerd/containerd/snapshots"
1920
"github.com/firecracker-microvm/firecracker-containerd/snapshotter/demux/proxy"
2021
)
2122

@@ -28,6 +29,9 @@ type Cache interface {
2829
// fetch function if the snapshotter is not currently cached.
2930
Get(ctx context.Context, key string, fetch SnapshotterProvider) (*proxy.RemoteSnapshotter, error)
3031

32+
// WalkAll applies the provided function across all cached snapshotters.
33+
WalkAll(ctx context.Context, fn snapshots.WalkFunc, filters ...string) error
34+
3135
// Closes the snapshotter and removes it from the cache.
3236
Evict(key string) error
3337

‎snapshotter/demux/cache/snapshotter_cache.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"fmt"
1919
"sync"
2020

21+
"github.com/containerd/containerd/snapshots"
2122
"github.com/firecracker-microvm/firecracker-containerd/snapshotter/demux/proxy"
2223
"github.com/hashicorp/go-multierror"
2324
)
@@ -56,6 +57,21 @@ func (cache *SnapshotterCache) Get(ctx context.Context, key string, fetch Snapsh
5657
return snapshotter, nil
5758
}
5859

60+
// WalkAll applies the provided function to all cached snapshotters.
61+
func (cache *SnapshotterCache) WalkAll(ctx context.Context, fn snapshots.WalkFunc, filters ...string) error {
62+
cache.mutex.RLock()
63+
defer cache.mutex.RUnlock()
64+
65+
var allErr error
66+
67+
for namespace, snapshotter := range cache.snapshotters {
68+
if err := snapshotter.Walk(ctx, fn, filters...); err != nil {
69+
allErr = multierror.Append(allErr, fmt.Errorf("failed to walk function on snapshotter[%s]: %w", namespace, err))
70+
}
71+
}
72+
return allErr
73+
}
74+
5975
// Evict removes a cached snapshotter for a given key.
6076
func (cache *SnapshotterCache) Evict(key string) error {
6177
cache.mutex.RLock()

‎snapshotter/demux/cache/snapshotter_cache_test.go‎

Lines changed: 71 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"context"
1818
"testing"
1919

20+
"github.com/containerd/containerd/snapshots"
2021
"github.com/hashicorp/go-multierror"
2122
"github.com/pkg/errors"
2223

@@ -55,13 +56,53 @@ func getCachedSnapshotter(uut *SnapshotterCache) error {
5556
return nil
5657
}
5758

58-
func getSnapshotterPropogatesErrors(uut *SnapshotterCache) error {
59+
func getSnapshotterPropagatesErrors(uut *SnapshotterCache) error {
5960
if _, err := uut.Get(context.Background(), "SnapshotterKey", getSnapshotterErrorFunction); err == nil {
6061
return errors.New("Get function did not propagate errors from snapshotter generator function")
6162
}
6263
return nil
6364
}
6465

66+
func successfulWalk(ctx context.Context, info snapshots.Info) error {
67+
return nil
68+
}
69+
70+
func applyWalkFunctionOnEmptyCache(uut *SnapshotterCache) error {
71+
if err := uut.WalkAll(context.Background(), successfulWalk); err != nil {
72+
return errors.New("WalkAll on empty cache incorrectly resulted in error")
73+
}
74+
return nil
75+
}
76+
77+
func applyWalkFunctionToAllCachedSnapshotters(uut *SnapshotterCache) error {
78+
if _, err := uut.Get(context.Background(), "Snapshotter-A", getSnapshotterOkFunction); err != nil {
79+
return errors.Wrap(err, "Adding snapshotter A to empty cache incorrectly resulted in error")
80+
}
81+
if _, err := uut.Get(context.Background(), "Snapshotter-B", getSnapshotterOkFunction); err != nil {
82+
return errors.Wrap(err, "Adding snapshotter B to empty cache incorrectly resulted in error")
83+
}
84+
if err := uut.WalkAll(context.Background(), successfulWalk); err != nil {
85+
return errors.New("WalkAll on populated cache incorrectly resulted in error")
86+
}
87+
return nil
88+
}
89+
90+
func applyWalkFunctionPropagatesErrors(uut *SnapshotterCache) error {
91+
if _, err := uut.Get(context.Background(), "Snapshotter-A", getFailingSnapshotterOkFunction); err != nil {
92+
return errors.Wrap(err, "Adding snapshotter A to empty cache incorrectly resulted in error")
93+
}
94+
// The failing snapshotter mock will fail all Walk calls before applying
95+
// the snapshots.WalkFunc, but for the purposes of this test that is fine.
96+
// In which case, any function will do.
97+
walkFunc := func(ctx context.Context, info snapshots.Info) error {
98+
return nil
99+
}
100+
if err := uut.WalkAll(context.Background(), walkFunc); err == nil {
101+
return errors.New("WalkAll did not propagate errors from walk function")
102+
}
103+
return nil
104+
}
105+
65106
func evictSnapshotterFromEmptyCache(uut *SnapshotterCache) error {
66107
if err := uut.Evict("SnapshotterKey"); err == nil {
67108
return errors.New("Evict function did not return error on call on empty cache")
@@ -80,7 +121,7 @@ func evictSnapshotterFromCache(uut *SnapshotterCache) error {
80121
return nil
81122
}
82123

83-
func evictSnapshotterFromCachePropogatesCloseError(uut *SnapshotterCache) error {
124+
func evictSnapshotterFromCachePropagatesCloseError(uut *SnapshotterCache) error {
84125
if _, err := uut.Get(context.Background(), "SnapshotterKey", getFailingSnapshotterOkFunction); err != nil {
85126
return errors.Wrap(err, "Adding snapshotter to empty cache incorrectly resulted in error")
86127
}
@@ -156,14 +197,36 @@ func TestGetSnapshotterFromCache(t *testing.T) {
156197
}{
157198
{"AddSnapshotterToCache", getSnapshotterFromEmptyCache},
158199
{"GetCachedSnapshotter", getCachedSnapshotter},
159-
{"PropogateFetchSnapshotterErrors", getSnapshotterPropogatesErrors},
200+
{"PropogateFetchSnapshotterErrors", getSnapshotterPropagatesErrors},
201+
}
202+
203+
for _, test := range tests {
204+
t.Run(test.name, func(t *testing.T) {
205+
uut := NewSnapshotterCache()
206+
if err := test.run(uut); err != nil {
207+
t.Fatalf("%s: %s", test.name, err.Error())
208+
}
209+
})
210+
}
211+
}
212+
213+
func TestWalkAllFunctionOnCache(t *testing.T) {
214+
t.Parallel()
215+
216+
tests := []struct {
217+
name string
218+
run func(*SnapshotterCache) error
219+
}{
220+
{"ApplyWalkFunctionOnEmptyCache", applyWalkFunctionOnEmptyCache},
221+
{"ApplyWalkFunctionToAllCachedSnapshotters", applyWalkFunctionToAllCachedSnapshotters},
222+
{"ApplyWalkFunctionPropogatesErrors", applyWalkFunctionPropagatesErrors},
160223
}
161224

162225
for _, test := range tests {
163226
t.Run(test.name, func(t *testing.T) {
164227
uut := NewSnapshotterCache()
165228
if err := test.run(uut); err != nil {
166-
t.Fatal(test.name + ": " + err.Error())
229+
t.Fatalf("%s: %s", test.name, err.Error())
167230
}
168231
})
169232
}
@@ -178,14 +241,14 @@ func TestEvictSnapshotterFromCache(t *testing.T) {
178241
}{
179242
{"EvictSnapshotterFromEmptyCache", evictSnapshotterFromEmptyCache},
180243
{"EvictSnapshotterFromCache", evictSnapshotterFromCache},
181-
{"PropogateEvictSnapshotterCloseErrors", evictSnapshotterFromCachePropogatesCloseError},
244+
{"PropogateEvictSnapshotterCloseErrors", evictSnapshotterFromCachePropagatesCloseError},
182245
}
183246

184247
for _, test := range tests {
185248
t.Run(test.name, func(t *testing.T) {
186249
uut := NewSnapshotterCache()
187250
if err := test.run(uut); err != nil {
188-
t.Fatal(test.name + ": " + err.Error())
251+
t.Fatalf("%s: %s", test.name, err.Error())
189252
}
190253
})
191254
}
@@ -207,7 +270,7 @@ func TestCloseCache(t *testing.T) {
207270
t.Run(test.name, func(t *testing.T) {
208271
uut := NewSnapshotterCache()
209272
if err := test.run(uut); err != nil {
210-
t.Fatal(test.name + ": " + err.Error())
273+
t.Fatalf("%s: %s", test.name, err.Error())
211274
}
212275
})
213276
}
@@ -228,7 +291,7 @@ func TestListSnapshotters(t *testing.T) {
228291
t.Run(test.name, func(t *testing.T) {
229292
uut := NewSnapshotterCache()
230293
if err := test.run(uut); err != nil {
231-
t.Fatal(test.name + ": " + err.Error())
294+
t.Fatalf("%s: %s", test.name, err.Error())
232295
}
233296
})
234297
}

‎snapshotter/demux/snapshotter.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,8 @@ func (s *Snapshotter) Walk(ctx context.Context, fn snapshots.WalkFunc, filters .
243243
contextLogger := log.G(ctx).WithField("function", "Walk")
244244
namespace, err := getNamespaceFromContext(ctx, contextLogger)
245245
if err != nil {
246-
return err
246+
contextLogger.Debug("no namespace found, proxying walk function to all cached snapshotters")
247+
return s.cache.WalkAll(ctx, fn, filters...)
247248
}
248249
logger := contextLogger.WithField("namespace", namespace)
249250

‎snapshotter/demux/snapshotter_test.go‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -73,15 +73,36 @@ func TestReturnErrorWhenCalledWithoutNamespacedContext(t *testing.T) {
7373
{"View", func() error { _, err := uut.View(ctx, "layerKey", ""); return err }},
7474
{"Commit", func() error { return uut.Commit(ctx, "layer1", "layerKey") }},
7575
{"Remove", func() error { return uut.Remove(ctx, "layerKey") }},
76+
}
77+
78+
for _, test := range tests {
79+
if err := test.run(); err == nil {
80+
t.Fatalf("%s call did not return error", test.name)
81+
}
82+
}
83+
}
84+
85+
func TestNoErrorWhenCalledWithoutNamespacedContext(t *testing.T) {
86+
t.Parallel()
87+
88+
cache := cache.NewSnapshotterCache()
89+
ctx := logtest.WithT(context.Background(), t)
90+
91+
uut := NewSnapshotter(cache, fetchOkSnapshotter)
92+
93+
tests := []struct {
94+
name string
95+
run func() error
96+
}{
7697
{"Walk", func() error {
7798
var callback = func(c context.Context, i snapshots.Info) error { return nil }
7899
return uut.Walk(ctx, callback)
79100
}},
80101
}
81102

82103
for _, test := range tests {
83-
if err := test.run(); err == nil {
84-
t.Fatal(test.name + " call did not return error")
104+
if err := test.run(); err != nil {
105+
t.Fatalf("%s call returned error on no namespace execution", test.name)
85106
}
86107
}
87108
}
@@ -116,7 +137,7 @@ func TestReturnErrorWhenSnapshotterNotFound(t *testing.T) {
116137

117138
for _, test := range tests {
118139
if err := test.run(); err == nil {
119-
t.Fatal(test.name + " call did not return error")
140+
t.Fatalf("%s call did not return error", test.name)
120141
}
121142
}
122143
}
@@ -153,7 +174,7 @@ func TestReturnErrorAfterProxyFunctionFailure(t *testing.T) {
153174
for _, test := range tests {
154175
t.Run(test.name+"ProxyFailure", func(t *testing.T) {
155176
if err := test.run(); err == nil {
156-
t.Fatal(test.name + " call did not return error")
177+
t.Fatalf("%s call did not return error", test.name)
157178
}
158179
})
159180
}
@@ -191,7 +212,7 @@ func TestNoErrorIsReturnedOnSuccessfulProxyExecution(t *testing.T) {
191212
for _, test := range tests {
192213
t.Run(test.name+"SuccessfulProxyCall", func(t *testing.T) {
193214
if err := test.run(); err != nil {
194-
t.Fatal(test.name + " call incorrectly returned an error")
215+
t.Fatalf("%s call incorrectly returned an error", test.name)
195216
}
196217
})
197218
}

0 commit comments

Comments
 (0)