Skip to content

Commit 2c9e6e8

Browse files
scotwellsclaude
andcommitted
fix(federation): read ReferencedDataErrorAnnotation in selectWDBlockingCondition
In federated topology the cell WD never receives the ReferencedDataReady status condition written by the hub-side resolver (Karmada status aggregation is cell→hub only). The ReferencedDataErrorAnnotation written by #38 already bridges terminal errors hub→cell via ObjectMeta propagation; this commit teaches the cell WD reconciler to read it. selectWDBlockingCondition now checks deployment.Annotations for the terminal error annotation after the existing status-condition path. When present and parseable, decodeTerminalError (same package) returns the raw terminal reason (SourceNotFound / SourceUnauthorized / SourceTooLarge, all priority 5) which feeds directly into the existing consider() priority-ranked selection. The annotation path is evaluated before the propagation-lag check so a terminal annotation wins over the AwaitingPropagation reason at the same bucket. No changes to the federator merge logic or Workload controller are needed: once the cell WD Available carries the correct reason, Karmada statusAggregation carries it hub-ward and syncStatusFromDownstream copies it to the project WD as-is. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit ec668d9)
1 parent 0b11849 commit 2c9e6e8

2 files changed

Lines changed: 184 additions & 0 deletions

File tree

internal/controller/workloaddeployment_controller.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,21 @@ func selectWDBlockingCondition(
413413
consider(computev1alpha.WorkloadDeploymentReasonReferencedDataNotReady, wdRefDataCond.Message)
414414
}
415415

