Merge pull request #104253 from mauriciopoppe/subpath-additional-moun…

…t-flag Pass additional flags to subpath mount to avoid flakes in certain conditions
kubernetes · Aug 11, 2021 · 3ca0145 · 3ca0145
2 parents 202f775 + 08bec6d
commit 3ca0145
Show file tree

Hide file tree

Showing 7 changed files with 84 additions and 19 deletions.
diff --git a/pkg/volume/util/subpath/subpath_linux.go b/pkg/volume/util/subpath/subpath_linux.go
@@ -209,8 +209,9 @@ func doBindSubPath(mounter mount.Interface, subpath Subpath) (hostPath string, e
 
 	// Do the bind mount
 	options := []string{"bind"}
+	mountFlags := []string{"--no-canonicalize"}
 	klog.V(5).Infof("bind mounting %q at %q", mountSource, bindPathTarget)
-	if err = mounter.MountSensitiveWithoutSystemd(mountSource, bindPathTarget, "" /*fstype*/, options, nil); err != nil {
+	if err = mounter.MountSensitiveWithoutSystemdWithMountFlags(mountSource, bindPathTarget, "" /*fstype*/, options, nil /* sensitiveOptions */, mountFlags); err != nil {
 		return "", fmt.Errorf("error mounting %s: %s", subpath.Path, err)
 	}
 	success = true

diff --git a/staging/src/k8s.io/mount-utils/fake_mounter.go b/staging/src/k8s.io/mount-utils/fake_mounter.go
@@ -136,6 +136,10 @@ func (f *FakeMounter) MountSensitiveWithoutSystemd(source string, target string,
 	return f.MountSensitive(source, target, fstype, options, nil /* sensitiveOptions */)
 }
 
+func (f *FakeMounter) MountSensitiveWithoutSystemdWithMountFlags(source string, target string, fstype string, options []string, sensitiveOptions []string, mountFlags []string) error {
+	return f.MountSensitive(source, target, fstype, options, nil /* sensitiveOptions */)
+}
+
 // Unmount records the unmount event and updates the in-memory mount points for FakeMounter
 func (f *FakeMounter) Unmount(target string) error {
 	f.mutex.Lock()

diff --git a/staging/src/k8s.io/mount-utils/mount.go b/staging/src/k8s.io/mount-utils/mount.go
@@ -49,6 +49,8 @@ type Interface interface {
 	MountSensitive(source string, target string, fstype string, options []string, sensitiveOptions []string) error
 	// MountSensitiveWithoutSystemd is the same as MountSensitive() but this method disable using systemd mount.
 	MountSensitiveWithoutSystemd(source string, target string, fstype string, options []string, sensitiveOptions []string) error
+	// MountSensitiveWithoutSystemdWithMountFlags is the same as MountSensitiveWithoutSystemd() with additional mount flags
+	MountSensitiveWithoutSystemdWithMountFlags(source string, target string, fstype string, options []string, sensitiveOptions []string, mountFlags []string) error
 	// Unmount unmounts given target.
 	Unmount(target string) error
 	// List returns a list of all mounted filesystems.  This can be large.

diff --git a/staging/src/k8s.io/mount-utils/mount_linux.go b/staging/src/k8s.io/mount-utils/mount_linux.go
@@ -89,11 +89,11 @@ func (mounter *Mounter) MountSensitive(source string, target string, fstype stri
 	mounterPath := ""
 	bind, bindOpts, bindRemountOpts, bindRemountOptsSensitive := MakeBindOptsSensitive(options, sensitiveOptions)
 	if bind {
-		err := mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, bindOpts, bindRemountOptsSensitive, true)
+		err := mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, bindOpts, bindRemountOptsSensitive, nil /* mountFlags */, true)
 		if err != nil {
 			return err
 		}
-		return mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, bindRemountOpts, bindRemountOptsSensitive, true)
+		return mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, bindRemountOpts, bindRemountOptsSensitive, nil /* mountFlags */, true)
 	}
 	// The list of filesystems that require containerized mounter on GCI image cluster
 	fsTypesNeedMounter := map[string]struct{}{
@@ -105,19 +105,24 @@ func (mounter *Mounter) MountSensitive(source string, target string, fstype stri
 	if _, ok := fsTypesNeedMounter[fstype]; ok {
 		mounterPath = mounter.mounterPath
 	}
-	return mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, options, sensitiveOptions, true)
+	return mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, options, sensitiveOptions, nil /* mountFlags */, true)
 }
 
 // MountSensitiveWithoutSystemd is the same as MountSensitive() but disable using systemd mount.
 func (mounter *Mounter) MountSensitiveWithoutSystemd(source string, target string, fstype string, options []string, sensitiveOptions []string) error {
+	return mounter.MountSensitiveWithoutSystemdWithMountFlags(source, target, fstype, options, sensitiveOptions, nil /* mountFlags */)
+}
+
+// MountSensitiveWithoutSystemdWithMountFlags is the same as MountSensitiveWithoutSystemd with additional mount flags.
+func (mounter *Mounter) MountSensitiveWithoutSystemdWithMountFlags(source string, target string, fstype string, options []string, sensitiveOptions []string, mountFlags []string) error {
 	mounterPath := ""
 	bind, bindOpts, bindRemountOpts, bindRemountOptsSensitive := MakeBindOptsSensitive(options, sensitiveOptions)
 	if bind {
-		err := mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, bindOpts, bindRemountOptsSensitive, false)
+		err := mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, bindOpts, bindRemountOptsSensitive, mountFlags, false)
 		if err != nil {
 			return err
 		}
