Skip to content

Commit

Permalink
server/container_create: Allow for nil Process
Browse files Browse the repository at this point in the history
OCI runtime callers (like CRI-O) are allowed to leave process unset
[1] for containers that they do not intend to 'start'.  When we don't
have any process.args, we *must* leave process unset (because
process.args is required [2]).  My personal preference would have been
to have both process and process.args optional [3], which would have
allowed for these settings to be decoupled, but that's not where the
spec ended up.

When we have no args and are clearing Process, we need to ensure that
we don't re-create an args-less structure later on by populating
process.user or similar.  This commit collects later process-creating
calls (e.g. setupContainerUser, which populates process.user) into the
"we have some args" branch.

This commit leaves earlier process-creating calls
(e.g. SetProcessTerminal) where they were.  Anything they do inside
Process will be clobbered later if we nil it, but that's fine.

[1]: https://github.com/opencontainers/runtime-spec/blame/v1.0.1/config.md#L145
[2]: https://github.com/opencontainers/runtime-spec/blame/v1.0.1/config.md#L157
[3]: opencontainers/runtime-spec#701 (comment)

Signed-off-by: W. Trevor King <[email protected]>
  • Loading branch information
wking committed Jan 24, 2018
1 parent c93ca85 commit 375ddf4
Showing 1 changed file with 41 additions and 39 deletions.
80 changes: 41 additions & 39 deletions server/container_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -395,10 +395,6 @@ func buildOCIProcessArgs(containerKubeConfig *pb.ContainerConfig, ociConfig *v1.
}
}

if len(kubeCommands) == 0 && len(kubeArgs) == 0 {
return nil, fmt.Errorf("no command specified")
}

processArgs := append(kubeCommands, kubeArgs...)

logrus.Debugf("OCI process args %v", processArgs)
Expand Down Expand Up @@ -1164,39 +1160,52 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID string,
}

processArgs := []string{}
if containerImageConfig != nil {
processArgs, err := buildOCIProcessArgs(containerConfig, &containerImageConfig.Config)
if err != nil {
return nil, err
}
if containerImageConfig == nil {
processArgs, err = buildOCIProcessArgs(containerConfig, nil)
} else {
processArgs, err = buildOCIProcessArgs(containerConfig, &containerImageConfig.Config)
}
specgen.SetProcessArgs(processArgs)

envs := mergeEnvs(containerImageConfig, containerConfig.GetEnvs())
for _, e := range envs {
parts := strings.SplitN(e, "=", 2)
specgen.AddProcessEnv(parts[0], parts[1])
if err != nil {
return nil, err
}
if len(processArgs) == 0 {
specgen.Spec().Process = nil
} else {
specgen.SetProcessArgs(processArgs)

// Set working directory
// Pick it up from image config first and override if specified in CRI
containerCwd := "/"
if containerImageConfig != nil {
imageCwd := containerImageConfig.Config.WorkingDir
if imageCwd != "" {
containerCwd = imageCwd
envs := mergeEnvs(containerImageConfig, containerConfig.GetEnvs())
for _, e := range envs {
parts := strings.SplitN(e, "=", 2)
specgen.AddProcessEnv(parts[0], parts[1])
}
}
runtimeCwd := containerConfig.WorkingDir
if runtimeCwd != "" {
containerCwd = runtimeCwd
}
specgen.SetProcessCwd(containerCwd)
if err := setupWorkingDirectory(mountPoint, mountLabel, containerCwd); err != nil {
if err1 := s.StorageRuntimeServer().StopContainer(containerID); err1 != nil {
return nil, fmt.Errorf("can't umount container after cwd error %v: %v", err, err1)

// Set working directory
// Pick it up from image config first and override if specified in CRI
containerCwd := "/"
if containerImageConfig != nil {
imageCwd := containerImageConfig.Config.WorkingDir
if imageCwd != "" {
containerCwd = imageCwd
}
}
runtimeCwd := containerConfig.WorkingDir
if runtimeCwd != "" {
containerCwd = runtimeCwd
}
specgen.SetProcessCwd(containerCwd)
if err := setupWorkingDirectory(mountPoint, mountLabel, containerCwd); err != nil {
if err1 := s.StorageRuntimeServer().StopContainer(containerID); err1 != nil {
return nil, fmt.Errorf("can't umount container after cwd error %v: %v", err, err1)
}
return nil, err
}

// Setup user and groups
if linux != nil {
if err = setupContainerUser(&specgen, mountPoint, linux.GetSecurityContext(), containerImageConfig); err != nil {
return nil, err
}
}
return nil, err
}

var secretMounts []rspec.Mount
Expand Down Expand Up @@ -1229,13 +1238,6 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID string,
return nil, err
}

// Setup user and groups
if linux != nil {
if err = setupContainerUser(&specgen, mountPoint, linux.GetSecurityContext(), containerImageConfig); err != nil {
return nil, err
}
}

// Set up pids limit if pids cgroup is mounted
_, err = cgroups.FindCgroupMountpoint("pids")
if err == nil {
Expand Down

0 comments on commit 375ddf4

Please sign in to comment.