Merge pull request #295 from miaoyq/use-mount-lookup

Get the mountInfo by 'LookupMount' in containerd

pkg/os/os.go

@@ -43,7 +43,6 @@ type OS interface {
 	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
-	GetMounts() ([]*mount.Info, error)
 	LookupMount(path string) (containerdmount.Info, error)
 	DeviceUUID(device uint64) (string, error)
 }
@@ -121,11 +120,6 @@ func (RealOS) Unmount(target string, flags int) error {
 	return unix.Unmount(target, flags)
 }
 
-// GetMounts retrieves a list of mounts for the current running process.
-func (RealOS) GetMounts() ([]*mount.Info, error) {
-	return mount.GetMounts()
-}
-
 // LookupMount gets mount info of a given path.
 func (RealOS) LookupMount(path string) (containerdmount.Info, error) {
 	return containerdmount.Lookup(path)

pkg/os/testing/fake_os.go

@@ -22,7 +22,6 @@ import (
 	"sync"
 
 	containerdmount "github.com/containerd/containerd/mount"
-	"github.com/docker/docker/pkg/mount"
 	"golang.org/x/net/context"
 
 	osInterface "github.com/kubernetes-incubator/cri-containerd/pkg/os"
@@ -50,7 +49,6 @@ type FakeOS struct {
 	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
-	GetMountsFn           func() ([]*mount.Info, error)
 	LookupMountFn         func(path string) (containerdmount.Info, error)
 	DeviceUUIDFn          func(device uint64) (string, error)
 	calls                 []CalledDetail
@@ -231,19 +229,6 @@ func (f *FakeOS) Unmount(target string, flags int) error {
 	return nil
 }
 
-// GetMounts retrieves a list of mounts for the current running process.
-func (f *FakeOS) GetMounts() ([]*mount.Info, error) {
-	f.appendCalls("GetMounts")
-	if err := f.getError("GetMounts"); err != nil {
-		return nil, err
-	}
-
-	if f.GetMountsFn != nil {
-		return f.GetMountsFn()
-	}
-	return nil, nil
-}
-
 // LookupMount is a fake call that invokes LookupMountFn or just return nil.
 func (f *FakeOS) LookupMount(path string) (containerdmount.Info, error) {
 	f.appendCalls("LookupMount", path)

pkg/server/container_create.go

@@ -28,9 +28,9 @@ import (
 	"github.com/containerd/containerd/contrib/apparmor"
 	"github.com/containerd/containerd/contrib/seccomp"
 	"github.com/containerd/containerd/linux/runcopts"
+	"github.com/containerd/containerd/mount"
 	"github.com/containerd/containerd/namespaces"
 	"github.com/containerd/typeurl"
-	"github.com/docker/docker/pkg/mount"
 	"github.com/golang/glog"
 	imagespec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/opencontainers/runc/libcontainer/devices"
@@ -577,10 +577,6 @@ func (c *criContainerdService) addOCIBindMounts(g *generate.Generator, mounts []
 			return fmt.Errorf("failed to resolve symlink %q: %v", src, err)
 		}
 
-		mountInfos, err := c.os.GetMounts()
-		if err != nil {
-			return err
-		}
 		options := []string{"rbind"}
 		switch mount.GetPropagation() {
 		case runtime.MountPropagation_PROPAGATION_PRIVATE:
@@ -588,13 +584,13 @@ func (c *criContainerdService) addOCIBindMounts(g *generate.Generator, mounts []
 			// Since default root propogation in runc is rprivate ignore
 			// setting the root propagation
 		case runtime.MountPropagation_PROPAGATION_BIDIRECTIONAL:
-			if err := ensureShared(src, mountInfos); err != nil {
+			if err := ensureShared(src, c.os.LookupMount); err != nil {
 				return err
 			}
 			options = append(options, "rshared")
 			g.SetLinuxRootPropagation("rshared") // nolint: errcheck
 		case runtime.MountPropagation_PROPAGATION_HOST_TO_CONTAINER:
-			if err := ensureSharedOrSlave(src, mountInfos); err != nil {
+			if err := ensureSharedOrSlave(src, c.os.LookupMount); err != nil {
 				return err
 			}
 			options = append(options, "rslave")
@@ -818,31 +814,31 @@ func generateApparmorSpecOpts(apparmorProf string, privileged, apparmorEnabled b
 }
 
 // Ensure mount point on which path is mounted, is shared.
-func ensureShared(path string, mountInfos []*mount.Info) error {
+func ensureShared(path string, lookupMount func(string) (mount.Info, error)) error {
-	sourceMount, optionalOpts, err := getSourceMount(path, mountInfos)
+	mountInfo, err := lookupMount(path)
 	if err != nil {
 		return err
 	}
 
 	// Make sure source mount point is shared.
-	optsSplit := strings.Split(optionalOpts, " ")
+	optsSplit := strings.Split(mountInfo.Optional, " ")
 	for _, opt := range optsSplit {
 		if strings.HasPrefix(opt, "shared:") {
 			return nil
 		}
 	}
 
-	return fmt.Errorf("path %q is mounted on %q but it is not a shared mount", path, sourceMount)
+	return fmt.Errorf("path %q is mounted on %q but it is not a shared mount", path, mountInfo.Mountpoint)
 }
 
 // Ensure mount point on which path is mounted, is either shared or slave.
-func ensureSharedOrSlave(path string, mountInfos []*mount.Info) error {
+func ensureSharedOrSlave(path string, lookupMount func(string) (mount.Info, error)) error {
-	sourceMount, optionalOpts, err := getSourceMount(path, mountInfos)
+	mountInfo, err := lookupMount(path)
 	if err != nil {
 		return err
 	}
 	// Make sure source mount point is shared.
-	optsSplit := strings.Split(optionalOpts, " ")
+	optsSplit := strings.Split(mountInfo.Optional, " ")
 	for _, opt := range optsSplit {
 		if strings.HasPrefix(opt, "shared:") {
 			return nil
@@ -850,5 +846,5 @@ func ensureSharedOrSlave(path string, mountInfos []*mount.Info) error {
 			return nil
 		}
 	}
-	return fmt.Errorf("path %q is mounted on %q but it is not a shared or slave mount", path, sourceMount)
+	return fmt.Errorf("path %q is mounted on %q but it is not a shared or slave mount", path, mountInfo.Mountpoint)
 }

pkg/server/container_create_test.go

@@ -24,7 +24,7 @@ import (
 	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/contrib/apparmor"
 	"github.com/containerd/containerd/contrib/seccomp"
-	"github.com/docker/docker/pkg/mount"
+	"github.com/containerd/containerd/mount"
 	imagespec "github.com/opencontainers/image-spec/specs-go/v1"
 	runtimespec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/opencontainers/runtime-tools/generate"
@@ -585,36 +585,30 @@ func TestPrivilegedBindMount(t *testing.T) {
 }
 
 func TestMountPropagation(t *testing.T) {
-	sharedGetMountsFn := func() ([]*mount.Info, error) {
+	sharedLookupMountFn := func(string) (mount.Info, error) {
-		return []*mount.Info{
+		return mount.Info{
-			{
 			Mountpoint: "host-path",
 			Optional:   "shared:",
-			},
 		}, nil
 	}
 
-	slaveGetMountsFn := func() ([]*mount.Info, error) {
+	slaveLookupMountFn := func(string) (mount.Info, error) {
-		return []*mount.Info{
+		return mount.Info{
-			{
 			Mountpoint: "host-path",
 			Optional:   "master:",
-			},
 		}, nil
 	}
 
-	othersGetMountsFn := func() ([]*mount.Info, error) {
+	othersLookupMountFn := func(string) (mount.Info, error) {
-		return []*mount.Info{
+		return mount.Info{
-			{
 			Mountpoint: "host-path",
 			Optional:   "others",
-			},
 		}, nil
 	}
 
 	for desc, test := range map[string]struct {
 		criMount          *runtime.Mount
-		fakeGetMountsFn func() ([]*mount.Info, error)
+		fakeLookupMountFn func(string) (mount.Info, error)
 		optionsCheck      []string
 		expectErr         bool
 	}{
@@ -624,7 +618,7 @@ func TestMountPropagation(t *testing.T) {
 				HostPath:      "host-path",
 				Propagation:   runtime.MountPropagation_PROPAGATION_PRIVATE,
 			},
-			fakeGetMountsFn: nil,
+			fakeLookupMountFn: nil,
 			optionsCheck:      []string{"rbind", "rprivate"},
 			expectErr:         false,
 		},
@@ -634,7 +628,7 @@ func TestMountPropagation(t *testing.T) {
 				HostPath:      "host-path",
 				Propagation:   runtime.MountPropagation_PROPAGATION_HOST_TO_CONTAINER,
 			},
-			fakeGetMountsFn: slaveGetMountsFn,
+			fakeLookupMountFn: slaveLookupMountFn,
 			optionsCheck:      []string{"rbind", "rslave"},
 			expectErr:         false,
 		},
@@ -644,7 +638,7 @@ func TestMountPropagation(t *testing.T) {
 				HostPath:      "host-path",
 				Propagation:   runtime.MountPropagation_PROPAGATION_BIDIRECTIONAL,
 			},
-			fakeGetMountsFn: sharedGetMountsFn,
+			fakeLookupMountFn: sharedLookupMountFn,
 			optionsCheck:      []string{"rbind", "rshared"},
 			expectErr:         false,
 		},
@@ -654,7 +648,7 @@ func TestMountPropagation(t *testing.T) {
 				HostPath:      "host-path",
 				Propagation:   runtime.MountPropagation(42),
 			},
-			fakeGetMountsFn: nil,
+			fakeLookupMountFn: nil,
 			optionsCheck:      []string{"rbind", "rprivate"},
 			expectErr:         false,
 		},
@@ -664,7 +658,7 @@ func TestMountPropagation(t *testing.T) {
 				HostPath:      "host-path",
 				Propagation:   runtime.MountPropagation_PROPAGATION_BIDIRECTIONAL,
 			},
-			fakeGetMountsFn: slaveGetMountsFn,
+			fakeLookupMountFn: slaveLookupMountFn,
 			expectErr:         true,
 		},
 		"Expect an error if HostPath isn't slave or shared and mount propagation is MountPropagation_PROPAGATION_HOST_TO_CONTAINER": {
@@ -673,14 +667,14 @@ func TestMountPropagation(t *testing.T) {
 				HostPath:      "host-path",
 				Propagation:   runtime.MountPropagation_PROPAGATION_HOST_TO_CONTAINER,
 			},
-			fakeGetMountsFn: othersGetMountsFn,
+			fakeLookupMountFn: othersLookupMountFn,
 			expectErr:         true,
 		},
 	} {
 		t.Logf("TestCase %q", desc)
 		g := generate.New()
 		c := newTestCRIContainerdService()
-		c.os.(*ostesting.FakeOS).GetMountsFn = test.fakeGetMountsFn
+		c.os.(*ostesting.FakeOS).LookupMountFn = test.fakeLookupMountFn
 		err := c.addOCIBindMounts(&g, []*runtime.Mount{test.criMount}, "")
 		if test.expectErr {
 			require.Error(t, err)

pkg/server/helpers.go

@@ -30,7 +30,6 @@ import (
 	"github.com/containerd/containerd/content"
 	"github.com/containerd/containerd/errdefs"
 	"github.com/docker/distribution/reference"
-	"github.com/docker/docker/pkg/mount"
 	imagedigest "github.com/opencontainers/go-digest"
 	"github.com/opencontainers/image-spec/identity"
 	imagespec "github.com/opencontainers/image-spec/specs-go/v1"
@@ -405,39 +404,6 @@ func isInCRIMounts(dst string, mounts []*runtime.Mount) bool {
 	return false
 }
 
-//TODO: Replace with `mount.Lookup()`in containerd after #257 is marged
-func getMountInfo(mountInfos []*mount.Info, dir string) *mount.Info {
-	for _, m := range mountInfos {
-		if m.Mountpoint == dir {
-			return m
-		}
-	}
-	return nil
-}
-
-func getSourceMount(source string, mountInfos []*mount.Info) (string, string, error) {
-	mountinfo := getMountInfo(mountInfos, source)
-	if mountinfo != nil {
-		return source, mountinfo.Optional, nil
-	}
-
-	path := source
-	for {
-		path = filepath.Dir(path)
-		mountinfo = getMountInfo(mountInfos, path)
-		if mountinfo != nil {
-			return path, mountinfo.Optional, nil
-		}
-
-		if path == "/" {
-			break
-		}
-	}
-
-	// If we are here, we did not find parent mount. Something is wrong.
-	return "", "", fmt.Errorf("Could not find source mount of %s", source)
-}
-
 // filterLabel returns a label filter. Use `%q` here because containerd
 // filter needs extra quote to work properly.
 func filterLabel(k, v string) string {