fix status route status overwrite

Author: zac-nixon

config/crd/bases/elbv2.k8s.aws_ingressclassparams.yaml

@@ -11,7 +11,7 @@ spec:
     kind: IngressClassParams
     listKind: IngressClassParamsList
     plural: ingressclassparams
-    singular: ingressclassparam
+    singular: ingressclassparams
   scope: Cluster
   versions:
   - additionalPrinterColumns:

pkg/gateway/routeutils/listener_attachment_helper.go

@@ -14,7 +14,7 @@ import (
 // listenerAttachmentHelper is an internal utility interface that can be used to determine if a listener will allow
 // a route to attach to it.
 type listenerAttachmentHelper interface {
-	listenerAllowsAttachment(ctx context.Context, gw gwv1.Gateway, listener gwv1.Listener, route preLoadRouteDescriptor, deferredRouteReconciler RouteReconcilerSubmitter, hostnamesFromHttpRoutes map[types.NamespacedName][]gwv1.Hostname, hostnamesFromGrpcRoutes map[types.NamespacedName][]gwv1.Hostname) (bool, error)
+	listenerAllowsAttachment(ctx context.Context, gw gwv1.Gateway, listener gwv1.Listener, route preLoadRouteDescriptor, hostnamesFromHttpRoutes map[types.NamespacedName][]gwv1.Hostname, hostnamesFromGrpcRoutes map[types.NamespacedName][]gwv1.Hostname) (bool, *RouteData, error)
 }
 
 var _ listenerAttachmentHelper = &listenerAttachmentHelperImpl{}
@@ -34,40 +34,33 @@ func newListenerAttachmentHelper(k8sClient client.Client, logger logr.Logger) li
 
 // listenerAllowsAttachment utility method to determine if a listener will allow a route to connect using
 // Gateway API rules to determine compatibility between lister and route.
-func (attachmentHelper *listenerAttachmentHelperImpl) listenerAllowsAttachment(ctx context.Context, gw gwv1.Gateway, listener gwv1.Listener, route preLoadRouteDescriptor, deferredRouteReconciler RouteReconcilerSubmitter, hostnamesFromHttpRoutes map[types.NamespacedName][]gwv1.Hostname, hostnamesFromGrpcRoutes map[types.NamespacedName][]gwv1.Hostname) (bool, error) {
+func (attachmentHelper *listenerAttachmentHelperImpl) listenerAllowsAttachment(ctx context.Context, gw gwv1.Gateway, listener gwv1.Listener, route preLoadRouteDescriptor, hostnamesFromHttpRoutes map[types.NamespacedName][]gwv1.Hostname, hostnamesFromGrpcRoutes map[types.NamespacedName][]gwv1.Hostname) (bool, *RouteData, error) {
 	// check namespace
 	namespaceOK, err := attachmentHelper.namespaceCheck(ctx, gw, listener, route)
 	if err != nil {
-		return false, err
+		return false, nil, err
 	}
 	if !namespaceOK {
-		deferredRouteReconciler.Enqueue(
-			GenerateRouteData(false, true, string(gwv1.RouteReasonNotAllowedByListeners), RouteStatusInfoRejectedMessageNamespaceNotMatch, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw),
-		)
-
-		return false, nil
+		rd := GenerateRouteData(false, true, string(gwv1.RouteReasonNotAllowedByListeners), RouteStatusInfoRejectedMessageNamespaceNotMatch, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw)
+		return false, &rd, nil
 	}
 
 	// check kind
 	kindOK := attachmentHelper.kindCheck(listener, route)
 	if !kindOK {
-		deferredRouteReconciler.Enqueue(
-			GenerateRouteData(false, true, string(gwv1.RouteReasonNotAllowedByListeners), RouteStatusInfoRejectedMessageKindNotMatch, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw),
-		)
-		return false, nil
+		rd := GenerateRouteData(false, true, string(gwv1.RouteReasonNotAllowedByListeners), RouteStatusInfoRejectedMessageKindNotMatch, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw)
+		return false, &rd, nil
 	}
 
 	// check hostname
 	if (route.GetRouteKind() == HTTPRouteKind || route.GetRouteKind() == GRPCRouteKind || route.GetRouteKind() == TLSRouteKind) && route.GetHostnames() != nil {
 		hostnameOK, err := attachmentHelper.hostnameCheck(listener, route)
 		if err != nil {
-			return false, err
+			return false, nil, err
 		}
 		if !hostnameOK {
-			deferredRouteReconciler.Enqueue(
-				GenerateRouteData(false, true, string(gwv1.RouteReasonNoMatchingListenerHostname), RouteStatusInfoRejectedMessageNoMatchingHostname, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw),
-			)
-			return false, nil
+			rd := GenerateRouteData(false, true, string(gwv1.RouteReasonNoMatchingListenerHostname), RouteStatusInfoRejectedMessageNoMatchingHostname, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw)
+			return false, &rd, nil
 		}
 	}
 
@@ -76,14 +69,12 @@ func (attachmentHelper *listenerAttachmentHelperImpl) listenerAllowsAttachment(c
 		hostnameUniquenessOK, conflictRoute := attachmentHelper.crossServingHostnameUniquenessCheck(route, hostnamesFromHttpRoutes, hostnamesFromGrpcRoutes)
 		if !hostnameUniquenessOK {
 			message := fmt.Sprintf("HTTPRoute and GRPCRoute have overlap hostname, attachment is rejected. Conflict route: %s", conflictRoute)
-			deferredRouteReconciler.Enqueue(
-				GenerateRouteData(false, true, string(gwv1.RouteReasonNotAllowedByListeners), message, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw),
-			)
-			return false, nil
+			rd := GenerateRouteData(false, true, string(gwv1.RouteReasonNotAllowedByListeners), message, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw)
+			return false, &rd, nil
 		}
 	}
 
-	return true, nil
+	return true, nil, nil
 }
 
 // namespaceCheck namespace check implements the Gateway API spec for namespace matching between listener

pkg/gateway/routeutils/listener_attachment_helper_test.go

@@ -22,12 +22,19 @@ func (mnss *mockNamespaceSelector) getNamespacesFromSelector(_ context.Context,
 }
 
 func Test_listenerAllowsAttachment(t *testing.T) {
+
+	type expectedRouteStatus struct {
+		reason  string
+		message string
+	}
+
 	testCases := []struct {
-		name             string
-		gwNamespace      string
-		routeNamespace   string
-		listenerProtocol gwv1.ProtocolType
-		expected         bool
+		name                 string
+		gwNamespace          string
+		routeNamespace       string
+		listenerProtocol     gwv1.ProtocolType
+		expectedStatusUpdate *expectedRouteStatus
+		expected             bool
 	}{
 		{
 			name:             "namespace and kind are ok",
@@ -41,12 +48,20 @@ func Test_listenerAllowsAttachment(t *testing.T) {
 			gwNamespace:      "ns1",
 			routeNamespace:   "ns2",
 			listenerProtocol: gwv1.HTTPProtocolType,
+			expectedStatusUpdate: &expectedRouteStatus{
+				reason:  string(gwv1.RouteReasonNotAllowedByListeners),
+				message: RouteStatusInfoRejectedMessageNamespaceNotMatch,
+			},
 		},
 		{
 			name:             "kind is not ok",
 			gwNamespace:      "ns1",
 			routeNamespace:   "ns1",
 			listenerProtocol: gwv1.TLSProtocolType,
+			expectedStatusUpdate: &expectedRouteStatus{
+				reason:  string(gwv1.RouteReasonNotAllowedByListeners),
+				message: RouteStatusInfoRejectedMessageKindNotMatch,
+			},
 		},
 	}
 
@@ -72,12 +87,22 @@ func Test_listenerAllowsAttachment(t *testing.T) {
 			}
 			hostnameFromHttpRoute := map[types.NamespacedName][]gwv1.Hostname{}
 			hostnameFromGrpcRoute := map[types.NamespacedName][]gwv1.Hostname{}
-			mockReconciler := NewMockRouteReconciler()
-			result, err := attachmentHelper.listenerAllowsAttachment(context.Background(), gw, gwv1.Listener{
+			result, statusUpdate, err := attachmentHelper.listenerAllowsAttachment(context.Background(), gw, gwv1.Listener{
 				Protocol: tc.listenerProtocol,
-			}, route, mockReconciler, hostnameFromHttpRoute, hostnameFromGrpcRoute)
+			}, route, hostnameFromHttpRoute, hostnameFromGrpcRoute)
 			assert.NoError(t, err)
 			assert.Equal(t, tc.expected, result)
+			if tc.expectedStatusUpdate == nil {
+				assert.Nil(t, statusUpdate)
+			} else {
+				assert.NotNil(t, statusUpdate)
+				assert.Equal(t, gw.Name, statusUpdate.ParentRefGateway.Name)
+				assert.Equal(t, gw.Namespace, statusUpdate.ParentRefGateway.Namespace)
+				assert.Equal(t, route.GetRouteNamespacedName().Name, statusUpdate.RouteMetadata.RouteName)
+				assert.Equal(t, route.GetRouteNamespacedName().Namespace, statusUpdate.RouteMetadata.RouteNamespace)
+				assert.Equal(t, tc.expectedStatusUpdate.message, statusUpdate.RouteStatusInfo.Message)
+				assert.Equal(t, tc.expectedStatusUpdate.reason, statusUpdate.RouteStatusInfo.Reason)
+			}
 		})
 	}
 }

