diff --git a/.github/.copilot/breadcrumbs/2025-07-25-0800-placement-source-annotation.md b/.github/.copilot/breadcrumbs/2025-07-25-0800-placement-source-annotation.md new file mode 100644 index 000000000..e3e0ba934 --- /dev/null +++ b/.github/.copilot/breadcrumbs/2025-07-25-0800-placement-source-annotation.md @@ -0,0 +1,62 @@ +# Placement Source Annotation Implementation + +## Context +Add an annotation to all applied resources in the Work controller to track what source placement resource created them. This will help with debugging, resource tracking, and understanding the origin of deployed resources. + +## Analysis Phase + +### Current Understanding +- Work controller applies manifests to member clusters through `ApplyWorkReconciler` +- Resources are applied via two strategies: client-side and server-side apply +- Work objects contain a label `fleetv1beta1.CRPTrackingLabel` that tracks the placement +- Applied resources currently get owner references but no source placement annotation + +### Implementation Options + +#### Option 1: Add annotation during manifest preparation in apply_controller.go +- **Pros**: Centralized location, affects all appliers +- **Cons**: Requires passing placement info through the call chain + +#### Option 2: Add annotation in each applier implementation +- **Pros**: More granular control, easier to implement +- **Cons**: Need to modify both client-side and server-side appliers + +#### Option 3: Add annotation in the applyManifests function +- **Pros**: Single point of change, before any applier is called +- **Cons**: Need to extract placement info from Work object + +### Questions and Decisions +1. **Annotation key**: Use `fleet.azure.com/source-placement` to follow existing pattern in `utils/common.go` +2. **Value format**: Need both the name of the placement and its namespace as we want to support both ResourcePlacement and ClusterResourcePlacement. +3. **Missing label handling**: Log warning and skip annotation if placement label is missing +4. **Placement info location**: Available on Work objects via `fleetv1beta1.PlacementTrackingLabel` label + +## Implementation Plan + +### Phase 1: Define Annotation Constant + +- [x] Add annotation key constant to APIs +- [ ] Decide on annotation format and value + +### Phase 2: Extract Placement Information +- [x] Modify applyManifests to extract placement from Work labels +- [x] Handle cases where placement label might be missing + +### Phase 3: Add Annotation to Manifests +- [x] Modify manifest objects before applying them +- [x] Ensure annotation is added consistently across both applier types + +### Phase 4: Testing +- [x] Write unit tests for annotation addition +- [ ] Test with both client-side and server-side apply strategies +- [ ] Verify annotation appears on deployed resources + +### Phase 5: Integration Testing +- [ ] Test end-to-end with actual placements +- [ ] Verify backwards compatibility + +## Success Criteria +- All applied resources have the source placement annotation +- Annotation is correctly set for both apply strategies +- No breaking changes to existing functionality +- Tests pass and cover the new functionality diff --git a/apis/placement/v1beta1/work_types.go b/apis/placement/v1beta1/work_types.go index d1339ac7b..83e6c854d 100644 --- a/apis/placement/v1beta1/work_types.go +++ b/apis/placement/v1beta1/work_types.go @@ -45,6 +45,9 @@ const ( // LastAppliedConfigAnnotation is to record the last applied configuration on the object. LastAppliedConfigAnnotation = fleetPrefix + "last-applied-configuration" + // SourcePlacementAnnotation is the annotation key used to track the source placement of applied resources. + SourcePlacementAnnotation = fleetPrefix + "source-placement" + // WorkConditionTypeApplied represents workload in Work is applied successfully on the spoke cluster. WorkConditionTypeApplied = "Applied" diff --git a/pkg/controllers/workapplier/apply.go b/pkg/controllers/workapplier/apply.go index 4e447e4bf..2d8a2b122 100644 --- a/pkg/controllers/workapplier/apply.go +++ b/pkg/controllers/workapplier/apply.go @@ -69,6 +69,7 @@ func (r *Reconciler) apply( manifestObj, inMemberClusterObj *unstructured.Unstructured, applyStrategy *fleetv1beta1.ApplyStrategy, expectedAppliedWorkOwnerRef *metav1.OwnerReference, + work *fleetv1beta1.Work, ) (*unstructured.Unstructured, error) { // Create a sanitized copy of the manifest object. // @@ -96,6 +97,11 @@ func (r *Reconciler) apply( return nil, fmt.Errorf("failed to set manifest hash annotation: %w", err) } + // Add the source placement annotation to track the placement that created this resource. + if err := setSourcePlacementAnnotation(manifestObjCopy, work); err != nil { + return nil, fmt.Errorf("failed to set source placement annotation: %w", err) + } + // Validate owner references. // // As previously mentioned, with the new capabilities, at this point of the workflow, @@ -457,6 +463,40 @@ func setManifestHashAnnotation(manifestObj *unstructured.Unstructured) error { return nil } +// setSourcePlacementAnnotation sets the source placement annotation on the provided unstructured object. +func setSourcePlacementAnnotation(manifestObj *unstructured.Unstructured, work *fleetv1beta1.Work) error { + // Extract the placement name from the Work object's placement tracking label. + workLabels := work.GetLabels() + if workLabels == nil { + // If there are no labels, we cannot determine the source placement. + // This is not an error case as some Work objects might not have placement tracking. + klog.V(2).InfoS("Work object has no labels, skipping source placement annotation", + "work", klog.KObj(work)) + return nil + } + + placementName, exists := workLabels[fleetv1beta1.PlacementTrackingLabel] + if !exists || placementName == "" { + // If the placement tracking label is missing or empty, we cannot determine the source placement. + // This is not an error case as some Work objects might not be created by a placement. + klog.V(2).InfoS("Work object has no placement tracking label, skipping source placement annotation", + "work", klog.KObj(work)) + return nil + } + + // Set the source placement annotation on the manifest object. + annotations := manifestObj.GetAnnotations() + if annotations == nil { + annotations = map[string]string{} + } + annotations[fleetv1beta1.SourcePlacementAnnotation] = placementName + manifestObj.SetAnnotations(annotations) + + klog.V(2).InfoS("Set source placement annotation on manifest", + "manifest", klog.KObj(manifestObj), "placement", placementName) + return nil +} + // setOwnerRef sets the expected owner reference (reference to an AppliedWork object) // on a manifest to be applied. func setOwnerRef(obj *unstructured.Unstructured, expectedAppliedWorkOwnerRef *metav1.OwnerReference) { diff --git a/pkg/controllers/workapplier/apply_test.go b/pkg/controllers/workapplier/apply_test.go index f73bac0bf..0abcab488 100644 --- a/pkg/controllers/workapplier/apply_test.go +++ b/pkg/controllers/workapplier/apply_test.go @@ -550,3 +550,126 @@ func TestShouldUseForcedServerSideApply(t *testing.T) { }) } } + +// TestSetSourcePlacementAnnotation tests the setSourcePlacementAnnotation function. +func TestSetSourcePlacementAnnotation(t *testing.T) { + // Base namespace object to use for testing + nsName := "ns-1" + ns := &corev1.Namespace{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "Namespace", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: nsName, + }, + } + + // Test case 1: Work with placement tracking label + workWithPlacement := &fleetv1beta1.Work{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-work", + Namespace: "test-namespace", + Labels: map[string]string{ + fleetv1beta1.PlacementTrackingLabel: "test-placement", + }, + }, + } + manifestObj1 := toUnstructured(t, ns.DeepCopy()) + wantManifestObj1 := toUnstructured(t, ns.DeepCopy()) + wantManifestObj1.SetAnnotations(map[string]string{ + fleetv1beta1.SourcePlacementAnnotation: "test-placement", + }) + + // Test case 2: Work without placement tracking label (no labels at all) + workWithoutLabels := &fleetv1beta1.Work{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-work-no-labels", + Namespace: "test-namespace", + }, + } + manifestObj2 := toUnstructured(t, ns.DeepCopy()) + wantManifestObj2 := toUnstructured(t, ns.DeepCopy()) + // No annotations should be added + + // Test case 3: Work with empty placement tracking label + workWithEmptyPlacement := &fleetv1beta1.Work{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-work-empty-placement", + Namespace: "test-namespace", + Labels: map[string]string{ + fleetv1beta1.PlacementTrackingLabel: "", + }, + }, + } + manifestObj3 := toUnstructured(t, ns.DeepCopy()) + wantManifestObj3 := toUnstructured(t, ns.DeepCopy()) + // No annotations should be added + + // Test case 4: Manifest with existing annotations + workWithPlacement2 := &fleetv1beta1.Work{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-work-2", + Namespace: "test-namespace", + Labels: map[string]string{ + fleetv1beta1.PlacementTrackingLabel: "another-placement", + }, + }, + } + manifestObj4 := toUnstructured(t, ns.DeepCopy()) + manifestObj4.SetAnnotations(map[string]string{ + "existing.annotation": "existing-value", + "another.annotation": "another-value", + }) + wantManifestObj4 := toUnstructured(t, ns.DeepCopy()) + wantManifestObj4.SetAnnotations(map[string]string{ + "existing.annotation": "existing-value", + "another.annotation": "another-value", + fleetv1beta1.SourcePlacementAnnotation: "another-placement", + }) + + testCases := []struct { + name string + work *fleetv1beta1.Work + manifestObj *unstructured.Unstructured + wantManifestObj *unstructured.Unstructured + }{ + { + name: "work with placement tracking label", + work: workWithPlacement, + manifestObj: manifestObj1, + wantManifestObj: wantManifestObj1, + }, + { + name: "work without labels", + work: workWithoutLabels, + manifestObj: manifestObj2, + wantManifestObj: wantManifestObj2, + }, + { + name: "work with empty placement tracking label", + work: workWithEmptyPlacement, + manifestObj: manifestObj3, + wantManifestObj: wantManifestObj3, + }, + { + name: "manifest with existing annotations", + work: workWithPlacement2, + manifestObj: manifestObj4, + wantManifestObj: wantManifestObj4, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := setSourcePlacementAnnotation(tc.manifestObj, tc.work) + if err != nil { + t.Fatalf("setSourcePlacementAnnotation() = %v, want no error", err) + } + + if diff := cmp.Diff(tc.manifestObj, tc.wantManifestObj); diff != "" { + t.Errorf("manifest obj mismatches (-got +want):\n%s", diff) + } + }) + } +} diff --git a/pkg/controllers/workapplier/process.go b/pkg/controllers/workapplier/process.go index 646433b1f..ae427cfb6 100644 --- a/pkg/controllers/workapplier/process.go +++ b/pkg/controllers/workapplier/process.go @@ -107,7 +107,7 @@ func (r *Reconciler) processOneManifest( } // Perform the apply op. - appliedObj, err := r.apply(ctx, bundle.gvr, bundle.manifestObj, bundle.inMemberClusterObj, work.Spec.ApplyStrategy, expectedAppliedWorkOwnerRef) + appliedObj, err := r.apply(ctx, bundle.gvr, bundle.manifestObj, bundle.inMemberClusterObj, work.Spec.ApplyStrategy, expectedAppliedWorkOwnerRef, work) if err != nil { bundle.applyOrReportDiffErr = fmt.Errorf("failed to apply the manifest: %w", err) bundle.applyOrReportDiffResTyp = ApplyOrReportDiffResTypeFailedToApply