diff --git a/RELEASES.md b/RELEASES.md index a8b3ce1f6..3be9f6cfe 100644 --- a/RELEASES.md +++ b/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) | 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.