Skip to content

Commit 234b7f4

Browse files
darkowlzzdarkowlzz
committed
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>
1 parent e4bfab2 commit 234b7f4

File tree

6 files changed

+4
-37
lines changed

6 files changed

+4
-37
lines changed

‎controllers/bucket_controller.go‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -626,10 +626,6 @@ func (r *BucketReconciler) reconcileArtifact(ctx context.Context,
626626
return sreconcile.ResultSuccess, nil
627627
}
628628

629-
// Mark reconciling because the artifact and remote source are different.
630-
// and they have to be reconciled.
631-
conditions.MarkReconciling(obj, "NewRevision", "new upstream revision '%s'", artifact.Revision)
632-
633629
// Ensure target path exists and is a directory
634630
if f, err := os.Stat(dir); err != nil {
635631
return sreconcile.ResultEmpty, &serror.Event{

‎controllers/bucket_controller_test.go‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -870,7 +870,6 @@ func TestBucketReconciler_reconcileArtifact(t *testing.T) {
870870
want: sreconcile.ResultSuccess,
871871
assertConditions: []metav1.Condition{
872872
*conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'existing'"),
873-
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'existing'"),
874873
},
875874
},
876875
{
@@ -896,7 +895,6 @@ func TestBucketReconciler_reconcileArtifact(t *testing.T) {
896895
want: sreconcile.ResultSuccess,
897896
assertConditions: []metav1.Condition{
898897
*conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'existing'"),
899-
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'existing'"),
900898
},
901899
},
902900
{
@@ -914,7 +912,6 @@ func TestBucketReconciler_reconcileArtifact(t *testing.T) {
914912
want: sreconcile.ResultSuccess,
915913
assertConditions: []metav1.Condition{
916914
*conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'existing'"),
917-
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'existing'"),
918915
},
919916
},
920917
{
@@ -924,9 +921,6 @@ func TestBucketReconciler_reconcileArtifact(t *testing.T) {
924921
},
925922
want: sreconcile.ResultEmpty,
926923
wantErr: true,
927-
assertConditions: []metav1.Condition{
928-
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'existing'"),
929-
},
930924
},
931925
{
932926
name: "Dir path is not a directory",
@@ -943,9 +937,6 @@ func TestBucketReconciler_reconcileArtifact(t *testing.T) {
943937
},
944938
want: sreconcile.ResultEmpty,
945939
wantErr: true,
946-
assertConditions: []metav1.Condition{
947-
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'existing'"),
948-
},
949940
},
950941
}
951942

‎controllers/gitrepository_controller.go‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -405,10 +405,6 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context,
405405
return sreconcile.ResultSuccess, nil
406406
}
407407

408-
// Mark reconciling because the artifact and remote source are different.
409-
// and they have to be reconciled.
410-
conditions.MarkReconciling(obj, "NewRevision", "new upstream revision '%s'", artifact.Revision)
411-
412408
// Ensure target path exists and is a directory
413409
if f, err := os.Stat(dir); err != nil {
414410
e := &serror.Event{
@@ -540,8 +536,9 @@ func (r *GitRepositoryReconciler) reconcileInclude(ctx context.Context,
540536

541537
// Observe if the artifacts still match the previous included ones
542538
if artifacts.Diff(obj.Status.IncludedArtifacts) {
543-
conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "IncludeChange",
544-
"included artifacts differ from last observed includes")
539+
message := fmt.Sprintf("included artifacts differ from last observed includes")
540+
conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "IncludeChange", message)
541+
conditions.MarkReconciling(obj, "IncludeChange", message)
545542
}
546543

547544
// Persist the artifactSet.

‎controllers/gitrepository_controller_test.go‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -717,7 +717,6 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
717717
want: sreconcile.ResultSuccess,
718718
assertConditions: []metav1.Condition{
719719
*conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'main/revision'"),
720-
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'main/revision'"),
721720
},
722721
},
723722
{
@@ -736,7 +735,6 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
736735
want: sreconcile.ResultSuccess,
737736
assertConditions: []metav1.Condition{
738737
*conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'main/revision'"),
739-
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'main/revision'"),
740738
},
741739
},
742740
{
@@ -770,7 +768,6 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
770768
want: sreconcile.ResultSuccess,
771769
assertConditions: []metav1.Condition{
772770
*conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'main/revision'"),
773-
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'main/revision'"),
774771
},
775772
},
776773
{
@@ -788,7 +785,6 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
788785
want: sreconcile.ResultSuccess,
789786
assertConditions: []metav1.Condition{
790787
*conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'main/revision'"),
791-
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'main/revision'"),
792788
},
793789
},
794790
{
@@ -809,24 +805,17 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
809805
want: sreconcile.ResultSuccess,
810806
assertConditions: []metav1.Condition{
811807
*conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'main/revision'"),
812-
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'main/revision'"),
813808
},
814809
},
815810
{
816811
name: "Target path does not exists",
817812
dir: "testdata/git/foo",
818813
wantErr: true,
819-
assertConditions: []metav1.Condition{
820-
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'main/revision'"),
821-
},
822814
},
823815
{
824816
name: "Target path is not a directory",
825817
dir: "testdata/git/repository/foo.txt",
826818
wantErr: true,
827-
assertConditions: []metav1.Condition{
828-
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'main/revision'"),
829-
},
830819
},
831820
}
832821

@@ -926,6 +915,7 @@ func TestGitRepositoryReconciler_reconcileInclude(t *testing.T) {
926915
want: sreconcile.ResultSuccess,
927916
assertConditions: []metav1.Condition{
928917
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "IncludeChange", "included artifacts differ from last observed includes"),
918+
*conditions.TrueCondition(meta.ReconcilingCondition, "IncludeChange", "included artifacts differ from last observed includes"),
929919
},
930920
},
931921
{

‎controllers/helmrepository_controller.go‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -396,10 +396,6 @@ func (r *HelmRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *s
396396
return sreconcile.ResultSuccess, nil
397397
}
398398

399-
// Mark reconciling because the artifact and remote source are different.
400-
// and they have to be reconciled.
401-
conditions.MarkReconciling(obj, "NewRevision", "new index revision '%s'", artifact.Revision)
402-
403399
// Create artifact dir
404400
if err := r.Storage.MkdirAll(*artifact); err != nil {
405401
return sreconcile.ResultEmpty, &serror.Event{

‎controllers/helmrepository_controller_test.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -520,7 +520,6 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) {
520520
want: sreconcile.ResultSuccess,
521521
assertConditions: []metav1.Condition{
522522
*conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'existing'"),
523-
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new index revision 'existing'"),
524523
},
525524
},
526525
{
@@ -546,7 +545,6 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) {
546545
want: sreconcile.ResultSuccess,
547546
assertConditions: []metav1.Condition{
548547
*conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'existing'"),
549-
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new index revision 'existing'"),
550548
},
551549
},
552550
{
@@ -564,7 +562,6 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) {
564562
want: sreconcile.ResultSuccess,
565563
assertConditions: []metav1.Condition{
566564
*conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'existing'"),
567-
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new index revision 'existing'"),
568565
},
569566
},
570567
}

0 commit comments

Comments
 (0)