From de967051d488d415f7c2b66b4fd89cf2467536a9 Mon Sep 17 00:00:00 2001 From: Lantao Liu Date: Mon, 3 Dec 2018 17:08:57 -0800 Subject: [PATCH] Fix kill when shared pid namespace. Signed-off-by: Lantao Liu --- pkg/server/container_stop.go | 26 ++------------------------ pkg/server/events.go | 5 +++-- pkg/server/sandbox_stop.go | 13 +------------ 3 files changed, 6 insertions(+), 38 deletions(-) diff --git a/pkg/server/container_stop.go b/pkg/server/container_stop.go index ca1e430d9..198c9de2c 100644 --- a/pkg/server/container_stop.go +++ b/pkg/server/container_stop.go @@ -19,7 +19,6 @@ package server import ( "time" - "github.com/containerd/containerd" "github.com/containerd/containerd/errdefs" "github.com/docker/docker/pkg/signal" "github.com/pkg/errors" @@ -105,7 +104,7 @@ func (c *criService) stopContainer(ctx context.Context, container containerstore } logrus.Infof("Kill container %q", id) - if err = task.Kill(ctx, unix.SIGKILL, containerd.WithKillAll); err != nil && !errdefs.IsNotFound(err) { + if err = task.Kill(ctx, unix.SIGKILL); err != nil && !errdefs.IsNotFound(err) { return errors.Wrapf(err, "failed to kill container %q", id) } @@ -113,28 +112,7 @@ func (c *criService) stopContainer(ctx context.Context, container containerstore if err = c.waitContainerStop(ctx, container, killContainerTimeout); err == nil { return nil } - logrus.WithError(err).Errorf("An error occurs during waiting for container %q to be killed", id) - - // This is a fix for `runc`, and should not break other runtimes. With - // containerd.WithKillAll, `runc` will get all processes from the container - // cgroups, and kill them. However, sometimes the processes may be moved - // out from the container cgroup, e.g. users manually move them by mistake, - // or systemd.Delegate=true is not set. - // In these cases, we should try our best to do cleanup, kill the container - // without containerd.WithKillAll, so that runc can kill the container init - // process directly. - // NOTE(random-liu): If pid namespace is shared inside the pod, non-init processes - // of this container will be left running until the pause container is stopped. - logrus.Infof("Kill container %q init process", id) - if err = task.Kill(ctx, unix.SIGKILL); err != nil && !errdefs.IsNotFound(err) { - return errors.Wrapf(err, "failed to kill container %q init process", id) - } - - // Wait for a fixed timeout until container stop is observed by event monitor. - if err = c.waitContainerStop(ctx, container, killContainerTimeout); err == nil { - return nil - } - return errors.Wrapf(err, "an error occurs during waiting for container %q init process to be killed", id) + return errors.Wrapf(err, "an error occurs during waiting for container %q to be killed", id) } // waitContainerStop waits for container to be stopped until timeout exceeds or context is cancelled. diff --git a/pkg/server/events.go b/pkg/server/events.go index ca13f1eac..87a1ee17b 100644 --- a/pkg/server/events.go +++ b/pkg/server/events.go @@ -20,6 +20,7 @@ import ( "sync" "time" + "github.com/containerd/containerd" eventtypes "github.com/containerd/containerd/api/events" containerdio "github.com/containerd/containerd/cio" "github.com/containerd/containerd/errdefs" @@ -267,7 +268,7 @@ func handleContainerExit(ctx context.Context, e *eventtypes.TaskExit, cntr conta } } else { // TODO(random-liu): [P1] This may block the loop, we may want to spawn a worker - if _, err = task.Delete(ctx); err != nil { + if _, err = task.Delete(ctx, containerd.WithProcessKill); err != nil { if !errdefs.IsNotFound(err) { return errors.Wrap(err, "failed to stop container") } @@ -303,7 +304,7 @@ func handleSandboxExit(ctx context.Context, e *eventtypes.TaskExit, sb sandboxst } } else { // TODO(random-liu): [P1] This may block the loop, we may want to spawn a worker - if _, err = task.Delete(ctx); err != nil { + if _, err = task.Delete(ctx, containerd.WithProcessKill); err != nil { if !errdefs.IsNotFound(err) { return errors.Wrap(err, "failed to stop sandbox") } diff --git a/pkg/server/sandbox_stop.go b/pkg/server/sandbox_stop.go index 11c231ce5..31a16c047 100644 --- a/pkg/server/sandbox_stop.go +++ b/pkg/server/sandbox_stop.go @@ -19,7 +19,6 @@ package server import ( "time" - "github.com/containerd/containerd" "github.com/containerd/containerd/errdefs" cni "github.com/containerd/go-cni" "github.com/pkg/errors" @@ -105,18 +104,8 @@ func (c *criService) stopSandboxContainer(ctx context.Context, sandbox sandboxst } // Kill the sandbox container. - if err = task.Kill(ctx, unix.SIGKILL, containerd.WithKillAll); err != nil && !errdefs.IsNotFound(err) { - return errors.Wrap(err, "failed to kill sandbox container") - } - - if err = c.waitSandboxStop(ctx, sandbox, killContainerTimeout); err == nil { - return nil - } - logrus.WithError(err).Errorf("An error occurs during waiting for sandbox %q to be killed", sandbox.ID) - - // Kill the sandbox container init process. if err = task.Kill(ctx, unix.SIGKILL); err != nil && !errdefs.IsNotFound(err) { - return errors.Wrap(err, "failed to kill sandbox container init process") + return errors.Wrap(err, "failed to kill sandbox container") } return c.waitSandboxStop(ctx, sandbox, killContainerTimeout)