Skip to content

Commit 4ff9558

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 f06856c commit 4ff9558

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
@@ -418,6 +418,21 @@ func selectWDBlockingCondition(
418418
consider(computev1alpha.WorkloadDeploymentReasonReferencedDataNotReady, wdRefDataCond.Message)
419419
}
420420

421+
// In federated topology the Karmada status-aggregation pathway carries status
422+
// cell→hub only, so the cell WD never receives the ReferencedDataReady condition
423+
// written by the hub-side resolver. The ReferencedDataErrorAnnotation bridges
424+
// terminal errors hub→cell alongside ObjectMeta (Karmada propagates annotations).
425+
// Read it here as a parallel path so the cell WD Available condition reflects the
426+
// terminal error even without the status condition. The annotation takes priority
427+
// over the propagation-lag check below when both could apply to the same bucket.
428+
if raw, ok := deployment.Annotations[computev1alpha.ReferencedDataErrorAnnotation]; ok && raw != "" {
429+
if reason, message, err := decodeTerminalError(raw); err == nil && reason != "" {
430+
consider(reason, message)
431+
}
432+
// Malformed annotation values are silently ignored: a parse failure here
433+
// should not block the WD from reporting whatever state it does know.
434+
}
435+
421436
if quotaBlockedReplicas > 0 {
422437
consider(computev1alpha.WorkloadDeploymentReasonQuotaNotGranted,
423438
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
@@ -655,3 +655,172 @@ func TestWDAvailableCondition_ObservedGeneration(t *testing.T) {
655655
require.NotNil(t, found)
656656
assert.Equal(t, gen, found.ObservedGeneration)
657657
}
658+
659+
// makeWDWithAnnotation builds a WorkloadDeployment carrying the given
660+
// ReferencedDataErrorAnnotation value but no status conditions, simulating a
661+
// cell WD copy that received the annotation from the hub via Karmada propagation
662+
// but has no locally-written resolver conditions.
663+
func makeWDWithAnnotation(generation int64, annotValue string) *computev1alpha.WorkloadDeployment {
664+
d := &computev1alpha.WorkloadDeployment{
665+
ObjectMeta: metav1.ObjectMeta{
666+
Name: wdControllerTestName,
667+
Namespace: wdControllerTestNS,
668+
Generation: generation,
669+
Annotations: map[string]string{
670+
computev1alpha.ReferencedDataErrorAnnotation: annotValue,
671+
},
672+
},
673+
}
674+
return d
675+
}
676+
677+
// mustEncodeTerminalError encodes a terminal error annotation value for use in
678+
// tests. It panics on encoding failure, which should never happen in tests.
679+
func mustEncodeTerminalError(reason, message string) string {
680+
v, err := encodeTerminalError(reason, message)
681+
if err != nil {
682+
panic(err)
683+
}
684+
return v
685+
}
686+
687+
// TestWDAvailableCondition_AnnotationSourceNotFound verifies that a cell WD
688+
// carrying the ReferencedDataErrorAnnotation (set by the hub-side resolver and
689+
// propagated by Karmada) promotes Available to False/SourceNotFound even when no
690+
// ReferencedDataReady status condition is present on the cell WD.
691+
//
692+
// This is the federation bridge introduced by task #40: the annotation is the
693+
// only terminal-error signal visible on the cell WD copy.
694+
func TestWDAvailableCondition_AnnotationSourceNotFound(t *testing.T) {
695+
const (
696+
gen = int64(3)
697+
desiredReplicas = int32(2)
698+
replicas = 2
699+
)
700+
annot := mustEncodeTerminalError(
701+
computev1alpha.ReferencedDataReasonSourceNotFound,
702+
testMsgConfigMapNotFound,
703+
)
704+
deployment := makeWDWithAnnotation(gen, annot)
705+
706+
cond := selectWDBlockingCondition(deployment, true, 0, 1, replicas, desiredReplicas)
707+
708+
assert.Equal(t, computev1alpha.WorkloadDeploymentAvailable, cond.Type)
709+
assert.Equal(t, metav1.ConditionFalse, cond.Status)
710+
// The annotation carries the raw terminal reason (SourceNotFound, priority 5),
711+
// which beats InstancesProvisioning (priority 1) and ReferencedDataNotReady (priority 4).
712+
assert.Equal(t, computev1alpha.ReferencedDataReasonSourceNotFound, cond.Reason)
713+
assert.Equal(t, testMsgConfigMapNotFound, cond.Message, "message must be the resolver message verbatim")
714+
assert.Equal(t, gen, cond.ObservedGeneration)
715+
}
716+
717+
// TestWDAvailableCondition_AnnotationAndConditionBothPresent verifies that when
718+
// both the annotation and the ReferencedDataReady=False status condition are
719+
// present (e.g. single-cluster mode, or a race during federation rollout), the
720+
// annotation path is taken at the same effective priority because both encode the
721+
// same terminal reason.
722+
func TestWDAvailableCondition_AnnotationAndConditionBothPresent(t *testing.T) {
723+
const (
724+
gen = int64(7)
725+
desiredReplicas = int32(1)
726+
replicas = 1
727+
)
728+
annot := mustEncodeTerminalError(
729+
computev1alpha.ReferencedDataReasonSourceNotFound,
730+
testMsgConfigMapNotFound,
731+
)
732+
// Stamp both the annotation and the status condition with matching reason+message.
733+
deployment := makeWDWithAnnotation(gen, annot)
734+
deployment.Status.Conditions = []metav1.Condition{
735+
{
736+
Type: computev1alpha.ReferencedDataReady,
737+
Status: metav1.ConditionFalse,
738+
Reason: computev1alpha.ReferencedDataReasonSourceNotFound,
739+
Message: testMsgConfigMapNotFound,
740+
LastTransitionTime: metav1.Now(),
741+
},
742+
}
743+
744+
cond := selectWDBlockingCondition(deployment, true, 0, 1, replicas, desiredReplicas)
745+
746+
assert.Equal(t, metav1.ConditionFalse, cond.Status)
747+
// Both paths arrive at the same terminal reason; the winner is stable regardless
748+
// of evaluation order since both carry priority 5.
749+
assert.Equal(t, computev1alpha.ReferencedDataReasonSourceNotFound, cond.Reason)
750+
assert.Equal(t, testMsgConfigMapNotFound, cond.Message)
751+
}
752+
753+
// TestWDAvailableCondition_AnnotationWinsOverQuota verifies that a terminal
754+
// annotation reason (priority 5) beats quotaBlockedReplicas (priority 3) even
755+
// when quota is also blocking.
756+
func TestWDAvailableCondition_AnnotationWinsOverQuota(t *testing.T) {
757+
const (
758+
gen = int64(2)
759+
desiredReplicas = int32(2)
760+
replicas = 2
761+
)
762+
annot := mustEncodeTerminalError(
763+
computev1alpha.ReferencedDataReasonSourceNotFound,
764+
testMsgConfigMapNotFound,
765+
)
766+
deployment := makeWDWithAnnotation(gen, annot)
767+
768+
// quotaBlockedReplicas=1 would normally surface QuotaNotGranted (priority 3).
769+
cond := selectWDBlockingCondition(deployment, true, 1, 0, replicas, desiredReplicas)
770+
771+
assert.Equal(t, computev1alpha.ReferencedDataReasonSourceNotFound, cond.Reason,
772+
"SourceNotFound (priority 5) must beat QuotaNotGranted (priority 3)")
773+
}
774+
775+
// TestWDAvailableCondition_NoAnnotationPropagationLag verifies that the existing
776+
// propagation-lag path (referencedDataBlockedReplicas > 0, no annotation, no
777+
// ReferencedDataReady condition) is unaffected by the annotation bridge change.
778+
func TestWDAvailableCondition_NoAnnotationPropagationLag(t *testing.T) {
779+
const (
780+
gen = int64(1)
781+
desiredReplicas = int32(2)
782+
replicas = 2
783+
)
784+
// No annotation, no ReferencedDataReady condition: companions still propagating.
785+
deployment := makeWDForAvailTest(gen, "", "", "")
786+
787+
cond := selectWDBlockingCondition(deployment, true, 0, 1, replicas, desiredReplicas)
788+
789+
assert.Equal(t, computev1alpha.WorkloadDeploymentReasonReferencedDataNotReady, cond.Reason,
790+
"propagation-lag path must still fire when annotation is absent")
791+
assert.Contains(t, cond.Message, "waiting for companion propagation")
792+
}
793+
794+
// TestWDAvailableCondition_AnnotationEmptyString verifies that an empty annotation
795+
// value is treated as absent and falls through to the existing logic.
796+
func TestWDAvailableCondition_AnnotationEmptyString(t *testing.T) {
797+
const (
798+
gen = int64(1)
799+
desiredReplicas = int32(1)
800+
replicas = 1
801+
)
802+
deployment := makeWDWithAnnotation(gen, "")
803+
804+
cond := selectWDBlockingCondition(deployment, true, 0, 0, replicas, desiredReplicas)
805+
806+
// No real blockers; falls through to InstancesProvisioning.
807+
assert.Equal(t, computev1alpha.WorkloadDeploymentReasonInstancesProvisioning, cond.Reason)
808+
}
809+
810+
// TestWDAvailableCondition_AnnotationMalformedJSON verifies that a malformed
811+
// annotation value is silently ignored (no panic) and the function falls through
812+
// to the next applicable blocking reason.
813+
func TestWDAvailableCondition_AnnotationMalformedJSON(t *testing.T) {
814+
const (
815+
gen = int64(1)
816+
desiredReplicas = int32(1)
817+
replicas = 1
818+
)
819+
deployment := makeWDWithAnnotation(gen, "not-valid-json{{")
820+
821+
// Should not panic; malformed annotation is skipped.
822+
cond := selectWDBlockingCondition(deployment, true, 0, 0, replicas, desiredReplicas)
823+
824+
assert.Equal(t, computev1alpha.WorkloadDeploymentReasonInstancesProvisioning, cond.Reason,
825+
"malformed annotation must be silently ignored; fallback to InstancesProvisioning")
826+
}

0 commit comments

Comments
 (0)