Skip to content

Commit 0af1ecd

Browse files
scotwellsclaude
andcommitted
fix: read Karmada PP name from ResourceBinding annotations, not labels
OrphanRBReconciler scoped referenced-data companion ResourceBindings by reading propagationpolicy.karmada.io/name from metadata.labels, but the running Karmada version stores that value in metadata.annotations (labels carry only permanent-id UUIDs). The label read always returned "", so the scope predicate rejected every ResourceBinding and the controller never reclaimed an orphaned binding — confirmed live in the lab, where the stranded cm-pristine/secret-pristine copies were never cleaned. Read from annotations in both isInScope and the watch predicate. The unit-test fixtures previously set the PP name as a label (matching the bug, which is why they passed against broken code); they now use the production annotation shape, so they fail against the label read and pass against the fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit e321370)
1 parent cebf0c1 commit 0af1ecd

2 files changed

Lines changed: 46 additions & 19 deletions

File tree

internal/controller/orphan_rb_controller.go

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,17 @@ const (
2525
// ResourceBindings whose hub companion was deleted before the controller started.
2626
orphanRBSweepInterval = 5 * time.Minute
2727

28-
// ppNameLabelKey is the label Karmada's binding-controller stamps on every
29-
// ResourceBinding to link it back to its governing PropagationPolicy.
30-
ppNameLabelKey = "propagationpolicy.karmada.io/name"
28+
// ppNameAnnotationKey is the annotation Karmada's binding-controller stamps on
29+
// every ResourceBinding to link it back to its governing PropagationPolicy.
30+
// Despite the "name" in the key string, Karmada stores this value in
31+
// metadata.annotations, NOT metadata.labels (labels carry only permanent-id
32+
// UUIDs). Reading from labels always returns "" and breaks scope filtering.
33+
ppNameAnnotationKey = "propagationpolicy.karmada.io/name"
3134

3235
// cityPPPrefix is the prefix of PropagationPolicy names created by
3336
// propagationPolicyNameFor in workloaddeployment_federator.go. RBs whose
34-
// PP label starts with this prefix were created for referenced-data companion
35-
// propagation.
37+
// PP annotation starts with this prefix were created for referenced-data
38+
// companion propagation.
3639
cityPPPrefix = "city-"
3740
)
3841

