Merge pull request containerd#10594 from AkihiroSuda/cri-remove-disab…

…leCgroup

CRI: remove `disable_cgroup`

RELEASES.md

@@ -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)            |

internal/cri/config/config.go

@@ -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"`

internal/cri/opts/spec_opts.go

@@ -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) {

internal/cri/server/container_create.go

@@ -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()

internal/cri/server/container_create_linux_test.go

@@ -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

internal/cri/server/podsandbox/sandbox_run_linux.go

@@ -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,

internal/cri/server/podsandbox/sandbox_run_linux_test.go

@@ -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.