CRI: remove disable_cgroup

`disable_cgroup` was implemenetd in containerd/cri PR 970 (Nov 2018) for supporting very early version of Usernetes on cgroup v1 hosts, when most distros were still not ready to support cgroup v2. This configuration is no longer needed, as cgroup v2 delegation is now supported on almost all distros. Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
2024-08-15 06:08:30 +09:00 · 2024-08-15 06:08:30 +09:00 · 6f3833f258
commit 6f3833f258
parent a406da9628
7 changed files with 9 additions and 61 deletions
--- a/RELEASES.md
+++ b/RELEASES.md
@ -461,6 +461,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)            |
--- a/internal/cri/config/config.go
+++ b/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"`
--- a/internal/cri/opts/spec_opts.go
+++ b/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) {
--- a/internal/cri/server/container_create.go
+++ b/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()
--- a/internal/cri/server/container_create_linux_test.go
+++ b/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
--- a/internal/cri/server/podsandbox/sandbox_run_linux.go
+++ b/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,
--- a/internal/cri/server/podsandbox/sandbox_run_linux_test.go
+++ b/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.