diff --git a/Vagrantfile b/Vagrantfile index 4a09789303..3502f8228d 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -278,6 +278,7 @@ EOF 'GOTESTSUM_JSONFILE': ENV['GOTESTSUM_JSONFILE'], 'GITHUB_WORKSPACE': '', 'CGROUP_DRIVER': ENV['CGROUP_DRIVER'], + 'RUNC_FLAVOR': ENV['RUNC_FLAVOR'] || "runc", } sh.inline = <<~SHELL #!/usr/bin/env bash @@ -306,6 +307,7 @@ EOF 'GOTEST': ENV['GOTEST'] || "go test", 'REPORT_DIR': ENV['REPORT_DIR'], 'CGROUP_DRIVER': ENV['CGROUP_DRIVER'], + 'RUNC_FLAVOR': ENV['RUNC_FLAVOR'] || "runc", } sh.inline = <<~SHELL #!/usr/bin/env bash diff --git a/integration/container_cgroup_mount_options_linux_test.go b/integration/container_cgroup_mount_options_linux_test.go new file mode 100644 index 0000000000..9de39eb4a6 --- /dev/null +++ b/integration/container_cgroup_mount_options_linux_test.go @@ -0,0 +1,79 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package integration + +import ( + "os" + "strings" + "testing" + + "github.com/containerd/cgroups/v3" + "github.com/containerd/containerd/v2/core/mount" + "github.com/containerd/containerd/v2/integration/images" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestPrivilegedContainerCgroupMountOptions(t *testing.T) { + if f := os.Getenv("RUNC_FLAVOR"); f == "crun" { + t.Skip("Skipping until crun supports cgroup v2 mount options (https://github.com/containers/crun/pull/2040)") + } + if cgroups.Mode() != cgroups.Unified { + t.Skip("Requires cgroup v2") + } + + hostMountBefore, err := mount.Lookup("/sys/fs/cgroup") + require.NoError(t, err) + + if !strings.Contains(hostMountBefore.VFSOptions, "nsdelegate") && !strings.Contains(hostMountBefore.VFSOptions, "memory_recursiveprot") { + t.Skip("requires host cgroup mount to have nsdelegate or memory_recursiveprot") + } + + testImage := images.Get(images.BusyBox) + EnsureImageExists(t, testImage) + + t.Log("Create a sandbox with privileged=true") + sb, sbConfig := PodSandboxConfigWithCleanup(t, "sandbox", "privileged-cgroup-mount-test", WithPodSecurityContext(true)) + + t.Log("Create a container with privileged=true") + cnConfig := ContainerConfig("container", testImage, WithCommand("sh", "-c", "sleep 1d"), WithSecurityContext(true)) + cn, err := runtimeService.CreateContainer(sb, cnConfig, sbConfig) + require.NoError(t, err) + t.Cleanup(func() { + if err := runtimeService.RemoveContainer(cn); err != nil { + t.Logf("failed to remove container %s: %v", cn, err) + } + }) + + t.Log("Start the container") + require.NoError(t, runtimeService.StartContainer(cn)) + t.Cleanup(func() { + if err := runtimeService.StopContainer(cn, 10); err != nil { + t.Logf("failed to stop container %s: %v", cn, err) + } + }) + + hostMountAfter, err := mount.Lookup("/sys/fs/cgroup") + require.NoError(t, err) + + if strings.Contains(hostMountBefore.VFSOptions, "nsdelegate") { + assert.Contains(t, hostMountAfter.VFSOptions, "nsdelegate", "nsdelegate should be preserved on the host cgroup mount") + } + if strings.Contains(hostMountBefore.VFSOptions, "memory_recursiveprot") { + assert.Contains(t, hostMountAfter.VFSOptions, "memory_recursiveprot", "memory_recursiveprot should be preserved on the host cgroup mount") + } +} diff --git a/internal/cri/opts/spec_linux_opts.go b/internal/cri/opts/spec_linux_opts.go index 41185d3959..88ec0ded43 100644 --- a/internal/cri/opts/spec_linux_opts.go +++ b/internal/cri/opts/spec_linux_opts.go @@ -23,6 +23,7 @@ import ( "maps" "os" "path/filepath" + "slices" "sort" "strconv" "strings" @@ -71,11 +72,35 @@ func withMounts(osi osinterface.OS, config *runtime.ContainerConfig, extra []*ru if cgroupWritable { mode = "rw" } + + cgroupOptions := []string{"nosuid", "noexec", "nodev", "relatime", mode} + + hasCgroupNS := false + if s.Linux != nil { + hasCgroupNS = slices.ContainsFunc(s.Linux.Namespaces, func(ns runtimespec.LinuxNamespace) bool { + return ns.Type == runtimespec.CgroupNamespace + }) + } + + // If a container shares the host's cgroup namespace, mounting cgroup2 + // inside the container applies the new mount options to the single shared + // cgroup2 VFS superblock. Therefore, explicitly copy these options from + // the host's /sys/fs/cgroup to avoid being stripped. + if !hasCgroupNS { + if mountInfo, err := osi.LookupMount("/sys/fs/cgroup"); err == nil { + for opt := range strings.SplitSeq(mountInfo.VFSOptions, ",") { + if opt == "nsdelegate" || opt == "memory_recursiveprot" { + cgroupOptions = append(cgroupOptions, opt) + } + } + } + } + s.Mounts = append(s.Mounts, runtimespec.Mount{ Source: "cgroup", Destination: "/sys/fs/cgroup", Type: "cgroup", - Options: []string{"nosuid", "noexec", "nodev", "relatime", mode}, + Options: cgroupOptions, }) // Copy all mounts from default mounts, except for diff --git a/internal/cri/opts/spec_linux_test.go b/internal/cri/opts/spec_linux_test.go index 1c9942f80c..2d729d1bb8 100644 --- a/internal/cri/opts/spec_linux_test.go +++ b/internal/cri/opts/spec_linux_test.go @@ -17,10 +17,15 @@ package opts import ( + "context" "testing" + "github.com/containerd/containerd/v2/core/mount" + ostesting "github.com/containerd/containerd/v2/pkg/os/testing" + runtimespec "github.com/opencontainers/runtime-spec/specs-go" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + runtime "k8s.io/cri-api/pkg/apis/runtime/v1" ) func TestMergeGids(t *testing.T) { @@ -45,3 +50,73 @@ func TestRestrictOOMScoreAdj(t *testing.T) { require.NoError(t, err) assert.Equal(t, got, current+1) } + +func TestWithMountsCgroupNamespaceOptions(t *testing.T) { + tests := []struct { + name string + hasCgroupNS bool + hostMountOpts string + expectedOpts []string + }{ + { + name: "has cgroupns, should use default options", + hasCgroupNS: true, + hostMountOpts: "rw,nosuid,nodev,noexec,relatime,nsdelegate,memory_recursiveprot", + expectedOpts: []string{"nosuid", "noexec", "nodev", "relatime", "ro"}, + }, + { + name: "no cgroupns, with host options present", + hasCgroupNS: false, + hostMountOpts: "rw,nosuid,nodev,noexec,relatime,nsdelegate,memory_recursiveprot", + expectedOpts: []string{"nosuid", "noexec", "nodev", "relatime", "ro", "nsdelegate", "memory_recursiveprot"}, + }, + { + name: "no cgroupns, with host missing nsdelegate", + hasCgroupNS: false, + hostMountOpts: "rw,nosuid,nodev,noexec,relatime,memory_recursiveprot", + expectedOpts: []string{"nosuid", "noexec", "nodev", "relatime", "ro", "memory_recursiveprot"}, + }, + { + name: "no cgroupns, with host missing all extra options", + hasCgroupNS: false, + hostMountOpts: "rw,nosuid,nodev,noexec,relatime", + expectedOpts: []string{"nosuid", "noexec", "nodev", "relatime", "ro"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fakeOS := ostesting.NewFakeOS() + fakeOS.LookupMountFn = func(path string) (mount.Info, error) { + if path == "/sys/fs/cgroup" { + return mount.Info{VFSOptions: tt.hostMountOpts}, nil + } + return mount.Info{}, nil + } + + config := &runtime.ContainerConfig{ + Linux: &runtime.LinuxContainerConfig{}, + } + + spec := &runtimespec.Spec{} + if tt.hasCgroupNS { + spec.Linux = &runtimespec.Linux{Namespaces: []runtimespec.LinuxNamespace{{Type: runtimespec.CgroupNamespace}}} + } + + opt := withMounts(fakeOS, config, nil, "", nil, false) + err := opt(context.Background(), nil, nil, spec) + require.NoError(t, err) + + var cgroupMount *runtimespec.Mount + for _, m := range spec.Mounts { + if m.Destination == "/sys/fs/cgroup" { + cgroupMount = &m + break + } + } + + require.NotNil(t, cgroupMount) + assert.ElementsMatch(t, tt.expectedOpts, cgroupMount.Options) + }) + } +} diff --git a/internal/cri/server/container_create.go b/internal/cri/server/container_create.go index 7ddb1b3937..43e612a073 100644 --- a/internal/cri/server/container_create.go +++ b/internal/cri/server/container_create.go @@ -792,6 +792,14 @@ func (c *criService) buildLinuxSpec( } }() + // cgroupns is used for hiding /sys/fs/cgroup from containers. + // For compatibility, cgroupns is not used when running in cgroup v1 mode or in privileged. + // https://github.com/containers/libpod/issues/4363 + // https://github.com/kubernetes/enhancements/blob/0e409b47497e398b369c281074485c8de129694f/keps/sig-node/20191118-cgroups-v2.md#cgroup-namespace + if isUnifiedCgroupsMode() && !securityContext.GetPrivileged() { + specOpts = append(specOpts, oci.WithLinuxNamespace(runtimespec.LinuxNamespace{Type: runtimespec.CgroupNamespace})) + } + var ociSpecOpts oci.SpecOpts if ociRuntime.CgroupWritable { ociSpecOpts = customopts.WithMountsCgroupWritable(c.os, config, extraMounts, mountLabel, runtimeHandler) @@ -930,14 +938,6 @@ func (c *criService) buildLinuxSpec( annotations.DefaultCRIAnnotations(sandboxID, containerName, imageName, sandboxConfig, false)..., ) - // cgroupns is used for hiding /sys/fs/cgroup from containers. - // For compatibility, cgroupns is not used when running in cgroup v1 mode or in privileged. - // https://github.com/containers/libpod/issues/4363 - // https://github.com/kubernetes/enhancements/blob/0e409b47497e398b369c281074485c8de129694f/keps/sig-node/20191118-cgroups-v2.md#cgroup-namespace - if isUnifiedCgroupsMode() && !securityContext.GetPrivileged() { - specOpts = append(specOpts, oci.WithLinuxNamespace(runtimespec.LinuxNamespace{Type: runtimespec.CgroupNamespace})) - } - return specOpts, nil } diff --git a/internal/cri/server/container_create_linux_test.go b/internal/cri/server/container_create_linux_test.go index 0242c0e83d..a37a849aa3 100644 --- a/internal/cri/server/container_create_linux_test.go +++ b/internal/cri/server/container_create_linux_test.go @@ -487,6 +487,52 @@ func TestPrivilegedBindMount(t *testing.T) { } } +func TestCgroupNamespace(t *testing.T) { + testPid := uint32(1234) + c := newTestCRIService() + testSandboxID := "sandbox-id" + testContainerName := "container-name" + containerConfig, sandboxConfig, imageConfig, _ := getCreateContainerTestData() + ociRuntime := config.Runtime{} + + tests := []struct { + desc string + privileged bool + expectCgroupNamespace bool + }{ + { + desc: "non-privileged container should get cgroup namespace", + privileged: false, + expectCgroupNamespace: true, + }, + { + desc: "privileged container should not get cgroup namespace", + privileged: true, + expectCgroupNamespace: false, + }, + } + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + containerConfig.Linux.SecurityContext.Privileged = tt.privileged + sandboxConfig.Linux.SecurityContext.Privileged = tt.privileged + + spec, err := c.buildContainerSpec(currentPlatform, t.Name(), testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime, nil) + assert.NoError(t, err) + + hasCgroupNS := false + for _, ns := range spec.Linux.Namespaces { + if ns.Type == runtimespec.CgroupNamespace { + hasCgroupNS = true + break + } + } + + assert.Equal(t, tt.expectCgroupNamespace, hasCgroupNS) + }) + } +} + func TestMountPropagation(t *testing.T) { sharedLookupMountFn := func(string) (mount.Info, error) { diff --git a/script/test/cri-integration.sh b/script/test/cri-integration.sh index 1ac35bad1c..fd696430ae 100755 --- a/script/test/cri-integration.sh +++ b/script/test/cri-integration.sh @@ -46,6 +46,10 @@ CMD="" if [ -n "${sudo}" ]; then CMD+="${sudo} " fi +CMD+="env " +if [ -n "${RUNC_FLAVOR:-}" ]; then + CMD+="RUNC_FLAVOR=${RUNC_FLAVOR} " +fi CMD+="${PWD}/bin/cri-integration.test" ${CMD} --test.run="${FOCUS}" --test.v \