fix: controller: return empty Result when bubbling errors

[c3d]

controller-runtime ignores the Result when err != nil, using exponential backoff instead. Returning both a RequeueAfter delay and an error is misleading - the delay is never applied.

Changed all error returns from:

  return ctrl.Result{Requeue: true, RequeueAfter: duration}, err

to:

  return ctrl.Result{}, err

This makes the error handling behavior explicit and matches controller-runtime semantics. The controller will still requeue, but using its default exponential backoff mechanism.

Found while addressing code review comments on PR #2410

Fixes: #2433

Assisted-By: Claude Sonnet 4.5

[coderabbitai]

Warning

Review limit reached

@c3d, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 58 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 5287a389-48dc-4bb5-96d7-4125a7d21208

📥 Commits

Reviewing files that changed from the base of the PR and between e953969 and 0115b42.

📒 Files selected for processing (2)

• config/peerpods/podvm/cloud-api-adaptor
• controllers/daemonset_reconcile.go

📝 Walkthrough

Walkthrough

Updated DaemonSet install and uninstall reconciliation error paths to return ctrl.Result{}, err instead of timed requeue results combined with errors. Changes cover DaemonSet operations, controller-reference setup, progress checks, cleanup, node eligibility, and post-install processing.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: jensfr, snir911

🚥 Pre-merge checks | ✅ 15

✅ Passed checks (15 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: returning an empty Result when bubbling errors from the controller.
Description check	✅ Passed	The description matches the code change and explains why the empty Result replaces delayed requeue handling.
Linked Issues check	✅ Passed	The updated error-return paths in controllers/daemonset_reconcile.go align with issue #2433's required behavior.
Out of Scope Changes check	✅ Passed	The changes appear scoped to the daemonset reconcile error paths and do not introduce unrelated modifications.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Stable And Deterministic Test Names	✅ Passed	No test titles were added or changed; the only modified file is production code and contains no Ginkgo It/Describe/Context/When blocks.
Test Structure And Quality	✅ Passed	No Ginkgo test code changed in this PR; only controllers/daemonset_reconcile.go was modified, so the test-structure check is not applicable.
Microshift Test Compatibility	✅ Passed	No new Ginkgo e2e tests were added; the changed tests are unit/envtest code, with no It/Describe/Context/When additions or MicroShift-specific guards needed.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	Only controller reconciliation error-handling changed; no Ginkgo e2e tests were added or modified, so this SNO check is not applicable.
Topology-Aware Scheduling Compatibility	✅ Passed	Only reconcile error-return paths changed; no manifests, replicas, nodeSelectors, affinities, tolerations, or topology checks were added or altered.
Ote Binary Stdout Contract	✅ Passed	Only controllers/daemonset_reconcile.go changed; it has no main/init/suite code or stdout writes, so the OTE stdout contract isn’t implicated.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	No new Ginkgo/e2e tests were added; the PR only changes controller error returns in daemonset_reconcile.go, so this compatibility check is not applicable.
No-Weak-Crypto	✅ Passed	Touched controller file only changes requeue/error handling; no weak-crypto terms, custom crypto, or secret-comparison helpers are present.
Container-Privileges	✅ Passed	Diff only changes reconcile error returns; no manifest or securityContext privilege fields were added or modified.
No-Sensitive-Data-In-Logs	✅ Passed	Diff only changes error return values; no new or modified logs expose sensitive data.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[c3d]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[Copilot]

Pull request overview

This PR updates the DaemonSet-based KataConfig reconciler to align with controller-runtime semantics: when a non-nil error is returned, the ctrl.Result is ignored and requeue timing is driven by controller-runtime’s exponential backoff. The change makes error returns explicit by returning an empty ctrl.Result{} alongside the error.

Changes:

• Replaced return ctrl.Result{Requeue: true, RequeueAfter: ...}, err with return ctrl.Result{}, err for error paths in the DaemonSet reconcile flow.
• Removes misleading explicit requeue delays in error returns (since they are not applied by controller-runtime when err != nil).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[c3d]

Closed since this was merged elsewhere

[openshift-ci]

@c3d: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions 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. I understand the commands that are listed here.