From f01c6d73a6f09c14b6d3668704576702feb58c09 Mon Sep 17 00:00:00 2001 From: Lantao Liu Date: Wed, 7 Mar 2018 07:05:27 +0000 Subject: [PATCH] Fix cleanup context. Signed-off-by: Lantao Liu --- pkg/containerd/importer/importer.go | 12 ++++++-- pkg/containerd/util/util.go | 35 ++++++++++++++++++++++++ pkg/server/container_create.go | 5 +++- pkg/server/container_execsync.go | 5 +++- pkg/server/container_start.go | 5 +++- pkg/server/container_update_resources.go | 5 +++- pkg/server/sandbox_run.go | 9 ++++-- 7 files changed, 67 insertions(+), 9 deletions(-) create mode 100644 pkg/containerd/util/util.go diff --git a/pkg/containerd/importer/importer.go b/pkg/containerd/importer/importer.go index 888a2b44f..fd6d97288 100644 --- a/pkg/containerd/importer/importer.go +++ b/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 { - log.G(ctx).WithError(err).Errorf("Failed to remove image %q", ref) - } + func() { + 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 { diff --git a/pkg/containerd/util/util.go b/pkg/containerd/util/util.go new file mode 100644 index 000000000..70f5f2edf --- /dev/null +++ b/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) +} diff --git a/pkg/server/container_create.go b/pkg/server/container_create.go index dee76a55d..174188a84 100644 --- a/pkg/server/container_create.go +++ b/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) } } diff --git a/pkg/server/container_execsync.go b/pkg/server/container_execsync.go index e38adc19c..6893a52b5 100644 --- a/pkg/server/container_execsync.go +++ b/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) } }() diff --git a/pkg/server/container_start.go b/pkg/server/container_start.go index 32fb2963a..b8acb789a 100644 --- a/pkg/server/container_start.go +++ b/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) } } diff --git a/pkg/server/container_update_resources.go b/pkg/server/container_update_resources.go index 261ff84f7..1b2841629 100644 --- a/pkg/server/container_update_resources.go +++ b/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) } } diff --git a/pkg/server/sandbox_run.go b/pkg/server/sandbox_run.go index 2e08d684d..7fe3af0d9 100644 --- a/pkg/server/sandbox_run.go +++ b/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) } }