Skip to content

✨ export queue's length of Controller #1734

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions pkg/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,11 @@ type Controller interface {

// GetLogger returns this controller logger prefilled with basic information.
GetLogger() logr.Logger

// Len returns the current queue length, for informational purposes only. You
// shouldn't e.g. gate a call to Add() or Get() on Len() being a particular
// value, that can't be synchronized properly.
Len() int
}

// New returns a new Controller registered with the Manager. The Manager will ensure that shared Caches have
Expand Down
9 changes: 9 additions & 0 deletions pkg/internal/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,15 @@ func (c *Controller) processNextWorkItem(ctx context.Context) bool {
return true
}

// Len returns the current queue length, for informational purposes only. You
// shouldn't e.g. gate a call to Add() or Get() on Len() being a particular
// value, that can't be synchronized properly.
func (c *Controller) Len() int {
c.mu.Lock()
defer c.mu.Unlock()
return c.Queue.Len()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have to check whether c.Queue is nil, for the field is assigned in later Start().

Also I'm not sure if we really need to export the length of queue. I will leave my comment in the issue.

}

const (
labelError = "error"
labelRequeueAfter = "requeue_after"
Expand Down
33 changes: 33 additions & 0 deletions pkg/internal/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,39 @@ var _ = Describe("controller", func() {
Eventually(func() int { return queue.NumRequeues(request) }).Should(Equal(0))
})

It("should requeue a Request if there is an error and continue processing items using Len()", func() {
ctx, cancel := context.WithCancel(context.Background())
req := reconcile.Request{
NamespacedName: types.NamespacedName{Namespace: "bar", Name: "foo"},
}
queue = &controllertest.Queue{
Interface: workqueue.New(),
}
ctrl.Queue = queue
defer cancel()
go func() {
defer GinkgoRecover()
Expect(ctrl.Start(ctx)).NotTo(HaveOccurred())
}()

queue.Add(request)
queue.Add(req)
Expect(ctrl.Len()).To(Equal(2))
By("Invoking Reconciler which will give an error")

fakeReconcile.AddResult(reconcile.Result{}, fmt.Errorf("expected error: reconcile"))
Expect(<-reconciled).To(Equal(req))

By("Invoking Reconciler a second time without error")
fakeReconcile.AddResult(reconcile.Result{}, nil)
Expect(ctrl.Len()).To(Equal(1))
Expect(<-reconciled).To(Equal(req))

By("Removing the item from the queue")
Expect(ctrl.Len()).To(Equal(0))
Eventually(func() int { return queue.NumRequeues(request) }).Should(Equal(0))
}, 1.0)

PIt("should forget an item if it is not a Request and continue processing items", func() {
// TODO(community): write this test
})
Expand Down