Skip to content

Commit 4512bd7

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 46be837 commit 4512bd7

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
@@ -424,6 +424,21 @@ func selectWDBlockingCondition(
424424
consider(computev1alpha.WorkloadDeploymentReasonReferencedDataNotReady, wdRefDataCond.Message)
425425
}
426426

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

0 commit comments

Comments
 (0)