Merge pull request #11456 from k8s-infra-cherrypick-robot/cherry-pick-11271-to-release/2.0

[release/2.0] Fix privileged container sysfs can't be rw because pod is ro by default

integration/container_stats_test.go

@@ -428,3 +428,83 @@ func testStats(t *testing.T,
 		require.NotEmpty(t, s.GetWritableLayer().GetInodesUsed().GetValue())
 	}
 }
+
+func TestContainerSysfsStatsWithPrivilegedPod(t *testing.T) {
+	if goruntime.GOOS == "windows" {
+		t.Skip("Doesn't care about filesystem properties on windows")
+	}
+	testImage := images.Get(images.BusyBox)
+	testcases := []struct {
+		name                               string
+		privilegedPod, privilegedContainer bool
+		expectedSysfsOption, expectedErr   string
+	}{
+		{
+			name:                "sandbox and container with privileged=true, sysfs is rw",
+			privilegedPod:       true,
+			privilegedContainer: true,
+			expectedSysfsOption: "rw",
+		},
+		{
+			name:                "sandbox with privileged=true, and container with privileged=false, sysfs is ro",
+			privilegedPod:       true,
+			privilegedContainer: false,
+			expectedSysfsOption: "ro",
+		},
+		{
+			name:                "sandbox and container with privileged=false, sysfs is ro",
+			privilegedPod:       false,
+			privilegedContainer: false,
+			expectedSysfsOption: "ro",
+		},
+		{
+			name:                "sandbox with privileged=false, and container with privileged=true, create container failed",
+			privilegedPod:       false,
+			privilegedContainer: true,
+			expectedErr:         "failed to generate spec opts: no privileged container allowed in sandbox",
+		},
+	}
+	for _, test := range testcases {
+		t.Run(test.name, func(t *testing.T) {
+			EnsureImageExists(t, testImage)
+			sb, sbConfig := PodSandboxConfigWithCleanup(
+				t,
+				"sandbox",
+				"sysfs-stats-with-privileged",
+				WithPodSecurityContext(test.privilegedPod),
+			)
+			cnConfig := ContainerConfig(
+				"container",
+				testImage,
+				WithCommand("sh", "-c", "top"),
+				WithSecurityContext(test.privilegedContainer),
+			)
+			cn, err := runtimeService.CreateContainer(sb, cnConfig, sbConfig)
+			if test.expectedErr != "" && err != nil {
+				assert.Contains(t, err.Error(), test.expectedErr)
+				return
+			}
+			assert.NoError(t, err)
+			defer func() {
+				if test.expectedErr == "" {
+					assert.NoError(t, runtimeService.RemoveContainer(cn))
+				}
+			}()
+
+			t.Log("Start the container")
+			require.NoError(t, runtimeService.StartContainer(cn))
+			defer func() {
+				assert.NoError(t, runtimeService.StopContainer(cn, 10))
+			}()
+
+			t.Logf("Execute cmd in container by sync")
+			mountinfo, _, err := runtimeService.ExecSync(cn, []string{
+				"sh",
+				"-c",
+				"mount |grep sysfs",
+			}, 10)
+			assert.NoError(t, err)
+			assert.Contains(t, string(mountinfo), test.expectedSysfsOption)
+		})
+	}
+}

integration/main_test.go

@@ -218,6 +218,19 @@ func WithPodLabels(kvs map[string]string) PodSandboxOpts {
 	}
 }
 
+// WithSecurityContext set container privileged.
+func WithPodSecurityContext(privileged bool) PodSandboxOpts {
+	return func(p *runtime.PodSandboxConfig) {
+		if p.Linux == nil {
+			p.Linux = &runtime.LinuxPodSandboxConfig{}
+		}
+		if p.Linux.SecurityContext == nil {
+			p.Linux.SecurityContext = &runtime.LinuxSandboxSecurityContext{}
+		}
+		p.Linux.SecurityContext.Privileged = privileged
+	}
+}
+
 // PodSandboxConfig generates a pod sandbox config for test.
 func PodSandboxConfig(name, ns string, opts ...PodSandboxOpts) *runtime.PodSandboxConfig {
 	var cgroupParent string
@@ -462,6 +475,19 @@ func WithSupplementalGroups(gids []int64) ContainerOpts {
 	}
 }
 
+// WithSecurityContext set container privileged.
+func WithSecurityContext(privileged bool) ContainerOpts {
+	return func(c *runtime.ContainerConfig) {
+		if c.Linux == nil {
+			c.Linux = &runtime.LinuxContainerConfig{}
+		}
+		if c.Linux.SecurityContext == nil {
+			c.Linux.SecurityContext = &runtime.LinuxContainerSecurityContext{}
+		}
+		c.Linux.SecurityContext.Privileged = privileged
+	}
+}
+
 // WithDevice adds a device mount.
 func WithDevice(containerPath, hostPath, permissions string) ContainerOpts {
 	return func(c *runtime.ContainerConfig) {

internal/cri/server/podsandbox/sandbox_run.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"path/filepath"
 
 	"github.com/containerd/log"
 	"github.com/containerd/nri"
@@ -136,6 +137,7 @@ func (c *Controller) Start(ctx context.Context, id string) (cin sandbox.Controll
 	if err != nil {
 		return cin, fmt.Errorf("failed to generate sandbox container spec: %w", err)
 	}
+
 	log.G(ctx).WithField("podsandboxid", id).Debugf("sandbox container spec: %#+v", spew.NewFormatter(spec))
 
 	metadata.ProcessLabel = spec.Process.SelinuxLabel
@@ -155,6 +157,18 @@ func (c *Controller) Start(ctx context.Context, id string) (cin sandbox.Controll
 		// If privileged don't set selinux label, but we still record the MCS label so that
 		// the unused label can be freed later.
 		spec.Process.SelinuxLabel = ""
+		// If privileged is enabled, sysfs should have the rw attribute
+		for i, k := range spec.Mounts {
+			if filepath.Clean(k.Destination) == "/sys" {
+				for j, v := range spec.Mounts[i].Options {
+					if v == "ro" {
+						spec.Mounts[i].Options[j] = "rw"
+						break
+					}
+				}
+				break
+			}
+		}
 	}
 
 	// Generate spec options that will be applied to the spec later.