Move unmount volume util from pkg/volume/util to pkg/util/mount

pkg/util/mount/BUILD

@@ -9,6 +9,7 @@ go_library(
         "exec_mount_unsupported.go",
         "fake.go",
         "mount.go",
+        "mount_helper.go",
         "mount_linux.go",
         "mount_unsupported.go",
         "mount_windows.go",
@@ -67,6 +68,7 @@ go_test(
     name = "go_default_test",
     srcs = [
         "exec_mount_test.go",
+        "mount_helper_test.go",
         "mount_linux_test.go",
         "mount_test.go",
         "mount_windows_test.go",

pkg/util/mount/mount_helper.go

@@ -0,0 +1,126 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+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
+
+import (
+	"fmt"
+	"os"
+	"syscall"
+
+	"k8s.io/klog"
+)
+
+// UnmountPath is a common unmount routine that unmounts the given path and
+// deletes the remaining directory if successful.
+func UnmountPath(mountPath string, mounter Interface) error {
+	return UnmountMountPoint(mountPath, mounter, false /* extensiveMountPointCheck */)
+}
+
+// UnmountMountPoint is a common unmount routine that unmounts the given path and
+// deletes the remaining directory if successful.
+// if extensiveMountPointCheck is true
+// IsNotMountPoint will be called instead of IsLikelyNotMountPoint.
+// IsNotMountPoint is more expensive but properly handles bind mounts.
+func UnmountMountPoint(mountPath string, mounter Interface, extensiveMountPointCheck bool) error {
+	pathExists, pathErr := PathExists(mountPath)
+	if !pathExists {
+		klog.Warningf("Warning: Unmount skipped because path does not exist: %v", mountPath)
+		return nil
+	}
+	corruptedMnt := IsCorruptedMnt(pathErr)
+	if pathErr != nil && !corruptedMnt {
+		return fmt.Errorf("Error checking path: %v", pathErr)
+	}
+	return doUnmountMountPoint(mountPath, mounter, extensiveMountPointCheck, corruptedMnt)
+}
+
+// doUnmountMountPoint is a common unmount routine that unmounts the given path and
+// deletes the remaining directory if successful.
+// if extensiveMountPointCheck is true
+// IsNotMountPoint will be called instead of IsLikelyNotMountPoint.
+// IsNotMountPoint is more expensive but properly handles bind mounts.
+// if corruptedMnt is true, it means that the mountPath is a corrupted mountpoint, Take it as an argument for convenience of testing
+func doUnmountMountPoint(mountPath string, mounter Interface, extensiveMountPointCheck bool, corruptedMnt bool) error {
+	if !corruptedMnt {
+		var notMnt bool
+		var err error
+		if extensiveMountPointCheck {
+			notMnt, err = IsNotMountPoint(mounter, mountPath)
+		} else {
+			notMnt, err = mounter.IsLikelyNotMountPoint(mountPath)
+		}
+
+		if err != nil {
+			return err
+		}
+
+		if notMnt {
+			klog.Warningf("Warning: %q is not a mountpoint, deleting", mountPath)
+			return os.Remove(mountPath)
+		}
+	}
+
+	// Unmount the mount path
+	klog.V(4).Infof("%q is a mountpoint, unmounting", mountPath)
+	if err := mounter.Unmount(mountPath); err != nil {
+		return err
+	}
+
+	notMnt, mntErr := mounter.IsLikelyNotMountPoint(mountPath)
+	if mntErr != nil {
+		return mntErr
+	}
+	if notMnt {
+		klog.V(4).Infof("%q is unmounted, deleting the directory", mountPath)
+		return os.Remove(mountPath)
+	}
+	return fmt.Errorf("Failed to unmount path %v", mountPath)
+}
+
+// PathExists returns true if the specified path exists.
+func PathExists(path string) (bool, error) {
+	_, err := os.Stat(path)
+	if err == nil {
+		return true, nil
+	} else if os.IsNotExist(err) {
+		return false, nil
+	} else if IsCorruptedMnt(err) {
+		return true, err
+	} else {
+		return false, err
+	}
+}
+
+// IsCorruptedMnt return true if err is about corrupted mount point
+func IsCorruptedMnt(err error) bool {
+	if err == nil {
+		return false
+	}
+	var underlyingError error
+	switch pe := err.(type) {
+	case nil:
+		return false
+	case *os.PathError:
+		underlyingError = pe.Err
+	case *os.LinkError:
+		underlyingError = pe.Err
+	case *os.SyscallError:
+		underlyingError = pe.Err
+	}
+
+	return underlyingError == syscall.ENOTCONN || underlyingError == syscall.ESTALE || underlyingError == syscall.EIO
+}

pkg/util/mount/mount_helper_test.go

@@ -0,0 +1,63 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+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
+
+import (
+	"os"
+	"testing"
+
+	utiltesting "k8s.io/client-go/util/testing"
+)
+
+func TestDoUnmountMountPoint(t *testing.T) {
+
+	tmpDir1, err1 := utiltesting.MkTmpdir("umount_test1")
+	if err1 != nil {
+		t.Fatalf("error creating temp dir: %v", err1)
+	}
+	defer os.RemoveAll(tmpDir1)
+
+	tmpDir2, err2 := utiltesting.MkTmpdir("umount_test2")
+	if err2 != nil {
+		t.Fatalf("error creating temp dir: %v", err2)
+	}
+	defer os.RemoveAll(tmpDir2)
+
+	// Second part: want no error
+	tests := []struct {
+		mountPath    string
+		corruptedMnt bool
+	}{
+		{
+			mountPath:    tmpDir1,
+			corruptedMnt: true,
+		},
+		{
+			mountPath:    tmpDir2,
+			corruptedMnt: false,
+		},
+	}
+
+	fake := &FakeMounter{}
+
+	for _, tt := range tests {
+		err := doUnmountMountPoint(tt.mountPath, fake, false, tt.corruptedMnt)
+		if err != nil {
+			t.Errorf("err Expected nil, but got: %v", err)
+		}
+	}
+}

pkg/volume/util/BUILD

@@ -61,7 +61,6 @@ go_test(
         "//pkg/apis/core/install:go_default_library",
         "//pkg/apis/core/v1/helper:go_default_library",
         "//pkg/kubelet/apis:go_default_library",
-        "//pkg/util/mount:go_default_library",
         "//pkg/util/slice:go_default_library",
         "//pkg/volume:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
@@ -69,8 +68,12 @@ go_test(
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
-        "//staging/src/k8s.io/client-go/util/testing:go_default_library",
+    ] + select({
-    ],
+        "@io_bazel_rules_go//go/platform:linux": [
+            "//staging/src/k8s.io/client-go/util/testing:go_default_library",
+        ],
+        "//conditions:default": [],
+    }),
 )
 
 filegroup(

pkg/volume/util/util.go

@@ -23,9 +23,8 @@ import (
 	"path"
 	"path/filepath"
 	"strings"
-	"syscall"
 
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	storage "k8s.io/api/storage/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -128,8 +127,9 @@ func SetReady(dir string) {
 
 // UnmountPath is a common unmount routine that unmounts the given path and
 // deletes the remaining directory if successful.
+// TODO: Change callers to call mount pkg directly
 func UnmountPath(mountPath string, mounter mount.Interface) error {
-	return UnmountMountPoint(mountPath, mounter, false /* extensiveMountPointCheck */)
+	return mount.UnmountMountPoint(mountPath, mounter, false /* extensiveMountPointCheck */)
 }
 
 // UnmountMountPoint is a common unmount routine that unmounts the given path and
@@ -137,93 +137,21 @@ func UnmountPath(mountPath string, mounter mount.Interface) error {
 // if extensiveMountPointCheck is true
 // IsNotMountPoint will be called instead of IsLikelyNotMountPoint.
 // IsNotMountPoint is more expensive but properly handles bind mounts.
+// TODO: Change callers to call mount pkg directly
 func UnmountMountPoint(mountPath string, mounter mount.Interface, extensiveMountPointCheck bool) error {
-	pathExists, pathErr := PathExists(mountPath)
+	return mount.UnmountMountPoint(mountPath, mounter, extensiveMountPointCheck)
-	if !pathExists {
-		klog.Warningf("Warning: Unmount skipped because path does not exist: %v", mountPath)
-		return nil
-	}
-	corruptedMnt := IsCorruptedMnt(pathErr)
-	if pathErr != nil && !corruptedMnt {
-		return fmt.Errorf("Error checking path: %v", pathErr)
-	}
-	return doUnmountMountPoint(mountPath, mounter, extensiveMountPointCheck, corruptedMnt)
-}
-
-// doUnmountMountPoint is a common unmount routine that unmounts the given path and
-// deletes the remaining directory if successful.
-// if extensiveMountPointCheck is true
-// IsNotMountPoint will be called instead of IsLikelyNotMountPoint.
-// IsNotMountPoint is more expensive but properly handles bind mounts.
-// if corruptedMnt is true, it means that the mountPath is a corrupted mountpoint, Take it as an argument for convenience of testing
-func doUnmountMountPoint(mountPath string, mounter mount.Interface, extensiveMountPointCheck bool, corruptedMnt bool) error {
-	if !corruptedMnt {
-		var notMnt bool
-		var err error
-		if extensiveMountPointCheck {
-			notMnt, err = mount.IsNotMountPoint(mounter, mountPath)
-		} else {
-			notMnt, err = mounter.IsLikelyNotMountPoint(mountPath)
-		}
-
-		if err != nil {
-			return err
-		}
-
-		if notMnt {
-			klog.Warningf("Warning: %q is not a mountpoint, deleting", mountPath)
-			return os.Remove(mountPath)
-		}
-	}
-
-	// Unmount the mount path
-	klog.V(4).Infof("%q is a mountpoint, unmounting", mountPath)
-	if err := mounter.Unmount(mountPath); err != nil {
-		return err
-	}
-	notMnt, mntErr := mounter.IsLikelyNotMountPoint(mountPath)
-	if mntErr != nil {
-		return mntErr
-	}
-	if notMnt {
-		klog.V(4).Infof("%q is unmounted, deleting the directory", mountPath)
-		return os.Remove(mountPath)
-	}
-	return fmt.Errorf("Failed to unmount path %v", mountPath)
 }
 
 // PathExists returns true if the specified path exists.
+// TODO: Change callers to call mount pkg directly
 func PathExists(path string) (bool, error) {
-	_, err := os.Stat(path)
+	return mount.PathExists(path)
-	if err == nil {
-		return true, nil
-	} else if os.IsNotExist(err) {
-		return false, nil
-	} else if IsCorruptedMnt(err) {
-		return true, err
-	}
-
-	return false, err
 }
 
 // IsCorruptedMnt return true if err is about corrupted mount point
+// TODO: Change callers to call mount pkg directly
 func IsCorruptedMnt(err error) bool {
-	if err == nil {
+	return mount.IsCorruptedMnt(err)
-		return false
-	}
-	var underlyingError error
-	switch pe := err.(type) {
-	case nil:
-		return false
-	case *os.PathError:
-		underlyingError = pe.Err
-	case *os.LinkError:
-		underlyingError = pe.Err
-	case *os.SyscallError:
-		underlyingError = pe.Err
-	}
-
-	return underlyingError == syscall.ENOTCONN || underlyingError == syscall.ESTALE || underlyingError == syscall.EIO
 }
 
 // GetSecretForPod locates secret by name in the pod's namespace and returns secret map

pkg/volume/util/util_test.go

@@ -22,16 +22,14 @@ import (
 	"runtime"
 	"testing"
 
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
-	utiltesting "k8s.io/client-go/util/testing"
 
 	// util.go uses api.Codecs.LegacyCodec so import this package to do some
 	// resource initialization.
 	"hash/fnv"
 
 	_ "k8s.io/kubernetes/pkg/apis/core/install"
-	"k8s.io/kubernetes/pkg/util/mount"
 
 	"reflect"
 	"strings"
@@ -354,45 +352,6 @@ func TestZonesToSet(t *testing.T) {
 	}
 }
 
-func TestDoUnmountMountPoint(t *testing.T) {
-
-	tmpDir1, err1 := utiltesting.MkTmpdir("umount_test1")
-	if err1 != nil {
-		t.Fatalf("error creating temp dir: %v", err1)
-	}
-	defer os.RemoveAll(tmpDir1)
-
-	tmpDir2, err2 := utiltesting.MkTmpdir("umount_test2")
-	if err2 != nil {
-		t.Fatalf("error creating temp dir: %v", err2)
-	}
-	defer os.RemoveAll(tmpDir2)
-
-	// Second part: want no error
-	tests := []struct {
-		mountPath    string
-		corruptedMnt bool
-	}{
-		{
-			mountPath:    tmpDir1,
-			corruptedMnt: true,
-		},
-		{
-			mountPath:    tmpDir2,
-			corruptedMnt: false,
-		},
-	}
-
-	fake := &mount.FakeMounter{}
-
-	for _, tt := range tests {
-		err := doUnmountMountPoint(tt.mountPath, fake, false, tt.corruptedMnt)
-		if err != nil {
-			t.Errorf("err Expected nil, but got: %v", err)
-		}
-	}
-}
-
 func TestCalculateTimeoutForVolume(t *testing.T) {
 	pv := &v1.PersistentVolume{
 		Spec: v1.PersistentVolumeSpec{