Merge pull request #68759 from davidz627/fix/testDelete

UnmountDevice (GetMountRefs) should not fail if path does not exist

pkg/util/mount/mount_linux.go

@@ -1005,6 +1005,11 @@ func (mounter *Mounter) SafeMakeDir(subdir string, base string, perm os.FileMode
 }
 
 func (mounter *Mounter) GetMountRefs(pathname string) ([]string, error) {
+	if _, err := os.Stat(pathname); os.IsNotExist(err) {
+		return []string{}, nil
+	} else if err != nil {
+		return nil, err
+	}
 	realpath, err := filepath.EvalSymlinks(pathname)
 	if err != nil {
 		return nil, err

pkg/util/mount/mount_linux_test.go

@@ -110,6 +110,10 @@ func TestGetMountRefs(t *testing.T) {
 				"/var/lib/kubelet/plugins/kubernetes.io/gce-pd/mounts/gce-pd2",
 			},
 		},
+		{
+			"/var/fake/directory/that/doesnt/exist",
+			[]string{},
+		},
 	}
 
 	for i, test := range tests {

pkg/util/mount/mount_windows.go

@@ -459,6 +459,11 @@ func getAllParentLinks(path string) ([]string, error) {
 }
 
 func (mounter *Mounter) GetMountRefs(pathname string) ([]string, error) {
+	if _, err := os.Stat(normalizeWindowsPath(pathname)); os.IsNotExist(err) {
+		return []string{}, nil
+	} else if err != nil {
+		return nil, err
+	}
 	refs, err := getAllParentLinks(normalizeWindowsPath(pathname))
 	if err != nil {
 		return nil, err

pkg/util/mount/nsenter_mount.go

@@ -337,6 +337,13 @@ func (mounter *NsenterMounter) SafeMakeDir(subdir string, base string, perm os.F
 }
 
 func (mounter *NsenterMounter) GetMountRefs(pathname string) ([]string, error) {
+	exists, err := mounter.ExistsPath(pathname)
+	if err != nil {
+		return nil, err
+	}
+	if !exists {
+		return []string{}, nil
+	}
 	hostpath, err := mounter.ne.EvalSymlinks(pathname, true /* mustExist */)
 	if err != nil {
 		return nil, err

pkg/volume/util/operationexecutor/operation_generator.go

@@ -1433,6 +1433,8 @@ func isDeviceOpened(deviceToDetach AttachedVolume, mounter mount.Interface) (boo
 		//TODO: refer to #36092
 		glog.V(3).Infof("The path isn't device path or doesn't exist. Skip checking device path: %s", deviceToDetach.DevicePath)
 		deviceOpened = false
+	} else if devicePathErr != nil {
+		return false, deviceToDetach.GenerateErrorDetailed("PathIsDevice failed", devicePathErr)
 	} else {
 		deviceOpened, deviceOpenedErr = mounter.DeviceOpened(deviceToDetach.DevicePath)
 		if deviceOpenedErr != nil {

test/e2e/framework/pv_util.go

@@ -507,6 +507,9 @@ func testPodSuccessOrFail(c clientset.Interface, ns string, pod *v1.Pod) error {
 // Deletes the passed-in pod and waits for the pod to be terminated. Resilient to the pod
 // not existing.
 func DeletePodWithWait(f *Framework, c clientset.Interface, pod *v1.Pod) error {
+	if pod == nil {
+		return nil
+	}
 	return DeletePodWithWaitByName(f, c, pod.GetName(), pod.GetNamespace())
 }
 

test/e2e/storage/generic_persistent_volume-disruptive.go

@@ -61,10 +61,11 @@ var _ = utils.SIGDescribe("GenericPersistentVolume[Disruptive]", func() {
 		var (
 			clientPod *v1.Pod
 			pvc       *v1.PersistentVolumeClaim
+			pv        *v1.PersistentVolume
 		)
 		BeforeEach(func() {
 			framework.Logf("Initializing pod and pvcs for test")
-			clientPod, pvc = createPodPVCFromSC(f, c, ns)
+			clientPod, pvc, pv = createPodPVCFromSC(f, c, ns)
 		})
 		for _, test := range disruptiveTestTable {
 			func(t disruptiveTest) {
@@ -76,13 +77,13 @@ var _ = utils.SIGDescribe("GenericPersistentVolume[Disruptive]", func() {
 		}
 		AfterEach(func() {
 			framework.Logf("Tearing down test spec")
-			tearDownTestCase(c, f, ns, clientPod, pvc, nil)
+			tearDownTestCase(c, f, ns, clientPod, pvc, pv, false)
 			pvc, clientPod = nil, nil
 		})
 	})
 })
 
-func createPodPVCFromSC(f *framework.Framework, c clientset.Interface, ns string) (*v1.Pod, *v1.PersistentVolumeClaim) {
+func createPodPVCFromSC(f *framework.Framework, c clientset.Interface, ns string) (*v1.Pod, *v1.PersistentVolumeClaim, *v1.PersistentVolume) {
 	var err error
 	test := storageClassTest{
 		name:      "default",
@@ -99,5 +100,5 @@ func createPodPVCFromSC(f *framework.Framework, c clientset.Interface, ns string
 	By("Creating a pod with dynamically provisioned volume")
 	pod, err := framework.CreateNginxPod(c, ns, nil, pvcClaims)
 	Expect(err).NotTo(HaveOccurred(), "While creating pods for kubelet restart test")
-	return pod, pvc
+	return pod, pvc, pvs[0]
 }

test/e2e/storage/nfs_persistent_volume-disruptive.go

@@ -208,7 +208,7 @@ var _ = utils.SIGDescribe("NFSPersistentVolumes[Disruptive][Flaky]", func() {
 
 		AfterEach(func() {
 			framework.Logf("Tearing down test spec")
-			tearDownTestCase(c, f, ns, clientPod, pvc, pv)
+			tearDownTestCase(c, f, ns, clientPod, pvc, pv, true /* force PV delete */)
 			pv, pvc, clientPod = nil, nil, nil
 		})
 
@@ -277,11 +277,14 @@ func initTestCase(f *framework.Framework, c clientset.Interface, pvConfig framew
 }
 
 // tearDownTestCase destroy resources created by initTestCase.
-func tearDownTestCase(c clientset.Interface, f *framework.Framework, ns string, client *v1.Pod, pvc *v1.PersistentVolumeClaim, pv *v1.PersistentVolume) {
+func tearDownTestCase(c clientset.Interface, f *framework.Framework, ns string, client *v1.Pod, pvc *v1.PersistentVolumeClaim, pv *v1.PersistentVolume, forceDeletePV bool) {
 	// Ignore deletion errors.  Failing on them will interrupt test cleanup.
 	framework.DeletePodWithWait(f, c, client)
 	framework.DeletePersistentVolumeClaim(c, pvc.Name, ns)
-	if pv != nil {
+	if forceDeletePV && pv != nil {
 		framework.DeletePersistentVolume(c, pv.Name)
+		return
 	}
+	err := framework.WaitForPersistentVolumeDeleted(c, pv.Name, 5*time.Second, 5*time.Minute)
+	framework.ExpectNoError(err, "Persistent Volume %v not deleted by dynamic provisioner", pv.Name)
 }