Add progressive status in gitrepository reconciler

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

controllers/gitrepository_controller.go

@@ -50,6 +50,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"
 
 	"github.com/fluxcd/pkg/sourceignore"
 	sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
@@ -102,6 +103,15 @@ var gitRepositoryFailConditions = []string{
 	sourcev1.StorageOperationFailedCondition,
 }
 
+// getPatchOptions composes patch options based on the given parameters.
+// It is used as the options used when patching an object.
+func getPatchOptions(ownedConditions []string, controllerName string) []patch.Option {
+	return []patch.Option{
+		patch.WithOwnedConditions{Conditions: ownedConditions},
+		patch.WithFieldOwner(controllerName),
+	}
+}
+
 // +kubebuilder:rbac:groups=source.toolkit.fluxcd.io,resources=gitrepositories,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=source.toolkit.fluxcd.io,resources=gitrepositories/status,verbs=get;update;patch
 // +kubebuilder:rbac:groups=source.toolkit.fluxcd.io,resources=gitrepositories/finalizers,verbs=get;create;update;patch;delete
@@ -118,6 +128,8 @@ type GitRepositoryReconciler struct {
 
 	requeueDependency time.Duration
 	features          map[string]bool
+
+	patchOptions []patch.Option
 }
 
 type GitRepositoryReconcilerOptions struct {
@@ -128,13 +140,15 @@ type GitRepositoryReconcilerOptions struct {
 
 // gitRepositoryReconcileFunc is the function type for all the
 // v1beta2.GitRepository (sub)reconcile functions.
-type gitRepositoryReconcileFunc func(ctx context.Context, obj *sourcev1.GitRepository, commit *git.Commit, includes *artifactSet, dir string) (sreconcile.Result, error)
+type gitRepositoryReconcileFunc func(ctx context.Context, sp *patch.SerialPatcher, obj *sourcev1.GitRepository, commit *git.Commit, includes *artifactSet, dir string) (sreconcile.Result, error)
 
 func (r *GitRepositoryReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return r.SetupWithManagerAndOptions(mgr, GitRepositoryReconcilerOptions{})
 }
 
 func (r *GitRepositoryReconciler) SetupWithManagerAndOptions(mgr ctrl.Manager, opts GitRepositoryReconcilerOptions) error {
+	r.patchOptions = getPatchOptions(gitRepositoryReadyCondition.Owned, r.ControllerName)
+
 	r.requeueDependency = opts.DependencyRequeueInterval
 
 	if r.features == nil {
@@ -167,10 +181,7 @@ func (r *GitRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	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
@@ -178,7 +189,7 @@ func (r *GitRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	// Always attempt to patch the object and status 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(gitRepositoryReadyCondition),
 			summarize.WithBiPolarityConditionTypes(sourcev1.SourceVerifiedCondition),
@@ -227,19 +238,36 @@ func (r *GitRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		r.reconcileInclude,
 		r.reconcileArtifact,
 	}
-	recResult, retErr = r.reconcile(ctx, obj, reconcilers)
+	recResult, retErr = r.reconcile(ctx, serialPatcher, obj, reconcilers)
 	return
 }
 
 // reconcile iterates through the gitRepositoryReconcileFunc tasks for the
 // object. It returns early on the first call that returns
 // reconcile.ResultRequeue, or produces an error.
-func (r *GitRepositoryReconciler) reconcile(ctx context.Context, obj *sourcev1.GitRepository, reconcilers []gitRepositoryReconcileFunc) (sreconcile.Result, error) {
+func (r *GitRepositoryReconciler) reconcile(ctx context.Context, sp *patch.SerialPatcher,
+	obj *sourcev1.GitRepository, reconcilers []gitRepositoryReconcileFunc) (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 recAtVal string
+	if v, ok := meta.ReconcileAnnotationValue(obj.GetAnnotations()); ok {
+		recAtVal = 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 recAtVal != obj.Status.GetLastHandledReconcileRequest():
+		if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
+			return sreconcile.ResultEmpty, err
+		}
 	}
 
 	// Create temp dir for Git clone
@@ -268,7 +296,7 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, obj *sourcev1.G
 		resErr error
 	)
 	for _, rec := range reconcilers {
-		recResult, err := rec(ctx, obj, &commit, &includes, tmpDir)
+		recResult, err := rec(ctx, sp, obj, &commit, &includes, tmpDir)
 		// Exit immediately on ResultRequeue.
 		if recResult == sreconcile.ResultRequeue {
 			return sreconcile.ResultRequeue, nil
@@ -359,23 +387,32 @@ func (r *GitRepositoryReconciler) shouldNotify(oldObj, newObj *sourcev1.GitRepos
 // 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 *GitRepositoryReconciler) reconcileStorage(ctx context.Context,
+func (r *GitRepositoryReconciler) reconcileStorage(ctx context.Context, sp *patch.SerialPatcher,
 	obj *sourcev1.GitRepository, _ *git.Commit, _ *artifactSet, _ string) (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
 	}
 
@@ -417,7 +454,7 @@ func (r *GitRepositoryReconciler) reconcileStorage(ctx context.Context,
 // and the local artifact are on the same revision, and no other source content
 // related configurations have changed since last reconciliation. If there's a
 // change, it short-circuits the whole reconciliation with an early return.
-func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
+func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch.SerialPatcher,
 	obj *sourcev1.GitRepository, commit *git.Commit, includes *artifactSet, dir string) (sreconcile.Result, error) {
 	// Remove previously failed source verification status conditions. The
 	// failing verification should be recalculated. But an existing successful
@@ -477,9 +514,15 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
 	// Observe if the artifacts still match the previous included ones
 	if artifacts.Diff(obj.Status.IncludedArtifacts) {
 		message := fmt.Sprintf("included artifacts differ from last observed includes")
+		if obj.Status.IncludedArtifacts != nil {
 			conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "IncludeChange", message)
-		conditions.MarkReconciling(obj, "IncludeChange", message)
 		}
+		rreconcile.ProgressiveStatus(true, obj, meta.ProgressingReason, "building artifact: %s", message)
+		if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
+			return sreconcile.ResultEmpty, err
+		}
+	}
+	conditions.Delete(obj, sourcev1.ArtifactOutdatedCondition)
 
 	// Persist the ArtifactSet.
 	*includes = *artifacts
@@ -540,8 +583,13 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
 	// Mark observations about the revision on the object
 	if !obj.GetArtifact().HasRevision(commit.String()) {
 		message := fmt.Sprintf("new upstream revision '%s'", commit.String())
+		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 {
+			return sreconcile.ResultEmpty, err
+		}
 	}
 	return sreconcile.ResultSuccess, nil
 }
@@ -558,7 +606,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
 // On a successful archive, the Artifact, Includes, observed ignore, recurse
 // submodules and observed include in the Status of the object are set, and the
 // symlink in the Storage is updated to its path.
-func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context,
+func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context, sp *patch.SerialPatcher,
 	obj *sourcev1.GitRepository, commit *git.Commit, includes *artifactSet, dir string) (sreconcile.Result, error) {
 
 	// Create potential new artifact with current available metadata
@@ -672,7 +720,7 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context,
 // v1beta2.IncludeUnavailableCondition from the object.
 // When the composed artifactSet differs from the current set in the Status of
 // the object, it marks the object with v1beta2.ArtifactOutdatedCondition=True.
-func (r *GitRepositoryReconciler) reconcileInclude(ctx context.Context,
+func (r *GitRepositoryReconciler) reconcileInclude(ctx context.Context, sp *patch.SerialPatcher,
 	obj *sourcev1.GitRepository, _ *git.Commit, includes *artifactSet, dir string) (sreconcile.Result, error) {
 
 	for i, incl := range obj.Spec.Include {

controllers/gitrepository_controller_test.go

@@ -38,7 +38,6 @@ import (
 	sshtestdata "golang.org/x/crypto/ssh/testdata"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/utils/pointer"
 	kstatus "sigs.k8s.io/cli-utils/pkg/kstatus/status"
@@ -241,16 +240,16 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
 		assertConditions []metav1.Condition
 	}{
 		{
-			name:     "HTTP without secretRef makes ArtifactOutdated=True",
+			name:     "HTTP without secretRef makes Reconciling=True",
 			protocol: "http",
 			want:     sreconcile.ResultSuccess,
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new upstream revision 'master/<commit>'"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new upstream revision 'master/<commit>'"),
-				*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'master/<commit>'"),
+				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new upstream revision 'master/<commit>'"),
 			},
 		},
 		{
-			name:     "HTTP with Basic Auth secret makes ArtifactOutdated=True",
+			name:     "HTTP with Basic Auth secret makes Reconciling=True",
 			protocol: "http",
 			server: options{
 				username: "git",
@@ -270,12 +269,12 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
 			},
 			want: sreconcile.ResultSuccess,
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new upstream revision 'master/<commit>'"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new upstream revision 'master/<commit>'"),
-				*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'master/<commit>'"),
+				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new upstream revision 'master/<commit>'"),
 			},
 		},
 		{
-			name:     "HTTPS with CAFile secret makes ArtifactOutdated=True",
+			name:     "HTTPS with CAFile secret makes Reconciling=True",
 			protocol: "https",
 			server: options{
 				publicKey:  tlsPublicKey,
@@ -295,8 +294,8 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
 			},
 			want: sreconcile.ResultSuccess,
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new upstream revision 'master/<commit>'"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new upstream revision 'master/<commit>'"),
-				*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'master/<commit>'"),
+				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new upstream revision 'master/<commit>'"),
 			},
 		},
 		{
@@ -317,6 +316,8 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
 			},
 			beforeFunc: func(obj *sourcev1.GitRepository) {
 				obj.Spec.SecretRef = &meta.LocalObjectReference{Name: "invalid-ca"}
+				conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
+				conditions.MarkUnknown(obj, meta.ReadyCondition, meta.ProgressingReason, "foo")
 			},
 			wantErr: true,
 			assertConditions: []metav1.Condition{
@@ -326,10 +327,12 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
 				//
 				// Trimming the expected error message for consistent results.
 				*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.GitOperationFailedReason, "x509: "),
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
+				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "foo"),
 			},
 		},
 		{
-			name:     "SSH with private key secret makes ArtifactOutdated=True",
+			name:     "SSH with private key secret makes Reconciling=True",
 			protocol: "ssh",
 			server: options{
 				username: "git",
@@ -348,12 +351,12 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
 			},
 			want: sreconcile.ResultSuccess,
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new upstream revision 'master/<commit>'"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new upstream revision 'master/<commit>'"),
-				*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'master/<commit>'"),
+				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new upstream revision 'master/<commit>"),
 			},
 		},
 		{
-			name:     "SSH with password protected private key secret makes ArtifactOutdated=True",
+			name:     "SSH with password protected private key secret makes Reconciling=True",
 			protocol: "ssh",
 			server: options{
 				username: "git",
@@ -373,8 +376,8 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
 			},
 			want: sreconcile.ResultSuccess,
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new upstream revision 'master/<commit>'"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new upstream revision 'master/<commit>'"),
-				*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'master/<commit>'"),
+				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new upstream revision 'master/<commit>'"),
 			},
 		},
 		{
@@ -385,10 +388,46 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
 			},
 			beforeFunc: func(obj *sourcev1.GitRepository) {
 				obj.Spec.SecretRef = &meta.LocalObjectReference{Name: "non-existing"}
+				conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
+				conditions.MarkUnknown(obj, meta.ReadyCondition, meta.ProgressingReason, "foo")
 			},
 			wantErr: true,
 			assertConditions: []metav1.Condition{
 				*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get secret '/non-existing': secrets \"non-existing\" not found"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
+				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "foo"),
+			},
+		},
+		{
+			name:     "Existing artifact makes ArtifactOutdated=True",
+			protocol: "http",
+			server: options{
+				username: "git",
+				password: "1234",
+			},
+			secret: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "basic-auth",
+				},
+				Data: map[string][]byte{
+					"username": []byte("git"),
+					"password": []byte("1234"),
+				},
+			},
+			beforeFunc: func(obj *sourcev1.GitRepository) {
+				obj.Spec.SecretRef = &meta.LocalObjectReference{Name: "basic-auth"}
+				obj.Status = sourcev1.GitRepositoryStatus{
+					Artifact: &sourcev1.Artifact{
+						Revision: "staging/some-revision",
+						Path:     randStringRunes(10),
+					},
+				}
+			},
+			want: sreconcile.ResultSuccess,
+			assertConditions: []metav1.Condition{
+				*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new upstream revision 'master/<commit>'"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new upstream revision 'master/<commit>'"),
+				*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new upstream revision 'master/<commit>'"),
 			},
 		},
 	}
