Skip to content

Commit e0e36f2

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 1e6638f commit e0e36f2

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)
@@ -1160,39 +1156,52 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID string,
11601156
}
11611157

11621158
processArgs := []string{}
1163-
if containerImageConfig != nil {
1164-
processArgs, err := buildOCIProcessArgs(containerConfig, &containerImageConfig.Config)
1165-
if err != nil {
1166-
return nil, err
1167-
}
1159+
if containerImageConfig == nil {
1160+
processArgs, err = buildOCIProcessArgs(containerConfig, nil)
1161+
} else {
1162+
processArgs, err = buildOCIProcessArgs(containerConfig, &containerImageConfig.Config)
11681163
}
1169-
specgen.SetProcessArgs(processArgs)
1170-
1171-
envs := mergeEnvs(containerImageConfig, containerConfig.GetEnvs())
1172-
for _, e := range envs {
1173-
parts := strings.SplitN(e, "=", 2)
1174-
specgen.AddProcessEnv(parts[0], parts[1])
1164+
if err != nil {
1165+
return nil, err
11751166
}
1167+
if len(processArgs) == 0 {
1168+
specgen.Spec().Process = nil
1169+
} else {
1170+
specgen.SetProcessArgs(processArgs)
11761171

1177-
// Set working directory
1178-
// Pick it up from image config first and override if specified in CRI
1179-
containerCwd := "/"
1180-
if containerImageConfig != nil {
1181-
imageCwd := containerImageConfig.Config.WorkingDir
1182-
if imageCwd != "" {
1183-
containerCwd = imageCwd
1172+
envs := mergeEnvs(containerImageConfig, containerConfig.GetEnvs())
1173+
for _, e := range envs {
1174+
parts := strings.SplitN(e, "=", 2)
1175+
specgen.AddProcessEnv(parts[0], parts[1])
11841176
}
1185-
}
1186-
runtimeCwd := containerConfig.WorkingDir
1187-
if runtimeCwd != "" {
1188-
containerCwd = runtimeCwd
1189-
}
1190-
specgen.SetProcessCwd(containerCwd)
1191-
if err := setupWorkingDirectory(mountPoint, mountLabel, containerCwd); err != nil {
1192-
if err1 := s.StorageRuntimeServer().StopContainer(containerID); err1 != nil {
1193-
return nil, fmt.Errorf("can't umount container after cwd error %v: %v", err, err1)
1177+
1178+
// Set working directory
1179+
// Pick it up from image config first and override if specified in CRI
1180+
containerCwd := "/"
1181+
if containerImageConfig != nil {
1182+
imageCwd := containerImageConfig.Config.WorkingDir
1183+
if imageCwd != "" {
1184+
containerCwd = imageCwd
1185+
}
1186+
}
1187+
runtimeCwd := containerConfig.WorkingDir
1188+
if runtimeCwd != "" {
1189+
containerCwd = runtimeCwd
1190+
}
1191+
specgen.SetProcessCwd(containerCwd)
1192+
if err := setupWorkingDirectory(mountPoint, mountLabel, containerCwd); err != nil {
1193+
if err1 := s.StorageRuntimeServer().StopContainer(containerID); err1 != nil {
1194+
return nil, fmt.Errorf("can't umount container after cwd error %v: %v", err, err1)
1195+
}
1196+
return nil, err
1197+
}
1198+
1199+
// Setup user and groups
1200+
if linux != nil {
1201+
if err = setupContainerUser(&specgen, mountPoint, linux.GetSecurityContext(), containerImageConfig); err != nil {
1202+
return nil, err
1203+
}
11941204
}
1195-
return nil, err
11961205
}
11971206

11981207
var secretMounts []rspec.Mount
@@ -1225,13 +1234,6 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID string,
12251234
return nil, err
12261235
}
12271236

1228-
// Setup user and groups
1229-
if linux != nil {
1230-
if err = setupContainerUser(&specgen, mountPoint, linux.GetSecurityContext(), containerImageConfig); err != nil {
1231-
return nil, err
1232-
}
1233-
}
1234-
12351237
// Set up pids limit if pids cgroup is mounted
12361238
_, err = cgroups.FindCgroupMountpoint("pids")
12371239
if err == nil {

0 commit comments

Comments
 (0)