address some comments

Signed-off-by: Britania Rodriguez Reyes <britaniar@microsoft.com>

Author: britaniar

pkg/controllers/workapplier/apply.go

@@ -517,10 +517,7 @@ func validateOwnerReferences(
 	// expected AppliedWork object. For safety reasons, Fleet will still do a sanity check.
 	found := false
 	for _, ownerRef := range inMemberClusterObjOwnerRefs {
-		if ownerRef.UID == expectedAppliedWorkOwnerRef.UID &&
-			ownerRef.Name == expectedAppliedWorkOwnerRef.Name &&
-			ownerRef.Kind == expectedAppliedWorkOwnerRef.Kind &&
-			ownerRef.APIVersion == expectedAppliedWorkOwnerRef.APIVersion {
+		if ownerRefEqualsExpected(&ownerRef, expectedAppliedWorkOwnerRef) {
 			found = true
 			break
 		}

pkg/controllers/workapplier/controller.go

@@ -488,7 +488,7 @@ func (r *Reconciler) garbageCollectAppliedWork(ctx context.Context, work *fleetv
 			return r.removeWorkFinalizer(ctx, work)
 		}
 		klog.ErrorS(err, "Failed to get AppliedWork", "appliedWork", work.Name)
-		return ctrl.Result{Requeue: true}, controller.NewAPIServerError(false, err)
+		return ctrl.Result{}, controller.NewAPIServerError(false, err)
 	}
 
 	// Delete the appliedWork object.
@@ -552,11 +552,7 @@ func (r *Reconciler) updateOwnerReference(ctx context.Context, appliedWork *flee
 			// Re-build the owner reference; update the blockOwnerDeletion field to false.
 			updated := false
 			for idx := range ownerRefs {
-				if ownerRefs[idx].UID == appliedWorkOwnerRef.UID &&
-					ownerRefs[idx].Name == appliedWorkOwnerRef.Name &&
-					ownerRefs[idx].Kind == appliedWorkOwnerRef.Kind &&
-					ownerRefs[idx].APIVersion == appliedWorkOwnerRef.APIVersion &&
-					*ownerRefs[idx].BlockOwnerDeletion {
+				if ownerRefEqualsExpected(&ownerRefs[idx], appliedWorkOwnerRef) {
 					ownerRefs[idx].BlockOwnerDeletion = ptr.To(false)
 					updated = true
 				}

pkg/controllers/workapplier/preprocess.go

@@ -543,10 +543,7 @@ func isInMemberClusterObjectDerivedFromManifestObj(inMemberClusterObj *unstructu
 	// Verify if the owner reference still stands.
 	curOwners := inMemberClusterObj.GetOwnerReferences()
 	for idx := range curOwners {
-		if curOwners[idx].UID == expectedAppliedWorkOwnerRef.UID &&
-			curOwners[idx].Name == expectedAppliedWorkOwnerRef.Name &&
-			curOwners[idx].Kind == expectedAppliedWorkOwnerRef.Kind &&
-			curOwners[idx].APIVersion == expectedAppliedWorkOwnerRef.APIVersion {
+		if ownerRefEqualsExpected(&curOwners[idx], expectedAppliedWorkOwnerRef) {
 			return true
 		}
 	}
@@ -560,14 +557,19 @@ func removeOwnerRef(obj *unstructured.Unstructured, expectedAppliedWorkOwnerRef
 
 	// Re-build the owner references; remove the given one from the list.
 	for idx := range ownerRefs {
-		if ownerRefs[idx].UID == expectedAppliedWorkOwnerRef.UID &&
-			ownerRefs[idx].Name == expectedAppliedWorkOwnerRef.Name &&
-			ownerRefs[idx].Kind == expectedAppliedWorkOwnerRef.Kind &&
-			ownerRefs[idx].APIVersion == expectedAppliedWorkOwnerRef.APIVersion {
+		if ownerRefEqualsExpected(&ownerRefs[idx], expectedAppliedWorkOwnerRef) {
 			// Skip the expected owner reference.
 			continue
 		}
 		updatedOwnerRefs = append(updatedOwnerRefs, ownerRefs[idx])
 	}
 	obj.SetOwnerReferences(updatedOwnerRefs)
 }
+
+// ownerRefEquals returns true if two owner references are equal based on UID, Name, Kind, and APIVersion.
+func ownerRefEqualsExpected(a, b *metav1.OwnerReference) bool {
+	return a.UID == b.UID &&
+		a.Name == b.Name &&
+		a.Kind == b.Kind &&
+		a.APIVersion == b.APIVersion
+}

pkg/controllers/workapplier/process.go

@@ -356,10 +356,7 @@ func canApplyWithOwnership(inMemberClusterObj *unstructured.Unstructured, expect
 	// Verify if the object is owned by Fleet.
 	curOwners := inMemberClusterObj.GetOwnerReferences()
 	for idx := range curOwners {
-		if curOwners[idx].UID == expectedAppliedWorkOwnerRef.UID &&
-			curOwners[idx].Name == expectedAppliedWorkOwnerRef.Name &&
-			curOwners[idx].Kind == expectedAppliedWorkOwnerRef.Kind &&
-			curOwners[idx].APIVersion == expectedAppliedWorkOwnerRef.APIVersion {
+		if ownerRefEqualsExpected(&curOwners[idx], expectedAppliedWorkOwnerRef) {
 			return true
 		}
 	}