@@ -400,6 +439,7 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
 			obj := &sourcev1.GitRepository{
 				ObjectMeta: metav1.ObjectMeta{
 					GenerateName: "auth-strategy-",
+					Generation:   1,
 				},
 				Spec: sourcev1.GitRepositorySpec{
 					Interval: metav1.Duration{Duration: interval},
@@ -468,6 +508,7 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
 				features: map[string]bool{
 					features.OptimizedGitClones: true,
 				},
+				patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"),
 			}
 
 			tmpDir := t.TempDir()
@@ -479,14 +520,24 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
 				assertConditions[k].Message = strings.ReplaceAll(assertConditions[k].Message, "<url>", obj.Spec.URL)
 			}
 
+			g.Expect(r.Client.Create(context.TODO(), obj)).ToNot(HaveOccurred())
+			defer func() {
+				g.Expect(r.Client.Delete(context.TODO(), obj)).ToNot(HaveOccurred())
+			}()
+
 			var commit git.Commit
 			var includes artifactSet
+			sp := patch.NewSerialPatcher(obj, r.Client)
 
-			got, err := r.reconcileSource(context.TODO(), obj, &commit, &includes, tmpDir)
+			got, err := r.reconcileSource(context.TODO(), sp, obj, &commit, &includes, tmpDir)
 			g.Expect(obj.Status.Conditions).To(conditions.MatchConditions(tt.assertConditions))
 			g.Expect(err != nil).To(Equal(tt.wantErr))
 			g.Expect(got).To(Equal(tt.want))
 			g.Expect(commit).ToNot(BeNil())
