Merge pull request #62462 from jsafrane/private-mount-propagation

Automatic merge from submit-queue (batch tested with PRs 60476, 62462, 61391, 62535, 62394). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Private mount propagation

This PR changes the default mount propagation from "rslave" (newly added in 1.10) to "private" (default in 1.9 and before). "rslave" as default causes regressions, see below.

Value `"None"` has to be added to `MountPropagationMode` enum in API ("I don't want any propagation at all"), which translates to "private" on Linux. [We did not have use cases for it](https://github.com/kubernetes/community/pull/659#discussion_r131454319), but we have them now.

**Which issue(s) this PR fixes**
Fixes #62397, fixes #62396

**Special notes for your reviewer**:
CRI already has an option for private mount propagation in volumes, however it's called "PRIVATE", while Kubernetes API value is "None". I did not change PRIVATE to NONE to keep the interface stable. See `kubelet_pods.go`.

**Release note**:
```release-note
Default mount propagation has changed from "HostToContainer" ("rslave" in Linux terminology) to "None" ("private") to match the behavior in 1.9 and earlier releases. "HostToContainer" as a default caused regressions in some pods.
```


/sig storage
/sig node

pkg/apis/core/types.go

@@ -1540,6 +1540,12 @@ type VolumeMount struct {
 type MountPropagationMode string
 
 const (
+	// MountPropagationNone means that the volume in a container will
+	// not receive new mounts from the host or other containers, and filesystems
+	// mounted inside the container won't be propagated to the host or other
+	// containers.
+	// Note that this mode corresponds to "private" in Linux terminology.
+	MountPropagationNone MountPropagationMode = "None"
 	// MountPropagationHostToContainer means that the volume in a container will
 	// receive new mounts from the host or other containers, but filesystems
 	// mounted inside the container won't be propagated to the host or other

pkg/apis/core/validation/validation.go

@@ -1140,7 +1140,7 @@ func validateMountPropagation(mountPropagation *core.MountPropagationMode, conta
 		return allErrs
 	}
 
-	supportedMountPropagations := sets.NewString(string(core.MountPropagationBidirectional), string(core.MountPropagationHostToContainer))
+	supportedMountPropagations := sets.NewString(string(core.MountPropagationBidirectional), string(core.MountPropagationHostToContainer), string(core.MountPropagationNone))
 	if !supportedMountPropagations.Has(string(*mountPropagation)) {
 		allErrs = append(allErrs, field.NotSupported(fldPath, *mountPropagation, supportedMountPropagations.List()))
 	}

pkg/apis/core/validation/validation_test.go

@@ -4704,6 +4704,7 @@ func TestValidateMountPropagation(t *testing.T) {
 
 	propagationBidirectional := core.MountPropagationBidirectional
 	propagationHostToContainer := core.MountPropagationHostToContainer
+	propagationNone := core.MountPropagationNone
 	propagationInvalid := core.MountPropagationMode("invalid")
 
 	tests := []struct {
@@ -4723,6 +4724,12 @@ func TestValidateMountPropagation(t *testing.T) {
 			defaultContainer,
 			false,
 		},
+		{
+			// non-privileged container + None
+			core.VolumeMount{Name: "foo", MountPath: "/foo", MountPropagation: &propagationNone},
+			defaultContainer,
+			false,
+		},
 		{
 			// error: implicitly non-privileged container + Bidirectional
 			core.VolumeMount{Name: "foo", MountPath: "/foo", MountPropagation: &propagationBidirectional},

pkg/kubelet/kubelet_pods.go

@@ -269,12 +269,14 @@ func translateMountPropagation(mountMode *v1.MountPropagationMode) (runtimeapi.M
 	}
 	switch {
 	case mountMode == nil:
-		// HostToContainer is the default
-		return runtimeapi.MountPropagation_PROPAGATION_HOST_TO_CONTAINER, nil
+		// PRIVATE is the default
+		return runtimeapi.MountPropagation_PROPAGATION_PRIVATE, nil
 	case *mountMode == v1.MountPropagationHostToContainer:
 		return runtimeapi.MountPropagation_PROPAGATION_HOST_TO_CONTAINER, nil
 	case *mountMode == v1.MountPropagationBidirectional:
 		return runtimeapi.MountPropagation_PROPAGATION_BIDIRECTIONAL, nil
+	case *mountMode == v1.MountPropagationNone:
+		return runtimeapi.MountPropagation_PROPAGATION_PRIVATE, nil
 	default:
 		return 0, fmt.Errorf("invalid MountPropagation mode: %q", mountMode)
 	}

pkg/kubelet/kubelet_pods_test.go

@@ -55,6 +55,7 @@ func TestMakeMounts(t *testing.T) {
 	bTrue := true
 	propagationHostToContainer := v1.MountPropagationHostToContainer
 	propagationBidirectional := v1.MountPropagationBidirectional
+	propagationNone := v1.MountPropagationNone
 
 	testCases := map[string]struct {
 		container      v1.Container
@@ -79,9 +80,10 @@ func TestMakeMounts(t *testing.T) {
 						MountPropagation: &propagationHostToContainer,
 					},
 					{
-						MountPath: "/mnt/path3",
-						Name:      "disk",
-						ReadOnly:  true,
+						MountPath:        "/mnt/path3",
+						Name:             "disk",
+						ReadOnly:         true,
+						MountPropagation: &propagationNone,
 					},
 					{
 						MountPath: "/mnt/path4",
@@ -110,7 +112,7 @@ func TestMakeMounts(t *testing.T) {
 					HostPath:       "/mnt/disk",
 					ReadOnly:       true,
 					SELinuxRelabel: false,
-					Propagation:    runtimeapi.MountPropagation_PROPAGATION_HOST_TO_CONTAINER,
+					Propagation:    runtimeapi.MountPropagation_PROPAGATION_PRIVATE,
 				},
 				{
 					Name:           "disk4",
@@ -118,7 +120,7 @@ func TestMakeMounts(t *testing.T) {
 					HostPath:       "/mnt/host",
 					ReadOnly:       false,
 					SELinuxRelabel: false,
-					Propagation:    runtimeapi.MountPropagation_PROPAGATION_HOST_TO_CONTAINER,
+					Propagation:    runtimeapi.MountPropagation_PROPAGATION_PRIVATE,
 				},
 				{
 					Name:           "disk5",
@@ -126,7 +128,7 @@ func TestMakeMounts(t *testing.T) {
 					HostPath:       "/var/lib/kubelet/podID/volumes/empty/disk5",
 					ReadOnly:       false,
 					SELinuxRelabel: false,
-					Propagation:    runtimeapi.MountPropagation_PROPAGATION_HOST_TO_CONTAINER,
+					Propagation:    runtimeapi.MountPropagation_PROPAGATION_PRIVATE,
 				},
 			},
 			expectErr: false,
@@ -185,7 +187,7 @@ func TestMakeMounts(t *testing.T) {
 					HostPath:       "/mnt/host",
 					ReadOnly:       false,
 					SELinuxRelabel: false,
-					Propagation:    runtimeapi.MountPropagation_PROPAGATION_HOST_TO_CONTAINER,
+					Propagation:    runtimeapi.MountPropagation_PROPAGATION_PRIVATE,
 				},
 			},
 			expectErr: false,

staging/src/k8s.io/api/core/v1/types.go

@@ -1624,6 +1624,12 @@ type VolumeMount struct {
 type MountPropagationMode string
 
 const (
+	// MountPropagationNone means that the volume in a container will
+	// not receive new mounts from the host or other containers, and filesystems
+	// mounted inside the container won't be propagated to the host or other
+	// containers.
+	// Note that this mode corresponds to "private" in Linux terminology.
+	MountPropagationNone MountPropagationMode = "None"
 	// MountPropagationHostToContainer means that the volume in a container will
 	// receive new mounts from the host or other containers, but filesystems
 	// mounted inside the container won't be propagated to the host or other

test/e2e/node/mount_propagation.go

@@ -28,7 +28,7 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-func preparePod(name string, node *v1.Node, propagation v1.MountPropagationMode, hostDir string) *v1.Pod {
+func preparePod(name string, node *v1.Node, propagation *v1.MountPropagationMode, hostDir string) *v1.Pod {
 	const containerName = "cntr"
 	bTrue := true
 	var oneSecond int64 = 1
@@ -49,7 +49,7 @@ func preparePod(name string, node *v1.Node, propagation v1.MountPropagationMode,
 						{
 							Name:             "host",
 							MountPath:        "/mnt/test",
-							MountPropagation: &propagation,
+							MountPropagation: propagation,
 						},
 					},
 					SecurityContext: &v1.SecurityContext{
@@ -105,12 +105,19 @@ var _ = SIGDescribe("Mount propagation", func() {
 		}()
 
 		podClient := f.PodClient()
-		master := podClient.CreateSync(preparePod("master", node, v1.MountPropagationBidirectional, hostDir))
-		slave := podClient.CreateSync(preparePod("slave", node, v1.MountPropagationHostToContainer, hostDir))
+		bidirectional := v1.MountPropagationBidirectional
+		master := podClient.CreateSync(preparePod("master", node, &bidirectional, hostDir))
+
+		hostToContainer := v1.MountPropagationHostToContainer
+		slave := podClient.CreateSync(preparePod("slave", node, &hostToContainer, hostDir))
+
+		none := v1.MountPropagationNone
+		private := podClient.CreateSync(preparePod("private", node, &none, hostDir))
+		defaultPropagation := podClient.CreateSync(preparePod("default", node, nil, hostDir))
 
 		// Check that the pods sees directories of each other. This just checks
 		// that they have the same HostPath, not the mount propagation.
-		podNames := []string{master.Name, slave.Name}
+		podNames := []string{master.Name, slave.Name, private.Name, defaultPropagation.Name}
 		for _, podName := range podNames {
 			for _, dirName := range podNames {
 				cmd := fmt.Sprintf("test -d /mnt/test/%s", dirName)
@@ -147,6 +154,10 @@ var _ = SIGDescribe("Mount propagation", func() {
 			"master": sets.NewString("master", "host"),
 			// Slave sees master's mount + itself.
 			"slave": sets.NewString("master", "slave", "host"),
+			// Private sees only its own mount
+			"private": sets.NewString("private"),
+			// Default (=private) sees only its own mount
+			"default": sets.NewString("default"),
 		}
 		dirNames := append(podNames, "host")
 		for podName, mounts := range expectedMounts {