[feat gw-api]implement finalizer

refactor code and add unit tests

Author: shuqz

controllers/gateway/eventhandlers/gateway_events.go

@@ -54,7 +54,7 @@ func (h *enqueueRequestsForGatewayEvent) Delete(ctx context.Context, e event.Typ
 
 func (h *enqueueRequestsForGatewayEvent) Generic(ctx context.Context, e event.TypedGenericEvent[*gwv1.Gateway], queue workqueue.TypedRateLimitingInterface[reconcile.Request]) {
 	gw := e.Object
-	h.logger.V(1).Info("enqueue gateway delete event", "gateway", k8s.NamespacedName(gw))
+	h.logger.V(1).Info("enqueue gateway generic event", "gateway", k8s.NamespacedName(gw))
 	h.enqueueImpactedGateway(ctx, gw, queue)
 }
 

controllers/gateway/eventhandlers/service_events.go

@@ -65,6 +65,9 @@ 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
+	RemoveTargetGroupConfigurationFinalizer(ctx, svc, h.k8sClient, h.logger, h.eventRecorder)
+
 	h.enqueueImpactedRoutes(ctx, svc)
 }
 

controllers/gateway/eventhandlers/utils.go

@@ -3,10 +3,16 @@ package eventhandlers
 import (
 	"context"
 	"fmt"
+	"github.com/go-logr/logr"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
+	"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/gateway/constants"
+	"sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/routeutils"
+	"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"
 )
@@ -154,3 +160,25 @@ func removeDuplicateParentRefs(parentRefs []gwv1.ParentReference, resourceNamesp
 	}
 	return result
 }
+
+// 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 := routeutils.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)
+	}
+}

controllers/gateway/gateway_controller.go

@@ -223,17 +223,17 @@ func (r *gatewayReconciler) reconcileHelper(ctx context.Context, req reconcile.R
 	}
 
 	if lb == nil {
-		err = r.reconcileDelete(ctx, gw, stack, allRoutes)
+		err = r.reconcileDelete(ctx, gw, gwClass, stack, allRoutes)
 		if err != nil {
 			r.logger.Error(err, "Failed to process gateway delete")
 		}
 		return err
 	}
 
-	return r.reconcileUpdate(ctx, gw, stack, lb, backendSGRequired)
+	return r.reconcileUpdate(ctx, gw, gwClass, stack, lb, backendSGRequired)
 }
 
-func (r *gatewayReconciler) reconcileDelete(ctx context.Context, gw *gwv1.Gateway, stack core.Stack, routes map[int32][]routeutils.RouteDescriptor) error {
+func (r *gatewayReconciler) reconcileDelete(ctx context.Context, gw *gwv1.Gateway, gwClass *gwv1.GatewayClass, 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))
@@ -250,21 +250,34 @@ 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 := RemoveLoadBalancerConfigurationFinalizers(ctx, gw, gwClass, r.k8sClient, r.finalizerManager, r.controllerName); 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.GatewayEventReasonFailedAddFinalizer, fmt.Sprintf("Failed remove finalizer due to %v", err))
+			r.eventRecorder.Event(gw, corev1.EventTypeWarning, k8s.GatewayEventReasonFailedRemoveFinalizer, fmt.Sprintf("Failed remove gateway finalizer due to %v", err))
 			return err
 		}
 	}
 	return nil
 }
 