+
+			// In-progress status condition validity.
+			checker := conditionscheck.NewInProgressChecker(r.Client)
+			checker.CheckErr(ctx, obj)
 		})
 	}
 }
@@ -505,12 +556,13 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
 		wantErr              bool
 		wantRevision         string
 		wantArtifactOutdated bool
+		wantReconciling      bool
 	}{
 		{
 			name:            "Nil reference (default branch)",
 			want:            sreconcile.ResultSuccess,
 			wantRevision:    "master/<commit>",
-			wantArtifactOutdated: true,
+			wantReconciling: true,
 		},
 		{
 			name: "Branch",
@@ -519,7 +571,7 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
 			},
 			want:            sreconcile.ResultSuccess,
 			wantRevision:    "staging/<commit>",
-			wantArtifactOutdated: true,
+			wantReconciling: true,
 		},
 		{
 			name: "Tag",
@@ -528,7 +580,7 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
 			},
 			want:            sreconcile.ResultSuccess,
 			wantRevision:    "v0.1.0/<commit>",
-			wantArtifactOutdated: true,
+			wantReconciling: true,
 		},
 		{
 			name: "Branch commit",
@@ -538,7 +590,7 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
 			},
 			want:            sreconcile.ResultSuccess,
 			wantRevision:    "staging/<commit>",
