diff --git a/pkg/reconciler/managed/reconciler.go b/pkg/reconciler/managed/reconciler.go index 49fc3a9..78a46a5 100644 --- a/pkg/reconciler/managed/reconciler.go +++ b/pkg/reconciler/managed/reconciler.go @@ -22,6 +22,8 @@ import ( "strings" "time" + v1 "k8s.io/api/core/v1" + kerrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/sets" @@ -632,7 +634,13 @@ func WithMetricRecorder(recorder MetricRecorder) ReconcilerOption { // interval. type PollIntervalHook func(managed resource.Managed, pollInterval time.Duration) time.Duration -func defaultPollIntervalHook(_ resource.Managed, pollInterval time.Duration) time.Duration { +func defaultPollIntervalHook(managed resource.Managed, pollInterval time.Duration) time.Duration { + if managed != nil && + managed.GetCondition(xpv1.TypeSynced).Status == v1.ConditionTrue && + managed.GetCondition(xpv1.TypeReady).Status == v1.ConditionTrue { + jitter := 30 * time.Minute + return time.Hour + time.Duration((rand.Float64()-0.5)*2*float64(jitter)).Round(time.Second) + } return pollInterval } @@ -1353,9 +1361,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu // changes, so we requeue a speculative reconcile after the specified poll // interval in order to observe it and react accordingly. // https://github.com/crossplane/crossplane/issues/289 - reconcileAfter := r.pollIntervalHook(managed, r.pollInterval) - log.Debug("Successfully requested update of external resource", "requeue-after", time.Now().Add(reconcileAfter)) + log.Debug("Successfully requested update of external resource", "requeue-after", time.Now().Add(r.pollInterval)) record.Event(managed, event.Normal(reasonUpdated, "Successfully requested update of external resource")) status.MarkConditions(xpv1.ReconcileSuccess()) - return reconcile.Result{RequeueAfter: reconcileAfter}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return reconcile.Result{RequeueAfter: r.pollInterval}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) } diff --git a/pkg/reconciler/managed/reconciler_test.go b/pkg/reconciler/managed/reconciler_test.go index 249422a..18a0fbb 100644 --- a/pkg/reconciler/managed/reconciler_test.go +++ b/pkg/reconciler/managed/reconciler_test.go @@ -22,6 +22,8 @@ import ( "testing" "time" + v1 "k8s.io/api/core/v1" + "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" kerrors "k8s.io/apimachinery/pkg/api/errors" @@ -2762,3 +2764,105 @@ func managedMockGetFn(err error, generation int64) test.MockGetFn { return nil }) } + +func TestDefaultPollIntervalHook(t *testing.T) { + type args struct { + duration time.Duration + managed resource.Managed + } + type want struct { + expected time.Duration + margin time.Duration + } + cases := map[string]struct { + reason string + args args + want want + }{ + "ResourceIsNil": { + reason: "Should return the default poll interval if the managed resource is nil.", + args: args{ + duration: defaultPollInterval, + managed: nil, + }, + want: want{expected: defaultPollInterval, margin: 0}, + }, + "ResourceNotReady": { + reason: "Should return the default poll interval if the managed resource is not ready.", + args: args{ + duration: defaultPollInterval, + managed: &fake.Managed{ + ConditionedStatus: xpv1.ConditionedStatus{ + Conditions: []xpv1.Condition{ + { + Type: xpv1.TypeReady, + Status: v1.ConditionFalse, + }, + { + Type: xpv1.TypeSynced, + Status: v1.ConditionTrue, + }, + }, + }, + }, + }, + want: want{expected: defaultPollInterval, margin: 0}, + }, + "ResourceNotSynced": { + reason: "Should return the default poll interval if the managed resource is not synced.", + args: args{ + duration: defaultPollInterval, + managed: &fake.Managed{ + ConditionedStatus: xpv1.ConditionedStatus{ + Conditions: []xpv1.Condition{ + { + Type: xpv1.TypeReady, + Status: v1.ConditionTrue, + }, + { + Type: xpv1.TypeSynced, + Status: v1.ConditionFalse, + }, + }, + }, + }, + }, + want: want{expected: defaultPollInterval, margin: 0}, + }, + "ResourceReady": { + reason: "Should return the provided duration if the managed resource is ready.", + args: args{ + duration: 1 * time.Minute, + managed: &fake.Managed{ + ConditionedStatus: xpv1.ConditionedStatus{ + Conditions: []xpv1.Condition{ + { + Type: xpv1.TypeReady, + Status: v1.ConditionTrue, + }, + { + Type: xpv1.TypeSynced, + Status: v1.ConditionTrue, + }, + }, + }, + }, + }, + want: want{expected: 1 * time.Hour, margin: 30 * time.Minute}, + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + r := defaultPollIntervalHook(tc.args.managed, tc.args.duration) + if tc.want.margin == 0 { + if diff := cmp.Diff(tc.want.expected, r); diff != "" { + t.Errorf("\nReason: %s\ndefaultPollIntervalHook(...): -want, +got:\n%s", tc.reason, diff) + } + } else { + if r < tc.want.expected-tc.want.margin || r > tc.want.expected+tc.want.margin { + t.Errorf("defaultPollIntervalHook(...): expected %v, got %v", tc.want.expected, r) + } + } + }) + } +}