Change mount.Interface.Mount to exec('mount'), instead of syscall

pkg/util/mount/fake.go

@@ -38,13 +38,13 @@ func (f *FakeMounter) ResetLog() {
 	f.Log = []FakeAction{}
 }
 
-func (f *FakeMounter) Mount(source string, target string, fstype string, flags uintptr, data string) error {
+func (f *FakeMounter) Mount(source string, target string, fstype string, options []string) error {
 	f.MountPoints = append(f.MountPoints, MountPoint{Device: source, Path: target, Type: fstype})
 	f.Log = append(f.Log, FakeAction{Action: FakeActionMount, Target: target, Source: source, FSType: fstype})
 	return nil
 }
 
-func (f *FakeMounter) Unmount(target string, flags int) error {
+func (f *FakeMounter) Unmount(target string, options []string) error {
 	newMountpoints := []MountPoint{}
 	for _, mp := range f.MountPoints {
 		if mp.Path != target {

pkg/util/mount/mount.go

@@ -18,14 +18,11 @@ limitations under the License.
 // an alternate platform, we will need to abstract further.
 package mount
 
-// Each supported platform must define the following flags:
-//   - FlagBind: specifies a bind mount
-//   - FlagReadOnly: the mount will be read-only
 type Interface interface {
-	// Mount wraps syscall.Mount().
-	Mount(source string, target string, fstype string, flags uintptr, data string) error
-	// Umount wraps syscall.Mount().
-	Unmount(target string, flags int) error
+	// Mount mounts source to target as fstype with given options.
+	Mount(source string, target string, fstype string, options []string) error
+	// Unmount unmounts target with given options.
+	Unmount(target string, options []string) error
 	// List returns a list of all mounted filesystems.  This can be large.
 	// On some platforms, reading mounts is not guaranteed consistent (i.e.
 	// it could change between chunked reads). This is guaranteed to be

pkg/util/mount/mount_linux.go

@@ -24,35 +24,88 @@ import (
 	"hash/adler32"
 	"io"
 	"os"
+	"os/exec"
 	"strconv"
 	"strings"
 	"syscall"
 
 	"github.com/golang/glog"
 )
 
-const FlagBind = syscall.MS_BIND
-const FlagReadOnly = syscall.MS_RDONLY
+const (
+	// How many times to retry for a consistent read of /proc/mounts.
+	maxListTries = 3
+	// Number of fields per line in "/proc/mounts", as per the fstab man page.
+	expectedNumFieldsPerLine = 6
+)
 
 // Mounter implements mount.Interface for linux platform.
 type Mounter struct{}
 
-// Mount wraps syscall.Mount()
-func (mounter *Mounter) Mount(source string, target string, fstype string, flags uintptr, data string) error {
-	glog.V(5).Infof("Mounting %s %s %s %d %s", source, target, fstype, flags, data)
-	return syscall.Mount(source, target, fstype, flags, data)
+// Mount mounts source to target as fstype with given options. 'source' and 'fstype' must
+// be an emtpy string in case it's not required, e.g. for remount, or for auto filesystem
+// type, where kernel handles fs type for you. The mount 'options' currently come from
+// mount(8), if no more option is required, call Mount with an empty string list or nil.
+func (mounter *Mounter) Mount(source string, target string, fstype string, options []string) error {
+	glog.V(5).Infof("Mounting %s %s %s %v", source, target, fstype, options)
+	// Build mount command as follows:
+	//   mount [-t $fstype] [-o $options] [$source] $target
+	mountArgs := []string{}
+	if len(fstype) > 0 {
+		mountArgs = append(mountArgs, "-t", fstype)
+	}
+	if options != nil {
+		// If options is an empty list, this is a no-op.
+		mountArgs = append(mountArgs, options...)
+	}
+	if len(source) > 0 {
+		mountArgs = append(mountArgs, source, target)
+	} else {
+		mountArgs = append(mountArgs, target)
+	}
+	command := exec.Command("mount", mountArgs...)
+	output, err := command.CombinedOutput()
+	if err != nil {
+		glog.Errorf("Mount failed: %v\nMounting arguments: %s %s %s %v\nOutput: %s\n",
+			err, source, target, fstype, options, string(output))
+		return err
+	}
+	// Do a remount if caller asks for readonly bind - the previous mount call
+	// will NOT respect readonly option and will mount target as read/write.
+	if isReadOnlyBind(options) {
+		glog.V(5).Infof("Remounting %s as readonly bind", target)
+		command = exec.Command("mount", "-o", "remount,ro", target)
+		output, err = command.CombinedOutput()
+		if err != nil {
+			glog.Errorf("Remount failed: %v\nMounting arguments: mount -o remount,ro %s\nOutput: %s",
+				err, target, string(output))
+			return err
+		}
+	}
+	return nil
 }
 
-// Unmount wraps syscall.Unmount()
-func (mounter *Mounter) Unmount(target string, flags int) error {
-	return syscall.Unmount(target, flags)
+// Unmount unmounts target with given options.
+func (mounter *Mounter) Unmount(target string, options []string) error {
+	glog.V(5).Infof("Unmounting %s %v", target, options)
+	unmountArgs := []string{}
+	if options != nil {
+		// If options is an empty list, this is a no-op.
+		unmountArgs = append(unmountArgs, options...)
+	}
+	unmountArgs = append(unmountArgs, target)
+	command := exec.Command("umount", unmountArgs...)
+	output, err := command.CombinedOutput()
+	if err != nil {
+		glog.Errorf("Unmount failed: %v\nUnmounting arguments: %s %v\nOutput: %s\n",
+			err, target, options, string(output))
+		return err
+	}
+	return nil
 }
 
-// How many times to retry for a consistent read of /proc/mounts.
-const maxListTries = 3
-
 // List returns a list of all mounted filesystems.
-func (*Mounter) List() ([]MountPoint, error) {
+func (mounter *Mounter) List() ([]MountPoint, error) {
 	hash1, err := readProcMounts(nil)
 	if err != nil {
 		return nil, err
@@ -74,7 +127,7 @@ func (*Mounter) List() ([]MountPoint, error) {
 }
 
 // IsMountPoint determines if a directory is a mountpoint, by comparing the device for the
-// directory with the device for it's parent.  If they are the same, it's not a mountpoint,
+// directory with the device for it's parent. If they are the same, it's not a mountpoint,
 // if they're different, it is.
 func (mounter *Mounter) IsMountPoint(file string) (bool, error) {
 	stat, err := os.Stat(file)
@@ -89,9 +142,6 @@ func (mounter *Mounter) IsMountPoint(file string) (bool, error) {
 	return stat.Sys().(*syscall.Stat_t).Dev != rootStat.Sys().(*syscall.Stat_t).Dev, nil
 }
 
-// As per the fstab man page.
-const expectedNumFieldsPerLine = 6
-
 // readProcMounts reads /proc/mounts and produces a hash of the contents.  If the out
 // argument is not nil, this fills it with MountPoint structs.
 func readProcMounts(out *[]MountPoint) (uint32, error) {
@@ -143,3 +193,18 @@ func readProcMountsFrom(file io.Reader, out *[]MountPoint) (uint32, error) {
 	}
 	return hash.Sum32(), nil
 }
+
+func isReadOnlyBind(options []string) bool {
+	readOnly := false
+	bind := false
+	for _, option := range options {
+		if strings.Contains(option, "bind") {
+			bind = true
+		}
+		if strings.Contains(option, "ro") {
+			readOnly = true
+		}
+	}
+
+	return bind && readOnly
+}

pkg/util/mount/mount_unsupported.go

@@ -18,16 +18,13 @@ limitations under the License.
 
 package mount
 
-const FlagBind = 0
-const FlagReadOnly = 0
-
 type Mounter struct{}
 
-func (mounter *Mounter) Mount(source string, target string, fstype string, flags uintptr, data string) error {
+func (mounter *Mounter) Mount(source string, target string, fstype string, options []string) error {
 	return nil
 }
 
-func (mounter *Mounter) Unmount(target string, flags int) error {
+func (mounter *Mounter) Unmount(target string, options []string) error {
 	return nil
 }
 

pkg/volume/empty_dir/empty_dir.go

@@ -195,7 +195,7 @@ func (ed *emptyDir) setupTmpfs(dir string) error {
 	if isMnt && medium == mediumMemory {
 		return nil // current state is what we expect
 	}
-	return ed.mounter.Mount("tmpfs", dir, "tmpfs", 0, "")
+	return ed.mounter.Mount("tmpfs", dir, "tmpfs", nil)
 }
 
 func (ed *emptyDir) GetPath() string {
@@ -242,7 +242,7 @@ func (ed *emptyDir) teardownTmpfs(dir string) error {
 	if ed.mounter == nil {
 		return fmt.Errorf("memory storage requested, but mounter is nil")
 	}
-	if err := ed.mounter.Unmount(dir, 0); err != nil {
+	if err := ed.mounter.Unmount(dir, nil); err != nil {
 		return err
 	}
 	if err := os.RemoveAll(dir); err != nil {

pkg/volume/gce_pd/gce_pd.go

@@ -197,27 +197,26 @@ func (pd *gcePersistentDisk) SetUpAt(dir string) error {
 		return err
 	}
 
-	flags := uintptr(0)
-	if pd.readOnly {
-		flags = mount.FlagReadOnly
-	}
-
 	if err := os.MkdirAll(dir, 0750); err != nil {
 		// TODO: we should really eject the attach/detach out into its own control loop.
 		detachDiskLogError(pd)
 		return err
 	}
 
 	// Perform a bind mount to the full path to allow duplicate mounts of the same PD.
-	err = pd.mounter.Mount(globalPDPath, dir, "", mount.FlagBind|flags, "")
+	options := []string{"--bind"}
+	if pd.readOnly {
+		options = append(options, "-o", "ro")
+	}
+	err = pd.mounter.Mount(globalPDPath, dir, "", options)
 	if err != nil {
 		mountpoint, mntErr := pd.mounter.IsMountPoint(dir)
 		if mntErr != nil {
 			glog.Errorf("isMountpoint check failed: %v", mntErr)
 			return err
 		}
 		if mountpoint {
-			if mntErr = pd.mounter.Unmount(dir, 0); mntErr != nil {
+			if mntErr = pd.mounter.Unmount(dir, nil); mntErr != nil {
 				glog.Errorf("Failed to unmount: %v", mntErr)
 				return err
 			}
@@ -275,7 +274,7 @@ func (pd *gcePersistentDisk) TearDownAt(dir string) error {
 		return err
 	}
 	// Unmount the bind-mount inside this pod
-	if err := pd.mounter.Unmount(dir, 0); err != nil {
+	if err := pd.mounter.Unmount(dir, nil); err != nil {
 		return err
 	}
 	// If len(refs) is 1, then all bind mounts have been removed, and the

pkg/volume/gce_pd/gce_util.go

@@ -39,10 +39,6 @@ func (util *GCEDiskUtil) AttachAndMountDisk(pd *gcePersistentDisk, globalPDPath
 	if err != nil {
 		return err
 	}
-	flags := uintptr(0)
-	if pd.readOnly {
-		flags = mount.FlagReadOnly
-	}
 	if err := gce.(*gce_cloud.GCECloud).AttachDisk(pd.pdName, pd.readOnly); err != nil {
 		return err
 	}
@@ -79,8 +75,12 @@ func (util *GCEDiskUtil) AttachAndMountDisk(pd *gcePersistentDisk, globalPDPath
 			return err
 		}
 	}
+	options := []string{}
+	if pd.readOnly {
+		options = append(options, "-o", "ro")
+	}
 	if !mountpoint {
-		err = pd.diskMounter.Mount(devicePath, globalPDPath, pd.fsType, flags, "")
+		err = pd.diskMounter.Mount(devicePath, globalPDPath, pd.fsType, options)
 		if err != nil {
 			os.Remove(globalPDPath)
 			return err
@@ -93,7 +93,7 @@ func (util *GCEDiskUtil) AttachAndMountDisk(pd *gcePersistentDisk, globalPDPath
 func (util *GCEDiskUtil) DetachDisk(pd *gcePersistentDisk) error {
 	// Unmount the global PD mount, which should be the only one.
 	globalPDPath := makeGlobalPDName(pd.plugin.host, pd.pdName)
-	if err := pd.mounter.Unmount(globalPDPath, 0); err != nil {
+	if err := pd.mounter.Unmount(globalPDPath, nil); err != nil {
 		return err
 	}
 	if err := os.Remove(globalPDPath); err != nil {
@@ -120,18 +120,20 @@ type gceSafeFormatAndMount struct {
 }
 
 // uses /usr/share/google/safe_format_and_mount to optionally mount, and format a disk
-func (mounter *gceSafeFormatAndMount) Mount(source string, target string, fstype string, flags uintptr, data string) error {
+func (mounter *gceSafeFormatAndMount) Mount(source string, target string, fstype string, options []string) error {
 	// Don't attempt to format if mounting as readonly. Go straight to mounting.
-	if (flags & mount.FlagReadOnly) != 0 {
-		return mounter.Interface.Mount(source, target, fstype, flags, data)
+	for _, option := range options {
+		if option == "ro" {
+			return mounter.Interface.Mount(source, target, fstype, options)
+		}
 	}
 	args := []string{}
 	// ext4 is the default for safe_format_and_mount
 	if len(fstype) > 0 && fstype != "ext4" {
 		args = append(args, "-m", fmt.Sprintf("mkfs.%s", fstype))
 	}
+	args = append(args, options...)
 	args = append(args, source, target)
-	// TODO: Accept other options here?
 	glog.V(5).Infof("exec-ing: /usr/share/google/safe_format_and_mount %v", args)
 	cmd := mounter.runner.Command("/usr/share/google/safe_format_and_mount", args...)
 	dataOut, err := cmd.CombinedOutput()

pkg/volume/gce_pd/gce_util_test.go

@@ -64,7 +64,7 @@ func TestSafeFormatAndMount(t *testing.T) {
 			runner: &fake,
 		}
 
-		err := mounter.Mount("/dev/foo", "/mnt/bar", test.fstype, 0, "")
+		err := mounter.Mount("/dev/foo", "/mnt/bar", test.fstype, nil)
 		if test.err == nil && err != nil {
 			t.Errorf("unexpected error: %v", err)
 		}