Add progressive status in helmrepo reconciler

Signed-off-by: Sunny <darkowlzz@protonmail.com>

controllers/helmrepository_controller.go

@@ -43,6 +43,7 @@ import (
 	helper "github.com/fluxcd/pkg/runtime/controller"
 	"github.com/fluxcd/pkg/runtime/patch"
 	"github.com/fluxcd/pkg/runtime/predicates"
+	rreconcile "github.com/fluxcd/pkg/runtime/reconcile"
 
 	sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
 	"github.com/fluxcd/source-controller/internal/cache"
@@ -109,6 +110,8 @@ type HelmRepositoryReconciler struct {
 	Cache *cache.Cache
 	TTL   time.Duration
 	*cache.CacheRecorder
+
+	patchOptions []patch.Option
 }
 
 type HelmRepositoryReconcilerOptions struct {
@@ -120,13 +123,15 @@ type HelmRepositoryReconcilerOptions struct {
 // v1beta2.HelmRepository (sub)reconcile functions. The type implementations
 // are grouped and executed serially to perform the complete reconcile of the
 // object.
-type helmRepositoryReconcileFunc func(ctx context.Context, obj *sourcev1.HelmRepository, artifact *sourcev1.Artifact, repo *repository.ChartRepository) (sreconcile.Result, error)
+type helmRepositoryReconcileFunc func(ctx context.Context, sp *patch.SerialPatcher, obj *sourcev1.HelmRepository, artifact *sourcev1.Artifact, repo *repository.ChartRepository) (sreconcile.Result, error)
 
 func (r *HelmRepositoryReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return r.SetupWithManagerAndOptions(mgr, HelmRepositoryReconcilerOptions{})
 }
 
 func (r *HelmRepositoryReconciler) SetupWithManagerAndOptions(mgr ctrl.Manager, opts HelmRepositoryReconcilerOptions) error {
+	r.patchOptions = getPatchOptions(helmRepositoryReadyCondition.Owned, r.ControllerName)
+
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&sourcev1.HelmRepository{}).
 		WithEventFilter(
@@ -160,10 +165,7 @@ func (r *HelmRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 	r.RecordSuspend(ctx, obj, obj.Spec.Suspend)
 
 	// Initialize the patch helper with the current version of the object.
-	patchHelper, err := patch.NewHelper(obj, r.Client)
+	serialPatcher := patch.NewSerialPatcher(obj, r.Client)
-	if err != nil {
-		return ctrl.Result{}, err
-	}
 
 	// recResult stores the abstracted reconcile result.
 	var recResult sreconcile.Result
@@ -171,7 +173,7 @@ func (r *HelmRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 	// Always attempt to patch the object after each reconciliation.
 	// NOTE: The final runtime result and error are set in this block.
 	defer func() {
-		summarizeHelper := summarize.NewHelper(r.EventRecorder, patchHelper)
+		summarizeHelper := summarize.NewHelper(r.EventRecorder, serialPatcher)
 		summarizeOpts := []summarize.Option{
 			summarize.WithConditions(helmRepositoryReadyCondition),
 			summarize.WithReconcileResult(recResult),
@@ -219,19 +221,36 @@ func (r *HelmRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 		r.reconcileSource,
 		r.reconcileArtifact,
 	}
-	recResult, retErr = r.reconcile(ctx, obj, reconcilers)
+	recResult, retErr = r.reconcile(ctx, serialPatcher, obj, reconcilers)
 	return
 }
 
 // reconcile iterates through the helmRepositoryReconcileFunc tasks for the
 // object. It returns early on the first call that returns
 // reconcile.ResultRequeue, or produces an error.
-func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, obj *sourcev1.HelmRepository, reconcilers []helmRepositoryReconcileFunc) (sreconcile.Result, error) {
+func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, sp *patch.SerialPatcher,
+	obj *sourcev1.HelmRepository, reconcilers []helmRepositoryReconcileFunc) (sreconcile.Result, error) {
 	oldObj := obj.DeepCopy()
 
-	// Mark as reconciling if generation differs.
+	rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "reconciliation in progress")
-	if obj.Generation != obj.Status.ObservedGeneration {
+
-		conditions.MarkReconciling(obj, "NewGeneration", "reconciling new object generation (%d)", obj.Generation)
+	var reconcileAtVal string
+	if v, ok := meta.ReconcileAnnotationValue(obj.GetAnnotations()); ok {
+		reconcileAtVal = v
+	}
+
+	// Persist reconciling if generation differs or reconciliation is requested.
+	switch {
+	case obj.Generation != obj.Status.ObservedGeneration:
+		rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason,
+			"processing object: new generation %d -> %d", obj.Status.ObservedGeneration, obj.Generation)
+		if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
+			return sreconcile.ResultEmpty, err
+		}
+	case reconcileAtVal != obj.Status.GetLastHandledReconcileRequest():
+		if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
+			return sreconcile.ResultEmpty, err
+		}
 	}
 
 	var chartRepo repository.ChartRepository
@@ -241,7 +260,7 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, obj *sourcev1.
 	var res sreconcile.Result
 	var resErr error
 	for _, rec := range reconcilers {
-		recResult, err := rec(ctx, obj, &artifact, &chartRepo)
+		recResult, err := rec(ctx, sp, obj, &artifact, &chartRepo)
 		// Exit immediately on ResultRequeue.
 		if recResult == sreconcile.ResultRequeue {
 			return sreconcile.ResultRequeue, nil
@@ -311,22 +330,32 @@ func (r *HelmRepositoryReconciler) notify(ctx context.Context, oldObj, newObj *s
 // condition is added.
 // The hostname of any URL in the Status of the object are updated, to ensure
 // they match the Storage server hostname of current runtime.
-func (r *HelmRepositoryReconciler) reconcileStorage(ctx context.Context, obj *sourcev1.HelmRepository, _ *sourcev1.Artifact, _ *repository.ChartRepository) (sreconcile.Result, error) {
+func (r *HelmRepositoryReconciler) reconcileStorage(ctx context.Context, sp *patch.SerialPatcher,
+	obj *sourcev1.HelmRepository, _ *sourcev1.Artifact, _ *repository.ChartRepository) (sreconcile.Result, error) {
 	// Garbage collect previous advertised artifact(s) from storage
 	_ = r.garbageCollect(ctx, obj)
 
 	// Determine if the advertised artifact is still in storage
+	var artifactMissing bool
 	if artifact := obj.GetArtifact(); artifact != nil && !r.Storage.ArtifactExist(*artifact) {
 		obj.Status.Artifact = nil
 		obj.Status.URL = ""
+		artifactMissing = true
 		// Remove the condition as the artifact doesn't exist.
 		conditions.Delete(obj, sourcev1.ArtifactInStorageCondition)
 	}
 
 	// Record that we do not have an artifact
 	if obj.GetArtifact() == nil {
-		conditions.MarkReconciling(obj, "NoArtifact", "no artifact for resource in storage")
+		msg := "building artifact"
+		if artifactMissing {
+			msg += ": disappeared from storage"
+		}
+		rreconcile.ProgressiveStatus(true, obj, meta.ProgressingReason, msg)
 		conditions.Delete(obj, sourcev1.ArtifactInStorageCondition)
+		if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
+			return sreconcile.ResultEmpty, err
+		}
 		return sreconcile.ResultSuccess, nil
 	}
 
@@ -346,7 +375,8 @@ func (r *HelmRepositoryReconciler) reconcileStorage(ctx context.Context, obj *so
 // If successful and the index is valid, any previous
 // v1beta2.FetchFailedCondition is removed, and the repository.ChartRepository
 // pointer is set to the newly fetched index.
-func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, obj *sourcev1.HelmRepository, artifact *sourcev1.Artifact, chartRepo *repository.ChartRepository) (sreconcile.Result, error) {
+func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch.SerialPatcher,
+	obj *sourcev1.HelmRepository, artifact *sourcev1.Artifact, chartRepo *repository.ChartRepository) (sreconcile.Result, error) {
 	var tlsConfig *tls.Config
 
 	// Configure Helm client to access repository
@@ -455,8 +485,14 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, obj *sou
 	// Mark observations about the revision on the object.
 	if !obj.GetArtifact().HasRevision(chartRepo.Checksum) {
 		message := fmt.Sprintf("new index revision '%s'", checksum)
+		if obj.GetArtifact() != nil {
 			conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", message)
-		conditions.MarkReconciling(obj, "NewRevision", message)
+		}
+		rreconcile.ProgressiveStatus(true, obj, meta.ProgressingReason, "building artifact: %s", message)
+		if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
+			ctrl.LoggerFrom(ctx).Error(err, "failed to patch")
+			return sreconcile.ResultEmpty, err
+		}
 	}
 
 	// Create potential new artifact.
@@ -482,13 +518,13 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, obj *sou
 // early.
 // On a successful archive, the Artifact in the Status of the object is set,
 // and the symlink in the Storage is updated to its path.
-func (r *HelmRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *sourcev1.HelmRepository, artifact *sourcev1.Artifact, chartRepo *repository.ChartRepository) (sreconcile.Result, error) {
+func (r *HelmRepositoryReconciler) reconcileArtifact(ctx context.Context, sp *patch.SerialPatcher, obj *sourcev1.HelmRepository, artifact *sourcev1.Artifact, chartRepo *repository.ChartRepository) (sreconcile.Result, error) {
 	// Set the ArtifactInStorageCondition if there's no drift.
 	defer func() {
 		if obj.GetArtifact().HasRevision(artifact.Revision) {
 			conditions.Delete(obj, sourcev1.ArtifactOutdatedCondition)
 			conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason,
-				"stored artifact for revision '%s'", artifact.Revision)
+				"stored artifact: revision '%s'", artifact.Revision)
 		}
 
 		chartRepo.Unload()

controllers/helmrepository_controller_test.go

@@ -159,6 +159,7 @@ func TestHelmRepositoryReconciler_reconcileStorage(t *testing.T) {
 					}
 				}
 				testStorage.SetArtifactURL(obj.Status.Artifact)
+				conditions.MarkTrue(obj, meta.ReadyCondition, "foo", "bar")
 				return nil
 			},
 			assertArtifact: &sourcev1.Artifact{
@@ -175,6 +176,17 @@ func TestHelmRepositoryReconciler_reconcileStorage(t *testing.T) {
 				"!/reconcile-storage/a.txt",
 			},
 			want: sreconcile.ResultSuccess,
+			assertConditions: []metav1.Condition{
+				*conditions.TrueCondition(meta.ReadyCondition, "foo", "bar"),
+			},
+		},
+		{
+			name: "build artifact first time",
+			want: sreconcile.ResultSuccess,
+			assertConditions: []metav1.Condition{
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact"),
+				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact"),
+			},
 		},
 		{
 			name: "notices missing artifact in storage",
@@ -191,7 +203,8 @@ func TestHelmRepositoryReconciler_reconcileStorage(t *testing.T) {
 				"!/reconcile-storage/invalid.txt",
 			},
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(meta.ReconcilingCondition, "NoArtifact", "no artifact for resource in storage"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: disappeared from storage"),
+				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: disappeared from storage"),
 			},
 		},
 		{
@@ -209,6 +222,7 @@ func TestHelmRepositoryReconciler_reconcileStorage(t *testing.T) {
 				if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader("file"), 0o640); err != nil {
 					return err
 				}
+				conditions.MarkTrue(obj, meta.ReadyCondition, "foo", "bar")
 				return nil
 			},
 			want: sreconcile.ResultSuccess,
@@ -222,6 +236,9 @@ func TestHelmRepositoryReconciler_reconcileStorage(t *testing.T) {
 				URL:      testStorage.Hostname + "/reconcile-storage/hostname.txt",
 				Size:     int64p(int64(len("file"))),
 			},
+			assertConditions: []metav1.Condition{
+				*conditions.TrueCondition(meta.ReadyCondition, "foo", "bar"),
+			},
 		},
 	}
 	for _, tt := range tests {
@@ -229,23 +246,32 @@ func TestHelmRepositoryReconciler_reconcileStorage(t *testing.T) {
 			g := NewWithT(t)
 
 			r := &HelmRepositoryReconciler{
+				Client:        fakeclient.NewClientBuilder().WithScheme(testEnv.GetScheme()).Build(),
 				EventRecorder: record.NewFakeRecorder(32),
 				Storage:       testStorage,
+				patchOptions:  getPatchOptions(helmRepositoryReadyCondition.Owned, "sc"),
 			}
 
 			obj := &sourcev1.HelmRepository{
 				ObjectMeta: metav1.ObjectMeta{
 					GenerateName: "test-",
+					Generation:   1,
 				},
 			}
 			if tt.beforeFunc != nil {
 				g.Expect(tt.beforeFunc(obj, testStorage)).To(Succeed())
 			}
 
+			g.Expect(r.Client.Create(context.TODO(), obj)).ToNot(HaveOccurred())
+			defer func() {
+				g.Expect(r.Client.Delete(context.TODO(), obj)).ToNot(HaveOccurred())
+			}()
+
 			var chartRepo repository.ChartRepository
 			var artifact sourcev1.Artifact
+			sp := patch.NewSerialPatcher(obj, r.Client)
 
-			got, err := r.reconcileStorage(context.TODO(), obj, &artifact, &chartRepo)
+			got, err := r.reconcileStorage(context.TODO(), sp, obj, &artifact, &chartRepo)
 			g.Expect(err != nil).To(Equal(tt.wantErr))
 			g.Expect(got).To(Equal(tt.want))
 
@@ -263,6 +289,10 @@ func TestHelmRepositoryReconciler_reconcileStorage(t *testing.T) {
 				}
 				g.Expect(absoluteP).NotTo(BeAnExistingFile())
 			}
+
+			// In-progress status condition validity.
+			checker := conditionscheck.NewInProgressChecker(r.Client)
+			checker.CheckErr(ctx, obj)
 		})
 	}
 }
@@ -302,8 +332,8 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 				},
 			},
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new index revision"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new index revision"),
-				*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new index revision"),
+				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new index revision"),
 			},
 		},
 		{
@@ -311,8 +341,8 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 			protocol: "http",
 			want:     sreconcile.ResultSuccess,
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new index revision"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new index revision"),
-				*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new index revision"),
+				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new index revision"),
 			},
 			afterFunc: func(t *WithT, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, chartRepo repository.ChartRepository) {
 				t.Expect(chartRepo.Checksum).ToNot(BeEmpty())
@@ -342,8 +372,8 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 			},
 			want: sreconcile.ResultSuccess,
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new index revision"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new index revision"),
-				*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new index revision"),
+				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new index revision"),
 			},
 			afterFunc: func(t *WithT, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, chartRepo repository.ChartRepository) {
 				t.Expect(chartRepo.Checksum).ToNot(BeEmpty())
@@ -373,8 +403,8 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 			},
 			want: sreconcile.ResultSuccess,
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new index revision"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new index revision"),
-				*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new index revision"),
+				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new index revision"),
 			},
 			afterFunc: func(t *WithT, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, chartRepo repository.ChartRepository) {
 				t.Expect(chartRepo.Checksum).ToNot(BeEmpty())
@@ -401,10 +431,14 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 			},
 			beforeFunc: func(t *WithT, obj *sourcev1.HelmRepository, checksum string) {
 				obj.Spec.SecretRef = &meta.LocalObjectReference{Name: "invalid-ca"}
+				conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
+				conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
 			},
 			wantErr: true,
 			assertConditions: []metav1.Condition{
 				*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to create TLS client config with secret data: cannot append certificate into certificate pool: invalid caFile"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
+				*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
 			},
 			afterFunc: func(t *WithT, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, chartRepo repository.ChartRepository) {
 				// No repo index due to fetch fail.
@@ -419,11 +453,15 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 			protocol: "http",
 			beforeFunc: func(t *WithT, obj *sourcev1.HelmRepository, checksum string) {
 				obj.Spec.URL = strings.ReplaceAll(obj.Spec.URL, "http://", "")
+				conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
+				conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
 			},
 			want:    sreconcile.ResultEmpty,
 			wantErr: true,
 			assertConditions: []metav1.Condition{
 				*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.URLInvalidReason, "first path segment in URL cannot contain colon"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
+				*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
 			},
 			afterFunc: func(t *WithT, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, chartRepo repository.ChartRepository) {
 				// No repo index due to fetch fail.
@@ -438,11 +476,15 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 			protocol: "http",
 			beforeFunc: func(t *WithT, obj *sourcev1.HelmRepository, checksum string) {
 				obj.Spec.URL = strings.ReplaceAll(obj.Spec.URL, "http://", "ftp://")
+				conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
+				conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
 			},
 			want:    sreconcile.ResultEmpty,
 			wantErr: true,
 			assertConditions: []metav1.Condition{
 				*conditions.TrueCondition(sourcev1.FetchFailedCondition, meta.FailedReason, "scheme \"ftp\" not supported"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
+				*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
 			},
 			afterFunc: func(t *WithT, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, chartRepo repository.ChartRepository) {
 				// No repo index due to fetch fail.
@@ -457,10 +499,14 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 			protocol: "http",
 			beforeFunc: func(t *WithT, obj *sourcev1.HelmRepository, checksum string) {
 				obj.Spec.SecretRef = &meta.LocalObjectReference{Name: "non-existing"}
+				conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
+				conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
 			},
 			wantErr: true,
 			assertConditions: []metav1.Condition{
 				*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "secrets \"non-existing\" not found"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
+				*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
 			},
 			afterFunc: func(t *WithT, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, chartRepo repository.ChartRepository) {
 				// No repo index due to fetch fail.
@@ -483,10 +529,14 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 			},
 			beforeFunc: func(t *WithT, obj *sourcev1.HelmRepository, checksum string) {
 				obj.Spec.SecretRef = &meta.LocalObjectReference{Name: "malformed-basic-auth"}
+				conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
+				conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
 			},
 			wantErr: true,
 			assertConditions: []metav1.Condition{
 				*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "required fields 'username' and 'password"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
+				*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
 			},
 			afterFunc: func(t *WithT, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, chartRepo repository.ChartRepository) {
 				// No repo index due to fetch fail.
@@ -504,6 +554,12 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 					Revision: checksum,
 					Checksum: checksum,
 				}
+				conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
+				conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
+			},
+			assertConditions: []metav1.Condition{
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
+				*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
 			},
 			afterFunc: func(t *WithT, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, chartRepo repository.ChartRepository) {
 				// chartRepo.Checksum isn't populated, artifact.Checksum is
@@ -523,6 +579,12 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 					Revision: checksum,
 					Checksum: "foo",
 				}
+				conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
+				conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
+			},
+			assertConditions: []metav1.Condition{
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
+				*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
 			},
 			afterFunc: func(t *WithT, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, chartRepo repository.ChartRepository) {
 				t.Expect(chartRepo.Checksum).ToNot(BeEmpty())
@@ -532,12 +594,29 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 			},
 			want: sreconcile.ResultSuccess,
 		},
+		{
+			name:     "Existing artifact makes ArtifactOutdated=True",
+			protocol: "http",
+			beforeFunc: func(t *WithT, obj *sourcev1.HelmRepository, checksum string) {
+				obj.Status.Artifact = &sourcev1.Artifact{
+					Path:     "some-path",
+					Revision: "some-rev",
+				}
+			},
+			want: sreconcile.ResultSuccess,
+			assertConditions: []metav1.Condition{
+				*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new index revision"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new index revision"),
+				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new index revision"),
+			},
+		},
 	}
 
 	for _, tt := range tests {
 		obj := &sourcev1.HelmRepository{
 			ObjectMeta: metav1.ObjectMeta{
 				GenerateName: "auth-strategy-",
+				Generation:   1,
 			},
 			Spec: sourcev1.HelmRepositorySpec{
 				Interval: metav1.Duration{Duration: interval},
@@ -642,11 +721,19 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 				Client:        builder.Build(),
 				Storage:       testStorage,
 				Getters:       testGetters,
+				patchOptions:  getPatchOptions(helmRepositoryReadyCondition.Owned, "sc"),
 			}
 
+			g.Expect(r.Client.Create(context.TODO(), obj)).ToNot(HaveOccurred())
+			defer func() {
+				g.Expect(r.Client.Delete(context.TODO(), obj)).ToNot(HaveOccurred())
+			}()
+
 			var chartRepo repository.ChartRepository
 			var artifact sourcev1.Artifact
-			got, err := r.reconcileSource(context.TODO(), obj, &artifact, &chartRepo)
+			sp := patch.NewSerialPatcher(obj, r.Client)
+
+			got, err := r.reconcileSource(context.TODO(), sp, obj, &artifact, &chartRepo)
 			defer os.Remove(chartRepo.CachePath)
 
 			g.Expect(obj.Status.Conditions).To(conditions.MatchConditions(tt.assertConditions))
@@ -656,6 +743,10 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 			if tt.afterFunc != nil {
 				tt.afterFunc(g, obj, artifact, chartRepo)
 			}
+
+			// In-progress status condition validity.
+			checker := conditionscheck.NewInProgressChecker(r.Client)
+			checker.CheckErr(ctx, obj)
 		})
 	}
 }
