Merge pull request #68626 from gnufied/fix-netdev-mount-opt

Apply _netdev mount option in bind mount if available

pkg/util/mount/BUILD

@@ -68,6 +68,7 @@ go_test(
     srcs = [
         "exec_mount_test.go",
         "mount_linux_test.go",
+        "mount_test.go",
         "mount_windows_test.go",
         "nsenter_mount_test.go",
         "safe_format_and_mount_test.go",

pkg/util/mount/exec_mount.go

@@ -44,10 +44,10 @@ var _ Interface = &execMounter{}
 
 // Mount runs mount(8) using given exec interface.
 func (m *execMounter) Mount(source string, target string, fstype string, options []string) error {
-	bind, bindRemountOpts := isBind(options)
+	bind, bindOpts, bindRemountOpts := isBind(options)
 
 	if bind {
-		err := m.doExecMount(source, target, fstype, []string{"bind"})
+		err := m.doExecMount(source, target, fstype, bindOpts)
 		if err != nil {
 			return err
 		}

pkg/util/mount/mount.go

@@ -286,7 +286,7 @@ func IsNotMountPoint(mounter Interface, file string) (bool, error) {
 // use in case of bind mount, due to the fact that bind mount doesn't respect mount options.
 // The list equals:
 //   options - 'bind' + 'remount' (no duplicate)
-func isBind(options []string) (bool, []string) {
+func isBind(options []string) (bool, []string, []string) {
 	// Because we have an FD opened on the subpath bind mount, the "bind" option
 	// needs to be included, otherwise the mount target will error as busy if you
 	// remount as readonly.
@@ -295,8 +295,14 @@ func isBind(options []string) (bool, []string) {
 	// volume mount to be read only.
 	bindRemountOpts := []string{"bind", "remount"}
 	bind := false
+	bindOpts := []string{"bind"}
+
+	// _netdev is a userspace mount option and does not automatically get added when
+	// bind mount is created and hence we must carry it over.
+	if checkForNetDev(options) {
+		bindOpts = append(bindOpts, "_netdev")
+	}
 
-	if len(options) != 0 {
 	for _, option := range options {
 		switch option {
 		case "bind":
@@ -308,9 +314,17 @@ func isBind(options []string) (bool, []string) {
 			bindRemountOpts = append(bindRemountOpts, option)
 		}
 	}
+
+	return bind, bindOpts, bindRemountOpts
 }
 
-	return bind, bindRemountOpts
+func checkForNetDev(options []string) bool {
+	for _, option := range options {
+		if option == "_netdev" {
+			return true
+		}
+	}
+	return false
 }
 
 // TODO: this is a workaround for the unmount device issue caused by gci mounter.

pkg/util/mount/mount_linux.go

@@ -89,9 +89,9 @@ func (mounter *Mounter) Mount(source string, target string, fstype string, optio
 	// Path to mounter binary if containerized mounter is needed. Otherwise, it is set to empty.
 	// All Linux distros are expected to be shipped with a mount utility that a support bind mounts.
 	mounterPath := ""
-	bind, bindRemountOpts := isBind(options)
+	bind, bindOpts, bindRemountOpts := isBind(options)
 	if bind {
-		err := mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, []string{"bind"})
+		err := mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, bindOpts)
 		if err != nil {
 			return err
 		}

pkg/util/mount/mount_test.go

@@ -0,0 +1,60 @@
+/*
+Copyright 2018 The Kubernetes 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 mount
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestIsBind(t *testing.T) {
+	tests := []struct {
+		mountOption         []string
+		isBind              bool
+		expectedBindOpts    []string
+		expectedRemountOpts []string
+	}{
+		{
+			[]string{"vers=2", "ro", "_netdev"},
+			false,
+			[]string{},
+			[]string{},
+		},
+		{
+
+			[]string{"bind", "vers=2", "ro", "_netdev"},
+			true,
+			[]string{"bind", "_netdev"},
+			[]string{"bind", "remount", "vers=2", "ro", "_netdev"},
+		},
+	}
+	for _, test := range tests {
+		bind, bindOpts, bindRemountOpts := isBind(test.mountOption)
+		if bind != test.isBind {
+			t.Errorf("Expected bind to be %v but got %v", test.isBind, bind)
+		}
+		if test.isBind {
+			if !reflect.DeepEqual(test.expectedBindOpts, bindOpts) {
+				t.Errorf("Expected bind mount options to be %+v got %+v", test.expectedBindOpts, bindOpts)
+			}
+			if !reflect.DeepEqual(test.expectedRemountOpts, bindRemountOpts) {
+				t.Errorf("Expected remount options to be %+v got %+v", test.expectedRemountOpts, bindRemountOpts)
+			}
+		}
+
+	}
+}

pkg/util/mount/mount_windows.go

@@ -68,7 +68,7 @@ func (mounter *Mounter) Mount(source string, target string, fstype string, optio
 	bindSource := ""
 
 	// tell it's going to mount azure disk or azure file according to options
-	if bind, _ := isBind(options); bind {
+	if bind, _, _ := isBind(options); bind {
 		// mount azure disk
 		bindSource = normalizeWindowsPath(source)
 	} else {

pkg/util/mount/nsenter_mount.go

@@ -61,10 +61,10 @@ var _ = Interface(&NsenterMounter{})
 // Mount runs mount(8) in the host's root mount namespace.  Aside from this
 // aspect, Mount has the same semantics as the mounter returned by mount.New()
 func (n *NsenterMounter) Mount(source string, target string, fstype string, options []string) error {
-	bind, bindRemountOpts := isBind(options)
+	bind, bindOpts, bindRemountOpts := isBind(options)
 
 	if bind {
-		err := n.doNsenterMount(source, target, fstype, []string{"bind"})
+		err := n.doNsenterMount(source, target, fstype, bindOpts)
 		if err != nil {
 			return err
 		}