Skip to content

Commit 2fa52b9

Browse files
committed
feat: add source placement annotation to track workload source
- Add setSourcePlacementAnnotation function to apply.go to annotate manifests with their source placement information - Update process.go to call setSourcePlacementAnnotation during manifest processing - Add comprehensive unit tests following established testing patterns - Update common.go with source placement annotation constant
1 parent 57464c9 commit 2fa52b9

File tree

5 files changed

+231
-1
lines changed

5 files changed

+231
-1
lines changed
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
# Placement Source Annotation Implementation
2+
3+
## Context
4+
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.
5+
6+
## Analysis Phase
7+
8+
### Current Understanding
9+
- Work controller applies manifests to member clusters through `ApplyWorkReconciler`
10+
- Resources are applied via two strategies: client-side and server-side apply
11+
- Work objects contain a label `fleetv1beta1.CRPTrackingLabel` that tracks the placement
12+
- Applied resources currently get owner references but no source placement annotation
13+
14+
### Implementation Options
15+
16+
#### Option 1: Add annotation during manifest preparation in apply_controller.go
17+
- **Pros**: Centralized location, affects all appliers
18+
- **Cons**: Requires passing placement info through the call chain
19+
20+
#### Option 2: Add annotation in each applier implementation
21+
- **Pros**: More granular control, easier to implement
22+
- **Cons**: Need to modify both client-side and server-side appliers
23+
24+
#### Option 3: Add annotation in the applyManifests function
25+
- **Pros**: Single point of change, before any applier is called
26+
- **Cons**: Need to extract placement info from Work object
27+
28+
### Questions and Decisions
29+
1. **Annotation key**: Use `fleet.azure.com/source-placement` to follow existing pattern in `utils/common.go`
30+
2. **Value format**: Need both the name of the placement and its namespace as we want to support both ResourcePlacement and ClusterResourcePlacement.
31+
3. **Missing label handling**: Log warning and skip annotation if placement label is missing
32+
4. **Placement info location**: Available on Work objects via `fleetv1beta1.PlacementTrackingLabel` label
33+
34+
## Implementation Plan
35+
36+
### Phase 1: Define Annotation Constant
37+
38+
- [x] Add annotation key constant to APIs
39+
- [ ] Decide on annotation format and value
40+
41+
### Phase 2: Extract Placement Information
42+
- [x] Modify applyManifests to extract placement from Work labels
43+
- [x] Handle cases where placement label might be missing
44+
45+
### Phase 3: Add Annotation to Manifests
46+
- [x] Modify manifest objects before applying them
47+
- [x] Ensure annotation is added consistently across both applier types
48+
49+
### Phase 4: Testing
50+
- [x] Write unit tests for annotation addition
51+
- [ ] Test with both client-side and server-side apply strategies
52+
- [ ] Verify annotation appears on deployed resources
53+
54+
### Phase 5: Integration Testing
55+
- [ ] Test end-to-end with actual placements
56+
- [ ] Verify backwards compatibility
57+
58+
## Success Criteria
59+
- All applied resources have the source placement annotation
60+
- Annotation is correctly set for both apply strategies
61+
- No breaking changes to existing functionality
62+
- Tests pass and cover the new functionality

