diff --git a/internal/reconcile/reconcile.go b/internal/reconcile/reconcile.go index 9b4bd76a..b1e11409 100644 --- a/internal/reconcile/reconcile.go +++ b/internal/reconcile/reconcile.go @@ -73,8 +73,19 @@ type AlwaysRequeueResultBuilder struct { // return values of a controller's Reconcile function. func (r AlwaysRequeueResultBuilder) BuildRuntimeResult(rr Result, err error) ctrl.Result { // Handle special errors that contribute to expressing the result. - if e, ok := err.(*serror.Waiting); ok { + switch e := err.(type) { + case *serror.Waiting: + // Safeguard: If no RequeueAfter is set, use the default success + // RequeueAfter value to ensure a requeue takes place after some time. + if e.RequeueAfter == 0 { + return ctrl.Result{RequeueAfter: r.RequeueAfter} + } return ctrl.Result{RequeueAfter: e.RequeueAfter} + case *serror.Generic: + // no-op error, reconcile at success interval. + if e.Ignore { + return ctrl.Result{RequeueAfter: r.RequeueAfter} + } } switch rr { @@ -132,6 +143,17 @@ func ComputeReconcileResult(obj conditions.Setter, res Result, recErr error, rb conditions.Delete(obj, meta.StalledCondition) // The reconciler needs to wait and retry. Return no error. return pOpts, result, nil + case *serror.Generic: + conditions.Delete(obj, meta.StalledCondition) + // If ignore, it's a no-op error, return no error, remove reconciling + // condition. + if t.Ignore { + // The current generation has been reconciled successfully with + // no-op result. Update status observed generation. + pOpts = append(pOpts, patch.WithStatusObservedGeneration{}) + conditions.Delete(obj, meta.ReconcilingCondition) + return pOpts, result, nil + } case nil: // The reconcile didn't result in any error, we are not in stalled // state. If a requeue is requested, the current generation has not been diff --git a/internal/reconcile/reconcile_test.go b/internal/reconcile/reconcile_test.go index a8edc5e4..3d3f4fc0 100644 --- a/internal/reconcile/reconcile_test.go +++ b/internal/reconcile/reconcile_test.go @@ -135,12 +135,46 @@ func TestComputeReconcileResult(t *testing.T) { name: "waiting error", result: ResultEmpty, recErr: &serror.Waiting{Err: fmt.Errorf("some error"), Reason: "some reason"}, - wantResult: ctrl.Result{}, + wantResult: ctrl.Result{RequeueAfter: testSuccessInterval}, wantErr: false, afterFunc: func(t *WithT, obj conditions.Setter, patchOpts *patch.HelperOptions) { t.Expect(patchOpts.IncludeStatusObservedGeneration).To(BeFalse()) }, }, + { + name: "generic error, Stalled=True, remove Stalled", + result: ResultEmpty, + beforeFunc: func(obj conditions.Setter) { + conditions.MarkStalled(obj, "SomeReason", "some message") + }, + recErr: &serror.Generic{ + Err: fmt.Errorf("some error"), Reason: "some reason", + }, + wantResult: ctrl.Result{}, + afterFunc: func(t *WithT, obj conditions.Setter, patchOpts *patch.HelperOptions) { + t.Expect(conditions.IsUnknown(obj, meta.StalledCondition)).To(BeTrue()) + }, + wantErr: true, + }, + { + name: "generic ignore error, Reconciling=True, remove Reconciling", + result: ResultEmpty, + beforeFunc: func(obj conditions.Setter) { + conditions.MarkReconciling(obj, "NewRevision", "new revision") + }, + recErr: &serror.Generic{ + Err: fmt.Errorf("some error"), Reason: "some reason", + Config: serror.Config{ + Ignore: true, + }, + }, + wantResult: ctrl.Result{RequeueAfter: testSuccessInterval}, + afterFunc: func(t *WithT, obj conditions.Setter, patchOpts *patch.HelperOptions) { + t.Expect(patchOpts.IncludeStatusObservedGeneration).To(BeTrue()) + t.Expect(conditions.IsUnknown(obj, meta.ReconcilingCondition)).To(BeTrue()) + }, + wantErr: false, + }, { name: "random error", result: ResultEmpty, @@ -188,7 +222,9 @@ func TestComputeReconcileResult(t *testing.T) { for _, o := range pOpts { o.ApplyToHelper(opts) } - tt.afterFunc(g, obj, opts) + if tt.afterFunc != nil { + tt.afterFunc(g, obj, opts) + } }) } }