reconcile: Add support for progressive status

Replace the patch Helper with SerialPatcher which is used for
progressive status patching.

Update the tests to use progressive status reasons in tests.

Add ProgressingWithRetry Reconciling reason for failed
reconciliation result to indicate a finished failure operation.

Signed-off-by: Sunny <[email protected]>

Author: darkowlzz

internal/reconcile/reconcile.go

@@ -124,6 +124,15 @@ func ComputeReconcileResult(obj conditions.Setter, res Result, recErr error, rb
 		conditions.Delete(obj, meta.ReconcilingCondition)
 	}
 
+	// Presence of reconciling means that the reconciliation didn't succeed.
+	// Set the Reconciling reason to ProgressingWithRetry to indicate a failure
+	// retry.
+	if conditions.IsReconciling(obj) {
+		reconciling := conditions.Get(obj, meta.ReconcilingCondition)
+		reconciling.Reason = meta.ProgressingWithRetryReason
+		conditions.Set(obj, reconciling)
+	}
+
 	// Analyze the reconcile error.
 	switch t := recErr.(type) {
 	case *serror.Stalling:

internal/reconcile/reconcile_test.go

@@ -206,6 +206,21 @@ func TestComputeReconcileResult(t *testing.T) {
 				t.Expect(conditions.IsUnknown(obj, meta.StalledCondition)).To(BeTrue())
 			},
 		},
