Merge pull request #1723 from lifupan/fixcleanup

fix the issue of hypervisor process is killed by kubelet

containerd-shim-v2/container.go

@@ -6,7 +6,6 @@
 package containerdshim
 
 import (
-	"sync"
 	"time"
 
 	"github.com/containerd/containerd/api/types/task"
@@ -31,7 +30,6 @@ type container struct {
 	stderr   string
 	bundle   string
 	cType    vc.ContainerType
-	mu       sync.Mutex
 	exit     uint32
 	status   task.Status
 	terminal bool

containerd-shim-v2/delete.go

@@ -17,7 +17,7 @@ import (
 )
 
 func deleteContainer(ctx context.Context, s *service, c *container) error {
-
+	if !c.cType.IsSandbox() {
 		status, err := s.sandbox.StatusContainer(c.id)
 		if err != nil {
 			return err
@@ -29,10 +29,10 @@ func deleteContainer(ctx context.Context, s *service, c *container) error {
 			}
 		}
 
-	_, err = s.sandbox.DeleteContainer(c.id)
+		if _, err = s.sandbox.DeleteContainer(c.id); err != nil {
-	if err != nil {
 			return err
 		}
+	}
 
 	// Run post-stop OCI hooks.
 	if err := katautils.PostStopHooks(ctx, *c.spec, s.sandbox.ID(), c.bundle); err != nil {

containerd-shim-v2/service.go

@@ -431,29 +431,12 @@ func (s *service) Delete(ctx context.Context, r *taskAPI.DeleteRequest) (_ *task
 	}
 
 	if r.ExecID == "" {
-		err = deleteContainer(ctx, s, c)
+		if err = deleteContainer(ctx, s, c); err != nil {
-		if err != nil {
 			return nil, err
 		}
 
-		// Take care of the use case where it is a sandbox.
-		// Right after the container representing the sandbox has
-		// been deleted, let's make sure we stop and delete the
-		// sandbox.
-		if c.cType.IsSandbox() {
-			if err = s.sandbox.Stop(); err != nil {
-				logrus.WithField("sandbox", s.sandbox.ID()).Error("failed to stop sandbox")
-				return nil, err
-			}
-
-			if err = s.sandbox.Delete(); err != nil {
-				logrus.WithField("sandbox", s.sandbox.ID()).Error("failed to delete sandbox")
-				return nil, err
-			}
-		}
-
 		s.send(&eventstypes.TaskDelete{
-			ContainerID: s.id,
+			ContainerID: c.id,
 			Pid:         s.pid,
 			ExitStatus:  c.exit,
 			ExitedAt:    c.exitTime,
@@ -677,6 +660,20 @@ func (s *service) Kill(ctx context.Context, r *taskAPI.KillRequest) (_ *ptypes.E
 		return nil, err
 	}
 
+	// According to CRI specs, kubelet will call StopPodSandbox()
+	// at least once before calling RemovePodSandbox, and this call
+	// is idempotent, and must not return an error if all relevant
+	// resources have already been reclaimed. And in that call it will
+	// send a SIGKILL signal first to try to stop the container, thus
+	// once the container has terminated, here should ignore this signal
+	// and return directly.
+	if signum == syscall.SIGKILL || signum == syscall.SIGTERM {
+		if c.status == task.StatusStopped {
+			logrus.WithField("sandbox", s.sandbox.ID()).WithField("Container", c.id).Debug("Container has already been stopped")
+			return empty, nil
+		}
+	}
+
 	processID := c.id
 	if r.ExecID != "" {
 		execs, err := c.getExec(r.ExecID)

containerd-shim-v2/wait.go

@@ -42,23 +42,41 @@ func wait(s *service, c *container, execID string) (int32, error) {
 	}
 
 	timeStamp := time.Now()
-	c.mu.Lock()
+
+	s.mu.Lock()
 	if execID == "" {
+		// Take care of the use case where it is a sandbox.
+		// Right after the container representing the sandbox has
+		// been deleted, let's make sure we stop and delete the
+		// sandbox.
+
+		if c.cType.IsSandbox() {
+			if err = s.sandbox.Stop(); err != nil {
+				logrus.WithField("sandbox", s.sandbox.ID()).Error("failed to stop sandbox")
+			}
+
+			if err = s.sandbox.Delete(); err != nil {
+				logrus.WithField("sandbox", s.sandbox.ID()).Error("failed to delete sandbox")
+			}
+		} else {
+			if _, err = s.sandbox.StopContainer(c.id); err != nil {
+				logrus.WithError(err).WithField("container", c.id).Warn("stop container failed")
+			}
+		}
 		c.status = task.StatusStopped
 		c.exit = uint32(ret)
 		c.exitTime = timeStamp
+
+		c.exitCh <- uint32(ret)
+
 	} else {
 		execs.status = task.StatusStopped
 		execs.exitCode = ret
 		execs.exitTime = timeStamp
-	}
-	c.mu.Unlock()
 
-	if execID == "" {
-		c.exitCh <- uint32(ret)
-	} else {
 		execs.exitCh <- uint32(ret)
 	}
+	s.mu.Unlock()
 
 	go cReap(s, int(ret), c.id, execID, timeStamp)