fix(router): readiness logic for multiple ctrl statuses (#908)

* chore: simplifies condition assertions by using custom funcs

Signed-off-by: Bartosz Majsak <[email protected]>

* fix(router): readiness logic for multiple ctrl statuses

When `HTTPRoute` has status from multiple controllers, current logic
fails to recognize it, expecting status only from gateway controller with
top-level standard condition `Accepted` being `true`.

This change handles scenario of the status with multiple controllers
updates, adjusting the logic to check for standard condition.

If `gatewayapi.RouteConditionAccepted` is not found in all reported
conditions it will be reported as missing.

Example: https://gist.github.com/bartoszmajsak/4329206afe107357afdcb9b92ed778bd

Signed-off-by: Bartosz Majsak <[email protected]>

---------

Signed-off-by: Bartosz Majsak <[email protected]>

Author: bartoszmajsak

pkg/controller/llmisvc/fixture/gwapi_builders.go

@@ -479,29 +479,77 @@ func WithGatewayReadyStatus() GatewayOption {
 
 // Advanced fixture patterns for custom conditions
 
-// WithCustomHTTPRouteConditions allows setting custom conditions on HTTPRoute parent status
-func WithCustomHTTPRouteConditions(parentRef gatewayapi.ParentReference, controllerName string, conditions ...metav1.Condition) HTTPRouteOption {
-	return WithHTTPRouteParentStatus(parentRef, controllerName, conditions...)
+// KuadrantControllerStatus creates a RouteParentStatus for Kuadrant policy controller
+func KuadrantControllerStatus(parentRef gatewayapi.ParentReference, generation int64) gatewayapi.RouteParentStatus {
+	return gatewayapi.RouteParentStatus{
+		ParentRef:      parentRef,
+		ControllerName: "kuadrant.io/policy-controller",
+		Conditions: []metav1.Condition{
+			{
+				Type:               "kuadrant.io/AuthPolicyAffected",
+				Status:             metav1.ConditionTrue,
+				Reason:             "Accepted",
+				Message:            "Object affected by AuthPolicy [openshift-ingress/gateway-auth-policy]",
+				LastTransitionTime: metav1.Now(),
+				ObservedGeneration: generation,
+			},
+			{
+				Type:               "kuadrant.io/RateLimitPolicyAffected",
+				Status:             metav1.ConditionTrue,
+				Reason:             "Accepted",
+				Message:            "Object affected by RateLimitPolicy [openshift-ingress/gateway-rate-limits]",
+				LastTransitionTime: metav1.Now(),
+				ObservedGeneration: generation,
+			},
+			{
+				Type:               "kuadrant.io/TokenRateLimitPolicyAffected",
+				Status:             metav1.ConditionTrue,
+				Reason:             "Accepted",
+				Message:            "Object affected by TokenRateLimitPolicy [openshift-ingress/gateway-token-rate-limits]",
+				LastTransitionTime: metav1.Now(),
+				ObservedGeneration: generation,
+			},
+		},
+	}
 }
 
-// WithGatewayNotReadyStatus sets the Gateway status to not ready (for negative testing)
-func WithGatewayNotReadyStatus(reason, message string) GatewayOption {
-	return func(gw *gatewayapi.Gateway) {
-		gw.Status.Conditions = []metav1.Condition{
+// GatewayAPIControllerStatus creates a RouteParentStatus for Gateway API controller
+func GatewayAPIControllerStatus(parentRef gatewayapi.ParentReference, generation int64) gatewayapi.RouteParentStatus {
+	return gatewayapi.RouteParentStatus{
+		ParentRef:      parentRef,
+		ControllerName: "openshift.io/gateway-controller/v1",
+		Conditions: []metav1.Condition{
 			{
-				Type:               string(gatewayapi.GatewayConditionAccepted),
+				Type:               string(gatewayapi.RouteConditionAccepted),
 				Status:             metav1.ConditionTrue,
 				Reason:             "Accepted",
-				Message:            "Gateway accepted",
+				Message:            "Route was valid",
 				LastTransitionTime: metav1.Now(),
+				ObservedGeneration: generation,
 			},
 			{
-				Type:               string(gatewayapi.GatewayConditionProgrammed),
-				Status:             metav1.ConditionFalse,
-				Reason:             reason,
-				Message:            message,
+				Type:               string(gatewayapi.RouteConditionResolvedRefs),
+				Status:             metav1.ConditionTrue,
+				Reason:             "ResolvedRefs",
+				Message:            "All references resolved",
 				LastTransitionTime: metav1.Now(),
+				ObservedGeneration: generation,
 			},
+		},
+	}
+}
+
+// StatusFunc is a function that creates a RouteParentStatus for a given parent ref and generation
+type StatusFunc func(parentRef gatewayapi.ParentReference, generation int64) gatewayapi.RouteParentStatus
+
+// WithHTTPRouteMultipleControllerStatus sets HTTPRoute status with multiple controllers
+// This simulates real-world scenarios where policy controllers and gateway controllers
+// both add status entries for the same parent ref
+func WithHTTPRouteMultipleControllerStatus(parentRef gatewayapi.ParentReference, statusFuncs ...StatusFunc) HTTPRouteOption {
+	return func(route *gatewayapi.HTTPRoute) {
+		for _, statusFunc := range statusFuncs {
+			status := statusFunc(parentRef, route.Generation)
+			route.Status.RouteStatus.Parents = append(route.Status.RouteStatus.Parents, status)
 		}
 	}
 }

pkg/controller/llmisvc/router_discovery.go

@@ -206,16 +206,6 @@ func IsExternalAddressNotFound(err error) bool {
 	return errors.As(err, &externalAddrNotFoundErr)
 }
 
-func filter[T any](s []T, predicateFn func(T) bool) []T {
-	out := make([]T, 0, len(s))
-	for _, x := range s {
-		if predicateFn(x) {
-			out = append(out, x)
-		}
-	}
-	return out
-}
-
 // EvaluateGatewayReadiness checks the readiness status of Gateways and returns those that are not ready
 func EvaluateGatewayReadiness(ctx context.Context, gateways []*gatewayapi.Gateway) []*gatewayapi.Gateway {
 	logger := log.FromContext(ctx)
@@ -269,11 +259,6 @@ func IsHTTPRouteReady(route *gatewayapi.HTTPRoute) bool {
 		return false
 	}
 
-	if len(route.Status.RouteStatus.Parents) != len(route.Spec.ParentRefs) {
-		// HTTPRoute is ready only when _all_ parents have accepted the route.
-		return false
-	}
-
 	if cond, missing := nonReadyHTTPRouteTopLevelCondition(route); cond != nil || missing {
 		return false
 	}
@@ -286,18 +271,24 @@ func nonReadyHTTPRouteTopLevelCondition(route *gatewayapi.HTTPRoute) (*metav1.Co
 		return nil, true
 	}
 
+	routeConditionAcceptedMissing := true
+
 	for _, parent := range route.Status.RouteStatus.Parents {
 		cond := meta.FindStatusCondition(parent.Conditions, string(gatewayapi.RouteConditionAccepted))
 		if cond == nil {
-			return nil, true
+			// This can happen when multiple controllers write to the status, e.g., besides the gateway controller, there
+			// are conditions reported from the policy controller.
+			// See example https://gist.github.com/bartoszmajsak/4329206afe107357afdcb9b92ed778bd
+			continue
 		}
+		routeConditionAcceptedMissing = false
 		staleCondition := cond.ObservedGeneration > 0 && cond.ObservedGeneration < route.Generation
 		if cond.Status != metav1.ConditionTrue || staleCondition {
 			return cond, false
 		}
 	}
 
-	return nil, false
+	return nil, routeConditionAcceptedMissing
 }
 
 // IsInferencePoolReady checks if an InferencePool has been accepted by all parents.