label namespace with parent-CRP

Signed-off-by: Wei Weng <[email protected]>

Author: Wei Weng

pkg/controllers/workapplier/apply.go

@@ -52,7 +52,12 @@ func (r *Reconciler) applyInDryRunMode(
 	ctx context.Context,
 	gvr *schema.GroupVersionResource,
 	manifestObj, inMemberClusterObj *unstructured.Unstructured,
+	parentCRPName string,
 ) (*unstructured.Unstructured, error) {
+	// Add parent-CRP label to namespace objects for namespace affinity support
+	manifestObjCopy := manifestObj.DeepCopy()
+	labelNamespaceWithParentCRP(manifestObjCopy, parentCRPName)
+
 	// In this method, Fleet will always use forced server-side apply
 	// w/o optimistic lock for diff calculation.
 	//
@@ -61,7 +66,7 @@ func (r *Reconciler) applyInDryRunMode(
 	// before the comparison.
 	//
 	// Note that full comparison can be carried out directly without involving the apply op.
-	return r.serverSideApply(ctx, gvr, manifestObj, inMemberClusterObj, true, false, true)
+	return r.serverSideApply(ctx, gvr, manifestObjCopy, inMemberClusterObj, true, false, true)
 }
 
 func (r *Reconciler) apply(
@@ -70,6 +75,7 @@ func (r *Reconciler) apply(
 	manifestObj, inMemberClusterObj *unstructured.Unstructured,
 	applyStrategy *fleetv1beta1.ApplyStrategy,
 	expectedAppliedWorkOwnerRef *metav1.OwnerReference,
+	parentCRPName string,
 ) (*unstructured.Unstructured, error) {
 	// Create a sanitized copy of the manifest object.
 	//
@@ -84,6 +90,9 @@ func (r *Reconciler) apply(
 	// backwards compatibility concerns.
 	manifestObjCopy := sanitizeManifestObject(manifestObj)
 
+	// Add parent-CRP label to namespace objects for namespace affinity support
+	labelNamespaceWithParentCRP(manifestObjCopy, parentCRPName)
+
 	// Compute the hash of the manifest object.
 	//
 	// Originally the manifest hash is kept only if three-way merge patch (client side apply)
@@ -129,7 +138,7 @@ func (r *Reconciler) apply(
 
 	// Create the object if it does not exist in the member cluster.
 	if inMemberClusterObj == nil {
-		return r.createManifestObject(ctx, gvr, manifestObjCopy)
+		return r.createManifestObject(ctx, gvr, manifestObjCopy, parentCRPName)
 	}
 
 	// Note: originally Fleet will add its owner reference and
@@ -199,6 +208,7 @@ func (r *Reconciler) createManifestObject(
 	ctx context.Context,
 	gvr *schema.GroupVersionResource,
 	manifestObject *unstructured.Unstructured,
+	parentCRPName string,
 ) (*unstructured.Unstructured, error) {
 	createOpts := metav1.CreateOptions{
 		FieldManager: workFieldManagerName,
@@ -649,3 +659,36 @@ func shouldUseForcedServerSideApply(inMemberClusterObj *unstructured.Unstructure
 		"GVK", inMemberClusterObj.GroupVersionKind(), "inMemberClusterObj", klog.KObj(inMemberClusterObj))
 	return true
 }
+
+// labelNamespaceWithParentCRP adds parent-CRP label to namespace objects being applied.
+// This supports the namespace affinity feature where ResourcePlacements can be scheduled
+// only on clusters that have the required namespace.
+func labelNamespaceWithParentCRP(manifestObj *unstructured.Unstructured, parentCRPName string) {
+	// Only label if this is a namespace object
+	if manifestObj.GetKind() != "Namespace" || manifestObj.GetAPIVersion() != "v1" {
+		return
+	}
+
+	// Skip if no parent CRP name is provided
+	if parentCRPName == "" {
+		klog.Warningf("No parent CRP name provided for namespace %s, skipping labeling",
+			manifestObj.GetName())
+		return
+	}
+
+	// Get existing labels or create new map
+	labels := manifestObj.GetLabels()
+	if labels == nil {
+		labels = make(map[string]string)
+	}
+
+	// Add the parent-CRP label
+	// Format: kubernetes-fleet.io/parent-CRP: {crp-name}
+	labels[fleetv1beta1.PlacementTrackingLabel] = parentCRPName
+
+	// Set the updated labels back
+	manifestObj.SetLabels(labels)
+
+	klog.V(2).InfoS("Added parent-CRP label to namespace",
+		"namespace", manifestObj.GetName(), "crpName", parentCRPName)
+}

pkg/controllers/workapplier/apply_test.go

@@ -552,3 +552,106 @@ func TestShouldUseForcedServerSideApply(t *testing.T) {
 		})
 	}
 }
+
+// TestLabelNamespaceWithParentCRP tests the labelNamespaceWithParentCRP function.
+func TestLabelNamespaceWithParentCRP(t *testing.T) {
+	testCases := []struct {
+		name          string
+		manifestObj   client.Object
+		parentCRPName string
+		wantLabels    map[string]string
+	}{
+		{
+			name: "namespace object with no existing labels",
+			manifestObj: &corev1.Namespace{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: "v1",
+					Kind:       "Namespace",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-namespace",
+				},
+			},
+			parentCRPName: "test-crp",
+			wantLabels: map[string]string{
+				fleetv1beta1.PlacementTrackingLabel: "test-crp",
+			},
+		},
+		{
+			name: "namespace object with existing labels",
+			manifestObj: &corev1.Namespace{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: "v1",
+					Kind:       "Namespace",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-namespace",
+					Labels: map[string]string{
+						"existing-label": "existing-value",
+					},
+				},
+			},
+			parentCRPName: "my-app-crp",
+			wantLabels: map[string]string{
+				"existing-label":                    "existing-value",
+				fleetv1beta1.PlacementTrackingLabel: "my-app-crp",
+			},
+		},
+		{
+			name: "non-namespace object should not be modified",
+			manifestObj: &appsv1.Deployment{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: "apps/v1",
+					Kind:       "Deployment",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-deployment",
+					Labels: map[string]string{
+						"app": "test",
+					},
+				},
+			},
+			parentCRPName: "test-crp",
+			wantLabels: map[string]string{
+				"app": "test",
+			},
+		},
+		{
+			name: "empty parentCRPName should not add label",
+			manifestObj: &corev1.Namespace{
+				TypeMeta: metav1.TypeMeta{
+					APIVersion: "v1",
+					Kind:       "Namespace",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-namespace",
+					Labels: map[string]string{
+						"existing": "label",
+					},
+				},
+			},
+			parentCRPName: "",
+			wantLabels: map[string]string{
+				"existing": "label",
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			// Convert to unstructured and make a copy to avoid modifying the original test data
+			manifestCopy := toUnstructured(t, tc.manifestObj).DeepCopy()
+
+			// Call the function under test
+			labelNamespaceWithParentCRP(manifestCopy, tc.parentCRPName)
+
+			// Get the resulting labels
+			gotLabels := manifestCopy.GetLabels()
+
+			// Compare the labels
+			if diff := cmp.Diff(tc.wantLabels, gotLabels); diff != "" {
+				t.Errorf("labelNamespaceWithParentCRP() labels mismatch (-want +got):\n%s", diff)
+			}
+		})
+	}
+}

