From 28d201feb7cde36b8cbef4ad810817144c4a8abb Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 24 Aug 2023 23:51:39 +0200 Subject: [PATCH 1/3] daemon/containerd: log errors when releasing leases Log a warning if we encounter an error when releasing leases. While it may not have direct consequences, failing to release the lease should be unexpected, so let's make them visible. Signed-off-by: Sebastiaan van Stijn --- daemon/containerd/image_builder.go | 8 ++++++-- daemon/containerd/image_commit.go | 8 ++++++-- daemon/containerd/image_import.go | 6 +++++- daemon/containerd/image_push.go | 5 ++--- 4 files changed, 19 insertions(+), 8 deletions(-) diff --git a/daemon/containerd/image_builder.go b/daemon/containerd/image_builder.go index 5fb10b27ad..c1a37d3662 100644 --- a/daemon/containerd/image_builder.go +++ b/daemon/containerd/image_builder.go @@ -460,11 +460,15 @@ func (i *ImageService) CreateImage(ctx context.Context, config []byte, parent st // necessary to prevent the contents from being GC'd // between writing them here and creating an image - ctx, done, err := i.client.WithLease(ctx, leases.WithRandomID(), leases.WithExpiration(1*time.Hour)) + ctx, release, err := i.client.WithLease(ctx, leases.WithRandomID(), leases.WithExpiration(1*time.Hour)) if err != nil { return nil, err } - defer done(ctx) + defer func() { + if err := release(ctx); err != nil { + log.G(ctx).WithError(err).Warn("failed to release lease created for create") + } + }() commitManifestDesc, err := writeContentsForImage(ctx, i.snapshotter, i.client.ContentStore(), ociImgToCreate, layers) if err != nil { diff --git a/daemon/containerd/image_commit.go b/daemon/containerd/image_commit.go index b2c7f3bd97..5bc4da7853 100644 --- a/daemon/containerd/image_commit.go +++ b/daemon/containerd/image_commit.go @@ -68,11 +68,15 @@ func (i *ImageService) CommitImage(ctx context.Context, cc backend.CommitConfig) ) // Don't gc me and clean the dirty data after 1 hour! - ctx, done, err := i.client.WithLease(ctx, leases.WithRandomID(), leases.WithExpiration(1*time.Hour)) + ctx, release, err := i.client.WithLease(ctx, leases.WithRandomID(), leases.WithExpiration(1*time.Hour)) if err != nil { return "", fmt.Errorf("failed to create lease for commit: %w", err) } - defer done(ctx) + defer func() { + if err := release(ctx); err != nil { + log.G(ctx).WithError(err).Warn("failed to release lease created for commit") + } + }() diffLayerDesc, diffID, err := createDiff(ctx, cc.ContainerID, sn, cs, differ) if err != nil { diff --git a/daemon/containerd/image_import.go b/daemon/containerd/image_import.go index 339c581820..12eb40a9d8 100644 --- a/daemon/containerd/image_import.go +++ b/daemon/containerd/image_import.go @@ -46,7 +46,11 @@ func (i *ImageService) ImportImage(ctx context.Context, ref reference.Named, pla if err != nil { return "", errdefs.System(err) } - defer release(ctx) + defer func() { + if err := release(ctx); err != nil { + logger.WithError(err).Warn("failed to release lease created for import") + } + }() if platform == nil { def := platforms.DefaultSpec() diff --git a/daemon/containerd/image_push.go b/daemon/containerd/image_push.go index feaa8356d8..24c37165c5 100644 --- a/daemon/containerd/image_push.go +++ b/daemon/containerd/image_push.go @@ -46,9 +46,8 @@ func (i *ImageService) PushImage(ctx context.Context, targetRef reference.Named, return err } defer func() { - err := release(leasedCtx) - if err != nil && !cerrdefs.IsNotFound(err) { - log.G(ctx).WithField("image", targetRef).WithError(err).Error("failed to delete lease created for push") + if err := release(leasedCtx); err != nil { + log.G(ctx).WithField("image", targetRef).WithError(err).Warn("failed to release lease created for push") } }() From aefbd49038800a5b9cc8bce0b001c9f3d423dbe3 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 25 Aug 2023 00:07:29 +0200 Subject: [PATCH 2/3] daemon/containerd: newROLayerForImage: remove unused args Also rename variables that collided with imports. Signed-off-by: Sebastiaan van Stijn --- daemon/containerd/image_builder.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/daemon/containerd/image_builder.go b/daemon/containerd/image_builder.go index c1a37d3662..1f371e4630 100644 --- a/daemon/containerd/image_builder.go +++ b/daemon/containerd/image_builder.go @@ -76,12 +76,12 @@ func (i *ImageService) GetImageAndReleasableLayer(ctx context.Context, refOrID s return nil, nil, system.ErrNotSupportedOperatingSystem } - layer, err := newROLayerForImage(ctx, &imgDesc, i, opts, refOrID, opts.Platform) + roLayer, err := newROLayerForImage(ctx, &imgDesc, i, opts.Platform) if err != nil { return nil, nil, err } - return img, layer, nil + return img, roLayer, nil } } @@ -105,12 +105,12 @@ func (i *ImageService) GetImageAndReleasableLayer(ctx context.Context, refOrID s return nil, nil, err } - layer, err := newROLayerForImage(ctx, imgDesc, i, opts, refOrID, opts.Platform) + roLayer, err := newROLayerForImage(ctx, imgDesc, i, opts.Platform) if err != nil { return nil, nil, err } - return img, layer, nil + return img, roLayer, nil } func (i *ImageService) pullForBuilder(ctx context.Context, name string, authConfigs map[string]registry.AuthConfig, output io.Writer, platform *ocispec.Platform) (*ocispec.Descriptor, error) { @@ -173,7 +173,7 @@ Please notify the image author to correct the configuration.`, return &imgDesc, err } -func newROLayerForImage(ctx context.Context, imgDesc *ocispec.Descriptor, i *ImageService, opts backend.GetImageAndLayerOptions, refOrID string, platform *ocispec.Platform) (builder.ROLayer, error) { +func newROLayerForImage(ctx context.Context, imgDesc *ocispec.Descriptor, i *ImageService, platform *ocispec.Platform) (builder.ROLayer, error) { if imgDesc == nil { return nil, fmt.Errorf("can't make an RO layer for a nil image :'(") } From e10eca3d1a57449b69fe1251f6fc85c0f76ac572 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 25 Aug 2023 01:02:30 +0200 Subject: [PATCH 3/3] daemon/containerd: rename some vars that collided with imports Signed-off-by: Sebastiaan van Stijn --- daemon/containerd/image_builder.go | 14 +++++++------- daemon/containerd/image_import.go | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/daemon/containerd/image_builder.go b/daemon/containerd/image_builder.go index 1f371e4630..958176e255 100644 --- a/daemon/containerd/image_builder.go +++ b/daemon/containerd/image_builder.go @@ -42,14 +42,14 @@ import ( // releasableLayer.Release() to prevent leaking of layers. func (i *ImageService) GetImageAndReleasableLayer(ctx context.Context, refOrID string, opts backend.GetImageAndLayerOptions) (builder.Image, builder.ROLayer, error) { if refOrID == "" { // from SCRATCH - os := runtime.GOOS + imgOS := runtime.GOOS if runtime.GOOS == "windows" { - os = "linux" + imgOS = "linux" } if opts.Platform != nil { - os = opts.Platform.OS + imgOS = opts.Platform.OS } - if !system.IsOSSupported(os) { + if !system.IsOSSupported(imgOS) { return nil, nil, system.ErrNotSupportedOperatingSystem } return nil, &rolayer{ @@ -390,12 +390,12 @@ func (i *ImageService) CreateImage(ctx context.Context, config []byte, parent st return nil, err } - rootfs := ocispec.RootFS{ + rootFS := ocispec.RootFS{ Type: imgToCreate.RootFS.Type, DiffIDs: []digest.Digest{}, } for _, diffId := range imgToCreate.RootFS.DiffIDs { - rootfs.DiffIDs = append(rootfs.DiffIDs, digest.Digest(diffId)) + rootFS.DiffIDs = append(rootFS.DiffIDs, digest.Digest(diffId)) } exposedPorts := make(map[string]struct{}, len(imgToCreate.Config.ExposedPorts)) for k, v := range imgToCreate.Config.ExposedPorts { @@ -424,7 +424,7 @@ func (i *ImageService) CreateImage(ctx context.Context, config []byte, parent st Labels: imgToCreate.Config.Labels, StopSignal: imgToCreate.Config.StopSignal, }, - RootFS: rootfs, + RootFS: rootFS, History: imgToCreate.History, } diff --git a/daemon/containerd/image_import.go b/daemon/containerd/image_import.go index 12eb40a9d8..7f9e55465d 100644 --- a/daemon/containerd/image_import.go +++ b/daemon/containerd/image_import.go @@ -259,9 +259,9 @@ func compressAndWriteBlob(ctx context.Context, cs content.Store, compression arc writeChan := make(chan digest.Digest) // Start copying the blob to the content store from the pipe. go func() { - digest, err := writeBlobAndReturnDigest(ctx, cs, mediaType, pr) + dgst, err := writeBlobAndReturnDigest(ctx, cs, mediaType, pr) pr.CloseWithError(err) - writeChan <- digest + writeChan <- dgst }() // Copy archive to the pipe and tee it to a digester.