pkg/controllers/workapplier/apply.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import (
3535
"sigs.k8s.io/controller-runtime/pkg/client"
3636

3737
fleetv1beta1 "github.com/kubefleet-dev/kubefleet/apis/placement/v1beta1"
38+
"github.com/kubefleet-dev/kubefleet/pkg/utils"
3839
"github.com/kubefleet-dev/kubefleet/pkg/utils/controller"
3940
"github.com/kubefleet-dev/kubefleet/pkg/utils/resource"
4041
)
@@ -69,6 +70,7 @@ func (r *Reconciler) apply(
6970
manifestObj, inMemberClusterObj *unstructured.Unstructured,
7071
applyStrategy *fleetv1beta1.ApplyStrategy,
7172
expectedAppliedWorkOwnerRef *metav1.OwnerReference,
73+
work *fleetv1beta1.Work,
7274
) (*unstructured.Unstructured, error) {
7375
// Create a sanitized copy of the manifest object.
7476
//
@@ -96,6 +98,11 @@ func (r *Reconciler) apply(
9698
return nil, fmt.Errorf("failed to set manifest hash annotation: %w", err)
9799
}
98100

101+
// Add the source placement annotation to track the placement that created this resource.
102+
if err := setSourcePlacementAnnotation(manifestObjCopy, work); err != nil {
103+
return nil, fmt.Errorf("failed to set source placement annotation: %w", err)
104+
}
105+
99106
// Validate owner references.
100107
//
101108
// As previously mentioned, with the new capabilities, at this point of the workflow,
@@ -457,6 +464,40 @@ func setManifestHashAnnotation(manifestObj *unstructured.Unstructured) error {
457464
return nil
458465
}
459466

467+
// setSourcePlacementAnnotation sets the source placement annotation on the provided unstructured object.
468+
func setSourcePlacementAnnotation(manifestObj *unstructured.Unstructured, work *fleetv1beta1.Work) error {
469+
// Extract the placement name from the Work object's placement tracking label.
470+
workLabels := work.GetLabels()
471+
if workLabels == nil {
472+
// If there are no labels, we cannot determine the source placement.
473+
// This is not an error case as some Work objects might not have placement tracking.
474+
klog.V(2).InfoS("Work object has no labels, skipping source placement annotation",
475+
"work", klog.KObj(work))
476+
return nil
477+
}
478+
479+
placementName, exists := workLabels[fleetv1beta1.PlacementTrackingLabel]
480+
if !exists || placementName == "" {
481+
// If the placement tracking label is missing or empty, we cannot determine the source placement.
482+
// This is not an error case as some Work objects might not be created by a placement.
483+
klog.V(2).InfoS("Work object has no placement tracking label, skipping source placement annotation",
484+
"work", klog.KObj(work))
485+
return nil
486+
}
487+
488+
// Set the source placement annotation on the manifest object.
489+
annotations := manifestObj.GetAnnotations()
490+
if annotations == nil {
491+
annotations = map[string]string{}
492+
}
493+
annotations[utils.SourcePlacementAnnotation] = placementName
494+
manifestObj.SetAnnotations(annotations)
495+
496+
klog.V(2).InfoS("Set source placement annotation on manifest",
497+
"manifest", klog.KObj(manifestObj), "placement", placementName)
498+
return nil
499+
}
500+
460501
// setOwnerRef sets the expected owner reference (reference to an AppliedWork object)
461502
// on a manifest to be applied.
462503
func setOwnerRef(obj *unstructured.Unstructured, expectedAppliedWorkOwnerRef *metav1.OwnerReference) {

pkg/controllers/workapplier/apply_test.go

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
"sigs.k8s.io/controller-runtime/pkg/client"
3232

3333
fleetv1beta1 "github.com/kubefleet-dev/kubefleet/apis/placement/v1beta1"
34+
"github.com/kubefleet-dev/kubefleet/pkg/utils"
3435
)
3536

3637
// Note (chenyu1): The fake client Fleet uses for unit tests has trouble processing certain requests
@@ -550,3 +551,126 @@ func TestShouldUseForcedServerSideApply(t *testing.T) {
550551
})
551552
}
552553
}
554+
555+
// TestSetSourcePlacementAnnotation tests the setSourcePlacementAnnotation function.
556+
func TestSetSourcePlacementAnnotation(t *testing.T) {
557+
// Base namespace object to use for testing
558+
nsName := "ns-1"
559+
ns := &corev1.Namespace{
560+
TypeMeta: metav1.TypeMeta{
561+
APIVersion: "v1",
562+
Kind: "Namespace",
563+
},
564+
ObjectMeta: metav1.ObjectMeta{
565+
Name: nsName,
566+
},
567+
}
568+
569+
// Test case 1: Work with placement tracking label
570+
workWithPlacement := &fleetv1beta1.Work{
571+
ObjectMeta: metav1.ObjectMeta{
572+
Name: "test-work",
573+
Namespace: "test-namespace",
574+
Labels: map[string]string{
575+
fleetv1beta1.PlacementTrackingLabel: "test-placement",
576+
},
577+
},
578+
}
579+
manifestObj1 := toUnstructured(t, ns.DeepCopy())
580+
wantManifestObj1 := toUnstructured(t, ns.DeepCopy())
581+
wantManifestObj1.SetAnnotations(map[string]string{
582+
utils.SourcePlacementAnnotation: "test-placement",
583+
})
584+
585+
// Test case 2: Work without placement tracking label (no labels at all)
586+
workWithoutLabels := &fleetv1beta1.Work{
587+
ObjectMeta: metav1.ObjectMeta{
588+
Name: "test-work-no-labels",
589+
Namespace: "test-namespace",
590+
},
591+
}
592+
manifestObj2 := toUnstructured(t, ns.DeepCopy())
593+
wantManifestObj2 := toUnstructured(t, ns.DeepCopy())
594+
// No annotations should be added
595+
596+
// Test case 3: Work with empty placement tracking label
597+
workWithEmptyPlacement := &fleetv1beta1.Work{
598+
ObjectMeta: metav1.ObjectMeta{
599+
Name: "test-work-empty-placement",
600+
Namespace: "test-namespace",
601+
Labels: map[string]string{
602+
fleetv1beta1.PlacementTrackingLabel: "",
603+
},
604+
},
605+
}
606+
manifestObj3 := toUnstructured(t, ns.DeepCopy())
607+
wantManifestObj3 := toUnstructured(t, ns.DeepCopy())
608+
// No annotations should be added
609+
610+
// Test case 4: Manifest with existing annotations
611+
workWithPlacement2 := &fleetv1beta1.Work{
612+
ObjectMeta: metav1.ObjectMeta{
613+
Name: "test-work-2",
614+
Namespace: "test-namespace",
615+
Labels: map[string]string{
616+
fleetv1beta1.PlacementTrackingLabel: "another-placement",
617+
},
618+
},
619+
}
620+
manifestObj4 := toUnstructured(t, ns.DeepCopy())
621+
manifestObj4.SetAnnotations(map[string]string{
622+
"existing.annotation": "existing-value",
623+
"another.annotation": "another-value",
624+
})
625+
wantManifestObj4 := toUnstructured(t, ns.DeepCopy())
626+
wantManifestObj4.SetAnnotations(map[string]string{
627+
"existing.annotation": "existing-value",
628+
"another.annotation": "another-value",
629+
utils.SourcePlacementAnnotation: "another-placement",
630+
})
631+
632+
testCases := []struct {
633+
name string
634+
work *fleetv1beta1.Work
635+
manifestObj *unstructured.Unstructured
636+
wantManifestObj *unstructured.Unstructured
637+
}{
638+
{
639+
name: "work with placement tracking label",
640+
work: workWithPlacement,
641+
manifestObj: manifestObj1,
642+
wantManifestObj: wantManifestObj1,
643+
},
644+
{
645+
name: "work without labels",
646+
work: workWithoutLabels,
647+
manifestObj: manifestObj2,
648+
wantManifestObj: wantManifestObj2,
649+
},
650+
{
651+
name: "work with empty placement tracking label",
652+
work: workWithEmptyPlacement,
653+
manifestObj: manifestObj3,
654+
wantManifestObj: wantManifestObj3,
655+
},
656+
{
657+
name: "manifest with existing annotations",
658+
work: workWithPlacement2,
659+
manifestObj: manifestObj4,
660+
wantManifestObj: wantManifestObj4,
661+
},
662+
}
663+
664+
for _, tc := range testCases {
665+
t.Run(tc.name, func(t *testing.T) {
666+
err := setSourcePlacementAnnotation(tc.manifestObj, tc.work)
667+
if err != nil {
668+
t.Fatalf("setSourcePlacementAnnotation() = %v, want no error", err)
669+
}
670+
671+
if diff := cmp.Diff(tc.manifestObj, tc.wantManifestObj); diff != "" {
672+
t.Errorf("manifest obj mismatches (-got +want):\n%s", diff)
673+
}
674+
})
675+
}
676+
}

pkg/controllers/workapplier/process.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ func (r *Reconciler) processOneManifest(
107107
}
108108

109109
// Perform the apply op.
110-
appliedObj, err := r.apply(ctx, bundle.gvr, bundle.manifestObj, bundle.inMemberClusterObj, work.Spec.ApplyStrategy, expectedAppliedWorkOwnerRef)
110+
appliedObj, err := r.apply(ctx, bundle.gvr, bundle.manifestObj, bundle.inMemberClusterObj, work.Spec.ApplyStrategy, expectedAppliedWorkOwnerRef, work)
111111
if err != nil {
112112
bundle.applyErr = fmt.Errorf("failed to apply the manifest: %w", err)
113113
bundle.applyResTyp = ManifestProcessingApplyResultTypeFailedToApply

pkg/utils/common.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,9 @@ const (
109109

110110
// FleetAnnotationPrefix is the prefix used to annotate fleet member cluster resources.
111111
FleetAnnotationPrefix = "fleet.azure.com"
112+
113+
// SourcePlacementAnnotation is the annotation key used to track the source placement of applied resources.
114+
SourcePlacementAnnotation = FleetAnnotationPrefix + "/source-placement"
112115
)
113116

114117
var (

0 commit comments

Comments
 (0)