-			wantArtifactOutdated: true,
+			wantReconciling: true,
 		},
 		{
 			name: "SemVer",
@@ -547,7 +599,7 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
 			},
 			want:            sreconcile.ResultSuccess,
 			wantRevision:    "v2.0.0/<commit>",
-			wantArtifactOutdated: true,
+			wantReconciling: true,
 		},
 		{
 			name: "SemVer range",
@@ -556,7 +608,7 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
 			},
 			want:            sreconcile.ResultSuccess,
 			wantRevision:    "0.2.0/<commit>",
-			wantArtifactOutdated: true,
+			wantReconciling: true,
 		},
 		{
 			name: "SemVer prerelease",
@@ -565,7 +617,27 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
 			},
 			wantRevision:    "v1.0.0-alpha/<commit>",
 			want:            sreconcile.ResultSuccess,
+			wantReconciling: true,
+		},
+		{
+			name: "Existing artifact makes ArtifactOutdated=True",
+			reference: &sourcev1.GitRepositoryRef{
+				Branch: "staging",
+			},
+			beforeFunc: func(obj *sourcev1.GitRepository, latestRev string) {
+				obj.Status = sourcev1.GitRepositoryStatus{
+					Artifact: &sourcev1.Artifact{
+						Revision: "staging/some-revision",
+						Path:     randStringRunes(10),
+					},
+				}
+				conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "foo")
+				conditions.MarkTrue(obj, meta.ReadyCondition, meta.SucceededReason, "foo")
+			},
+			want:                 sreconcile.ResultSuccess,
+			wantRevision:         "staging/<commit>",
 			wantArtifactOutdated: true,
