Merge pull request #53537 from jeffvance/pod-delete

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

wait for pod to be fully deleted

**What this PR does / why we need it**:
Fix flaky glusterfs io-streaming tests.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #49529 

**Special notes for your reviewer**:
1) max potential wait for complete pod deletion is ~~15m~~ 5m.
2) ~~removed [Flaky] from HostCleanup, _e2e/node/kubelet.go_ since pod deletion is reliable now.~~
3) ~~added tag [Slow] to HostCleanup due to long max wait for pod deletion.~~

After all CI tests run reliably we can consider removing the [Flaky] tag (2, above), or do that in a separate pr.

```release-note
NONE
```
cc @msau42

test/e2e/framework/framework.go

@@ -381,6 +381,11 @@ func (f *Framework) WaitForPodTerminated(podName, reason string) error {
 	return waitForPodTerminatedInNamespace(f.ClientSet, podName, reason, f.Namespace.Name)
 }
 
+// WaitForPodNotFound waits for the pod to be completely terminated (not "Get-able").
+func (f *Framework) WaitForPodNotFound(podName string, timeout time.Duration) error {
+	return waitForPodNotFoundInNamespace(f.ClientSet, podName, f.Namespace.Name, timeout)
+}
+
 // WaitForPodRunning waits for the pod to run in the namespace.
 func (f *Framework) WaitForPodRunning(podName string) error {
 	return WaitForPodNameRunningInNamespace(f.ClientSet, podName, f.Namespace.Name)

test/e2e/framework/pv_util.go

@@ -494,25 +494,22 @@ 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 {
+	const maxWait = 5 * time.Minute
 	if pod == nil {
 		return nil
 	}
-	Logf("Deleting pod %v", pod.Name)
+	Logf("Deleting pod %q in namespace %q", pod.Name, pod.Namespace)
 	err := c.CoreV1().Pods(pod.Namespace).Delete(pod.Name, nil)
 	if err != nil {
 		if apierrs.IsNotFound(err) {
-			return nil // assume pod was deleted already
+			return nil // assume pod was already deleted
 		}
-		return fmt.Errorf("pod Get API error: %v", err)
+		return fmt.Errorf("pod Delete API error: %v", err)
 	}
-
+	Logf("Wait up to %v for pod %q to be fully deleted", maxWait, pod.Name)
-	// wait for pod to terminate
+	err = f.WaitForPodNotFound(pod.Name, maxWait)
-	err = f.WaitForPodTerminated(pod.Name, "")
+	if err != nil {
-	if err != nil && !apierrs.IsNotFound(err) {
+		return fmt.Errorf("pod %q was not deleted: %v", pod.Name, err)
-		return fmt.Errorf("error deleting pod %q: %v", pod.Name, err)
-	}
-	if apierrs.IsNotFound(err) {
-		Logf("Ignore \"not found\" error above. Pod %q successfully deleted", pod.Name)
 	}
 	return nil
 }

test/e2e/framework/util.go

@@ -1445,6 +1445,23 @@ func waitForPodTerminatedInNamespace(c clientset.Interface, podName, reason, nam
 	})
 }
 
+// waitForPodNotFoundInNamespace returns an error if it takes too long for the pod to fully terminate.
+// Unlike `waitForPodTerminatedInNamespace`, the pod's Phase and Reason are ignored. If the pod Get
+// api returns IsNotFound then the wait stops and nil is returned. If the Get api returns an error other
+// than "not found" then that error is returned and the wait stops.
+func waitForPodNotFoundInNamespace(c clientset.Interface, podName, ns string, timeout time.Duration) error {
+	return wait.PollImmediate(Poll, timeout, func() (bool, error) {
+		_, err := c.CoreV1().Pods(ns).Get(podName, metav1.GetOptions{})
+		if apierrs.IsNotFound(err) {
+			return true, nil // done
+		}
+		if err != nil {
+			return true, err // stop wait with error
+		}
+		return false, nil
+	})
+}
+
 // waitForPodSuccessInNamespaceTimeout returns nil if the pod reached state success, or an error if it reached failure or ran too long.
 func waitForPodSuccessInNamespaceTimeout(c clientset.Interface, podName string, namespace string, timeout time.Duration) error {
 	return WaitForPodCondition(c, namespace, podName, "success or failure", timeout, func(pod *v1.Pod) (bool, error) {

test/e2e/node/kubelet.go

@@ -423,8 +423,10 @@ var _ = SIGDescribe("kubelet", func() {
 			})
 
 			AfterEach(func() {
-				framework.ExpectNoError(framework.DeletePodWithWait(f, c, pod), "AfterEach: Failed to delete pod ", pod.Name)
+				err := framework.DeletePodWithWait(f, c, pod)
-				framework.ExpectNoError(framework.DeletePodWithWait(f, c, nfsServerPod), "AfterEach: Failed to delete pod ", nfsServerPod.Name)
+				Expect(err).NotTo(HaveOccurred(), "AfterEach: Failed to delete client pod ", pod.Name)
+				err = framework.DeletePodWithWait(f, c, nfsServerPod)
+				Expect(err).NotTo(HaveOccurred(), "AfterEach: Failed to delete server pod ", nfsServerPod.Name)
 			})
 
 			// execute It blocks from above table of tests
@@ -435,8 +437,9 @@ var _ = SIGDescribe("kubelet", func() {
 					By("Stop the NFS server")
 					stopNfsServer(nfsServerPod)
 
-					By("Delete the pod mounted to the NFS volume")
+					By("Delete the pod mounted to the NFS volume -- expect failure")
-					framework.ExpectNoError(framework.DeletePodWithWait(f, c, pod), "Failed to delete pod ", pod.Name)
+					err := framework.DeletePodWithWait(f, c, pod)
+					Expect(err).To(HaveOccurred())
 					// pod object is now stale, but is intentionally not nil
 
 					By("Check if pod's host has been cleaned up -- expect not")
@@ -445,7 +448,7 @@ var _ = SIGDescribe("kubelet", func() {
 					By("Restart the nfs server")
 					restartNfsServer(nfsServerPod)
 
-					By("Verify host running the deleted pod is now cleaned up")
+					By("Verify that the deleted client pod is now cleaned up")
 					checkPodCleanup(c, pod, true)
 				})
 			}