fix: quota-vs-referenced-data priority when both gates present

When QuotaGranted=False and scheduling gates are present, the previous
code early-returned Ready=False/PendingQuota before the evaluate-all-then-
pick block could run. This meant SourceNotFound (priority 5) was masked by
PendingQuota (priority 3) — the same class of short-circuit bug the
evaluate-all redesign was meant to eliminate.

Fix: when scheduling gates are present, quota is fed into consider() like
any other blocking cause so instanceBlockingReasonPriority picks the winner.
The Programmed=False and Running=False side effects of quota denial are
preserved unconditionally regardless of which reason wins Ready — they
reflect quota state independently.

The quota early-return is retained only for the no-gates case, where quota
is the sole active blocker and the three-condition atomic write is correct.

The scheduling-gates evaluation block is extracted into
reconcileGatedReadyCondition to keep reconcileInstanceReadyCondition within
the project's cyclomatic-complexity lint limit (gocyclo ≤ 30).

Adds TestReconcileInstanceReadyCondition_QuotaVsReferencedData (RFC §8.1
headline case): QuotaGranted=False/QuotaExceeded + ReferencedDataReady=
False/SourceNotFound → Ready=False/SourceNotFound (priority 5 > 3), with
Programmed=False/Running=False still set.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
(cherry picked from commit e4d419b)

Author: scotwells

internal/controller/instance_controller.go

@@ -623,6 +623,103 @@ func (r *InstanceReconciler) setReferencedDataConditionWithTransition(
 	return changed
 }
 
+// reconcileGatedReadyCondition handles the scheduling-gates branch of
+// reconcileInstanceReadyCondition. It evaluates ALL blocking sub-conditions
+// (quota, referenced-data, network failure) via the priority function and sets
+// Instance.Ready to the highest-priority cause.
+//
+// When quota is denied, Programmed=False and Running=False are also set
+// regardless of which reason wins Ready — quota gates programmatic activity
+// independently of the Ready reason displayed to the user.
+//
+// This is extracted from reconcileInstanceReadyCondition to keep that function's
+// cyclomatic complexity within the project lint limit.
+func (r *InstanceReconciler) reconcileGatedReadyCondition(
+	ctx context.Context,
+	clusterClient client.Client,
+	instance *computev1alpha.Instance,
+	quotaDenied bool,
+	quotaGrantedCondition *metav1.Condition,
+	readyCondition *metav1.Condition,
+	checker networkFailureChecker,
+) (changed bool, err error) {
+	schedulingGateNames := make([]string, 0, len(instance.Spec.Controller.SchedulingGates))
+	for _, gate := range instance.Spec.Controller.SchedulingGates {
+		schedulingGateNames = append(schedulingGateNames, gate.Name)
+	}
+
+	type candidate struct {
+		status   metav1.ConditionStatus
+		reason   string
+		message  string
+		priority int
+	}
+
+	// Start with the generic fallback so there is always a winner.
+	best := candidate{
+		status:   metav1.ConditionFalse,
+		reason:   computev1alpha.InstanceReadyReasonSchedulingGatesPresent,
+		message:  fmt.Sprintf("Scheduling gates present: %s", strings.Join(schedulingGateNames, ", ")),
+		priority: 0,
+	}
+
+	consider := func(status metav1.ConditionStatus, reason, message string) {
+		p := instanceBlockingReasonPriority(reason)
+		if p > best.priority {
+			best = candidate{status: status, reason: reason, message: message, priority: p}
+		}
+	}
+
+	// Quota is a gate-level blocker: feed it through the priority function so it
+	// competes fairly with other causes (priority 3). A co-occurring SourceNotFound
+	// (priority 5) will correctly beat it.
+	if quotaDenied {
+		consider(metav1.ConditionFalse, computev1alpha.InstanceProgrammedReasonPendingQuota, quotaGrantedCondition.Message)
+	}
+
+	// Check the ReferencedDataReady sub-condition set earlier in this reconcile.
+	if refDataCond := apimeta.FindStatusCondition(instance.Status.Conditions, computev1alpha.ReferencedDataReady); refDataCond != nil && refDataCond.Status != metav1.ConditionTrue {
+		consider(refDataCond.Status, refDataCond.Reason, refDataCond.Message)
+	}
+
+	// Network creation failure is a hard error; call the checker unconditionally
+	// so priority logic can compare it against other blocking causes.
+	networkCreationFailure, networkCreationFailureMessage, err := checker(ctx, clusterClient, instance)
+	if err != nil {
+		return false, fmt.Errorf("failed checking for network creation failure: %w", err)
+	}
+	if networkCreationFailure {
+		consider(metav1.ConditionFalse, reasonNetworkFailedToCreate, networkCreationFailureMessage)
+	}
+
+	// When quota is denied, always stamp Programmed=False and Available=False
+	// regardless of which reason wins Ready. These reflect quota state independently
+	// of the Ready reason selection.
+	if quotaDenied {
+		msg := quotaGrantedCondition.Message
+		changed = apimeta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{
+			Type:               computev1alpha.InstanceProgrammed,
+			Status:             metav1.ConditionFalse,
+			Reason:             computev1alpha.InstanceProgrammedReasonPendingQuota,
+			Message:            msg,
+			ObservedGeneration: instance.Generation,
+		})
+		changed = apimeta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{
+			Type:               computev1alpha.InstanceAvailable,
+			Status:             metav1.ConditionFalse,
+			Reason:             computev1alpha.InstanceProgrammedReasonPendingQuota,
+			Message:            msg,
+			ObservedGeneration: instance.Generation,
+		}) || changed
+	}
+
+	readyCondition.Status = best.status
+	readyCondition.Reason = best.reason
+	readyCondition.Message = best.message
+
+	return apimeta.SetStatusCondition(&instance.Status.Conditions, *readyCondition) || changed, nil
+}
+
 // isTerminalReferencedDataReason reports whether the given ReferencedData reason
 // is terminal — i.e., the companion will never arrive because the source object
 // is permanently unavailable, not just slow to propagate.
