Skip to content

Commit 76d673a

Browse files
Merge pull request #657 from austinvazquez/remote-snapshotter-cleaner
add: asynchronous cleanup support to demux snapshotter
2 parents 7005764 + 06b840a commit 76d673a

File tree

5 files changed

+69
-104
lines changed

5 files changed

+69
-104
lines changed

snapshotter/demux/internal/failing_snapshotter.go

+5
Original file line numberDiff line numberDiff line change
@@ -74,3 +74,8 @@ func (s *FailingSnapshotter) Walk(ctx context.Context, fn snapshots.WalkFunc, fi
7474
func (s *FailingSnapshotter) Close() error {
7575
return errors.New("mock Close error from remote snapshotter")
7676
}
77+
78+
// Cleanup mocks a failing remote call with a non-nil error.
79+
func (s *FailingSnapshotter) Cleanup(ctx context.Context) error {
80+
return errors.New("mock Cleanup error from remote snapshotter")
81+
}

snapshotter/demux/internal/successful_snapshotter.go

+5
Original file line numberDiff line numberDiff line change
@@ -73,3 +73,8 @@ func (s *SuccessfulSnapshotter) Walk(ctx context.Context, fn snapshots.WalkFunc,
7373
func (s *SuccessfulSnapshotter) Close() error {
7474
return nil
7575
}
76+
77+
// Cleanup mocks a successful remote call with a nil error.
78+
func (s *SuccessfulSnapshotter) Cleanup(ctx context.Context) error {
79+
return nil
80+
}

snapshotter/demux/proxy/snapshotter.go

+8
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,14 @@ func NewRemoteSnapshotter(ctx context.Context, address string,
7373
return &RemoteSnapshotter{proxy.NewSnapshotter(snapshotsapi.NewSnapshotsClient(gRPCConn), address), metricsProxy}, nil
7474
}
7575

76+
// Cleanup implements the Cleaner interface for snapshotters.
77+
// This enables asynchronous resource cleanup by remote snapshotters.
78+
//
79+
// See https://github.com/containerd/containerd/blob/v1.6.4/snapshots/snapshotter.go
80+
func (rs *RemoteSnapshotter) Cleanup(ctx context.Context) error {
81+
return rs.Snapshotter.(snapshots.Cleaner).Cleanup(ctx)
82+
}
83+
7684
// MetricsProxyPort returns the metrics proxy port for a remote snapshotter.
7785
func (rs *RemoteSnapshotter) MetricsProxyPort() int {
7886
return rs.metricsProxy.Port

snapshotter/demux/snapshotter.go

+47-104
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,6 @@ import (
2727
mountutil "github.com/firecracker-microvm/firecracker-containerd/snapshotter/internal/mount"
2828
)
2929

30-
const proxiedFunctionCallErrorString = "Proxied function call failed"
31-
3230
// Snapshotter routes snapshotter requests to their destined
3331
// remote snapshotter via their snapshotter namespace.
3432
//
@@ -46,234 +44,179 @@ func NewSnapshotter(cache cache.Cache, fetchSnapshotter cache.SnapshotterProvide
4644

4745
// Stat proxies remote snapshotter stat request.
4846
//
49-
// See https://github.com/containerd/containerd/blob/main/snapshots/snapshotter.go
47+
// See https://github.com/containerd/containerd/blob/v1.6.4/snapshots/snapshotter.go
5048
func (s *Snapshotter) Stat(ctx context.Context, key string) (snapshots.Info, error) {
5149
contextLogger := log.G(ctx).WithField("function", "Stat")
52-
namespace, err := getNamespaceFromContext(ctx, contextLogger)
53-
if err != nil {
54-
return snapshots.Info{}, err
55-
}
56-
logger := contextLogger.WithField("namespace", namespace)
5750

58-
snapshotter, err := s.getSnapshotterFromCache(ctx, namespace, logger)
51+
snapshotter, err := s.getSnapshotterFromCache(ctx, contextLogger)
5952
if err != nil {
6053
return snapshots.Info{}, err
6154
}
6255

63-
info, err := snapshotter.Stat(ctx, key)
64-
if err != nil {
65-
contextLogger.WithError(err).Error(proxiedFunctionCallErrorString)
66-
return snapshots.Info{}, err
67-
}
68-
return info, nil
56+
return snapshotter.Stat(ctx, key)
6957
}
7058

7159
// Update proxies remote snapshotter update request.
7260
//
73-
// See https://github.com/containerd/containerd/blob/main/snapshots/snapshotter.go
61+
// See https://github.com/containerd/containerd/blob/v1.6.4/snapshots/snapshotter.go
7462
func (s *Snapshotter) Update(ctx context.Context, info snapshots.Info, fieldpaths ...string) (snapshots.Info, error) {
7563
contextLogger := log.G(ctx).WithField("function", "Update")
76-
namespace, err := getNamespaceFromContext(ctx, contextLogger)
77-
if err != nil {
78-
return snapshots.Info{}, err
79-
}
80-
logger := contextLogger.WithField("namespace", namespace)
8164

82-
snapshotter, err := s.getSnapshotterFromCache(ctx, namespace, logger)
65+
snapshotter, err := s.getSnapshotterFromCache(ctx, contextLogger)
8366
if err != nil {
8467
return snapshots.Info{}, err
8568
}
8669

87-
updatedInfo, err := snapshotter.Update(ctx, info, fieldpaths...)
88-
if err != nil {
89-
contextLogger.WithError(err).Error(proxiedFunctionCallErrorString)
90-
return snapshots.Info{}, err
91-
}
92-
return updatedInfo, nil
70+
return snapshotter.Update(ctx, info, fieldpaths...)
9371
}
9472

9573
// Usage proxies remote snapshotter usage request.
9674
//
97-
// See https://github.com/containerd/containerd/blob/main/snapshots/snapshotter.go
75+
// See https://github.com/containerd/containerd/blob/v1.6.4/snapshots/snapshotter.go
9876
func (s *Snapshotter) Usage(ctx context.Context, key string) (snapshots.Usage, error) {
9977
contextLogger := log.G(ctx).WithField("function", "Usage")
100-
namespace, err := getNamespaceFromContext(ctx, contextLogger)
101-
if err != nil {
102-
return snapshots.Usage{}, err
103-
}
104-
logger := contextLogger.WithField("namespace", namespace)
10578

106-
snapshotter, err := s.getSnapshotterFromCache(ctx, namespace, logger)
79+
snapshotter, err := s.getSnapshotterFromCache(ctx, contextLogger)
10780
if err != nil {
10881
return snapshots.Usage{}, err
10982
}
11083

111-
usage, err := snapshotter.Usage(ctx, key)
112-
if err != nil {
113-
contextLogger.WithError(err).Error(proxiedFunctionCallErrorString)
114-
return snapshots.Usage{}, err
115-
}
116-
return usage, nil
84+
return snapshotter.Usage(ctx, key)
11785
}
11886

11987
// Mounts proxies remote snapshotter mounts request.
12088
//
121-
// See https://github.com/containerd/containerd/blob/main/snapshots/snapshotter.go
89+
// See https://github.com/containerd/containerd/blob/v1.6.4/snapshots/snapshotter.go
12290
func (s *Snapshotter) Mounts(ctx context.Context, key string) ([]mount.Mount, error) {
12391
contextLogger := log.G(ctx).WithField("function", "Mounts")
124-
namespace, err := getNamespaceFromContext(ctx, contextLogger)
125-
if err != nil {
126-
return []mount.Mount{}, err
127-
}
128-
logger := contextLogger.WithField("namespace", namespace)
12992

130-
snapshotter, err := s.getSnapshotterFromCache(ctx, namespace, logger)
93+
snapshotter, err := s.getSnapshotterFromCache(ctx, contextLogger)
13194
if err != nil {
13295
return []mount.Mount{}, err
13396
}
13497

13598
mounts, err := snapshotter.Mounts(ctx, key)
13699
if err != nil {
137-
contextLogger.WithError(err).Error(proxiedFunctionCallErrorString)
138100
return []mount.Mount{}, err
139101
}
140102
return mountutil.Map(mounts, vm.AddLocalMountIdentifier), nil
141103
}
142104

143105
// Prepare proxies remote snapshotter prepare request.
144106
//
145-
// See https://github.com/containerd/containerd/blob/main/snapshots/snapshotter.go
107+
// See https://github.com/containerd/containerd/blob/v1.6.4/snapshots/snapshotter.go
146108
func (s *Snapshotter) Prepare(ctx context.Context, key string, parent string, opts ...snapshots.Opt) ([]mount.Mount, error) {
147109
contextLogger := log.G(ctx).WithField("function", "Prepare")
148-
namespace, err := getNamespaceFromContext(ctx, contextLogger)
149-
if err != nil {
150-
return []mount.Mount{}, err
151-
}
152-
logger := contextLogger.WithField("namespace", namespace)
153110

154-
snapshotter, err := s.getSnapshotterFromCache(ctx, namespace, logger)
111+
snapshotter, err := s.getSnapshotterFromCache(ctx, contextLogger)
155112
if err != nil {
156113
return []mount.Mount{}, err
157114
}
158115

159116
mounts, err := snapshotter.Prepare(ctx, key, parent, opts...)
160117
if err != nil {
161-
contextLogger.WithError(err).Error(proxiedFunctionCallErrorString)
162118
return []mount.Mount{}, err
163119
}
164120
return mountutil.Map(mounts, vm.AddLocalMountIdentifier), nil
165121
}
166122

167123
// View proxies remote snapshotter view request.
168124
//
169-
// See https://github.com/containerd/containerd/blob/main/snapshots/snapshotter.go
125+
// See https://github.com/containerd/containerd/blob/v1.6.4/snapshots/snapshotter.go
170126
func (s *Snapshotter) View(ctx context.Context, key string, parent string, opts ...snapshots.Opt) ([]mount.Mount, error) {
171127
contextLogger := log.G(ctx).WithField("function", "View")
172-
namespace, err := getNamespaceFromContext(ctx, contextLogger)
173-
if err != nil {
174-
return []mount.Mount{}, err
175-
}
176-
logger := contextLogger.WithField("namespace", namespace)
177128

178-
snapshotter, err := s.getSnapshotterFromCache(ctx, namespace, logger)
129+
snapshotter, err := s.getSnapshotterFromCache(ctx, contextLogger)
179130
if err != nil {
180131
return []mount.Mount{}, err
181132
}
182133

183134
mounts, err := snapshotter.View(ctx, key, parent, opts...)
184135
if err != nil {
185-
contextLogger.WithError(err).Error(proxiedFunctionCallErrorString)
186136
return []mount.Mount{}, err
187137
}
188138
return mountutil.Map(mounts, vm.AddLocalMountIdentifier), nil
189139
}
190140

191141
// Commit proxies remote snapshotter commit request.
192142
//
193-
// See https://github.com/containerd/containerd/blob/main/snapshots/snapshotter.go
143+
// See https://github.com/containerd/containerd/blob/v1.6.4/snapshots/snapshotter.go
194144
func (s *Snapshotter) Commit(ctx context.Context, name string, key string, opts ...snapshots.Opt) error {
195145
contextLogger := log.G(ctx).WithField("function", "Commit")
196-
namespace, err := getNamespaceFromContext(ctx, contextLogger)
197-
if err != nil {
198-
return err
199-
}
200-
logger := contextLogger.WithField("namespace", namespace)
201146

202-
snapshotter, err := s.getSnapshotterFromCache(ctx, namespace, logger)
147+
snapshotter, err := s.getSnapshotterFromCache(ctx, contextLogger)
203148
if err != nil {
204149
return err
205150
}
206151

207-
err = snapshotter.Commit(ctx, name, key, opts...)
208-
if err != nil {
209-
contextLogger.WithError(err).Error(proxiedFunctionCallErrorString)
210-
return err
211-
}
212-
return nil
152+
return snapshotter.Commit(ctx, name, key, opts...)
213153
}
214154

215155
// Remove proxies remote snapshotter remove request.
216156
//
217-
// See https://github.com/containerd/containerd/blob/main/snapshots/snapshotter.go
157+
// See https://github.com/containerd/containerd/blob/v1.6.4/snapshots/snapshotter.go
218158
func (s *Snapshotter) Remove(ctx context.Context, key string) error {
219159
contextLogger := log.G(ctx).WithField("function", "Remove")
220-
namespace, err := getNamespaceFromContext(ctx, contextLogger)
221-
if err != nil {
222-
return err
223-
}
224-
logger := contextLogger.WithField("namespace", namespace)
225160

226-
snapshotter, err := s.getSnapshotterFromCache(ctx, namespace, logger)
161+
snapshotter, err := s.getSnapshotterFromCache(ctx, contextLogger)
227162
if err != nil {
228163
return err
229164
}
230165

231-
err = snapshotter.Remove(ctx, key)
232-
if err != nil {
233-
contextLogger.WithError(err).Error(proxiedFunctionCallErrorString)
234-
return err
235-
}
236-
return nil
166+
return snapshotter.Remove(ctx, key)
237167
}
238168

239169
// Walk proxies remote snapshotter walk request.
240170
//
241-
// See https://github.com/containerd/containerd/blob/main/snapshots/snapshotter.go
171+
// See https://github.com/containerd/containerd/blob/v1.6.4/snapshots/snapshotter.go
242172
func (s *Snapshotter) Walk(ctx context.Context, fn snapshots.WalkFunc, filters ...string) error {
243173
contextLogger := log.G(ctx).WithField("function", "Walk")
244-
namespace, err := getNamespaceFromContext(ctx, contextLogger)
174+
175+
_, err := getNamespaceFromContext(ctx, contextLogger)
245176
if err != nil {
246177
contextLogger.Debug("no namespace found, proxying walk function to all cached snapshotters")
247178
return s.cache.WalkAll(ctx, fn, filters...)
248179
}
249-
logger := contextLogger.WithField("namespace", namespace)
250180

251-
snapshotter, err := s.getSnapshotterFromCache(ctx, namespace, logger)
181+
snapshotter, err := s.getSnapshotterFromCache(ctx, contextLogger)
252182
if err != nil {
253183
return err
254184
}
255185

256-
err = snapshotter.Walk(ctx, fn, filters...)
257-
if err != nil {
258-
contextLogger.WithError(err).Error(proxiedFunctionCallErrorString)
259-
return err
260-
}
261-
return nil
186+
return snapshotter.Walk(ctx, fn, filters...)
262187
}
263188

264189
// Close calls close on all cached remote snapshotters.
265190
//
266-
// See https://github.com/containerd/containerd/blob/main/snapshots/snapshotter.go
191+
// See https://github.com/containerd/containerd/blob/v1.6.4/snapshots/snapshotter.go
267192
func (s *Snapshotter) Close() error {
268193
return s.cache.Close()
269194
}
270195

196+
// Cleanup proxies remote snapshotter cleanup request.
197+
//
198+
// See https://github.com/containerd/containerd/blob/v1.6.4/snapshots/snapshotter.go
199+
func (s *Snapshotter) Cleanup(ctx context.Context) error {
200+
contextLogger := log.G(ctx).WithField("function", "Cleanup")
201+
202+
snapshotter, err := s.getSnapshotterFromCache(ctx, contextLogger)
203+
if err != nil {
204+
return err
205+
}
206+
207+
return snapshotter.(snapshots.Cleaner).Cleanup(ctx)
208+
}
209+
271210
const snapshotterNotFoundErrorString = "Snapshotter not found in cache"
272211

273-
func (s *Snapshotter) getSnapshotterFromCache(ctx context.Context, namespace string, log *logrus.Entry) (snapshots.Snapshotter, error) {
212+
func (s *Snapshotter) getSnapshotterFromCache(ctx context.Context, log *logrus.Entry) (snapshots.Snapshotter, error) {
213+
namespace, err := getNamespaceFromContext(ctx, log)
214+
if err != nil {
215+
return nil, err
216+
}
274217
snapshotter, err := s.cache.Get(ctx, namespace, s.fetchSnapshotter)
275218
if err != nil {
276-
log.WithError(err).Error(snapshotterNotFoundErrorString)
219+
log.WithField("namespace", namespace).WithError(err).Error(snapshotterNotFoundErrorString)
277220
return nil, err
278221
}
279222
return snapshotter, nil

snapshotter/demux/snapshotter_test.go

+4
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ 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+
{"Cleanup", func() error { return uut.(snapshots.Cleaner).Cleanup(ctx) }},
7677
}
7778

7879
for _, test := range tests {
@@ -133,6 +134,7 @@ func TestReturnErrorWhenSnapshotterNotFound(t *testing.T) {
133134
var callback = func(c context.Context, i snapshots.Info) error { return nil }
134135
return uut.Walk(ctx, callback)
135136
}},
137+
{"Cleanup", func() error { return uut.(snapshots.Cleaner).Cleanup(ctx) }},
136138
}
137139

138140
for _, test := range tests {
@@ -169,6 +171,7 @@ func TestReturnErrorAfterProxyFunctionFailure(t *testing.T) {
169171
return uut.Walk(ctx, callback)
170172
}},
171173
{"Close", func() error { return uut.Close() }},
174+
{"Cleanup", func() error { return uut.(snapshots.Cleaner).Cleanup(ctx) }},
172175
}
173176

174177
for _, test := range tests {
@@ -207,6 +210,7 @@ func TestNoErrorIsReturnedOnSuccessfulProxyExecution(t *testing.T) {
207210
return uut.Walk(ctx, callback)
208211
}},
209212
{"Close", func() error { return uut.Close() }},
213+
{"Cleanup", func() error { return uut.(snapshots.Cleaner).Cleanup(ctx) }},
210214
}
211215

212216
for _, test := range tests {

0 commit comments

Comments
 (0)