Adding adaptive jitter depending on asset state

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(r.pollInterval))
-	log.Debug("Successfully requested update of external resource", "requeue-after", time.Now().Add(reconcileAfter))
 	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)
 }

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)
+				}
+			}
+		})
+	}
+}