Merge pull request #595 from fluxcd/redundant-reconciling-condition

controllers: Remove redundant reconciling condition in reconcileArtifact

Author: darkowlzz

controllers/bucket_controller.go

@@ -626,10 +626,6 @@ func (r *BucketReconciler) reconcileArtifact(ctx context.Context,
 		return sreconcile.ResultSuccess, nil
 	}
 
-	// Mark reconciling because the artifact and remote source are different.
-	// and they have to be reconciled.
-	conditions.MarkReconciling(obj, "NewRevision", "new upstream revision '%s'", artifact.Revision)
-
 	// Ensure target path exists and is a directory
 	if f, err := os.Stat(dir); err != nil {
 		return sreconcile.ResultEmpty, &serror.Event{

controllers/bucket_controller_test.go

@@ -870,7 +870,6 @@ func TestBucketReconciler_reconcileArtifact(t *testing.T) {
 			want: sreconcile.ResultSuccess,
 			assertConditions: []metav1.Condition{
 				*conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'existing'"),
-				*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'existing'"),
 			},
 		},
 		{
@@ -896,7 +895,6 @@ func TestBucketReconciler_reconcileArtifact(t *testing.T) {
 			want: sreconcile.ResultSuccess,
 			assertConditions: []metav1.Condition{
 				*conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'existing'"),
-				*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'existing'"),
 			},
 		},
 		{
@@ -914,7 +912,6 @@ func TestBucketReconciler_reconcileArtifact(t *testing.T) {
 			want: sreconcile.ResultSuccess,
 			assertConditions: []metav1.Condition{
 				*conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'existing'"),
-				*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'existing'"),
 			},
 		},
 		{
@@ -924,9 +921,6 @@ func TestBucketReconciler_reconcileArtifact(t *testing.T) {
 			},
 			want:    sreconcile.ResultEmpty,
 			wantErr: true,
-			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'existing'"),
-			},
 		},
 		{
 			name: "Dir path is not a directory",
@@ -943,9 +937,6 @@ func TestBucketReconciler_reconcileArtifact(t *testing.T) {
 			},
 			want:    sreconcile.ResultEmpty,
 			wantErr: true,
-			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'existing'"),
-			},
 		},
 	}
 

controllers/gitrepository_controller.go

@@ -405,10 +405,6 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context,
 		return sreconcile.ResultSuccess, nil
 	}
 
-	// Mark reconciling because the artifact and remote source are different.
-	// and they have to be reconciled.
-	conditions.MarkReconciling(obj, "NewRevision", "new upstream revision '%s'", artifact.Revision)
-
 	// Ensure target path exists and is a directory
 	if f, err := os.Stat(dir); err != nil {
 		e := &serror.Event{
@@ -540,8 +536,9 @@ func (r *GitRepositoryReconciler) reconcileInclude(ctx context.Context,
 
 	// Observe if the artifacts still match the previous included ones
 	if artifacts.Diff(obj.Status.IncludedArtifacts) {
-		conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "IncludeChange",
-			"included artifacts differ from last observed includes")
+		message := fmt.Sprintf("included artifacts differ from last observed includes")
+		conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "IncludeChange", message)
+		conditions.MarkReconciling(obj, "IncludeChange", message)
 	}
 
 	// Persist the artifactSet.

controllers/gitrepository_controller_test.go

@@ -717,7 +717,6 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
 			want: sreconcile.ResultSuccess,
 			assertConditions: []metav1.Condition{
 				*conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'main/revision'"),
-				*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'main/revision'"),
 			},
 		},
 		{
@@ -736,7 +735,6 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
 			want: sreconcile.ResultSuccess,
 			assertConditions: []metav1.Condition{
 				*conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'main/revision'"),
-				*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'main/revision'"),
 			},
 		},
 		{
@@ -770,7 +768,6 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
 			want: sreconcile.ResultSuccess,
 			assertConditions: []metav1.Condition{
 				*conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'main/revision'"),
-				*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'main/revision'"),
 			},
 		},
 		{
@@ -788,7 +785,6 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
 			want: sreconcile.ResultSuccess,
 			assertConditions: []metav1.Condition{
 				*conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'main/revision'"),
-				*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'main/revision'"),
 			},
 		},
 		{
@@ -809,24 +805,17 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
 			want: sreconcile.ResultSuccess,
 			assertConditions: []metav1.Condition{
 				*conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'main/revision'"),
-				*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'main/revision'"),
 			},
 		},
 		{
 			name:    "Target path does not exists",
 			dir:     "testdata/git/foo",
 			wantErr: true,
-			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'main/revision'"),
-			},
 		},
 		{
 			name:    "Target path is not a directory",
 			dir:     "testdata/git/repository/foo.txt",
 			wantErr: true,
-			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'main/revision'"),
-			},
 		},
 	}
 
@@ -926,6 +915,7 @@ func TestGitRepositoryReconciler_reconcileInclude(t *testing.T) {
 			want: sreconcile.ResultSuccess,
 			assertConditions: []metav1.Condition{
 				*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "IncludeChange", "included artifacts differ from last observed includes"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, "IncludeChange", "included artifacts differ from last observed includes"),
 			},
 		},
 		{

controllers/helmrepository_controller.go

@@ -396,10 +396,6 @@ func (r *HelmRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *s
 		return sreconcile.ResultSuccess, nil
 	}
 
-	// Mark reconciling because the artifact and remote source are different.
-	// and they have to be reconciled.
-	conditions.MarkReconciling(obj, "NewRevision", "new index revision '%s'", artifact.Revision)
-
 	// Create artifact dir
 	if err := r.Storage.MkdirAll(*artifact); err != nil {
 		return sreconcile.ResultEmpty, &serror.Event{

controllers/helmrepository_controller_test.go

@@ -520,7 +520,6 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) {
 			want: sreconcile.ResultSuccess,
 			assertConditions: []metav1.Condition{
 				*conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'existing'"),
-				*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new index revision 'existing'"),
 			},
 		},
 		{
@@ -546,7 +545,6 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) {
 			want: sreconcile.ResultSuccess,
 			assertConditions: []metav1.Condition{
 				*conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'existing'"),
-				*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new index revision 'existing'"),
 			},
 		},
 		{
@@ -564,7 +562,6 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) {
 			want: sreconcile.ResultSuccess,
 			assertConditions: []metav1.Condition{
 				*conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'existing'"),
-				*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new index revision 'existing'"),
 			},
 		},
 	}