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
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions apis/placement/v1beta1/work_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
40 changes: 40 additions & 0 deletions pkg/controllers/workapplier/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down
123 changes: 123 additions & 0 deletions pkg/controllers/workapplier/apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
}
2 changes: 1 addition & 1 deletion pkg/controllers/workapplier/process.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading