From e881a291073a1106bea202ce9b58e2ec1cc049fe Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Thu, 13 Sep 2018 12:27:16 -0400 Subject: [PATCH] Apply _netdev mount option in bind mount if available _netdev mount option is a userspace mount option and isn't copied over when bind mount is created and remount also does not copies it over and hence must be explicitly used with bind mount --- pkg/util/mount/BUILD | 1 + pkg/util/mount/exec_mount.go | 4 +-- pkg/util/mount/mount.go | 40 +++++++++++++++------- pkg/util/mount/mount_linux.go | 4 +-- pkg/util/mount/mount_test.go | 60 +++++++++++++++++++++++++++++++++ pkg/util/mount/mount_windows.go | 2 +- pkg/util/mount/nsenter_mount.go | 4 +-- 7 files changed, 95 insertions(+), 20 deletions(-) create mode 100644 pkg/util/mount/mount_test.go diff --git a/pkg/util/mount/BUILD b/pkg/util/mount/BUILD index e8bf8b8617..8a7211ec4a 100644 --- a/pkg/util/mount/BUILD +++ b/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", diff --git a/pkg/util/mount/exec_mount.go b/pkg/util/mount/exec_mount.go index 3c6638328f..226f02704c 100644 --- a/pkg/util/mount/exec_mount.go +++ b/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 } diff --git a/pkg/util/mount/mount.go b/pkg/util/mount/mount.go index b48caaffbb..ffe9d2ee43 100644 --- a/pkg/util/mount/mount.go +++ b/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,22 +295,36 @@ func isBind(options []string) (bool, []string) { // volume mount to be read only. bindRemountOpts := []string{"bind", "remount"} bind := false + bindOpts := []string{"bind"} - if len(options) != 0 { - for _, option := range options { - switch option { - case "bind": - bind = true - break - case "remount": - break - default: - bindRemountOpts = append(bindRemountOpts, option) - } + // _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") + } + + for _, option := range options { + switch option { + case "bind": + bind = true + break + case "remount": + break + default: + bindRemountOpts = append(bindRemountOpts, option) } } - return bind, bindRemountOpts + return bind, bindOpts, 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. diff --git a/pkg/util/mount/mount_linux.go b/pkg/util/mount/mount_linux.go index 6b1c001044..183303b3e1 100644 --- a/pkg/util/mount/mount_linux.go +++ b/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 } diff --git a/pkg/util/mount/mount_test.go b/pkg/util/mount/mount_test.go new file mode 100644 index 0000000000..26ba8d79d3 --- /dev/null +++ b/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) + } + } + + } +} diff --git a/pkg/util/mount/mount_windows.go b/pkg/util/mount/mount_windows.go index b3206d18c1..738001a665 100644 --- a/pkg/util/mount/mount_windows.go +++ b/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 { diff --git a/pkg/util/mount/nsenter_mount.go b/pkg/util/mount/nsenter_mount.go index a798defe9b..6859e301a2 100644 --- a/pkg/util/mount/nsenter_mount.go +++ b/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 }