Merge pull request #754 from kolyshkin/mount

os.Unmount: do not consult mountinfo

pkg/os/os.go

@@ -22,9 +22,8 @@ import (
 	"os"
 	"path/filepath"
 
-	containerdmount "github.com/containerd/containerd/mount"
+	"github.com/containerd/containerd/mount"
 	"github.com/containerd/fifo"
-	"github.com/docker/docker/pkg/mount"
 	"github.com/docker/docker/pkg/symlink"
 	"golang.org/x/net/context"
 	"golang.org/x/sys/unix"
@@ -42,8 +41,8 @@ type OS interface {
 	CopyFile(src, dest string, perm os.FileMode) error
 	WriteFile(filename string, data []byte, perm os.FileMode) error
 	Mount(source string, target string, fstype string, flags uintptr, data string) error
-	Unmount(target string, flags int) error
-	LookupMount(path string) (containerdmount.Info, error)
+	Unmount(target string) error
+	LookupMount(path string) (mount.Info, error)
 }
 
 // RealOS is used to dispatch the real system level operations.
@@ -115,20 +114,23 @@ func (RealOS) Mount(source string, target string, fstype string, flags uintptr,
 }
 
 // Unmount will call Unmount to unmount the file.
-func (RealOS) Unmount(target string, flags int) error {
-	return Unmount(target, flags)
+func (RealOS) Unmount(target string) error {
+	return Unmount(target)
 }
 
 // LookupMount gets mount info of a given path.
-func (RealOS) LookupMount(path string) (containerdmount.Info, error) {
-	return containerdmount.Lookup(path)
+func (RealOS) LookupMount(path string) (mount.Info, error) {
+	return mount.Lookup(path)
 }
 
-// Unmount will call unix.Unmount to unmount the file. The function doesn't
-// return error if target is not mounted.
-func Unmount(target string, flags int) error {
-	if mounted, err := mount.Mounted(target); err != nil || !mounted {
-		return err
+// Unmount unmounts the target. It does not return an error in case the target is not mounted.
+// In case the target does not exist, the appropriate error is returned.
+func Unmount(target string) error {
+	err := unix.Unmount(target, unix.MNT_DETACH)
+	if err == unix.EINVAL {
+		// ignore "not mounted" error
+		err = nil
 	}
-	return unix.Unmount(target, flags)
+
+	return err
 }

pkg/os/testing/fake_os.go

@@ -49,7 +49,7 @@ type FakeOS struct {
 	CopyFileFn             func(string, string, os.FileMode) error
 	WriteFileFn            func(string, []byte, os.FileMode) error
 	MountFn                func(source string, target string, fstype string, flags uintptr, data string) error
-	UnmountFn              func(target string, flags int) error
+	UnmountFn              func(target string) error
 	LookupMountFn          func(path string) (containerdmount.Info, error)
 	calls                  []CalledDetail
 	errors                 map[string]error
@@ -230,14 +230,14 @@ func (f *FakeOS) Mount(source string, target string, fstype string, flags uintpt
 }
 
 // Unmount is a fake call that invokes UnmountFn or just return nil.
-func (f *FakeOS) Unmount(target string, flags int) error {
-	f.appendCalls("Unmount", target, flags)
+func (f *FakeOS) Unmount(target string) error {
+	f.appendCalls("Unmount", target)
 	if err := f.getError("Unmount"); err != nil {
 		return err
 	}
 
 	if f.UnmountFn != nil {
-		return f.UnmountFn(target, flags)
+		return f.UnmountFn(target)
 	}
 	return nil
 }

pkg/server/sandbox_run.go

@@ -494,16 +494,14 @@ func parseDNSOptions(servers, searches, options []string) (string, error) {
 }
 
 // unmountSandboxFiles unmount some sandbox files, we rely on the removal of sandbox root directory to
-// remove these files. Unmount should *NOT* return error when:
-//  1) The mount point is already unmounted.
-//  2) The mount point doesn't exist.
+// remove these files. Unmount should *NOT* return error if the mount point is already unmounted.
 func (c *criService) unmountSandboxFiles(id string, config *runtime.PodSandboxConfig) error {
 	if config.GetLinux().GetSecurityContext().GetNamespaceOptions().GetIpc() != runtime.NamespaceMode_NODE {
 		path, err := c.os.FollowSymlinkInScope(c.getSandboxDevShm(id), "/")
 		if err != nil {
 			return errors.Wrap(err, "failed to follow symlink")
 		}
-		if err := c.os.Unmount(path, unix.MNT_DETACH); err != nil && !os.IsNotExist(err) {
+		if err := c.os.Unmount(path); err != nil && !os.IsNotExist(err) {
 			return errors.Wrapf(err, "failed to unmount %q", path)
 		}
 	}

pkg/store/sandbox/netns.go

@@ -23,7 +23,6 @@ import (
 	cnins "github.com/containernetworking/plugins/pkg/ns"
 	"github.com/docker/docker/pkg/symlink"
 	"github.com/pkg/errors"
-	"golang.org/x/sys/unix"
 
 	osinterface "github.com/containerd/cri/pkg/os"
 )
@@ -93,7 +92,7 @@ func (n *NetNS) Remove() error {
 		if err != nil {
 			return errors.Wrap(err, "failed to follow symlink")
 		}
-		if err := osinterface.Unmount(path, unix.MNT_DETACH); err != nil && !os.IsNotExist(err) {
+		if err := osinterface.Unmount(path); err != nil && !os.IsNotExist(err) {
 			return errors.Wrap(err, "failed to umount netns")
 		}
 		if err := os.RemoveAll(path); err != nil {