Merge pull request #58683 from pospispa/566-postpone-pvc-deletion-if-used-in-a-pod-e2e-tests-for-scheduler-changes

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>.

PVC Protection E2E Tests for Failed Scheduling

**What this PR does / why we need it**:
Change in scheduler that causes that scheduling of a pod that uses PVC that is being deleted fails was introduced in:
- https://github.com/kubernetes/kubernetes/pull/55957

This PR adds an E2E test for the above merged PR.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
N/A

**Special notes for your reviewer**:
N/A

**Release note**:

```release-note
NONE
```

test/e2e/framework/pv_util.go

@@ -926,6 +926,26 @@ func CreateClientPod(c clientset.Interface, ns string, pvc *v1.PersistentVolumeC
 	return CreatePod(c, ns, nil, []*v1.PersistentVolumeClaim{pvc}, true, "")
 }
 
+// CreateUnschedulablePod with given claims based on node selector
+func CreateUnschedulablePod(client clientset.Interface, namespace string, nodeSelector map[string]string, pvclaims []*v1.PersistentVolumeClaim, isPrivileged bool, command string) (*v1.Pod, error) {
+	pod := MakePod(namespace, nodeSelector, pvclaims, isPrivileged, command)
+	pod, err := client.CoreV1().Pods(namespace).Create(pod)
+	if err != nil {
+		return nil, fmt.Errorf("pod Create API error: %v", err)
+	}
+	// Waiting for pod to become Unschedulable
+	err = WaitForPodNameUnschedulableInNamespace(client, pod.Name, namespace)
+	if err != nil {
+		return pod, fmt.Errorf("pod %q is not Unschedulable: %v", pod.Name, err)
+	}
+	// get fresh pod info
+	pod, err = client.CoreV1().Pods(namespace).Get(pod.Name, metav1.GetOptions{})
+	if err != nil {
+		return pod, fmt.Errorf("pod Get API error: %v", err)
+	}
+	return pod, nil
+}
+
 // wait until all pvcs phase set to bound
 func WaitForPVClaimBoundPhase(client clientset.Interface, pvclaims []*v1.PersistentVolumeClaim, timeout time.Duration) ([]*v1.PersistentVolume, error) {
 	persistentvolumes := make([]*v1.PersistentVolume, len(pvclaims))

test/e2e/framework/util.go

@@ -1551,6 +1551,28 @@ func WaitForPodToDisappear(c clientset.Interface, ns, podName string, label labe
 	})
 }
 