-func (r *gatewayReconciler) reconcileUpdate(ctx context.Context, gw *gwv1.Gateway, stack core.Stack,
+func (r *gatewayReconciler) reconcileUpdate(ctx context.Context, gw *gwv1.Gateway, gwClass *gwv1.GatewayClass, stack core.Stack,
 	lb *elbv2model.LoadBalancer, backendSGRequired bool) error {
-
+	// add gateway finalizer
 	if err := r.finalizerManager.AddFinalizers(ctx, gw, r.finalizer); err != nil {
-		r.eventRecorder.Event(gw, corev1.EventTypeWarning, k8s.GatewayEventReasonFailedAddFinalizer, fmt.Sprintf("Failed add finalizer due to %v", err))
+		r.eventRecorder.Event(gw, corev1.EventTypeWarning, k8s.GatewayEventReasonFailedAddFinalizer, fmt.Sprintf("Failed add gateway finalizer due to %v", err))
 		return err
 	}
+
+	// add load balancer configuration finalizer
+	if err := AddLoadBalancerConfigurationFinalizers(ctx, gw, gwClass, r.k8sClient, r.finalizerManager); err != nil {
+		r.eventRecorder.Event(gw, corev1.EventTypeWarning, k8s.LoadBalancerConfigurationEventReasonFailedAddFinalizer, fmt.Sprintf("Failed add load balancer configuration finalizer due to %v", err))
+		return err
+	}
+
 	err := r.deployModel(ctx, gw, stack)
 	if err != nil {
 		return err

controllers/gateway/utils.go

@@ -6,8 +6,13 @@ import (
 	"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/controllers/gateway/eventhandlers"
+	"sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/constants"
 	"sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/routeutils"
+	"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"
 	"sort"
@@ -180,3 +185,94 @@ func generateRouteList(listenerRoutes map[int32][]routeutils.RouteDescriptor) st
 
 	return strings.Join(allRoutes, ",")
 }
+
+// AddLoadBalancerConfigurationFinalizers add finalizer to load balancer configuration when it is in use by gateway or gatewayClass
+func AddLoadBalancerConfigurationFinalizers(ctx context.Context, gw *gwv1.Gateway, gwClass *gwv1.GatewayClass, k8sClient client.Client, manager k8s.FinalizerManager) error {
+	// add finalizer to lbConfig referred by gatewayClass
+	if gwClass.Spec.ParametersRef != nil && string(gwClass.Spec.ParametersRef.Kind) == constants.LoadBalancerConfiguration {
+		lbConfig := &elbv2gw.LoadBalancerConfiguration{}
+		if err := k8sClient.Get(ctx, types.NamespacedName{
+			Namespace: string(*gwClass.Spec.ParametersRef.Namespace),
+			Name:      gwClass.Spec.ParametersRef.Name,
+		}, lbConfig); err != nil {
+			return client.IgnoreNotFound(err)
+		}
+		if err := manager.AddFinalizers(ctx, lbConfig, shared_constants.LoadBalancerConfigurationFinalizer); err != nil {
+			return err
+		}
+	}
+
+	// add finalizer to lbConfig referred by gateway
+	if gw.Spec.Infrastructure != nil && gw.Spec.Infrastructure.ParametersRef != nil && string(gw.Spec.Infrastructure.ParametersRef.Kind) == constants.LoadBalancerConfiguration {
+		lbConfig := &elbv2gw.LoadBalancerConfiguration{}
+		if err := k8sClient.Get(ctx, types.NamespacedName{
+			Namespace: gw.Namespace,
+			Name:      gw.Spec.Infrastructure.ParametersRef.Name,
+		}, lbConfig); err != nil {
+			return client.IgnoreNotFound(err)
+		}
+		if err := manager.AddFinalizers(ctx, lbConfig, shared_constants.LoadBalancerConfigurationFinalizer); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func RemoveLoadBalancerConfigurationFinalizers(ctx context.Context, gw *gwv1.Gateway, gwClass *gwv1.GatewayClass, k8sClient client.Client, manager k8s.FinalizerManager, controllerName string) error {
+	// remove finalizer from lbConfig - gatewayClass
+	if gwClass.Spec.ParametersRef != nil && string(gwClass.Spec.ParametersRef.Kind) == constants.LoadBalancerConfiguration {
+		lbConfig := &elbv2gw.LoadBalancerConfiguration{}
+		if err := k8sClient.Get(ctx, types.NamespacedName{
+			Namespace: string(*gwClass.Spec.ParametersRef.Namespace),
+			Name:      gwClass.Spec.ParametersRef.Name,
+		}, lbConfig); err != nil {
+			return client.IgnoreNotFound(err)
+		}
+		// remove finalizer if it exists and it not in use
+		if k8s.HasFinalizer(lbConfig, shared_constants.LoadBalancerConfigurationFinalizer) && !isLBConfigInUse(ctx, lbConfig, gw, gwClass, k8sClient, controllerName) {
+			if err := manager.RemoveFinalizers(ctx, lbConfig, shared_constants.LoadBalancerConfigurationFinalizer); err != nil {
+				return err
+			}
+		}
+	}
+
+	// remove finalizer from lbConfig - gateway
+	if gw.Spec.Infrastructure != nil && gw.Spec.Infrastructure.ParametersRef != nil && string(gw.Spec.Infrastructure.ParametersRef.Kind) == constants.LoadBalancerConfiguration {
+		lbConfig := &elbv2gw.LoadBalancerConfiguration{}
+		if err := k8sClient.Get(ctx, types.NamespacedName{
+			Namespace: gw.Namespace,
+			Name:      gw.Spec.Infrastructure.ParametersRef.Name,
+		}, lbConfig); err != nil {
+			return client.IgnoreNotFound(err)
+		}
+		// remove finalizer if it exists and it is not in use
+		if k8s.HasFinalizer(lbConfig, shared_constants.LoadBalancerConfigurationFinalizer) && !isLBConfigInUse(ctx, lbConfig, gw, gwClass, k8sClient, controllerName) {
+			if err := manager.RemoveFinalizers(ctx, lbConfig, shared_constants.LoadBalancerConfigurationFinalizer); err != nil {
+				//r.eventRecorder.Event(gw, corev1.EventTypeWarning, LoadBalancerConfigurationEventReasonFailedRemoveFinalizer, fmt.Sprintf("Failed to remove load balancer configuration finalizer due to %v", err))
+				return err
+			}
+		}
+	}
+	return nil
+}
+
+func isLBConfigInUse(ctx context.Context, lbConfig *elbv2gw.LoadBalancerConfiguration, gw *gwv1.Gateway, gwClass *gwv1.GatewayClass, k8sClient client.Client, controllerName string) bool {
+	// check if lbConfig is referred by any other gateway
+	gwsUsingLBConfig := eventhandlers.GetImpactedGatewaysFromLbConfig(ctx, k8sClient, lbConfig, controllerName)
+	for _, gwUsingLBConfig := range gwsUsingLBConfig {
+		if gwUsingLBConfig.Name != gw.Name || gwUsingLBConfig.Namespace != gw.Namespace {
+			return true
+		}
+	}
+
+	// check if lbConfig is referred by any other gatewayClass
+	gwClassesUsingLBConfig := eventhandlers.GetImpactedGatewayClassesFromLbConfig(ctx, k8sClient, lbConfig, sets.New(controllerName))
+	for _, gwClassUsingLBConfig := range gwClassesUsingLBConfig {
+		// at this point, no other gateway and gatewayClass is using LBConfig, only the given GWClass is using it, we remove finalizer here
+		// if we do not handle it here, no gateway will trigger reconcile deletion to remove finalizer from this gatewayClass
+		if gwClassUsingLBConfig.Name != gwClass.Name || gwClassUsingLBConfig.Namespace != gwClass.Namespace {
+			return true
+		}
+	}
+	return false
+}