fix: return empty result on rule reconcile errors - #472
kubernetes-prow[bot] merged 1 commit into
Conversation
In RuleReconciler.Reconcile, five error return sites returned ctrl.Result{RequeueAfter: time.Minute} alongside a non-nil error. controller-runtime unconditionally discards RequeueAfter when err != nil, routes the reconcile to exponential backoff, and logs a warning on each attempt.
Align RuleReconciler error returns with NodeReconciler by returning ctrl.Result{}, err so backoff applies without warnings, and add a unit test verifying the return result.
Signed-off-by: Divyansh Rawat <divyanshrawatofficial@gmail.com>
kubernetes-prow
Bot
added
the
kind/bug
✅ Deploy Preview for node-readiness-controller canceled.
|
kubernetes-prow
Bot
added
cncf-cla: yes
|
Hi @DsThakurRawat. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
kubernetes-prow
Bot
added
the
size/M
Thanks for the PR. It was intended that we retry after a cool-off. I wasn't aware of this behavior. /cc @Karthik-K-N |
Karthik-K-N
left a comment
Karthik-K-N
left a comment
There was a problem hiding this comment.
Thanks for the cleanup
/lgtm
kubernetes-prow
Bot
added
the
lgtm
|
/approve Thanks |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ajaysundark, DsThakurRawat The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
kubernetes-prow
Bot
added
the
approved
3 participants
Description
In
RuleReconciler.Reconcile, five return sites returnedctrl.Result{RequeueAfter: time.Minute}, erron errors. Whenerr != nil, controller-runtime always ignoresRequeueAfterand logs:Warning: Reconciler returned both a result with either RequeueAfter or Requeue set and a non-nil error. RequeueAfter and Requeue will always be ignored if the error is non-nil.This change updates those sites to
return ctrl.Result{}, err, matching the pattern used inNodeReconciler.Reconcile(internal/controller/node_controller.go:107), and adds a unit test innodereadinessrule_controller_test.goverifying the result on reconcile failure.Related Issue
Fixes #468
Type of Change
/kind bug
Testing
Ran unit tests and linters locally:
make test: all controller tests passing, 86.9% statement coverage ininternal/controller.make lint: clean run, 0 issues across active linters.Checklist
make testpassesmake lintpassesDoes this PR introduce a user-facing change?
Doc #(issue)
Generative AI Usage Disclosure
How they were used:
Used an AI assistant to edit the five return statements in nodereadinessrule_controller.go and add a unit test in nodereadinessrule_controller_test.go. Verified all changes through local code review, make test, and make lint.