Skip to content

Commit d48f490

Browse files
scotwellsclaude
andcommitted
fix(federation): bridge terminal referenced-data error hub→cell via annotation
The referenced-data resolver sets ReferencedDataReady=False/SourceNotFound on the hub WorkloadDeployment, but Karmada statusAggregation only flows cell->hub, so the cell Instance controller (which reads the cell WD copy) never saw the verdict and the Instance stayed Ready=Unknown/Resolving instead of Ready=False/SourceNotFound. Bridge it via an annotation (compute.datumapis.com/referenced-data-error), which DOES propagate hub->cell as part of ObjectMeta. The resolver stamps it on terminal errors (SourceNotFound/TooLarge/Unauthorized) and clears it on resolve; the cell instance controller reads it from the owner WD and surfaces the terminal reason+ message, with a condition-based fallback preserved for single-cluster. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit 3751f78)
1 parent f46d986 commit d48f490

6 files changed

Lines changed: 570 additions & 156 deletions

api/v1alpha/annotations.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,25 @@ const (
2525
// gate. Its value is an RFC3339 timestamp. Used to compute gate-wait duration
2626
// for the compute_referenced_data_gate_wait_seconds histogram.
2727
ReferencedDataGateStartAnnotation = AnnotationNamespace + "/referenced-data-gate-start"
28+
29+
// ReferencedDataErrorAnnotation is stamped on a WorkloadDeployment by the
30+
// ReferencedDataController when a terminal source error occurs (SourceNotFound,
31+
// SourceUnauthorized, or SourceTooLarge). Its value is a JSON object with
32+
// "reason" and "message" fields carrying the authoritative resolver verdict.
33+
//
34+
// Example value:
35+
// {"reason":"SourceNotFound","message":"ConfigMap \"app-config\" not found in namespace \"default\""}
36+
//
37+
// This annotation bridges the federation boundary: Karmada propagates
38+
// metadata.annotations hub→cell alongside WorkloadDeployment objects, but
39+
// status.conditions do not propagate in that direction. The cell
40+
// InstanceReconciler reads this annotation from the cell WD copy (returned by
41+
// fetchOwnerWorkloadDeployment) and promotes it to the Instance's
42+
// ReferencedDataReady condition so the terminal error is visible at the Instance
43+
// level without requiring a cross-plane condition read.
44+
//
45+
// The annotation is removed when the error resolves (companion materialises /
46+
// ReferencedDataReady flips True), so the absence of the annotation means
47+
// either no error or the error has cleared.
48+
ReferencedDataErrorAnnotation = AnnotationNamespace + "/referenced-data-error"
2849
)

internal/controller/instance_controller.go

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,14 @@ func (r *InstanceReconciler) reconcileReferencedDataCondition(
501501
return referencedDataResult{}, 0, err
502502
}
503503

504+
// When the resolver has determined a terminal source error (the source object
505+
// is missing, unauthorized, or too large), the companion will never arrive.
506+
// Promote the terminal error to the Instance condition so it is visible without
507+
// a secondary fetch. Returns (changed, true) when a terminal error was found.
508+
if changed, terminal := r.applyTerminalErrorFromWD(ctx, wd, instance); terminal {
509+
return referencedDataResult{conditionChanged: changed}, 0, nil
510+
}
511+
504512
// Annotation not yet present — the resolver hasn't finished. Signal Resolving
505513
// and requeue so we re-check even without a new watch event.
506514
annoRaw, hasAnno := wd.Annotations[computev1alpha.ExpectedReferencedDataAnnotation]
@@ -613,6 +621,75 @@ func (r *InstanceReconciler) setReferencedDataConditionWithTransition(
613621
return changed
614622
}
615623