416+
// In federated topology the Karmada status-aggregation pathway carries status
417+
// cell→hub only, so the cell WD never receives the ReferencedDataReady condition
418+
// written by the hub-side resolver. The ReferencedDataErrorAnnotation bridges
419+
// terminal errors hub→cell alongside ObjectMeta (Karmada propagates annotations).
420+
// Read it here as a parallel path so the cell WD Available condition reflects the
421+
// terminal error even without the status condition. The annotation takes priority
422+
// over the propagation-lag check below when both could apply to the same bucket.
423+
if raw, ok := deployment.Annotations[computev1alpha.ReferencedDataErrorAnnotation]; ok && raw != "" {
424+
if reason, message, err := decodeTerminalError(raw); err == nil && reason != "" {
425+
consider(reason, message)
426+
}
427+
// Malformed annotation values are silently ignored: a parse failure here
428+
// should not block the WD from reporting whatever state it does know.
429+
}
430+
416431
if quotaBlockedReplicas > 0 {
417432
consider(computev1alpha.WorkloadDeploymentReasonQuotaNotGranted,
418433
fmt.Sprintf("%d of %d desired instances pending quota", quotaBlockedReplicas, desiredReplicas))

internal/controller/workloaddeployment_controller_test.go

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -863,3 +863,172 @@ func TestWDAvailableCondition_ObservedGeneration(t *testing.T) {
863863
require.NotNil(t, found)
864864
assert.Equal(t, gen, found.ObservedGeneration)
865865
}
866+
867+
// makeWDWithAnnotation builds a WorkloadDeployment carrying the given
868+
// ReferencedDataErrorAnnotation value but no status conditions, simulating a
869+
// cell WD copy that received the annotation from the hub via Karmada propagation
870+
// but has no locally-written resolver conditions.
871+
func makeWDWithAnnotation(generation int64, annotValue string) *computev1alpha.WorkloadDeployment {
872+
d := &computev1alpha.WorkloadDeployment{
873+
ObjectMeta: metav1.ObjectMeta{
874+
Name: wdControllerTestName,
875+
Namespace: wdControllerTestNS,
876+
Generation: generation,
877+
Annotations: map[string]string{
878+
computev1alpha.ReferencedDataErrorAnnotation: annotValue,
879+
},
880+
},
881+
}
882+
return d
883+
}
884+
885+
// mustEncodeTerminalError encodes a terminal error annotation value for use in
886+
// tests. It panics on encoding failure, which should never happen in tests.
887+
func mustEncodeTerminalError(reason, message string) string {
888+
v, err := encodeTerminalError(reason, message)
889+
if err != nil {
890+
panic(err)
891+
}
892+
return v
893+
}
894+
895+
// TestWDAvailableCondition_AnnotationSourceNotFound verifies that a cell WD
896+
// carrying the ReferencedDataErrorAnnotation (set by the hub-side resolver and
897+
// propagated by Karmada) promotes Available to False/SourceNotFound even when no
898+
// ReferencedDataReady status condition is present on the cell WD.
899+
//
900+
// This is the federation bridge introduced by task #40: the annotation is the
901+
// only terminal-error signal visible on the cell WD copy.
902+
func TestWDAvailableCondition_AnnotationSourceNotFound(t *testing.T) {
903+
const (
904+
gen = int64(3)
905+
desiredReplicas = int32(2)
906+
replicas = 2
907+
)
908+
annot := mustEncodeTerminalError(
909+
computev1alpha.ReferencedDataReasonSourceNotFound,
910+
testMsgConfigMapNotFound,
911+
)
912+
deployment := makeWDWithAnnotation(gen, annot)
913+
914+
cond := selectWDBlockingCondition(deployment, true, 0, 1, replicas, desiredReplicas)
915+
916+
assert.Equal(t, computev1alpha.WorkloadDeploymentAvailable, cond.Type)
917+
assert.Equal(t, metav1.ConditionFalse, cond.Status)
918+
// The annotation carries the raw terminal reason (SourceNotFound, priority 5),
919+
// which beats InstancesProvisioning (priority 1) and ReferencedDataNotReady (priority 4).
920+
assert.Equal(t, computev1alpha.ReferencedDataReasonSourceNotFound, cond.Reason)
921+
assert.Equal(t, testMsgConfigMapNotFound, cond.Message, "message must be the resolver message verbatim")
922+
assert.Equal(t, gen, cond.ObservedGeneration)
923+
}
924+
925+
// TestWDAvailableCondition_AnnotationAndConditionBothPresent verifies that when
926+
// both the annotation and the ReferencedDataReady=False status condition are
927+
// present (e.g. single-cluster mode, or a race during federation rollout), the
928+
// annotation path is taken at the same effective priority because both encode the
929+
// same terminal reason.
930+
func TestWDAvailableCondition_AnnotationAndConditionBothPresent(t *testing.T) {
931+
const (
932+
gen = int64(7)
933+
desiredReplicas = int32(1)
934+
replicas = 1
935+
)
936+
annot := mustEncodeTerminalError(
937+
computev1alpha.ReferencedDataReasonSourceNotFound,
938+
testMsgConfigMapNotFound,
939+
)
940+
// Stamp both the annotation and the status condition with matching reason+message.
941+
deployment := makeWDWithAnnotation(gen, annot)
942+
deployment.Status.Conditions = []metav1.Condition{
943+
{
944+
Type: computev1alpha.ReferencedDataReady,
945+
Status: metav1.ConditionFalse,
946+
Reason: computev1alpha.ReferencedDataReasonSourceNotFound,
947+
Message: testMsgConfigMapNotFound,
948+
LastTransitionTime: metav1.Now(),
949+
},
950+
}
951+
952+
cond := selectWDBlockingCondition(deployment, true, 0, 1, replicas, desiredReplicas)
953+
954+
assert.Equal(t, metav1.ConditionFalse, cond.Status)
955+
// Both paths arrive at the same terminal reason; the winner is stable regardless
956+
// of evaluation order since both carry priority 5.
957+
assert.Equal(t, computev1alpha.ReferencedDataReasonSourceNotFound, cond.Reason)
958+
assert.Equal(t, testMsgConfigMapNotFound, cond.Message)
959+
}
960+
961+
// TestWDAvailableCondition_AnnotationWinsOverQuota verifies that a terminal
962+
// annotation reason (priority 5) beats quotaBlockedReplicas (priority 3) even
963+
// when quota is also blocking.
964+
func TestWDAvailableCondition_AnnotationWinsOverQuota(t *testing.T) {
965+
const (
966+
gen = int64(2)
967+
desiredReplicas = int32(2)
968+
replicas = 2
969+
)
970+
annot := mustEncodeTerminalError(
971+
computev1alpha.ReferencedDataReasonSourceNotFound,
972+
testMsgConfigMapNotFound,
973+
)
974+
deployment := makeWDWithAnnotation(gen, annot)
975+
976+
// quotaBlockedReplicas=1 would normally surface QuotaNotGranted (priority 3).
977+
cond := selectWDBlockingCondition(deployment, true, 1, 0, replicas, desiredReplicas)
978+
979+
assert.Equal(t, computev1alpha.ReferencedDataReasonSourceNotFound, cond.Reason,
980+
"SourceNotFound (priority 5) must beat QuotaNotGranted (priority 3)")
981+
}
982+
983+
// TestWDAvailableCondition_NoAnnotationPropagationLag verifies that the existing
984+
// propagation-lag path (referencedDataBlockedReplicas > 0, no annotation, no
985+
// ReferencedDataReady condition) is unaffected by the annotation bridge change.
986+
func TestWDAvailableCondition_NoAnnotationPropagationLag(t *testing.T) {
987+
const (
988+
gen = int64(1)
989+
desiredReplicas = int32(2)
990+
replicas = 2
991+
)
992+
// No annotation, no ReferencedDataReady condition: companions still propagating.
993+
deployment := makeWDForAvailTest(gen, "", "", "")
994+
995+
cond := selectWDBlockingCondition(deployment, true, 0, 1, replicas, desiredReplicas)
996+
997+
assert.Equal(t, computev1alpha.WorkloadDeploymentReasonReferencedDataNotReady, cond.Reason,
998+
"propagation-lag path must still fire when annotation is absent")
999+
assert.Contains(t, cond.Message, "waiting for companion propagation")
1000+
}
1001+
1002+
// TestWDAvailableCondition_AnnotationEmptyString verifies that an empty annotation
1003+
// value is treated as absent and falls through to the existing logic.
1004+
func TestWDAvailableCondition_AnnotationEmptyString(t *testing.T) {
1005+
const (
1006+
gen = int64(1)
1007+
desiredReplicas = int32(1)
1008+
replicas = 1
1009+
)
1010+
deployment := makeWDWithAnnotation(gen, "")
1011+
1012+
cond := selectWDBlockingCondition(deployment, true, 0, 0, replicas, desiredReplicas)
1013+
1014+
// No real blockers; falls through to InstancesProvisioning.
1015+
assert.Equal(t, computev1alpha.WorkloadDeploymentReasonInstancesProvisioning, cond.Reason)
1016+
}
1017+
1018+
// TestWDAvailableCondition_AnnotationMalformedJSON verifies that a malformed
1019+
// annotation value is silently ignored (no panic) and the function falls through
1020+
// to the next applicable blocking reason.
1021+
func TestWDAvailableCondition_AnnotationMalformedJSON(t *testing.T) {
1022+
const (
1023+
gen = int64(1)
1024+
desiredReplicas = int32(1)
1025+
replicas = 1
1026+
)
1027+
deployment := makeWDWithAnnotation(gen, "not-valid-json{{")
1028+
1029+
// Should not panic; malformed annotation is skipped.
1030+
cond := selectWDBlockingCondition(deployment, true, 0, 0, replicas, desiredReplicas)
1031+
1032+
assert.Equal(t, computev1alpha.WorkloadDeploymentReasonInstancesProvisioning, cond.Reason,
1033+
"malformed annotation must be silently ignored; fallback to InstancesProvisioning")
1034+
}

0 commit comments

Comments
 (0)