From f41bf4688c4dfa59e2b345336f0ea743b443be7a Mon Sep 17 00:00:00 2001
From: "W. Trevor King" <wking@tremily.us>
Date: Thu, 18 Jan 2018 16:12:44 -0800
Subject: [PATCH] server/sandbox_run: Drop PodInfraCommand fallback

Use buildOCIProcessArgs to calculate process.args considering
PauseCommand (if it was set) and the image's configured Entrypoint and
Cmd.  This improves the previous implementation, which had been
ignoring Entrypoint, despite the image-spec recommendations to
consider it [1].

There's no reason to assume the pause image contains a /pause command,
and it seems more sane to assume that the pause container has
configured Entrypoint and Cmd appropriately without our having to
repair anything.  Now that we're considering Entrypoint,
kubernetes/pause (which sets Entrypoint [2]) no longer needs a
PauseCommand "override".

I've dropped the podContainer.Config nil check because later code is
using it without a guard (e.g. podContainer.Config.Config.StopSignal)
and that hasn't caused problems.  The check
was added in c0333b10 (Integrate containers/storage, 2016-10-18,
#189), but there was too much going on for a motivational comment in
the commit message.

When processArgs is empty, I'm setting Process nil (which the runtime
spec allows [3]) following createSandboxContainer.  If 'start' is
called on those containers and errors (as the spec requires [4]), I
think that's an "image was broken" problem and not a "cri-o didn't
guess the right value" problem.  I've shifted the process.args code
down within RunPodSandbox where the possible Process nil can clobber
the Process object created by SetProcessSelinuxLabel.  I didn't see
any later configuration that looked like it would create Process
(which would require process.args [5]).

Down the road we might be able to use image-tools image -> runtime
config converter [6].  It's currently a private method with public
access restricted to full bundle unpacks.

[1]: https://github.com/opencontainers/image-spec/blob/v1.0.1/conversion.md#verbatim-fields
[2]: https://github.com/kubernetes/kubernetes/blob/v1.9.2/build/pause/Dockerfile#L18
[3]: https://github.com/opencontainers/runtime-spec/blame/v1.0.1/config.md#L145
[4]: https://github.com/opencontainers/runtime-spec/blame/v1.0.1/runtime.md#L119
[5]: https://github.com/opencontainers/runtime-spec/blame/v1.0.1/config.md#L157
[6]: https://github.com/opencontainers/image-tools/blob/v0.3.0/image/config.go#L68

Signed-off-by: W. Trevor King <wking@tremily.us>
---
 lib/sandbox/sandbox.go |  3 ---
 server/sandbox_run.go  | 23 ++++++++++++++---------
 2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/lib/sandbox/sandbox.go b/lib/sandbox/sandbox.go
index 7624b072..641e1734 100644
--- a/lib/sandbox/sandbox.go
+++ b/lib/sandbox/sandbox.go
@@ -168,9 +168,6 @@ const (
 	// NsRunDir is the default directory in which running network namespaces
 	// are stored
 	NsRunDir = "/var/run/netns"
-	// PodInfraCommand is the default command when starting a pod infrastructure
-	// container
-	PodInfraCommand = "/pause"
 )
 
 var (
diff --git a/server/sandbox_run.go b/server/sandbox_run.go
index 5ba007c2..951e113e 100644
--- a/server/sandbox_run.go
+++ b/server/sandbox_run.go
@@ -186,15 +186,6 @@ func (s *Server) RunPodSandbox(ctx context.Context, req *pb.RunPodSandboxRequest
 
 	// setup defaults for the pod sandbox
 	g.SetRootReadonly(true)
-	if s.config.PauseCommand == "" {
-		if podContainer.Config != nil {
-			g.SetProcessArgs(podContainer.Config.Config.Cmd)
-		} else {
-			g.SetProcessArgs([]string{sandbox.PodInfraCommand})
-		}
-	} else {
-		g.SetProcessArgs([]string{s.config.PauseCommand})
-	}
 
 	// set DNS options
 	if req.GetConfig().GetDnsConfig() != nil {
@@ -286,6 +277,20 @@ func (s *Server) RunPodSandbox(ctx context.Context, req *pb.RunPodSandboxRequest
 	g.SetProcessSelinuxLabel(processLabel)
 	g.SetLinuxMountLabel(mountLabel)
 
+	containerKubeConfig := &pb.ContainerConfig{}
+	if s.config.PauseCommand != "" {
+		containerKubeConfig.Command = []string{s.config.PauseCommand}
+	}
+	processArgs, err := buildOCIProcessArgs(containerKubeConfig, &podContainer.Config.Config)
+	if err != nil {
+		return nil, err
+	}
+	if len(processArgs) == 0 {
+		g.Spec().Process = nil
+	} else {
+		g.SetProcessArgs(processArgs)
+	}
+
 	// create shm mount for the pod containers.
 	var shmPath string
 	if securityContext.GetNamespaceOptions().GetHostIpc() {