Skip to content

Commit 9574d0e

Browse files
committed
server/container_create: Allow for nil Process
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]>
1 parent d3d9de5 commit 9574d0e

File tree

1 file changed

+41
-39
lines changed

1 file changed

+41
-39
lines changed

server/container_create.go

Lines changed: 41 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -395,10 +395,6 @@ func buildOCIProcessArgs(containerKubeConfig *pb.ContainerConfig, ociConfig *v1.
395395
}
396396
}
397397

398-
if len(kubeCommands) == 0 && len(kubeArgs) == 0 {
399-
return nil, fmt.Errorf("no command specified")
400-
}
401-
402398
processArgs := append(kubeCommands, kubeArgs...)
403399

404400
logrus.Debugf("OCI process args %v", processArgs)
@@ -1148,39 +1144,52 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID string,
11481144
}
11491145

11501146
processArgs := []string{}
1151-
if containerImageConfig != nil {
1152-
processArgs, err := buildOCIProcessArgs(containerConfig, &containerImageConfig.Config)
1153-
if err != nil {
1154-
return nil, err
1155-
}
1147+
if containerImageConfig == nil {
1148+
processArgs, err = buildOCIProcessArgs(containerConfig, nil)
1149+
} else {
1150+
processArgs, err = buildOCIProcessArgs(containerConfig, &containerImageConfig.Config)
11561151
}
1157-
specgen.SetProcessArgs(processArgs)
1158-
1159-
envs := mergeEnvs(containerImageConfig, containerConfig.GetEnvs())
1160-
for _, e := range envs {
1161-
parts := strings.SplitN(e, "=", 2)
1162-
specgen.AddProcessEnv(parts[0], parts[1])
1152+
if err != nil {
1153+
return nil, err
11631154
}
1155+
if len(processArgs) == 0 {
1156+
specgen.Spec().Process = nil
1157+
} else {
1158+
specgen.SetProcessArgs(processArgs)
11641159

1165-
// Set working directory
1166-
// Pick it up from image config first and override if specified in CRI
1167-
containerCwd := "/"
1168-
if containerImageConfig != nil {
1169-
imageCwd := containerImageConfig.Config.WorkingDir
1170-
if imageCwd != "" {
1171-
containerCwd = imageCwd
1160+
envs := mergeEnvs(containerImageConfig, containerConfig.GetEnvs())
1161+
for _, e := range envs {
1162+
parts := strings.SplitN(e, "=", 2)
1163+
specgen.AddProcessEnv(parts[0], parts[1])
11721164
}
1173-
}
1174-
runtimeCwd := containerConfig.WorkingDir
1175-
if runtimeCwd != "" {
1176-
containerCwd = runtimeCwd
1177-
}
1178-
specgen.SetProcessCwd(containerCwd)
1179-
if err := setupWorkingDirectory(mountPoint, mountLabel, containerCwd); err != nil {
1180-
if err1 := s.StorageRuntimeServer().StopContainer(containerID); err1 != nil {
1181-
return nil, fmt.Errorf("can't umount container after cwd error %v: %v", err, err1)
1165+
1166+
// Set working directory
1167+
// Pick it up from image config first and override if specified in CRI
1168+
containerCwd := "/"
1169+
if containerImageConfig != nil {
1170+
imageCwd := containerImageConfig.Config.WorkingDir
1171+
if imageCwd != "" {
1172+
containerCwd = imageCwd
1173+
}
1174+
}
1175+
runtimeCwd := containerConfig.WorkingDir
1176+
if runtimeCwd != "" {
1177+
containerCwd = runtimeCwd
1178+
}
1179+
specgen.SetProcessCwd(containerCwd)
1180+
if err := setupWorkingDirectory(mountPoint, mountLabel, containerCwd); err != nil {
1181+
if err1 := s.StorageRuntimeServer().StopContainer(containerID); err1 != nil {
1182+
return nil, fmt.Errorf("can't umount container after cwd error %v: %v", err, err1)
1183+
}
1184+
return nil, err
1185+
}
1186+
1187+
// Setup user and groups
1188+
if linux != nil {
1189+
if err = setupContainerUser(&specgen, mountPoint, linux.GetSecurityContext(), containerImageConfig); err != nil {
1190+
return nil, err
1191+
}
11821192
}
1183-
return nil, err
11841193
}
11851194

11861195
var secretMounts []rspec.Mount
@@ -1213,13 +1222,6 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID string,
12131222
return nil, err
12141223
}
12151224

1216-
// Setup user and groups
1217-
if linux != nil {
1218-
if err = setupContainerUser(&specgen, mountPoint, linux.GetSecurityContext(), containerImageConfig); err != nil {
1219-
return nil, err
1220-
}
1221-
}
1222-
12231225
// Set up pids limit if pids cgroup is mounted
12241226
_, err = cgroups.FindCgroupMountpoint("pids")
12251227
if err == nil {

0 commit comments

Comments
 (0)