namespace: Check if pid namespaces need to be shared

k8s provides a configuration for sharing PID namespace
among containers. In case of crio and cri plugin, an infra
container is started first. All following containers are
supposed to share the pid namespace of this container.

In case a non-empty pid namespace path is provided for a container,
we check for the above condition while creating a container
and pass this out to the kata agent in the CreatContainer
request as SandboxPidNs flag. We clear out the PID namespaces
in the configuration passed to the kata agent.

Fixes #343

Signed-off-by: Archana Shinde <archana.m.shinde@intel.com>

virtcontainers/container.go

@@ -15,6 +15,7 @@ import (
 	"syscall"
 	"time"
 
+	"github.com/kata-containers/runtime/virtcontainers/pkg/annotations"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/sys/unix"
@@ -657,6 +658,12 @@ func createContainer(sandbox *Sandbox, contConfig ContainerConfig) (c *Container
 	}
 	c.process = *process
 
+	// If this is a sandbox container, store the pid for sandbox
+	ann := c.GetAnnotations()
+	if ann[annotations.ContainerTypeKey] == string(PodSandbox) {
+		sandbox.setSandboxPid(c.process.Pid)
+	}
+
 	// Store the container process returned by the agent.
 	if err = c.storeProcess(); err != nil {
 		return

virtcontainers/kata_agent.go

@@ -506,9 +506,8 @@ func (k *kataAgent) startSandbox(sandbox *Sandbox) error {
 	}
 
 	req := &grpc.CreateSandboxRequest{
-		Hostname:     hostname,
-		Storages:     []*grpc.Storage{sharedVolume},
-		SandboxPidns: false,
+		Hostname: hostname,
+		Storages: []*grpc.Storage{sharedVolume},
 	}
 
 	_, err = k.sendReq(req)
@@ -777,16 +776,22 @@ func (k *kataAgent) createContainer(sandbox *Sandbox, c *Container) (p *Process,
 	// We need to give the OCI spec our absolute rootfs path in the guest.
 	grpcSpec.Root.Path = rootPath
 
+	sharedPidNs, err := k.handlePidNamespace(grpcSpec, sandbox)
+	if err != nil {
+		return nil, err
+	}
+
 	// We need to constraint the spec to make sure we're not passing
 	// irrelevant information to the agent.
 	constraintGRPCSpec(grpcSpec)
 
 	req := &grpc.CreateContainerRequest{
-		ContainerId: c.id,
-		ExecId:      c.id,
-		Storages:    ctrStorages,
-		Devices:     ctrDevices,
-		OCI:         grpcSpec,
+		ContainerId:  c.id,
+		ExecId:       c.id,
+		Storages:     ctrStorages,
+		Devices:      ctrDevices,
+		OCI:          grpcSpec,
+		SandboxPidns: sharedPidNs,
 	}
 
 	if _, err = k.sendReq(req); err != nil {
@@ -843,6 +848,52 @@ func (k *kataAgent) handleBlockVolumes(c *Container) []*grpc.Storage {
 	return volumeStorages
 }
 
+// handlePidNamespace checks if Pid namespace for a container needs to be shared with its sandbox
+// pid namespace. This function also modifies the grpc spec to remove the pid namespace
+// from the list of namespaces passed to the agent.
+func (k *kataAgent) handlePidNamespace(grpcSpec *grpc.Spec, sandbox *Sandbox) (bool, error) {
+	sharedPidNs := false
+	pidIndex := -1
+
+	for i, ns := range grpcSpec.Linux.Namespaces {
+		if ns.Type != string(specs.PIDNamespace) {
+			continue
+		}
+
+		pidIndex = i
+
+		if ns.Path == "" || sandbox.state.Pid == 0 {
+			break
+		}
+
+		pidNsPath := fmt.Sprintf("/proc/%d/ns/pid", sandbox.state.Pid)
+
+		//  Check if pid namespace path is the same as the sandbox
+		if ns.Path == pidNsPath {
+			sharedPidNs = true
+		} else {
+			ln, err := filepath.EvalSymlinks(ns.Path)
+			if err != nil {
+				return sharedPidNs, err
+			}
+
+			// We have arbitrary pid namespace path here.
+			if ln != pidNsPath {
+				return sharedPidNs, fmt.Errorf("Pid namespace path %s other than sandbox %s", ln, pidNsPath)
+			}
+			sharedPidNs = true
+		}
+
+		break
+	}
+
+	// Remove pid namespace.
+	if pidIndex >= 0 {
+		grpcSpec.Linux.Namespaces = append(grpcSpec.Linux.Namespaces[:pidIndex], grpcSpec.Linux.Namespaces[pidIndex+1:]...)
+	}
+	return sharedPidNs, nil
+}
+
 func (k *kataAgent) startContainer(sandbox *Sandbox, c *Container) error {
 	req := &grpc.StartContainerRequest{
 		ContainerId: c.id,

virtcontainers/kata_agent_test.go

@@ -455,3 +455,82 @@ func TestConstraintGRPCSpec(t *testing.T) {
 	assert.NotEmpty(g.Mounts[0].Source)
 	assert.NotEmpty(g.Mounts[0].Options)
 }
+
+func testIsPidNamespacePresent(grpcSpec *pb.Spec) bool {
+	for _, ns := range grpcSpec.Linux.Namespaces {
+		if ns.Type == string(specs.PIDNamespace) {
+			return true
+		}
+	}
+
+	return false
+}
+
+func TestHandlePidNamespace(t *testing.T) {
+	assert := assert.New(t)
+
+	g := &pb.Spec{
+		Linux: &pb.Linux{
+			Namespaces: []pb.LinuxNamespace{
+				{
+					Type: specs.NetworkNamespace,
+					Path: "/abc/123",
+				},
+				{
+					Type: specs.MountNamespace,
+					Path: "/abc/123",
+				},
+			},
+		},
+	}
+
+	sandbox := &Sandbox{}
+	sandbox.state.Pid = 0
+
+	k := kataAgent{}
+
+	sharedPid, err := k.handlePidNamespace(g, sandbox)
+	assert.Nil(err)
+	assert.False(sharedPid)
+	assert.False(testIsPidNamespacePresent(g))
+
+	pidNs := pb.LinuxNamespace{
+		Type: string(specs.PIDNamespace),
+		Path: "",
+	}
+
+	utsNs := pb.LinuxNamespace{
+		Type: specs.UTSNamespace,
+		Path: "",
+	}
+
+	g.Linux.Namespaces = append(g.Linux.Namespaces, pidNs)
+	g.Linux.Namespaces = append(g.Linux.Namespaces, utsNs)
+
+	sharedPid, err = k.handlePidNamespace(g, sandbox)
+	assert.Nil(err)
+	assert.False(sharedPid)
+	assert.False(testIsPidNamespacePresent(g))
+
+	sandbox.state.Pid = 112
+	pidNs = pb.LinuxNamespace{
+		Type: string(specs.PIDNamespace),
+		Path: "/proc/112/ns/pid",
+	}
+	g.Linux.Namespaces = append(g.Linux.Namespaces, pidNs)
+
+	sharedPid, err = k.handlePidNamespace(g, sandbox)
+	assert.Nil(err)
+	assert.True(sharedPid)
+	assert.False(testIsPidNamespacePresent(g))
+
+	// Arbitrary path
+	pidNs = pb.LinuxNamespace{
+		Type: string(specs.PIDNamespace),
+		Path: "/proc/234/ns/pid",
+	}
+	g.Linux.Namespaces = append(g.Linux.Namespaces, pidNs)
+
+	_, err = k.handlePidNamespace(g, sandbox)
+	assert.NotNil(err)
+}

virtcontainers/sandbox.go

@@ -70,6 +70,10 @@ type State struct {
 
 	// PCI slot at which the block device backing the container rootfs is attached.
 	RootfsPCIAddr string `json:"rootfsPCIAddr"`
+
+	// Pid is the process id of the sandbox container which is the first
+	// container to be started.
+	Pid int `json:"pid"`
 }
 
 // valid checks that the sandbox state is valid.
@@ -1274,6 +1278,15 @@ func (s *Sandbox) decrementSandboxBlockIndex() error {
 	return nil
 }
 
+// setSandboxPid sets the Pid of the the shim process belonging to the
+// sandbox container as the Pid of the sandbox.
+func (s *Sandbox) setSandboxPid(pid int) error {
+	s.state.Pid = pid
+
+	// update on-disk state
+	return s.storage.storeSandboxResource(s.id, stateFileType, s.state)
+}
+
 func (s *Sandbox) setContainersState(state stateString) error {
 	if state == "" {
 		return errNeedState