Remove redundant reconciling in reconcileArtifact

reconcileSource() adds reconciling condition with accurate information. Remove setting reconciling condition in reconcileArtifact(). Signed-off-by: Sunny <darkowlzz@protonmail.com>
fluxcd · Feb 25, 2022 · 7a0221b · 7a0221b
1 parent a5f65a2
commit 7a0221b
Show file tree

Hide file tree

Showing 6 changed files with 4 additions and 37 deletions.
diff --git a/controllers/bucket_controller.go b/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{

diff --git a/controllers/bucket_controller_test.go b/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'"),
- },
  },
  }
 

diff --git a/controllers/gitrepository_controller.go b/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.

diff --git a/controllers/gitrepository_controller_test.go b/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"),
  },
  },
  {

diff --git a/controllers/helmrepository_controller.go b/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{

diff --git a/controllers/helmrepository_controller_test.go b/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'"),
  },
  },
  }