From a853bfa95f2c91821ae236c6ab2f0b90164bb890 Mon Sep 17 00:00:00 2001
From: fupan <lifupan@gmail.com>
Date: Thu, 29 Nov 2018 14:55:09 +0800
Subject: [PATCH] shimv2: fix the error of reaping qemu process mistakenly

For kata shimv2, it doesn't need to reap it's container processes
since all of them are running in VM, thus there is no need to
set it as a subreaper and also no need to take care of the SIGCHLD
singal.

So this commit will try to unset the subreaper and ignore the SIGCHLD
signal. Since ignoring the SIGCHLD signal will break the containerd
shim's events.Publisher publish method, thus here re-write a publish
function to publish the events to containerd.

Fixes: #939

Signed-off-by: fupan <lifupan@gmail.com>
---
 containerd-shim-v2/service.go | 43 +++++++++++++++++++++++++++++++++--
 containerd-shim-v2/utils.go   | 24 +++++++++++++++++++
 2 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/containerd-shim-v2/service.go b/containerd-shim-v2/service.go
index 375a670d7d..6083654909 100644
--- a/containerd-shim-v2/service.go
+++ b/containerd-shim-v2/service.go
@@ -8,9 +8,12 @@ package containerdshim
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"os"
 	sysexec "os/exec"
+	"os/signal"
 	"path/filepath"
+	"reflect"
 	"sync"
 	"syscall"
 	"time"
@@ -23,6 +26,7 @@ import (
 	cdruntime "github.com/containerd/containerd/runtime"
 	cdshim "github.com/containerd/containerd/runtime/v2/shim"
 	taskAPI "github.com/containerd/containerd/runtime/v2/task"
+	"github.com/containerd/containerd/sys"
 	"github.com/kata-containers/runtime/pkg/katautils"
 	vc "github.com/kata-containers/runtime/virtcontainers"
 	"github.com/kata-containers/runtime/virtcontainers/pkg/oci"
@@ -77,6 +81,15 @@ func New(ctx context.Context, id string, publisher events.Publisher) (cdshim.Shi
 
 	go s.forward(publisher)
 
+	// Disable the subreaper which has been set in containerd shim, for
+	// Kata the shimv2 doesn't need to reap the container processes since
+	// all of them are running in VM.
+	sys.SetSubreaper(0)
+
+	// Ignore the SIGCHLD signal, the reason is kata shimv2 doesn't need
+	// to reap it's children.
+	signal.Reset(unix.SIGCHLD)
+
 	return s, nil
 }
 
@@ -202,9 +215,35 @@ func (s *service) StartShim(ctx context.Context, id, containerdBinary, container
 	return address, nil
 }
 
-func (s *service) forward(publisher events.Publisher) {
+func (s *service) forward(l events.Publisher) {
+	var addr string
+	var binaryPath string
+
+	// Since containerd shim doesn't export
+	//    type remoteEventsPublisher struct {
+	//         address              string
+	//         containerdBinaryPath string
+	//    }
+	// thus it's needed to get the struct's
+	// fields value from an interface value
+	// using reflection.
+	elem := reflect.ValueOf(l).Elem()
+	typeOfT := elem.Type()
+
+	for i := 0; i < elem.NumField(); i++ {
+		f := elem.Field(i)
+		switch typeOfT.Field(i).Name {
+		case "address":
+			addr = fmt.Sprint(f)
+		case "containerdBinaryPath":
+			binaryPath = fmt.Sprint(f)
+		default:
+			logrus.Errorf("cannot find the field %s", typeOfT.Field(i).Name)
+		}
+	}
+
 	for e := range s.events {
-		if err := publisher.Publish(s.context, getTopic(s.context, e), e); err != nil {
+		if err := publish(s.context, binaryPath, addr, getTopic(s.context, e), e); err != nil {
 			logrus.WithError(err).Error("post event")
 		}
 	}
diff --git a/containerd-shim-v2/utils.go b/containerd-shim-v2/utils.go
index c768cf131f..34e11fdd48 100644
--- a/containerd-shim-v2/utils.go
+++ b/containerd-shim-v2/utils.go
@@ -7,15 +7,20 @@
 package containerdshim
 
 import (
+	"bytes"
 	"context"
 	"fmt"
 	"os"
+	sysexec "os/exec"
 	"path/filepath"
 	"syscall"
 	"time"
 
+	"github.com/containerd/containerd/events"
 	"github.com/containerd/containerd/mount"
+	"github.com/containerd/containerd/namespaces"
 	cdshim "github.com/containerd/containerd/runtime/v2/shim"
+	"github.com/containerd/typeurl"
 	"github.com/kata-containers/runtime/pkg/katautils"
 	vc "github.com/kata-containers/runtime/virtcontainers"
 	"github.com/kata-containers/runtime/virtcontainers/pkg/oci"
@@ -167,3 +172,22 @@ func removeNamespace(s *oci.CompatOCISpec, nsType specs.LinuxNamespaceType) {
 		}
 	}
 }
+
+func publish(ctx context.Context, binaryPath, address, topic string, event events.Event) error {
+	ns, _ := namespaces.Namespace(ctx)
+	encoded, err := typeurl.MarshalAny(event)
+	if err != nil {
+		return err
+	}
+	data, err := encoded.Marshal()
+	if err != nil {
+		return err
+	}
+	cmd := sysexec.CommandContext(ctx, binaryPath, "--address", address, "publish", "--topic", topic, "--namespace", ns)
+	cmd.Stdin = bytes.NewReader(data)
+	err = cmd.Run()
+	if err != nil {
+		logrus.WithError(err).Errorf("failed to publish event")
+	}
+	return err
+}