From 03ee25d4eff162ca1c257fc849b5b516e3d491da Mon Sep 17 00:00:00 2001
From: Peng Tao <bergwolf@hyper.sh>
Date: Tue, 9 Apr 2019 01:14:23 -0700
Subject: [PATCH] agent: treat container as shared pidns whenever it has pidns
 path

Current approach cannot work for shimv2 as there is no kata-shim thus
sandbox.state.pid is always -1. Let's just simplify things by always
making a container share pidns if it has a pidns path.

Signed-off-by: Peng Tao <bergwolf@hyper.sh>
---
 virtcontainers/kata_agent.go      | 34 +++++++------------------------
 virtcontainers/kata_agent_test.go | 20 +++---------------
 2 files changed, 10 insertions(+), 44 deletions(-)

diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go
index 56a3caa661..f8bea7f767 100644
--- a/virtcontainers/kata_agent.go
+++ b/virtcontainers/kata_agent.go
@@ -1071,10 +1071,7 @@ 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
-	}
+	sharedPidNs := k.handlePidNamespace(grpcSpec, sandbox)
 
 	passSeccomp := !sandbox.config.DisableGuestSeccomp && sandbox.seccompSupported
 
@@ -1191,7 +1188,7 @@ func (k *kataAgent) handleBlockVolumes(c *Container) []*grpc.Storage {
 // 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) {
+func (k *kataAgent) handlePidNamespace(grpcSpec *grpc.Spec, sandbox *Sandbox) bool {
 	sharedPidNs := false
 	pidIndex := -1
 
@@ -1201,29 +1198,11 @@ func (k *kataAgent) handlePidNamespace(grpcSpec *grpc.Spec, sandbox *Sandbox) (b
 		}
 
 		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)
-			}
+		// host pidns path does not make sense in kata. Let's just align it with
+		// sandbox namespace whenever it is set.
+		if ns.Path != "" {
 			sharedPidNs = true
 		}
-
 		break
 	}
 
@@ -1231,7 +1210,8 @@ func (k *kataAgent) handlePidNamespace(grpcSpec *grpc.Spec, sandbox *Sandbox) (b
 	if pidIndex >= 0 {
 		grpcSpec.Linux.Namespaces = append(grpcSpec.Linux.Namespaces[:pidIndex], grpcSpec.Linux.Namespaces[pidIndex+1:]...)
 	}
-	return sharedPidNs, nil
+
+	return sharedPidNs
 }
 
 func (k *kataAgent) startContainer(sandbox *Sandbox, c *Container) error {
diff --git a/virtcontainers/kata_agent_test.go b/virtcontainers/kata_agent_test.go
index 788e090d0d..cc3b8a97b7 100644
--- a/virtcontainers/kata_agent_test.go
+++ b/virtcontainers/kata_agent_test.go
@@ -574,8 +574,7 @@ func TestHandlePidNamespace(t *testing.T) {
 
 	k := kataAgent{}
 
-	sharedPid, err := k.handlePidNamespace(g, sandbox)
-	assert.Nil(err)
+	sharedPid := k.handlePidNamespace(g, sandbox)
 	assert.False(sharedPid)
 	assert.False(testIsPidNamespacePresent(g))
 
@@ -592,32 +591,19 @@ func TestHandlePidNamespace(t *testing.T) {
 	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)
+	sharedPid = k.handlePidNamespace(g, sandbox)
 	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)
+	sharedPid = k.handlePidNamespace(g, sandbox)
 	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)
 }
 
 func TestAgentPathAPI(t *testing.T) {