@@ -49,8 +52,9 @@ const (
4952
// Only ResourceBindings satisfying ALL three conditions are ever deleted:
5053
// 1. Name ends with "-configmap" or "-secret" (Karmada's kind-suffix for
5154
// namespace-scoped ConfigMap/Secret RBs — WD RBs end in "-workloaddeployment").
52-
// 2. The propagationpolicy.karmada.io/name label starts with "city-" (all
53-
// referenced-data PropagationPolicies use this prefix).
55+
// 2. The propagationpolicy.karmada.io/name annotation starts with "city-" (all
56+
// referenced-data PropagationPolicies use this prefix; Karmada stores this
57+
// in annotations, not labels).
5458
// 3. The hub companion derived by stripping the kind suffix does NOT exist
5559
// in the same namespace (and has no deletionTimestamp — a terminating
5660
// companion means Karmada cascade is still in progress).
@@ -116,9 +120,9 @@ func (r *OrphanRBReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
116120
}
117121

118122
// isInScope returns true when the ResourceBinding falls within the tight scope
119-
// of referenced-data companion RBs: city-PP label AND kind suffix.
123+
// of referenced-data companion RBs: city-PP annotation AND kind suffix.
120124
func (r *OrphanRBReconciler) isInScope(rb *karmadaworkv1alpha2.ResourceBinding) bool {
121-
ppName := rb.Labels[ppNameLabelKey]
125+
ppName := rb.Annotations[ppNameAnnotationKey]
122126
if !strings.HasPrefix(ppName, cityPPPrefix) {
123127
return false
124128
}
@@ -187,7 +191,7 @@ func (r *OrphanRBReconciler) SetupWithManager(mgr manager.Manager) error {
187191
return ctrl.NewControllerManagedBy(mgr).
188192
For(&karmadaworkv1alpha2.ResourceBinding{},
189193
builder.WithPredicates(predicate.NewPredicateFuncs(func(obj client.Object) bool {
190-
ppName := obj.GetLabels()[ppNameLabelKey]
194+
ppName := obj.GetAnnotations()[ppNameAnnotationKey]
191195
if !strings.HasPrefix(ppName, cityPPPrefix) {
192196
return false
193197
}

internal/controller/orphan_rb_controller_test.go

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,14 @@ const (
2525
// orbTestNS is the hub namespace where companion RBs and their companions live.
2626
orbTestNS = "ns-efdf8ca1-9c2d-4ac8-b161-1951503a2879"
2727

28-
// orbCityPPName is the PropagationPolicy name label used on companion RBs.
28+
// orbCityPPName is the PropagationPolicy name stored in metadata.annotations
29+
// on companion RBs. Karmada writes it to annotations, not labels (labels only
30+
// carry permanent-id UUIDs).
2931
orbCityPPName = "city-dfw"
32+
33+
// Karmada permanent-id label keys — present in metadata.labels on production RBs.
34+
orbPPPermanentIDKey = "propagationpolicy.karmada.io/permanent-id"
35+
orbRBPermanentIDKey = "resourcebinding.karmada.io/permanent-id"
3036
)
3137

3238
// ─── Helpers ──────────────────────────────────────────────────────────────────
@@ -40,15 +46,22 @@ func newOrphanRBReconciler(objs ...client.Object) (*OrphanRBReconciler, client.C
4046
return &OrphanRBReconciler{HubClient: hubCl}, hubCl
4147
}
4248

43-
// makeCompanionRB returns a ResourceBinding that represents an in-scope
44-
// companion RB (city-PP label + configmap/secret name suffix). The RB is
45-
// placed in orbTestNS.
49+
// makeCompanionRB returns a ResourceBinding that mirrors the production shape
50+
// observed in the lab: PP name in metadata.annotations (NOT labels), and
51+
// permanent-id UUIDs in labels. This matches what Karmada's binding-controller
52+
// actually stamps on each ResourceBinding.
4653
func makeCompanionRB(name string) *karmadaworkv1alpha2.ResourceBinding {
4754
return &karmadaworkv1alpha2.ResourceBinding{
4855
ObjectMeta: metav1.ObjectMeta{
4956
Namespace: orbTestNS,
5057
Name: name,
51-
Labels: map[string]string{ppNameLabelKey: orbCityPPName},
58+
Labels: map[string]string{
59+
orbPPPermanentIDKey: "a3d5b1ac-0000-0000-0000-000000000001",
60+
orbRBPermanentIDKey: "ee4b0939-0000-0000-0000-000000000001",
61+
},
62+
Annotations: map[string]string{
63+
ppNameAnnotationKey: orbCityPPName,
64+
},
5265
},
5366
}
5467
}
@@ -153,12 +166,17 @@ func TestOrphanRB_SkipsTerminatingCompanion(t *testing.T) {
153166
func TestOrphanRB_IgnoresNonCityPPRB(t *testing.T) {
154167
t.Parallel()
155168

156-
// RB has a non-city PP label (e.g. a WD's PP or an unrelated tenant's PP).
169+
// RB has a non-city PP annotation (e.g. a WD's PP or an unrelated tenant's PP).
170+
// Production shape: PP name in annotations, permanent-ids in labels.
157171
nonCityRB := &karmadaworkv1alpha2.ResourceBinding{
158172
ObjectMeta: metav1.ObjectMeta{
159173
Namespace: orbTestNS,
160174
Name: "cm-other-configmap",
161-
Labels: map[string]string{ppNameLabelKey: "other-pp"},
175+
Labels: map[string]string{
176+
"propagationpolicy.karmada.io/permanent-id": "a3d5b1ac-0000-0000-0000-000000000099",
177+
"resourcebinding.karmada.io/permanent-id": "ee4b0939-0000-0000-0000-000000000099",
178+
},
179+
Annotations: map[string]string{ppNameAnnotationKey: "other-pp"},
162180
},
163181
}
164182
r, hubCl := newOrphanRBReconciler(nonCityRB)
@@ -179,12 +197,17 @@ func TestOrphanRB_IgnoresNonCityPPRB(t *testing.T) {
179197
func TestOrphanRB_IgnoresWorkloadDeploymentRB(t *testing.T) {
180198
t.Parallel()
181199

182-
// WD RB: correct city- PP label but wrong kind suffix.
200+
// WD RB: correct city- PP annotation but wrong kind suffix ("-workloaddeployment"
201+
// instead of "-configmap"/"-secret"). Production shape: PP name in annotations.
183202
wdRB := &karmadaworkv1alpha2.ResourceBinding{
184203
ObjectMeta: metav1.ObjectMeta{
185204
Namespace: orbTestNS,
186205
Name: "my-workload-workloaddeployment",
187-
Labels: map[string]string{ppNameLabelKey: "city-dfw"},
206+
Labels: map[string]string{
207+
"propagationpolicy.karmada.io/permanent-id": "a3d5b1ac-0000-0000-0000-000000000088",
208+
"resourcebinding.karmada.io/permanent-id": "ee4b0939-0000-0000-0000-000000000088",
209+
},
210+
Annotations: map[string]string{ppNameAnnotationKey: "city-dfw"},
188211
},
189212
}
190213
r, hubCl := newOrphanRBReconciler(wdRB)

0 commit comments

Comments
 (0)