@@ -676,7 +767,7 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) {
 			},
 			want: sreconcile.ResultSuccess,
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'existing'"),
+				*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact: revision 'existing'"),
 			},
 		},
 		{
@@ -690,7 +781,7 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) {
 			},
 			want: sreconcile.ResultSuccess,
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'existing'"),
+				*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact: revision 'existing'"),
 			},
 		},
 		{
@@ -701,7 +792,7 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) {
 			},
 			want: sreconcile.ResultSuccess,
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'existing'"),
+				*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact: revision 'existing'"),
 			},
 		},
 		{
@@ -718,7 +809,7 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) {
 			},
 			want: sreconcile.ResultSuccess,
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'existing'"),
+				*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact: revision 'existing'"),
 			},
 		},
 	}
@@ -728,8 +819,10 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) {
 			g := NewWithT(t)
 
 			r := &HelmRepositoryReconciler{
+				Client:        fakeclient.NewClientBuilder().WithScheme(testEnv.GetScheme()).Build(),
 				EventRecorder: record.NewFakeRecorder(32),
 				Storage:       testStorage,
+				patchOptions:  getPatchOptions(helmRepositoryReadyCondition.Owned, "sc"),
 			}
 
 			obj := &sourcev1.HelmRepository{
@@ -766,8 +859,9 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) {
 			if tt.beforeFunc != nil {
 				tt.beforeFunc(g, obj, artifact, chartRepo)
 			}
+			sp := patch.NewSerialPatcher(obj, r.Client)
 
-			got, err := r.reconcileArtifact(context.TODO(), obj, &artifact, chartRepo)
+			got, err := r.reconcileArtifact(context.TODO(), sp, obj, &artifact, chartRepo)
 			g.Expect(err != nil).To(Equal(tt.wantErr))
 			g.Expect(got).To(Equal(tt.want))
 
@@ -788,7 +882,7 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) {
 func TestHelmRepositoryReconciler_reconcileSubRecs(t *testing.T) {
 	// Helper to build simple helmRepositoryReconcileFunc with result and error.
 	buildReconcileFuncs := func(r sreconcile.Result, e error) helmRepositoryReconcileFunc {
-		return func(ctx context.Context, obj *sourcev1.HelmRepository, artifact *sourcev1.Artifact, repo *repository.ChartRepository) (sreconcile.Result, error) {
+		return func(ctx context.Context, sp *patch.SerialPatcher, obj *sourcev1.HelmRepository, artifact *sourcev1.Artifact, repo *repository.ChartRepository) (sreconcile.Result, error) {
 			return r, e
 		}
 	}
@@ -809,6 +903,10 @@ func TestHelmRepositoryReconciler_reconcileSubRecs(t *testing.T) {
 			},
 			wantResult: sreconcile.ResultSuccess,
 			wantErr:    false,
+			assertConditions: []metav1.Condition{
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "reconciliation in progress"),
+				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "reconciliation in progress"),
+			},
 		},
 		{
 			name:               "successful reconciliation with generation difference",
@@ -820,7 +918,8 @@ func TestHelmRepositoryReconciler_reconcileSubRecs(t *testing.T) {
 			wantResult: sreconcile.ResultSuccess,
 			wantErr:    false,
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(meta.ReconcilingCondition, "NewGeneration", "reconciling new object generation (3)"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "processing object: new generation 2 -> 3"),
+				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "processing object: new generation 2 -> 3"),
 			},
 		},
 		{
@@ -830,16 +929,20 @@ func TestHelmRepositoryReconciler_reconcileSubRecs(t *testing.T) {
 			},
 			wantResult: sreconcile.ResultEmpty,
 			wantErr:    true,
+			assertConditions: []metav1.Condition{
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "reconciliation in progress"),
+				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "reconciliation in progress"),
+			},
 		},
 		{
 			name: "multiple object status conditions mutations",
 			reconcileFuncs: []helmRepositoryReconcileFunc{
-				func(ctx context.Context, obj *sourcev1.HelmRepository, artifact *sourcev1.Artifact, repo *repository.ChartRepository) (sreconcile.Result, error) {
+				func(ctx context.Context, sp *patch.SerialPatcher, obj *sourcev1.HelmRepository, artifact *sourcev1.Artifact, repo *repository.ChartRepository) (sreconcile.Result, error) {
 					conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", "new index revision")
 					return sreconcile.ResultSuccess, nil
 				},
-				func(ctx context.Context, obj *sourcev1.HelmRepository, artifact *sourcev1.Artifact, repo *repository.ChartRepository) (sreconcile.Result, error) {
+				func(ctx context.Context, sp *patch.SerialPatcher, obj *sourcev1.HelmRepository, artifact *sourcev1.Artifact, repo *repository.ChartRepository) (sreconcile.Result, error) {
-					conditions.MarkTrue(obj, meta.ReconcilingCondition, "Progressing", "creating artifact")
+					conditions.MarkTrue(obj, meta.ReconcilingCondition, meta.ProgressingReason, "creating artifact")
 					return sreconcile.ResultSuccess, nil
 				},
 			},
@@ -847,7 +950,8 @@ func TestHelmRepositoryReconciler_reconcileSubRecs(t *testing.T) {
 			wantErr:    false,
 			assertConditions: []metav1.Condition{
 				*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new index revision"),
-				*conditions.TrueCondition(meta.ReconcilingCondition, "Progressing", "creating artifact"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "creating artifact"),
+				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "reconciliation in progress"),
 			},
 		},
 		{
@@ -859,6 +963,10 @@ func TestHelmRepositoryReconciler_reconcileSubRecs(t *testing.T) {
 			},
 			wantResult: sreconcile.ResultRequeue,
 			wantErr:    false,
+			assertConditions: []metav1.Condition{
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "reconciliation in progress"),
+				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "reconciliation in progress"),
+			},
 		},
 		{
 			name: "subrecs with error before result=Requeue",
@@ -869,6 +977,10 @@ func TestHelmRepositoryReconciler_reconcileSubRecs(t *testing.T) {
 			},
 			wantResult: sreconcile.ResultEmpty,
 			wantErr:    true,
+			assertConditions: []metav1.Condition{
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "reconciliation in progress"),
+				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "reconciliation in progress"),
+			},
 		},
 	}
 