624+
// isTerminalReferencedDataReason reports whether the given ReferencedData reason
625+
// is terminal — i.e., the companion will never arrive because the source object
626+
// is permanently unavailable, not just slow to propagate.
627+
func isTerminalReferencedDataReason(reason string) bool {
628+
switch reason {
629+
case computev1alpha.ReferencedDataReasonSourceNotFound,
630+
computev1alpha.ReferencedDataReasonSourceUnauthorized,
631+
computev1alpha.ReferencedDataReasonSourceTooLarge:
632+
return true
633+
}
634+
return false
635+
}
636+
637+
// applyTerminalErrorFromWD reads the terminal-error signal from the owner WD and,
638+
// when a valid terminal error is found, sets the Instance's ReferencedDataReady
639+
// condition. Returns (changed, true) when a terminal error was applied, or
640+
// (false, false) when there is no terminal error to apply.
641+
//
642+
// The signal is read from two places in priority order:
643+
// 1. ReferencedDataErrorAnnotation on the WD metadata — the primary path in
644+
// federation, because Karmada propagates annotations hub→cell but does not
645+
// propagate status.conditions in that direction.
646+
// 2. The WD's ReferencedDataReady status condition — the fallback for
647+
// single-cluster deployments or during a controller version upgrade before
648+
// the annotation was introduced.
649+
func (r *InstanceReconciler) applyTerminalErrorFromWD(
650+
ctx context.Context,
651+
wd *computev1alpha.WorkloadDeployment,
652+
instance *computev1alpha.Instance,
653+
) (changed, terminal bool) {
654+
// Path 1: annotation (federation-safe).
655+
if termErrRaw := wd.Annotations[computev1alpha.ReferencedDataErrorAnnotation]; termErrRaw != "" {
656+
termReason, termMessage, decodeErr := decodeTerminalErrorAnnotation(termErrRaw)
657+
if decodeErr != nil {
658+
log.FromContext(ctx).V(1).Info("malformed referenced-data-error annotation on WD; ignoring",
659+
"workloadDeployment", wd.Name, "error", decodeErr)
660+
} else if isTerminalReferencedDataReason(termReason) {
661+
ch := r.setReferencedDataConditionWithTransition(instance, metav1.ConditionFalse,
662+
termReason, termMessage)
663+
return ch, true
664+
}
665+
}
666+
667+
// Path 2: status condition fallback (single-cluster).
668+
wdCond := apimeta.FindStatusCondition(wd.Status.Conditions, computev1alpha.ReferencedDataReady)
669+
if wdCond != nil && wdCond.Status == metav1.ConditionFalse && isTerminalReferencedDataReason(wdCond.Reason) {
670+
ch := r.setReferencedDataConditionWithTransition(instance, metav1.ConditionFalse,
671+
wdCond.Reason, wdCond.Message)
672+
return ch, true
673+
}
674+
675+
return false, false
676+
}
677+
678+
// decodeTerminalErrorAnnotation parses the value of ReferencedDataErrorAnnotation
679+
// into (reason, message). Returns an error when the annotation is malformed.
680+
// Both files in the same package share this unexported helper through Go's
681+
// package-level visibility — no cross-file coupling issue.
682+
func decodeTerminalErrorAnnotation(raw string) (reason, message string, err error) {
683+
var payload struct {
684+
Reason string `json:"reason"`
685+
Message string `json:"message"`
686+
}
687+
if err := json.Unmarshal([]byte(raw), &payload); err != nil {
688+
return "", "", fmt.Errorf("decode referenced-data-error annotation %q: %w", raw, err)
689+
}
690+
return payload.Reason, payload.Message, nil
691+
}
692+
616693
// listPresentCompanionsByKindName returns a set keyed by kind-qualified tokens
617694
// ("Kind/name", e.g. "ConfigMap/app-config") for every companion ConfigMap and
618695
// Secret present in the given namespace (matched by ReferencedDataLabel). This

internal/controller/instance_referenced_data_test.go

Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -564,6 +564,217 @@ func TestReferencedDataStaleConditionGuard(t *testing.T) {
564564
}
565565
}
566566

