From d3503a28816598caae35974fec1471c7365497b5 Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Tue, 11 Aug 2020 10:57:42 +0200 Subject: [PATCH 1/3] Fix waiting for PVCs to get Bound When Get() returns error, the loop should not return with nil. Make sure that the faulty Get() marks phaseFoundInAllClaims as not satisfied. --- test/e2e/framework/pv/pv.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/e2e/framework/pv/pv.go b/test/e2e/framework/pv/pv.go index f778e40d2b9..949aa8f4d71 100644 --- a/test/e2e/framework/pv/pv.go +++ b/test/e2e/framework/pv/pv.go @@ -749,7 +749,8 @@ func WaitForPersistentVolumeClaimsPhase(phase v1.PersistentVolumeClaimPhase, c c pvc, err := c.CoreV1().PersistentVolumeClaims(ns).Get(context.TODO(), pvcName, metav1.GetOptions{}) if err != nil { framework.Logf("Failed to get claim %q, retrying in %v. Error: %v", pvcName, Poll, err) - continue + phaseFoundInAllClaims = false + break } if pvc.Status.Phase == phase { framework.Logf("PersistentVolumeClaim %s found and phase=%s (%v)", pvcName, phase, time.Since(start)) From 6a83a39ae6e7220e5efd29e30ee3575f71b4bb2e Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Tue, 11 Aug 2020 11:06:25 +0200 Subject: [PATCH 2/3] Fix Poll variable name Parameter names should start with lowercase --- test/e2e/framework/pv/pv.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/test/e2e/framework/pv/pv.go b/test/e2e/framework/pv/pv.go index 949aa8f4d71..e4d67e57a22 100644 --- a/test/e2e/framework/pv/pv.go +++ b/test/e2e/framework/pv/pv.go @@ -714,12 +714,12 @@ func WaitForPVClaimBoundPhase(client clientset.Interface, pvclaims []*v1.Persist } // WaitForPersistentVolumePhase waits for a PersistentVolume to be in a specific phase or until timeout occurs, whichever comes first. -func WaitForPersistentVolumePhase(phase v1.PersistentVolumePhase, c clientset.Interface, pvName string, Poll, timeout time.Duration) error { +func WaitForPersistentVolumePhase(phase v1.PersistentVolumePhase, c clientset.Interface, pvName string, poll, timeout time.Duration) error { framework.Logf("Waiting up to %v for PersistentVolume %s to have phase %s", timeout, pvName, phase) - for start := time.Now(); time.Since(start) < timeout; time.Sleep(Poll) { + for start := time.Now(); time.Since(start) < timeout; time.Sleep(poll) { pv, err := c.CoreV1().PersistentVolumes().Get(context.TODO(), pvName, metav1.GetOptions{}) if err != nil { - framework.Logf("Get persistent volume %s in failed, ignoring for %v: %v", pvName, Poll, err) + framework.Logf("Get persistent volume %s in failed, ignoring for %v: %v", pvName, poll, err) continue } if pv.Status.Phase == phase { @@ -732,23 +732,23 @@ func WaitForPersistentVolumePhase(phase v1.PersistentVolumePhase, c clientset.In } // WaitForPersistentVolumeClaimPhase waits for a PersistentVolumeClaim to be in a specific phase or until timeout occurs, whichever comes first. -func WaitForPersistentVolumeClaimPhase(phase v1.PersistentVolumeClaimPhase, c clientset.Interface, ns string, pvcName string, Poll, timeout time.Duration) error { - return WaitForPersistentVolumeClaimsPhase(phase, c, ns, []string{pvcName}, Poll, timeout, true) +func WaitForPersistentVolumeClaimPhase(phase v1.PersistentVolumeClaimPhase, c clientset.Interface, ns string, pvcName string, poll, timeout time.Duration) error { + return WaitForPersistentVolumeClaimsPhase(phase, c, ns, []string{pvcName}, poll, timeout, true) } // WaitForPersistentVolumeClaimsPhase waits for any (if matchAny is true) or all (if matchAny is false) PersistentVolumeClaims // to be in a specific phase or until timeout occurs, whichever comes first. -func WaitForPersistentVolumeClaimsPhase(phase v1.PersistentVolumeClaimPhase, c clientset.Interface, ns string, pvcNames []string, Poll, timeout time.Duration, matchAny bool) error { +func WaitForPersistentVolumeClaimsPhase(phase v1.PersistentVolumeClaimPhase, c clientset.Interface, ns string, pvcNames []string, poll, timeout time.Duration, matchAny bool) error { if len(pvcNames) == 0 { return fmt.Errorf("Incorrect parameter: Need at least one PVC to track. Found 0") } framework.Logf("Waiting up to %v for PersistentVolumeClaims %v to have phase %s", timeout, pvcNames, phase) - for start := time.Now(); time.Since(start) < timeout; time.Sleep(Poll) { + for start := time.Now(); time.Since(start) < timeout; time.Sleep(poll) { phaseFoundInAllClaims := true for _, pvcName := range pvcNames { pvc, err := c.CoreV1().PersistentVolumeClaims(ns).Get(context.TODO(), pvcName, metav1.GetOptions{}) if err != nil { - framework.Logf("Failed to get claim %q, retrying in %v. Error: %v", pvcName, Poll, err) + framework.Logf("Failed to get claim %q, retrying in %v. Error: %v", pvcName, poll, err) phaseFoundInAllClaims = false break } @@ -828,9 +828,9 @@ func SkipIfNoDefaultStorageClass(c clientset.Interface) { } // WaitForPersistentVolumeDeleted waits for a PersistentVolume to get deleted or until timeout occurs, whichever comes first. -func WaitForPersistentVolumeDeleted(c clientset.Interface, pvName string, Poll, timeout time.Duration) error { +func WaitForPersistentVolumeDeleted(c clientset.Interface, pvName string, poll, timeout time.Duration) error { framework.Logf("Waiting up to %v for PersistentVolume %s to get deleted", timeout, pvName) - for start := time.Now(); time.Since(start) < timeout; time.Sleep(Poll) { + for start := time.Now(); time.Since(start) < timeout; time.Sleep(poll) { pv, err := c.CoreV1().PersistentVolumes().Get(context.TODO(), pvName, metav1.GetOptions{}) if err == nil { framework.Logf("PersistentVolume %s found and phase=%s (%v)", pvName, pv.Status.Phase, time.Since(start)) @@ -840,7 +840,7 @@ func WaitForPersistentVolumeDeleted(c clientset.Interface, pvName string, Poll, framework.Logf("PersistentVolume %s was removed", pvName) return nil } - framework.Logf("Get persistent volume %s in failed, ignoring for %v: %v", pvName, Poll, err) + framework.Logf("Get persistent volume %s in failed, ignoring for %v: %v", pvName, poll, err) } return fmt.Errorf("PersistentVolume %s still exists within %v", pvName, timeout) } From 96eeb982155690e501b11919669b24697d8c6c7c Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Tue, 11 Aug 2020 11:08:20 +0200 Subject: [PATCH 3/3] Fix error messages Separate different errors by ": ". --- test/e2e/storage/csi_mock_volume.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e/storage/csi_mock_volume.go b/test/e2e/storage/csi_mock_volume.go index 0039308ab73..fd206e3cba6 100644 --- a/test/e2e/storage/csi_mock_volume.go +++ b/test/e2e/storage/csi_mock_volume.go @@ -1192,7 +1192,7 @@ func startPausePod(cs clientset.Interface, t testsuites.StorageClassTest, node e _, err = cs.StorageV1().StorageClasses().Get(context.TODO(), class.Name, metav1.GetOptions{}) if err != nil { class, err = cs.StorageV1().StorageClasses().Create(context.TODO(), class, metav1.CreateOptions{}) - framework.ExpectNoError(err, "Failed to create class : %v", err) + framework.ExpectNoError(err, "Failed to create class: %v", err) } claim := e2epv.MakePersistentVolumeClaim(e2epv.PersistentVolumeClaimConfig{ @@ -1206,7 +1206,7 @@ func startPausePod(cs clientset.Interface, t testsuites.StorageClassTest, node e if !t.DelayBinding { pvcClaims := []*v1.PersistentVolumeClaim{claim} _, err = e2epv.WaitForPVClaimBoundPhase(cs, pvcClaims, framework.ClaimProvisionTimeout) - framework.ExpectNoError(err, "Failed waiting for PVC to be bound %v", err) + framework.ExpectNoError(err, "Failed waiting for PVC to be bound: %v", err) } pod, err := startPausePodWithClaim(cs, claim, node, ns)