Browse Source

Merge pull request #46686 from thaJeztah/24.0_backport_c8d_push_defer_noncancellable_context

[24.0 backport] daemon/c8d: Use non cancellable context in defers
Sebastiaan van Stijn 1 year ago
parent
commit
c2e7c32b34

+ 19 - 14
daemon/containerd/image_builder.go

@@ -18,6 +18,7 @@ import (
 	"github.com/docker/docker/api/types/backend"
 	imagetypes "github.com/docker/docker/api/types/image"
 	"github.com/docker/docker/api/types/registry"
+	"github.com/docker/docker/internal/compatcontext"
 	registrypkg "github.com/docker/docker/registry"
 
 	// "github.com/docker/docker/api/types/container"
@@ -42,14 +43,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{
@@ -76,12 +77,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 +106,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 +174,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 :'(")
 	}
@@ -390,12 +391,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 {
@@ -436,7 +437,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: ociHistory,
 	}
 
@@ -472,11 +473,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(compatcontext.WithoutCancel(ctx)); err != nil {
+			logrus.WithError(err).Warn("failed to release lease created for create")
+		}
+	}()
 
 	commitManifestDesc, err := writeContentsForImage(ctx, i.snapshotter, i.client.ContentStore(), ociImgToCreate, layers)
 	if err != nil {

+ 7 - 2
daemon/containerd/image_commit.go

@@ -20,6 +20,7 @@ import (
 	"github.com/containerd/containerd/snapshots"
 	"github.com/docker/docker/api/types/backend"
 	"github.com/docker/docker/image"
+	"github.com/docker/docker/internal/compatcontext"
 	"github.com/docker/docker/pkg/archive"
 	"github.com/opencontainers/go-digest"
 	"github.com/opencontainers/image-spec/identity"
@@ -68,11 +69,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(compatcontext.WithoutCancel(ctx)); err != nil {
+			logrus.WithError(err).Warn("failed to release lease created for commit")
+		}
+	}()
 
 	diffLayerDesc, diffID, err := createDiff(ctx, cc.ContainerID, sn, cs, differ)
 	if err != nil {

+ 8 - 3
daemon/containerd/image_import.go

@@ -18,6 +18,7 @@ import (
 	"github.com/docker/docker/builder/dockerfile"
 	"github.com/docker/docker/errdefs"
 	"github.com/docker/docker/image"
+	"github.com/docker/docker/internal/compatcontext"
 	"github.com/docker/docker/pkg/archive"
 	"github.com/docker/docker/pkg/pools"
 	"github.com/google/uuid"
@@ -46,7 +47,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(compatcontext.WithoutCancel(ctx)); err != nil {
+			logger.WithError(err).Warn("failed to release lease created for import")
+		}
+	}()
 
 	if platform == nil {
 		def := platforms.DefaultSpec()
@@ -255,9 +260,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.

+ 3 - 3
daemon/containerd/image_push.go

@@ -17,6 +17,7 @@ import (
 	"github.com/docker/distribution/reference"
 	"github.com/docker/docker/api/types/registry"
 	"github.com/docker/docker/errdefs"
+	"github.com/docker/docker/internal/compatcontext"
 	"github.com/docker/docker/pkg/streamformatter"
 	"github.com/opencontainers/go-digest"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
@@ -46,9 +47,8 @@ func (i *ImageService) PushImage(ctx context.Context, targetRef reference.Named,
 		return err
 	}
 	defer func() {
-		err := release(leasedCtx)
-		if err != nil && !cerrdefs.IsNotFound(err) {
-			logrus.WithField("image", targetRef).WithError(err).Error("failed to delete lease created for push")
+		if err := release(compatcontext.WithoutCancel(leasedCtx)); err != nil {
+			logrus.WithField("image", targetRef).WithError(err).Warn("failed to release lease created for push")
 		}
 	}()
 

+ 89 - 0
internal/compatcontext/cancel.go

@@ -0,0 +1,89 @@
+//go:build !go1.21
+
+// Copyright (c) 2009 The Go Authors. All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//   - Redistributions of source code must retain the above copyright
+//
+// notice, this list of conditions and the following disclaimer.
+//   - Redistributions in binary form must reproduce the above
+//
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//   - Neither the name of Google Inc. nor the names of its
+//
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+//
+// Source: https://cs.opensource.google/go/go/+/refs/tags/go1.21.1:src/context/context.go
+// The only modifications to the original source were:
+// - replacing the usage of internal reflectlite with reflect
+// - replacing the usage of private value function with Value method call
+package compatcontext // import "github.com/docker/docker/internal/compatcontext"
+
+import (
+	"context"
+	"reflect"
+	"time"
+)
+
+// WithoutCancel returns a copy of parent that is not canceled when parent is canceled.
+// The returned context returns no Deadline or Err, and its Done channel is nil.
+// Calling [Cause] on the returned context returns nil.
+func WithoutCancel(parent context.Context) context.Context {
+	if parent == nil {
+		panic("cannot create context from nil parent")
+	}
+	return withoutCancelCtx{parent}
+}
+
+type withoutCancelCtx struct {
+	c context.Context
+}
+
+func (withoutCancelCtx) Deadline() (deadline time.Time, ok bool) {
+	return
+}
+
+func (withoutCancelCtx) Done() <-chan struct{} {
+	return nil
+}
+
+func (withoutCancelCtx) Err() error {
+	return nil
+}
+
+func (c withoutCancelCtx) Value(key any) any {
+	return c.c.Value(key)
+}
+
+func (c withoutCancelCtx) String() string {
+	return contextName(c.c) + ".WithoutCancel"
+}
+
+type stringer interface {
+	String() string
+}
+
+func contextName(c context.Context) string {
+	if s, ok := c.(stringer); ok {
+		return s.String()
+	}
+	return reflect.TypeOf(c).String()
+}

+ 9 - 0
internal/compatcontext/cancel_go121.go

@@ -0,0 +1,9 @@
+//go:build go1.21
+
+package compatcontext // import "github.com/docker/docker/internal/compatcontext"
+
+import "context"
+
+func WithoutCancel(ctx context.Context) context.Context {
+	return context.WithoutCancel(ctx)
+}