Merge pull request #71804 from msau42/fix-subpath-nfs

Use UnmountMountPoint util to clean up subpaths
pull/564/head
Kubernetes Prow Robot 2019-01-03 20:21:32 -08:00 committed by GitHub
commit 252a74d4bb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 348 additions and 187 deletions

View File

@ -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",

View File

@ -30,6 +30,8 @@ type FakeMounter struct {
MountPoints []MountPoint
Log []FakeAction
Filesystem map[string]FileType
// Error to return for a path when calling IsLikelyNotMountPoint
MountCheckErrors map[string]error
// Some tests run things in parallel, make sure the mounter does not produce
// any golang's DATA RACE warnings.
mutex sync.Mutex
@ -119,6 +121,7 @@ func (f *FakeMounter) Unmount(target string) error {
}
f.MountPoints = newMountpoints
f.Log = append(f.Log, FakeAction{Action: FakeActionUnmount, Target: absTarget})
delete(f.MountCheckErrors, target)
return nil
}
@ -141,7 +144,12 @@ func (f *FakeMounter) IsLikelyNotMountPoint(file string) (bool, error) {
f.mutex.Lock()
defer f.mutex.Unlock()
_, err := os.Stat(file)
err := f.MountCheckErrors[file]
if err != nil {
return false, err
}
_, err = os.Stat(file)
if err != nil {
return true, err
}

View File

@ -0,0 +1,124 @@
/*
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"
)
// CleanupMountPoint 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 within the same fs.
func CleanupMountPoint(mountPath string, mounter Interface, extensiveMountPointCheck bool) error {
// mounter.ExistsPath cannot be used because for containerized kubelet, we need to check
// the path in the kubelet container, not on the host.
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 doCleanupMountPoint(mountPath, mounter, extensiveMountPointCheck, corruptedMnt)
}
// doCleanupMountPoint 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 within the same fs.
// if corruptedMnt is true, it means that the mountPath is a corrupted mountpoint, and the mount point check
// will be skipped
func doCleanupMountPoint(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)
}
// TODO: clean this up to use pkg/util/file/FileExists
// 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
}

View File

@ -0,0 +1,152 @@
/*
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"
"io/ioutil"
"os"
"path/filepath"
"syscall"
"testing"
)
func TestDoCleanupMountPoint(t *testing.T) {
const testMount = "test-mount"
const defaultPerm = 0750
tests := map[string]struct {
corruptedMnt bool
// Function that prepares the directory structure for the test under
// the given base directory.
// Returns a fake MountPoint, a fake error for the mount point,
// and error if the prepare function encountered a fatal error.
prepare func(base string) (MountPoint, error, error)
expectErr bool
}{
"mount-ok": {
prepare: func(base string) (MountPoint, error, error) {
path := filepath.Join(base, testMount)
if err := os.MkdirAll(path, defaultPerm); err != nil {
return MountPoint{}, nil, err
}
return MountPoint{Device: "/dev/sdb", Path: path}, nil, nil
},
},
"mount-corrupted": {
prepare: func(base string) (MountPoint, error, error) {
path := filepath.Join(base, testMount)
if err := os.MkdirAll(path, defaultPerm); err != nil {
return MountPoint{}, nil, err
}
return MountPoint{Device: "/dev/sdb", Path: path}, os.NewSyscallError("fake", syscall.ESTALE), nil
},
corruptedMnt: true,
},
"mount-err-not-corrupted": {
prepare: func(base string) (MountPoint, error, error) {
path := filepath.Join(base, testMount)
if err := os.MkdirAll(path, defaultPerm); err != nil {
return MountPoint{}, nil, err
}
return MountPoint{Device: "/dev/sdb", Path: path}, os.NewSyscallError("fake", syscall.ETIMEDOUT), nil
},
expectErr: true,
},
}
for name, tt := range tests {
t.Run(name, func(t *testing.T) {
tmpDir, err := ioutil.TempDir("", "unmount-mount-point-test")
if err != nil {
t.Fatalf("failed to create tmpdir: %v", err)
}
defer os.RemoveAll(tmpDir)
if tt.prepare == nil {
t.Fatalf("prepare function required")
}
mountPoint, mountError, err := tt.prepare(tmpDir)
if err != nil {
t.Fatalf("failed to prepare test: %v", err)
}
fake := &FakeMounter{
MountPoints: []MountPoint{mountPoint},
MountCheckErrors: map[string]error{mountPoint.Path: mountError},
}
err = doCleanupMountPoint(mountPoint.Path, fake, true, tt.corruptedMnt)
if tt.expectErr {
if err == nil {
t.Errorf("test %s failed, expected error, got none", name)
}
if err := validateDirExists(mountPoint.Path); err != nil {
t.Errorf("test %s failed, mount path doesn't exist: %v", name, err)
}
}
if !tt.expectErr {
if err != nil {
t.Errorf("test %s failed: %v", name, err)
}
if err := validateDirNotExists(mountPoint.Path); err != nil {
t.Errorf("test %s failed, mount path still exists: %v", name, err)
}
}
})
}
}
func validateDirEmpty(dir string) error {
files, err := ioutil.ReadDir(dir)
if err != nil {
return err
}
if len(files) != 0 {
return fmt.Errorf("Directory %q is not empty", dir)
}
return nil
}
func validateDirExists(dir string) error {
_, err := ioutil.ReadDir(dir)
if err != nil {
return err
}
return nil
}
func validateDirNotExists(dir string) error {
_, err := ioutil.ReadDir(dir)
if os.IsNotExist(err) {
return nil
}
if err != nil {
return err
}
return fmt.Errorf("dir %q still exists", dir)
}
func validateFileExists(file string) error {
if _, err := os.Stat(file); err != nil {
return err
}
return nil
}

View File

@ -891,15 +891,22 @@ func doCleanSubPaths(mounter Interface, podDir string, volumeName string) error
// scan /var/lib/kubelet/pods/<uid>/volume-subpaths/<volume>/<container name>/*
fullContainerDirPath := filepath.Join(subPathDir, containerDir.Name())
subPaths, err := ioutil.ReadDir(fullContainerDirPath)
if err != nil {
return fmt.Errorf("error reading %s: %s", fullContainerDirPath, err)
}
for _, subPath := range subPaths {
if err = doCleanSubPath(mounter, fullContainerDirPath, subPath.Name()); err != nil {
err = filepath.Walk(fullContainerDirPath, func(path string, info os.FileInfo, err error) error {
if path == fullContainerDirPath {
// Skip top level directory
return nil
}
// pass through errors and let doCleanSubPath handle them
if err = doCleanSubPath(mounter, fullContainerDirPath, filepath.Base(path)); err != nil {
return err
}
return nil
})
if err != nil {
return fmt.Errorf("error processing %s: %s", fullContainerDirPath, err)
}
// Whole container has been processed, remove its directory.
if err := os.Remove(fullContainerDirPath); err != nil {
return fmt.Errorf("error deleting %s: %s", fullContainerDirPath, err)
@ -926,22 +933,12 @@ func doCleanSubPath(mounter Interface, fullContainerDirPath, subPathIndex string
// process /var/lib/kubelet/pods/<uid>/volume-subpaths/<volume>/<container name>/<subPathName>
klog.V(4).Infof("Cleaning up subpath mounts for subpath %v", subPathIndex)
fullSubPath := filepath.Join(fullContainerDirPath, subPathIndex)
notMnt, err := IsNotMountPoint(mounter, fullSubPath)
if err != nil {
return fmt.Errorf("error checking %s for mount: %s", fullSubPath, err)
if err := CleanupMountPoint(fullSubPath, mounter, true); err != nil {
return fmt.Errorf("error cleaning subpath mount %s: %s", fullSubPath, err)
}
// Unmount it
if !notMnt {
if err = mounter.Unmount(fullSubPath); err != nil {
return fmt.Errorf("error unmounting %s: %s", fullSubPath, err)
}
klog.V(5).Infof("Unmounted %s", fullSubPath)
}
// Remove it *non*-recursively, just in case there were some hiccups.
if err = os.Remove(fullSubPath); err != nil {
return fmt.Errorf("error deleting %s: %s", fullSubPath, err)
}
klog.V(5).Infof("Removed %s", fullSubPath)
klog.V(4).Infof("Successfully cleaned subpath directory %s", fullSubPath)
return nil
}

View File

@ -666,44 +666,6 @@ func TestSafeMakeDir(t *testing.T) {
}
}
func validateDirEmpty(dir string) error {
files, err := ioutil.ReadDir(dir)
if err != nil {
return err
}
if len(files) != 0 {
return fmt.Errorf("Directory %q is not empty", dir)
}
return nil
}
func validateDirExists(dir string) error {
_, err := ioutil.ReadDir(dir)
if err != nil {
return err
}
return nil
}
func validateDirNotExists(dir string) error {
_, err := ioutil.ReadDir(dir)
if os.IsNotExist(err) {
return nil
}
if err != nil {
return err
}
return fmt.Errorf("dir %q still exists", dir)
}
func validateFileExists(file string) error {
if _, err := os.Stat(file); err != nil {
return err
}
return nil
}
func TestRemoveEmptyDirs(t *testing.T) {
defaultPerm := os.FileMode(0750)
tests := []struct {

View File

@ -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(

View File

@ -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: Remove this function and change callers to call mount pkg directly
func UnmountPath(mountPath string, mounter mount.Interface) error {
return UnmountMountPoint(mountPath, mounter, false /* extensiveMountPointCheck */)
return mount.CleanupMountPoint(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.CleanupMountPoint(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

View File

@ -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{

View File

@ -22,7 +22,7 @@ import (
"regexp"
"strings"
"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/rand"
"k8s.io/apimachinery/pkg/util/wait"
@ -377,6 +377,32 @@ func testSubPath(input *subPathTestInput) {
// Pod should fail
testPodFailSubpath(input.f, input.pod, true)
})
It("should be able to unmount after the subpath directory is deleted", func() {
// Change volume container to busybox so we can exec later
input.pod.Spec.Containers[1].Image = imageutils.GetE2EImage(imageutils.BusyBox)
input.pod.Spec.Containers[1].Command = []string{"/bin/sh", "-ec", "sleep 100000"}
By(fmt.Sprintf("Creating pod %s", input.pod.Name))
pod, err := input.f.ClientSet.CoreV1().Pods(input.f.Namespace.Name).Create(input.pod)
Expect(err).ToNot(HaveOccurred(), "while creating pod")
defer func() {
By(fmt.Sprintf("Deleting pod %s", pod.Name))
framework.DeletePodWithWait(input.f, input.f.ClientSet, pod)
}()
// Wait for pod to be running
err = framework.WaitForPodRunningInNamespace(input.f.ClientSet, pod)
Expect(err).ToNot(HaveOccurred(), "while waiting for pod to be running")
// Exec into container that mounted the volume, delete subpath directory
rmCmd := fmt.Sprintf("rm -rf %s", input.subPathDir)
_, err = podContainerExec(pod, 1, rmCmd)
Expect(err).ToNot(HaveOccurred(), "while removing subpath directory")
// Delete pod (from defer) and wait for it to be successfully deleted
})
// TODO: add a test case for the same disk with two partitions
}