Minor changes

pkg/controllers/workapplier/availability_tracker.go

@@ -20,7 +20,7 @@ import (
 	"go.goms.io/fleet/pkg/utils/controller"
 )
 
-func (r *Reconciler) trackInMemberClusterObjAvailability(ctx context.Context, bundles []*manifestProcessingBundle, workRef *klog.ObjectRef) {
+func (r *Reconciler) trackInMemberClusterObjAvailability(ctx context.Context, bundles []*manifestProcessingBundle, workRef klog.ObjectRef) {
 	// Track the availability of all the applied objects in the member cluster in parallel.
 	//
 	// This is concurrency-safe as the bundles slice has been pre-allocated.
@@ -46,12 +46,12 @@ func (r *Reconciler) trackInMemberClusterObjAvailability(ctx context.Context, bu
 			bundle.availabilityResTyp = ManifestProcessingAvailabilityResultTypeFailed
 			klog.ErrorS(err,
 				"Failed to track the availability of the applied object in the member cluster",
-				"work", *workRef, "GVR", *bundle.gvr, "inMemberClusterObj", klog.KObj(bundle.inMemberClusterObj))
+				"work", workRef, "GVR", *bundle.gvr, "inMemberClusterObj", klog.KObj(bundle.inMemberClusterObj))
 			return
 		}
 		bundle.availabilityResTyp = availabilityResTyp
 		klog.V(2).InfoS("Tracked availability of a resource",
-			"work", *workRef, "GVR", *bundle.gvr, "inMemberClusterObj", klog.KObj(bundle.inMemberClusterObj),
+			"work", workRef, "GVR", *bundle.gvr, "inMemberClusterObj", klog.KObj(bundle.inMemberClusterObj),
 			"availabilityResTyp", availabilityResTyp)
 	}
 

pkg/controllers/workapplier/controller.go

@@ -280,7 +280,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	r.processManifests(ctx, bundles, work, expectedAppliedWorkOwnerRef)
 
 	// Track the availability information.
-	r.trackInMemberClusterObjAvailability(ctx, bundles, &workRef)
+	r.trackInMemberClusterObjAvailability(ctx, bundles, workRef)
 
 	trackWorkApplyLatencyMetric(work)
 
