diff --git a/go.mod b/go.mod index 3e9b4b7d3..c3534cd30 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( dario.cat/mergo v1.0.2 github.com/AdaLogics/go-fuzz-headers v0.0.0-20240806141605-e8a1dd7889d6 github.com/Masterminds/semver/v3 v3.4.0 - github.com/crossplane/crossplane/apis/v2 v2.0.0-20260407152912-8f8e265fb638 + github.com/crossplane/crossplane/apis/v2 v2.0.0-20260420215119-8ed4b5288fc1 github.com/evanphx/json-patch v5.9.11+incompatible github.com/go-logr/logr v1.4.3 github.com/google/go-cmp v0.7.0 diff --git a/go.sum b/go.sum index 0fd0889e5..c06899f57 100644 --- a/go.sum +++ b/go.sum @@ -136,8 +136,8 @@ github.com/coreos/go-oidc/v3 v3.17.0/go.mod h1:wqPbKFrVnE90vty060SB40FCJ8fTHTxSw github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= github.com/creack/pty v1.1.18 h1:n56/Zwd5o6whRC5PMGretI4IdRLlmBXYNjScPaBgsbY= github.com/creack/pty v1.1.18/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4= -github.com/crossplane/crossplane/apis/v2 v2.0.0-20260407152912-8f8e265fb638 h1:CcM0AkXpmfFTPjqYV3PYKcNIdaUFpYl8c/iJrUjcCdg= -github.com/crossplane/crossplane/apis/v2 v2.0.0-20260407152912-8f8e265fb638/go.mod h1:pl+8j97av1Tv1az8CwIzXepl1YfwI5/fOsWO6Bh4sNI= +github.com/crossplane/crossplane/apis/v2 v2.0.0-20260420215119-8ed4b5288fc1 h1:hbCo4ir8DehMobOmjTdbIbyHw304BgL6P0bR6u2J5Ws= +github.com/crossplane/crossplane/apis/v2 v2.0.0-20260420215119-8ed4b5288fc1/go.mod h1:h7KE74Z4TFs1L/FFv3RdsiG9Uax7L56oHpcggSZnONg= github.com/cyberphone/json-canonicalization v0.0.0-20241213102144-19d51d7fe467 h1:uX1JmpONuD549D73r6cgnxyUu18Zb7yHAy5AYU0Pm4Q= github.com/cyberphone/json-canonicalization v0.0.0-20241213102144-19d51d7fe467/go.mod h1:uzvlm1mxhHkdfqitSA92i7Se+S9ksOn3a3qmv/kyOCw= github.com/danieljoos/wincred v1.2.3 h1:v7dZC2x32Ut3nEfRH+vhoZGvN72+dQ/snVXo/vMFLdQ= diff --git a/gomod2nix.toml b/gomod2nix.toml index 018a9781f..cad3f6fe0 100644 --- a/gomod2nix.toml +++ b/gomod2nix.toml @@ -125,8 +125,8 @@ schema = 3 version = "v3.17.0" hash = "sha256-b9dCq5GN5ac64UG23Rijv1qcmUZNcxb8DJQycAa96EQ=" [mod."github.com/crossplane/crossplane/apis/v2"] - version = "v2.0.0-20260407152912-8f8e265fb638" - hash = "sha256-5d79zPJ85YcoSTaAEi+Zs67IfM32YURgVpR3hv2m8PU=" + version = "v2.0.0-20260420215119-8ed4b5288fc1" + hash = "sha256-wtcG/nMC4A9nebFxsfSlWZjdupAQ6IjHinFhvFB6KNk=" [mod."github.com/cyberphone/json-canonicalization"] version = "v0.0.0-20241213102144-19d51d7fe467" hash = "sha256-eqH3UKAZ9eOlZjYdN7nWuJ1hFm2JAP1PVbJInQk6OLw=" diff --git a/pkg/reconciler/managed/reconciler.go b/pkg/reconciler/managed/reconciler.go index 188835416..e2c511590 100644 --- a/pkg/reconciler/managed/reconciler.go +++ b/pkg/reconciler/managed/reconciler.go @@ -24,6 +24,7 @@ import ( xpv2 "github.com/crossplane/crossplane/apis/v2/core/v2" kerrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/sets" "sigs.k8s.io/controller-runtime/pkg/client" @@ -92,7 +93,8 @@ const ( reasonUpdated event.Reason = "UpdatedExternalResource" reasonPending event.Reason = "PendingExternalResource" - reasonReconciliationPaused event.Reason = "ReconciliationPaused" + reasonReconciliationPaused event.Reason = "ReconciliationPaused" + reasonReconcileRequestHandled event.Reason = "ReconcileRequestHandled" ) // ControllerName returns the recommended name for controllers that use this @@ -124,6 +126,14 @@ type ManagementPoliciesChecker interface { //nolint:interfacebloat // This has t ShouldDelete() bool } +// A reconcileRequestTracker can record which reconcile-request token was last +// handled. Managed resources that embed ObservedStatus implement this +// interface automatically. +type reconcileRequestTracker interface { + GetLastHandledReconcileAt() string + SetLastHandledReconcileAt(token string) +} + // A CriticalAnnotationUpdater is used when it is critical that annotations must // be updated before returning from the Reconcile loop. type CriticalAnnotationUpdater interface { @@ -572,6 +582,7 @@ type Reconciler struct { newManaged func() resource.Managed pollInterval time.Duration + minPollInterval time.Duration pollIntervalHook PollIntervalHook timeout time.Duration @@ -673,6 +684,15 @@ func WithPollInterval(after time.Duration) ReconcilerOption { } } +// WithMinPollInterval specifies the shortest poll interval a resource may +// request via annotation. Annotation values below this floor are clamped to +// the minimum. +func WithMinPollInterval(d time.Duration) ReconcilerOption { + return func(r *Reconciler) { + r.minPollInterval = d + } +} + // WithMetricRecorder configures the Reconciler to use the supplied MetricRecorder. func WithMetricRecorder(recorder MetricRecorder) ReconcilerOption { return func(r *Reconciler) { @@ -841,6 +861,19 @@ func WithDeterministicExternalName(b bool) ReconcilerOption { } } +// effectivePollInterval returns the poll interval for the given resource, +// taking into account any per-resource override via annotation. Overrides +// below the configured minimum are clamped to the minimum. +func (r *Reconciler) effectivePollInterval(o metav1.Object) time.Duration { + if d, ok := meta.GetPollInterval(o); ok { + if d >= r.minPollInterval { + return d + } + return r.minPollInterval + } + return r.pollInterval +} + // NewReconciler returns a Reconciler that reconciles managed resources of the // supplied ManagedKind with resources in an external system such as a cloud // provider API. It panics if asked to reconcile a managed resource kind that is @@ -945,6 +978,33 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu return reconcile.Result{}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) } + // Detect a new reconcile-request token early and emit the event once. + // The actual status mutation is deferred to updateStatus so it survives + // full-object Updates (late-init, create annotations) that reset + // in-memory status. + var reconcileRequestToken string + if token, ok := meta.GetReconcileRequest(managed); ok { + if tracker, ok := managed.(reconcileRequestTracker); ok { + if tracker.GetLastHandledReconcileAt() != token { + log.Debug("Processing reconcile request", "token", token) + record.Event(managed, event.Normal(reasonReconcileRequestHandled, "Handling reconcile request", "token", token)) + reconcileRequestToken = token + } + } + } + + // updateStatus applies the reconcile-request token (if any) immediately + // before persisting status. This ensures the token is not lost by + // intervening full-object Updates. + updateStatus := func() error { + if reconcileRequestToken != "" { + if tracker, ok := managed.(reconcileRequestTracker); ok { + tracker.SetLastHandledReconcileAt(reconcileRequestToken) + } + } + return r.client.Status().Update(ctx, managed) + } + // Check if the ManagementPolicies is set to a non-default value while the // feature is not enabled. This is a safety check to let users know that // they need to enable the feature flag before using the feature. For @@ -963,7 +1023,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu record.Event(managed, event.Warning(reasonManagementPolicyInvalid, err)) status.MarkConditions(xpv2.ReconcileError(err)) - return reconcile.Result{}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return reconcile.Result{}, errors.Wrap(updateStatus(), errUpdateManagedStatus) } // If managed resource has a deletion timestamp and a deletion policy of @@ -991,7 +1051,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu record.Event(managed, event.Warning(reasonCannotUnpublish, err)) status.MarkConditions(xpv2.Deleting(), xpv2.ReconcileError(err)) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return reconcile.Result{Requeue: true}, errors.Wrap(updateStatus(), errUpdateManagedStatus) } if err := r.managed.RemoveFinalizer(ctx, managed); err != nil { @@ -1007,7 +1067,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu status.MarkConditions(xpv2.Deleting(), xpv2.ReconcileError(err)) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return reconcile.Result{Requeue: true}, errors.Wrap(updateStatus(), errUpdateManagedStatus) } // We've successfully unpublished our managed resource's connection @@ -1033,7 +1093,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu record.Event(managed, event.Warning(reasonCannotInitialize, err)) status.MarkConditions(xpv2.ReconcileError(err)) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return reconcile.Result{Requeue: true}, errors.Wrap(updateStatus(), errUpdateManagedStatus) } // If we started but never completed creation of an external resource we @@ -1047,7 +1107,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu record.Event(managed, event.Warning(reasonCannotInitialize, errors.New(errCreateIncomplete))) status.MarkConditions(xpv2.Creating(), xpv2.ReconcileError(errors.New(errCreateIncomplete))) - return reconcile.Result{Requeue: false}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return reconcile.Result{Requeue: false}, errors.Wrap(updateStatus(), errUpdateManagedStatus) } log.Debug("Cannot determine creation result, but proceeding due to deterministic external name") @@ -1078,7 +1138,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu record.Event(managed, event.Warning(reasonCannotResolveRefs, err)) status.MarkConditions(xpv2.ReconcileError(err)) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return reconcile.Result{Requeue: true}, errors.Wrap(updateStatus(), errUpdateManagedStatus) } } @@ -1098,7 +1158,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu record.Event(managed, event.Warning(reasonCannotConnect, err)) status.MarkConditions(xpv2.ReconcileError(errors.Wrap(err, errReconcileConnect))) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return reconcile.Result{Requeue: true}, errors.Wrap(updateStatus(), errUpdateManagedStatus) } defer func() { @@ -1132,7 +1192,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu record.Event(managed, event.Warning(reasonCannotObserve, err)) status.MarkConditions(xpv2.ReconcileError(errors.Wrap(err, errReconcileObserve))) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return reconcile.Result{Requeue: true}, errors.Wrap(updateStatus(), errUpdateManagedStatus) } // In the observe-only mode, !observation.ResourceExists will be an error @@ -1141,7 +1201,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu record.Event(managed, event.Warning(reasonCannotObserve, errors.New(errExternalResourceNotExist))) status.MarkConditions(xpv2.ReconcileError(errors.Wrap(errors.New(errExternalResourceNotExist), errReconcileObserve))) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return reconcile.Result{Requeue: true}, errors.Wrap(updateStatus(), errUpdateManagedStatus) } // If this resource has a non-zero creation grace period we want to wait @@ -1190,7 +1250,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu record.Event(managed, event.Warning(reasonCannotDelete, err)) status.MarkConditions(xpv2.Deleting(), xpv2.ReconcileError(errors.Wrap(err, errReconcileDelete))) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return reconcile.Result{Requeue: true}, errors.Wrap(updateStatus(), errUpdateManagedStatus) } // We've successfully requested deletion of our external resource. @@ -1209,7 +1269,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu record.Event(managed, event.Normal(reasonDeleted, "Successfully requested deletion of external resource")) status.MarkConditions(xpv2.Deleting(), xpv2.ReconcileSuccess()) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return reconcile.Result{Requeue: true}, errors.Wrap(updateStatus(), errUpdateManagedStatus) } if err := r.managed.UnpublishConnection(ctx, managed, observation.ConnectionDetails); err != nil { @@ -1226,7 +1286,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu record.Event(managed, event.Warning(reasonCannotUnpublish, err)) status.MarkConditions(xpv2.Deleting(), xpv2.ReconcileError(err)) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return reconcile.Result{Requeue: true}, errors.Wrap(updateStatus(), errUpdateManagedStatus) } if err := r.managed.RemoveFinalizer(ctx, managed); err != nil { @@ -1242,7 +1302,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu status.MarkConditions(xpv2.Deleting(), xpv2.ReconcileError(err)) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return reconcile.Result{Requeue: true}, errors.Wrap(updateStatus(), errUpdateManagedStatus) } // We've successfully deleted our external resource (if necessary) and @@ -1268,7 +1328,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu record.Event(managed, event.Warning(reasonCannotPublish, err)) status.MarkConditions(xpv2.ReconcileError(err)) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return reconcile.Result{Requeue: true}, errors.Wrap(updateStatus(), errUpdateManagedStatus) } if err := r.managed.AddFinalizer(ctx, managed); err != nil { @@ -1283,7 +1343,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu status.MarkConditions(xpv2.ReconcileError(err)) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return reconcile.Result{Requeue: true}, errors.Wrap(updateStatus(), errUpdateManagedStatus) } if !observation.ResourceExists && policy.ShouldCreate() { @@ -1306,7 +1366,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu record.Event(managed, event.Warning(reasonCannotUpdateManaged, errors.Wrap(err, errUpdateManaged))) status.MarkConditions(xpv2.Creating(), xpv2.ReconcileError(errors.Wrap(err, errUpdateManaged))) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return reconcile.Result{Requeue: true}, errors.Wrap(updateStatus(), errUpdateManagedStatus) } creation, err := external.Create(externalCtx, managed) @@ -1347,7 +1407,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu status.MarkConditions(xpv2.Creating(), xpv2.ReconcileError(errors.Wrap(err, errReconcileCreate))) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return reconcile.Result{Requeue: true}, errors.Wrap(updateStatus(), errUpdateManagedStatus) } // In some cases our external-name may be set by Create above. @@ -1380,7 +1440,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu record.Event(managed, event.Warning(reasonCannotUpdateManaged, errors.Wrap(err, errUpdateManagedAnnotations))) status.MarkConditions(xpv2.Creating(), xpv2.ReconcileError(errors.Wrap(err, errUpdateManagedAnnotations))) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return reconcile.Result{Requeue: true}, errors.Wrap(updateStatus(), errUpdateManagedStatus) } if _, err := r.managed.PublishConnection(ctx, managed, creation.ConnectionDetails); err != nil { @@ -1396,7 +1456,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu record.Event(managed, event.Warning(reasonCannotPublish, err)) status.MarkConditions(xpv2.Creating(), xpv2.ReconcileError(err)) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return reconcile.Result{Requeue: true}, errors.Wrap(updateStatus(), errUpdateManagedStatus) } // We've successfully created our external resource. In many cases the @@ -1407,7 +1467,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu record.Event(managed, event.Normal(reasonCreated, "Successfully requested creation of external resource")) status.MarkConditions(xpv2.Creating(), xpv2.ReconcileSuccess()) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return reconcile.Result{Requeue: true}, errors.Wrap(updateStatus(), errUpdateManagedStatus) } if observation.ResourceLateInitialized && policy.ShouldLateInitialize() { @@ -1423,7 +1483,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu record.Event(managed, event.Warning(reasonCannotUpdateManaged, err)) status.MarkConditions(xpv2.ReconcileError(errors.Wrap(err, errUpdateManaged))) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return reconcile.Result{Requeue: true}, errors.Wrap(updateStatus(), errUpdateManagedStatus) } } @@ -1434,7 +1494,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) + reconcileAfter := r.pollIntervalHook(managed, r.effectivePollInterval(managed)) log.Debug("External resource is up to date", "requeue-after", time.Now().Add(reconcileAfter)) status.MarkConditions(xpv2.ReconcileSuccess()) r.metricRecorder.recordFirstTimeReady(managed) @@ -1445,7 +1505,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu // that the external object would not have been updated. r.metricRecorder.recordUnchanged(managed.GetName()) - return reconcile.Result{RequeueAfter: reconcileAfter}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return reconcile.Result{RequeueAfter: reconcileAfter}, errors.Wrap(updateStatus(), errUpdateManagedStatus) } if observation.Diff != "" { @@ -1454,11 +1514,11 @@ 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) + reconcileAfter := r.pollIntervalHook(managed, r.effectivePollInterval(managed)) log.Debug("Skipping update due to managementPolicies. Reconciliation succeeded", "requeue-after", time.Now().Add(reconcileAfter)) status.MarkConditions(xpv2.ReconcileSuccess()) - return reconcile.Result{RequeueAfter: reconcileAfter}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return reconcile.Result{RequeueAfter: reconcileAfter}, errors.Wrap(updateStatus(), errUpdateManagedStatus) } update, err := external.Update(externalCtx, managed) @@ -1477,7 +1537,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu record.Event(managed, event.Warning(reasonCannotUpdate, err)) status.MarkConditions(xpv2.ReconcileError(errors.Wrap(err, errReconcileUpdate))) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return reconcile.Result{Requeue: true}, errors.Wrap(updateStatus(), errUpdateManagedStatus) } // record the drift after the successful update. @@ -1495,7 +1555,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu record.Event(managed, event.Warning(reasonCannotPublish, err)) status.MarkConditions(xpv2.ReconcileError(err)) - return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return reconcile.Result{Requeue: true}, errors.Wrap(updateStatus(), errUpdateManagedStatus) } // We've successfully updated our external resource. Per the below issue @@ -1503,10 +1563,10 @@ 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) + reconcileAfter := r.pollIntervalHook(managed, r.effectivePollInterval(managed)) 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(xpv2.ReconcileSuccess()) - return reconcile.Result{RequeueAfter: reconcileAfter}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus) + return reconcile.Result{RequeueAfter: reconcileAfter}, errors.Wrap(updateStatus(), errUpdateManagedStatus) } diff --git a/pkg/reconciler/managed/reconciler_modern_test.go b/pkg/reconciler/managed/reconciler_modern_test.go index 3e980dd58..8bab04d9c 100644 --- a/pkg/reconciler/managed/reconciler_modern_test.go +++ b/pkg/reconciler/managed/reconciler_modern_test.go @@ -2853,6 +2853,246 @@ func TestReconcilerChangeLogs(t *testing.T) { } } +func TestEffectivePollInterval(t *testing.T) { + cases := map[string]struct { + reason string + pollInterval time.Duration + minPollInterval time.Duration + annotation string + want time.Duration + }{ + "NoAnnotationReturnsDefault": { + reason: "When no annotation is set, the default poll interval is returned.", + pollInterval: 10 * time.Minute, + minPollInterval: 1 * time.Second, + want: 10 * time.Minute, + }, + "ValidAnnotationAboveMinimumReturnsAnnotation": { + reason: "When the annotation is at or above the minimum, it overrides the default.", + pollInterval: 10 * time.Minute, + minPollInterval: 1 * time.Second, + annotation: "24h", + want: 24 * time.Hour, + }, + "ValidAnnotationAtMinimumReturnsAnnotation": { + reason: "When the annotation equals the minimum exactly, it is returned as-is.", + pollInterval: 10 * time.Minute, + minPollInterval: 30 * time.Second, + annotation: "30s", + want: 30 * time.Second, + }, + "ValidAnnotationBelowMinimumReturnsMinimum": { + reason: "When the annotation is below the minimum, the minimum is returned.", + pollInterval: 10 * time.Minute, + minPollInterval: 30 * time.Second, + annotation: "1s", + want: 30 * time.Second, + }, + "InvalidAnnotationReturnsDefault": { + reason: "When the annotation cannot be parsed, the default poll interval is returned.", + pollInterval: 5 * time.Minute, + minPollInterval: 1 * time.Second, + annotation: "not-a-duration", + want: 5 * time.Minute, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + r := &Reconciler{ + pollInterval: tc.pollInterval, + minPollInterval: tc.minPollInterval, + } + + mg := &fake.ModernManaged{} + if tc.annotation != "" { + mg.SetAnnotations(map[string]string{ + meta.AnnotationKeyPollInterval: tc.annotation, + }) + } + + got := r.effectivePollInterval(mg) + if diff := cmp.Diff(tc.want, got); diff != "" { + t.Errorf("\n%s\neffectivePollInterval(...): -want, +got:\n%s", tc.reason, diff) + } + }) + } +} + +func TestReconcilePollIntervalAnnotation(t *testing.T) { + cases := map[string]struct { + reason string + pollInterval time.Duration + minPollInterval time.Duration + annotation string + wantApprox time.Duration + wantTolerance time.Duration + }{ + "AnnotationOverridesPollInterval": { + reason: "When a valid poll interval annotation is set, it should override the controller-level poll interval.", + pollInterval: 1 * time.Minute, + minPollInterval: 1 * time.Second, + annotation: "24h", + wantApprox: 24 * time.Hour, + wantTolerance: 1 * time.Second, + }, + "InvalidAnnotationFallsBack": { + reason: "When an invalid poll interval annotation is set, the controller-level poll interval should be used.", + pollInterval: 5 * time.Minute, + minPollInterval: 1 * time.Second, + annotation: "not-a-duration", + wantApprox: 5 * time.Minute, + wantTolerance: 1 * time.Second, + }, + "AnnotationBelowMinimumClampsToMin": { + reason: "When a poll interval annotation is below the configured minimum, the minimum should be used.", + pollInterval: 10 * time.Minute, + minPollInterval: 30 * time.Second, + annotation: "1s", + wantApprox: 30 * time.Second, + wantTolerance: 1 * time.Second, + }, + "NoAnnotationUsesDefault": { + reason: "When no poll interval annotation is set, the controller-level poll interval should be used.", + pollInterval: 10 * time.Minute, + minPollInterval: 1 * time.Second, + wantApprox: 10 * time.Minute, + wantTolerance: 1 * time.Second, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + r := NewReconciler(&fake.Manager{ + Client: &test.MockClient{ + MockGet: test.NewMockGetFn(nil, func(obj client.Object) error { + mg := asModernManaged(obj, 42) + if tc.annotation != "" { + mg.SetAnnotations(map[string]string{ + meta.AnnotationKeyPollInterval: tc.annotation, + }) + } + return nil + }), + MockStatusUpdate: test.MockSubResourceUpdateFn(func(_ context.Context, _ client.Object, _ ...client.SubResourceUpdateOption) error { + return nil + }), + }, + Scheme: fake.SchemeWith(&fake.ModernManaged{}), + }, resource.ManagedKind(fake.GVK(&fake.ModernManaged{})), + WithPollInterval(tc.pollInterval), + WithMinPollInterval(tc.minPollInterval), + WithInitializers(), + WithReferenceResolver(ReferenceResolverFn(func(_ context.Context, _ resource.Managed) error { return nil })), + WithExternalConnector(ExternalConnectorFn(func(_ context.Context, _ resource.Managed) (ExternalClient, error) { + return &ExternalClientFns{ + ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) { + return ExternalObservation{ResourceExists: true, ResourceUpToDate: true}, nil + }, + DisconnectFn: func(_ context.Context) error { return nil }, + }, nil + })), + WithFinalizer(resource.FinalizerFns{AddFinalizerFn: func(_ context.Context, _ resource.Object) error { return nil }}), + ) + + got, err := r.Reconcile(context.Background(), reconcile.Request{}) + if diff := cmp.Diff(nil, err, cmpopts.EquateErrors()); diff != "" { + t.Fatalf("\n%s\nr.Reconcile(...): -want error, +got error:\n%s", tc.reason, diff) + } + + diff := got.RequeueAfter - tc.wantApprox + if diff < 0 { + diff = -diff + } + if diff > tc.wantTolerance { + t.Errorf("\n%s\nr.Reconcile(...): want RequeueAfter ~%v (±%v), got %v", + tc.reason, tc.wantApprox, tc.wantTolerance, got.RequeueAfter) + } + }) + } +} + +func TestReconcileRequestAnnotation(t *testing.T) { + type want struct { + result reconcile.Result + err error + token string + } + + cases := map[string]struct { + reason string + annotation string + handled string + want want + }{ + "NewReconcileRequestRecordsToken": { + reason: "A new reconcile-requested-at token should be recorded in status.lastHandledReconcileAt.", + annotation: "1705312200", + want: want{ + result: reconcile.Result{RequeueAfter: defaultPollInterval}, + token: "1705312200", + }, + }, + "AlreadyHandledReconcileRequestIsNoOp": { + reason: "When the token matches lastHandledReconcileAt, status should remain unchanged.", + annotation: "already-handled", + handled: "already-handled", + want: want{ + result: reconcile.Result{RequeueAfter: defaultPollInterval}, + token: "already-handled", + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + r := NewReconciler(&fake.Manager{ + Client: &test.MockClient{ + MockGet: test.NewMockGetFn(nil, func(obj client.Object) error { + mg := asModernManaged(obj, 42) + mg.SetAnnotations(map[string]string{ + meta.AnnotationKeyReconcileRequestedAt: tc.annotation, + }) + if tc.handled != "" { + mg.SetLastHandledReconcileAt(tc.handled) + } + return nil + }), + MockStatusUpdate: test.MockSubResourceUpdateFn(func(_ context.Context, obj client.Object, _ ...client.SubResourceUpdateOption) error { + mg := obj.(*fake.ModernManaged) + got := mg.GetLastHandledReconcileAt() + if diff := cmp.Diff(tc.want.token, got); diff != "" { + t.Errorf("\n%s\nstatus.lastHandledReconcileAt: -want, +got:\n%s", tc.reason, diff) + } + return nil + }), + }, + Scheme: fake.SchemeWith(&fake.ModernManaged{}), + }, resource.ManagedKind(fake.GVK(&fake.ModernManaged{})), + WithInitializers(), + WithReferenceResolver(ReferenceResolverFn(func(_ context.Context, _ resource.Managed) error { return nil })), + WithExternalConnector(ExternalConnectorFn(func(_ context.Context, _ resource.Managed) (ExternalClient, error) { + return &ExternalClientFns{ + ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) { + return ExternalObservation{ResourceExists: true, ResourceUpToDate: true}, nil + }, + DisconnectFn: func(_ context.Context) error { return nil }, + }, nil + })), + WithFinalizer(resource.FinalizerFns{AddFinalizerFn: func(_ context.Context, _ resource.Object) error { return nil }}), + ) + + got, err := r.Reconcile(context.Background(), reconcile.Request{}) + if diff := cmp.Diff(tc.want.err, err, cmpopts.EquateErrors()); diff != "" { + t.Errorf("\n%s\nr.Reconcile(...): -want error, +got error:\n%s", tc.reason, diff) + } + if diff := cmp.Diff(tc.want.result, got); diff != "" { + t.Errorf("\n%s\nr.Reconcile(...): -want result, +got result:\n%s", tc.reason, diff) + } + }) + } +} + func asModernManaged(obj client.Object, generation int64) *fake.ModernManaged { mg := obj.(*fake.ModernManaged) mg.Generation = generation diff --git a/pkg/resource/fake/mocks.go b/pkg/resource/fake/mocks.go index 145ff5622..7992cfc1b 100644 --- a/pkg/resource/fake/mocks.go +++ b/pkg/resource/fake/mocks.go @@ -373,6 +373,7 @@ type ModernManaged struct { LocalConnectionSecretWriterTo Manageable xpv2.ConditionedStatus + xpv2.ObservedStatus } // GetObjectKind returns schema.ObjectKind.