From 002a4e33d8b08cb0b3c289a4b23d5fc261f5b65e Mon Sep 17 00:00:00 2001 From: Michelle Au Date: Wed, 26 Dec 2018 14:42:22 -0800 Subject: [PATCH] Move unmount volume util from pkg/volume/util to pkg/util/mount --- pkg/util/mount/BUILD | 2 + pkg/util/mount/mount_helper.go | 126 ++++++++++++++++++++++++++++ pkg/util/mount/mount_helper_test.go | 63 ++++++++++++++ pkg/volume/util/BUILD | 9 +- pkg/volume/util/util.go | 90 ++------------------ pkg/volume/util/util_test.go | 43 +--------- 6 files changed, 207 insertions(+), 126 deletions(-) create mode 100644 pkg/util/mount/mount_helper.go create mode 100644 pkg/util/mount/mount_helper_test.go diff --git a/pkg/util/mount/BUILD b/pkg/util/mount/BUILD index 221afb7a9c..c9b9ce8a47 100644 --- a/pkg/util/mount/BUILD +++ b/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", diff --git a/pkg/util/mount/mount_helper.go b/pkg/util/mount/mount_helper.go new file mode 100644 index 0000000000..a2bd375e69 --- /dev/null +++ b/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 +} diff --git a/pkg/util/mount/mount_helper_test.go b/pkg/util/mount/mount_helper_test.go new file mode 100644 index 0000000000..2897d33afa --- /dev/null +++ b/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) + } + } +} diff --git a/pkg/volume/util/BUILD b/pkg/volume/util/BUILD index d53a67c9c1..d2bcab775e 100644 --- a/pkg/volume/util/BUILD +++ b/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( diff --git a/pkg/volume/util/util.go b/pkg/volume/util/util.go index 71a5a47362..e3a6e8ed05 100644 --- a/pkg/volume/util/util.go +++ b/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) - 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) + return mount.UnmountMountPoint(mountPath, mounter, extensiveMountPointCheck) } // 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) - if err == nil { - return true, nil - } else if os.IsNotExist(err) { - return false, nil - } else if IsCorruptedMnt(err) { - return true, err - } - - return false, err + return mount.PathExists(path) } // 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 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 + return mount.IsCorruptedMnt(err) } // GetSecretForPod locates secret by name in the pod's namespace and returns secret map diff --git a/pkg/volume/util/util_test.go b/pkg/volume/util/util_test.go index c7b7d1a1fb..257410e226 100644 --- a/pkg/volume/util/util_test.go +++ b/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{