@@ -1433,7 +1530,21 @@ func (r *InstanceReconciler) reconcileInstanceReadyCondition(
 	logger := log.FromContext(ctx)
 
 	quotaGrantedCondition := apimeta.FindStatusCondition(instance.Status.Conditions, computev1alpha.InstanceQuotaGranted)
-	if quotaGrantedCondition != nil && quotaGrantedCondition.Status == metav1.ConditionFalse {
+	quotaDenied := quotaGrantedCondition != nil && quotaGrantedCondition.Status == metav1.ConditionFalse
+
+	// When scheduling gates are present, all blocking causes — including quota —
+	// are evaluated together so the priority function picks the most actionable
+	// one to surface on Ready. Quota side effects (Programmed=False, Running=False)
+	// are preserved unconditionally when quota is denied, regardless of which
+	// reason wins Ready.
+	//
+	// When no gates are present and quota is denied, fall through to the early
+	// return below which sets Ready, Programmed, and Running atomically.
+	hasSchedulingGates := instance.Spec.Controller != nil && len(instance.Spec.Controller.SchedulingGates) > 0
+
+	if quotaDenied && !hasSchedulingGates {
+		// No gates: quota is the only active blocking cause. Set all three
+		// conditions atomically and return — same behavior as before.
 		msg := quotaGrantedCondition.Message
 		changed = apimeta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{
 			Type:               computev1alpha.InstanceProgrammed,
@@ -1472,58 +1583,8 @@ func (r *InstanceReconciler) reconcileInstanceReadyCondition(
 		readyCondition = readyCondition.DeepCopy()
 	}
 
-	if instance.Spec.Controller != nil && len(instance.Spec.Controller.SchedulingGates) > 0 {
-		var schedulingGateNames []string
-		for _, gate := range instance.Spec.Controller.SchedulingGates {
-			schedulingGateNames = append(schedulingGateNames, gate.Name)
-		}
-
-		// Evaluate ALL blocking sub-conditions before choosing which to surface.
-		// A short-circuit on the first match would let a low-priority transient
-		// cause (e.g. network provisioning) mask a high-priority actionable error
-		// (e.g. a missing source ConfigMap).
-		type candidate struct {
-			status   metav1.ConditionStatus
-			reason   string
-			message  string
-			priority int
-		}
-
-		// Start with the generic fallback so there is always a winner.
-		best := candidate{
-			status:   metav1.ConditionFalse,
-			reason:   computev1alpha.InstanceReadyReasonSchedulingGatesPresent,
-			message:  fmt.Sprintf("Scheduling gates present: %s", strings.Join(schedulingGateNames, ", ")),
-			priority: 0,
-		}
-
-		consider := func(status metav1.ConditionStatus, reason, message string) {
-			p := instanceBlockingReasonPriority(reason)
-			if p > best.priority {
-				best = candidate{status: status, reason: reason, message: message, priority: p}
-			}
-		}
-
-		// Check the ReferencedDataReady sub-condition set earlier in this reconcile.
-		if refDataCond := apimeta.FindStatusCondition(instance.Status.Conditions, computev1alpha.ReferencedDataReady); refDataCond != nil && refDataCond.Status != metav1.ConditionTrue {
-			consider(refDataCond.Status, refDataCond.Reason, refDataCond.Message)
-		}
-
-		// Network creation failure is a hard error; call the checker unconditionally
-		// so priority logic can compare it against other blocking causes.
-		networkCreationFailure, networkCreationFailureMessage, err := networkFailureChecker(ctx, clusterClient, instance)
-		if err != nil {
-			return false, fmt.Errorf("failed checking for network creation failure: %w", err)
-		}
-		if networkCreationFailure {
-			consider(metav1.ConditionFalse, reasonNetworkFailedToCreate, networkCreationFailureMessage)
-		}
-
-		readyCondition.Status = best.status
-		readyCondition.Reason = best.reason
-		readyCondition.Message = best.message
-
-		return apimeta.SetStatusCondition(&instance.Status.Conditions, *readyCondition), nil
+	if hasSchedulingGates {
+		return r.reconcileGatedReadyCondition(ctx, clusterClient, instance, quotaDenied, quotaGrantedCondition, readyCondition, networkFailureChecker)
 	}
 
 	pendingReason := "Pending"

internal/controller/instance_controller_test.go

@@ -57,6 +57,9 @@ const (
 	// testMsgNetworkCreationFailed is the network-failure message used by the
 	// evaluate-all-then-pick blocking-reason tests.
 	testMsgNetworkCreationFailed = "Network creation failed: timeout"
+
+	// testPlacementA is the placement key used in Workload status rollup tests.
+	testPlacementA = "placement-a"
 )
 
 // newTestScheme builds a runtime.Scheme with the types needed for instance reconcile tests.
@@ -2439,6 +2442,85 @@ func TestReconcileInstanceReadyCondition_EvaluateAllThenPick(t *testing.T) {
 	})
 }
 
+// TestReconcileInstanceReadyCondition_QuotaVsReferencedData is the RFC §8.1
+// headline case: QuotaGranted=False/QuotaExceeded AND
+// ReferencedDataReady=False/SourceNotFound co-occur.
+//
+// SourceNotFound (priority 5) must win over PendingQuota (priority 3) on Ready.
+// Programmed=False and Running=False must still be set (quota side effects are
+// preserved regardless of which reason wins Ready).
+func TestReconcileInstanceReadyCondition_QuotaVsReferencedData(t *testing.T) {
+	noNetworkFailure := func(_ context.Context, _ client.Client, _ *computev1alpha.Instance) (bool, string, error) {
+		return false, "", nil
+	}
+
+	// Instance has both the Quota gate and the ReferencedData gate, matching the
+	// scenario where reconcileQuotaCondition and reconcileReferencedDataCondition
+	// have both already run and written their respective sub-conditions.
+	inst := &computev1alpha.Instance{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:       testInstanceName,
+			Namespace:  testDefaultNamespace,
+			Generation: 1,
+		},
+		Spec: computev1alpha.InstanceSpec{
+			Controller: &computev1alpha.InstanceController{
+				SchedulingGates: []computev1alpha.SchedulingGate{
+					{Name: instancecontrol.QuotaSchedulingGate.String()},
+					{Name: instancecontrol.ReferencedDataSchedulingGate.String()},
+				},
+			},
+		},
+		Status: computev1alpha.InstanceStatus{
+			Conditions: []metav1.Condition{
+				{
+					Type:               computev1alpha.InstanceQuotaGranted,
+					Status:             metav1.ConditionFalse,
+					Reason:             computev1alpha.InstanceQuotaGrantedReasonQuotaExceeded,
+					Message:            testMsgQuotaExceeded,
+					LastTransitionTime: metav1.Now(),
+				},
+				{
+					Type:               computev1alpha.ReferencedDataReady,
+					Status:             metav1.ConditionFalse,
+					Reason:             computev1alpha.ReferencedDataReasonSourceNotFound,
+					Message:            testMsgConfigMapNotFound,
+					LastTransitionTime: metav1.Now(),
+				},
+			},
+		},
+	}
+
+	r := &InstanceReconciler{}
+	changed, err := r.reconcileInstanceReadyCondition(context.Background(), nil, inst, noNetworkFailure)
+	require.NoError(t, err)
+	assert.True(t, changed)
+
+	// Ready must carry the higher-priority SourceNotFound reason (priority 5),
+	// not PendingQuota (priority 3).
+	readyCond := apimeta.FindStatusCondition(inst.Status.Conditions, computev1alpha.InstanceReady)
+	require.NotNil(t, readyCond, "Ready condition must be set")
+	assert.Equal(t, metav1.ConditionFalse, readyCond.Status)
+	assert.Equal(t, computev1alpha.ReferencedDataReasonSourceNotFound, readyCond.Reason,
+		"SourceNotFound (priority 5) must beat PendingQuota (priority 3)")
+	assert.Equal(t, testMsgConfigMapNotFound, readyCond.Message,
+		"Ready message must be the SourceNotFound message verbatim")
+
+	// Programmed and Running must still be set to False/PendingQuota — quota
+	// side effects are preserved regardless of which reason wins Ready.
+	programmedCond := apimeta.FindStatusCondition(inst.Status.Conditions, computev1alpha.InstanceProgrammed)
+	require.NotNil(t, programmedCond, "Programmed condition must be set when quota is denied")
+	assert.Equal(t, metav1.ConditionFalse, programmedCond.Status)
+	assert.Equal(t, computev1alpha.InstanceProgrammedReasonPendingQuota, programmedCond.Reason,
+		"Programmed must reflect quota denial even when Ready surfaces a different reason")
+
+	availableCond := apimeta.FindStatusCondition(inst.Status.Conditions, computev1alpha.InstanceAvailable)
+	require.NotNil(t, availableCond, "Available condition must be set when quota is denied")
+	assert.Equal(t, metav1.ConditionFalse, availableCond.Status)
+	assert.Equal(t, computev1alpha.InstanceProgrammedReasonPendingQuota, availableCond.Reason,
+		"Available must reflect quota denial even when Ready surfaces a different reason")
+}
+
 // TestInstanceBlockingReasonPriority exhaustively verifies the priority table for
 // Instance.Ready reason selection, as extended in this layer to rank
 // referenced-data reasons. Every listed reason must return the expected integer;