Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 45 additions & 2 deletions pkg/controllers/workapplier/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand All @@ -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(
Expand All @@ -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.
//
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
}
103 changes: 103 additions & 0 deletions pkg/controllers/workapplier/apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
}
9 changes: 6 additions & 3 deletions pkg/controllers/workapplier/drift_detection_takeover.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion pkg/controllers/workapplier/drift_detection_takeover_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
37 changes: 32 additions & 5 deletions pkg/controllers/workapplier/process.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 ""
}
Loading
Loading