@@ -876,7 +988,10 @@ func TestHelmRepositoryReconciler_reconcileSubRecs(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			g := NewWithT(t)
 
-			r := &HelmRepositoryReconciler{}
+			r := &HelmRepositoryReconciler{
+				Client:       fakeclient.NewClientBuilder().WithScheme(testEnv.GetScheme()).Build(),
+				patchOptions: getPatchOptions(helmRepositoryReadyCondition.Owned, "sc"),
+			}
 			obj := &sourcev1.HelmRepository{
 				ObjectMeta: metav1.ObjectMeta{
 					GenerateName: "test-",
@@ -887,9 +1002,15 @@ func TestHelmRepositoryReconciler_reconcileSubRecs(t *testing.T) {
 				},
 			}
 
-			ctx := context.TODO()
+			g.Expect(r.Client.Create(context.TODO(), obj)).ToNot(HaveOccurred())
+			defer func() {
+				g.Expect(r.Client.Delete(context.TODO(), obj)).ToNot(HaveOccurred())
+			}()
 
-			gotRes, gotErr := r.reconcile(ctx, obj, tt.reconcileFuncs)
+			ctx := context.TODO()
+			sp := patch.NewSerialPatcher(obj, r.Client)
+
+			gotRes, gotErr := r.reconcile(ctx, sp, obj, tt.reconcileFuncs)
 			g.Expect(gotErr != nil).To(Equal(tt.wantErr))
 			g.Expect(gotRes).To(Equal(tt.wantResult))
 
@@ -960,8 +1081,7 @@ func TestHelmRepositoryReconciler_statusConditions(t *testing.T) {
 			clientBuilder.WithObjects(obj)
 			c := clientBuilder.Build()
 
-			patchHelper, err := patch.NewHelper(obj, c)
+			serialPatcher := patch.NewSerialPatcher(obj, c)
-			g.Expect(err).ToNot(HaveOccurred())
 
 			if tt.beforeFunc != nil {
 				tt.beforeFunc(obj)
@@ -971,7 +1091,7 @@ func TestHelmRepositoryReconciler_statusConditions(t *testing.T) {
 			recResult := sreconcile.ResultSuccess
 			var retErr error
 
-			summarizeHelper := summarize.NewHelper(record.NewFakeRecorder(32), patchHelper)
+			summarizeHelper := summarize.NewHelper(record.NewFakeRecorder(32), serialPatcher)
 			summarizeOpts := []summarize.Option{
 				summarize.WithConditions(helmRepositoryReadyCondition),
 				summarize.WithReconcileResult(recResult),
@@ -1084,6 +1204,7 @@ func TestHelmRepositoryReconciler_notify(t *testing.T) {
 
 			reconciler := &HelmRepositoryReconciler{
 				EventRecorder: recorder,
+				patchOptions:  getPatchOptions(helmRepositoryReadyCondition.Owned, "sc"),
 			}
 			chartRepo := repository.ChartRepository{
 				URL: "some-address",