diff --git a/apis/elbv2/v1beta1/ingressclassparams_types.go b/apis/elbv2/v1beta1/ingressclassparams_types.go index 7c68bc70e7..f68ceeeeed 100644 --- a/apis/elbv2/v1beta1/ingressclassparams_types.go +++ b/apis/elbv2/v1beta1/ingressclassparams_types.go @@ -192,7 +192,7 @@ type IngressClassParamsSpec struct { } // +kubebuilder:object:root=true -// +kubebuilder:resource:scope=Cluster +// +kubebuilder:resource:scope=Cluster,singular=ingressclassparam // +kubebuilder:storageversion // +kubebuilder:printcolumn:name="GROUP-NAME",type="string",JSONPath=".spec.group.name",description="The Ingress Group name" // +kubebuilder:printcolumn:name="SCHEME",type="string",JSONPath=".spec.scheme",description="The AWS Load Balancer scheme" diff --git a/controllers/gateway/gateway_controller.go b/controllers/gateway/gateway_controller.go index 8b91934e4b..b5ee15901e 100644 --- a/controllers/gateway/gateway_controller.go +++ b/controllers/gateway/gateway_controller.go @@ -51,13 +51,13 @@ const ( var _ Reconciler = &gatewayReconciler{} // NewNLBGatewayReconciler constructs a gateway reconciler to handle specifically for NLB gateways -func NewNLBGatewayReconciler(routeLoader routeutils.Loader, referenceCounter referencecounter.ServiceReferenceCounter, cloud services.Cloud, k8sClient client.Client, eventRecorder record.EventRecorder, controllerConfig config.ControllerConfig, finalizerManager k8s.FinalizerManager, networkingManager networking.NetworkingManager, networkingSGReconciler networking.SecurityGroupReconciler, networkingSGManager networking.SecurityGroupManager, elbv2TaggingManager elbv2deploy.TaggingManager, subnetResolver networking.SubnetsResolver, vpcInfoProvider networking.VPCInfoProvider, backendSGProvider networking.BackendSGProvider, sgResolver networking.SecurityGroupResolver, logger logr.Logger, metricsCollector lbcmetrics.MetricCollector, reconcileCounters *metricsutil.ReconcileCounters, routeReconciler routeutils.RouteReconciler) Reconciler { - return newGatewayReconciler(constants.NLBGatewayController, elbv2model.LoadBalancerTypeNetwork, controllerConfig.NLBGatewayMaxConcurrentReconciles, constants.NLBGatewayTagPrefix, shared_constants.NLBGatewayFinalizer, routeLoader, referenceCounter, routeutils.L4RouteFilter, cloud, k8sClient, eventRecorder, controllerConfig, finalizerManager, networkingSGReconciler, networkingManager, networkingSGManager, elbv2TaggingManager, subnetResolver, vpcInfoProvider, backendSGProvider, sgResolver, nlbAddons, logger, metricsCollector, reconcileCounters.IncrementNLBGateway, routeReconciler) +func NewNLBGatewayReconciler(routeLoader routeutils.Loader, referenceCounter referencecounter.ServiceReferenceCounter, cloud services.Cloud, k8sClient client.Client, eventRecorder record.EventRecorder, controllerConfig config.ControllerConfig, finalizerManager k8s.FinalizerManager, networkingManager networking.NetworkingManager, networkingSGReconciler networking.SecurityGroupReconciler, networkingSGManager networking.SecurityGroupManager, elbv2TaggingManager elbv2deploy.TaggingManager, subnetResolver networking.SubnetsResolver, vpcInfoProvider networking.VPCInfoProvider, backendSGProvider networking.BackendSGProvider, sgResolver networking.SecurityGroupResolver, logger logr.Logger, metricsCollector lbcmetrics.MetricCollector, reconcileCounters *metricsutil.ReconcileCounters) Reconciler { + return newGatewayReconciler(constants.NLBGatewayController, elbv2model.LoadBalancerTypeNetwork, controllerConfig.NLBGatewayMaxConcurrentReconciles, constants.NLBGatewayTagPrefix, shared_constants.NLBGatewayFinalizer, routeLoader, referenceCounter, routeutils.L4RouteFilter, cloud, k8sClient, eventRecorder, controllerConfig, finalizerManager, networkingSGReconciler, networkingManager, networkingSGManager, elbv2TaggingManager, subnetResolver, vpcInfoProvider, backendSGProvider, sgResolver, nlbAddons, logger, metricsCollector, reconcileCounters.IncrementNLBGateway) } // NewALBGatewayReconciler constructs a gateway reconciler to handle specifically for ALB gateways -func NewALBGatewayReconciler(routeLoader routeutils.Loader, cloud services.Cloud, k8sClient client.Client, referenceCounter referencecounter.ServiceReferenceCounter, eventRecorder record.EventRecorder, controllerConfig config.ControllerConfig, finalizerManager k8s.FinalizerManager, networkingManager networking.NetworkingManager, networkingSGReconciler networking.SecurityGroupReconciler, networkingSGManager networking.SecurityGroupManager, elbv2TaggingManager elbv2deploy.TaggingManager, subnetResolver networking.SubnetsResolver, vpcInfoProvider networking.VPCInfoProvider, backendSGProvider networking.BackendSGProvider, sgResolver networking.SecurityGroupResolver, logger logr.Logger, metricsCollector lbcmetrics.MetricCollector, reconcileCounters *metricsutil.ReconcileCounters, routeReconciler routeutils.RouteReconciler) Reconciler { - return newGatewayReconciler(constants.ALBGatewayController, elbv2model.LoadBalancerTypeApplication, controllerConfig.ALBGatewayMaxConcurrentReconciles, constants.ALBGatewayTagPrefix, shared_constants.ALBGatewayFinalizer, routeLoader, referenceCounter, routeutils.L7RouteFilter, cloud, k8sClient, eventRecorder, controllerConfig, finalizerManager, networkingSGReconciler, networkingManager, networkingSGManager, elbv2TaggingManager, subnetResolver, vpcInfoProvider, backendSGProvider, sgResolver, albAddons, logger, metricsCollector, reconcileCounters.IncrementALBGateway, routeReconciler) +func NewALBGatewayReconciler(routeLoader routeutils.Loader, cloud services.Cloud, k8sClient client.Client, referenceCounter referencecounter.ServiceReferenceCounter, eventRecorder record.EventRecorder, controllerConfig config.ControllerConfig, finalizerManager k8s.FinalizerManager, networkingManager networking.NetworkingManager, networkingSGReconciler networking.SecurityGroupReconciler, networkingSGManager networking.SecurityGroupManager, elbv2TaggingManager elbv2deploy.TaggingManager, subnetResolver networking.SubnetsResolver, vpcInfoProvider networking.VPCInfoProvider, backendSGProvider networking.BackendSGProvider, sgResolver networking.SecurityGroupResolver, logger logr.Logger, metricsCollector lbcmetrics.MetricCollector, reconcileCounters *metricsutil.ReconcileCounters) Reconciler { + return newGatewayReconciler(constants.ALBGatewayController, elbv2model.LoadBalancerTypeApplication, controllerConfig.ALBGatewayMaxConcurrentReconciles, constants.ALBGatewayTagPrefix, shared_constants.ALBGatewayFinalizer, routeLoader, referenceCounter, routeutils.L7RouteFilter, cloud, k8sClient, eventRecorder, controllerConfig, finalizerManager, networkingSGReconciler, networkingManager, networkingSGManager, elbv2TaggingManager, subnetResolver, vpcInfoProvider, backendSGProvider, sgResolver, albAddons, logger, metricsCollector, reconcileCounters.IncrementALBGateway) } // newGatewayReconciler constructs a reconciler that responds to gateway object changes @@ -68,7 +68,7 @@ func newGatewayReconciler(controllerName string, lbType elbv2model.LoadBalancerT networkingManager networking.NetworkingManager, networkingSGManager networking.SecurityGroupManager, elbv2TaggingManager elbv2deploy.TaggingManager, subnetResolver networking.SubnetsResolver, vpcInfoProvider networking.VPCInfoProvider, backendSGProvider networking.BackendSGProvider, sgResolver networking.SecurityGroupResolver, supportedAddons []addon.Addon, logger logr.Logger, metricsCollector lbcmetrics.MetricCollector, - reconcileTracker func(namespaceName types.NamespacedName), routeReconciler routeutils.RouteReconciler) Reconciler { + reconcileTracker func(namespaceName types.NamespacedName)) Reconciler { trackingProvider := tracking.NewDefaultProvider(gatewayTagPrefix, controllerConfig.ClusterName) modelBuilder := gatewaymodel.NewModelBuilder(subnetResolver, vpcInfoProvider, cloud.VpcID(), lbType, trackingProvider, elbv2TaggingManager, controllerConfig, cloud.EC2(), cloud.ELBV2(), cloud.ACM(), controllerConfig.FeatureGates, controllerConfig.ClusterName, controllerConfig.DefaultTags, sets.New(controllerConfig.ExternalManagedTags...), controllerConfig.DefaultSSLPolicy, controllerConfig.DefaultTargetType, controllerConfig.DefaultLoadBalancerScheme, backendSGProvider, sgResolver, controllerConfig.EnableBackendSecurityGroup, controllerConfig.DisableRestrictedSGRules, controllerConfig.IngressConfig.AllowedCertificateAuthorityARNs, supportedAddons, logger) @@ -96,7 +96,6 @@ func newGatewayReconciler(controllerName string, lbType elbv2model.LoadBalancerT metricsCollector: metricsCollector, reconcileTracker: reconcileTracker, cfgResolver: cfgResolver, - routeReconciler: routeReconciler, serviceReferenceCounter: serviceReferenceCounter, gatewayConditionUpdater: prepareGatewayConditionUpdate, } @@ -123,8 +122,7 @@ type gatewayReconciler struct { serviceReferenceCounter referencecounter.ServiceReferenceCounter gatewayConditionUpdater func(gw *gwv1.Gateway, targetConditionType string, newStatus metav1.ConditionStatus, reason string, message string) bool - cfgResolver gatewayConfigResolver - routeReconciler routeutils.RouteReconciler + cfgResolver gatewayConfigResolver } //+kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=referencegrants,verbs=get;list;watch;patch @@ -211,7 +209,7 @@ func (r *gatewayReconciler) reconcileHelper(ctx context.Context, req reconcile.R return err } - allRoutes, err := r.gatewayLoader.LoadRoutesForGateway(ctx, *gw, r.routeFilter, r.routeReconciler) + allRoutes, err := r.gatewayLoader.LoadRoutesForGateway(ctx, *gw, r.routeFilter) if err != nil { var loaderErr routeutils.LoaderError diff --git a/helm/aws-load-balancer-controller/crds/crds.yaml b/helm/aws-load-balancer-controller/crds/crds.yaml index 7e43cb1732..15145de6af 100644 --- a/helm/aws-load-balancer-controller/crds/crds.yaml +++ b/helm/aws-load-balancer-controller/crds/crds.yaml @@ -10,7 +10,7 @@ spec: kind: IngressClassParams listKind: IngressClassParamsList plural: ingressclassparams - singular: ingressclassparams + singular: ingressclassparam scope: Cluster versions: - additionalPrinterColumns: diff --git a/main.go b/main.go index 306440338b..685ce1cbb9 100644 --- a/main.go +++ b/main.go @@ -103,7 +103,6 @@ type gatewayControllerConfig struct { sgResolver networking.SecurityGroupResolver metricsCollector lbcmetrics.MetricCollector reconcileCounters *metricsutil.ReconcileCounters - routeReconciler routeutils.RouteReconciler serviceReferenceCounter referencecounter.ServiceReferenceCounter networkingManager networking.NetworkingManager } @@ -243,7 +242,6 @@ func main() { sgResolver: sgResolver, metricsCollector: lbcMetricsCollector, reconcileCounters: reconcileCounters, - routeReconciler: routeReconciler, networkingManager: networkingManager, serviceReferenceCounter: serviceReferenceCounter, } @@ -251,7 +249,7 @@ func main() { enabledControllers := sets.Set[string]{} routeLoaderCreator := sync.OnceValue(func() routeutils.Loader { - return routeutils.NewLoader(mgr.GetClient(), mgr.GetLogger().WithName("gateway-route-loader")) + return routeutils.NewLoader(mgr.GetClient(), routeReconciler, mgr.GetLogger().WithName("gateway-route-loader")) }) // Setup NLB Gateway controller if enabled @@ -450,7 +448,6 @@ func setupGatewayController(ctx context.Context, mgr ctrl.Manager, cfg *gatewayC logger, cfg.metricsCollector, cfg.reconcileCounters, - cfg.routeReconciler, ) case gateway_constants.ALBGatewayController: reconciler = gateway.NewALBGatewayReconciler( @@ -472,7 +469,6 @@ func setupGatewayController(ctx context.Context, mgr ctrl.Manager, cfg *gatewayC logger, cfg.metricsCollector, cfg.reconcileCounters, - cfg.routeReconciler, ) default: return fmt.Errorf("unknown controller type: %s", controllerType) diff --git a/pkg/gateway/routeutils/listener_attachment_helper.go b/pkg/gateway/routeutils/listener_attachment_helper.go index 491a587eca..c64017889b 100644 --- a/pkg/gateway/routeutils/listener_attachment_helper.go +++ b/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 RouteReconciler, 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 RouteReconciler, 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 diff --git a/pkg/gateway/routeutils/listener_attachment_helper_test.go b/pkg/gateway/routeutils/listener_attachment_helper_test.go index 7ea0df846d..8f22cd31cf 100644 --- a/pkg/gateway/routeutils/listener_attachment_helper_test.go +++ b/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) + } }) } } diff --git a/pkg/gateway/routeutils/loader.go b/pkg/gateway/routeutils/loader.go index e8fc688994..8c851dd03f 100644 --- a/pkg/gateway/routeutils/loader.go +++ b/pkg/gateway/routeutils/loader.go @@ -48,21 +48,23 @@ var L7RouteFilter LoadRouteFilter = &routeFilterImpl{ // Loader will load all data Kubernetes that are pertinent to a gateway (Routes, Services, Target Group Configurations). // It will output the data using a map which maps listener port to the various routing rules for that port. type Loader interface { - LoadRoutesForGateway(ctx context.Context, gw gwv1.Gateway, filter LoadRouteFilter, reconciler RouteReconciler) (map[int32][]RouteDescriptor, error) + LoadRoutesForGateway(ctx context.Context, gw gwv1.Gateway, filter LoadRouteFilter) (map[int32][]RouteDescriptor, error) } var _ Loader = &loaderImpl{} type loaderImpl struct { mapper listenerToRouteMapper + routeSubmitter RouteReconcilerSubmitter k8sClient client.Client logger logr.Logger allRouteLoaders map[RouteKind]func(context context.Context, client client.Client, opts ...client.ListOption) ([]preLoadRouteDescriptor, error) } -func NewLoader(k8sClient client.Client, logger logr.Logger) Loader { +func NewLoader(k8sClient client.Client, routeSubmitter RouteReconcilerSubmitter, logger logr.Logger) Loader { return &loaderImpl{ mapper: newListenerToRouteMapper(k8sClient, logger.WithName("route-mapper")), + routeSubmitter: routeSubmitter, k8sClient: k8sClient, allRouteLoaders: allRoutes, logger: logger, @@ -70,10 +72,27 @@ func NewLoader(k8sClient client.Client, logger logr.Logger) Loader { } // LoadRoutesForGateway loads all relevant data for a single Gateway. -func (l *loaderImpl) LoadRoutesForGateway(ctx context.Context, gw gwv1.Gateway, filter LoadRouteFilter, deferredRouteReconciler RouteReconciler) (map[int32][]RouteDescriptor, error) { +func (l *loaderImpl) LoadRoutesForGateway(ctx context.Context, gw gwv1.Gateway, filter LoadRouteFilter) (map[int32][]RouteDescriptor, error) { // 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) @@ -88,13 +107,17 @@ 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, deferredRouteReconciler) + 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, deferredRouteReconciler, gw) + loadedRoute, childRouteLoadUpdates, err := l.loadChildResources(ctx, mappedRoutes, gw) + routeStatusUpdates = append(routeStatusUpdates, childRouteLoadUpdates...) if err != nil { return nil, err } @@ -102,21 +125,19 @@ func (l *loaderImpl) LoadRoutesForGateway(ctx context.Context, gw gwv1.Gateway, // update status for accepted routes for _, routeList := range loadedRoute { for _, route := range routeList { - deferredRouteReconciler.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, deferredRouteReconciler RouteReconciler, 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 { @@ -135,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) { - deferredRouteReconciler.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 } } } @@ -149,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) } diff --git a/pkg/gateway/routeutils/loader_test.go b/pkg/gateway/routeutils/loader_test.go index 23f8828f8b..b99acea49c 100644 --- a/pkg/gateway/routeutils/loader_test.go +++ b/pkg/gateway/routeutils/loader_test.go @@ -4,6 +4,7 @@ import ( "context" "github.com/go-logr/logr" "github.com/stretchr/testify/assert" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" "sigs.k8s.io/controller-runtime/pkg/client" @@ -13,14 +14,15 @@ import ( ) type mockMapper struct { - t *testing.T - expectedRoutes []preLoadRouteDescriptor - mapToReturn map[int][]preLoadRouteDescriptor + t *testing.T + expectedRoutes []preLoadRouteDescriptor + mapToReturn map[int][]preLoadRouteDescriptor + routeStatusUpdates []RouteData } -func (m *mockMapper) mapGatewayAndRoutes(context context.Context, gw gwv1.Gateway, routes []preLoadRouteDescriptor, routeReconciler RouteReconciler) (map[int][]preLoadRouteDescriptor, error) { +func (m *mockMapper) mapGatewayAndRoutes(context context.Context, gw gwv1.Gateway, routes []preLoadRouteDescriptor) (map[int][]preLoadRouteDescriptor, []RouteData, error) { assert.ElementsMatch(m.t, m.expectedRoutes, routes) - return m.mapToReturn, nil + return m.mapToReturn, m.routeStatusUpdates, nil } var _ RouteDescriptor = &mockRoute{} @@ -151,18 +153,21 @@ func TestLoadRoutesForGateway(t *testing.T) { } testCases := []struct { - name string - acceptedKinds sets.Set[RouteKind] - expectedMap map[int32][]RouteDescriptor - expectedPreloadMap map[int][]preLoadRouteDescriptor - expectedPreMappedRoutes []preLoadRouteDescriptor - expectError bool + name string + acceptedKinds sets.Set[RouteKind] + expectedMap map[int32][]RouteDescriptor + expectedPreloadMap map[int][]preLoadRouteDescriptor + expectedPreMappedRoutes []preLoadRouteDescriptor + mapperRouteStatusUpdates []RouteData + expectedReconcileQueue map[string]bool // generateRouteDataCacheKey -> succeeded + expectError bool }{ { name: "filter allows no routes", acceptedKinds: make(sets.Set[RouteKind]), expectedPreMappedRoutes: make([]preLoadRouteDescriptor, 0), expectedMap: make(map[int32][]RouteDescriptor), + expectedReconcileQueue: map[string]bool{}, }, { name: "filter only allows http route", @@ -174,6 +179,11 @@ func TestLoadRoutesForGateway(t *testing.T) { expectedMap: map[int32][]RouteDescriptor{ 80: loadedHTTPRoutes, }, + expectedReconcileQueue: map[string]bool{ + "http1-http1-ns-HTTPRoute-gw-gw-ns": true, + "http2-http2-ns-HTTPRoute-gw-gw-ns": true, + "http3-http3-ns-HTTPRoute-gw-gw-ns": true, + }, }, { name: "filter only allows http route, multiple ports", @@ -187,6 +197,11 @@ func TestLoadRoutesForGateway(t *testing.T) { 80: loadedHTTPRoutes, 443: loadedHTTPRoutes, }, + expectedReconcileQueue: map[string]bool{ + "http1-http1-ns-HTTPRoute-gw-gw-ns": true, + "http2-http2-ns-HTTPRoute-gw-gw-ns": true, + "http3-http3-ns-HTTPRoute-gw-gw-ns": true, + }, }, { name: "filter only allows tcp route", @@ -198,6 +213,11 @@ func TestLoadRoutesForGateway(t *testing.T) { expectedMap: map[int32][]RouteDescriptor{ 80: loadedTCPRoutes, }, + expectedReconcileQueue: map[string]bool{ + "tcp1-tcp1-ns-TCPRoute-gw-gw-ns": true, + "tcp2-tcp2-ns-TCPRoute-gw-gw-ns": true, + "tcp3-tcp3-ns-TCPRoute-gw-gw-ns": true, + }, }, { name: "filter allows both route kinds", @@ -211,30 +231,92 @@ func TestLoadRoutesForGateway(t *testing.T) { 80: loadedTCPRoutes, 443: loadedHTTPRoutes, }, + expectedReconcileQueue: map[string]bool{ + "http1-http1-ns-HTTPRoute-gw-gw-ns": true, + "http2-http2-ns-HTTPRoute-gw-gw-ns": true, + "http3-http3-ns-HTTPRoute-gw-gw-ns": true, + "tcp1-tcp1-ns-TCPRoute-gw-gw-ns": true, + "tcp2-tcp2-ns-TCPRoute-gw-gw-ns": true, + "tcp3-tcp3-ns-TCPRoute-gw-gw-ns": true, + }, + }, + { + name: "failed route should lead to only failed version status getting published", + acceptedKinds: sets.New[RouteKind](TCPRouteKind, HTTPRouteKind), + expectedPreMappedRoutes: append(preLoadHTTPRoutes, preLoadTCPRoutes...), + expectedPreloadMap: map[int][]preLoadRouteDescriptor{ + 80: preLoadTCPRoutes, + 443: preLoadHTTPRoutes, + }, + expectedMap: map[int32][]RouteDescriptor{ + 80: loadedTCPRoutes, + 443: loadedHTTPRoutes, + }, + expectedReconcileQueue: map[string]bool{ + "http1-http1-ns-HTTPRoute-gw-gw-ns": true, + "http2-http2-ns-HTTPRoute-gw-gw-ns": true, + "http3-http3-ns-HTTPRoute-gw-gw-ns": true, + "tcp1-tcp1-ns-TCPRoute-gw-gw-ns": true, + "tcp2-tcp2-ns-TCPRoute-gw-gw-ns": false, + "tcp3-tcp3-ns-TCPRoute-gw-gw-ns": true, + }, + mapperRouteStatusUpdates: []RouteData{ + { + RouteStatusInfo: RouteStatusInfo{ + Accepted: false, + }, + RouteMetadata: RouteMetadata{ + RouteName: "tcp2", + RouteNamespace: "tcp2-ns", + RouteKind: string(TCPRouteKind), + RouteGeneration: 0, + }, + ParentRefGateway: ParentRefGateway{ + Name: "gw", + Namespace: "gw-ns", + }, + }, + }, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + routeReconciler := NewMockRouteReconciler() loader := loaderImpl{ mapper: &mockMapper{ - t: t, - expectedRoutes: tc.expectedPreMappedRoutes, - mapToReturn: tc.expectedPreloadMap, + t: t, + expectedRoutes: tc.expectedPreMappedRoutes, + mapToReturn: tc.expectedPreloadMap, + routeStatusUpdates: tc.mapperRouteStatusUpdates, }, allRouteLoaders: allRouteLoaders, logger: logr.Discard(), + routeSubmitter: routeReconciler, } filter := &routeFilterImpl{acceptedKinds: tc.acceptedKinds} - mockReconciler := NewMockRouteReconciler() - result, err := loader.LoadRoutesForGateway(context.Background(), gwv1.Gateway{}, filter, mockReconciler) + result, err := loader.LoadRoutesForGateway(context.Background(), gwv1.Gateway{ObjectMeta: v1.ObjectMeta{ + Name: "gw", + Namespace: "gw-ns", + }}, filter) if tc.expectError { assert.Error(t, err) return } + assert.NoError(t, err) assert.Equal(t, tc.expectedMap, result) + assert.Equal(t, len(tc.expectedReconcileQueue), len(routeReconciler.Enqueued)) + + for _, actual := range routeReconciler.Enqueued { + ak := generateRouteDataCacheKey(actual.RouteData) + + v, ok := tc.expectedReconcileQueue[ak] + assert.True(t, ok) + assert.Equal(t, v, actual.RouteData.RouteStatusInfo.Accepted, ak) + } + }) } } diff --git a/pkg/gateway/routeutils/mock_route_reconciler.go b/pkg/gateway/routeutils/mock_route_reconciler.go index 938d4dcfec..790c73a146 100644 --- a/pkg/gateway/routeutils/mock_route_reconciler.go +++ b/pkg/gateway/routeutils/mock_route_reconciler.go @@ -14,13 +14,9 @@ func NewMockRouteReconciler() *MockRouteReconciler { } func (m *MockRouteReconciler) Enqueue(routeData RouteData) { - if m.Enqueued == nil { - m.Enqueued = make([]EnqueuedType, 0) - } else { - m.Enqueued = append(m.Enqueued, EnqueuedType{ - routeData, - }) - } + m.Enqueued = append(m.Enqueued, EnqueuedType{ + routeData, + }) } func (m *MockRouteReconciler) Run() { diff --git a/pkg/gateway/routeutils/route_listener_mapper.go b/pkg/gateway/routeutils/route_listener_mapper.go index c2bde2782a..0e1fce5ffe 100644 --- a/pkg/gateway/routeutils/route_listener_mapper.go +++ b/pkg/gateway/routeutils/route_listener_mapper.go @@ -11,7 +11,7 @@ import ( // listenerToRouteMapper is an internal utility that will map a list of routes to the listeners of a gateway // if the gateway and/or route are incompatible, then the route is discarded. type listenerToRouteMapper interface { - mapGatewayAndRoutes(context context.Context, gw gwv1.Gateway, routes []preLoadRouteDescriptor, deferredRouteReconciler RouteReconciler) (map[int][]preLoadRouteDescriptor, error) + mapGatewayAndRoutes(context context.Context, gw gwv1.Gateway, routes []preLoadRouteDescriptor) (map[int][]preLoadRouteDescriptor, []RouteData, error) } var _ listenerToRouteMapper = &listenerToRouteMapperImpl{} @@ -31,8 +31,9 @@ func newListenerToRouteMapper(k8sClient client.Client, logger logr.Logger) liste } // mapGatewayAndRoutes will map route to the corresponding listener ports using the Gateway API spec rules. -func (ltr *listenerToRouteMapperImpl) mapGatewayAndRoutes(ctx context.Context, gw gwv1.Gateway, routes []preLoadRouteDescriptor, deferredRouteReconciler RouteReconciler) (map[int][]preLoadRouteDescriptor, error) { +func (ltr *listenerToRouteMapperImpl) mapGatewayAndRoutes(ctx context.Context, gw gwv1.Gateway, routes []preLoadRouteDescriptor) (map[int][]preLoadRouteDescriptor, []RouteData, error) { result := make(map[int][]preLoadRouteDescriptor) + failedRoutes := make([]RouteData, 0) // First filter out any routes that are not intended for this Gateway. routesForGateway := make([]preLoadRouteDescriptor, 0) @@ -57,9 +58,13 @@ func (ltr *listenerToRouteMapperImpl) mapGatewayAndRoutes(ctx context.Context, g continue } - allowedAttachment, err := ltr.listenerAttachmentHelper.listenerAllowsAttachment(ctx, gw, listener, route, deferredRouteReconciler, hostnamesFromHttpRoutes, hostnamesFromGrpcRoutes) + allowedAttachment, failedRouteData, err := ltr.listenerAttachmentHelper.listenerAllowsAttachment(ctx, gw, listener, route, hostnamesFromHttpRoutes, hostnamesFromGrpcRoutes) if err != nil { - return nil, err + return nil, failedRoutes, err + } + + if failedRouteData != nil { + failedRoutes = append(failedRoutes, *failedRouteData) } ltr.logger.V(1).Info("listener allows attachment", "route", route.GetRouteNamespacedName(), "allowedAttachment", allowedAttachment) @@ -69,5 +74,5 @@ func (ltr *listenerToRouteMapperImpl) mapGatewayAndRoutes(ctx context.Context, g } } } - return result, nil + return result, failedRoutes, nil } diff --git a/pkg/gateway/routeutils/route_listener_mapper_test.go b/pkg/gateway/routeutils/route_listener_mapper_test.go index 8641e3e974..0f7bf62d16 100644 --- a/pkg/gateway/routeutils/route_listener_mapper_test.go +++ b/pkg/gateway/routeutils/route_listener_mapper_test.go @@ -20,9 +20,9 @@ func makeListenerAttachmentMapKey(listener gwv1.Listener, route preLoadRouteDesc return fmt.Sprintf("%s-%d-%s-%s", listener.Name, listener.Port, nsn.Name, nsn.Namespace) } -func (m *mockListenerAttachmentHelper) listenerAllowsAttachment(ctx context.Context, gw gwv1.Gateway, listener gwv1.Listener, route preLoadRouteDescriptor, routeReconciler RouteReconciler, hostnamesFromHttpRoutes map[types.NamespacedName][]gwv1.Hostname, hostnamesFromGrpcRoutes map[types.NamespacedName][]gwv1.Hostname) (bool, error) { +func (m *mockListenerAttachmentHelper) 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) { k := makeListenerAttachmentMapKey(listener, route) - return m.attachmentMap[k], nil + return m.attachmentMap[k], nil, nil } type mockRouteAttachmentHelper struct { @@ -313,9 +313,7 @@ func Test_mapGatewayAndRoutes(t *testing.T) { }, logger: logr.Discard(), } - - mockReconciler := NewMockRouteReconciler() - result, err := mapper.mapGatewayAndRoutes(context.Background(), tc.gw, tc.routes, mockReconciler) + result, statusUpdates, err := mapper.mapGatewayAndRoutes(context.Background(), tc.gw, tc.routes) if tc.expectErr { assert.Error(t, err) @@ -325,6 +323,8 @@ func Test_mapGatewayAndRoutes(t *testing.T) { assert.NoError(t, err) assert.Equal(t, len(tc.expected), len(result)) + assert.Equal(t, 0, len(statusUpdates)) + for k, v := range tc.expected { assert.ElementsMatch(t, v, result[k]) } diff --git a/pkg/gateway/routeutils/route_reconciler_utils.go b/pkg/gateway/routeutils/route_reconciler_utils.go index a09e956248..93acbcdcc9 100644 --- a/pkg/gateway/routeutils/route_reconciler_utils.go +++ b/pkg/gateway/routeutils/route_reconciler_utils.go @@ -37,8 +37,12 @@ type ParentRefGateway struct { } type RouteReconciler interface { - Enqueue(routeData RouteData) Run() + Enqueue(routeData RouteData) +} + +type RouteReconcilerSubmitter interface { + Enqueue(routeData RouteData) } // constants diff --git a/pkg/ingress/model_build_frontend_nlb.go b/pkg/ingress/model_build_frontend_nlb.go index 416a186ced..ae366b80dc 100644 --- a/pkg/ingress/model_build_frontend_nlb.go +++ b/pkg/ingress/model_build_frontend_nlb.go @@ -753,8 +753,6 @@ func buildFrontendNlbResourceID(resourceType string, protocol elbv2model.Protoco func mergeHealthCheckField[T comparable](fieldName string, finalValue **T, currentValue *T, explicit map[string]bool, explicitFields map[string]bool, configIndex int) error { if explicit[fieldName] { if explicitFields[fieldName] { - fmt.Printf("*finalValue (%T): %v\n", **finalValue, **finalValue) - fmt.Printf("newValue (%T): %v\n", *currentValue, *currentValue) if **finalValue != *currentValue { return errors.Errorf("conflicting %s, config %d: %v, previous: %v", fieldName, configIndex+1, *currentValue, **finalValue) } diff --git a/pkg/ingress/model_build_load_balancer_attributes_test.go b/pkg/ingress/model_build_load_balancer_attributes_test.go index 4c623bd8ee..ae98c3a4c3 100644 --- a/pkg/ingress/model_build_load_balancer_attributes_test.go +++ b/pkg/ingress/model_build_load_balancer_attributes_test.go @@ -1,7 +1,6 @@ package ingress import ( - "fmt" "testing" "github.com/pkg/errors" @@ -376,7 +375,6 @@ func Test_defaultModelBuildTask_buildIngressLoadBalancerAttributes(t *testing.T) } got, err := task.buildIngressLoadBalancerAttributes(tt.args.ing) if tt.wantErr != nil { - fmt.Println(err) assert.EqualError(t, err, tt.wantErr.Error()) } else { assert.NoError(t, err) diff --git a/pkg/ingress/model_build_load_balancer_capacity_reservation_test.go b/pkg/ingress/model_build_load_balancer_capacity_reservation_test.go index 5549e6ebe6..f29dff4cbc 100644 --- a/pkg/ingress/model_build_load_balancer_capacity_reservation_test.go +++ b/pkg/ingress/model_build_load_balancer_capacity_reservation_test.go @@ -2,7 +2,6 @@ package ingress import ( "context" - "fmt" "github.com/pkg/errors" "github.com/stretchr/testify/assert" networking "k8s.io/api/networking/v1" @@ -444,7 +443,6 @@ func Test_defaultModelBuildTask_buildIngressLoadBalancerMinimumCapacity(t *testi } got, err := task.buildIngressLoadBalancerMinimumCapacity(tt.args.ing) if tt.wantErr != nil { - fmt.Println(err) assert.EqualError(t, err, tt.wantErr.Error()) } else { assert.NoError(t, err) diff --git a/test/e2e/ingress/utils.go b/test/e2e/ingress/utils.go index 609143e708..dc4c82a8ee 100644 --- a/test/e2e/ingress/utils.go +++ b/test/e2e/ingress/utils.go @@ -1,7 +1,6 @@ package ingress import ( - "fmt" "strings" networking "k8s.io/api/networking/v1" @@ -19,7 +18,6 @@ func FindIngressDNSName(ing *networking.Ingress) string { func FindIngressTwoDNSName(ing *networking.Ingress) (albDNS string, nlbDNS string) { for _, ingSTS := range ing.Status.LoadBalancer.Ingress { if ingSTS.Hostname != "" { - fmt.Println("ingSTS.Hostname", ingSTS.Hostname) if strings.Contains(ingSTS.Hostname, "elb.amazonaws.com") { albDNS = ingSTS.Hostname } else {