Add per-resource poll interval and reconcile-request annotations#982
Add per-resource poll interval and reconcile-request annotations#982adamwg merged 1 commit intocrossplane:mainfrom
Conversation
e191129 to
e671e68
Compare
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 45 minutes and 30 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR introduces reconciliation request tracking via a new interface that records when reconcile requests are handled, and adds configurable minimum poll intervals with per-resource annotation overrides to the managed resource reconciler. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/reconciler/managed/reconciler_modern_test.go`:
- Around line 2856-3082: The test doesn't assert the full reconcile.Result and
lacks a case for a poll-interval annotation below the configured minimum; update
TestReconcilePollIntervalAnnotation to (1) extend the case struct to include
minPollInterval, pass it to WithMinPollInterval(...) when constructing the
Reconciler, and add a case "AnnotationBelowMinimumClampsToMin" with an
annotation shorter than minPollInterval expecting Reconcile.Result.RequeueAfter
~ minPollInterval and Requeue==false; (2) replace the current partial
RequeueAfter comparison with a full cmp.Diff on the entire reconcile.Result (use
cmpopts.EquateErrors() for any error comparisons) so unexpected Requeue bools or
other fields are caught; and (3) ensure the effectivePollInterval and
WithMinPollInterval behavior is exercised by referencing effectivePollInterval
in assertions indirectly via the Reconcile call on the NewReconciler instance.
In `@pkg/reconciler/managed/reconciler.go`:
- Around line 981-991: The reconcile-request token is being set on the managed
object early and can be lost by later full-object Update paths; instead, ensure
the token is persisted at the moment you persist status by moving the
SetLastHandledReconcileAt write into the status-update wrapper (use/update the
updateStatus() helper) so the token is applied to the in-memory status just
before calling Status().Update. Concretely: change code that currently calls
tracker.SetLastHandledReconcileAt(token) near the top to only record the event
(record.Event with reasonReconcileRequestHandled) and defer updating
GetLastHandledReconcileAt/SetLastHandledReconcileAt until inside the
updateStatus() function (and update all status persistence callsites to use
updateStatus()), ensuring meta.GetReconcileRequest, reconcileRequestTracker,
tracker.GetLastHandledReconcileAt and tracker.SetLastHandledReconcileAt are used
to detect/assign the token immediately prior to the Status().Update call.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 12824fb8-3530-4159-a891-917b6c366abf
⛔ Files ignored due to path filters (3)
go.modis excluded by none and included by nonego.sumis excluded by!**/*.sumand included by nonepkg/resource/fake/mocks.gois excluded by!**/fake/**and included by**/*.go
📒 Files selected for processing (2)
pkg/reconciler/managed/reconciler.gopkg/reconciler/managed/reconciler_modern_test.go
e671e68 to
e8f2b84
Compare
…nnotations Fixes crossplane/crossplane#7204 Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
e8f2b84 to
727ced0
Compare
adamwg
left a comment
There was a problem hiding this comment.
lgtm, thanks for seeing this through!
ObservedStatusautomatically satisfy the reconcile-request tracker interface