pkg/gateway/routeutils/loader.go

@@ -76,6 +76,23 @@ func (l *loaderImpl) LoadRoutesForGateway(ctx context.Context, gw gwv1.Gateway,
 	// 1. Load all relevant routes according to the filter
 
 	loadedRoutes := make([]preLoadRouteDescriptor, 0)
+
+	routeStatusUpdates := make([]RouteData, 0)
+
+	defer func() {
+		seenCache := sets.NewString()
+		// As we process the failures first, we ensure that we don't flip flop the route status from
+		// failed -> ok.
+		for _, v := range routeStatusUpdates {
+			k := generateRouteDataCacheKey(v)
+			if seenCache.Has(k) {
+				continue
+			}
+			seenCache.Insert(k)
+			l.routeSubmitter.Enqueue(v)
+		}
+	}()
+
 	for route, loader := range l.allRouteLoaders {
 		applicable := filter.IsApplicable(route)
 		l.logger.V(1).Info("Processing route", "route", route, "is applicable", applicable)
@@ -90,35 +107,37 @@ func (l *loaderImpl) LoadRoutesForGateway(ctx context.Context, gw gwv1.Gateway,
 
 	// 2. Remove routes that aren't granted attachment by the listener.
 	// Map any routes that are granted attachment to the listener port that allows the attachment.
-	mappedRoutes, err := l.mapper.mapGatewayAndRoutes(ctx, gw, loadedRoutes, l.routeSubmitter)
+	mappedRoutes, statusUpdates, err := l.mapper.mapGatewayAndRoutes(ctx, gw, loadedRoutes)
+
+	routeStatusUpdates = append(routeStatusUpdates, statusUpdates...)
+
 	if err != nil {
 		return nil, err
 	}
 
 	// 3. Load the underlying resource(s) for each route that is configured.
-	loadedRoute, err := l.loadChildResources(ctx, mappedRoutes, gw)
+	loadedRoute, childRouteLoadUpdates, err := l.loadChildResources(ctx, mappedRoutes, gw)
+	routeStatusUpdates = append(routeStatusUpdates, childRouteLoadUpdates...)
 	if err != nil {
 		return nil, err
 	}
 
 	// update status for accepted routes
 	for _, routeList := range loadedRoute {
 		for _, route := range routeList {
-			l.routeSubmitter.Enqueue(
-				GenerateRouteData(true, true, string(gwv1.RouteConditionAccepted), RouteStatusInfoAcceptedMessage, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw),
-			)
+			routeStatusUpdates = append(routeStatusUpdates, GenerateRouteData(true, true, string(gwv1.RouteConditionAccepted), RouteStatusInfoAcceptedMessage, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw))
 		}
 	}
 	return loadedRoute, nil
 }
 
 // loadChildResources responsible for loading all resources that a route descriptor references.
-func (l *loaderImpl) loadChildResources(ctx context.Context, preloadedRoutes map[int][]preLoadRouteDescriptor, gw gwv1.Gateway) (map[int32][]RouteDescriptor, error) {
+func (l *loaderImpl) loadChildResources(ctx context.Context, preloadedRoutes map[int][]preLoadRouteDescriptor, gw gwv1.Gateway) (map[int32][]RouteDescriptor, []RouteData, error) {
 	// Cache to reduce duplicate route lookups.
 	// Kind -> [NamespacedName:Previously Loaded Descriptor]
 	resourceCache := make(map[string]RouteDescriptor)
-
 	loadedRouteData := make(map[int32][]RouteDescriptor)
+	failedRoutes := make([]RouteData, 0)
 
 	for port, preloadedRouteList := range preloadedRoutes {
 		for _, preloadedRoute := range preloadedRouteList {
@@ -137,12 +156,10 @@ func (l *loaderImpl) loadChildResources(ctx context.Context, preloadedRoutes map
 				for _, lare := range loadAttachedRulesErrors {
 					var loaderErr LoaderError
 					if errors.As(lare.Err, &loaderErr) {
-						l.routeSubmitter.Enqueue(
-							GenerateRouteData(false, false, string(loaderErr.GetRouteReason()), loaderErr.GetRouteMessage(), preloadedRoute.GetRouteNamespacedName(), preloadedRoute.GetRouteKind(), preloadedRoute.GetRouteGeneration(), gw),
-						)
+						failedRoutes = append(failedRoutes, GenerateRouteData(false, false, string(loaderErr.GetRouteReason()), loaderErr.GetRouteMessage(), preloadedRoute.GetRouteNamespacedName(), preloadedRoute.GetRouteKind(), preloadedRoute.GetRouteGeneration(), gw))
 					}
 					if lare.Fatal {
-						return nil, lare.Err
+						return nil, failedRoutes, lare.Err
 					}
 				}
 			}
@@ -151,5 +168,9 @@ func (l *loaderImpl) loadChildResources(ctx context.Context, preloadedRoutes map
 		}
 	}
 
-	return loadedRouteData, nil
+	return loadedRouteData, failedRoutes, nil
+}
+
+func generateRouteDataCacheKey(rd RouteData) string {
+	return fmt.Sprintf("%s-%s-%s-%s-%s", rd.RouteMetadata.RouteName, rd.RouteMetadata.RouteNamespace, rd.RouteMetadata.RouteKind, rd.ParentRefGateway.Name, rd.ParentRefGateway.Namespace)
 }