From c65921b16a4b8bbd0b91df59de97a581850fe78a Mon Sep 17 00:00:00 2001 From: Yanqiang Miao Date: Wed, 27 Sep 2017 20:54:20 +0800 Subject: [PATCH] Get the mountInfo by 'LookupMount' in containerd Signed-off-by: Yanqiang Miao --- pkg/os/os.go | 6 --- pkg/os/testing/fake_os.go | 15 ------ pkg/server/container_create.go | 26 +++++----- pkg/server/container_create_test.go | 74 +++++++++++++---------------- pkg/server/helpers.go | 34 ------------- 5 files changed, 45 insertions(+), 110 deletions(-) diff --git a/pkg/os/os.go b/pkg/os/os.go index 4f1ae4e28..2d7bc948a 100644 --- a/pkg/os/os.go +++ b/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) diff --git a/pkg/os/testing/fake_os.go b/pkg/os/testing/fake_os.go index 61338241b..33fd72459 100644 --- a/pkg/os/testing/fake_os.go +++ b/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) diff --git a/pkg/server/container_create.go b/pkg/server/container_create.go index 2a61bc4b9..9e5d36550 100644 --- a/pkg/server/container_create.go +++ b/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 { - sourceMount, optionalOpts, err := getSourceMount(path, mountInfos) +func ensureShared(path string, lookupMount func(string) (mount.Info, error)) error { + 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 { - sourceMount, optionalOpts, err := getSourceMount(path, mountInfos) +func ensureSharedOrSlave(path string, lookupMount func(string) (mount.Info, error)) error { + 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) } diff --git a/pkg/server/container_create_test.go b/pkg/server/container_create_test.go index 21f0fbd32..5c4309ad4 100644 --- a/pkg/server/container_create_test.go +++ b/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,38 +585,32 @@ func TestPrivilegedBindMount(t *testing.T) { } func TestMountPropagation(t *testing.T) { - sharedGetMountsFn := func() ([]*mount.Info, error) { - return []*mount.Info{ - { - Mountpoint: "host-path", - Optional: "shared:", - }, + sharedLookupMountFn := func(string) (mount.Info, error) { + return mount.Info{ + Mountpoint: "host-path", + Optional: "shared:", }, nil } - slaveGetMountsFn := func() ([]*mount.Info, error) { - return []*mount.Info{ - { - Mountpoint: "host-path", - Optional: "master:", - }, + slaveLookupMountFn := func(string) (mount.Info, error) { + return mount.Info{ + Mountpoint: "host-path", + Optional: "master:", }, nil } - othersGetMountsFn := func() ([]*mount.Info, error) { - return []*mount.Info{ - { - Mountpoint: "host-path", - Optional: "others", - }, + othersLookupMountFn := func(string) (mount.Info, error) { + return mount.Info{ + Mountpoint: "host-path", + Optional: "others", }, nil } for desc, test := range map[string]struct { - criMount *runtime.Mount - fakeGetMountsFn func() ([]*mount.Info, error) - optionsCheck []string - expectErr bool + criMount *runtime.Mount + fakeLookupMountFn func(string) (mount.Info, error) + optionsCheck []string + expectErr bool }{ "HostPath should mount as 'rprivate' if propagation is MountPropagation_PROPAGATION_PRIVATE": { criMount: &runtime.Mount{ @@ -624,9 +618,9 @@ func TestMountPropagation(t *testing.T) { HostPath: "host-path", Propagation: runtime.MountPropagation_PROPAGATION_PRIVATE, }, - fakeGetMountsFn: nil, - optionsCheck: []string{"rbind", "rprivate"}, - expectErr: false, + fakeLookupMountFn: nil, + optionsCheck: []string{"rbind", "rprivate"}, + expectErr: false, }, "HostPath should mount as 'rslave' if propagation is MountPropagation_PROPAGATION_HOST_TO_CONTAINER": { criMount: &runtime.Mount{ @@ -634,9 +628,9 @@ func TestMountPropagation(t *testing.T) { HostPath: "host-path", Propagation: runtime.MountPropagation_PROPAGATION_HOST_TO_CONTAINER, }, - fakeGetMountsFn: slaveGetMountsFn, - optionsCheck: []string{"rbind", "rslave"}, - expectErr: false, + fakeLookupMountFn: slaveLookupMountFn, + optionsCheck: []string{"rbind", "rslave"}, + expectErr: false, }, "HostPath should mount as 'rshared' if propagation is MountPropagation_PROPAGATION_BIDIRECTIONAL": { criMount: &runtime.Mount{ @@ -644,9 +638,9 @@ func TestMountPropagation(t *testing.T) { HostPath: "host-path", Propagation: runtime.MountPropagation_PROPAGATION_BIDIRECTIONAL, }, - fakeGetMountsFn: sharedGetMountsFn, - optionsCheck: []string{"rbind", "rshared"}, - expectErr: false, + fakeLookupMountFn: sharedLookupMountFn, + optionsCheck: []string{"rbind", "rshared"}, + expectErr: false, }, "HostPath should mount as 'rprivate' if propagation is illegal": { criMount: &runtime.Mount{ @@ -654,9 +648,9 @@ func TestMountPropagation(t *testing.T) { HostPath: "host-path", Propagation: runtime.MountPropagation(42), }, - fakeGetMountsFn: nil, - optionsCheck: []string{"rbind", "rprivate"}, - expectErr: false, + fakeLookupMountFn: nil, + optionsCheck: []string{"rbind", "rprivate"}, + expectErr: false, }, "Expect an error if HostPath isn't shared and mount propagation is MountPropagation_PROPAGATION_BIDIRECTIONAL": { criMount: &runtime.Mount{ @@ -664,8 +658,8 @@ func TestMountPropagation(t *testing.T) { HostPath: "host-path", Propagation: runtime.MountPropagation_PROPAGATION_BIDIRECTIONAL, }, - fakeGetMountsFn: slaveGetMountsFn, - expectErr: true, + fakeLookupMountFn: slaveLookupMountFn, + expectErr: true, }, "Expect an error if HostPath isn't slave or shared and mount propagation is MountPropagation_PROPAGATION_HOST_TO_CONTAINER": { criMount: &runtime.Mount{ @@ -673,14 +667,14 @@ func TestMountPropagation(t *testing.T) { HostPath: "host-path", Propagation: runtime.MountPropagation_PROPAGATION_HOST_TO_CONTAINER, }, - fakeGetMountsFn: othersGetMountsFn, - expectErr: true, + 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) diff --git a/pkg/server/helpers.go b/pkg/server/helpers.go index a6e716e5a..f89d6df14 100644 --- a/pkg/server/helpers.go +++ b/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 {