+			wantReconciling:      true,
 		},
 		{
 			name: "Optimized clone",
@@ -581,11 +653,12 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
 					},
 				}
 				conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "foo")
+				conditions.MarkTrue(obj, meta.ReadyCondition, meta.SucceededReason, "foo")
 			},
 			want:            sreconcile.ResultEmpty,
 			wantErr:         true,
 			wantRevision:    "staging/<commit>",
-			wantArtifactOutdated: false,
+			wantReconciling: false,
 		},
 		{
 			name: "Optimized clone different ignore",
@@ -603,10 +676,11 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
 					},
 				}
 				conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "foo")
+				conditions.MarkTrue(obj, meta.ReadyCondition, meta.SucceededReason, "foo")
 			},
 			want:            sreconcile.ResultSuccess,
 			wantRevision:    "staging/<commit>",
-			wantArtifactOutdated: false,
+			wantReconciling: false,
 		},
 	}
 
@@ -632,12 +706,13 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
 	}
 
 	r := &GitRepositoryReconciler{
-		Client:        fakeclient.NewClientBuilder().WithScheme(runtime.NewScheme()).Build(),
+		Client:        fakeclient.NewClientBuilder().WithScheme(testEnv.GetScheme()).Build(),
 		EventRecorder: record.NewFakeRecorder(32),
 		Storage:       testStorage,
 		features: map[string]bool{
 			features.OptimizedGitClones: true,
 		},
+		patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"),
 	}
 
 	for _, tt := range tests {
@@ -645,6 +720,7 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
 			obj := &sourcev1.GitRepository{
 				ObjectMeta: metav1.ObjectMeta{
 					GenerateName: "checkout-strategy-",
+					Generation:   1,
 				},
 				Spec: sourcev1.GitRepositorySpec{
 					Interval:  metav1.Duration{Duration: interval},
@@ -664,9 +740,15 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
 				tt.beforeFunc(obj, headRef.Hash().String())
 			}
 
+			g.Expect(r.Client.Create(context.TODO(), obj)).ToNot(HaveOccurred())
+			defer func() {
+				g.Expect(r.Client.Delete(context.TODO(), obj)).ToNot(HaveOccurred())
+			}()
+
 			var commit git.Commit
 			var includes artifactSet
-			got, err := r.reconcileSource(ctx, obj, &commit, &includes, tmpDir)
+			sp := patch.NewSerialPatcher(obj, r.Client)
+			got, err := r.reconcileSource(ctx, sp, obj, &commit, &includes, tmpDir)
 			if err != nil {
 				println(err.Error())
 			}
@@ -676,7 +758,11 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
 				revision := strings.ReplaceAll(tt.wantRevision, "<commit>", headRef.Hash().String())
 				g.Expect(commit.String()).To(Equal(revision))
 				g.Expect(conditions.IsTrue(obj, sourcev1.ArtifactOutdatedCondition)).To(Equal(tt.wantArtifactOutdated))
+				g.Expect(conditions.IsTrue(obj, meta.ReconcilingCondition)).To(Equal(tt.wantReconciling))
 			}
+			// In-progress status condition validity.
+			checker := conditionscheck.NewInProgressChecker(r.Client)
+			checker.CheckErr(ctx, obj)
 		})
 	}
 }
