From d2be1ebb5ff6161fcefac62e2d1d29319bb54dbf Mon Sep 17 00:00:00 2001 From: pospispa Date: Sat, 20 Jan 2018 16:29:41 +0100 Subject: [PATCH 1/2] Moved func WaitForPersistentVolumeClaimBeRemoved Among Other WaitFor Functions It's better to have all WaitFor functions in the same location even though the func WaitForPersistentVolumeClaimBeRemoved is used only in one file. This was requested in [1]. [1] https://github.com/kubernetes/kubernetes/pull/56931#discussion_r156724019 --- test/e2e/framework/util.go | 16 ++++++++++++++++ test/e2e/storage/pvc_protection.go | 24 ++---------------------- 2 files changed, 18 insertions(+), 22 deletions(-) diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index 5142760840a..fdb5ad2dd4f 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -5118,3 +5118,19 @@ func waitForServerPreferredNamespacedResources(d discovery.DiscoveryInterface, t } return resources, nil } + +// 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 { + 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) +} diff --git a/test/e2e/storage/pvc_protection.go b/test/e2e/storage/pvc_protection.go index 5ead6272635..8eaac20439c 100644 --- a/test/e2e/storage/pvc_protection.go +++ b/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.WaitForPersistentVolumeClaimBeRemoved(client, pvc.Namespace, pvc.Name, framework.Poll, framework.ClaimProvisionTimeout) pvcCreatedAndNotDeleted = false }) @@ -104,23 +100,7 @@ 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.WaitForPersistentVolumeClaimBeRemoved(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) -} From 52a43b19d8dea27dd6e7047efb26a19df87a447d Mon Sep 17 00:00:00 2001 From: pospispa Date: Tue, 23 Jan 2018 13:02:13 +0100 Subject: [PATCH 2/2] PVC Protection E2E Tests for Failed Scheduling The PR [2] introduced a change into a scheduler that causes that scheduling of pods that use PVC that is being deleted fail. That's why E2E test for the PR [2] is added. This E2E test also addresses the review comment [1]. [1] https://github.com/kubernetes/kubernetes/pull/56931#pullrequestreview-82564849 [2] https://github.com/kubernetes/kubernetes/pull/55957 --- test/e2e/framework/pv_util.go | 20 +++++++++++++++ test/e2e/framework/util.go | 26 +++++++++++++++++-- test/e2e/storage/pvc_protection.go | 41 ++++++++++++++++++++++++++++-- 3 files changed, 83 insertions(+), 4 deletions(-) diff --git a/test/e2e/framework/pv_util.go b/test/e2e/framework/pv_util.go index c81e024b603..3b4784f848c 100644 --- a/test/e2e/framework/pv_util.go +++ b/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)) diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index fdb5ad2dd4f..6a5566d842c 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -1547,6 +1547,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) { @@ -5119,8 +5141,8 @@ func waitForServerPreferredNamespacedResources(d discovery.DiscoveryInterface, t return resources, nil } -// 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 { +// 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{}) diff --git a/test/e2e/storage/pvc_protection.go b/test/e2e/storage/pvc_protection.go index 8eaac20439c..16f2aa0e57e 100644 --- a/test/e2e/storage/pvc_protection.go +++ b/test/e2e/storage/pvc_protection.go @@ -76,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") - framework.WaitForPersistentVolumeClaimBeRemoved(client, pvc.Namespace, pvc.Name, framework.Poll, framework.ClaimProvisionTimeout) + framework.WaitForPersistentVolumeClaimDeleted(client, pvc.Namespace, pvc.Name, framework.Poll, framework.ClaimProvisionTimeout) pvcCreatedAndNotDeleted = false }) @@ -100,7 +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") - framework.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 }) })