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>
This commit is contained in:
W. Trevor King 2018-01-18 16:12:44 -08:00
parent 375ddf4b3a
commit f41bf4688c
2 changed files with 14 additions and 12 deletions

View file

@ -168,9 +168,6 @@ const (
// NsRunDir is the default directory in which running network namespaces // NsRunDir is the default directory in which running network namespaces
// are stored // are stored
NsRunDir = "/var/run/netns" NsRunDir = "/var/run/netns"
// PodInfraCommand is the default command when starting a pod infrastructure
// container
PodInfraCommand = "/pause"
) )
var ( var (

View file

@ -186,15 +186,6 @@ func (s *Server) RunPodSandbox(ctx context.Context, req *pb.RunPodSandboxRequest
// setup defaults for the pod sandbox // setup defaults for the pod sandbox
g.SetRootReadonly(true) 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 // set DNS options
if req.GetConfig().GetDnsConfig() != nil { if req.GetConfig().GetDnsConfig() != nil {
@ -286,6 +277,20 @@ func (s *Server) RunPodSandbox(ctx context.Context, req *pb.RunPodSandboxRequest
g.SetProcessSelinuxLabel(processLabel) g.SetProcessSelinuxLabel(processLabel)
g.SetLinuxMountLabel(mountLabel) 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. // create shm mount for the pod containers.
var shmPath string var shmPath string
if securityContext.GetNamespaceOptions().GetHostIpc() { if securityContext.GetNamespaceOptions().GetHostIpc() {