runtime-v1: kill shim in cleanupAfterDeadShim

Ace-Tang · Ace-Tang · commit dfa51c92799b · 2019-04-22T14:29:40.000+08:00
1. kill shim in cleanupAfterDeadShim avoid shim leak
2. refactor cleanupAfterDeadShim, get pid from bundle
path instead of make pid as a parameter

Signed-off-by: Ace-Tang &lt;aceapril@126.com&gt;
diff --git a/runtime/v1/linux/runtime.go b/runtime/v1/linux/runtime.go
@@ -191,18 +191,13 @@ func (r *Runtime) Create(ctx context.Context, id string, opts runtime.CreateOpts
 		}
 		exitHandler := func() {
 			log.G(ctx).WithField("id", id).Info("shim reaped")
-			t, err := r.tasks.Get(ctx, id)
-			if err != nil {
+
+			if _, err := r.tasks.Get(ctx, id); err != nil {
 				// Task was never started or was already successfully deleted
 				return
 			}
-			lc := t.(*Task)
 
-			log.G(ctx).WithFields(logrus.Fields{
-				"id":        id,
-				"namespace": namespace,
-			}).Warn("cleaning up after killed shim")
-			if err = r.cleanupAfterDeadShim(context.Background(), bundle, namespace, id, lc.pid); err != nil {
+			if err = r.cleanupAfterDeadShim(context.Background(), bundle, namespace, id); err != nil {
 				log.G(ctx).WithError(err).WithFields(logrus.Fields{
 					"id":        id,
 					"namespace": namespace,
@@ -338,12 +333,12 @@ func (r *Runtime) loadTasks(ctx context.Context, ns string) ([]*Task, error) {
 		ctx = namespaces.WithNamespace(ctx, ns)
 		pid, _ := runc.ReadPidFile(filepath.Join(bundle.path, proc.InitPidFile))
 		s, err := bundle.NewShimClient(ctx, ns, ShimConnect(r.config, func() {
-			_, err := r.tasks.Get(ctx, id)
-			if err != nil {
+			if _, err := r.tasks.Get(ctx, id); err != nil {
 				// Task was never started or was already successfully deleted
 				return
 			}
-			if err := r.cleanupAfterDeadShim(ctx, bundle, ns, id, pid); err != nil {
+
+			if err := r.cleanupAfterDeadShim(ctx, bundle, ns, id); err != nil {
 				log.G(ctx).WithError(err).WithField("bundle", bundle.path).
 					Error("cleaning up after dead shim")
 			}
@@ -353,7 +348,7 @@ func (r *Runtime) loadTasks(ctx context.Context, ns string) ([]*Task, error) {
 				"id":        id,
 				"namespace": ns,
 			}).Error("connecting to shim")
-			err := r.cleanupAfterDeadShim(ctx, bundle, ns, id, pid)
+			err := r.cleanupAfterDeadShim(ctx, bundle, ns, id)
 			if err != nil {
 				log.G(ctx).WithError(err).WithField("bundle", bundle.path).
 					Error("cleaning up after dead shim")
@@ -395,7 +390,13 @@ func (r *Runtime) loadTasks(ctx context.Context, ns string) ([]*Task, error) {
 	return o, nil
 }
 
-func (r *Runtime) cleanupAfterDeadShim(ctx context.Context, bundle *bundle, ns, id string, pid int) error {
+func (r *Runtime) cleanupAfterDeadShim(ctx context.Context, bundle *bundle, ns, id string) error {
+	log.G(ctx).WithFields(logrus.Fields{
+		"id":        id,
+		"namespace": ns,
+	}).Warn("cleaning up after shim dead")
+
+	pid, _ := runc.ReadPidFile(filepath.Join(bundle.path, proc.InitPidFile))
 	ctx = namespaces.WithNamespace(ctx, ns)
 	if err := r.terminate(ctx, bundle, ns, id); err != nil {
 		if r.config.ShimDebug {
@@ -418,6 +419,10 @@ func (r *Runtime) cleanupAfterDeadShim(ctx context.Context, bundle *bundle, ns,
 	if err := bundle.Delete(); err != nil {
 		log.G(ctx).WithError(err).Error("delete bundle")
 	}
+	// kill shim
+	if shimPid, err := runc.ReadPidFile(filepath.Join(bundle.path, "shim.pid")); err == nil && shimPid > 0 {
+		unix.Kill(shimPid, unix.SIGKILL)
+	}
 
 	r.events.Publish(ctx, runtime.TaskDeleteEventTopic, &eventstypes.TaskDelete{
 		ContainerID: id,
diff --git a/runtime/v1/shim/client/client.go b/runtime/v1/shim/client/client.go
@@ -26,6 +26,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"sync"
 	"syscall"
@@ -110,7 +111,10 @@ func WithStart(binary, address, daemonAddress, cgroup string, debug bool, exitHa
 			"debug":   debug,
 		}).Infof("shim %s started", binary)
 
-		if err := writeAddress(filepath.Join(config.Path, "address"), address); err != nil {
+		if err := writeFile(filepath.Join(config.Path, "address"), address); err != nil {
+			return nil, nil, err
+		}
+		if err := writeFile(filepath.Join(config.Path, "shim.pid"), strconv.Itoa(cmd.Process.Pid)); err != nil {
 			return nil, nil, err
 		}
 		// set shim in cgroup if it is provided
@@ -172,8 +176,8 @@ func newCommand(binary, daemonAddress string, debug bool, config shim.Config, so
 	return cmd, nil
 }
 
-// writeAddress writes a address file atomically
-func writeAddress(path, address string) error {
+// writeFile writes a address file atomically
+func writeFile(path, address string) error {
 	path, err := filepath.Abs(path)
 	if err != nil {
 		return err
