Skip to content

Commit

Permalink
Add a hook for customising the poll interval
Browse files Browse the repository at this point in the history
  • Loading branch information
toastwaffle committed Sep 4, 2023
1 parent 701d760 commit 1b1b951
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 3 deletions.
34 changes: 31 additions & 3 deletions pkg/reconciler/managed/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -469,8 +469,10 @@ type Reconciler struct {
client client.Client
newManaged func() resource.Managed

pollInterval time.Duration
pollJitter time.Duration
pollInterval time.Duration
pollJitter time.Duration
pollIntervalHook PollIntervalHook

timeout time.Duration
creationGracePeriod time.Duration

Expand Down Expand Up @@ -555,6 +557,27 @@ func WithPollJitter(jitter time.Duration) ReconcilerOption {
}
}

// PollIntervalHook represents the function type passed to the
// WithPollIntervalHook option to support dynamic computation of the poll
// interval. The default behaviour is to add configuredPollInterval and
// computedJitter together.
type PollIntervalHook func(managed resource.Managed, configuredPollInterval, computedJitter time.Duration) time.Duration

func defaultPollIntervalHook(managed resource.Managed, configuredPollInterval, computedJitter time.Duration) time.Duration {
return configuredPollInterval + computedJitter
}

// WithPollIntervalHook configures a hook that can be used to configure the
// delay before an up-to-date resource is reconciled again after a successful
// reconcile. The default behaviour is to add computed jitter to the configured
// poll interval, as configured by the WithPollInterval and WithPollJitter
// options.
func WithPollIntervalHook(hook PollIntervalHook) ReconcilerOption {
return func(r *Reconciler) {
r.pollIntervalHook = hook
}
}

// WithCreationGracePeriod configures an optional period during which we will
// wait for the external API to report that a newly created external resource
// exists. This allows us to tolerate eventually consistent APIs that do not
Expand Down Expand Up @@ -673,6 +696,8 @@ func NewReconciler(m manager.Manager, of resource.ManagedKind, o ...ReconcilerOp
client: m.GetClient(),
newManaged: nm,
pollInterval: defaultPollInterval,
pollJitter: defaultPollJitter,
pollIntervalHook: defaultPollIntervalHook,
creationGracePeriod: defaultGracePeriod,
timeout: reconcileTimeout,
managed: defaultMRManaged(m),
Expand Down Expand Up @@ -1087,7 +1112,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
}
}

reconcileAfter := r.pollInterval + time.Duration((rand.Float64()-0.5)*2*float64(r.pollJitter)) //#nosec G404 -- no need for secure randomness
computedJitter := time.Duration((rand.Float64()-0.5)*2*float64(r.pollJitter)) //#nosec G404 -- no need for secure randomness

if observation.ResourceUpToDate {
// We did not need to create, update, or delete our external resource.
Expand All @@ -1096,6 +1121,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
// 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, computedJitter)
log.Debug("External resource is up to date", "requeue-after", time.Now().Add(reconcileAfter))
managed.SetConditions(xpv1.ReconcileSuccess())
return reconcile.Result{RequeueAfter: reconcileAfter}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
Expand All @@ -1107,6 +1133,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu

// skip the update if the management policy is set to ignore updates
if !policy.ShouldUpdate() {
reconcileAfter := r.pollIntervalHook(managed, r.pollInterval, computedJitter)
log.Debug("Skipping update due to managementPolicies. Reconciliation succeeded", "requeue-after", time.Now().Add(reconcileAfter))
managed.SetConditions(xpv1.ReconcileSuccess())
return reconcile.Result{RequeueAfter: reconcileAfter}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
Expand Down Expand Up @@ -1140,6 +1167,7 @@ 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, computedJitter)
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"))
managed.SetConditions(xpv1.ReconcileSuccess())
Expand Down
49 changes: 49 additions & 0 deletions pkg/reconciler/managed/reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -998,6 +998,55 @@ func TestReconciler(t *testing.T) {
})},
},
},
"ExternalResourceUpToDateWithJitterAndIntervalHook": {
reason: "When the external resource exists and is up to date a requeue should be triggered after a long wait processed by the interval hook.",
args: args{
m: &fake.Manager{
Client: &test.MockClient{
MockGet: test.NewMockGetFn(nil),
MockStatusUpdate: test.MockSubResourceUpdateFn(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error {
want := &fake.Managed{}
want.SetConditions(xpv1.ReconcileSuccess())
if diff := cmp.Diff(want, obj, test.EquateConditions()); diff != "" {
reason := "A successful no-op reconcile should be reported as a conditioned status."
t.Errorf("\nReason: %s\n-want, +got:\n%s", reason, diff)
}
return nil
}),
},
Scheme: fake.SchemeWith(&fake.Managed{}),
},
mg: resource.ManagedKind(fake.GVK(&fake.Managed{})),
o: []ReconcilerOption{
WithInitializers(),
WithReferenceResolver(ReferenceResolverFn(func(_ context.Context, _ resource.Managed) error { return nil })),
WithExternalConnecter(ExternalConnectorFn(func(_ context.Context, mg resource.Managed) (ExternalClient, error) {
c := &ExternalClientFns{
ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) {
return ExternalObservation{ResourceExists: true, ResourceUpToDate: true}, nil
},
}
return c, nil
})),
WithConnectionPublishers(),
WithFinalizer(resource.FinalizerFns{AddFinalizerFn: func(_ context.Context, _ resource.Object) error { return nil }}),
WithPollJitter(time.Second),
WithPollIntervalHook(func(managed resource.Managed, configuredPollInterval, computedJitter time.Duration) time.Duration {
return 2*configuredPollInterval + computedJitter
}),
},
},
want: want{
result: reconcile.Result{RequeueAfter: 2*defaultPollInterval},
resultCmpOpts: []cmp.Option{cmp.Comparer(func(l, r time.Duration) bool {
diff := l - r
if diff < 0 {
diff = -diff
}
return diff < time.Second
})},
},
},
"UpdateExternalError": {
reason: "Errors while updating an external resource should trigger a requeue after a short wait.",
args: args{
Expand Down

0 comments on commit 1b1b951

Please sign in to comment.