change update func

pkg/controllers/workgenerator/controller.go

@@ -151,6 +151,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, req controllerruntime.Reques
 		resourceBinding.RemoveCondition(string(i.ResourceBindingConditionType()))
 	}
 	resourceBinding.Status.FailedPlacements = nil
+	resourceBinding.Status.DriftedPlacements = nil
+	resourceBinding.Status.DiffedPlacements = nil
 	if overrideSucceeded {
 		overrideReason := condition.OverriddenSucceededReason
 		overrideMessage := "Successfully applied the override rules on the resources"
@@ -1152,8 +1154,8 @@ func (r *Reconciler) SetupWithManager(mgr controllerruntime.Manager) error {
 				newAvailableCondition := meta.FindStatusCondition(newWork.Status.Conditions, fleetv1beta1.WorkConditionTypeAvailable)
 
 				// we try to filter out events, we only need to handle the updated event if the applied or available condition flip between true and false
-				// or the failed placements are changed.
-				if condition.EqualCondition(oldAppliedCondition, newAppliedCondition) && condition.EqualCondition(oldAvailableCondition, newAvailableCondition) {
+				// or the failed/diffed/drifted placements are changed.
+				if !changedDriftDetails(oldWork, newWork) && !changedDiffDetails(oldWork, newWork) && condition.EqualCondition(oldAppliedCondition, newAppliedCondition) && condition.EqualCondition(oldAvailableCondition, newAvailableCondition) {
 					if condition.IsConditionStatusFalse(newAppliedCondition, newWork.Generation) || condition.IsConditionStatusFalse(newAvailableCondition, newWork.Generation) {
 						// we need to compare the failed placement if the work is not applied or available
 						oldFailedPlacements := extractFailedResourcePlacementsFromWork(oldWork)
@@ -1182,21 +1184,6 @@ func (r *Reconciler) SetupWithManager(mgr controllerruntime.Manager) error {
 						}
 					}
 				}
-				// we need to compare the drift placement
-				oldDriftedPlacements := extractDriftedResourcePlacementsFromWork(oldWork)
-				newDriftedPlacements := extractDriftedResourcePlacementsFromWork(newWork)
-				if utils.IsDriftedResourcePlacementsEqual(oldDriftedPlacements, newDriftedPlacements) {
-					klog.V(2).InfoS("The drifted placement list didn't change, no need to reconcile", "oldWork", klog.KObj(oldWork), "newWork", klog.KObj(newWork))
-					return
-				}
-
-				// we need to compare the diffed placement
-				oldDiffedPlacements := extractDiffedResourcePlacementsFromWork(oldWork)
-				newDiffedPlacements := extractDiffedResourcePlacementsFromWork(newWork)
-				if utils.IsDiffedResourcePlacementsEqual(oldDiffedPlacements, newDiffedPlacements) {
-					klog.V(2).InfoS("The diffed placement list didn't change, no need to reconcile", "oldWork", klog.KObj(oldWork), "newWork", klog.KObj(newWork))
-					return
-				}
 				// We need to update the binding status in this case
 				klog.V(2).InfoS("Received a work update event that we need to handle", "work", klog.KObj(newWork), "parentBindingName", parentBindingName)
 				queue.Add(reconcile.Request{NamespacedName: types.NamespacedName{
@@ -1206,3 +1193,35 @@ func (r *Reconciler) SetupWithManager(mgr controllerruntime.Manager) error {
 		}).
 		Complete(r)
 }
+
+// changedDiffDetails checks if the drift details are changed between the old and new work
+func changedDiffDetails(oldWork, newWork *fleetv1beta1.Work) bool {
+	for i, oldManifestCondition := range oldWork.Status.ManifestConditions {
+		if oldManifestCondition.DiffDetails == nil && newWork.Status.ManifestConditions[i].DiffDetails == nil {
+			continue
+		}
+		if oldManifestCondition.DiffDetails.ObservedInMemberClusterGeneration != newWork.Status.ManifestConditions[i].DiffDetails.ObservedInMemberClusterGeneration {
+			return true
+		} else if oldManifestCondition.DiffDetails.ObservationTime != newWork.Status.ManifestConditions[i].DiffDetails.ObservationTime {
+			return true
+		}
+	}
+	klog.V(2).InfoS("No change in the drift details", "oldWork", klog.KObj(oldWork), "newWork", klog.KObj(newWork))
+	return false
+}
+
+// changedDriftDetails checks if the drift details are changed between the old and new work
+func changedDriftDetails(oldWork, newWork *fleetv1beta1.Work) bool {
+	for i, oldManifestCondition := range oldWork.Status.ManifestConditions {
+		if oldManifestCondition.DriftDetails == nil && newWork.Status.ManifestConditions[i].DriftDetails == nil {
+			continue
+		}
+		if oldManifestCondition.DriftDetails.ObservedInMemberClusterGeneration != newWork.Status.ManifestConditions[i].DriftDetails.ObservedInMemberClusterGeneration {
+			return true
+		} else if oldManifestCondition.DriftDetails.ObservationTime != newWork.Status.ManifestConditions[i].DriftDetails.ObservationTime {
+			return true
+		}
+	}
+	klog.V(2).InfoS("No change in the drift details", "oldWork", klog.KObj(oldWork), "newWork", klog.KObj(newWork))
+	return false
+}

pkg/controllers/workgenerator/controller_test.go

@@ -1238,7 +1238,7 @@ func TestSetBindingStatus(t *testing.T) {
 									FirstDriftedObservedTime:          metav1.NewTime(timeNow.Add(-time.Hour)),
 									ObservedDrifts: []fleetv1beta1.PatchDetail{
 										{
-											Path:          "/spec/ports/1/port",
+											Path:          "/spec/ports/0/port",
 											ValueInHub:    "80",
 											ValueInMember: "90",
 										},
@@ -1339,7 +1339,7 @@ func TestSetBindingStatus(t *testing.T) {
 					FirstDriftedObservedTime:        metav1.NewTime(timeNow.Add(-time.Hour)),
 					ObservedDrifts: []fleetv1beta1.PatchDetail{
 						{
-							Path:          "/spec/ports/1/port",
+							Path:          "/spec/ports/0/port",
 							ValueInHub:    "80",
 							ValueInMember: "90",
 						},
@@ -1442,7 +1442,7 @@ func TestSetBindingStatus(t *testing.T) {
 									FirstDiffedObservedTime:           metav1.NewTime(timeNow.Add(-time.Second)),
 									ObservedDiffs: []fleetv1beta1.PatchDetail{
 										{
-											Path:          "/spec/ports/1/port",
+											Path:          "/spec/ports/0/port",
 											ValueInHub:    "80",
 											ValueInMember: "90",
 										},
@@ -1472,12 +1472,12 @@ func TestSetBindingStatus(t *testing.T) {
 						Group:     "",
 						Version:   "v1",
 						Kind:      "Service",
-						Name:      "svc-name-1",
+						Name:      "svc-name",
 						Namespace: "svc-namespace",
 					},
 					ObservationTime:                 metav1.NewTime(timeNow),
 					TargetClusterObservedGeneration: 2,
-					FirstDiffedObservedTime:         metav1.NewTime(timeNow.Add(-time.Second)),
+					FirstDiffedObservedTime:         metav1.NewTime(timeNow.Add(-time.Hour)),
 					ObservedDiffs: []fleetv1beta1.PatchDetail{
 						{
 							Path:          "/spec/ports/1/port",
@@ -1694,9 +1694,9 @@ func TestSetBindingStatus(t *testing.T) {
 			binding := &fleetv1beta1.ClusterResourceBinding{}
 			setBindingStatus(tt.works, binding)
 			got := binding.Status.FailedPlacements
-			// setBindingStatus is using map to populate the failedResourcePlacement.
+			// setBindingStatus is using map to populate the placements.
 			// There is no default order in traversing the map.
-			// When the result of failedResourcePlacement exceeds the limit, the result will be truncated and cannot be
+			// When the result of Placements exceeds the limit, the result will be truncated and cannot be
 			// guaranteed.
 			if maxFailedResourcePlacementLimit == len(tt.wantFailedResourcePlacements) {
 				opt := cmp.Comparer(func(x, y fleetv1beta1.FailedResourcePlacement) bool {
@@ -1735,8 +1735,8 @@ func TestSetBindingStatus(t *testing.T) {
 						}
 						return n1.Namespace < n2.Namespace
 					}),
-					cmpopts.SortSlices(func(f1, f2 fleetv1beta1.DriftedResourcePlacement) bool {
-						return f1.ResourceIdentifier.Kind < f2.ResourceIdentifier.Kind
+					cmp.Comparer(func(f1, f2 fleetv1beta1.DriftedResourcePlacement) bool {
+						return f1.ResourceIdentifier.Kind == f2.ResourceIdentifier.Kind
 					}),
 					cmp.Comparer(func(t1, t2 metav1.Time) bool {
 						if t1.Time.IsZero() || t2.Time.IsZero() {
@@ -1757,13 +1757,13 @@ func TestSetBindingStatus(t *testing.T) {
 
 			resourceCmpOptions := []cmp.Option{
 				cmpopts.SortSlices(func(i, j fleetv1beta1.DriftedResourcePlacement) bool {
-					if i.ResourceIdentifier.Group < j.ResourceIdentifier.Group {
+					if i.Group < j.Group {
 						return true
 					}
-					if i.ResourceIdentifier.Kind < j.ResourceIdentifier.Kind {
+					if i.Kind < j.Kind {
 						return true
 					}
-					return i.ResourceIdentifier.Name < j.ResourceIdentifier.Name
+					return i.Name < j.Name
 				}),
 			}
 			if diff := cmp.Diff(gotDrifted, tt.wantDriftedResourcePlacements, resourceCmpOptions...); diff != "" {
@@ -1804,13 +1804,13 @@ func TestSetBindingStatus(t *testing.T) {
 
 			resourceCmpOptions = []cmp.Option{
 				cmpopts.SortSlices(func(i, j fleetv1beta1.DiffedResourcePlacement) bool {
-					if i.ResourceIdentifier.Group < j.ResourceIdentifier.Group {
+					if i.Group < j.Group {
 						return true
 					}
-					if i.ResourceIdentifier.Kind < j.ResourceIdentifier.Kind {
+					if i.Kind < j.Kind {
 						return true
 					}
-					return i.ResourceIdentifier.Name < j.ResourceIdentifier.Name
+					return i.Name < j.Name
 				}),
 			}
 			if diff := cmp.Diff(gotDiffed, tt.wantDiffedResourcePlacements, resourceCmpOptions...); diff != "" {

pkg/utils/common.go

@@ -555,108 +555,6 @@ func IsFailedResourcePlacementsEqual(oldFailedResourcePlacements, newFailedResou
 	return true
 }
 
-// LessFuncDriftedResourcePlacements is a less function for sorting drifted resource placements
-var LessFuncDriftedResourcePlacements = func(a, b placementv1beta1.DriftedResourcePlacement) bool {
-	var aStr, bStr string
-	if a.Envelope != nil {
-		aStr = fmt.Sprintf(ResourceIdentifierWithEnvelopeIdentifierStringFormat, a.Group, a.Version, a.Kind, a.Namespace, a.Name, a.Envelope.Type, a.Envelope.Namespace, a.Envelope.Name)
-	} else {
-		aStr = fmt.Sprintf(ResourceIdentifierStringFormat, a.Group, a.Version, a.Kind, a.Namespace, a.Name)
-	}
-	if b.Envelope != nil {
-		bStr = fmt.Sprintf(ResourceIdentifierWithEnvelopeIdentifierStringFormat, b.Group, b.Version, b.Kind, b.Namespace, b.Name, b.Envelope.Type, b.Envelope.Namespace, b.Envelope.Name)
-	} else {
-		bStr = fmt.Sprintf(ResourceIdentifierStringFormat, b.Group, b.Version, b.Kind, b.Namespace, b.Name)
-
-	}
-	return aStr < bStr
-}
-
-func IsDriftedResourcePlacementsEqual(oldDriftedResourcePlacements, newDriftedResourcePlacements []placementv1beta1.DriftedResourcePlacement) bool {
-	if len(oldDriftedResourcePlacements) != len(newDriftedResourcePlacements) {
-		return false
-	}
-	sort.Slice(oldDriftedResourcePlacements, func(i, j int) bool {
-		return LessFuncDriftedResourcePlacements(oldDriftedResourcePlacements[i], oldDriftedResourcePlacements[j])
-	})
-	sort.Slice(newDriftedResourcePlacements, func(i, j int) bool {
-		return LessFuncDriftedResourcePlacements(newDriftedResourcePlacements[i], newDriftedResourcePlacements[j])
-	})
-	for i := range oldDriftedResourcePlacements {
-		oldDriftedResourcePlacement := oldDriftedResourcePlacements[i]
-		newDriftedResourcePlacement := newDriftedResourcePlacements[i]
-		if !equality.Semantic.DeepEqual(oldDriftedResourcePlacement.ResourceIdentifier, newDriftedResourcePlacement.ResourceIdentifier) {
-			return false
-		}
-		if !equality.Semantic.DeepEqual(oldDriftedResourcePlacement.ObservationTime, newDriftedResourcePlacement.ObservationTime) {
-			return false
-		}
-		if oldDriftedResourcePlacement.TargetClusterObservedGeneration != newDriftedResourcePlacement.TargetClusterObservedGeneration {
-			return false
-		}
-		if !equality.Semantic.DeepEqual(oldDriftedResourcePlacement.FirstDriftedObservedTime, newDriftedResourcePlacement.FirstDriftedObservedTime) {
-			return false
-		}
-		for j := range oldDriftedResourcePlacement.ObservedDrifts {
-			if !equality.Semantic.DeepEqual(oldDriftedResourcePlacement.ObservedDrifts[j], newDriftedResourcePlacement.ObservedDrifts[j]) {
-				return false
-			}
-		}
-	}
-	return true
-}
-
-// LessFuncDiffedResourcePlacements is a less function for sorting diffed resource placements
-var LessFuncDiffedResourcePlacements = func(a, b placementv1beta1.DiffedResourcePlacement) bool {
-	var aStr, bStr string
-	if a.Envelope != nil {
-		aStr = fmt.Sprintf(ResourceIdentifierWithEnvelopeIdentifierStringFormat, a.Group, a.Version, a.Kind, a.Namespace, a.Name, a.Envelope.Type, a.Envelope.Namespace, a.Envelope.Name)
-	} else {
-		aStr = fmt.Sprintf(ResourceIdentifierStringFormat, a.Group, a.Version, a.Kind, a.Namespace, a.Name)
-	}
-	if b.Envelope != nil {
-		bStr = fmt.Sprintf(ResourceIdentifierWithEnvelopeIdentifierStringFormat, b.Group, b.Version, b.Kind, b.Namespace, b.Name, b.Envelope.Type, b.Envelope.Namespace, b.Envelope.Name)
-	} else {
-		bStr = fmt.Sprintf(ResourceIdentifierStringFormat, b.Group, b.Version, b.Kind, b.Namespace, b.Name)
-
-	}
-	return aStr < bStr
-}
-
-func IsDiffedResourcePlacementsEqual(oldDiffedResourcePlacements, newDiffedResourcePlacements []placementv1beta1.DiffedResourcePlacement) bool {
-	if len(oldDiffedResourcePlacements) != len(newDiffedResourcePlacements) {
-		return false
-	}
-	sort.Slice(oldDiffedResourcePlacements, func(i, j int) bool {
-		return LessFuncDiffedResourcePlacements(oldDiffedResourcePlacements[i], oldDiffedResourcePlacements[j])
-	})
-	sort.Slice(newDiffedResourcePlacements, func(i, j int) bool {
-		return LessFuncDiffedResourcePlacements(newDiffedResourcePlacements[i], newDiffedResourcePlacements[j])
-	})
-	for i := range oldDiffedResourcePlacements {
-		oldDiffedResourcePlacement := oldDiffedResourcePlacements[i]
-		newDiffedResourcePlacement := newDiffedResourcePlacements[i]
-		if !equality.Semantic.DeepEqual(oldDiffedResourcePlacement.ResourceIdentifier, newDiffedResourcePlacement.ResourceIdentifier) {
-			return false
-		}
-		if !equality.Semantic.DeepEqual(oldDiffedResourcePlacement.ObservationTime, newDiffedResourcePlacement.ObservationTime) {
-			return false
-		}
-		if oldDiffedResourcePlacement.TargetClusterObservedGeneration != newDiffedResourcePlacement.TargetClusterObservedGeneration {
-			return false
-		}
-		if !equality.Semantic.DeepEqual(oldDiffedResourcePlacement.FirstDiffedObservedTime, newDiffedResourcePlacement.FirstDiffedObservedTime) {
-			return false
-		}
-		for j := range oldDiffedResourcePlacement.ObservedDiffs {
-			if !equality.Semantic.DeepEqual(oldDiffedResourcePlacement.ObservedDiffs[j], newDiffedResourcePlacement.ObservedDiffs[j]) {
-				return false
-			}
-		}
-	}
-	return true
-}
-
 // IsFleetAnnotationPresent returns true if a key with fleet prefix is present in the annotations map.
 func IsFleetAnnotationPresent(annotations map[string]string) bool {
 	for k := range annotations {