Skip to content

Commit b550526

Browse files
dmcgowandmcgowan
committed
Use cleanup.Background instead of context.Background for cleanup
Use the cleanup context to re-use values from the original context Signed-off-by: Derek McGowan <[email protected]>
1 parent f606c4e commit b550526

8 files changed

Lines changed: 56 additions & 68 deletions

File tree

‎metadata/db.go‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import (
3232
"github.com/containerd/containerd/gc"
3333
"github.com/containerd/containerd/log"
3434
"github.com/containerd/containerd/namespaces"
35+
"github.com/containerd/containerd/pkg/cleanup"
3536
"github.com/containerd/containerd/snapshots"
3637
bolt "go.etcd.io/bbolt"
3738
)
@@ -423,7 +424,7 @@ func (m *DB) GarbageCollect(ctx context.Context) (gc.Stats, error) {
423424
log.G(ctx).WithField("snapshotter", snapshotterName).Debug("schedule snapshotter cleanup")
424425
go func(snapshotterName string) {
425426
st1 := time.Now()
426-
m.cleanupSnapshotter(snapshotterName)
427+
m.cleanupSnapshotter(ctx, snapshotterName)
427428

428429
sl.Lock()
429430
stats.SnapshotD[snapshotterName] = time.Since(st1)
@@ -440,7 +441,7 @@ func (m *DB) GarbageCollect(ctx context.Context) (gc.Stats, error) {
440441
log.G(ctx).Debug("schedule content cleanup")
441442
go func() {
442443
ct1 := time.Now()
443-
m.cleanupContent()
444+
m.cleanupContent(ctx)
444445
stats.ContentD = time.Since(ct1)
445446
wg.Done()
446447
}()
@@ -506,8 +507,8 @@ func (m *DB) getMarked(ctx context.Context, c *gcContext) (map[gc.Node]struct{},
506507
return marked, nil
507508
}
508509

509-
func (m *DB) cleanupSnapshotter(name string) (time.Duration, error) {
510-
ctx := context.Background()
510+
func (m *DB) cleanupSnapshotter(ctx context.Context, name string) (time.Duration, error) {
511+
ctx = cleanup.Background(ctx)
511512
sn, ok := m.ss[name]
512513
if !ok {
513514
return 0, nil
@@ -523,8 +524,8 @@ func (m *DB) cleanupSnapshotter(name string) (time.Duration, error) {
523524
return d, err
524525
}
525526

526-
func (m *DB) cleanupContent() (time.Duration, error) {
527-
ctx := context.Background()
527+
func (m *DB) cleanupContent(ctx context.Context) (time.Duration, error) {
528+
ctx = cleanup.Background(ctx)
528529
if m.cs == nil {
529530
return 0, nil
530531
}

‎pkg/unpack/unpacker.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import (
3535
"github.com/containerd/containerd/labels"
3636
"github.com/containerd/containerd/log"
3737
"github.com/containerd/containerd/mount"
38+
"github.com/containerd/containerd/pkg/cleanup"
3839
"github.com/containerd/containerd/pkg/kmutex"
3940
"github.com/containerd/containerd/platforms"
4041
"github.com/containerd/containerd/snapshots"
@@ -368,28 +369,28 @@ func (u *Unpacker) unpack(
368369

369370
select {
370371
case <-ctx.Done():
371-
abort(context.Background()) // Cleanup context
372+
cleanup.Do(ctx, abort)
372373
return ctx.Err()
373374
case err := <-fetchErr:
374375
if err != nil {
375-
abort(ctx)
376+
cleanup.Do(ctx, abort)
376377
return err
377378
}
378379
case <-fetchC[i-fetchOffset]:
379380
}
380381

381382
diff, err := a.Apply(ctx, desc, mounts, unpack.ApplyOpts...)
382383
if err != nil {
383-
abort(ctx)
384+
cleanup.Do(ctx, abort)
384385
return fmt.Errorf("failed to extract layer %s: %w", diffIDs[i], err)
385386
}
386387
if diff.Digest != diffIDs[i] {
387-
abort(ctx)
388+
cleanup.Do(ctx, abort)
388389
return fmt.Errorf("wrong diff id calculated on extraction %q", diffIDs[i])
389390
}
390391

391392
if err = sn.Commit(ctx, chainID, key, opts...); err != nil {
392-
abort(ctx)
393+
cleanup.Do(ctx, abort)
393394
if errdefs.IsAlreadyExists(err) {
394395
return nil
395396
}

‎rootfs/diff.go‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import (
2222

2323
"github.com/containerd/containerd/diff"
2424
"github.com/containerd/containerd/mount"
25-
"github.com/containerd/containerd/namespaces"
25+
"github.com/containerd/containerd/pkg/cleanup"
2626
"github.com/containerd/containerd/snapshots"
2727
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
2828
)
@@ -32,13 +32,6 @@ import (
3232
// the content creation and the provided snapshotter and mount differ are used
3333
// for calculating the diff. The descriptor for the layer diff is returned.
3434
func CreateDiff(ctx context.Context, snapshotID string, sn snapshots.Snapshotter, d diff.Comparer, opts ...diff.Opt) (ocispec.Descriptor, error) {
35-
// dctx is used to handle cleanup things just in case the param ctx
36-
// has been canceled, which causes that the defer cleanup fails.
37-
dctx := context.Background()
38-
if ns, ok := namespaces.Namespace(ctx); ok {
39-
dctx = namespaces.WithNamespace(dctx, ns)
40-
}
41-
4235
info, err := sn.Stat(ctx, snapshotID)
4336
if err != nil {
4437
return ocispec.Descriptor{}, err
@@ -49,7 +42,9 @@ func CreateDiff(ctx context.Context, snapshotID string, sn snapshots.Snapshotter
4942
if err != nil {
5043
return ocispec.Descriptor{}, err
5144
}
52-
defer sn.Remove(dctx, lowerKey)
45+
defer cleanup.Do(ctx, func(ctx context.Context) {
46+
sn.Remove(ctx, lowerKey)
47+
})
5348

5449
var upper []mount.Mount
5550
if info.Kind == snapshots.KindActive {
@@ -63,7 +58,9 @@ func CreateDiff(ctx context.Context, snapshotID string, sn snapshots.Snapshotter
6358
if err != nil {
6459
return ocispec.Descriptor{}, err
6560
}
66-
defer sn.Remove(dctx, upperKey)
61+
defer cleanup.Do(ctx, func(ctx context.Context) {
62+
sn.Remove(ctx, upperKey)
63+
})
6764
}
6865

6966
return d.Compare(ctx, lower, upper, opts...)

‎runtime/v1/linux/bundle.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ func (b *bundle) Delete() error {
183183
if err2 == nil {
184184
return err
185185
}
186-
return fmt.Errorf("Failed to remove both bundle and workdir locations: %v: %w", err2, err)
186+
return fmt.Errorf("failed to remove both bundle and workdir locations: %v: %w", err2, err)
187187
}
188188

189189
func (b *bundle) legacyShimAddress(namespace string) string {

‎runtime/v1/linux/runtime.go‎

Lines changed: 19 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import (
3737
"github.com/containerd/containerd/metadata"
3838
"github.com/containerd/containerd/mount"
3939
"github.com/containerd/containerd/namespaces"
40+
"github.com/containerd/containerd/pkg/cleanup"
4041
"github.com/containerd/containerd/pkg/process"
4142
"github.com/containerd/containerd/platforms"
4243
"github.com/containerd/containerd/plugin"
@@ -165,6 +166,10 @@ func (r *Runtime) Create(ctx context.Context, id string, opts runtime.CreateOpts
165166
if err != nil {
166167
return nil, err
167168
}
169+
ctx = log.WithLogger(ctx, log.G(ctx).WithError(err).WithFields(logrus.Fields{
170+
"id": id,
171+
"namespace": namespace,
172+
}))
168173

169174
if err := identifiers.Validate(id); err != nil {
170175
return nil, fmt.Errorf("invalid task id: %w", err)
@@ -206,11 +211,8 @@ func (r *Runtime) Create(ctx context.Context, id string, opts runtime.CreateOpts
206211
return
207212
}
208213

209-
if err = r.cleanupAfterDeadShim(context.Background(), bundle, namespace, id); err != nil {
210-
log.G(ctx).WithError(err).WithFields(logrus.Fields{
211-
"id": id,
212-
"namespace": namespace,
213-
}).Warn("failed to clean up after killed shim")
214+
if err = r.cleanupAfterDeadShim(cleanup.Background(ctx), bundle, namespace, id); err != nil {
215+
log.G(ctx).WithError(err).Warn("failed to clean up after killed shim")
214216
}
215217
}
216218
shimopt = ShimRemote(r.config, r.address, cgroup, exitHandler)
@@ -222,8 +224,7 @@ func (r *Runtime) Create(ctx context.Context, id string, opts runtime.CreateOpts
222224
}
223225
defer func() {
224226
if err != nil {
225-
deferCtx, deferCancel := context.WithTimeout(
226-
namespaces.WithNamespace(context.TODO(), namespace), cleanupTimeout)
227+
deferCtx, deferCancel := context.WithTimeout(cleanup.Background(ctx), cleanupTimeout)
227228
defer deferCancel()
228229
if kerr := s.KillShim(deferCtx); kerr != nil {
229230
log.G(ctx).WithError(kerr).Error("failed to kill shim")
@@ -359,6 +360,11 @@ func (r *Runtime) loadTasks(ctx context.Context, ns string) ([]*Task, error) {
359360
filepath.Join(r.root, ns, id),
360361
)
361362
ctx = namespaces.WithNamespace(ctx, ns)
363+
ctx = log.WithLogger(ctx, log.G(ctx).WithError(err).WithFields(logrus.Fields{
364+
"id": id,
365+
"namespace": ns,
366+
}))
367+
362368
pid, _ := runc.ReadPidFile(filepath.Join(bundle.path, process.InitPidFile))
363369
shimExit := make(chan struct{})
364370
s, err := bundle.NewShimClient(ctx, ns, ShimConnect(r.config, func() {
@@ -374,10 +380,7 @@ func (r *Runtime) loadTasks(ctx context.Context, ns string) ([]*Task, error) {
374380
}
375381
}), nil)
376382
if err != nil {
377-
log.G(ctx).WithError(err).WithFields(logrus.Fields{
378-
"id": id,
379-
"namespace": ns,
380-
}).Error("connecting to shim")
383+
log.G(ctx).WithError(err).Error("connecting to shim")
381384
err := r.cleanupAfterDeadShim(ctx, bundle, ns, id)
382385
if err != nil {
383386
log.G(ctx).WithError(err).WithField("bundle", bundle.path).
@@ -402,11 +405,8 @@ func (r *Runtime) loadTasks(ctx context.Context, ns string) ([]*Task, error) {
402405
}
403406
shimStdoutLog, err := v1.OpenShimStdoutLog(ctx, logDirPath)
404407
if err != nil {
405-
log.G(ctx).WithError(err).WithFields(logrus.Fields{
406-
"id": id,
407-
"namespace": ns,
408-
"logDirPath": logDirPath,
409-
}).Error("opening shim stdout log pipe")
408+
log.G(ctx).WithError(err).WithField("logDirPath", logDirPath).
409+
Error("opening shim stdout log pipe")
410410
continue
411411
}
412412
if r.config.ShimDebug {
@@ -417,11 +417,8 @@ func (r *Runtime) loadTasks(ctx context.Context, ns string) ([]*Task, error) {
417417

418418
shimStderrLog, err := v1.OpenShimStderrLog(ctx, logDirPath)
419419
if err != nil {
420-
log.G(ctx).WithError(err).WithFields(logrus.Fields{
421-
"id": id,
422-
"namespace": ns,
423-
"logDirPath": logDirPath,
424-
}).Error("opening shim stderr log pipe")
420+
log.G(ctx).WithError(err).WithField("logDirPath", logDirPath).
421+
Error("opening shim stderr log pipe")
425422
continue
426423
}
427424
if r.config.ShimDebug {
@@ -441,13 +438,9 @@ func (r *Runtime) loadTasks(ctx context.Context, ns string) ([]*Task, error) {
441438
}
442439

443440
func (r *Runtime) cleanupAfterDeadShim(ctx context.Context, bundle *bundle, ns, id string) error {
444-
log.G(ctx).WithFields(logrus.Fields{
445-
"id": id,
446-
"namespace": ns,
447-
}).Warn("cleaning up after shim dead")
441+
log.G(ctx).Warn("cleaning up after shim dead")
448442

449443
pid, _ := runc.ReadPidFile(filepath.Join(bundle.path, process.InitPidFile))
450-
ctx = namespaces.WithNamespace(ctx, ns)
451444
if err := r.terminate(ctx, bundle, ns, id); err != nil {
452445
if r.config.ShimDebug {
453446
return fmt.Errorf("failed to terminate task, leaving bundle for debugging: %w", err)

‎runtime/v2/manager.go‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
"github.com/containerd/containerd/log"
3232
"github.com/containerd/containerd/metadata"
3333
"github.com/containerd/containerd/namespaces"
34+
"github.com/containerd/containerd/pkg/cleanup"
3435
"github.com/containerd/containerd/pkg/timeout"
3536
"github.com/containerd/containerd/platforms"
3637
"github.com/containerd/containerd/plugin"
@@ -231,7 +232,7 @@ func (m *ShimManager) Start(ctx context.Context, id string, opts runtime.CreateO
231232
}
232233
defer func() {
233234
if retErr != nil {
234-
m.cleanupShim(shim)
235+
m.cleanupShim(ctx, shim)
235236
}
236237
}()
237238

@@ -247,6 +248,7 @@ func (m *ShimManager) startShim(ctx context.Context, bundle *Bundle, id string,
247248
if err != nil {
248249
return nil, err
249250
}
251+
ctx = log.WithLogger(ctx, log.G(ctx).WithField("namespace", ns))
250252

251253
topts := opts.TaskOptions
252254
if topts == nil || topts.GetValue() == nil {
@@ -267,7 +269,7 @@ func (m *ShimManager) startShim(ctx context.Context, bundle *Bundle, id string,
267269
shim, err := b.Start(ctx, protobuf.FromAny(topts), func() {
268270
log.G(ctx).WithField("id", id).Info("shim disconnected")
269271

270-
cleanupAfterDeadShim(context.Background(), id, ns, m.shims, m.events, b)
272+
cleanupAfterDeadShim(cleanup.Background(ctx), id, m.shims, m.events, b)
271273
// Remove self from the runtime task list. Even though the cleanupAfterDeadShim()
272274
// would publish taskExit event, but the shim.Delete() would always failed with ttrpc
273275
// disconnect and there is no chance to remove this dead task from runtime task lists.
@@ -360,8 +362,8 @@ func (m *ShimManager) resolveRuntimePath(runtime string) (string, error) {
360362
}
361363

362364
// cleanupShim attempts to properly delete and cleanup shim after error
363-
func (m *ShimManager) cleanupShim(shim *shim) {
364-
dctx, cancel := timeout.WithContext(context.Background(), cleanupTimeout)
365+
func (m *ShimManager) cleanupShim(ctx context.Context, shim *shim) {
366+
dctx, cancel := timeout.WithContext(cleanup.Background(ctx), cleanupTimeout)
365367
defer cancel()
366368

367369
_ = shim.Delete(dctx)
@@ -429,14 +431,14 @@ func (m *TaskManager) Create(ctx context.Context, taskID string, opts runtime.Cr
429431
// NOTE: ctx contains required namespace information.
430432
m.manager.shims.Delete(ctx, taskID)
431433

432-
dctx, cancel := timeout.WithContext(context.Background(), cleanupTimeout)
434+
dctx, cancel := timeout.WithContext(cleanup.Background(ctx), cleanupTimeout)
433435
defer cancel()
434436

435437
sandboxed := opts.SandboxID != ""
436438
_, errShim := shimTask.delete(dctx, sandboxed, func(context.Context, string) {})
437439
if errShim != nil {
438440
if errdefs.IsDeadlineExceeded(errShim) {
439-
dctx, cancel = timeout.WithContext(context.Background(), cleanupTimeout)
441+
dctx, cancel = timeout.WithContext(cleanup.Background(ctx), cleanupTimeout)
440442
defer cancel()
441443
}
442444

‎runtime/v2/shim.go‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,13 @@ import (
3232
"github.com/containerd/containerd/events/exchange"
3333
"github.com/containerd/containerd/identifiers"
3434
"github.com/containerd/containerd/log"
35-
"github.com/containerd/containerd/namespaces"
3635
"github.com/containerd/containerd/pkg/timeout"
3736
"github.com/containerd/containerd/protobuf"
3837
ptypes "github.com/containerd/containerd/protobuf/types"
3938
"github.com/containerd/containerd/runtime"
4039
client "github.com/containerd/containerd/runtime/v2/shim"
4140
"github.com/containerd/ttrpc"
4241
"github.com/hashicorp/go-multierror"
43-
"github.com/sirupsen/logrus"
4442
)
4543

4644
const (
@@ -131,21 +129,14 @@ func loadShim(ctx context.Context, bundle *Bundle, onClose func()) (_ ShimInstan
131129
return shim, nil
132130
}
133131

134-
func cleanupAfterDeadShim(ctx context.Context, id, ns string, rt *runtime.NSMap[ShimInstance], events *exchange.Exchange, binaryCall *binary) {
135-
ctx = namespaces.WithNamespace(ctx, ns)
132+
func cleanupAfterDeadShim(ctx context.Context, id string, rt *runtime.NSMap[ShimInstance], events *exchange.Exchange, binaryCall *binary) {
136133
ctx, cancel := timeout.WithContext(ctx, cleanupTimeout)
137134
defer cancel()
138135

139-
log.G(ctx).WithFields(logrus.Fields{
140-
"id": id,
141-
"namespace": ns,
142-
}).Warn("cleaning up after shim disconnected")
136+
log.G(ctx).WithField("id", id).Warn("cleaning up after shim disconnected")
143137
response, err := binaryCall.Delete(ctx)
144138
if err != nil {
145-
log.G(ctx).WithError(err).WithFields(logrus.Fields{
146-
"id": id,
147-
"namespace": ns,
148-
}).Warn("failed to clean up after shim disconnected")
139+
log.G(ctx).WithError(err).WithField("id", id).Warn("failed to clean up after shim disconnected")
149140
}
150141

151142
if _, err := rt.Get(ctx, id); err != nil {

‎runtime/v2/shim_load.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/containerd/containerd/log"
2727
"github.com/containerd/containerd/mount"
2828
"github.com/containerd/containerd/namespaces"
29+
"github.com/containerd/containerd/pkg/cleanup"
2930
)
3031

3132
func (m *ShimManager) loadExistingTasks(ctx context.Context) error {
@@ -60,6 +61,8 @@ func (m *ShimManager) loadShims(ctx context.Context) error {
6061
if err != nil {
6162
return err
6263
}
64+
ctx = log.WithLogger(ctx, log.G(ctx).WithField("namespace", ns))
65+
6366
shimDirs, err := os.ReadDir(filepath.Join(m.state, ns))
6467
if err != nil {
6568
return err
@@ -133,12 +136,12 @@ func (m *ShimManager) loadShims(ctx context.Context) error {
133136
instance, err := loadShim(ctx, bundle, func() {
134137
log.G(ctx).WithField("id", id).Info("shim disconnected")
135138

136-
cleanupAfterDeadShim(context.Background(), id, ns, m.shims, m.events, binaryCall)
139+
cleanupAfterDeadShim(cleanup.Background(ctx), id, m.shims, m.events, binaryCall)
137140
// Remove self from the runtime task list.
138141
m.shims.Delete(ctx, id)
139142
})
140143
if err != nil {
141-
cleanupAfterDeadShim(ctx, id, ns, m.shims, m.events, binaryCall)
144+
cleanupAfterDeadShim(ctx, id, m.shims, m.events, binaryCall)
142145
continue
143146
}
144147
shim := newShimTask(instance)

0 commit comments

Comments
 (0)