Merge pull request kubernetes#18225 from humblec/kubelet-mount-fix

Automatic merge from submit-queue (batch tested with PRs 18225, 18351, 18331, 18340, 18326).

UPSTREAM: 52324: Fix bug on kubelet failure to umount mount points.

Origin-commit: f2b1e4215f6bf560f69f6aad24e6662e29ead850

Author: k8s-publishing-bot

pkg/volume/util/util.go

@@ -23,6 +23,7 @@ import (
 	"path"
 	"path/filepath"
 	"strings"
+	"syscall"
 
 	"github.com/golang/glog"
 	"k8s.io/api/core/v1"
@@ -96,29 +97,42 @@ func UnmountPath(mountPath string, mounter mount.Interface) error {
 // IsNotMountPoint will be called instead of IsLikelyNotMountPoint.
 // IsNotMountPoint is more expensive but properly handles bind mounts.
 func UnmountMountPoint(mountPath string, mounter mount.Interface, extensiveMountPointCheck bool) error {
-	if pathExists, pathErr := PathExists(mountPath); pathErr != nil {
-		return fmt.Errorf("Error checking if path exists: %v", pathErr)
-	} else if !pathExists {
+	pathExists, pathErr := PathExists(mountPath)
+	if !pathExists {
 		glog.Warningf("Warning: Unmount skipped because path does not exist: %v", mountPath)
 		return nil
 	}
-
-	var notMnt bool
-	var err error
-
-	if extensiveMountPointCheck {
-		notMnt, err = mount.IsNotMountPoint(mounter, mountPath)
-	} else {
-		notMnt, err = mounter.IsLikelyNotMountPoint(mountPath)
+	corruptedMnt := isCorruptedMnt(pathErr)
+	if pathErr != nil && !corruptedMnt {
+		return fmt.Errorf("Error checking path: %v", pathErr)
 	}
+	return doUnmountMountPoint(mountPath, mounter, extensiveMountPointCheck, corruptedMnt)
+}
 
-	if err != nil {
-		return err
-	}
+// 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 notMnt {
-		glog.Warningf("Warning: %q is not a mountpoint, deleting", mountPath)
-		return os.Remove(mountPath)
+		if err != nil {
+			return err
+		}
+
+		if notMnt {
+			glog.Warningf("Warning: %q is not a mountpoint, deleting", mountPath)
+			return os.Remove(mountPath)
+		}
 	}
 
 	// Unmount the mount path
@@ -128,7 +142,7 @@ func UnmountMountPoint(mountPath string, mounter mount.Interface, extensiveMount
 	}
 	notMnt, mntErr := mounter.IsLikelyNotMountPoint(mountPath)
 	if mntErr != nil {
-		return err
+		return mntErr
 	}
 	if notMnt {
 		glog.V(4).Infof("%q is unmounted, deleting the directory", mountPath)
@@ -144,11 +158,32 @@ func PathExists(path string) (bool, error) {
 		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
+}
+
 // GetSecretForPod locates secret by name in the pod's namespace and returns secret map
 func GetSecretForPod(pod *v1.Pod, secretName string, kubeClient clientset.Interface) (map[string]string, error) {
 	secret := make(map[string]string)

pkg/volume/util/util_test.go

@@ -24,10 +24,12 @@ import (
 	"k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/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.
 	_ "k8s.io/kubernetes/pkg/apis/core/install"
 	"k8s.io/kubernetes/pkg/apis/core/v1/helper"
+        "k8s.io/kubernetes/pkg/util/mount"
 )
 
 var nodeLabels map[string]string = map[string]string{
@@ -263,3 +265,42 @@ 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)
+		}
+	}
+}