@@ -855,6 +941,7 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
 				EventRecorder: record.NewFakeRecorder(32),
 				Storage:       testStorage,
 				features:      features.FeatureGates(),
+				patchOptions:  getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"),
 			}
 
 			obj := &sourcev1.GitRepository{
@@ -873,8 +960,9 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
 				Hash:      []byte("revision"),
 				Reference: "refs/heads/main",
 			}
+			sp := patch.NewSerialPatcher(obj, r.Client)
 
-			got, err := r.reconcileArtifact(ctx, obj, &commit, &tt.includes, tt.dir)
+			got, err := r.reconcileArtifact(ctx, sp, obj, &commit, &tt.includes, tt.dir)
 			g.Expect(obj.Status.Conditions).To(conditions.MatchConditions(tt.assertConditions))
 			g.Expect(err != nil).To(Equal(tt.wantErr))
 			g.Expect(got).To(Equal(tt.want))
@@ -1000,6 +1088,7 @@ func TestGitRepositoryReconciler_reconcileInclude(t *testing.T) {
 				Storage:           storage,
 				requeueDependency: dependencyInterval,
 				features:          features.FeatureGates(),
+				patchOptions:      getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"),
 			}
 
 			obj := &sourcev1.GitRepository{
@@ -1036,7 +1125,9 @@ func TestGitRepositoryReconciler_reconcileInclude(t *testing.T) {
 			g.Expect(err).ToNot(HaveOccurred())
 			includes = *artifactSet
 
-			got, err := r.reconcileInclude(ctx, obj, &commit, &includes, tmpDir)
+			sp := patch.NewSerialPatcher(obj, r.Client)
+
+			got, err := r.reconcileInclude(ctx, sp, obj, &commit, &includes, tmpDir)
 			g.Expect(obj.GetConditions()).To(conditions.MatchConditions(tt.assertConditions))
 			g.Expect(err != nil).To(Equal(tt.wantErr))
 			if err == nil {
@@ -1093,6 +1184,7 @@ func TestGitRepositoryReconciler_reconcileStorage(t *testing.T) {
 					}
 				}
 				testStorage.SetArtifactURL(obj.Status.Artifact)
+				conditions.MarkTrue(obj, meta.ReadyCondition, "foo", "bar")
 				return nil
 			},
 			assertArtifact: &sourcev1.Artifact{
@@ -1109,6 +1201,17 @@ func TestGitRepositoryReconciler_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",
@@ -1125,7 +1228,8 @@ func TestGitRepositoryReconciler_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"),
 			},
 		},
 		{
@@ -1143,6 +1247,7 @@ func TestGitRepositoryReconciler_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,
@@ -1156,6 +1261,9 @@ func TestGitRepositoryReconciler_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 {
@@ -1167,23 +1275,32 @@ func TestGitRepositoryReconciler_reconcileStorage(t *testing.T) {
 			}()
 
 			r := &GitRepositoryReconciler{
+				Client:        fakeclient.NewClientBuilder().WithScheme(testEnv.GetScheme()).Build(),
 				EventRecorder: record.NewFakeRecorder(32),
 				Storage:       testStorage,
 				features:      features.FeatureGates(),
+				patchOptions:  getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"),
 			}
 
 			obj := &sourcev1.GitRepository{
 				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 c *git.Commit
 			var as artifactSet
-			got, err := r.reconcileStorage(context.TODO(), obj, c, &as, "")
+			sp := patch.NewSerialPatcher(obj, r.Client)
+			got, err := r.reconcileStorage(context.TODO(), sp, obj, c, &as, "")
 			g.Expect(err != nil).To(Equal(tt.wantErr))
 			g.Expect(got).To(Equal(tt.want))
 
@@ -1201,6 +1318,10 @@ func TestGitRepositoryReconciler_reconcileStorage(t *testing.T) {
 				}
 				g.Expect(absoluteP).NotTo(BeAnExistingFile())
 			}
+
+			// In-progress status condition validity.
+			checker := conditionscheck.NewInProgressChecker(r.Client)
+			checker.CheckErr(ctx, obj)
 		})
 	}
 }