+		{
+			name: "failed with Reconciling=True adds ProgressingWithRetry reason",
+			beforeFunc: func(obj conditions.Setter) {
+				conditions.MarkReconciling(obj, meta.ProgressingReason, "some msg")
+			},
+			result:     ResultEmpty,
+			recErr:     fmt.Errorf("some error"),
+			wantResult: ctrl.Result{},
+			wantErr:    true,
+			afterFunc: func(t *WithT, obj conditions.Setter, patchOpts *patch.HelperOptions) {
+			},
+			assertConditions: []metav1.Condition{
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingWithRetryReason, "some msg"),
+			},
+		},
 	}
 
 	for _, tt := range tests {

internal/reconcile/summarize/summary.go

@@ -50,15 +50,15 @@ type Conditions struct {
 
 // Helper is SummarizeAndPatch helper.
 type Helper struct {
-	recorder    kuberecorder.EventRecorder
-	patchHelper *patch.Helper
+	recorder      kuberecorder.EventRecorder
+	serialPatcher *patch.SerialPatcher
 }
 
 // NewHelper returns an initialized Helper.
-func NewHelper(recorder kuberecorder.EventRecorder, patchHelper *patch.Helper) *Helper {
+func NewHelper(recorder kuberecorder.EventRecorder, serialPatcher *patch.SerialPatcher) *Helper {
 	return &Helper{
-		recorder:    recorder,
-		patchHelper: patchHelper,
+		recorder:      recorder,
+		serialPatcher: serialPatcher,
 	}
 }
 
@@ -250,7 +250,7 @@ func (h *Helper) SummarizeAndPatch(ctx context.Context, obj conditions.Setter, o
 	}
 
 	// Finally, patch the resource.
-	if err := h.patchHelper.Patch(ctx, obj, patchOpts...); err != nil {
+	if err := h.serialPatcher.Patch(ctx, obj, patchOpts...); err != nil {
 		// Ignore patch error "not found" when the object is being deleted.
 		if opts.IgnoreNotFound && !obj.GetDeletionTimestamp().IsZero() {
 			err = kerrors.FilterOut(err, func(e error) bool { return apierrors.IsNotFound(e) })

internal/reconcile/summarize/summary_test.go

@@ -128,7 +128,7 @@ func TestSummarizeAndPatch(t *testing.T) {
 			name:       "Success, removes reconciling for successful result",
 			generation: 2,
 			beforeFunc: func(obj conditions.Setter) {
-				conditions.MarkReconciling(obj, "NewRevision", "new index version")
+				conditions.MarkReconciling(obj, meta.ProgressingReason, "new index version")
 				conditions.MarkTrue(obj, meta.ReadyCondition, meta.SucceededReason, "stored artifact")
 			},
 			conditions: []Conditions{testReadyConditions},
@@ -167,15 +167,15 @@ func TestSummarizeAndPatch(t *testing.T) {
 			generation: 7,
 			beforeFunc: func(obj conditions.Setter) {
 				conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", "new index revision")
-				conditions.MarkReconciling(obj, "NewRevision", "new index revision")
+				conditions.MarkReconciling(obj, meta.ProgressingReason, "new index revision")
 			},
 			conditions:   []Conditions{testReadyConditions},
 			reconcileErr: fmt.Errorf("failed to create dir"),
 			wantErr:      true,
 			assertConditions: []metav1.Condition{
 				*conditions.FalseCondition(meta.ReadyCondition, "NewRevision", "new index revision"),
 				*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new index revision"),
-				*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new index revision"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingWithRetryReason, "new index revision"),
 			},
 			afterFunc: func(t *WithT, obj client.Object) {
 				t.Expect(obj).ToNot(HaveStatusObservedGeneration(7))
@@ -264,7 +264,7 @@ func TestSummarizeAndPatch(t *testing.T) {
 			name:       "Fail, reconciling with bipolar condition False, Ready gets bipolar failure value",
 			generation: 2,
 			beforeFunc: func(obj conditions.Setter) {
-				conditions.MarkReconciling(obj, "NewRevision", "new index revision")
+				conditions.MarkReconciling(obj, meta.ProgressingReason, "new index revision")
 				conditions.MarkFalse(obj, sourcev1.SourceVerifiedCondition, "VerifyFailed", "verify failed")
 			},
 			result:            reconcile.ResultEmpty,
@@ -275,14 +275,14 @@ func TestSummarizeAndPatch(t *testing.T) {
 			assertConditions: []metav1.Condition{
 				*conditions.FalseCondition(meta.ReadyCondition, "VerifyFailed", "verify failed"),
 				*conditions.FalseCondition(sourcev1.SourceVerifiedCondition, "VerifyFailed", "verify failed"),
-				*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new index revision"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingWithRetryReason, "new index revision"),
 			},
 		},
 		{
 			name:       "Fail, bipolar condition True, negative polarity True, Ready gets negative polarity value",
 			generation: 2,
 			beforeFunc: func(obj conditions.Setter) {
-				conditions.MarkReconciling(obj, "NewGeneration", "new obj gen")
+				conditions.MarkReconciling(obj, meta.ProgressingReason, "new obj gen")
 				conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", "new digest")
 				conditions.MarkTrue(obj, sourcev1.SourceVerifiedCondition, "Success", "verified")
 			},
@@ -294,7 +294,7 @@ func TestSummarizeAndPatch(t *testing.T) {
 			assertConditions: []metav1.Condition{
 				*conditions.FalseCondition(meta.ReadyCondition, "NewRevision", "new digest"),
 				*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new digest"),
-				*conditions.TrueCondition(meta.ReconcilingCondition, "NewGeneration", "new obj gen"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingWithRetryReason, "new obj gen"),
 				*conditions.TrueCondition(sourcev1.SourceVerifiedCondition, "Success", "verified"),
 			},
 		},
@@ -345,10 +345,9 @@ func TestSummarizeAndPatch(t *testing.T) {
 
 			ctx := context.TODO()
 			g.Expect(client.Create(ctx, obj)).To(Succeed())
-			patchHelper, err := patch.NewHelper(obj, client)
-			g.Expect(err).ToNot(HaveOccurred())
+			serialPatcher := patch.NewSerialPatcher(obj, client)
 
-			summaryHelper := NewHelper(record.NewFakeRecorder(32), patchHelper)
+			summaryHelper := NewHelper(record.NewFakeRecorder(32), serialPatcher)
 			summaryOpts := []Option{
 				WithReconcileResult(tt.result),
 				WithReconcileError(tt.reconcileErr),
@@ -471,15 +470,14 @@ func TestSummarizeAndPatch_Intermediate(t *testing.T) {
 
 			ctx := context.TODO()
 			g.Expect(kclient.Create(ctx, obj)).To(Succeed())
-			patchHelper, err := patch.NewHelper(obj, kclient)
-			g.Expect(err).ToNot(HaveOccurred())
+			serialPatcher := patch.NewSerialPatcher(obj, kclient)
 
-			summaryHelper := NewHelper(record.NewFakeRecorder(32), patchHelper)
+			summaryHelper := NewHelper(record.NewFakeRecorder(32), serialPatcher)
 			summaryOpts := []Option{
 				WithConditions(tt.conditions...),
 				WithResultBuilder(reconcile.AlwaysRequeueResultBuilder{RequeueAfter: interval}),
 			}
-			_, err = summaryHelper.SummarizeAndPatch(ctx, obj, summaryOpts...)
+			_, err := summaryHelper.SummarizeAndPatch(ctx, obj, summaryOpts...)
 			g.Expect(err).ToNot(HaveOccurred())
 
 			g.Expect(obj.Status.Conditions).To(conditions.MatchConditions(tt.assertConditions))