Merge pull request #10594 from AkihiroSuda/cri-remove-disableCgroup
CRI: remove `disable_cgroup`
This commit is contained in:
		| @@ -462,6 +462,7 @@ The deprecated properties in [`config.toml`](./docs/cri/config.md) are shown in | ||||
| |`[plugins."io.containerd.grpc.v1.cri".containerd]`                    | `default_runtime`            | containerd v1.3     | containerd v2.0 ✅         | Use `default_runtime_name`                      | | ||||
| |`[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.*]`         | `runtime_engine`             | containerd v1.3     | containerd v2.0 ✅         | Use runtime v2                                  | | ||||
| |`[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.*]`         | `runtime_root`               | containerd v1.3     | containerd v2.0 ✅         | Use `options.Root`                              | | ||||
| |`[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.*]`         | `disable_cgroup`             | -                   | containerd v2.0 ✅         | Use [cgroup v2 delegation](https://rootlesscontaine.rs/getting-started/common/cgroup2/) | | ||||
| |`[plugins."io.containerd.grpc.v1.cri".containerd.runtimes.*.options]` | `CriuPath`                   | containerd v1.7     | containerd v2.0 ✅         | Set `$PATH` to the `criu` binary                | | ||||
| |`[plugins."io.containerd.grpc.v1.cri".registry]`                      | `auths`                      | containerd v1.3     | containerd v2.1            | Use [`ImagePullSecrets`](https://kubernetes.io/docs/tasks/configure-pod-container/pull-image-private-registry/). See also [#8228](https://github.com/containerd/containerd/issues/8228). | | ||||
| |`[plugins."io.containerd.grpc.v1.cri".registry]`                      | `configs`                    | containerd v1.5     | containerd v2.1            | Use [`config_path`](./docs/hosts.md)            | | ||||
|   | ||||
| @@ -340,9 +340,6 @@ type RuntimeConfig struct { | ||||
| 	// Log line longer than the limit will be split into multiple lines. Non-positive | ||||
| 	// value means no limit. | ||||
| 	MaxContainerLogLineSize int `toml:"max_container_log_line_size" json:"maxContainerLogSize"` | ||||
| 	// DisableCgroup indicates to disable the cgroup support. | ||||
| 	// This is useful when the containerd does not have permission to access cgroup. | ||||
| 	DisableCgroup bool `toml:"disable_cgroup" json:"disableCgroup"` | ||||
| 	// DisableApparmor indicates to disable the apparmor support. | ||||
| 	// This is useful when the containerd does not have permission to access Apparmor. | ||||
| 	DisableApparmor bool `toml:"disable_apparmor" json:"disableApparmor"` | ||||
|   | ||||
| @@ -222,15 +222,6 @@ func WithoutAmbientCaps(_ context.Context, _ oci.Client, c *containers.Container | ||||
| 	return nil | ||||
| } | ||||
|  | ||||
| // WithDisabledCgroups clears the Cgroups Path from the spec | ||||
| func WithDisabledCgroups(_ context.Context, _ oci.Client, c *containers.Container, s *runtimespec.Spec) error { | ||||
| 	if s.Linux == nil { | ||||
| 		s.Linux = &runtimespec.Linux{} | ||||
| 	} | ||||
| 	s.Linux.CgroupsPath = "" | ||||
| 	return nil | ||||
| } | ||||
|  | ||||
| // WithSelinuxLabels sets the mount and process labels | ||||
| func WithSelinuxLabels(process, mount string) oci.SpecOpts { | ||||
| 	return func(ctx context.Context, client oci.Client, c *containers.Container, s *runtimespec.Spec) (err error) { | ||||
|   | ||||
| @@ -761,14 +761,10 @@ func (c *criService) buildLinuxSpec( | ||||
| 		specOpts = append(specOpts, oci.WithRootFSReadonly()) | ||||
| 	} | ||||
|  | ||||
| 	if c.config.DisableCgroup { | ||||
| 		specOpts = append(specOpts, customopts.WithDisabledCgroups) | ||||
| 	} else { | ||||
| 		specOpts = append(specOpts, customopts.WithResources(config.GetLinux().GetResources(), c.config.TolerateMissingHugetlbController, c.config.DisableHugetlbController)) | ||||
| 		if sandboxConfig.GetLinux().GetCgroupParent() != "" { | ||||
| 			cgroupsPath := getCgroupsPath(sandboxConfig.GetLinux().GetCgroupParent(), id) | ||||
| 			specOpts = append(specOpts, oci.WithCgroup(cgroupsPath)) | ||||
| 		} | ||||
| 	specOpts = append(specOpts, customopts.WithResources(config.GetLinux().GetResources(), c.config.TolerateMissingHugetlbController, c.config.DisableHugetlbController)) | ||||
| 	if sandboxConfig.GetLinux().GetCgroupParent() != "" { | ||||
| 		cgroupsPath := getCgroupsPath(sandboxConfig.GetLinux().GetCgroupParent(), id) | ||||
| 		specOpts = append(specOpts, oci.WithCgroup(cgroupsPath)) | ||||
| 	} | ||||
|  | ||||
| 	supplementalGroups := securityContext.GetSupplementalGroups() | ||||
|   | ||||
| @@ -1343,22 +1343,6 @@ func TestHostname(t *testing.T) { | ||||
| 	} | ||||
| } | ||||
|  | ||||
| func TestDisableCgroup(t *testing.T) { | ||||
| 	containerConfig, sandboxConfig, imageConfig, _ := getCreateContainerTestData() | ||||
| 	ociRuntime := config.Runtime{} | ||||
| 	c := newTestCRIService() | ||||
| 	c.config.DisableCgroup = true | ||||
| 	spec, err := c.buildContainerSpec(currentPlatform, "test-id", "sandbox-id", 1234, "", "container-name", testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime, nil) | ||||
| 	require.NoError(t, err) | ||||
|  | ||||
| 	t.Log("resource limit should not be set") | ||||
| 	assert.Nil(t, spec.Linux.Resources.Memory) | ||||
| 	assert.Nil(t, spec.Linux.Resources.CPU) | ||||
|  | ||||
| 	t.Log("cgroup path should be empty") | ||||
| 	assert.Empty(t, spec.Linux.CgroupsPath) | ||||
| } | ||||
|  | ||||
| func TestGenerateUserString(t *testing.T) { | ||||
| 	type testcase struct { | ||||
| 		// the name of the test case | ||||
|   | ||||
| @@ -58,13 +58,9 @@ func (c *Controller) sandboxContainerSpec(id string, config *runtime.PodSandboxC | ||||
| 	specOpts = append(specOpts, oci.WithProcessArgs(append(imageConfig.Entrypoint, imageConfig.Cmd...)...)) | ||||
|  | ||||
| 	// Set cgroups parent. | ||||
| 	if c.config.DisableCgroup { | ||||
| 		specOpts = append(specOpts, customopts.WithDisabledCgroups) | ||||
| 	} else { | ||||
| 		if config.GetLinux().GetCgroupParent() != "" { | ||||
| 			cgroupsPath := getCgroupsPath(config.GetLinux().GetCgroupParent(), id) | ||||
| 			specOpts = append(specOpts, oci.WithCgroup(cgroupsPath)) | ||||
| 		} | ||||
| 	if config.GetLinux().GetCgroupParent() != "" { | ||||
| 		cgroupsPath := getCgroupsPath(config.GetLinux().GetCgroupParent(), id) | ||||
| 		specOpts = append(specOpts, oci.WithCgroup(cgroupsPath)) | ||||
| 	} | ||||
|  | ||||
| 	// When cgroup parent is not set, containerd-shim will create container in a child cgroup | ||||
| @@ -174,9 +170,7 @@ func (c *Controller) sandboxContainerSpec(id string, config *runtime.PodSandboxC | ||||
|  | ||||
| 	// Note: LinuxSandboxSecurityContext does not currently provide an apparmor profile | ||||
|  | ||||
| 	if !c.config.DisableCgroup { | ||||
| 		specOpts = append(specOpts, customopts.WithDefaultSandboxShares) | ||||
| 	} | ||||
| 	specOpts = append(specOpts, customopts.WithDefaultSandboxShares) | ||||
|  | ||||
| 	if res := config.GetLinux().GetResources(); res != nil { | ||||
| 		specOpts = append(specOpts, | ||||
|   | ||||
| @@ -758,20 +758,5 @@ options timeout:1 | ||||
| 	} | ||||
| } | ||||
|  | ||||
| func TestSandboxDisableCgroup(t *testing.T) { | ||||
| 	config, imageConfig, _ := getRunPodSandboxTestData() | ||||
| 	c := newControllerService() | ||||
| 	c.config.DisableCgroup = true | ||||
| 	spec, err := c.sandboxContainerSpec("test-id", config, imageConfig, "test-cni", []string{}) | ||||
| 	require.NoError(t, err) | ||||
|  | ||||
| 	t.Log("resource limit should not be set") | ||||
| 	assert.Nil(t, spec.Linux.Resources.Memory) | ||||
| 	assert.Nil(t, spec.Linux.Resources.CPU) | ||||
|  | ||||
| 	t.Log("cgroup path should be empty") | ||||
| 	assert.Empty(t, spec.Linux.CgroupsPath) | ||||
| } | ||||
|  | ||||
| // TODO(random-liu): [P1] Add unit test for different error cases to make sure | ||||
| // the function cleans up on error properly. | ||||
|   | ||||
		Reference in New Issue
	
	Block a user
	 Maksym Pavlenko
					Maksym Pavlenko