@@ -1212,6 +1333,7 @@ func TestGitRepositoryReconciler_reconcileDelete(t *testing.T) {
 		EventRecorder: record.NewFakeRecorder(32),
 		Storage:       testStorage,
 		features:      features.FeatureGates(),
+		patchOptions:  getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"),
 	}
 
 	obj := &sourcev1.GitRepository{
@@ -1350,6 +1472,7 @@ func TestGitRepositoryReconciler_verifyCommitSignature(t *testing.T) {
 				EventRecorder: record.NewFakeRecorder(32),
 				Client:        builder.Build(),
 				features:      features.FeatureGates(),
+				patchOptions:  getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"),
 			}
 
 			obj := &sourcev1.GitRepository{
@@ -1492,6 +1615,7 @@ func TestGitRepositoryReconciler_ConditionsUpdate(t *testing.T) {
 				EventRecorder: record.NewFakeRecorder(32),
 				Storage:       testStorage,
 				features:      features.FeatureGates(),
+				patchOptions:  getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"),
 			}
 
 			key := client.ObjectKeyFromObject(obj)
@@ -1710,8 +1834,7 @@ func TestGitRepositoryReconciler_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)
@@ -1721,9 +1844,10 @@ func TestGitRepositoryReconciler_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(gitRepositoryReadyCondition),
+				summarize.WithBiPolarityConditionTypes(sourcev1.SourceVerifiedCondition),
 				summarize.WithReconcileResult(recResult),
 				summarize.WithReconcileError(retErr),
 				summarize.WithIgnoreNotFound(),
@@ -1857,6 +1981,7 @@ func TestGitRepositoryReconciler_notify(t *testing.T) {
 			reconciler := &GitRepositoryReconciler{
 				EventRecorder: recorder,
 				features:      features.FeatureGates(),
+				patchOptions:  getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"),
 			}
 			reconciler.notify(ctx, oldObj, newObj, tt.commit, tt.res, tt.resErr)
 
@@ -1996,6 +2121,7 @@ func TestGitRepositoryReconciler_fetchIncludes(t *testing.T) {
 			r := &GitRepositoryReconciler{
 				Client:        builder.Build(),
 				EventRecorder: record.NewFakeRecorder(32),
+				patchOptions:  getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"),
 			}
 
 			obj := &sourcev1.GitRepository{