rollouts: check entire spec when patching rootsync

rollouts/controllers/rollout_controller.go

@@ -21,12 +21,12 @@ import (
 	"flag"
 	"fmt"
 	"math"
-	"reflect"
 	"sort"
 	"strings"
 	"sync"
 	"time"
 
+	"k8s.io/apimachinery/pkg/api/equality"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -432,7 +432,10 @@ func (r *RolloutReconciler) computeTargets(ctx context.Context,
 			}
 		} else {
 			// remotesync already exists
-			updated, needsUpdate := pkgNeedsUpdate(ctx, rollout, rs, pkg)
+			updated, needsUpdate := rsNeedsUpdate(ctx, rollout, &rs, &clusterPackagePair{
+				cluster:    cluster,
+				packageRef: pkg,
+			})
 			if needsUpdate {
 				targets.ToBeUpdated = append(targets.ToBeUpdated, updated)
 			} else {
@@ -448,18 +451,21 @@ func (r *RolloutReconciler) computeTargets(ctx context.Context,
 	return targets, nil
 }
 
-func pkgNeedsUpdate(ctx context.Context, rollout *gitopsv1alpha1.Rollout, rs gitopsv1alpha1.RemoteSync, pkg *packagediscovery.DiscoveredPackage) (*gitopsv1alpha1.RemoteSync, bool) {
-	// TODO: We need to check other things here besides git.Revision and metadata
-	metadata := getSpecMetadata(rollout)
-	if pkg.Revision != rs.Spec.Template.Spec.Git.Revision ||
-		!reflect.DeepEqual(metadata, rs.Spec.Template.Metadata) ||
-		rs.Spec.Type != rollout.GetSyncTemplateType() {
+// rsNeedsUpdate checks if the underlying remotesync needs to be updated by creating a new RemoteSync object and comparing it to the existing one
+func rsNeedsUpdate(ctx context.Context, rollout *gitopsv1alpha1.Rollout, oldRS *gitopsv1alpha1.RemoteSync, target *clusterPackagePair) (*gitopsv1alpha1.RemoteSync, bool) {
+	newRS := newRemoteSync(rollout,
+		gitopsv1alpha1.ClusterRef{Name: target.cluster.Name},
+		toSyncSpec(target.packageRef),
+		pkgID(target.packageRef),
+		getSpecMetadata(rollout),
+	)
 
-		rs.Spec.Template.Spec.Git.Revision = pkg.Revision
-		rs.Spec.Template.Metadata = metadata
-		rs.Spec.Type = rollout.GetSyncTemplateType()
-		return &rs, true
+	// if the spec of the new sync object is identical to the existing one, then no update is necessary
+	if !equality.Semantic.DeepEqual(oldRS.Spec, newRS.Spec) {
+		oldRS.Spec = newRS.Spec
+		return oldRS, true
 	}
+
 	return nil, false
 }
 
@@ -547,7 +553,6 @@ func (r *RolloutReconciler) rolloutTargets(ctx context.Context, rollout *gitopsv
 			gitopsv1alpha1.ClusterRef{Name: target.cluster.Name},
 			syncSpec,
 			pkgID(target.packageRef),
-			wave.Name,
 			getSpecMetadata(rollout),
 		)
 
@@ -723,7 +728,6 @@ func newRemoteSync(rollout *gitopsv1alpha1.Rollout,
 	clusterRef gitopsv1alpha1.ClusterRef,
 	rssSpec *gitopsv1alpha1.SyncSpec,
 	pkgID string,
-	waveName string,
 	metadata *gitopsv1alpha1.Metadata) *gitopsv1alpha1.RemoteSync {
 	t := true
 	clusterName := clusterRef.Name[strings.LastIndex(clusterRef.Name, "/")+1:]