From 6f3833f258cb0827878b282c29f50d88669b91a8 Mon Sep 17 00:00:00 2001 From: Akihiro Suda Date: Thu, 15 Aug 2024 06:08:30 +0900 Subject: [PATCH] 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 --- RELEASES.md | 1 + internal/cri/config/config.go | 3 --- internal/cri/opts/spec_opts.go | 9 --------- internal/cri/server/container_create.go | 12 ++++-------- .../cri/server/container_create_linux_test.go | 16 ---------------- .../cri/server/podsandbox/sandbox_run_linux.go | 14 ++++---------- .../server/podsandbox/sandbox_run_linux_test.go | 15 --------------- 7 files changed, 9 insertions(+), 61 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index 84537577b..5f5c57d33 100644 --- 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) | diff --git a/internal/cri/config/config.go b/internal/cri/config/config.go index 1479a684e..a5c6b1d5b 100644 --- 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"` diff --git a/internal/cri/opts/spec_opts.go b/internal/cri/opts/spec_opts.go index 49dc70076..9dda38af3 100644 --- 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) { diff --git a/internal/cri/server/container_create.go b/internal/cri/server/container_create.go index 6d97048c7..32523529c 100644 --- 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() diff --git a/internal/cri/server/container_create_linux_test.go b/internal/cri/server/container_create_linux_test.go index 6b78e97a0..c502a3533 100644 --- 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 diff --git a/internal/cri/server/podsandbox/sandbox_run_linux.go b/internal/cri/server/podsandbox/sandbox_run_linux.go index 87606b2e1..832df4dc3 100644 --- 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, diff --git a/internal/cri/server/podsandbox/sandbox_run_linux_test.go b/internal/cri/server/podsandbox/sandbox_run_linux_test.go index d2e206522..8935a9e31 100644 --- 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.