daemon/containerd: log errors when releasing leases

thaJeztah · thaJeztah · commit 28d201feb7cd · 2023-08-29T12:54:14.000+02:00
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 &lt;github@gone.nl&gt;
diff --git 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
@@ -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
@@ -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
@@ -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")
 		}
 	}()
 

Original file line number	Diff line number	Diff line change
`@@ -46,9 +46,8 @@ func (i *ImageService) PushImage(ctx context.Context, targetRef reference.Named,`
`46`	`46`	`return err`
`47`	`47`	`}`
`48`	`48`	`defer func() {`
`49`		`- err := release(leasedCtx)`
`50`		`- if err != nil && !cerrdefs.IsNotFound(err) {`
`51`		`- log.G(ctx).WithField("image", targetRef).WithError(err).Error("failed to delete lease created for push")`
	`49`	`+ if err := release(leasedCtx); err != nil {`
	`50`	`+ log.G(ctx).WithField("image", targetRef).WithError(err).Warn("failed to release lease created for push")`
`52`	`51`	`}`
`53`	`52`	`}()`
`54`	`53`