Skip to content

Commit

Permalink
Refactor to avoid nested Eventually()
Browse files Browse the repository at this point in the history
  • Loading branch information
@PBundyra
PBundyra committed Jul 10, 2024
1 parent 8d564aa commit 2ff8f1c
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 17 deletions.
6 changes: 2 additions & 4 deletions pkg/controller/admissionchecks/provisioning/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,10 +245,8 @@ func (c *Controller) syncOwnedProvisionRequest(ctx context.Context, wl *kueue.Wo
attempt := int32(1)
shouldCreatePr := false
if exists {
if isFailed(oldPr) || (isBookingExpired(oldPr) && !workload.IsAdmitted(wl)){
// if the workload is Admitted we don't want to retry on BookingExpired
break
}
if isFailed(oldPr) || (isBookingExpired(oldPr) && !workload.IsAdmitted(wl)) {
// if the workload is Admitted we don't want to retry on BookingExpired
attempt = getAttempt(log, oldPr, wl.Name, checkName)
if attempt <= c.maxRetries {
remainingTime := c.remainingTimeToRetry(oldPr, attempt)
Expand Down
37 changes: 24 additions & 13 deletions test/integration/controller/admissionchecks/provisioning/provisioning_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -497,8 +497,9 @@ var _ = ginkgo.Describe("Provisioning", ginkgo.Ordered, ginkgo.ContinueOnFailure
ginkgo.By("Setting the admission check to the workload", func() {
gomega.Eventually(func(g gomega.Gomega) {
g.Expect(k8sClient.Get(ctx, wlKey, &updatedWl)).Should(gomega.Succeed())
util.SetWorkloadsAdmissionCheck(ctx, k8sClient, &updatedWl, ac.Name, kueue.CheckStatePending, false)
}, util.Timeout, util.Interval).Should(gomega.Succeed())

util.SetWorkloadsAdmissionCheck(ctx, k8sClient, &updatedWl, ac.Name, kueue.CheckStatePending, false)
})

ginkgo.By("Admitting the workload", func() {
Expand Down Expand Up @@ -558,17 +559,19 @@ var _ = ginkgo.Describe("Provisioning", ginkgo.Ordered, ginkgo.ContinueOnFailure
g.Expect(k8sClient.Get(ctx, wlKey, &updatedWl)).To(gomega.Succeed())

g.Expect(workload.IsEvictedByDeactivation(&updatedWl)).To(gomega.BeTrue())
util.ExpectEvictedWorkloadsTotalMetric(cq.Name, kueue.WorkloadEvictedByDeactivation, 1)
}, util.Timeout, util.Interval).Should(gomega.Succeed())

util.ExpectEvictedWorkloadsTotalMetric(cq.Name, kueue.WorkloadEvictedByDeactivation, 1)
})
})

ginkgo.It("Should set AdmissionCheck status to Rejected, deactivate Workload, emit an event, and bump metrics when workloads is not Admitted, and the ProvisioningRequest's condition is set to CapacityRevoked", func() {
ginkgo.By("Setting the admission check to the workload", func() {
gomega.Eventually(func(g gomega.Gomega) {
g.Expect(k8sClient.Get(ctx, wlKey, &updatedWl)).Should(gomega.Succeed())
util.SetWorkloadsAdmissionCheck(ctx, k8sClient, &updatedWl, ac.Name, kueue.CheckStatePending, false)
}, util.Timeout, util.Interval).Should(gomega.Succeed())

util.SetWorkloadsAdmissionCheck(ctx, k8sClient, &updatedWl, ac.Name, kueue.CheckStatePending, false)
})

ginkgo.By("Setting the quota reservation to the workload", func() {
Expand Down Expand Up @@ -615,17 +618,19 @@ var _ = ginkgo.Describe("Provisioning", ginkgo.Ordered, ginkgo.ContinueOnFailure
g.Expect(k8sClient.Get(ctx, wlKey, &updatedWl)).To(gomega.Succeed())

g.Expect(workload.IsEvictedByDeactivation(&updatedWl)).To(gomega.BeTrue())
util.ExpectEvictedWorkloadsTotalMetric(cq.Name, kueue.WorkloadEvictedByDeactivation, 1)
}, util.Timeout, util.Interval).Should(gomega.Succeed())

util.ExpectEvictedWorkloadsTotalMetric(cq.Name, kueue.WorkloadEvictedByDeactivation, 1)
})
})

ginkgo.It("Should not set AdmissionCheck status to Rejected, deactivate Workload, emit an event, and bump metrics when workload is Finished, and the ProvisioningRequest's condition is set to CapacityRevoked", func() {
ginkgo.By("Setting the admission check to the workload", func() {
gomega.Eventually(func(g gomega.Gomega) {
g.Expect(k8sClient.Get(ctx, wlKey, &updatedWl)).To(gomega.Succeed())
util.SetWorkloadsAdmissionCheck(ctx, k8sClient, &updatedWl, ac.Name, kueue.CheckStatePending, false)
}, util.Timeout, util.Interval).Should(gomega.Succeed())

util.SetWorkloadsAdmissionCheck(ctx, k8sClient, &updatedWl, ac.Name, kueue.CheckStatePending, false)
})

ginkgo.By("Setting the quota reservation to the workload", func() {
Expand Down Expand Up @@ -662,8 +667,9 @@ var _ = ginkgo.Describe("Provisioning", ginkgo.Ordered, ginkgo.ContinueOnFailure
ginkgo.By("Marking the workload as Finished", func() {
gomega.Eventually(func(g gomega.Gomega) {
g.Expect(k8sClient.Get(ctx, wlKey, &updatedWl)).To(gomega.Succeed())
util.FinishWorkloads(ctx, k8sClient, &updatedWl)
}, util.Timeout, util.Interval).Should(gomega.Succeed())

util.FinishWorkloads(ctx, k8sClient, &updatedWl)
})

ginkgo.By("Setting the ProvisioningRequest as CapacityRevoked", func() {
Expand Down Expand Up @@ -692,17 +698,19 @@ var _ = ginkgo.Describe("Provisioning", ginkgo.Ordered, ginkgo.ContinueOnFailure
g.Expect(k8sClient.Get(ctx, wlKey, &updatedWl)).To(gomega.Succeed())
g.Expect(workload.IsActive(&updatedWl)).To(gomega.BeTrue())
g.Expect(workload.IsEvictedByDeactivation(&updatedWl)).To(gomega.BeFalse())
util.ExpectEvictedWorkloadsTotalMetric(cq.Name, kueue.WorkloadEvictedByDeactivation, 0)
}, util.Timeout, util.Interval).Should(gomega.Succeed())

util.ExpectEvictedWorkloadsTotalMetric(cq.Name, kueue.WorkloadEvictedByDeactivation, 0)
})
})

ginkgo.It("Should ignore the change if Workload is Admitted and the ProvisioningRequest's condition is set to BookingExpired", func() {
ginkgo.By("Setting the admission check to the workload", func() {
gomega.Eventually(func(g gomega.Gomega) {
g.Expect(k8sClient.Get(ctx, wlKey, &updatedWl)).Should(gomega.Succeed())
util.SetWorkloadsAdmissionCheck(ctx, k8sClient, &updatedWl, ac.Name, kueue.CheckStatePending, false)
}, util.Timeout, util.Interval).Should(gomega.Succeed())

util.SetWorkloadsAdmissionCheck(ctx, k8sClient, &updatedWl, ac.Name, kueue.CheckStatePending, false)
})

ginkgo.By("Setting the quota reservation to the workload", func() {
Expand Down Expand Up @@ -759,9 +767,10 @@ var _ = ginkgo.Describe("Provisioning", ginkgo.Ordered, ginkgo.ContinueOnFailure
ginkgo.By("Setting the admission check to the workload", func() {
gomega.Eventually(func(g gomega.Gomega) {
g.Expect(k8sClient.Get(ctx, wlKey, &updatedWl)).Should(gomega.Succeed())
util.SetWorkloadsAdmissionCheck(ctx, k8sClient, &updatedWl, ac.Name, kueue.CheckStatePending, false)
util.SetWorkloadsAdmissionCheck(ctx, k8sClient, &updatedWl, pendingAC.Name, kueue.CheckStatePending, true)
}, util.Timeout, util.Interval).Should(gomega.Succeed())

util.SetWorkloadsAdmissionCheck(ctx, k8sClient, &updatedWl, ac.Name, kueue.CheckStatePending, false)
util.SetWorkloadsAdmissionCheck(ctx, k8sClient, &updatedWl, pendingAC.Name, kueue.CheckStatePending, true)
})

ginkgo.By("Setting the quota reservation to the workload", func() {
Expand Down Expand Up @@ -843,8 +852,9 @@ var _ = ginkgo.Describe("Provisioning", ginkgo.Ordered, ginkgo.ContinueOnFailure
g.Expect(k8sClient.Get(ctx, wlKey, &updatedWl)).To(gomega.Succeed())

g.Expect(workload.IsEvictedByDeactivation(&updatedWl)).To(gomega.BeTrue(), "The workload should be evicted by deactivation")
util.ExpectEvictedWorkloadsTotalMetric(cq.Name, kueue.WorkloadEvictedByDeactivation, 1)
}, util.Timeout, util.Interval).Should(gomega.Succeed())

util.ExpectEvictedWorkloadsTotalMetric(cq.Name, kueue.WorkloadEvictedByDeactivation, 1)
})
})

Expand Down Expand Up @@ -1395,9 +1405,10 @@ var _ = ginkgo.Describe("Provisioning", ginkgo.Ordered, ginkgo.ContinueOnFailure
ginkgo.By("Setting the admission check to the workload", func() {
gomega.Eventually(func(g gomega.Gomega) {
g.Expect(k8sClient.Get(ctx, wlKey, &updatedWl)).Should(gomega.Succeed())
util.SetWorkloadsAdmissionCheck(ctx, k8sClient, &updatedWl, ac.Name, kueue.CheckStatePending, false)
util.SetWorkloadsAdmissionCheck(ctx, k8sClient, &updatedWl, pendingAC.Name, kueue.CheckStatePending, true)
}, util.Timeout, util.Interval).Should(gomega.Succeed())

util.SetWorkloadsAdmissionCheck(ctx, k8sClient, &updatedWl, ac.Name, kueue.CheckStatePending, false)
util.SetWorkloadsAdmissionCheck(ctx, k8sClient, &updatedWl, pendingAC.Name, kueue.CheckStatePending, true)
})

ginkgo.By("Setting the quota reservation to the workload", func() {
Expand Down

0 comments on commit 2ff8f1c

Please sign in to comment.