Skip to content

Commit

Permalink
Correctly wait for resourcequota "ready" state in tests (kubevirt#2801)
Browse files Browse the repository at this point in the history
Apparently the correct way to understand if a quota is "ready"
is to look at the `used` field. Hopefully this eliminates
```bash
persistentvolumeclaims "test-pvc" is forbidden: status unknown for quota: test-storage-quota
```

Sources:
https://github.com/kubernetes/apiserver/blob/a32e26b98aff9e1dfdf0fb8d0d49d9b56287b875/pkg/admission/plugin/resourcequota/controller.go#L473-L475
https://github.com/kubernetes/kubernetes/blob/a24aef65108bb3549c9ad31f51aab8097c640447/test/e2e/apimachinery/resource_quota.go#L2152

Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>
Co-authored-by: Alex Kalenyuk <akalenyu@redhat.com>
  • Loading branch information
kubevirt-bot and akalenyu committed Jul 12, 2023
1 parent 3045f0a commit 2a11e39
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 5 deletions.
15 changes: 11 additions & 4 deletions tests/framework/framework.go
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ func (f *Framework) CreateQuotaInSpecifiedNs(ns string, requestCPU, requestMemor
if err != nil {
return false, err
}
return len(quota.Status.Hard) == 4, nil
return len(quota.Status.Used) == 4, nil
})
}

Expand All @@ -466,7 +466,14 @@ func (f *Framework) UpdateQuotaInNs(requestCPU, requestMemory, limitsCPU, limits
if err != nil {
ginkgo.Fail("Unable to set resource quota " + err.Error())
}
return err
return wait.PollImmediate(5*time.Second, nsDeleteTime, func() (bool, error) {
quota, err := f.K8sClient.CoreV1().ResourceQuotas(f.Namespace.GetName()).Get(context.TODO(), "test-quota", metav1.GetOptions{})
if err != nil {
fmt.Fprintf(ginkgo.GinkgoWriter, "ERROR: GET ResourceQuota failed once, retrying: %v\n", err.Error())
return false, nil
}
return len(quota.Status.Used) == 4, nil
})
}

// CreateStorageQuota creates a quota to limit pvc count and cumulative storage capacity
Expand Down Expand Up @@ -494,7 +501,7 @@ func (f *Framework) CreateStorageQuota(numPVCs, requestStorage int64) error {
fmt.Fprintf(ginkgo.GinkgoWriter, "ERROR: GET ResourceQuota failed once, retrying: %v\n", err.Error())
return false, nil
}
return len(quota.Status.Hard) == 2, nil
return len(quota.Status.Used) == 2, nil
})
}

Expand Down Expand Up @@ -524,7 +531,7 @@ func (f *Framework) UpdateStorageQuota(numPVCs, requestStorage int64) error {
}
requestStorageUpdated := resource.NewQuantity(requestStorage, resource.DecimalSI).Cmp(quota.Status.Hard[v1.ResourceRequestsStorage])
numPVCsUpdated := resource.NewQuantity(numPVCs, resource.DecimalSI).Cmp(quota.Status.Hard[v1.ResourcePersistentVolumeClaims])
return requestStorageUpdated+numPVCsUpdated == 0, nil
return len(quota.Status.Used) == 2 && requestStorageUpdated+numPVCsUpdated == 0, nil
})
}

Expand Down
11 changes: 10 additions & 1 deletion tests/transfer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ var _ = Describe("[rfe_id:5630][crit:high]ObjectTransfer tests", func() {
Entry("with same namespace and explicit name", false, &[]string{"target-name"}[0]),
)

It("[posneg:negative][test_id:5734]should handle quota failure", func() {
It("[posneg:negative][test_id:5734]should report quota failure on dv transfer and succeed once quota is large enough", func() {
sq := int64(100 * 1024 * 1024)
bq := int64(1024 * 1024 * 1024)
dataVolume := createDV(f.Namespace.Name, "source-dv")
Expand All @@ -423,9 +423,17 @@ var _ = Describe("[rfe_id:5630][crit:high]ObjectTransfer tests", func() {
},
},
}
quotaUpdated := func() int {
quota, err := f.K8sClient.CoreV1().ResourceQuotas(targetNs.Name).Get(context.TODO(), rq.Name, metav1.GetOptions{})
if err != nil {
return -1
}
return len(quota.Status.Used)
}

rq, err = f.K8sClient.CoreV1().ResourceQuotas(targetNs.Name).Create(context.TODO(), rq, metav1.CreateOptions{})
Expect(err).ToNot(HaveOccurred())
Eventually(quotaUpdated, 2*time.Minute, 2*time.Second).Should(BeNumerically("==", 1))

ot := &cdiv1.ObjectTransfer{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -470,6 +478,7 @@ var _ = Describe("[rfe_id:5630][crit:high]ObjectTransfer tests", func() {
rq.Spec.Hard[corev1.ResourceRequestsStorage] = *resource.NewQuantity(bq, resource.DecimalSI)
rq, err = f.K8sClient.CoreV1().ResourceQuotas(targetNs.Name).Update(context.TODO(), rq, metav1.UpdateOptions{})
Expect(err).ToNot(HaveOccurred())
Eventually(quotaUpdated, 2*time.Minute, 2*time.Second).Should(BeNumerically("==", 1))

Eventually(func() bool {
ot2, err := f.CdiClient.CdiV1beta1().ObjectTransfers().Get(context.TODO(), ot.Name, metav1.GetOptions{})
Expand Down

0 comments on commit 2a11e39

Please sign in to comment.