+// WaitForPodNameUnschedulableInNamespace returns an error if it takes too long for the pod to become Pending
+// and have condition Status equal to Unschedulable,
+// if the pod Get api returns an error (IsNotFound or other), or if the pod failed with an unexpected reason.
+// Typically called to test that the passed-in pod is Pending and Unschedulable.
+func WaitForPodNameUnschedulableInNamespace(c clientset.Interface, podName, namespace string) error {
+	return WaitForPodCondition(c, namespace, podName, "Unschedulable", PodStartTimeout, func(pod *v1.Pod) (bool, error) {
+		// Only consider Failed pods. Successful pods will be deleted and detected in
+		// waitForPodCondition's Get call returning `IsNotFound`
+		if pod.Status.Phase == v1.PodPending {
+			for _, cond := range pod.Status.Conditions {
+				if cond.Type == v1.PodScheduled && cond.Status == v1.ConditionFalse && cond.Reason == "Unschedulable" {
+					return true, nil
+				}
+			}
+		}
+		if pod.Status.Phase == v1.PodRunning || pod.Status.Phase == v1.PodSucceeded || pod.Status.Phase == v1.PodFailed {
+			return true, fmt.Errorf("Expected pod %q in namespace %q to be in phase Pending, but got phase: %v", podName, namespace, pod.Status.Phase)
+		}
+		return false, nil
+	})
+}
+
 // WaitForService waits until the service appears (exist == true), or disappears (exist == false)
 func WaitForService(c clientset.Interface, namespace, name string, exist bool, interval, timeout time.Duration) error {
 	err := wait.PollImmediate(interval, timeout, func() (bool, error) {
@@ -5093,3 +5115,19 @@ func waitForServerPreferredNamespacedResources(d discovery.DiscoveryInterface, t
 	}
 	return resources, nil
 }
+
+// WaitForPersistentVolumeClaimDeleted waits for a PersistentVolumeClaim to be removed from the system until timeout occurs, whichever comes first.
+func WaitForPersistentVolumeClaimDeleted(c clientset.Interface, ns string, pvcName string, Poll, timeout time.Duration) error {
+	Logf("Waiting up to %v for PersistentVolumeClaim %s to be removed", timeout, pvcName)
+	for start := time.Now(); time.Since(start) < timeout; time.Sleep(Poll) {
+		_, err := c.CoreV1().PersistentVolumeClaims(ns).Get(pvcName, metav1.GetOptions{})
+		if err != nil {
+			if apierrs.IsNotFound(err) {
+				Logf("Claim %q in namespace %q doesn't exist in the system", pvcName, ns)
+				return nil
+			}
+			Logf("Failed to get claim %q in namespace %q, retrying in %v. Error: %v", pvcName, ns, Poll, err)
+		}
+	}
+	return fmt.Errorf("PersistentVolumeClaim %s is not removed from the system within %v", pvcName, timeout)
+}

test/e2e/storage/pvc_protection.go

@@ -17,14 +17,10 @@ limitations under the License.
 package storage
 
 import (
-	"fmt"
-	"time"
-
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 
 	"k8s.io/api/core/v1"
-	apierrs "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/kubernetes/pkg/util/slice"
@@ -80,7 +76,7 @@ var _ = utils.SIGDescribe("PVC Protection [Feature:PVCProtection]", func() {
 		By("Deleting the PVC")
 		err = client.CoreV1().PersistentVolumeClaims(pvc.Namespace).Delete(pvc.Name, metav1.NewDeleteOptions(0))
 		Expect(err).NotTo(HaveOccurred(), "Error deleting PVC")
-		waitForPersistentVolumeClaimBeRemoved(client, pvc.Namespace, pvc.Name, framework.Poll, framework.ClaimProvisionTimeout)
+		framework.WaitForPersistentVolumeClaimDeleted(client, pvc.Namespace, pvc.Name, framework.Poll, framework.ClaimProvisionTimeout)
 		pvcCreatedAndNotDeleted = false
 	})
 
@@ -104,23 +100,44 @@ var _ = utils.SIGDescribe("PVC Protection [Feature:PVCProtection]", func() {
 		Expect(err).NotTo(HaveOccurred(), "Error terminating and deleting pod")
 
 		By("Checking that the PVC is automatically removed from the system because it's no longer in active use by a pod")
-		waitForPersistentVolumeClaimBeRemoved(client, pvc.Namespace, pvc.Name, framework.Poll, framework.ClaimProvisionTimeout)
+		framework.WaitForPersistentVolumeClaimDeleted(client, pvc.Namespace, pvc.Name, framework.Poll, framework.ClaimProvisionTimeout)
+		pvcCreatedAndNotDeleted = false
+	})
+
+	It("Verify that scheduling of a pod that uses PVC that is being deleted fails and the pod becomes Unschedulable", func() {
+		By("Creating first Pod that becomes Running and therefore is actively using the PVC")
+		pvcClaims := []*v1.PersistentVolumeClaim{pvc}
+		firstPod, err := framework.CreatePod(client, nameSpace, nil, pvcClaims, false, "")
+		Expect(err).NotTo(HaveOccurred(), "While creating pod that uses the PVC or waiting for the Pod to become Running")
+
+		By("Deleting the PVC, however, the PVC must not be removed from the system as it's in active use by a pod")
+		err = client.CoreV1().PersistentVolumeClaims(pvc.Namespace).Delete(pvc.Name, metav1.NewDeleteOptions(0))
+		Expect(err).NotTo(HaveOccurred(), "Error deleting PVC")
+
+		By("Checking that the PVC status is Terminating")
+		pvc, err = client.CoreV1().PersistentVolumeClaims(pvc.Namespace).Get(pvc.Name, metav1.GetOptions{})
+		Expect(err).NotTo(HaveOccurred(), "While checking PVC status")
+		Expect(pvc.ObjectMeta.DeletionTimestamp).NotTo(Equal(nil))
+
+		By("Creating second Pod whose scheduling fails because it uses a PVC that is being deleted")
+		secondPod, err2 := framework.CreateUnschedulablePod(client, nameSpace, nil, pvcClaims, false, "")
+		Expect(err2).NotTo(HaveOccurred(), "While creating second pod that uses a PVC that is being deleted and that is Unschedulable")
+
+		By("Deleting the second pod that uses the PVC that is being deleted")
+		err = framework.DeletePodWithWait(f, client, secondPod)
+		Expect(err).NotTo(HaveOccurred(), "Error terminating and deleting pod")
+
+		By("Checking again that the PVC status is Terminating")
+		pvc, err = client.CoreV1().PersistentVolumeClaims(pvc.Namespace).Get(pvc.Name, metav1.GetOptions{})
+		Expect(err).NotTo(HaveOccurred(), "While checking PVC status")
+		Expect(pvc.ObjectMeta.DeletionTimestamp).NotTo(Equal(nil))
+
+		By("Deleting the first pod that uses the PVC")
+		err = framework.DeletePodWithWait(f, client, firstPod)
+		Expect(err).NotTo(HaveOccurred(), "Error terminating and deleting pod")
+
+		By("Checking that the PVC is automatically removed from the system because it's no longer in active use by a pod")
+		framework.WaitForPersistentVolumeClaimDeleted(client, pvc.Namespace, pvc.Name, framework.Poll, framework.ClaimProvisionTimeout)
 		pvcCreatedAndNotDeleted = false
 	})
 })
-
-// waitForPersistentVolumeClaimBeRemoved waits for a PersistentVolumeClaim to be removed from the system until timeout occurs, whichever comes first.
-func waitForPersistentVolumeClaimBeRemoved(c clientset.Interface, ns string, pvcName string, Poll, timeout time.Duration) error {
-	framework.Logf("Waiting up to %v for PersistentVolumeClaim %s to be removed", timeout, pvcName)
-	for start := time.Now(); time.Since(start) < timeout; time.Sleep(Poll) {
-		_, err := c.CoreV1().PersistentVolumeClaims(ns).Get(pvcName, metav1.GetOptions{})
-		if err != nil {
-			if apierrs.IsNotFound(err) {
-				framework.Logf("Claim %q in namespace %q doesn't exist in the system", pvcName, ns)
-				return nil
-			}
-			framework.Logf("Failed to get claim %q in namespace %q, retrying in %v. Error: %v", pvcName, ns, Poll, err)
-		}
-	}
-	return fmt.Errorf("PersistentVolumeClaim %s is not removed from the system within %v", pvcName, timeout)
-}