Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Factor mount utility code out gce_pd volume plugin #5036

Merged
merged 1 commit into from Mar 5, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 6 additions & 6 deletions pkg/kubelet/volume/gce_pd/gce_pd.go
Expand Up @@ -170,7 +170,7 @@ func (pd *gcePersistentDisk) SetUp() error {
}

// TODO: handle failed mounts here.
mountpoint, err := isMountPoint(pd.GetPath())
mountpoint, err := mount.IsMountPoint(pd.GetPath())
glog.V(4).Infof("PersistentDisk set up: %s %v %v", pd.GetPath(), mountpoint, err)
if err != nil && !os.IsNotExist(err) {
return err
Expand Down Expand Up @@ -199,7 +199,7 @@ func (pd *gcePersistentDisk) SetUp() error {
// Perform a bind mount to the full path to allow duplicate mounts of the same PD.
err = pd.mounter.Mount(globalPDPath, pd.GetPath(), "", mount.FlagBind|flags, "")
if err != nil {
mountpoint, mntErr := isMountPoint(pd.GetPath())
mountpoint, mntErr := mount.IsMountPoint(pd.GetPath())
if mntErr != nil {
glog.Errorf("isMountpoint check failed: %v", mntErr)
return err
Expand All @@ -209,7 +209,7 @@ func (pd *gcePersistentDisk) SetUp() error {
glog.Errorf("Failed to unmount: %v", mntErr)
return err
}
mountpoint, mntErr := isMountPoint(pd.GetPath())
mountpoint, mntErr := mount.IsMountPoint(pd.GetPath())
if mntErr != nil {
glog.Errorf("isMountpoint check failed: %v", mntErr)
return err
Expand Down Expand Up @@ -244,15 +244,15 @@ func (pd *gcePersistentDisk) GetPath() string {
// Unmounts the bind mount, and detaches the disk only if the PD
// resource was the last reference to that disk on the kubelet.
func (pd *gcePersistentDisk) TearDown() error {
mountpoint, err := isMountPoint(pd.GetPath())
mountpoint, err := mount.IsMountPoint(pd.GetPath())
if err != nil {
return err
}
if !mountpoint {
return os.Remove(pd.GetPath())
}

refs, err := getMountRefs(pd.mounter, pd.GetPath())
refs, err := mount.GetMountRefs(pd.mounter, pd.GetPath())
if err != nil {
return err
}
Expand All @@ -269,7 +269,7 @@ func (pd *gcePersistentDisk) TearDown() error {
return err
}
}
mountpoint, mntErr := isMountPoint(pd.GetPath())
mountpoint, mntErr := mount.IsMountPoint(pd.GetPath())
if mntErr != nil {
glog.Errorf("isMountpoint check failed: %v", mntErr)
return err
Expand Down
5 changes: 3 additions & 2 deletions pkg/kubelet/volume/gce_pd/gce_pd_test.go
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
"github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/volume"
"github.com/GoogleCloudPlatform/kubernetes/pkg/types"
"github.com/GoogleCloudPlatform/kubernetes/pkg/util/mount"
)

func TestCanSupport(t *testing.T) {
Expand Down Expand Up @@ -80,7 +81,7 @@ func TestPlugin(t *testing.T) {
},
},
}
builder, err := plug.(*gcePersistentDiskPlugin).newBuilderInternal(spec, types.UID("poduid"), &fakePDManager{}, &fakeMounter{})
builder, err := plug.(*gcePersistentDiskPlugin).newBuilderInternal(spec, types.UID("poduid"), &fakePDManager{}, &mount.FakeMounter{})
if err != nil {
t.Errorf("Failed to make a new Builder: %v", err)
}
Expand Down Expand Up @@ -111,7 +112,7 @@ func TestPlugin(t *testing.T) {
}
}

cleaner, err := plug.(*gcePersistentDiskPlugin).newCleanerInternal("vol1", types.UID("poduid"), &fakePDManager{}, &fakeMounter{})
cleaner, err := plug.(*gcePersistentDiskPlugin).newCleanerInternal("vol1", types.UID("poduid"), &fakePDManager{}, &mount.FakeMounter{})
if err != nil {
t.Errorf("Failed to make a new Cleaner: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/kubelet/volume/gce_pd/gce_util.go
Expand Up @@ -68,7 +68,7 @@ func (util *GCEDiskUtil) AttachAndMountDisk(pd *gcePersistentDisk, globalPDPath
}

// Only mount the PD globally once.
mountpoint, err := isMountPoint(globalPDPath)
mountpoint, err := mount.IsMountPoint(globalPDPath)
if err != nil {
if os.IsNotExist(err) {
if err := os.MkdirAll(globalPDPath, 0750); err != nil {
Expand Down
48 changes: 0 additions & 48 deletions pkg/kubelet/volume/gce_pd/mount_util.go

This file was deleted.

99 changes: 0 additions & 99 deletions pkg/kubelet/volume/gce_pd/mount_util_test.go

This file was deleted.

34 changes: 34 additions & 0 deletions pkg/util/mount/fake.go
@@ -0,0 +1,34 @@
/*
Copyright 2015 Google Inc. All rights reserved.

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

// FakeMounter implements mount.Interface.
type FakeMounter struct {
mountPoints []MountPoint
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is one supposed to set the fake mount points if it is a private field?

}

func (f *FakeMounter) Mount(source string, target string, fstype string, flags uintptr, data string) error {
return nil
}

func (f *FakeMounter) Unmount(target string, flags int) error {
return nil
}

func (f *FakeMounter) List() ([]MountPoint, error) {
return f.mountPoints, nil
}
27 changes: 27 additions & 0 deletions pkg/util/mount/mount.go
Expand Up @@ -49,3 +49,30 @@ type MountPoint struct {
Freq int
Pass int
}

// Examines /proc/mounts to find all other references to the device referenced
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not just create a mount_util.go?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rootfs things are in the files they are to ensure that the build flags for
each function / type didn't change across this commit.
On Wed, Mar 4, 2015 at 7:58 PM Huamin Chen notifications@github.com wrote:

In pkg/util/mount/mount.go
#5036 (comment)
:

@@ -49,3 +49,30 @@ type MountPoint struct {
Freq int
Pass int
}
+
+// Examines /proc/mounts to find all other references to the device referenced

why not just create a mount_util.go?


Reply to this email directly or view it on GitHub
https://github.com/GoogleCloudPlatform/kubernetes/pull/5036/files#r25830927
.

// by mountPath.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment should detail that it returns a list of paths.

func GetMountRefs(mounter Interface, mountPath string) ([]string, error) {
mps, err := mounter.List()
if err != nil {
return nil, err
}

// Find the device name.
deviceName := ""
for i := range mps {
if mps[i].Path == mountPath {
deviceName = mps[i].Device
break
}
}

// Find all references to the device.
var refs []string
for i := range mps {
if mps[i].Device == deviceName && mps[i].Path != mountPath {
refs = append(refs, mps[i].Path)
}
}
return refs, nil
}
File renamed without changes.
Expand Up @@ -91,3 +91,63 @@ func slicesEqual(a, b []string) bool {
}
return true
}

func TestGetMountRefs(t *testing.T) {
fm := &FakeMounter{
[]MountPoint{
{Device: "/dev/sdb", Path: "/var/lib/kubelet/plugins/kubernetes.io/gce-pd/mounts/gce-pd"},
{Device: "/dev/sdb", Path: "/var/lib/kubelet/pods/some-pod/volumes/kubernetes.io~gce-pd/gce-pd-in-pod"},
{Device: "/dev/sdc", Path: "/var/lib/kubelet/plugins/kubernetes.io/gce-pd/mounts/gce-pd2"},
{Device: "/dev/sdc", Path: "/var/lib/kubelet/pods/some-pod/volumes/kubernetes.io~gce-pd/gce-pd2-in-pod"},
{Device: "/dev/sdc", Path: "/var/lib/kubelet/pods/some-pod/volumes/kubernetes.io~gce-pd/gce-pd2-in-pod2"},
},
}

tests := []struct {
mountPath string
expectedRefs []string
}{
{
"/var/lib/kubelet/pods/some-pod/volumes/kubernetes.io~gce-pd/gce-pd-in-pod",
[]string{
"/var/lib/kubelet/plugins/kubernetes.io/gce-pd/mounts/gce-pd",
},
},
{
"/var/lib/kubelet/pods/some-pod/volumes/kubernetes.io~gce-pd/gce-pd2-in-pod",
[]string{
"/var/lib/kubelet/pods/some-pod/volumes/kubernetes.io~gce-pd/gce-pd2-in-pod2",
"/var/lib/kubelet/plugins/kubernetes.io/gce-pd/mounts/gce-pd2",
},
},
}

for i, test := range tests {
if refs, err := GetMountRefs(fm, test.mountPath); err != nil || !setEquivalent(test.expectedRefs, refs) {
t.Errorf("%d. getMountRefs(%q) = %v, %v; expected %v, nil", i, test.mountPath, refs, err, test.expectedRefs)
}
}
}

func setEquivalent(set1, set2 []string) bool {
map1 := make(map[string]bool)
map2 := make(map[string]bool)
for _, s := range set1 {
map1[s] = true
}
for _, s := range set2 {
map2[s] = true
}

for s := range map1 {
if !map2[s] {
return false
}
}
for s := range map2 {
if !map1[s] {
return false
}
}
return true
}
File renamed without changes.
Expand Up @@ -16,7 +16,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package gce_pd
package mount

import (
"os"
Expand All @@ -26,7 +26,7 @@ import (
// Determine 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, if they're
// different, it is.
func isMountPoint(file string) (bool, error) {
func IsMountPoint(file string) (bool, error) {
stat, err := os.Stat(file)
if err != nil {
return false, err
Expand Down