From 133be67280b3f2fd68fad3f61989623cd2fc1a7e Mon Sep 17 00:00:00 2001 From: Tomas Aschan Date: Mon, 29 Apr 2024 11:40:50 +0200 Subject: [PATCH] Respect ErrorResults returned from preflight checks A common pattern in our operators is to use preflight checks to call out to some external system for more information (that can be written e.g. to the status of the `DeclarativeObject`) or to validate that some property of the `spec` meets requirements to be able to expand the channel manifests. If the check fails, sometimes the right behavior is to requeue with backoff - but often it's not: e.g., if the spec is deemed invalid, there's no reason to requeue the resource. If spec changes we'll get a new event anyway, and until spec changes we won't be able to reconcile. There is already a `ErrorResult` type that can be used in a Preflight implementation to signal what we want the reconciliation function to return, but it's currently not being respected. This change addresses that. --- pkg/patterns/declarative/reconciler.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/pkg/patterns/declarative/reconciler.go b/pkg/patterns/declarative/reconciler.go index a5aba244..263ca86c 100644 --- a/pkg/patterns/declarative/reconciler.go +++ b/pkg/patterns/declarative/reconciler.go @@ -95,7 +95,11 @@ type ErrorResult struct { } func (e *ErrorResult) Error() string { - return e.Err.Error() + if e.Err != nil { + return e.Err.Error() + } + + return "" } // For mocking @@ -183,6 +187,11 @@ func (r *Reconciler) Reconcile(ctx context.Context, request reconcile.Request) ( if r.options.status != nil { if err := r.options.status.Preflight(ctx, instance); err != nil { + if errorResult, ok := err.(*ErrorResult); ok { + // the user was specific about what they wanted to return; respect that + return errorResult.Result, errorResult.Err + } + log.Error(err, "preflight check failed, not reconciling") statusInfo.Err = err return result, statusInfo.Err