567+
// ─── Federation annotation bridge: terminal error propagation hub→cell ────────
568+
569+
// makeWDWithTerminalError returns a WD carrying the ReferencedDataErrorAnnotation
570+
// as a cell WD copy would after Karmada propagation from the hub.
571+
func makeWDWithTerminalError(reason, message string) *computev1alpha.WorkloadDeployment {
572+
raw, _ := encodeTerminalError(reason, message)
573+
wd := &computev1alpha.WorkloadDeployment{
574+
ObjectMeta: metav1.ObjectMeta{
575+
Name: refDataTestDeployment,
576+
Namespace: refDataTestNamespace,
577+
UID: refDataTestWDUID,
578+
Annotations: map[string]string{
579+
computev1alpha.ReferencedDataErrorAnnotation: raw,
580+
},
581+
},
582+
}
583+
return wd
584+
}
585+
586+
// TestFederated_TerminalAnnotation_SourceNotFound verifies that when the cell WD
587+
// copy carries the ReferencedDataErrorAnnotation with SourceNotFound, the Instance
588+
// gets Ready=False/SourceNotFound with the hub message rather than AwaitingPropagation.
589+
func TestFederated_TerminalAnnotation_SourceNotFound(t *testing.T) {
590+
msg := `ConfigMap "app-config" not found in namespace "default"`
591+
wd := makeWDWithTerminalError(computev1alpha.ReferencedDataReasonSourceNotFound, msg)
592+
inst := makeInstanceWithRefDataGate()
593+
594+
r, projectClient, _ := newRefDataReconciler(t, []client.Object{inst, wd})
595+
reconcileRefData(t, r)
596+
597+
var updated computev1alpha.Instance
598+
require.NoError(t, projectClient.Get(context.Background(),
599+
types.NamespacedName{Namespace: refDataTestNamespace, Name: refDataTestInstance}, &updated))
600+
601+
cond := apimeta.FindStatusCondition(updated.Status.Conditions, computev1alpha.ReferencedDataReady)
602+
require.NotNil(t, cond, "ReferencedDataReady condition should be set")
603+
assert.Equal(t, metav1.ConditionFalse, cond.Status)
604+
assert.Equal(t, computev1alpha.ReferencedDataReasonSourceNotFound, cond.Reason,
605+
"should use the hub resolver reason, not generic AwaitingPropagation")
606+
assert.Equal(t, msg, cond.Message,
607+
"should carry the hub resolver message verbatim")
608+
609+
// Gate must remain until the error is resolved (companion will never arrive).
610+
hasGate := false
611+
if updated.Spec.Controller != nil {
612+
for _, g := range updated.Spec.Controller.SchedulingGates {
613+
if g.Name == instancecontrol.ReferencedDataSchedulingGate.String() {
614+
hasGate = true
615+
}
616+
}
617+
}
618+
assert.True(t, hasGate, "ReferencedData gate should remain when a terminal error is present")
619+
}
620+
621+
// TestFederated_TerminalAnnotation_SourceTooLarge verifies SourceTooLarge propagates
622+
// via the annotation the same way as SourceNotFound.
623+
func TestFederated_TerminalAnnotation_SourceTooLarge(t *testing.T) {
624+
msg := `ConfigMap "fat-config" in namespace "default" exceeds per-object size limit`
625+
wd := makeWDWithTerminalError(computev1alpha.ReferencedDataReasonSourceTooLarge, msg)
626+
inst := makeInstanceWithRefDataGate()
627+
628+
r, projectClient, _ := newRefDataReconciler(t, []client.Object{inst, wd})
629+
reconcileRefData(t, r)
630+
631+
var updated computev1alpha.Instance
632+
require.NoError(t, projectClient.Get(context.Background(),
633+
types.NamespacedName{Namespace: refDataTestNamespace, Name: refDataTestInstance}, &updated))
634+
635+
cond := apimeta.FindStatusCondition(updated.Status.Conditions, computev1alpha.ReferencedDataReady)
636+
require.NotNil(t, cond)
637+
assert.Equal(t, metav1.ConditionFalse, cond.Status)
638+
assert.Equal(t, computev1alpha.ReferencedDataReasonSourceTooLarge, cond.Reason)
639+
assert.Equal(t, msg, cond.Message)
640+
}
641+
642+
// TestFederated_TerminalAnnotation_SourceUnauthorized verifies SourceUnauthorized propagates
643+
// via the annotation.
644+
func TestFederated_TerminalAnnotation_SourceUnauthorized(t *testing.T) {
645+
msg := `not authorized to read ConfigMap "secret-cfg" in namespace "default"`
646+
wd := makeWDWithTerminalError(computev1alpha.ReferencedDataReasonSourceUnauthorized, msg)
647+
inst := makeInstanceWithRefDataGate()
648+
649+
r, projectClient, _ := newRefDataReconciler(t, []client.Object{inst, wd})
650+
reconcileRefData(t, r)
651+
652+
var updated computev1alpha.Instance
653+
require.NoError(t, projectClient.Get(context.Background(),
654+
types.NamespacedName{Namespace: refDataTestNamespace, Name: refDataTestInstance}, &updated))
655+
656+
cond := apimeta.FindStatusCondition(updated.Status.Conditions, computev1alpha.ReferencedDataReady)
657+
require.NotNil(t, cond)
658+
assert.Equal(t, metav1.ConditionFalse, cond.Status)
659+
assert.Equal(t, computev1alpha.ReferencedDataReasonSourceUnauthorized, cond.Reason)
660+
assert.Equal(t, msg, cond.Message)
661+
}
662+
663+
// TestFederated_NoAnnotation_AnnotationAbsent verifies that when the WD has no
664+
// terminal-error annotation AND no expected-data annotation (resolver not done),
665+
// the Instance gets Resolving/Unknown — NOT SourceNotFound.
666+
func TestFederated_NoAnnotation_AnnotationAbsent(t *testing.T) {
667+
// WD has neither annotation — Karmada propagated it before the resolver ran.
668+
wd := makeWDForCell("") // no annotations
669+
inst := makeInstanceWithRefDataGate()
670+
671+
r, projectClient, _ := newRefDataReconciler(t, []client.Object{inst, wd})
672+
reconcileRefData(t, r)
673+
674+
var updated computev1alpha.Instance
675+
require.NoError(t, projectClient.Get(context.Background(),
676+
types.NamespacedName{Namespace: refDataTestNamespace, Name: refDataTestInstance}, &updated))
677+
678+
cond := apimeta.FindStatusCondition(updated.Status.Conditions, computev1alpha.ReferencedDataReady)
679+
require.NotNil(t, cond)
680+
assert.Equal(t, metav1.ConditionUnknown, cond.Status,
681+
"should be Unknown/Resolving when no annotation is present")
682+
assert.Equal(t, computev1alpha.ReferencedDataReasonResolving, cond.Reason)
683+
}
684+
685+
// TestFederated_AnnotationPresent_CompanionsMissing verifies that when the WD has
686+
// the expected-data annotation (resolver succeeded, no terminal error) but companions
687+
// haven't arrived yet, the Instance gets AwaitingPropagation — not SourceNotFound.
688+
func TestFederated_AnnotationPresent_CompanionsMissing(t *testing.T) {
689+
expected := []string{refDataTestCMToken}
690+
annoVal, _ := json.Marshal(expected)
691+
// WD has the expected annotation but NOT the terminal-error annotation.
692+
wd := makeWDForCell(string(annoVal))
693+
inst := makeInstanceWithRefDataGate()
694+
695+
// No companion ConfigMap present yet.
696+
r, projectClient, _ := newRefDataReconciler(t, []client.Object{inst, wd})
697+
reconcileRefData(t, r)
698+
699+
var updated computev1alpha.Instance
700+
require.NoError(t, projectClient.Get(context.Background(),
701+
types.NamespacedName{Namespace: refDataTestNamespace, Name: refDataTestInstance}, &updated))
702+
703+
cond := apimeta.FindStatusCondition(updated.Status.Conditions, computev1alpha.ReferencedDataReady)
704+
require.NotNil(t, cond)
705+
assert.Equal(t, metav1.ConditionFalse, cond.Status)
706+
assert.Equal(t, computev1alpha.ReferencedDataReasonAwaitingPropagation, cond.Reason,
707+
"should be AwaitingPropagation (not SourceNotFound) when resolver succeeded but companion is still in flight")
708+
assert.Contains(t, cond.Message, refDataTestCMToken)
709+
}
710+
711+
// TestFederated_AnnotationPresent_CompanionsPresent verifies that when the WD
712+
// has the expected-data annotation and all companions are present (healthy path),
713+
// the Instance advances to Ready=True.
714+
func TestFederated_AnnotationPresent_CompanionsPresent(t *testing.T) {
715+
expected := []string{refDataTestCMToken}
716+
annoVal, _ := json.Marshal(expected)
717+
wd := makeWDForCell(string(annoVal))
718+
inst := makeInstanceWithRefDataGate()
719+
companionCM := makeCompanionConfigMap(refDataTestCMCompanionName)
720+
721+
r, projectClient, _ := newRefDataReconciler(t, []client.Object{inst, wd, companionCM})
722+
reconcileRefData(t, r)
723+
724+
var updated computev1alpha.Instance
725+
require.NoError(t, projectClient.Get(context.Background(),
726+
types.NamespacedName{Namespace: refDataTestNamespace, Name: refDataTestInstance}, &updated))
727+
728+
cond := apimeta.FindStatusCondition(updated.Status.Conditions, computev1alpha.ReferencedDataReady)
729+
require.NotNil(t, cond)
730+
assert.Equal(t, metav1.ConditionTrue, cond.Status,
731+
"should be Ready=True when all companions are present and no terminal error")
732+
}
733+
734+
// TestFederated_QuotaAndSourceNotFound_SourceNotFoundWins verifies that when
735+
// both the Quota gate and ReferencedData gate are present, and the cell WD
736+
// carries a terminal-error annotation, Instance.Ready picks SourceNotFound
737+
// (priority 5) over PendingQuota (priority 3).
738+
func TestFederated_QuotaAndSourceNotFound_SourceNotFoundWins(t *testing.T) {
739+
msg := `ConfigMap "app-config" not found in namespace "default"`
740+
wd := makeWDWithTerminalError(computev1alpha.ReferencedDataReasonSourceNotFound, msg)
741+
inst := makeInstanceWithRefDataGate()
742+
743+
// Add the Quota gate alongside ReferencedData.
744+
inst.Spec.Controller.SchedulingGates = append(inst.Spec.Controller.SchedulingGates,
745+
computev1alpha.SchedulingGate{Name: instancecontrol.QuotaSchedulingGate.String()},
746+
)
747+
748+
// Seed a QuotaGranted=False/QuotaExceeded condition on the instance.
749+
inst.Status.Conditions = []metav1.Condition{
750+
{
751+
Type: computev1alpha.InstanceQuotaGranted,
752+
Status: metav1.ConditionFalse,
753+
Reason: computev1alpha.InstanceQuotaGrantedReasonQuotaExceeded,
754+
Message: "Quota exceeded for project",
755+
ObservedGeneration: 1,
756+
LastTransitionTime: metav1.Now(),
757+
},
758+
}
759+
760+
r, projectClient, _ := newRefDataReconciler(t, []client.Object{inst, wd})
761+
reconcileRefData(t, r)
762+
763+
var updated computev1alpha.Instance
764+
require.NoError(t, projectClient.Get(context.Background(),
765+
types.NamespacedName{Namespace: refDataTestNamespace, Name: refDataTestInstance}, &updated))
766+
767+
cond := apimeta.FindStatusCondition(updated.Status.Conditions, computev1alpha.ReferencedDataReady)
768+
require.NotNil(t, cond)
769+
assert.Equal(t, computev1alpha.ReferencedDataReasonSourceNotFound, cond.Reason,
770+
"SourceNotFound should be set on ReferencedDataReady so reconcileGatedReadyCondition picks priority 5")
771+
772+
// Note: reconcileGatedReadyCondition (called by reconcileInstanceReadyCondition,
773+
// not tested here) is where the priority competition between SourceNotFound (p5)
774+
// and PendingQuota (p3) is resolved. This test verifies that reconcileReferencedDataCondition
775+
// sets the right ReferencedDataReady sub-condition that feeds into that priority logic.
776+
}
777+
567778
// containsAll returns true when s contains all substrings.
568779
func containsAll(s string, subs ...string) bool {
569780
for _, sub := range subs {

0 commit comments

Comments
 (0)