Merge pull request #648 from Random-Liu/fix-context

Fix cleanup context.

pkg/containerd/importer/importer.go

@@ -36,6 +36,7 @@ import (
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
 
+	ctrdutil "github.com/containerd/cri-containerd/pkg/containerd/util"
 	"github.com/containerd/cri-containerd/pkg/util"
 )
 
@@ -80,6 +81,7 @@ func Import(ctx context.Context, client *containerd.Client, reader io.Reader) (_
 	if err != nil {
 		return nil, err
 	}
+	// TODO(random-liu): Fix this after containerd client is fixed (containerd/containerd#2193)
 	defer done() // nolint: errcheck
 
 	cs := client.ContentStore()
@@ -134,9 +136,13 @@ func Import(ctx context.Context, client *containerd.Client, reader io.Reader) (_
 		// TODO(random-liu): Consider whether we should keep images already imported
 		// even when there is an error.
 		for _, ref := range refs {
-			if err := is.Delete(ctx, ref); err != nil {
+			func() {
-				log.G(ctx).WithError(err).Errorf("Failed to remove image %q", ref)
+				deferCtx, deferCancel := ctrdutil.DeferContext()
-			}
+				defer deferCancel()
+				if err := is.Delete(deferCtx, ref); err != nil {
+					log.G(ctx).WithError(err).Errorf("Failed to remove image %q", ref)
+				}
+			}()
 		}
 	}()
 	for _, mfst := range mfsts {

pkg/containerd/util/util.go

@@ -0,0 +1,35 @@
+/*
+Copyright 2018 The containerd Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package util
+
+import (
+	"time"
+
+	"golang.org/x/net/context"
+)
+
+// deferCleanupTimeout is the default timeout for containerd cleanup operations
+// in defer.
+const deferCleanupTimeout = 1 * time.Minute
+
+// DeferContext returns a context for containerd cleanup operations in defer.
+// A default timeout is applied to avoid cleanup operation pending forever.
+// TODO(random-liu): Add namespace after local services are used.
+// (containerd/containerd#2183)
+func DeferContext() (context.Context, context.CancelFunc) {
+	return context.WithTimeout(context.Background(), deferCleanupTimeout)
+}

pkg/server/container_create.go

@@ -47,6 +47,7 @@ import (
 
 	"github.com/containerd/cri-containerd/pkg/annotations"
 	customopts "github.com/containerd/cri-containerd/pkg/containerd/opts"
+	ctrdutil "github.com/containerd/cri-containerd/pkg/containerd/util"
 	cio "github.com/containerd/cri-containerd/pkg/server/io"
 	containerstore "github.com/containerd/cri-containerd/pkg/store/container"
 	"github.com/containerd/cri-containerd/pkg/util"
@@ -240,7 +241,9 @@ func (c *criContainerdService) CreateContainer(ctx context.Context, r *runtime.C
 	}
 	defer func() {
 		if retErr != nil {
-			if err := cntr.Delete(ctx, containerd.WithSnapshotCleanup); err != nil {
+			deferCtx, deferCancel := ctrdutil.DeferContext()
+			defer deferCancel()
+			if err := cntr.Delete(deferCtx, containerd.WithSnapshotCleanup); err != nil {
 				logrus.WithError(err).Errorf("Failed to delete containerd container %q", id)
 			}
 		}

pkg/server/container_execsync.go

@@ -31,6 +31,7 @@ import (
 	"k8s.io/client-go/tools/remotecommand"
 	runtime "k8s.io/kubernetes/pkg/kubelet/apis/cri/runtime/v1alpha2"
 
+	ctrdutil "github.com/containerd/cri-containerd/pkg/containerd/util"
 	cioutil "github.com/containerd/cri-containerd/pkg/ioutil"
 	cio "github.com/containerd/cri-containerd/pkg/server/io"
 	"github.com/containerd/cri-containerd/pkg/util"
@@ -128,7 +129,9 @@ func (c *criContainerdService) execInContainer(ctx context.Context, id string, o
 		return nil, fmt.Errorf("failed to create exec %q: %v", execID, err)
 	}
 	defer func() {
-		if _, err := process.Delete(ctx); err != nil {
+		deferCtx, deferCancel := ctrdutil.DeferContext()
+		defer deferCancel()
+		if _, err := process.Delete(deferCtx); err != nil {
 			logrus.WithError(err).Errorf("Failed to delete exec process %q for container %q", execID, id)
 		}
 	}()

pkg/server/container_start.go

@@ -28,6 +28,7 @@ import (
 	"golang.org/x/net/context"
 	runtime "k8s.io/kubernetes/pkg/kubelet/apis/cri/runtime/v1alpha2"
 
+	ctrdutil "github.com/containerd/cri-containerd/pkg/containerd/util"
 	cio "github.com/containerd/cri-containerd/pkg/server/io"
 	containerstore "github.com/containerd/cri-containerd/pkg/store/container"
 	sandboxstore "github.com/containerd/cri-containerd/pkg/store/sandbox"
@@ -121,8 +122,10 @@ func (c *criContainerdService) startContainer(ctx context.Context,
 	}
 	defer func() {
 		if retErr != nil {
+			deferCtx, deferCancel := ctrdutil.DeferContext()
+			defer deferCancel()
 			// It's possible that task is deleted by event monitor.
-			if _, err := task.Delete(ctx, containerd.WithProcessKill); err != nil && !errdefs.IsNotFound(err) {
+			if _, err := task.Delete(deferCtx, containerd.WithProcessKill); err != nil && !errdefs.IsNotFound(err) {
 				logrus.WithError(err).Errorf("Failed to delete containerd task %q", id)
 			}
 		}

pkg/server/container_update_resources.go

@@ -29,6 +29,7 @@ import (
 	"golang.org/x/net/context"
 	runtime "k8s.io/kubernetes/pkg/kubelet/apis/cri/runtime/v1alpha2"
 
+	ctrdutil "github.com/containerd/cri-containerd/pkg/containerd/util"
 	containerstore "github.com/containerd/cri-containerd/pkg/store/container"
 	"github.com/containerd/cri-containerd/pkg/util"
 )
@@ -78,8 +79,10 @@ func (c *criContainerdService) updateContainerResources(ctx context.Context,
 	}
 	defer func() {
 		if retErr != nil {
+			deferCtx, deferCancel := ctrdutil.DeferContext()
+			defer deferCancel()
 			// Reset spec on error.
-			if err := updateContainerSpec(ctx, cntr.Container, oldSpec); err != nil {
+			if err := updateContainerSpec(deferCtx, cntr.Container, oldSpec); err != nil {
 				logrus.WithError(err).Errorf("Failed to update spec %+v for container %q", oldSpec, id)
 			}
 		}

pkg/server/sandbox_run.go

@@ -37,6 +37,7 @@ import (
 
 	"github.com/containerd/cri-containerd/pkg/annotations"
 	customopts "github.com/containerd/cri-containerd/pkg/containerd/opts"
+	ctrdutil "github.com/containerd/cri-containerd/pkg/containerd/util"
 	"github.com/containerd/cri-containerd/pkg/log"
 	sandboxstore "github.com/containerd/cri-containerd/pkg/store/sandbox"
 	"github.com/containerd/cri-containerd/pkg/util"
@@ -193,7 +194,9 @@ func (c *criContainerdService) RunPodSandbox(ctx context.Context, r *runtime.Run
 	}
 	defer func() {
 		if retErr != nil {
-			if err := container.Delete(ctx, containerd.WithSnapshotCleanup); err != nil {
+			deferCtx, deferCancel := ctrdutil.DeferContext()
+			defer deferCancel()
+			if err := container.Delete(deferCtx, containerd.WithSnapshotCleanup); err != nil {
 				logrus.WithError(err).Errorf("Failed to delete containerd container %q", id)
 			}
 		}
@@ -288,9 +291,11 @@ func (c *criContainerdService) RunPodSandbox(ctx context.Context, r *runtime.Run
 		}
 		defer func() {
 			if retErr != nil {
+				deferCtx, deferCancel := ctrdutil.DeferContext()
+				defer deferCancel()
 				// Cleanup the sandbox container if an error is returned.
 				// It's possible that task is deleted by event monitor.
-				if _, err := task.Delete(ctx, containerd.WithProcessKill); err != nil && !errdefs.IsNotFound(err) {
+				if _, err := task.Delete(deferCtx, containerd.WithProcessKill); err != nil && !errdefs.IsNotFound(err) {
 					logrus.WithError(err).Errorf("Failed to delete sandbox container %q", id)
 				}
 			}