diff --git a/controllers/gateway/eventhandlers/gateway_class_events.go b/controllers/gateway/eventhandlers/gateway_class_events.go index cf217bcd1b..49bf5184f4 100644 --- a/controllers/gateway/eventhandlers/gateway_class_events.go +++ b/controllers/gateway/eventhandlers/gateway_class_events.go @@ -16,13 +16,12 @@ import ( // NewEnqueueRequestsForGatewayClassEvent creates handler for GatewayClass resources func NewEnqueueRequestsForGatewayClassEvent( - k8sClient client.Client, eventRecorder record.EventRecorder, gwController string, finalizerManager k8s.FinalizerManager, logger logr.Logger) handler.TypedEventHandler[*gatewayv1.GatewayClass, reconcile.Request] { + k8sClient client.Client, eventRecorder record.EventRecorder, gwController string, logger logr.Logger) handler.TypedEventHandler[*gatewayv1.GatewayClass, reconcile.Request] { return &enqueueRequestsForGatewayClassEvent{ - k8sClient: k8sClient, - finalizerManager: finalizerManager, - eventRecorder: eventRecorder, - gwController: gwController, - logger: logger, + k8sClient: k8sClient, + eventRecorder: eventRecorder, + gwController: gwController, + logger: logger, } } @@ -30,11 +29,10 @@ var _ handler.TypedEventHandler[*gatewayv1.GatewayClass, reconcile.Request] = (* // enqueueRequestsForGatewayClassEvent handles GatewayClass events type enqueueRequestsForGatewayClassEvent struct { - k8sClient client.Client - eventRecorder record.EventRecorder - gwController string - finalizerManager k8s.FinalizerManager - logger logr.Logger + k8sClient client.Client + eventRecorder record.EventRecorder + gwController string + logger logr.Logger } func (h *enqueueRequestsForGatewayClassEvent) Create(ctx context.Context, e event.TypedCreateEvent[*gatewayv1.GatewayClass], queue workqueue.TypedRateLimitingInterface[reconcile.Request]) { @@ -60,7 +58,12 @@ func (h *enqueueRequestsForGatewayClassEvent) Generic(ctx context.Context, e eve } func (h *enqueueRequestsForGatewayClassEvent) enqueueImpactedGateways(ctx context.Context, gwClass *gatewayv1.GatewayClass, queue workqueue.TypedRateLimitingInterface[reconcile.Request]) { - gwList := gatewayutils.GetGatewaysManagedByGatewayClass(ctx, h.k8sClient, gwClass, h.gwController) + gwList, err := gatewayutils.GetGatewaysManagedByGatewayClass(ctx, h.k8sClient, gwClass) + + if err != nil { + h.logger.Error(err, "failed to get gateways managed by gatewayclass", "gwClass", gwClass.Name) + return + } for _, gw := range gwList { h.logger.V(1).Info("enqueue gateway for gatewayclass event", diff --git a/controllers/gateway/eventhandlers/gatewayclass/load_balancer_configuration_events.go b/controllers/gateway/eventhandlers/gatewayclass/load_balancer_configuration_events.go index 50046e8d51..4ee0ba32a7 100644 --- a/controllers/gateway/eventhandlers/gatewayclass/load_balancer_configuration_events.go +++ b/controllers/gateway/eventhandlers/gatewayclass/load_balancer_configuration_events.go @@ -9,7 +9,6 @@ import ( elbv2gw "sigs.k8s.io/aws-load-balancer-controller/apis/gateway/v1beta1" "sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/gatewayutils" "sigs.k8s.io/aws-load-balancer-controller/pkg/k8s" - "sigs.k8s.io/aws-load-balancer-controller/pkg/shared_constants" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" @@ -19,13 +18,12 @@ import ( // NewEnqueueRequestsForLoadBalancerConfigurationEvent creates handler for LoadBalancerConfiguration resources func NewEnqueueRequestsForLoadBalancerConfigurationEvent(gwClassEventChan chan<- event.TypedGenericEvent[*gatewayv1.GatewayClass], - k8sClient client.Client, eventRecorder record.EventRecorder, gwControllers sets.Set[string], finalizerManager k8s.FinalizerManager, logger logr.Logger) handler.TypedEventHandler[*elbv2gw.LoadBalancerConfiguration, reconcile.Request] { + k8sClient client.Client, eventRecorder record.EventRecorder, gwControllers sets.Set[string], logger logr.Logger) handler.TypedEventHandler[*elbv2gw.LoadBalancerConfiguration, reconcile.Request] { return &enqueueRequestsForLoadBalancerConfigurationEvent{ gwClassEventChan: gwClassEventChan, k8sClient: k8sClient, eventRecorder: eventRecorder, gwControllers: gwControllers, - finalizerManager: finalizerManager, logger: logger, } } @@ -38,7 +36,6 @@ type enqueueRequestsForLoadBalancerConfigurationEvent struct { k8sClient client.Client eventRecorder record.EventRecorder gwControllers sets.Set[string] - finalizerManager k8s.FinalizerManager logger logr.Logger } @@ -51,14 +48,6 @@ func (h *enqueueRequestsForLoadBalancerConfigurationEvent) Create(ctx context.Co func (h *enqueueRequestsForLoadBalancerConfigurationEvent) Update(ctx context.Context, e event.TypedUpdateEvent[*elbv2gw.LoadBalancerConfiguration], queue workqueue.TypedRateLimitingInterface[reconcile.Request]) { lbconfigNew := e.ObjectNew h.logger.V(1).Info("enqueue loadbalancerconfiguration update event", "loadbalancerconfiguration", k8s.NamespacedName(lbconfigNew)) - // to remove finalizers on this residual unused lb config so that the deletion can be done on these - if !lbconfigNew.DeletionTimestamp.IsZero() && k8s.HasFinalizer(lbconfigNew, shared_constants.LoadBalancerConfigurationFinalizer) && !gatewayutils.IsLBConfigInUse(ctx, lbconfigNew, nil, nil, h.k8sClient) { - if err := h.finalizerManager.RemoveFinalizers(ctx, lbconfigNew, shared_constants.LoadBalancerConfigurationFinalizer); err != nil { - h.logger.V(1).Info("failed to remove finalizers on load balancer configuration as its currently in use", "load balancer configuration", lbconfigNew.Name) - return - } - return - } h.enqueueImpactedGatewayClass(ctx, lbconfigNew, queue) } @@ -75,7 +64,12 @@ func (h *enqueueRequestsForLoadBalancerConfigurationEvent) Generic(ctx context.C } func (h *enqueueRequestsForLoadBalancerConfigurationEvent) enqueueImpactedGatewayClass(ctx context.Context, lbconfig *elbv2gw.LoadBalancerConfiguration, queue workqueue.TypedRateLimitingInterface[reconcile.Request]) { - gwClasses := gatewayutils.GetImpactedGatewayClassesFromLbConfig(ctx, h.k8sClient, lbconfig, h.gwControllers) + gwClasses, err := gatewayutils.GetImpactedGatewayClassesFromLbConfig(ctx, h.k8sClient, lbconfig, h.gwControllers) + if err != nil { + h.logger.Error(err, "failed to get impacted gatewayClasses from loadbalancerconfiguration event", + "loadbalancerconfiguration", k8s.NamespacedName(lbconfig)) + return + } for _, gwClass := range gwClasses { h.logger.V(1).Info("enqueue gatewayClass for loadbalancerconfiguration event", "loadbalancerconfiguration", k8s.NamespacedName(lbconfig), diff --git a/controllers/gateway/eventhandlers/load_balancer_configuration_events.go b/controllers/gateway/eventhandlers/load_balancer_configuration_events.go index f31fca8bb9..34c3919e70 100644 --- a/controllers/gateway/eventhandlers/load_balancer_configuration_events.go +++ b/controllers/gateway/eventhandlers/load_balancer_configuration_events.go @@ -68,7 +68,11 @@ func (h *enqueueRequestsForLoadBalancerConfigurationEvent) Generic(ctx context.C func (h *enqueueRequestsForLoadBalancerConfigurationEvent) enqueueImpactedService(ctx context.Context, lbconfig *elbv2gw.LoadBalancerConfiguration, queue workqueue.TypedRateLimitingInterface[reconcile.Request]) { // NOTE: That LB Config changes for GatewayClass are done a little differently. // LB config change -> gateway class reconciler -> patch status for new version of LB config on Gateway Class -> Trigger the Gateway Class event handler. - gateways := gatewayutils.GetImpactedGatewaysFromLbConfig(ctx, h.k8sClient, lbconfig, h.gwController) + gateways, err := gatewayutils.GetImpactedGatewaysFromLbConfig(ctx, h.k8sClient, lbconfig, h.gwController) + if err != nil { + h.logger.Error(err, "failed to get impacted gateways from loadbalancerconfiguration", "loadbalancerconfiguration", k8s.NamespacedName(lbconfig)) + return + } for _, gw := range gateways { h.logger.V(1).Info("enqueue gateway for loadbalancerconfiguration event", "loadbalancerconfiguration", k8s.NamespacedName(lbconfig), diff --git a/controllers/gateway/eventhandlers/service_events.go b/controllers/gateway/eventhandlers/service_events.go index 91be2bdf15..2fa7573c6f 100644 --- a/controllers/gateway/eventhandlers/service_events.go +++ b/controllers/gateway/eventhandlers/service_events.go @@ -65,9 +65,6 @@ func (h *enqueueRequestsForServiceEvent) Update(ctx context.Context, e event.Typ func (h *enqueueRequestsForServiceEvent) Delete(ctx context.Context, e event.TypedDeleteEvent[*corev1.Service], queue workqueue.TypedRateLimitingInterface[reconcile.Request]) { svc := e.Object h.logger.V(1).Info("enqueue service delete event", "service", svc.Name) - // remove target group configuration finalizer when service is deleted - routeutils.RemoveTargetGroupConfigurationFinalizer(ctx, svc, h.k8sClient, h.logger, h.eventRecorder) - h.enqueueImpactedRoutes(ctx, svc) } diff --git a/controllers/gateway/gateway_class_controller.go b/controllers/gateway/gateway_class_controller.go index a764804dee..8e42934502 100644 --- a/controllers/gateway/gateway_class_controller.go +++ b/controllers/gateway/gateway_class_controller.go @@ -14,6 +14,7 @@ import ( "sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/gatewayutils" "sigs.k8s.io/aws-load-balancer-controller/pkg/k8s" "sigs.k8s.io/aws-load-balancer-controller/pkg/runtime" + "sigs.k8s.io/aws-load-balancer-controller/pkg/shared_constants" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" @@ -42,6 +43,7 @@ func NewGatewayClassReconciler(k8sClient client.Client, eventRecorder record.Eve updateGwClassAcceptedFn: updateGatewayClassAcceptedCondition, updateLastProcessedConfigFn: updateGatewayClassLastProcessedConfig, configResolverFn: gatewayutils.ResolveLoadBalancerConfig, + gatewayResolverFn: gatewayutils.GetGatewaysManagedByGatewayClass, } } @@ -57,12 +59,13 @@ type gatewayClassReconciler struct { updateGwClassAcceptedFn func(ctx context.Context, k8sClient client.Client, gwClass *gwv1.GatewayClass, status metav1.ConditionStatus, reason string, message string) error updateLastProcessedConfigFn func(ctx context.Context, k8sClient client.Client, gwClass *gwv1.GatewayClass, lbConf *elbv2gw.LoadBalancerConfiguration) error configResolverFn func(ctx context.Context, k8sClient client.Client, reference *gwv1.ParametersReference) (*elbv2gw.LoadBalancerConfiguration, error) + gatewayResolverFn func(ctx context.Context, k8sClient client.Client, gwClass *gwv1.GatewayClass) ([]*gwv1.Gateway, error) } func (r *gatewayClassReconciler) SetupWatches(_ context.Context, ctrl controller.Controller, mgr ctrl.Manager) error { gwClassEventChan := make(chan event.TypedGenericEvent[*gwv1.GatewayClass]) - lbEventHandler := gatewayclasseventhandlers.NewEnqueueRequestsForLoadBalancerConfigurationEvent(gwClassEventChan, r.k8sClient, r.eventRecorder, r.enabledControllers, r.finalizerManager, r.logger) + lbEventHandler := gatewayclasseventhandlers.NewEnqueueRequestsForLoadBalancerConfigurationEvent(gwClassEventChan, r.k8sClient, r.eventRecorder, r.enabledControllers, r.logger) if err := ctrl.Watch(source.Kind(mgr.GetCache(), &gwv1.GatewayClass{}, &handler.TypedEnqueueRequestForObject[*gwv1.GatewayClass]{})); err != nil { return err @@ -100,6 +103,21 @@ func (r *gatewayClassReconciler) reconcile(ctx context.Context, req reconcile.Re return nil } + if gwClass.DeletionTimestamp == nil || gwClass.DeletionTimestamp.IsZero() { + return r.handleUpdate(ctx, gwClass) + } + + return r.handleDelete(ctx, gwClass) +} + +func (r *gatewayClassReconciler) handleUpdate(ctx context.Context, gwClass *gwv1.GatewayClass) error { + if !k8s.HasFinalizer(gwClass, shared_constants.GatewayClassFinalizer) { + err := r.finalizerManager.AddFinalizers(context.Background(), gwClass, shared_constants.GatewayClassFinalizer) + if err != nil { + return err + } + } + var lbConf *elbv2gw.LoadBalancerConfiguration lbConf, err := r.configResolverFn(ctx, r.k8sClient, gwClass.Spec.ParametersRef) @@ -113,19 +131,32 @@ func (r *gatewayClassReconciler) reconcile(ctx context.Context, req reconcile.Re err = r.updateLastProcessedConfigFn(ctx, r.k8sClient, gwClass, lbConf) if err != nil { - r.logger.Error(err, "Unable to update last processed annotation") return err } err = r.updateGwClassAcceptedFn(ctx, r.k8sClient, gwClass, metav1.ConditionTrue, string(gwv1.GatewayClassReasonAccepted), string(gwv1.GatewayClassReasonAccepted)) if err != nil { - r.logger.Error(err, "Unable to update condition") return err } return nil } +func (r *gatewayClassReconciler) handleDelete(ctx context.Context, gwClass *gwv1.GatewayClass) error { + if !k8s.HasFinalizer(gwClass, shared_constants.GatewayClassFinalizer) { + return nil + } + + refCount, err := r.gatewayResolverFn(ctx, r.k8sClient, gwClass) + if err != nil { + return err + } + if len(refCount) != 0 { + return fmt.Errorf("unable to delete GatewayClass [%+v], as it is still referenced by Gateways", gwClass.Name) + } + return r.finalizerManager.RemoveFinalizers(ctx, gwClass, shared_constants.GatewayClassFinalizer) +} + func (r *gatewayClassReconciler) getNotFoundMessage(paramRef *gwv1.ParametersReference) string { var ns string if paramRef.Namespace == nil { diff --git a/controllers/gateway/gateway_controller.go b/controllers/gateway/gateway_controller.go index 64b2e65696..931f058ce4 100644 --- a/controllers/gateway/gateway_controller.go +++ b/controllers/gateway/gateway_controller.go @@ -19,8 +19,8 @@ import ( elbv2deploy "sigs.k8s.io/aws-load-balancer-controller/pkg/deploy/elbv2" "sigs.k8s.io/aws-load-balancer-controller/pkg/deploy/tracking" "sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/constants" - "sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/gatewayutils" gatewaymodel "sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/model" + "sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/referencecounter" "sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/routeutils" "sigs.k8s.io/aws-load-balancer-controller/pkg/k8s" lbcmetrics "sigs.k8s.io/aws-load-balancer-controller/pkg/metrics/lbc" @@ -49,18 +49,18 @@ const ( var _ Reconciler = &gatewayReconciler{} // NewNLBGatewayReconciler constructs a gateway reconciler to handle specifically for NLB gateways -func NewNLBGatewayReconciler(routeLoader routeutils.Loader, 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, routeutils.L4RouteFilter, cloud, k8sClient, eventRecorder, controllerConfig, finalizerManager, networkingSGReconciler, networkingManager, networkingSGManager, elbv2TaggingManager, subnetResolver, vpcInfoProvider, backendSGProvider, sgResolver, 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, 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, logger, metricsCollector, reconcileCounters.IncrementNLBGateway, routeReconciler) } // NewALBGatewayReconciler constructs a gateway reconciler to handle specifically for ALB gateways -func NewALBGatewayReconciler(routeLoader routeutils.Loader, 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.ALBGatewayController, elbv2model.LoadBalancerTypeApplication, controllerConfig.ALBGatewayMaxConcurrentReconciles, constants.ALBGatewayTagPrefix, shared_constants.ALBGatewayFinalizer, routeLoader, routeutils.L7RouteFilter, cloud, k8sClient, eventRecorder, controllerConfig, finalizerManager, networkingSGReconciler, networkingManager, networkingSGManager, elbv2TaggingManager, subnetResolver, vpcInfoProvider, backendSGProvider, sgResolver, 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, 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, logger, metricsCollector, reconcileCounters.IncrementALBGateway, routeReconciler) } // newGatewayReconciler constructs a reconciler that responds to gateway object changes func newGatewayReconciler(controllerName string, lbType elbv2model.LoadBalancerType, maxConcurrentReconciles int, - gatewayTagPrefix string, finalizer string, routeLoader routeutils.Loader, routeFilter routeutils.LoadRouteFilter, + gatewayTagPrefix string, finalizer string, routeLoader routeutils.Loader, serviceReferenceCounter referencecounter.ServiceReferenceCounter, routeFilter routeutils.LoadRouteFilter, cloud services.Cloud, k8sClient client.Client, eventRecorder record.EventRecorder, controllerConfig config.ControllerConfig, finalizerManager k8s.FinalizerManager, networkingSGReconciler networking.SecurityGroupReconciler, networkingManager networking.NetworkingManager, networkingSGManager networking.SecurityGroupManager, elbv2TaggingManager elbv2deploy.TaggingManager, @@ -95,6 +95,7 @@ func newGatewayReconciler(controllerName string, lbType elbv2model.LoadBalancerT reconcileTracker: reconcileTracker, cfgResolver: cfgResolver, routeReconciler: routeReconciler, + serviceReferenceCounter: serviceReferenceCounter, gatewayConditionUpdater: prepareGatewayConditionUpdate, } } @@ -117,6 +118,7 @@ type gatewayReconciler struct { logger logr.Logger metricsCollector lbcmetrics.MetricCollector reconcileTracker func(namespaceName types.NamespacedName) + serviceReferenceCounter referencecounter.ServiceReferenceCounter gatewayConditionUpdater func(gw *gwv1.Gateway, targetConditionType string, newStatus metav1.ConditionStatus, reason string, message string) bool cfgResolver gatewayConfigResolver @@ -224,17 +226,24 @@ func (r *gatewayReconciler) reconcileHelper(ctx context.Context, req reconcile.R } if lb == nil { - err = r.reconcileDelete(ctx, gw, gwClass, stack, allRoutes) + err = r.reconcileDelete(ctx, gw, stack, allRoutes) if err != nil { r.logger.Error(err, "Failed to process gateway delete") + return err } + r.serviceReferenceCounter.UpdateRelations([]types.NamespacedName{}, k8s.NamespacedName(gw), true) + return nil + } + r.serviceReferenceCounter.UpdateRelations(getServicesFromRoutes(allRoutes), k8s.NamespacedName(gw), false) + err = r.reconcileUpdate(ctx, gw, gwClass, stack, lb, backendSGRequired) + if err != nil { + r.logger.Error(err, "Failed to process gateway update", "gw", k8s.NamespacedName(gw)) return err } - - return r.reconcileUpdate(ctx, gw, gwClass, stack, lb, backendSGRequired) + return nil } -func (r *gatewayReconciler) reconcileDelete(ctx context.Context, gw *gwv1.Gateway, gwClass *gwv1.GatewayClass, stack core.Stack, routes map[int32][]routeutils.RouteDescriptor) error { +func (r *gatewayReconciler) reconcileDelete(ctx context.Context, gw *gwv1.Gateway, stack core.Stack, routes map[int32][]routeutils.RouteDescriptor) error { for _, routeList := range routes { if len(routeList) != 0 { err := errors.Errorf("Gateway deletion invoked with routes attached [%s]", generateRouteList(routes)) @@ -251,11 +260,6 @@ func (r *gatewayReconciler) reconcileDelete(ctx context.Context, gw *gwv1.Gatewa if err := r.backendSGProvider.Release(ctx, networking.ResourceTypeGateway, []types.NamespacedName{k8s.NamespacedName(gw)}); err != nil { return err } - // remove load balancer configuration finalizer - if err := gatewayutils.RemoveLoadBalancerConfigurationFinalizers(ctx, gw, gwClass, r.k8sClient, r.finalizerManager); err != nil { - r.eventRecorder.Event(gw, corev1.EventTypeWarning, k8s.LoadBalancerConfigurationEventReasonFailedRemoveFinalizer, fmt.Sprintf("Failed remove load balancer configuration finalizer due to %v", err)) - return err - } // remove gateway finalizer if err := r.finalizerManager.RemoveFinalizers(ctx, gw, r.finalizer); err != nil { r.eventRecorder.Event(gw, corev1.EventTypeWarning, k8s.GatewayEventReasonFailedRemoveFinalizer, fmt.Sprintf("Failed remove gateway finalizer due to %v", err)) @@ -418,7 +422,7 @@ func (r *gatewayReconciler) setupCommonGatewayControllerWatches(ctrl controller. gwClassEventChan := make(chan event.TypedGenericEvent[*gwv1.GatewayClass]) lbConfigEventChan := make(chan event.TypedGenericEvent[*elbv2gw.LoadBalancerConfiguration]) - gwClassEventHandler := eventhandlers.NewEnqueueRequestsForGatewayClassEvent(r.k8sClient, r.eventRecorder, r.controllerName, r.finalizerManager, + gwClassEventHandler := eventhandlers.NewEnqueueRequestsForGatewayClassEvent(r.k8sClient, r.eventRecorder, r.controllerName, loggerPrefix.WithName("GatewayClass")) lbConfigEventHandler := eventhandlers.NewEnqueueRequestsForLoadBalancerConfigurationEvent(gwClassEventChan, r.k8sClient, r.eventRecorder, r.controllerName, loggerPrefix.WithName("LoadBalancerConfiguration")) @@ -535,7 +539,5 @@ func isGatewayProgrammed(lbStatus elbv2model.LoadBalancerStatus) bool { if lbStatus.ProvisioningState == nil { return false } - return lbStatus.ProvisioningState.Code == elbv2types.LoadBalancerStateEnumActive || lbStatus.ProvisioningState.Code == elbv2types.LoadBalancerStateEnumActiveImpaired - } diff --git a/controllers/gateway/loadbalancer_configuration_controller.go b/controllers/gateway/loadbalancer_configuration_controller.go new file mode 100644 index 0000000000..97c1af361b --- /dev/null +++ b/controllers/gateway/loadbalancer_configuration_controller.go @@ -0,0 +1,102 @@ +package gateway + +import ( + "context" + "fmt" + "github.com/go-logr/logr" + "k8s.io/client-go/tools/record" + elbv2gw "sigs.k8s.io/aws-load-balancer-controller/apis/gateway/v1beta1" + "sigs.k8s.io/aws-load-balancer-controller/pkg/config" + "sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/constants" + "sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/gatewayutils" + "sigs.k8s.io/aws-load-balancer-controller/pkg/k8s" + "sigs.k8s.io/aws-load-balancer-controller/pkg/runtime" + "sigs.k8s.io/aws-load-balancer-controller/pkg/shared_constants" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + "sigs.k8s.io/controller-runtime/pkg/source" +) + +// NewLoadbalancerConfigurationReconciler constructs a reconciler that responds to loadbalancer configuration changes +func NewLoadbalancerConfigurationReconciler(k8sClient client.Client, eventRecorder record.EventRecorder, controllerConfig config.ControllerConfig, finalizerManager k8s.FinalizerManager, logger logr.Logger) Reconciler { + + return &loadbalancerConfigurationReconciler{ + k8sClient: k8sClient, + eventRecorder: eventRecorder, + logger: logger, + finalizerManager: finalizerManager, + workers: controllerConfig.GatewayClassMaxConcurrentReconciles, + } +} + +// loadbalancerConfigurationReconciler reconciles load balancer configurations +type loadbalancerConfigurationReconciler struct { + k8sClient client.Client + logger logr.Logger + eventRecorder record.EventRecorder + finalizerManager k8s.FinalizerManager + workers int +} + +func (r *loadbalancerConfigurationReconciler) SetupWatches(_ context.Context, ctrl controller.Controller, mgr ctrl.Manager) error { + + if err := ctrl.Watch(source.Kind(mgr.GetCache(), &elbv2gw.LoadBalancerConfiguration{}, &handler.TypedEnqueueRequestForObject[*elbv2gw.LoadBalancerConfiguration]{})); err != nil { + return err + } + + return nil +} + +func (r *loadbalancerConfigurationReconciler) Reconcile(ctx context.Context, req reconcile.Request) (ctrl.Result, error) { + return runtime.HandleReconcileError(r.reconcile(ctx, req), r.logger) +} + +func (r *loadbalancerConfigurationReconciler) reconcile(ctx context.Context, req reconcile.Request) error { + lbConf := &elbv2gw.LoadBalancerConfiguration{} + if err := r.k8sClient.Get(ctx, req.NamespacedName, lbConf); err != nil { + return client.IgnoreNotFound(err) + } + + r.logger.V(1).Info("Found loadbalancer configuration", "cfg", lbConf) + + if lbConf.DeletionTimestamp == nil || lbConf.DeletionTimestamp.IsZero() { + return r.handleUpdate(lbConf) + } + + return r.handleDelete(lbConf) +} + +func (r *loadbalancerConfigurationReconciler) handleUpdate(lbConf *elbv2gw.LoadBalancerConfiguration) error { + if k8s.HasFinalizer(lbConf, shared_constants.LoadBalancerConfigurationFinalizer) { + return nil + } + return r.finalizerManager.AddFinalizers(context.Background(), lbConf, shared_constants.LoadBalancerConfigurationFinalizer) +} + +func (r *loadbalancerConfigurationReconciler) handleDelete(lbConf *elbv2gw.LoadBalancerConfiguration) error { + if !k8s.HasFinalizer(lbConf, shared_constants.LoadBalancerConfigurationFinalizer) { + return nil + } + + inUse, err := gatewayutils.IsLBConfigInUse(context.Background(), lbConf, r.k8sClient, constants.FullGatewayControllerSet) + + if err != nil { + return err + } + // if the loadbalancer configuration is still in use, we should not delete it + if inUse { + return fmt.Errorf("loadbalancer configuration [%+v] is still in use", k8s.NamespacedName(lbConf)) + } + return r.finalizerManager.RemoveFinalizers(context.Background(), lbConf, shared_constants.LoadBalancerConfigurationFinalizer) +} + +func (r *loadbalancerConfigurationReconciler) SetupWithManager(_ context.Context, mgr ctrl.Manager) (controller.Controller, error) { + return controller.New(constants.LoadBalancerConfigurationController, mgr, controller.Options{ + MaxConcurrentReconciles: r.workers, + Reconciler: r, + }) + +} diff --git a/controllers/gateway/targetgroup_configuration_controller.go b/controllers/gateway/targetgroup_configuration_controller.go new file mode 100644 index 0000000000..59bed4d66a --- /dev/null +++ b/controllers/gateway/targetgroup_configuration_controller.go @@ -0,0 +1,124 @@ +package gateway + +import ( + "context" + "fmt" + "github.com/go-logr/logr" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" + elbv2gw "sigs.k8s.io/aws-load-balancer-controller/apis/gateway/v1beta1" + "sigs.k8s.io/aws-load-balancer-controller/pkg/config" + "sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/constants" + "sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/gatewayutils" + "sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/referencecounter" + "sigs.k8s.io/aws-load-balancer-controller/pkg/k8s" + "sigs.k8s.io/aws-load-balancer-controller/pkg/runtime" + "sigs.k8s.io/aws-load-balancer-controller/pkg/shared_constants" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + "sigs.k8s.io/controller-runtime/pkg/source" + gwv1 "sigs.k8s.io/gateway-api/apis/v1" +) + +// NewTargetGroupConfigurationReconciler constructs a reconciler that responds to targetgroup configuration changes +func NewTargetGroupConfigurationReconciler(k8sClient client.Client, eventRecorder record.EventRecorder, controllerConfig config.ControllerConfig, serviceReferenceCounter referencecounter.ServiceReferenceCounter, finalizerManager k8s.FinalizerManager, logger logr.Logger) Reconciler { + + return &targetgroupConfigurationReconciler{ + k8sClient: k8sClient, + eventRecorder: eventRecorder, + logger: logger, + finalizerManager: finalizerManager, + serviceReferenceCounter: serviceReferenceCounter, + gwRetrieveFn: gatewayutils.GetGatewaysManagedByLBController, + workers: controllerConfig.GatewayClassMaxConcurrentReconciles, + } +} + +// targetgroupConfigurationReconciler reconciles target group configurations +type targetgroupConfigurationReconciler struct { + k8sClient client.Client + logger logr.Logger + eventRecorder record.EventRecorder + finalizerManager k8s.FinalizerManager + serviceReferenceCounter referencecounter.ServiceReferenceCounter + + gwRetrieveFn func(ctx context.Context, k8sClient client.Client, gwController string) ([]*gwv1.Gateway, error) + workers int +} + +func (r *targetgroupConfigurationReconciler) SetupWatches(_ context.Context, ctrl controller.Controller, mgr ctrl.Manager) error { + + if err := ctrl.Watch(source.Kind(mgr.GetCache(), &elbv2gw.TargetGroupConfiguration{}, &handler.TypedEnqueueRequestForObject[*elbv2gw.TargetGroupConfiguration]{})); err != nil { + return err + } + + return nil +} + +func (r *targetgroupConfigurationReconciler) Reconcile(ctx context.Context, req reconcile.Request) (ctrl.Result, error) { + return runtime.HandleReconcileError(r.reconcile(ctx, req), r.logger) +} + +func (r *targetgroupConfigurationReconciler) reconcile(ctx context.Context, req reconcile.Request) error { + tgConf := &elbv2gw.TargetGroupConfiguration{} + if err := r.k8sClient.Get(ctx, req.NamespacedName, tgConf); err != nil { + return client.IgnoreNotFound(err) + } + + r.logger.V(1).Info("Found tg configuration", "cfg", tgConf) + + if tgConf.DeletionTimestamp == nil || tgConf.DeletionTimestamp.IsZero() { + return r.handleUpdate(tgConf) + } + return r.handleDelete(tgConf) +} + +func (r *targetgroupConfigurationReconciler) handleUpdate(tgConf *elbv2gw.TargetGroupConfiguration) error { + if k8s.HasFinalizer(tgConf, shared_constants.TargetGroupConfigurationFinalizer) { + return nil + } + return r.finalizerManager.AddFinalizers(context.Background(), tgConf, shared_constants.TargetGroupConfigurationFinalizer) +} + +func (r *targetgroupConfigurationReconciler) handleDelete(tgConf *elbv2gw.TargetGroupConfiguration) error { + if !k8s.HasFinalizer(tgConf, shared_constants.TargetGroupConfigurationFinalizer) { + return nil + } + + allGateways := make([]types.NamespacedName, 0) + + for _, c := range constants.FullGatewayControllerSet.UnsortedList() { + partial, err := r.gwRetrieveFn(context.Background(), r.k8sClient, c) + if err != nil { + return err + } + + for _, gw := range partial { + allGateways = append(allGateways, k8s.NamespacedName(gw)) + } + } + + svcReference := types.NamespacedName{ + Namespace: tgConf.Namespace, + Name: tgConf.Spec.TargetReference.Name, + } + + eligibleForRemoval := r.serviceReferenceCounter.IsEligibleForRemoval(svcReference, allGateways) + + // if the targetgroup configuration is still in use, we should not delete it + if !eligibleForRemoval { + return fmt.Errorf("targetgroup configuration [%+v] is still in use", k8s.NamespacedName(tgConf)) + } + return r.finalizerManager.RemoveFinalizers(context.Background(), tgConf, shared_constants.TargetGroupConfigurationFinalizer) +} + +func (r *targetgroupConfigurationReconciler) SetupWithManager(_ context.Context, mgr ctrl.Manager) (controller.Controller, error) { + return controller.New(constants.TargetGroupConfigurationController, mgr, controller.Options{ + MaxConcurrentReconciles: r.workers, + Reconciler: r, + }) + +} diff --git a/controllers/gateway/utils.go b/controllers/gateway/utils.go index 0cc2b0ad10..d89c32d9ba 100644 --- a/controllers/gateway/utils.go +++ b/controllers/gateway/utils.go @@ -5,8 +5,11 @@ import ( "fmt" "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" elbv2gw "sigs.k8s.io/aws-load-balancer-controller/apis/gateway/v1beta1" "sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/routeutils" + "sigs.k8s.io/aws-load-balancer-controller/pkg/k8s" "sigs.k8s.io/controller-runtime/pkg/client" gwv1 "sigs.k8s.io/gateway-api/apis/v1" "sort" @@ -159,3 +162,18 @@ func generateRouteList(listenerRoutes map[int32][]routeutils.RouteDescriptor) st return strings.Join(allRoutes, ",") } + +func getServicesFromRoutes(listenerRouteMap map[int32][]routeutils.RouteDescriptor) []types.NamespacedName { + res := sets.New[types.NamespacedName]() + + for _, routes := range listenerRouteMap { + for _, route := range routes { + for _, rr := range route.GetAttachedRules() { + for _, be := range rr.GetBackends() { + res.Insert(k8s.NamespacedName(be.Service)) + } + } + } + } + return res.UnsortedList() +} diff --git a/main.go b/main.go index 2737932c5d..0d6df91c64 100644 --- a/main.go +++ b/main.go @@ -25,6 +25,7 @@ import ( "sigs.k8s.io/aws-load-balancer-controller/controllers/gateway" "sigs.k8s.io/aws-load-balancer-controller/pkg/aws/services" gateway_constants "sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/constants" + "sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/referencecounter" "sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/routeutils" "sigs.k8s.io/controller-runtime/pkg/client" gwv1 "sigs.k8s.io/gateway-api/apis/v1" @@ -86,22 +87,23 @@ func init() { // Define a struct to hold common gateway controller dependencies type gatewayControllerConfig struct { - routeLoader routeutils.Loader - cloud services.Cloud - k8sClient client.Client - controllerCFG config.ControllerConfig - finalizerManager k8s.FinalizerManager - sgReconciler networking.SecurityGroupReconciler - sgManager networking.SecurityGroupManager - elbv2TaggingManager elbv2deploy.TaggingManager - subnetResolver networking.SubnetsResolver - vpcInfoProvider networking.VPCInfoProvider - backendSGProvider networking.BackendSGProvider - sgResolver networking.SecurityGroupResolver - metricsCollector lbcmetrics.MetricCollector - reconcileCounters *metricsutil.ReconcileCounters - routeReconciler routeutils.RouteReconciler - networkingManager networking.NetworkingManager + routeLoader routeutils.Loader + cloud services.Cloud + k8sClient client.Client + controllerCFG config.ControllerConfig + finalizerManager k8s.FinalizerManager + sgReconciler networking.SecurityGroupReconciler + sgManager networking.SecurityGroupManager + elbv2TaggingManager elbv2deploy.TaggingManager + subnetResolver networking.SubnetsResolver + vpcInfoProvider networking.VPCInfoProvider + backendSGProvider networking.BackendSGProvider + sgResolver networking.SecurityGroupResolver + metricsCollector lbcmetrics.MetricCollector + reconcileCounters *metricsutil.ReconcileCounters + routeReconciler routeutils.RouteReconciler + serviceReferenceCounter referencecounter.ServiceReferenceCounter + networkingManager networking.NetworkingManager } func main() { @@ -223,23 +225,25 @@ func main() { Name: "gateway-route-status-update-reconciler", }) routeReconciler := gateway.NewRouteReconciler(delayingQueue, mgr.GetClient(), ctrl.Log.WithName("routeReconciler")) + serviceReferenceCounter := referencecounter.NewServiceReferenceCounter() gwControllerConfig := &gatewayControllerConfig{ - cloud: cloud, - k8sClient: mgr.GetClient(), - controllerCFG: controllerCFG, - finalizerManager: finalizerManager, - sgReconciler: sgReconciler, - sgManager: sgManager, - elbv2TaggingManager: elbv2TaggingManager, - subnetResolver: subnetResolver, - vpcInfoProvider: vpcInfoProvider, - backendSGProvider: backendSGProvider, - sgResolver: sgResolver, - metricsCollector: lbcMetricsCollector, - reconcileCounters: reconcileCounters, - routeReconciler: routeReconciler, - networkingManager: networkingManager, + cloud: cloud, + k8sClient: mgr.GetClient(), + controllerCFG: controllerCFG, + finalizerManager: finalizerManager, + sgReconciler: sgReconciler, + sgManager: sgManager, + elbv2TaggingManager: elbv2TaggingManager, + subnetResolver: subnetResolver, + vpcInfoProvider: vpcInfoProvider, + backendSGProvider: backendSGProvider, + sgResolver: sgResolver, + metricsCollector: lbcMetricsCollector, + reconcileCounters: reconcileCounters, + routeReconciler: routeReconciler, + networkingManager: networkingManager, + serviceReferenceCounter: serviceReferenceCounter, } enabledControllers := sets.Set[string]{} @@ -291,6 +295,47 @@ func main() { os.Exit(1) } + loadbalancerConfigurationReconciler := gateway.NewLoadbalancerConfigurationReconciler( + mgr.GetClient(), + mgr.GetEventRecorderFor(gateway_constants.LoadBalancerConfigurationController), + controllerCFG, + finalizerManager, + mgr.GetLogger().WithName("loadbalancerconfiguration-controller"), + ) + + lbCfgController, err := loadbalancerConfigurationReconciler.SetupWithManager(ctx, mgr) + if err != nil { + setupLog.Error(err, "Unable to set up LoadBalancerConfiguration Manager") + os.Exit(1) + } + + err = loadbalancerConfigurationReconciler.SetupWatches(ctx, lbCfgController, mgr) + if err != nil { + setupLog.Error(err, "Unable to set up LoadBalancerConfiguration Watches") + os.Exit(1) + } + + targetGroupConfigurationReconciler := gateway.NewTargetGroupConfigurationReconciler( + mgr.GetClient(), + mgr.GetEventRecorderFor(gateway_constants.LoadBalancerConfigurationController), + controllerCFG, + serviceReferenceCounter, + finalizerManager, + mgr.GetLogger().WithName("targetgroupconfiguration-controller"), + ) + + tgCfgController, err := targetGroupConfigurationReconciler.SetupWithManager(ctx, mgr) + if err != nil { + setupLog.Error(err, "Unable to set up TargetGroupConfiguration Manager") + os.Exit(1) + } + + err = targetGroupConfigurationReconciler.SetupWatches(ctx, tgCfgController, mgr) + if err != nil { + setupLog.Error(err, "Unable to set up TargetGroupConfiguration Watches") + os.Exit(1) + } + go func() { setupLog.Info("starting gateway route reconciler") routeReconciler.Run() @@ -366,6 +411,7 @@ func setupGatewayController(ctx context.Context, mgr ctrl.Manager, cfg *gatewayC case gateway_constants.NLBGatewayController: reconciler = gateway.NewNLBGatewayReconciler( cfg.routeLoader, + cfg.serviceReferenceCounter, cfg.cloud, cfg.k8sClient, mgr.GetEventRecorderFor(controllerType), @@ -389,6 +435,7 @@ func setupGatewayController(ctx context.Context, mgr ctrl.Manager, cfg *gatewayC cfg.routeLoader, cfg.cloud, cfg.k8sClient, + cfg.serviceReferenceCounter, mgr.GetEventRecorderFor(controllerType), cfg.controllerCFG, cfg.finalizerManager, diff --git a/pkg/gateway/constants/controller_constants.go b/pkg/gateway/constants/controller_constants.go index 2f257e2076..4e63a099a0 100644 --- a/pkg/gateway/constants/controller_constants.go +++ b/pkg/gateway/constants/controller_constants.go @@ -1,9 +1,15 @@ package constants +import "k8s.io/apimachinery/pkg/util/sets" + /* Common constants */ +var ( + FullGatewayControllerSet = sets.New(ALBGatewayController, NLBGatewayController) +) + const ( // GatewayResourceGroupVersion the groupVersion used by Gateway & GatewayClass resources. GatewayResourceGroupVersion = "gateway.networking.k8s.io/v1" @@ -45,4 +51,10 @@ const ( const ( // GatewayClassController the controller that reconciles gateway class changes GatewayClassController = "aws-lbc-gateway-class-controller" + + //LoadBalancerConfigurationController the controller that reconciles LoadBalancerConfiguration changes + LoadBalancerConfigurationController = "aws-lbc-loadbalancerconfiguration-controller" + + //TargetGroupConfigurationController the controller that reconciles TargetGroupConfiguration changes + TargetGroupConfigurationController = "aws-lbc-targetgroupconfiguration-controller" ) diff --git a/pkg/gateway/gatewayutils/gateway_utils.go b/pkg/gateway/gatewayutils/gateway_utils.go index ce16a3a89f..e5197e2af7 100644 --- a/pkg/gateway/gatewayutils/gateway_utils.go +++ b/pkg/gateway/gatewayutils/gateway_utils.go @@ -26,11 +26,11 @@ func IsGatewayManagedByLBController(ctx context.Context, k8sClient client.Client } // GetGatewayClassesManagedByLBController retrieves all GatewayClasses managed by the ALB/NLB Gateway Controller. -func GetGatewayClassesManagedByLBController(ctx context.Context, k8sClient client.Client, gwControllers sets.Set[string]) []*gwv1.GatewayClass { +func GetGatewayClassesManagedByLBController(ctx context.Context, k8sClient client.Client, gwControllers sets.Set[string]) ([]*gwv1.GatewayClass, error) { managedGatewayClasses := make([]*gwv1.GatewayClass, 0) gwClassList := &gwv1.GatewayClassList{} if err := k8sClient.List(ctx, gwClassList); err != nil { - return managedGatewayClasses + return managedGatewayClasses, err } managedGatewayClasses = make([]*gwv1.GatewayClass, 0, len(gwClassList.Items)) @@ -39,16 +39,16 @@ func GetGatewayClassesManagedByLBController(ctx context.Context, k8sClient clien managedGatewayClasses = append(managedGatewayClasses, &gwClassList.Items[i]) } } - return managedGatewayClasses + return managedGatewayClasses, nil } // GetGatewaysManagedByLBController retrieves all Gateways managed by the ALB/NLB Gateway Controller. -func GetGatewaysManagedByLBController(ctx context.Context, k8sClient client.Client, gwController string) []*gwv1.Gateway { +func GetGatewaysManagedByLBController(ctx context.Context, k8sClient client.Client, gwController string) ([]*gwv1.Gateway, error) { managedGateways := make([]*gwv1.Gateway, 0) gwList := &gwv1.GatewayList{} if err := k8sClient.List(ctx, gwList); err != nil { - return managedGateways + return managedGateways, err } managedGateways = make([]*gwv1.Gateway, 0, len(gwList.Items)) @@ -58,7 +58,7 @@ func GetGatewaysManagedByLBController(ctx context.Context, k8sClient client.Clie managedGateways = append(managedGateways, &gwList.Items[i]) } } - return managedGateways + return managedGateways, nil } // GetImpactedGatewaysFromParentRefs identifies Gateways affected by changes in parent references. @@ -105,47 +105,60 @@ func GetImpactedGatewaysFromParentRefs(ctx context.Context, k8sClient client.Cli // GetImpactedGatewayClassesFromLbConfig identifies GatewayClasses affected by LoadBalancer configuration changes. // Returns GatewayClasses that reference the specified LoadBalancer configuration. -func GetImpactedGatewayClassesFromLbConfig(ctx context.Context, k8sClient client.Client, lbconfig *elbv2gw.LoadBalancerConfiguration, gwControllers sets.Set[string]) map[string]*gwv1.GatewayClass { +func GetImpactedGatewayClassesFromLbConfig(ctx context.Context, k8sClient client.Client, lbconfig *elbv2gw.LoadBalancerConfiguration, gwControllers sets.Set[string]) (map[string]*gwv1.GatewayClass, error) { if lbconfig == nil { - return nil + return nil, nil + } + managedGwClasses, err := GetGatewayClassesManagedByLBController(ctx, k8sClient, gwControllers) + if err != nil { + return nil, err } - managedGwClasses := GetGatewayClassesManagedByLBController(ctx, k8sClient, gwControllers) impactedGatewayClasses := make(map[string]*gwv1.GatewayClass, len(managedGwClasses)) for _, gwClass := range managedGwClasses { if gwClass.Spec.ParametersRef != nil && string(gwClass.Spec.ParametersRef.Kind) == constants.LoadBalancerConfiguration && string(*gwClass.Spec.ParametersRef.Namespace) == lbconfig.Namespace && gwClass.Spec.ParametersRef.Name == lbconfig.Name { impactedGatewayClasses[gwClass.Name] = gwClass } } - return impactedGatewayClasses + return impactedGatewayClasses, nil } // GetImpactedGatewaysFromLbConfig identifies Gateways affected by LoadBalancer configuration changes. // Returns Gateways that reference the specified LoadBalancer configuration. -func GetImpactedGatewaysFromLbConfig(ctx context.Context, k8sClient client.Client, lbconfig *elbv2gw.LoadBalancerConfiguration, gwController string) []*gwv1.Gateway { +func GetImpactedGatewaysFromLbConfig(ctx context.Context, k8sClient client.Client, lbconfig *elbv2gw.LoadBalancerConfiguration, gwController string) ([]*gwv1.Gateway, error) { if lbconfig == nil { - return nil + return nil, nil + } + managedGateways, err := GetGatewaysManagedByLBController(ctx, k8sClient, gwController) + if err != nil { + return nil, err } - managedGateways := GetGatewaysManagedByLBController(ctx, k8sClient, gwController) impactedGateways := make([]*gwv1.Gateway, 0, len(managedGateways)) for _, gw := range managedGateways { + if gw.Namespace != lbconfig.Namespace { + continue + } + if gw.Spec.Infrastructure != nil && gw.Spec.Infrastructure.ParametersRef != nil && string(gw.Spec.Infrastructure.ParametersRef.Kind) == constants.LoadBalancerConfiguration && gw.Spec.Infrastructure.ParametersRef.Name == lbconfig.Name { impactedGateways = append(impactedGateways, gw) } } - return impactedGateways + return impactedGateways, nil } // GetGatewaysManagedByGatewayClass identifies Gateways managed by a GatewayClass. // Returns Gateways that refer the specified GatewayClass. -func GetGatewaysManagedByGatewayClass(ctx context.Context, k8sClient client.Client, gwClass *gwv1.GatewayClass, gwController string) []*gwv1.Gateway { - gwList := GetGatewaysManagedByLBController(ctx, k8sClient, gwController) +func GetGatewaysManagedByGatewayClass(ctx context.Context, k8sClient client.Client, gwClass *gwv1.GatewayClass) ([]*gwv1.Gateway, error) { + gwList, err := GetGatewaysManagedByLBController(ctx, k8sClient, string(gwClass.Spec.ControllerName)) + if err != nil { + return nil, err + } managedGw := make([]*gwv1.Gateway, 0, len(gwList)) for _, gw := range gwList { if string(gw.Spec.GatewayClassName) == gwClass.Name { managedGw = append(managedGw, gw) } } - return managedGw + return managedGw, nil } // removeDuplicateParentRefs make sure parentRefs in list is unique diff --git a/pkg/gateway/gatewayutils/gateway_utils_test.go b/pkg/gateway/gatewayutils/gateway_utils_test.go index b5bb7cfc40..1f1af74d24 100644 --- a/pkg/gateway/gatewayutils/gateway_utils_test.go +++ b/pkg/gateway/gatewayutils/gateway_utils_test.go @@ -210,8 +210,9 @@ func Test_GetGatewayClassesManagedByLBController(t *testing.T) { k8sClient.Create(context.Background(), gwClass) } - got := GetGatewayClassesManagedByLBController(context.Background(), k8sClient, tt.args.gwControllers) + got, err := GetGatewayClassesManagedByLBController(context.Background(), k8sClient, tt.args.gwControllers) assert.Equal(t, tt.want, len(got)) + assert.NoError(t, err) }) } } @@ -655,8 +656,9 @@ func Test_GetImpactedGatewayClassesFromLbConfig(t *testing.T) { k8sClient.Create(context.Background(), gwClass) } - got := GetImpactedGatewayClassesFromLbConfig(context.Background(), k8sClient, tt.args.lbConfig, tt.args.gwControllers) + got, err := GetImpactedGatewayClassesFromLbConfig(context.Background(), k8sClient, tt.args.lbConfig, tt.args.gwControllers) assert.Equal(t, tt.want, len(got)) + assert.NoError(t, err) }) } } @@ -753,6 +755,21 @@ func Test_GetImpactedGatewaysFromLbConfig(t *testing.T) { }, }, gateways: []*gwv1.Gateway{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-managed-gw", + Namespace: "other namespace", + }, + Spec: gwv1.GatewaySpec{ + GatewayClassName: "test-managed-class", + Infrastructure: &gwv1.GatewayInfrastructure{ + ParametersRef: &gwv1.LocalParametersReference{ + Kind: "LoadBalancerConfiguration", + Name: "test-config", + }, + }, + }, + }, { ObjectMeta: metav1.ObjectMeta{ Name: "test-managed-gw", @@ -826,17 +843,17 @@ func Test_GetImpactedGatewaysFromLbConfig(t *testing.T) { for _, gw := range tt.args.gateways { k8sClient.Create(context.Background(), gw) } - got := GetImpactedGatewaysFromLbConfig(context.Background(), k8sClient, tt.args.lbConfig, tt.args.gwController) + got, err := GetImpactedGatewaysFromLbConfig(context.Background(), k8sClient, tt.args.lbConfig, tt.args.gwController) assert.Equal(t, tt.want, len(got)) + assert.NoError(t, err) }) } } func Test_GetGatewaysManagedByGatewayClass(t *testing.T) { type args struct { - gateways []*gwv1.Gateway - gwClasses []*gwv1.GatewayClass - gwController string + gateways []*gwv1.Gateway + gwClasses []*gwv1.GatewayClass } tests := []struct { name string @@ -893,7 +910,6 @@ func Test_GetGatewaysManagedByGatewayClass(t *testing.T) { }, }, }, - gwController: constants.NLBGatewayController, }, want: 2, }, @@ -947,7 +963,6 @@ func Test_GetGatewaysManagedByGatewayClass(t *testing.T) { }, }, }, - gwController: constants.ALBGatewayController, }, want: 1, }, @@ -983,7 +998,6 @@ func Test_GetGatewaysManagedByGatewayClass(t *testing.T) { }, }, }, - gwController: constants.NLBGatewayController, }, want: 0, }, @@ -1028,7 +1042,6 @@ func Test_GetGatewaysManagedByGatewayClass(t *testing.T) { }, }, }, - gwController: constants.ALBGatewayController, }, want: 0, }, @@ -1044,8 +1057,9 @@ func Test_GetGatewaysManagedByGatewayClass(t *testing.T) { k8sClient.Create(context.Background(), gw) } - got := GetGatewaysManagedByGatewayClass(context.Background(), k8sClient, tt.args.gwClasses[0], tt.args.gwController) + got, err := GetGatewaysManagedByGatewayClass(context.Background(), k8sClient, tt.args.gwClasses[0]) assert.Equal(t, tt.want, len(got)) + assert.NoError(t, err) }) } } diff --git a/pkg/gateway/gatewayutils/lb_config_utils.go b/pkg/gateway/gatewayutils/lb_config_utils.go index 037fe6f792..152fd08c25 100644 --- a/pkg/gateway/gatewayutils/lb_config_utils.go +++ b/pkg/gateway/gatewayutils/lb_config_utils.go @@ -6,49 +6,10 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" elbv2gw "sigs.k8s.io/aws-load-balancer-controller/apis/gateway/v1beta1" - "sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/constants" - "sigs.k8s.io/aws-load-balancer-controller/pkg/k8s" - "sigs.k8s.io/aws-load-balancer-controller/pkg/shared_constants" "sigs.k8s.io/controller-runtime/pkg/client" gwv1 "sigs.k8s.io/gateway-api/apis/v1" ) -func RemoveLoadBalancerConfigurationFinalizers(ctx context.Context, gw *gwv1.Gateway, gwClass *gwv1.GatewayClass, k8sClient client.Client, manager k8s.FinalizerManager) error { - // remove finalizer from lbConfig - gatewayClass - if gwClass != nil { - gatewayClassLBConfig, err := ResolveLoadBalancerConfig(ctx, k8sClient, gwClass.Spec.ParametersRef) - if err != nil { - return client.IgnoreNotFound(err) - } - // remove finalizer if it exists and it not in use - if gatewayClassLBConfig != nil && - k8s.HasFinalizer(gatewayClassLBConfig, shared_constants.LoadBalancerConfigurationFinalizer) && - !IsLBConfigInUse(ctx, gatewayClassLBConfig, gw, gwClass, k8sClient) { - if err := manager.RemoveFinalizers(ctx, gatewayClassLBConfig, shared_constants.LoadBalancerConfigurationFinalizer); err != nil { - return err - } - } - } - // remove finalizer from lbConfig - gateway - if gw != nil { - var gwParametersRef = GetNamespacedParamRefForGateway(gw) - gatewayLBConfig, err := ResolveLoadBalancerConfig(ctx, k8sClient, gwParametersRef) - if err != nil { - return client.IgnoreNotFound(err) - } - // remove finalizer if it exists and it is not in use - if gatewayLBConfig != nil && - k8s.HasFinalizer(gatewayLBConfig, shared_constants.LoadBalancerConfigurationFinalizer) && - !IsLBConfigInUse(ctx, gatewayLBConfig, gw, gwClass, k8sClient) { - if err := manager.RemoveFinalizers(ctx, gatewayLBConfig, shared_constants.LoadBalancerConfigurationFinalizer); err != nil { - return err - } - } - - } - return nil -} - // ResolveLoadBalancerConfig returns the lb config referenced in the ParametersReference. func ResolveLoadBalancerConfig(ctx context.Context, k8sClient client.Client, reference *gwv1.ParametersReference) (*elbv2gw.LoadBalancerConfiguration, error) { var lbConf *elbv2gw.LoadBalancerConfiguration @@ -69,71 +30,37 @@ func ResolveLoadBalancerConfig(ctx context.Context, k8sClient client.Client, ref return lbConf, err } -func IsLBConfigInUse(ctx context.Context, lbConfig *elbv2gw.LoadBalancerConfiguration, gw *gwv1.Gateway, gwClass *gwv1.GatewayClass, k8sClient client.Client) bool { - //we want to make sure that we check the lb config is being used either by L4 or L7 gateways - controllerNames := sets.New(constants.ALBGatewayController, constants.NLBGatewayController) - return IsLBConfigInUseByGatewayClass(ctx, lbConfig, gw, gwClass, k8sClient, controllerNames) || - IsLBConfigInUseByGateway(ctx, lbConfig, gw, k8sClient, controllerNames) -} - -// checks if the lbconfig is indirectly being used by any gateways of the gwclass -func IsLBConfigInUseByGatewayClass(ctx context.Context, lbConfig *elbv2gw.LoadBalancerConfiguration, currGw *gwv1.Gateway, gwClass *gwv1.GatewayClass, k8sClient client.Client, controllerNames sets.Set[string]) bool { - // fetch all the gateway classes referenced by lb config - gwClassesUsingLBConfig := GetImpactedGatewayClassesFromLbConfig(ctx, k8sClient, lbConfig, controllerNames) +func IsLBConfigInUse(ctx context.Context, lbConfig *elbv2gw.LoadBalancerConfiguration, k8sClient client.Client, controllerNames sets.Set[string]) (bool, error) { + inUse, err := IsLBConfigInUseByGatewayClass(ctx, lbConfig, k8sClient, controllerNames) - // if a specific GatewayClass is supplied as a function parameter, it must be ensured - // that this particular GatewayClass is included within the collection of classes - // slated for evaluation, thereby guaranteeing its assessment for active Gateway management. - if gwClass != nil { - found := false - for _, gc := range gwClassesUsingLBConfig { - if gc.Name == gwClass.Name { - found = true - break - } - } - if !found { - gwClassesUsingLBConfig[gwClass.Name] = gwClass - } + if err != nil { + return false, err } - // iterate through each GatewayClass identified as referencing the LoadBalancerConfiguration - // the lbconfig is deemed to be in active use if any of these GatewayClasses - // are found to be managing one or more active Gateway resources. - gwsUsingLBConfig := make([]*gwv1.Gateway, 0) - for _, controllerName := range controllerNames.UnsortedList() { - for _, gwClassUsingLBConfig := range gwClassesUsingLBConfig { - gwList := GetGatewaysManagedByGatewayClass(ctx, k8sClient, gwClassUsingLBConfig, controllerName) - gwsUsingLBConfig = append(gwsUsingLBConfig, gwList...) - } + if inUse { + return true, nil } - if currGw == nil { - return len(gwsUsingLBConfig) > 0 - } - //skip the current gw from the list if it is not nil - for _, gw := range gwsUsingLBConfig { - if k8s.NamespacedName(currGw) != k8s.NamespacedName(gw) { - return true - } + + return IsLBConfigInUseByGateway(ctx, lbConfig, k8sClient, controllerNames) +} +func IsLBConfigInUseByGatewayClass(ctx context.Context, lbConfig *elbv2gw.LoadBalancerConfiguration, k8sClient client.Client, controllerNames sets.Set[string]) (bool, error) { + // fetch all the gateway classes referenced by lb config + gwClassesUsingLBConfig, err := GetImpactedGatewayClassesFromLbConfig(ctx, k8sClient, lbConfig, controllerNames) + if err != nil { + return false, err } - return false + return len(gwClassesUsingLBConfig) > 0, nil } -// checks if lbconfig is directly being used by any gateways -func IsLBConfigInUseByGateway(ctx context.Context, lbConfig *elbv2gw.LoadBalancerConfiguration, gw *gwv1.Gateway, k8sClient client.Client, controllerNames sets.Set[string]) bool { - var gwsUsingLBConfig []*gwv1.Gateway +func IsLBConfigInUseByGateway(ctx context.Context, lbConfig *elbv2gw.LoadBalancerConfiguration, k8sClient client.Client, controllerNames sets.Set[string]) (bool, error) { for _, controllerName := range controllerNames.UnsortedList() { - gws := GetImpactedGatewaysFromLbConfig(ctx, k8sClient, lbConfig, controllerName) - gwsUsingLBConfig = append(gwsUsingLBConfig, gws...) - } - if gw == nil { - return len(gwsUsingLBConfig) > 0 - } - // check if lbConfig is referred by any other gateway - for _, gwUsingLBConfig := range gwsUsingLBConfig { - if k8s.NamespacedName(gwUsingLBConfig) != k8s.NamespacedName(gw) { - return true + gws, err := GetImpactedGatewaysFromLbConfig(ctx, k8sClient, lbConfig, controllerName) + if err != nil { + return false, err + } + if len(gws) > 0 { + return true, nil } } - return false + return false, nil } diff --git a/pkg/gateway/gatewayutils/lb_config_utils_test.go b/pkg/gateway/gatewayutils/lb_config_utils_test.go index cf3fa6e7a0..dd2be12f37 100644 --- a/pkg/gateway/gatewayutils/lb_config_utils_test.go +++ b/pkg/gateway/gatewayutils/lb_config_utils_test.go @@ -2,19 +2,11 @@ package gatewayutils import ( "context" - "fmt" awssdk "github.com/aws/aws-sdk-go-v2/aws" - "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" elbv2gw "sigs.k8s.io/aws-load-balancer-controller/apis/gateway/v1beta1" - mock_client "sigs.k8s.io/aws-load-balancer-controller/mocks/controller-runtime/client" - "sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/constants" - "sigs.k8s.io/aws-load-balancer-controller/pkg/k8s" - "sigs.k8s.io/aws-load-balancer-controller/pkg/shared_constants" "sigs.k8s.io/aws-load-balancer-controller/pkg/testutils" - "sigs.k8s.io/controller-runtime/pkg/client" gwv1 "sigs.k8s.io/gateway-api/apis/v1" "testing" "time" @@ -76,198 +68,3 @@ func Test_ResolveLoadBalancerConfig(t *testing.T) { }) } } - -func Test_RemoveLoadBalancerConfigurationFinalizers(t *testing.T) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - k8sClient := mock_client.NewMockClient(ctrl) - k8sFinalizerManager := k8s.NewMockFinalizerManager(ctrl) - ctx := context.Background() - testGwName := "test-gw" - testNamespace := "test-ns" - testLbConfigName := "test-lb-config" - - tests := []struct { - name string - gateway *gwv1.Gateway - gatewayClass *gwv1.GatewayClass - setupMocks func() - wantErr bool - }{ - { - name: "remove finalizer from gateway LB config", - gateway: &gwv1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Name: testGwName, - Namespace: testNamespace, - }, - Spec: gwv1.GatewaySpec{ - Infrastructure: &gwv1.GatewayInfrastructure{ - ParametersRef: &gwv1.LocalParametersReference{ - Kind: gwv1.Kind(constants.LoadBalancerConfiguration), - Name: testLbConfigName, - }, - }, - }, - }, - gatewayClass: nil, - setupMocks: func() { - k8sClient.EXPECT(). - Get(ctx, types.NamespacedName{ - Namespace: testNamespace, - Name: testLbConfigName, - }, gomock.Any()). - DoAndReturn(func(_ context.Context, _ types.NamespacedName, obj *elbv2gw.LoadBalancerConfiguration, _ ...client.GetOption) error { - obj.Finalizers = []string{shared_constants.LoadBalancerConfigurationFinalizer} - return nil - }) - k8sClient.EXPECT(). - List(ctx, &gwv1.GatewayClassList{}, gomock.Any()). - Return(nil) - k8sClient.EXPECT(). - List(ctx, &gwv1.GatewayList{}, gomock.Any()). - Return(nil) - k8sClient.EXPECT(). - List(ctx, &gwv1.GatewayList{}, gomock.Any()). - Return(nil) - k8sFinalizerManager.EXPECT(). - RemoveFinalizers(ctx, gomock.Any(), shared_constants.LoadBalancerConfigurationFinalizer). - Return(nil) - }, - wantErr: false, - }, - { - name: "failed in remove finalizer", - gateway: &gwv1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Name: testGwName, - Namespace: testNamespace, - }, - Spec: gwv1.GatewaySpec{ - Infrastructure: &gwv1.GatewayInfrastructure{ - ParametersRef: &gwv1.LocalParametersReference{ - Kind: gwv1.Kind(constants.LoadBalancerConfiguration), - Name: testLbConfigName, - }, - }, - }, - }, - gatewayClass: nil, - setupMocks: func() { - k8sClient.EXPECT(). - Get(ctx, gomock.Any(), gomock.Any(), gomock.Any()). - DoAndReturn(func(_ context.Context, _ types.NamespacedName, obj *elbv2gw.LoadBalancerConfiguration, _ ...client.GetOption) error { - obj.Finalizers = []string{shared_constants.LoadBalancerConfigurationFinalizer} - return nil - }) - k8sClient.EXPECT(). - List(ctx, &gwv1.GatewayClassList{}, gomock.Any()). - Return(nil) - k8sClient.EXPECT(). - List(ctx, &gwv1.GatewayList{}, gomock.Any()). - Return(nil) - k8sClient.EXPECT(). - List(ctx, &gwv1.GatewayList{}, gomock.Any()). - Return(nil) - k8sFinalizerManager.EXPECT(). - RemoveFinalizers(ctx, gomock.Any(), shared_constants.LoadBalancerConfigurationFinalizer). - Return(fmt.Errorf("failed to remove finalizer")) - }, - wantErr: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - tt.setupMocks() - err := RemoveLoadBalancerConfigurationFinalizers(ctx, tt.gateway, tt.gatewayClass, k8sClient, k8sFinalizerManager) - if tt.wantErr { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } - }) - } - -} - -func Test_isLBConfigInUse(t *testing.T) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - k8sClient := mock_client.NewMockClient(ctrl) - ctx := context.Background() - testNamespace := "test-ns" - - tests := []struct { - name string - lbConfig *elbv2gw.LoadBalancerConfiguration - gateway *gwv1.Gateway - gatewayClass *gwv1.GatewayClass - setupMocks func() - want bool - }{ - { - name: "LB config not in use", - lbConfig: &elbv2gw.LoadBalancerConfiguration{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-config", - Namespace: testNamespace, - }, - }, - gateway: &gwv1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-gw", - Namespace: testNamespace, - }, - }, - gatewayClass: &gwv1.GatewayClass{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-gwclass", - }, - }, - setupMocks: func() { - k8sClient.EXPECT(). - Get(gomock.Any(), gomock.Any(), gomock.Any()). - Return(nil).AnyTimes() - - k8sClient.EXPECT(). - List(gomock.Any(), &gwv1.GatewayList{}, gomock.Any()). - DoAndReturn(func(_ context.Context, list *gwv1.GatewayList, _ ...client.ListOption) error { - list.Items = []gwv1.Gateway{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "test-gw", - Namespace: testNamespace, - }, - }, - } - return nil - }).AnyTimes() - - k8sClient.EXPECT(). - List(gomock.Any(), &gwv1.GatewayClassList{}, gomock.Any()). - DoAndReturn(func(_ context.Context, list *gwv1.GatewayClassList, _ ...client.ListOption) error { - list.Items = []gwv1.GatewayClass{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "test-gwclass", - }, - }, - } - return nil - }).AnyTimes() - }, - want: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - tt.setupMocks() - got := IsLBConfigInUse(ctx, tt.lbConfig, tt.gateway, tt.gatewayClass, k8sClient) - assert.Equal(t, tt.want, got) - }) - } -} diff --git a/pkg/gateway/referencecounter/service_reference_counter.go b/pkg/gateway/referencecounter/service_reference_counter.go new file mode 100644 index 0000000000..c1d870f6ff --- /dev/null +++ b/pkg/gateway/referencecounter/service_reference_counter.go @@ -0,0 +1,90 @@ +package referencecounter + +import ( + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" + "sync" +) + +// ServiceReferenceCounter tracks gateways and their relations to service objects. +type ServiceReferenceCounter interface { + UpdateRelations(svcs []types.NamespacedName, gateway types.NamespacedName, isDelete bool) + IsEligibleForRemoval(svcName types.NamespacedName, expectedGateways []types.NamespacedName) bool +} + +type serviceReferenceCounter struct { + mutex sync.RWMutex + // key: gateway, value: set of svc + relations map[types.NamespacedName]sets.Set[types.NamespacedName] + refCount map[types.NamespacedName]int +} + +// UpdateRelations updates the Gateway -> Service. mapping. +func (t *serviceReferenceCounter) UpdateRelations(svcs []types.NamespacedName, gateway types.NamespacedName, isDelete bool) { + t.mutex.Lock() + defer t.mutex.Unlock() + + existingValues, exists := t.relations[gateway] + + // Remove the old values from the ref count, we may add them back later. + if exists { + t.updateRefCount(existingValues, true) + } + + // If this a delete, we just need to remove the deleted gateway from the relations map. + if isDelete { + if exists { + delete(t.relations, gateway) + } + return + } + + // On additions, we simply create a new set, save the gateway -> relations set, and update the ref count map. + svcsSet := sets.New(svcs...) + + t.relations[gateway] = svcsSet + t.updateRefCount(svcsSet, false) +} + +// updateRefCount updates the ref count field, so consumers don't have to calculate ref counts on each IsEligibleForRemoval +func (t *serviceReferenceCounter) updateRefCount(svcs sets.Set[types.NamespacedName], isRemove bool) { + modifier := 1 + if isRemove { + modifier = -1 + } + for _, svc := range svcs.UnsortedList() { + t.refCount[svc] += modifier + + if t.refCount[svc] == 0 { + delete(t.refCount, svc) + } + } +} + +// IsEligibleForRemoval determines if it is safe to remove to no longer track resources that care about a particular service. +func (t *serviceReferenceCounter) IsEligibleForRemoval(svcName types.NamespacedName, expectedGateways []types.NamespacedName) bool { + t.mutex.RLock() + defer t.mutex.RUnlock() + _, ok := t.refCount[svcName] + // If we have a ref count for this service, we can't remove it. + // updateRefCount should always remove 0 entry services. + if ok { + return false + } + + // Next we check if the Gateway cache is correctly populated. This prevents premature removal of items + // when the cache is not warm. + for _, gw := range expectedGateways { + if _, exists := t.relations[gw]; !exists { + return false + } + } + return true +} + +func NewServiceReferenceCounter() ServiceReferenceCounter { + return &serviceReferenceCounter{ + relations: make(map[types.NamespacedName]sets.Set[types.NamespacedName]), + refCount: make(map[types.NamespacedName]int), + } +} diff --git a/pkg/gateway/referencecounter/service_reference_counter_test.go b/pkg/gateway/referencecounter/service_reference_counter_test.go new file mode 100644 index 0000000000..298acd3b55 --- /dev/null +++ b/pkg/gateway/referencecounter/service_reference_counter_test.go @@ -0,0 +1,424 @@ +package referencecounter + +import ( + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/types" + "testing" +) + +func TestServiceReferenceCounter(t *testing.T) { + + type insertion struct { + svcs []types.NamespacedName + gateway types.NamespacedName + isDelete bool + } + + type deletion struct { + svcName types.NamespacedName + expectedGateways []types.NamespacedName + expected bool + } + + testCases := []struct { + name string + insertions []insertion + deletions []deletion + }{ + { + name: "no insertions", + deletions: []deletion{ + { + svcName: types.NamespacedName{ + Namespace: "test-ns", + Name: "test-svc", + }, + expectedGateways: []types.NamespacedName{ + { + Namespace: "test-ns", + Name: "test-gw", + }, + }, + expected: false, + }, + { + svcName: types.NamespacedName{ + Namespace: "test-ns1", + Name: "test-svc1", + }, + expectedGateways: []types.NamespacedName{}, + expected: true, + }, + { + svcName: types.NamespacedName{ + Namespace: "test-ns2", + Name: "test-svc2", + }, + expectedGateways: []types.NamespacedName{}, + expected: true, + }, + }, + }, + { + name: "some valid deletions", + insertions: []insertion{ + { + svcs: []types.NamespacedName{ + { + Namespace: "test-ns", + Name: "test-svc1", + }, + { + Namespace: "test-ns", + Name: "test-svc2", + }, + { + Namespace: "test-ns", + Name: "test-svc3", + }, + }, + gateway: types.NamespacedName{ + Name: "gw1", + Namespace: "ns2", + }, + }, + { + svcs: []types.NamespacedName{ + { + Namespace: "test-ns", + Name: "test-svc1", + }, + { + Namespace: "test-ns", + Name: "test-svc2", + }, + { + Namespace: "test-ns", + Name: "test-svc3", + }, + }, + gateway: types.NamespacedName{ + Name: "gw2", + Namespace: "ns2", + }, + }, + { + svcs: []types.NamespacedName{ + { + Namespace: "test-ns", + Name: "test-svc1", + }, + { + Namespace: "test-ns", + Name: "test-svc2", + }, + { + Namespace: "test-ns", + Name: "test-svc3", + }, + }, + gateway: types.NamespacedName{ + Name: "gw3", + Namespace: "ns2", + }, + }, + }, + deletions: []deletion{ + // Wrong number of expected. + { + svcName: types.NamespacedName{ + Namespace: "test-ns", + Name: "test-svc", + }, + expectedGateways: []types.NamespacedName{ + { + Namespace: "test-ns", + Name: "test-gw", + }, + }, + expected: false, + }, + // Still has references + { + svcName: types.NamespacedName{ + Namespace: "test-ns", + Name: "test-svc1", + }, + expectedGateways: []types.NamespacedName{ + { + Name: "gw3", + Namespace: "ns2", + }, + { + Name: "gw2", + Namespace: "ns2", + }, + { + Name: "gw1", + Namespace: "ns2", + }, + }, + expected: false, + }, + // No references, valid expected gateways + { + svcName: types.NamespacedName{ + Namespace: "def doesnt exist", + Name: "test-svc1", + }, + expectedGateways: []types.NamespacedName{ + { + Name: "gw3", + Namespace: "ns2", + }, + { + Name: "gw2", + Namespace: "ns2", + }, + { + Name: "gw1", + Namespace: "ns2", + }, + }, + expected: true, + }, + }, + }, + { + name: "try to delete with references around", + insertions: []insertion{ + { + svcs: []types.NamespacedName{ + { + Namespace: "test-ns", + Name: "test-svc1", + }, + }, + gateway: types.NamespacedName{ + Name: "gw1", + Namespace: "ns2", + }, + }, + { + svcs: []types.NamespacedName{ + { + Namespace: "test-ns", + Name: "test-svc1", + }, + }, + gateway: types.NamespacedName{ + Name: "gw2", + Namespace: "ns2", + }, + }, + { + svcs: []types.NamespacedName{ + { + Namespace: "test-ns", + Name: "test-svc1", + }, + }, + gateway: types.NamespacedName{ + Name: "gw3", + Namespace: "ns2", + }, + }, + { + svcs: []types.NamespacedName{ + { + Namespace: "test-ns", + Name: "test-svc1", + }, + }, + gateway: types.NamespacedName{ + Name: "gw4", + Namespace: "ns2", + }, + }, + { + svcs: []types.NamespacedName{ + { + Namespace: "test-ns", + Name: "test-svc1", + }, + }, + gateway: types.NamespacedName{ + Name: "gw2", + Namespace: "ns2", + }, + isDelete: true, + }, + }, + deletions: []deletion{ + // Wrong number of expected. + { + svcName: types.NamespacedName{ + Namespace: "test-ns", + Name: "test-svc1", + }, + expectedGateways: []types.NamespacedName{ + { + Name: "gw1", + Namespace: "ns2", + }, + { + Name: "gw3", + Namespace: "ns2", + }, + { + Name: "gw4", + Namespace: "ns2", + }, + }, + expected: false, + }, + }, + }, + { + name: "all references cleared", + insertions: []insertion{ + { + svcs: []types.NamespacedName{ + { + Namespace: "test-ns", + Name: "test-svc1", + }, + }, + gateway: types.NamespacedName{ + Name: "gw1", + Namespace: "ns2", + }, + }, + { + svcs: []types.NamespacedName{ + { + Namespace: "test-ns", + Name: "test-svc1", + }, + }, + gateway: types.NamespacedName{ + Name: "gw2", + Namespace: "ns2", + }, + }, + { + svcs: []types.NamespacedName{ + { + Namespace: "test-ns", + Name: "test-svc1", + }, + }, + gateway: types.NamespacedName{ + Name: "gw3", + Namespace: "ns2", + }, + }, + { + svcs: []types.NamespacedName{ + { + Namespace: "test-ns", + Name: "test-svc1", + }, + }, + gateway: types.NamespacedName{ + Name: "gw4", + Namespace: "ns2", + }, + }, + { + svcs: []types.NamespacedName{ + { + Namespace: "test-ns", + Name: "test-svc1", + }, + }, + gateway: types.NamespacedName{ + Name: "gw2", + Namespace: "ns2", + }, + isDelete: true, + }, + { + svcs: []types.NamespacedName{ + { + Namespace: "test-ns", + Name: "test-svc1", + }, + }, + gateway: types.NamespacedName{ + Name: "gw1", + Namespace: "ns2", + }, + isDelete: true, + }, + { + svcs: []types.NamespacedName{ + { + Namespace: "test-ns", + Name: "test-svc1", + }, + }, + gateway: types.NamespacedName{ + Name: "gw3", + Namespace: "ns2", + }, + isDelete: true, + }, + { + svcs: []types.NamespacedName{ + { + Namespace: "test-ns", + Name: "test-svc1", + }, + }, + gateway: types.NamespacedName{ + Name: "gw4", + Namespace: "ns2", + }, + isDelete: true, + }, + }, + deletions: []deletion{ + // Wrong number of expected. + { + svcName: types.NamespacedName{ + Namespace: "test-ns", + Name: "test-svc1", + }, + expectedGateways: []types.NamespacedName{}, + expected: true, + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + refCounter := NewServiceReferenceCounter() + + for _, ii := range tc.insertions { + refCounter.UpdateRelations(ii.svcs, ii.gateway, ii.isDelete) + } + + for _, del := range tc.deletions { + res := refCounter.IsEligibleForRemoval(del.svcName, del.expectedGateways) + assert.Equal(t, del.expected, res, "tc %v", del) + } + }) + } + + t.Run("create -> check -> fail -> delete -> check -> work", func(t *testing.T) { + refCounter := NewServiceReferenceCounter() + refCounter.UpdateRelations([]types.NamespacedName{ + {Name: "svc1", Namespace: "ns"}, + }, types.NamespacedName{Name: "gw1", Namespace: "ns"}, false) + assert.False(t, refCounter.IsEligibleForRemoval(types.NamespacedName{Name: "svc1", Namespace: "ns"}, []types.NamespacedName{ + {Name: "gw1", Namespace: "ns"}, + })) + refCounter.UpdateRelations([]types.NamespacedName{ + {Name: "svc1", Namespace: "ns"}, + }, types.NamespacedName{Name: "gw1", Namespace: "ns"}, true) + assert.True(t, refCounter.IsEligibleForRemoval(types.NamespacedName{Name: "svc1", Namespace: "ns"}, []types.NamespacedName{})) + }) + +} diff --git a/pkg/gateway/routeutils/backend.go b/pkg/gateway/routeutils/backend.go index 2bdfdc0bcd..16734b514f 100644 --- a/pkg/gateway/routeutils/backend.go +++ b/pkg/gateway/routeutils/backend.go @@ -113,12 +113,6 @@ func commonBackendLoader(ctx context.Context, k8sClient client.Client, typeSpeci // As of right now, this error can only be thrown because of a k8s api error hence no status update. return nil, errors.Wrap(err, fmt.Sprintf("Unable to fetch tg config object")) } - // add TGConfig finalizer - if tgConfig != nil { - if err := AddTargetGroupConfigurationFinalizer(ctx, k8sClient, tgConfig); err != nil { - return nil, errors.Wrap(err, fmt.Sprintf("Unable to add finalizer to tg config object")) - } - } if servicePort == nil { initialErrorMessage := fmt.Sprintf("Unable to find service port for port %d", *backendRef.Port) diff --git a/pkg/gateway/routeutils/tg_config_utils.go b/pkg/gateway/routeutils/tg_config_utils.go deleted file mode 100644 index d562706be6..0000000000 --- a/pkg/gateway/routeutils/tg_config_utils.go +++ /dev/null @@ -1,47 +0,0 @@ -package routeutils - -import ( - "context" - "fmt" - "github.com/go-logr/logr" - corev1 "k8s.io/api/core/v1" - "k8s.io/client-go/tools/record" - elbv2gw "sigs.k8s.io/aws-load-balancer-controller/apis/gateway/v1beta1" - "sigs.k8s.io/aws-load-balancer-controller/pkg/k8s" - "sigs.k8s.io/aws-load-balancer-controller/pkg/shared_constants" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -// Implements helper function to add finalizer for target group configuration -func AddTargetGroupConfigurationFinalizer(ctx context.Context, k8sClient client.Client, tgConfig *elbv2gw.TargetGroupConfiguration) error { - finalizer := shared_constants.TargetGroupConfigurationFinalizer - // check if finalizer already exist - if k8s.HasFinalizer(tgConfig, finalizer) { - return nil - } - finalizerManager := k8s.NewDefaultFinalizerManager(k8sClient, logr.Discard()) - - return finalizerManager.AddFinalizers(ctx, tgConfig, finalizer) -} - -// RemoveTargetGroupConfigurationFinalizer removes target group configuration finalizer when service is deleted -func RemoveTargetGroupConfigurationFinalizer(ctx context.Context, svc *corev1.Service, k8sClient client.Client, logger logr.Logger, recorder record.EventRecorder) { - tgConfig, err := LookUpTargetGroupConfiguration(ctx, k8sClient, k8s.NamespacedName(svc)) - if err != nil { - logger.Error(err, "failed to look up target group configuration", "service", svc.Name) - return - } - if tgConfig == nil { - logger.V(1).Info("TargetGroupConfigurationNotFound, ignoring remove finalizer.", "TargetGroupConfiguration", svc.Name) - return - } - - tgFinalizer := shared_constants.TargetGroupConfigurationFinalizer - if k8s.HasFinalizer(tgConfig, tgFinalizer) { - finalizerManager := k8s.NewDefaultFinalizerManager(k8sClient, logr.Discard()) - if err := finalizerManager.RemoveFinalizers(ctx, tgConfig, tgFinalizer); err != nil { - recorder.Event(tgConfig, corev1.EventTypeWarning, k8s.TargetGroupBindingEventReasonFailedRemoveFinalizer, fmt.Sprintf("Failed to remove target group configuration finalizer due to %v", err)) - } - logger.V(1).Info("Successfully removed target group configuration finalizer.", "TargetGroupConfiguration", tgConfig.Name) - } -} diff --git a/pkg/shared_constants/finalizers.go b/pkg/shared_constants/finalizers.go index 2fd2fc0f4d..3fe0022bcc 100644 --- a/pkg/shared_constants/finalizers.go +++ b/pkg/shared_constants/finalizers.go @@ -10,6 +10,9 @@ const ( // ServiceFinalizer the finalizer used on service resources ServiceFinalizer = "service.k8s.aws/resources" + // GatewayClassFinalizer the finalizer we attach to an in-use LBC GatewayClass + GatewayClassFinalizer = "gateway.k8s.aws/gatewayclass" + // NLBGatewayFinalizer the finalizer we attach to an NLB Gateway resource NLBGatewayFinalizer = "gateway.k8s.aws/nlb"