-		return mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, bindRemountOpts, bindRemountOptsSensitive, false)
+		return mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, bindRemountOpts, bindRemountOptsSensitive, mountFlags, false)
 	}
 	// The list of filesystems that require containerized mounter on GCI image cluster
 	fsTypesNeedMounter := map[string]struct{}{
@@ -129,14 +134,14 @@ func (mounter *Mounter) MountSensitiveWithoutSystemd(source string, target strin
 	if _, ok := fsTypesNeedMounter[fstype]; ok {
 		mounterPath = mounter.mounterPath
 	}
-	return mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, options, sensitiveOptions, false)
+	return mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, options, sensitiveOptions, mountFlags, false)
 }
 
 // doMount runs the mount command. mounterPath is the path to mounter binary if containerized mounter is used.
 // sensitiveOptions is an extension of options except they will not be logged (because they may contain sensitive material)
 // systemdMountRequired is an extension of option to decide whether uses systemd mount.
-func (mounter *Mounter) doMount(mounterPath string, mountCmd string, source string, target string, fstype string, options []string, sensitiveOptions []string, systemdMountRequired bool) error {
-	mountArgs, mountArgsLogStr := MakeMountArgsSensitive(source, target, fstype, options, sensitiveOptions)
+func (mounter *Mounter) doMount(mounterPath string, mountCmd string, source string, target string, fstype string, options []string, sensitiveOptions []string, mountFlags []string, systemdMountRequired bool) error {
+	mountArgs, mountArgsLogStr := MakeMountArgsSensitiveWithMountFlags(source, target, fstype, options, sensitiveOptions, mountFlags)
 	if len(mounterPath) > 0 {
 		mountArgs = append([]string{mountCmd}, mountArgs...)
 		mountArgsLogStr = mountCmd + " " + mountArgsLogStr
@@ -227,10 +232,22 @@ func MakeMountArgs(source, target, fstype string, options []string) (mountArgs [
 // MakeMountArgsSensitive makes the arguments to the mount(8) command.
 // sensitiveOptions is an extension of options except they will not be logged (because they may contain sensitive material)
 func MakeMountArgsSensitive(source, target, fstype string, options []string, sensitiveOptions []string) (mountArgs []string, mountArgsLogStr string) {
+	return MakeMountArgsSensitiveWithMountFlags(source, target, fstype, options, sensitiveOptions, nil /* mountFlags */)
+}
+
+// MakeMountArgsSensitiveWithMountFlags makes the arguments to the mount(8) command.
+// sensitiveOptions is an extension of options except they will not be logged (because they may contain sensitive material)
+// mountFlags are additional mount flags that are not related with the fstype
+// and mount options
+func MakeMountArgsSensitiveWithMountFlags(source, target, fstype string, options []string, sensitiveOptions []string, mountFlags []string) (mountArgs []string, mountArgsLogStr string) {
 	// Build mount command as follows:
-	//   mount [-t $fstype] [-o $options] [$source] $target
+	//   mount [$mountFlags] [-t $fstype] [-o $options] [$source] $target
 	mountArgs = []string{}
 	mountArgsLogStr = ""
+
+	mountArgs = append(mountArgs, mountFlags...)
+	mountArgsLogStr += strings.Join(mountFlags, " ")
+
 	if len(fstype) > 0 {
 		mountArgs = append(mountArgs, "-t", fstype)
 		mountArgsLogStr += strings.Join(mountArgs, " ")

diff --git a/staging/src/k8s.io/mount-utils/mount_linux_test.go b/staging/src/k8s.io/mount-utils/mount_linux_test.go
@@ -447,6 +447,7 @@ func TestSensitiveMountOptions(t *testing.T) {
 		fstype           string
 		options          []string
 		sensitiveOptions []string
+		mountFlags       []string
 	}{
 		{
 
@@ -455,6 +456,7 @@ func TestSensitiveMountOptions(t *testing.T) {
 			fstype:           "myFS",
 			options:          []string{"o1", "o2"},
 			sensitiveOptions: []string{"s1", "s2"},
+			mountFlags:       []string{},
 		},
 		{
 
@@ -463,6 +465,7 @@ func TestSensitiveMountOptions(t *testing.T) {
 			fstype:           "myFS",
 			options:          []string{},
 			sensitiveOptions: []string{"s1", "s2"},
+			mountFlags:       []string{},
 		},
 		{
 
@@ -471,26 +474,44 @@ func TestSensitiveMountOptions(t *testing.T) {
 			fstype:           "myFS",
 			options:          []string{"o1", "o2"},
 			sensitiveOptions: []string{},
+			mountFlags:       []string{},
+		},
+		{
+
+			source:           "mySrc",
+			target:           "myTarget",
+			fstype:           "myFS",
+			options:          []string{"o1", "o2"},
+			sensitiveOptions: []string{"s1", "s2"},
+			mountFlags:       []string{"--no-canonicalize"},
 		},
 	}
 
 	for _, v := range testcases {
 		// Act
-		mountArgs, mountArgsLogStr := MakeMountArgsSensitive(v.source, v.target, v.fstype, v.options, v.sensitiveOptions)
+		mountArgs, mountArgsLogStr := MakeMountArgsSensitiveWithMountFlags(v.source, v.target, v.fstype, v.options, v.sensitiveOptions, v.mountFlags)
 
 		// Assert
 		t.Logf("\r\nmountArgs =%q\r\nmountArgsLogStr=%q", mountArgs, mountArgsLogStr)
+		for _, mountFlag := range v.mountFlags {
+			if found := mountArgsContainString(t, mountArgs, mountFlag); !found {
+				t.Errorf("Expected mountFlag (%q) to exist in returned mountArgs (%q), but it does not", mountFlag, mountArgs)
+			}
+			if !strings.Contains(mountArgsLogStr, mountFlag) {
+				t.Errorf("Expected mountFlag (%q) to exist in returned mountArgsLogStr (%q), but it does", mountFlag, mountArgsLogStr)
+			}
+		}
 		for _, option := range v.options {
-			if found := contains(mountArgs, option, t); !found {
-				t.Errorf("Expected option (%q) to exist in returned mountArts (%q), but it does not", option, mountArgs)
+			if found := mountArgsContainOption(t, mountArgs, option); !found {
+				t.Errorf("Expected option (%q) to exist in returned mountArgs (%q), but it does not", option, mountArgs)
 			}
 			if !strings.Contains(mountArgsLogStr, option) {
 				t.Errorf("Expected option (%q) to exist in returned mountArgsLogStr (%q), but it does", option, mountArgsLogStr)
 			}
 		}
 		for _, sensitiveOption := range v.sensitiveOptions {
-			if found := contains(mountArgs, sensitiveOption, t); !found {
-				t.Errorf("Expected sensitiveOption (%q) to exist in returned mountArts (%q), but it does not", sensitiveOption, mountArgs)
+			if found := mountArgsContainOption(t, mountArgs, sensitiveOption); !found {
+				t.Errorf("Expected sensitiveOption (%q) to exist in returned mountArgs (%q), but it does not", sensitiveOption, mountArgs)
 			}
 			if strings.Contains(mountArgsLogStr, sensitiveOption) {
 				t.Errorf("Expected sensitiveOption (%q) to not exist in returned mountArgsLogStr (%q), but it does", sensitiveOption, mountArgsLogStr)
@@ -499,18 +520,27 @@ func TestSensitiveMountOptions(t *testing.T) {
 	}
 }
 
-func contains(slice []string, str string, t *testing.T) bool {
+func mountArgsContainString(t *testing.T, mountArgs []string, wanted string) bool {
+	for _, mountArg := range mountArgs {
+		if mountArg == wanted {
+			return true
+		}
+	}
+	return false
+}
+
+func mountArgsContainOption(t *testing.T, mountArgs []string, option string) bool {
 	optionsIndex := -1
-	for i, s := range slice {
+	for i, s := range mountArgs {
 		if s == "-o" {
 			optionsIndex = i + 1
 			break
 		}
 	}
 
-	if optionsIndex < 0 || optionsIndex >= len(slice) {
+	if optionsIndex < 0 || optionsIndex >= len(mountArgs) {
 		return false
 	}
 
-	return strings.Contains(slice[optionsIndex], str)
+	return strings.Contains(mountArgs[optionsIndex], option)
 }
diff --git a/staging/src/k8s.io/mount-utils/mount_unsupported.go b/staging/src/k8s.io/mount-utils/mount_unsupported.go
@@ -53,6 +53,11 @@ func (mounter *Mounter) MountSensitiveWithoutSystemd(source string, target strin
 	return errUnsupported
 }
 
+// MountSensitiveWithoutSystemdWithMountFlags always returns an error on unsupported platforms
+func (mounter *Mounter) MountSensitiveWithoutSystemdWithMountFlags(source string, target string, fstype string, options []string, sensitiveOptions []string, mountFlags []string) error {
+	return errUnsupported
+}
+
 // Unmount always returns an error on unsupported platforms
 func (mounter *Mounter) Unmount(target string) error {
 	return errUnsupported

diff --git a/staging/src/k8s.io/mount-utils/mount_windows.go b/staging/src/k8s.io/mount-utils/mount_windows.go
@@ -64,6 +64,12 @@ func (mounter *Mounter) MountSensitiveWithoutSystemd(source string, target strin
 	return mounter.MountSensitive(source, target, fstype, options, sensitiveOptions /* sensitiveOptions */)
 }
 
+// MountSensitiveWithoutSystemdWithMountFlags is the same as MountSensitiveWithoutSystemd with additional mount flags
+// Windows not supported systemd mount, this function degrades to MountSensitive().
+func (mounter *Mounter) MountSensitiveWithoutSystemdWithMountFlags(source string, target string, fstype string, options []string, sensitiveOptions []string, mountFlags []string) error {
+	return mounter.MountSensitive(source, target, fstype, options, sensitiveOptions /* sensitiveOptions */)
+}
+
 // MountSensitive is the same as Mount() but this method allows
 // sensitiveOptions to be passed in a separate parameter from the normal
 // mount options and ensures the sensitiveOptions are never logged. This