pkg/controllers/workapplier/drift_detection_takeover.go

@@ -47,6 +47,7 @@ func (r *Reconciler) takeOverPreExistingObject(
 	manifestObj, inMemberClusterObj *unstructured.Unstructured,
 	applyStrategy *fleetv1beta1.ApplyStrategy,
 	expectedAppliedWorkOwnerRef *metav1.OwnerReference,
+	parentCRPName string,
 ) (*unstructured.Unstructured, []fleetv1beta1.PatchDetail, bool, error) {
 	inMemberClusterObjCopy := inMemberClusterObj.DeepCopy()
 	existingOwnerRefs := inMemberClusterObjCopy.GetOwnerReferences()
@@ -91,7 +92,7 @@ func (r *Reconciler) takeOverPreExistingObject(
 	//
 	// Note that the default takeover action is AlwaysApply.
 	if applyStrategy.WhenToTakeOver == fleetv1beta1.WhenToTakeOverTypeIfNoDiff {
-		configDiffs, diffCalculatedInDegradedMode, err := r.diffBetweenManifestAndInMemberClusterObjects(ctx, gvr, manifestObj, inMemberClusterObjCopy, applyStrategy.ComparisonOption)
+		configDiffs, diffCalculatedInDegradedMode, err := r.diffBetweenManifestAndInMemberClusterObjects(ctx, gvr, manifestObj, inMemberClusterObjCopy, applyStrategy.ComparisonOption, parentCRPName)
 		switch {
 		case err != nil:
 			return nil, nil, false, fmt.Errorf("failed to calculate configuration diffs between the manifest object and the object from the member cluster: %w", err)
@@ -121,10 +122,11 @@ func (r *Reconciler) diffBetweenManifestAndInMemberClusterObjects(
 	gvr *schema.GroupVersionResource,
 	manifestObj, inMemberClusterObj *unstructured.Unstructured,
 	cmpOption fleetv1beta1.ComparisonOptionType,
+	parentCRPName string,
 ) ([]fleetv1beta1.PatchDetail, bool, error) {
 	switch cmpOption {
 	case fleetv1beta1.ComparisonOptionTypePartialComparison:
-		return r.partialDiffBetweenManifestAndInMemberClusterObjects(ctx, gvr, manifestObj, inMemberClusterObj)
+		return r.partialDiffBetweenManifestAndInMemberClusterObjects(ctx, gvr, manifestObj, inMemberClusterObj, parentCRPName)
 	case fleetv1beta1.ComparisonOptionTypeFullComparison:
 		// For the full comparison, Fleet compares directly the JSON representations of the
 		// manifest object and the object in the member cluster.
@@ -143,10 +145,11 @@ func (r *Reconciler) partialDiffBetweenManifestAndInMemberClusterObjects(
 	ctx context.Context,
 	gvr *schema.GroupVersionResource,
 	manifestObj, inMemberClusterObj *unstructured.Unstructured,
+	parentCRPName string,
 ) ([]fleetv1beta1.PatchDetail, bool, error) {
 	// Fleet calculates the partial diff between two objects by running apply ops in the dry-run
 	// mode.
-	appliedObj, err := r.applyInDryRunMode(ctx, gvr, manifestObj, inMemberClusterObj)
+	appliedObj, err := r.applyInDryRunMode(ctx, gvr, manifestObj, inMemberClusterObj, parentCRPName)
 
 	// After the dry-run apply op, all the managed fields should have been overwritten using the
 	// values from the manifest object, while leaving all the unmanaged fields untouched. This

pkg/controllers/workapplier/drift_detection_takeover_test.go

@@ -215,7 +215,8 @@ func TestTakeOverPreExistingObject(t *testing.T) {
 				tc.gvr,
 				tc.manifestObj, tc.inMemberClusterObj,
 				tc.applyStrategy,
-				tc.expectedAppliedWorkOwnerRef)
+				tc.expectedAppliedWorkOwnerRef,
+				"test-crp")
 			if tc.wantErred {
 				if err == nil {
 					t.Errorf("takeOverPreExistingObject() = nil, want erred")

pkg/controllers/workapplier/process.go

@@ -165,7 +165,10 @@ func (r *Reconciler) processOneManifest(
 	}
 
 	// Perform the apply op.
-	appliedObj, err := r.apply(ctx, bundle.gvr, bundle.manifestObj, bundle.inMemberClusterObj, work.Spec.ApplyStrategy, expectedAppliedWorkOwnerRef)
+	// Extract parent-CRP name from Work object labels for namespace affinity support
+	parentCRPName := getParentCRPNameFromWork(work)
+
+	appliedObj, err := r.apply(ctx, bundle.gvr, bundle.manifestObj, bundle.inMemberClusterObj, work.Spec.ApplyStrategy, expectedAppliedWorkOwnerRef, parentCRPName)
 	if err != nil {
 		bundle.applyOrReportDiffErr = fmt.Errorf("failed to apply the manifest: %w", err)
 		bundle.applyOrReportDiffResTyp = ApplyOrReportDiffResTypeFailedToApply
@@ -256,9 +259,12 @@ func (r *Reconciler) takeOverInMemberClusterObjectIfApplicable(
 
 	// Take over the object. Note that this steps adds only the owner reference; no other
 	// fields are modified (on the object from the member cluster).
+	// Extract parent-CRP name for namespace affinity support
+	parentCRPName := getParentCRPNameFromWork(work)
+
 	takenOverInMemberClusterObj, configDiffs, diffCalculatedInDegradedMode, err := r.takeOverPreExistingObject(ctx,
 		bundle.gvr, bundle.manifestObj, bundle.inMemberClusterObj,
-		work.Spec.ApplyStrategy, expectedAppliedWorkOwnerRef)
+		work.Spec.ApplyStrategy, expectedAppliedWorkOwnerRef, parentCRPName)
 	switch {
 	case err != nil:
 		// An unexpected error has occurred.
@@ -371,10 +377,14 @@ func (r *Reconciler) reportDiffOnlyIfApplicable(
 
 	// The object has been created in the member cluster; Fleet will calculate the configuration
 	// diffs between the manifest object and the object from the member cluster.
+	// Extract parent-CRP name for namespace affinity support
+	parentCRPName := getParentCRPNameFromWork(work)
+
 	configDiffs, diffCalculatedInDegradedMode, err := r.diffBetweenManifestAndInMemberClusterObjects(ctx,
 		bundle.gvr,
 		bundle.manifestObj, bundle.inMemberClusterObj,
-		work.Spec.ApplyStrategy.ComparisonOption)
+		work.Spec.ApplyStrategy.ComparisonOption,
+		parentCRPName)
 	switch {
 	case err != nil:
 		// Failed to calculate the configuration diffs.
@@ -463,10 +473,14 @@ func (r *Reconciler) performPreApplyDriftDetectionIfApplicable(
 		return false
 	default:
 		// Run the drift detection process.
+		// Extract parent-CRP name for namespace affinity support
+		parentCRPName := getParentCRPNameFromWork(work)
+
 		drifts, driftsCalculatedInDegradedMode, err := r.diffBetweenManifestAndInMemberClusterObjects(ctx,
 			bundle.gvr,
 			bundle.manifestObj, bundle.inMemberClusterObj,
-			work.Spec.ApplyStrategy.ComparisonOption)
+			work.Spec.ApplyStrategy.ComparisonOption,
+			parentCRPName)
 		switch {
 		case err != nil:
 			// An unexpected error has occurred.
@@ -546,10 +560,14 @@ func (r *Reconciler) performPostApplyDriftDetectionIfApplicable(
 		return false
 	}
 
+	// Extract parent-CRP name for namespace affinity support
+	parentCRPName := getParentCRPNameFromWork(work)
+
 	drifts, driftsCalculatedInDegradedMode, err := r.diffBetweenManifestAndInMemberClusterObjects(ctx,
 		bundle.gvr,
 		bundle.manifestObj, bundle.inMemberClusterObj,
-		work.Spec.ApplyStrategy.ComparisonOption)
+		work.Spec.ApplyStrategy.ComparisonOption,
+		parentCRPName)
 	switch {
 	case err != nil:
 		// An unexpected error has occurred.
@@ -595,3 +613,12 @@ func shouldPerformPostApplyDriftDetection(applyStrategy *fleetv1beta1.ApplyStrat
 	// * The apply strategy dictates that drift detection should run in full comparison mode.
 	return applyStrategy.ComparisonOption == fleetv1beta1.ComparisonOptionTypeFullComparison
 }
+
+// getParentCRPNameFromWork extracts the parent CRP name from Work object labels.
+// Returns empty string if the label is not found.
+func getParentCRPNameFromWork(work *fleetv1beta1.Work) string {
+	if workLabels := work.GetLabels(); workLabels != nil {
+		return workLabels[fleetv1beta1.PlacementTrackingLabel]
+	}
+	return ""
+}