@@ -322,11 +322,8 @@ func (r *Reconciler) preProcessManifests(
 	defer cancel()
 
 	doWork := func(pieces int) {
+		// At this moment the bundles are just created.
 		bundle := bundles[pieces]
-		if bundle.applyErr != nil {
-			// Skip a manifest if it cannot be processed.
-			return
-		}
 
 		gvr, manifestObj, err := r.decodeManifest(bundle.manifest)
 		// Build the identifier. Note that this would return an identifier even if the decoding

pkg/controllers/workapplier/drift_detection_takeover.go

@@ -89,7 +89,9 @@ func (r *Reconciler) diffBetweenManifestAndInMemberClusterObjects(
 	case fleetv1beta1.ComparisonOptionTypePartialComparison:
 		return r.partialDiffBetweenManifestAndInMemberClusterObjects(ctx, gvr, manifestObj, inMemberClusterObj)
 	case fleetv1beta1.ComparisonOptionTypeFullComparison:
-		return fullDiffBetweenManifestAndInMemberClusterObjects(manifestObj, inMemberClusterObj)
+		// For the full comparison, Fleet compares directly the JSON representations of the
+		// manifest object and the object in the member cluster.
+		return preparePatchDetails(manifestObj, inMemberClusterObj)
 	default:
 		return nil, fmt.Errorf("an invalid comparison option is specified")
 	}
@@ -115,64 +117,8 @@ func (r *Reconciler) partialDiffBetweenManifestAndInMemberClusterObjects(
 	// imply that running an actual apply op would lead to unexpected changes, which signifies
 	// the presence of partial drifts (drifts in managed fields).
 
-	// Discard certain fields from both objects before comparison.
-	inMemberClusterObjCopy := discardFieldsIrrelevantInComparisonFrom(inMemberClusterObj)
-	appliedObjCopy := discardFieldsIrrelevantInComparisonFrom(appliedObj)
-
-	// Marshal the objects into JSON.
-	inMemberClusterObjCopyJSONBytes, err := inMemberClusterObjCopy.MarshalJSON()
-	if err != nil {
-		return nil, fmt.Errorf("failed to marshal the object from the member cluster into JSON: %w", err)
-	}
-	appliedObjJSONBytes, err := appliedObjCopy.MarshalJSON()
-	if err != nil {
-		return nil, fmt.Errorf("failed to marshal the object returned by the dry-run apply op into JSON: %w", err)
-	}
-
-	// Compare the JSON representations.
-	patch, err := jsondiff.CompareJSON(appliedObjJSONBytes, inMemberClusterObjCopyJSONBytes)
-	if err != nil {
-		return nil, fmt.Errorf("failed to compare the JSON representations of the the object from the member cluster and the object returned by the dry-run apply op: %w", err)
-	}
-
-	details, err := organizeJSONPatchIntoFleetPatchDetails(patch, appliedObjCopy.Object, inMemberClusterObj.Object)
-	if err != nil {
-		return nil, fmt.Errorf("failed to organize the JSON patch into Fleet patch details: %w", err)
-	}
-	return details, nil
-}
-
-func fullDiffBetweenManifestAndInMemberClusterObjects(
-	manifestObj, inMemberClusterObj *unstructured.Unstructured,
-) ([]fleetv1beta1.PatchDetail, error) {
-	// Fleet calculates the full diff between two objects by directly comparing their JSON
-	// representations.
-
-	// Discard certain fields from both objects before comparison.
-	manifestObjCopy := discardFieldsIrrelevantInComparisonFrom(manifestObj)
-	inMemberClusterObjCopy := discardFieldsIrrelevantInComparisonFrom(inMemberClusterObj)
-
-	// Marshal the objects into JSON.
-	manifestObjJSONBytes, err := manifestObjCopy.MarshalJSON()
-	if err != nil {
-		return nil, fmt.Errorf("failed to marshal the manifest object into JSON: %w", err)
-	}
-	inMemberClusterObjJSONBytes, err := inMemberClusterObjCopy.MarshalJSON()
-	if err != nil {
-		return nil, fmt.Errorf("failed to marshal the object from the member cluster into JSON: %w", err)
-	}
-
-	// Compare the JSON representations.
-	patch, err := jsondiff.CompareJSON(manifestObjJSONBytes, inMemberClusterObjJSONBytes)
-	if err != nil {
-		return nil, fmt.Errorf("failed to compare the JSON representations of the manifest object and the object from the member cluster: %w", err)
-	}
-
-	details, err := organizeJSONPatchIntoFleetPatchDetails(patch, manifestObjCopy.Object, inMemberClusterObj.Object)
-	if err != nil {
-		return nil, fmt.Errorf("failed to organize JSON patch operations into Fleet patch details: %w", err)
-	}
-	return details, nil
+	// Prepare the patch details.
+	return preparePatchDetails(appliedObj, inMemberClusterObj)
 }
 
 func organizeJSONPatchIntoFleetPatchDetails(patch jsondiff.Patch, manifestObjMap, inMemberClusterObjMap map[string]interface{}) ([]fleetv1beta1.PatchDetail, error) {
@@ -263,3 +209,32 @@ func organizeJSONPatchIntoFleetPatchDetails(patch jsondiff.Patch, manifestObjMap
 
 	return details, nil
 }
+
+func preparePatchDetails(srcObj, destObj *unstructured.Unstructured) ([]fleetv1beta1.PatchDetail, error) {
+	// Discard certain fields from both objects before comparison.
+	srcObjCopy := discardFieldsIrrelevantInComparisonFrom(srcObj)
+	destObjCopy := discardFieldsIrrelevantInComparisonFrom(destObj)
+
+	// Marshal the objects into JSON.
+	srcObjJSONBytes, err := srcObjCopy.MarshalJSON()
+	if err != nil {
+		return nil, fmt.Errorf("failed to marshal the source object into JSON: %w", err)
+	}
+
+	destObjJSONBytes, err := destObjCopy.MarshalJSON()
+	if err != nil {
+		return nil, fmt.Errorf("failed to marshal the destination object into JSON: %w", err)
+	}
+
+	// Compare the JSON representations.
+	patch, err := jsondiff.CompareJSON(srcObjJSONBytes, destObjJSONBytes)
+	if err != nil {
+		return nil, fmt.Errorf("failed to compare the JSON representations of the source and destination objects: %w", err)
+	}
+
+	details, err := organizeJSONPatchIntoFleetPatchDetails(patch, srcObjCopy.Object, destObjCopy.Object)
+	if err != nil {
+		return nil, fmt.Errorf("failed to organize JSON patch operations into Fleet patch details: %w", err)
+	}
+	return details, nil
+}

pkg/controllers/workapplier/utils.go

@@ -473,8 +473,9 @@ func shouldPerformPreApplyDriftDetection(manifestObj, inMemberClusterObj *unstru
 	// * Fleet reports that the manifest has been applied before (i.e., inMemberClusterObj exists); and
 	// * The apply strategy dictates that an apply op should only run if there is no
 	//   detected drift; and
-	// * The hash of the manifest object is consistently with that bookkept in the live object
-	//   annotations (i.e., the manifest object has been applied before).
+	// * The hash of the manifest object is consistent with the last applied manifest object hash
+	//   annotation on the corresponding resource in the member cluster (i.e., the same manifest
+	//   object has been applied before).
 	if applyStrategy.WhenToApply != fleetv1beta1.WhenToApplyTypeIfNotDrifted || inMemberClusterObj == nil {
 		// A shortcut to save some overhead.
 		return false, nil