Skip to content

Commit b9f30df

Browse files
authored
Merge pull request #396 from alaypatel07/issue/392
🐛 Reset Failure count for reconcile loops using RequestAfter
2 parents 3a8f89d + 5a841fe commit b9f30df

File tree

2 files changed

+49
-3
lines changed

2 files changed

+49
-3
lines changed

pkg/internal/controller/controller.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,11 @@ func (c *Controller) processNextWorkItem() bool {
214214
ctrlmetrics.ReconcileTotal.WithLabelValues(c.Name, "error").Inc()
215215
return false
216216
} else if result.RequeueAfter > 0 {
217+
// The result.RequeueAfter request will be lost, if it is returned
218+
// along with a non-nil error. But this is intended as
219+
// We need to drive to stable reconcile loops before queuing due
220+
// to result.RequestAfter
221+
c.Queue.Forget(obj)
217222
c.Queue.AddAfter(req, result.RequeueAfter)
218223
ctrlmetrics.ReconcileTotal.WithLabelValues(c.Name, "requeue_after").Inc()
219224
return true

pkg/internal/controller/controller_test.go

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ var _ = Describe("controller", func() {
360360
})
361361

362362
It("should requeue a Request after a duration if the Result sets Requeue:true and "+
363-
"RequeueAfter is set", func() {
363+
"RequeueAfter is set and forget the item", func() {
364364
fakeReconcile.Result.RequeueAfter = time.Millisecond * 100
365365
go func() {
366366
defer GinkgoRecover()
@@ -375,14 +375,14 @@ var _ = Describe("controller", func() {
375375

376376
By("Invoking Reconciler which will ask for requeue")
377377
Expect(<-reconciled).To(Equal(request))
378-
Expect(dq.countAdd).To(Equal(1))
378+
Expect(dq.countAdd).To(Equal(0))
379379
Expect(dq.countAddAfter).To(Equal(1))
380380
Expect(dq.countAddRateLimited).To(Equal(0))
381381

382382
By("Invoking Reconciler a second time without asking for requeue")
383383
fakeReconcile.Result.Requeue = false
384384
Expect(<-reconciled).To(Equal(request))
385-
Expect(dq.countAdd).To(Equal(1))
385+
Expect(dq.countAdd).To(Equal(0))
386386
Expect(dq.countAddAfter).To(Equal(1))
387387
Expect(dq.countAddRateLimited).To(Equal(0))
388388

@@ -391,6 +391,42 @@ var _ = Describe("controller", func() {
391391
Eventually(func() int { return ctrl.Queue.NumRequeues(request) }).Should(Equal(0))
392392
})
393393

394+
PIt("should not requeue a Request after a duration if the Result sets Requeue:true and "+
395+
"RequeueAfter is set and err is not nil", func() {
396+
397+
fakeReconcile.Result.RequeueAfter = time.Millisecond * 100
398+
fakeReconcile.Err = fmt.Errorf("expected error: reconcile")
399+
go func() {
400+
defer GinkgoRecover()
401+
Expect(ctrl.Start(stop)).NotTo(HaveOccurred())
402+
}()
403+
dq := &DelegatingQueue{RateLimitingInterface: ctrl.Queue}
404+
ctrl.Queue = dq
405+
ctrl.JitterPeriod = time.Millisecond
406+
ctrl.Queue.Add(request)
407+
Expect(dq.countAdd).To(Equal(1))
408+
Expect(dq.countAddAfter).To(Equal(0))
409+
Expect(dq.countAddRateLimited).To(Equal(0))
410+
411+
By("Invoking Reconciler which will ask for requeue with an error")
412+
Expect(<-reconciled).To(Equal(request))
413+
Expect(dq.countAdd).To(Equal(1))
414+
Expect(dq.countAddAfter).To(Equal(0))
415+
Expect(dq.countAddRateLimited).To(Equal(1))
416+
417+
fakeReconcile.Result.RequeueAfter = time.Millisecond * 100
418+
fakeReconcile.Err = nil
419+
By("Invoking Reconciler a second time asking for requeue without errors")
420+
Expect(<-reconciled).To(Equal(request))
421+
Expect(dq.countAdd).To(Equal(0))
422+
Expect(dq.countAddAfter).To(Equal(1))
423+
Expect(dq.countAddRateLimited).To(Equal(1))
424+
425+
By("Removing the item from the queue")
426+
Eventually(ctrl.Queue.Len).Should(Equal(0))
427+
Eventually(func() int { return ctrl.Queue.NumRequeues(request) }).Should(Equal(0))
428+
})
429+
394430
PIt("should forget the Request if Reconciler is successful", func() {
395431
// TODO(community): write this test
396432
})
@@ -639,3 +675,8 @@ func (q *DelegatingQueue) Add(item interface{}) {
639675
q.countAdd++
640676
q.RateLimitingInterface.Add(item)
641677
}
678+
679+
func (q *DelegatingQueue) Forget(item interface{}) {
680+
q.countAdd--
681+
q.RateLimitingInterface.Forget(item)
682+
}

0 commit comments

Comments
 (0)