Skip to content

Commit 1dbb7bd

Browse files
authored
[feat gw-api]implement finalizer (#4217)
refactor code and add unit tests fix logic bug
1 parent 4178d3a commit 1dbb7bd

File tree

10 files changed

+497
-12
lines changed

10 files changed

+497
-12
lines changed

controllers/gateway/eventhandlers/gateway_events.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ func (h *enqueueRequestsForGatewayEvent) Delete(ctx context.Context, e event.Typ
5454

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

controllers/gateway/eventhandlers/service_events.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,9 @@ func (h *enqueueRequestsForServiceEvent) Update(ctx context.Context, e event.Typ
6565
func (h *enqueueRequestsForServiceEvent) Delete(ctx context.Context, e event.TypedDeleteEvent[*corev1.Service], queue workqueue.TypedRateLimitingInterface[reconcile.Request]) {
6666
svc := e.Object
6767
h.logger.V(1).Info("enqueue service delete event", "service", svc.Name)
68+
// remove target group configuration finalizer when service is deleted
69+
RemoveTargetGroupConfigurationFinalizer(ctx, svc, h.k8sClient, h.logger, h.eventRecorder)
70+
6871
h.enqueueImpactedRoutes(ctx, svc)
6972
}
7073

controllers/gateway/eventhandlers/utils.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,16 @@ package eventhandlers
33
import (
44
"context"
55
"fmt"
6+
"github.com/go-logr/logr"
7+
corev1 "k8s.io/api/core/v1"
68
"k8s.io/apimachinery/pkg/types"
79
"k8s.io/apimachinery/pkg/util/sets"
10+
"k8s.io/client-go/tools/record"
811
elbv2gw "sigs.k8s.io/aws-load-balancer-controller/apis/gateway/v1beta1"
912
"sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/constants"
13+
"sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/routeutils"
14+
"sigs.k8s.io/aws-load-balancer-controller/pkg/k8s"
15+
"sigs.k8s.io/aws-load-balancer-controller/pkg/shared_constants"
1016
"sigs.k8s.io/controller-runtime/pkg/client"
1117
gwv1 "sigs.k8s.io/gateway-api/apis/v1"
1218
)
@@ -154,3 +160,25 @@ func removeDuplicateParentRefs(parentRefs []gwv1.ParentReference, resourceNamesp
154160
}
155161
return result
156162
}
163+
164+
// RemoveTargetGroupConfigurationFinalizer removes target group configuration finalizer when service is deleted
165+
func RemoveTargetGroupConfigurationFinalizer(ctx context.Context, svc *corev1.Service, k8sClient client.Client, logger logr.Logger, recorder record.EventRecorder) {
166+
tgConfig, err := routeutils.LookUpTargetGroupConfiguration(ctx, k8sClient, k8s.NamespacedName(svc))
167+
if err != nil {
168+
logger.Error(err, "failed to look up target group configuration", "service", svc.Name)
169+
return
170+
}
171+
if tgConfig == nil {
172+
logger.V(1).Info("TargetGroupConfigurationNotFound, ignoring remove finalizer.", "TargetGroupConfiguration", svc.Name)
173+
return
174+
}
175+
176+
tgFinalizer := shared_constants.TargetGroupConfigurationFinalizer
177+
if k8s.HasFinalizer(tgConfig, tgFinalizer) {
178+
finalizerManager := k8s.NewDefaultFinalizerManager(k8sClient, logr.Discard())
179+
if err := finalizerManager.RemoveFinalizers(ctx, tgConfig, tgFinalizer); err != nil {
180+
recorder.Event(tgConfig, corev1.EventTypeWarning, k8s.TargetGroupBindingEventReasonFailedRemoveFinalizer, fmt.Sprintf("Failed to remove target group configuration finalizer due to %v", err))
181+
}
182+
logger.V(1).Info("Successfully removed target group configuration finalizer.", "TargetGroupConfiguration", tgConfig.Name)
183+
}
184+
}

controllers/gateway/gateway_controller.go

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -223,17 +223,17 @@ func (r *gatewayReconciler) reconcileHelper(ctx context.Context, req reconcile.R
223223
}
224224

225225
if lb == nil {
226-
err = r.reconcileDelete(ctx, gw, stack, allRoutes)
226+
err = r.reconcileDelete(ctx, gw, gwClass, stack, allRoutes)
227227
if err != nil {
228228
r.logger.Error(err, "Failed to process gateway delete")
229229
}
230230
return err
231231
}
232232

233-
return r.reconcileUpdate(ctx, gw, stack, lb, backendSGRequired)
233+
return r.reconcileUpdate(ctx, gw, gwClass, stack, lb, backendSGRequired)
234234
}
235235

236-
func (r *gatewayReconciler) reconcileDelete(ctx context.Context, gw *gwv1.Gateway, stack core.Stack, routes map[int32][]routeutils.RouteDescriptor) error {
236+
func (r *gatewayReconciler) reconcileDelete(ctx context.Context, gw *gwv1.Gateway, gwClass *gwv1.GatewayClass, stack core.Stack, routes map[int32][]routeutils.RouteDescriptor) error {
237237
for _, routeList := range routes {
238238
if len(routeList) != 0 {
239239
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
250250
if err := r.backendSGProvider.Release(ctx, networking.ResourceTypeGateway, []types.NamespacedName{k8s.NamespacedName(gw)}); err != nil {
251251
return err
252252
}
253+
// remove load balancer configuration finalizer
254+
if err := RemoveLoadBalancerConfigurationFinalizers(ctx, gw, gwClass, r.k8sClient, r.finalizerManager, r.controllerName); err != nil {
255+
r.eventRecorder.Event(gw, corev1.EventTypeWarning, k8s.LoadBalancerConfigurationEventReasonFailedRemoveFinalizer, fmt.Sprintf("Failed remove load balancer configuration finalizer due to %v", err))
256+
return err
257+
}
258+
// remove gateway finalizer
253259
if err := r.finalizerManager.RemoveFinalizers(ctx, gw, r.finalizer); err != nil {
254-
r.eventRecorder.Event(gw, corev1.EventTypeWarning, k8s.GatewayEventReasonFailedAddFinalizer, fmt.Sprintf("Failed remove finalizer due to %v", err))
260+
r.eventRecorder.Event(gw, corev1.EventTypeWarning, k8s.GatewayEventReasonFailedRemoveFinalizer, fmt.Sprintf("Failed remove gateway finalizer due to %v", err))
255261
return err
256262
}
257263
}
258264
return nil
259265
}
260266

261-
func (r *gatewayReconciler) reconcileUpdate(ctx context.Context, gw *gwv1.Gateway, stack core.Stack,
267+
func (r *gatewayReconciler) reconcileUpdate(ctx context.Context, gw *gwv1.Gateway, gwClass *gwv1.GatewayClass, stack core.Stack,
262268
lb *elbv2model.LoadBalancer, backendSGRequired bool) error {
263-
269+
// add gateway finalizer
264270
if err := r.finalizerManager.AddFinalizers(ctx, gw, r.finalizer); err != nil {
265-
r.eventRecorder.Event(gw, corev1.EventTypeWarning, k8s.GatewayEventReasonFailedAddFinalizer, fmt.Sprintf("Failed add finalizer due to %v", err))
271+
r.eventRecorder.Event(gw, corev1.EventTypeWarning, k8s.GatewayEventReasonFailedAddFinalizer, fmt.Sprintf("Failed add gateway finalizer due to %v", err))
266272
return err
267273
}
274+
275+
// add load balancer configuration finalizer
276+
if err := AddLoadBalancerConfigurationFinalizers(ctx, gw, gwClass, r.k8sClient, r.finalizerManager, r.controllerName); err != nil {
277+
r.eventRecorder.Event(gw, corev1.EventTypeWarning, k8s.LoadBalancerConfigurationEventReasonFailedAddFinalizer, fmt.Sprintf("Failed add load balancer configuration finalizer due to %v", err))
278+
return err
279+
}
280+
268281
err := r.deployModel(ctx, gw, stack)
269282
if err != nil {
270283
return err

controllers/gateway/utils.go

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,13 @@ import (
66
"github.com/pkg/errors"
77
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
88
"k8s.io/apimachinery/pkg/types"
9+
"k8s.io/apimachinery/pkg/util/sets"
910
elbv2gw "sigs.k8s.io/aws-load-balancer-controller/apis/gateway/v1beta1"
11+
"sigs.k8s.io/aws-load-balancer-controller/controllers/gateway/eventhandlers"
12+
"sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/constants"
1013
"sigs.k8s.io/aws-load-balancer-controller/pkg/gateway/routeutils"
14+
"sigs.k8s.io/aws-load-balancer-controller/pkg/k8s"
15+
"sigs.k8s.io/aws-load-balancer-controller/pkg/shared_constants"
1116
"sigs.k8s.io/controller-runtime/pkg/client"
1217
gwv1 "sigs.k8s.io/gateway-api/apis/v1"
1318
"sort"
@@ -180,3 +185,86 @@ func generateRouteList(listenerRoutes map[int32][]routeutils.RouteDescriptor) st
180185

181186
return strings.Join(allRoutes, ",")
182187
}
188+
189+
// AddLoadBalancerConfigurationFinalizers add finalizer to load balancer configuration when it is in use by gateway or gatewayClass
190+
func AddLoadBalancerConfigurationFinalizers(ctx context.Context, gw *gwv1.Gateway, gwClass *gwv1.GatewayClass, k8sClient client.Client, manager k8s.FinalizerManager, controllerName string) error {
191+
// add finalizer to lbConfig referred by gatewayClass
192+
if gwClass.Spec.ParametersRef != nil && string(gwClass.Spec.ParametersRef.Kind) == constants.LoadBalancerConfiguration {
193+
lbConfig := &elbv2gw.LoadBalancerConfiguration{}
194+
if err := k8sClient.Get(ctx, types.NamespacedName{
195+
Namespace: string(*gwClass.Spec.ParametersRef.Namespace),
196+
Name: gwClass.Spec.ParametersRef.Name,
197+
}, lbConfig); err != nil {
198+
return client.IgnoreNotFound(err)
199+
}
200+
if err := manager.AddFinalizers(ctx, lbConfig, shared_constants.LoadBalancerConfigurationFinalizer); err != nil {
201+
return err
202+
}
203+
}
204+
205+
// add finalizer to lbConfig referred by gateway
206+
if gw.Spec.Infrastructure != nil && gw.Spec.Infrastructure.ParametersRef != nil && string(gw.Spec.Infrastructure.ParametersRef.Kind) == constants.LoadBalancerConfiguration {
207+
lbConfig := &elbv2gw.LoadBalancerConfiguration{}
208+
if err := k8sClient.Get(ctx, types.NamespacedName{
209+
Namespace: gw.Namespace,
210+
Name: gw.Spec.Infrastructure.ParametersRef.Name,
211+
}, lbConfig); err != nil {
212+
return client.IgnoreNotFound(err)
213+
}
214+
if err := manager.AddFinalizers(ctx, lbConfig, shared_constants.LoadBalancerConfigurationFinalizer); err != nil {
215+
return err
216+
}
217+
}
218+
return nil
219+
}
220+
221+
func RemoveLoadBalancerConfigurationFinalizers(ctx context.Context, gw *gwv1.Gateway, gwClass *gwv1.GatewayClass, k8sClient client.Client, manager k8s.FinalizerManager, controllerName string) error {
222+
// remove finalizer from lbConfig - gatewayClass
223+
if gwClass.Spec.ParametersRef != nil && string(gwClass.Spec.ParametersRef.Kind) == constants.LoadBalancerConfiguration {
224+
lbConfig := &elbv2gw.LoadBalancerConfiguration{}
225+
if err := k8sClient.Get(ctx, types.NamespacedName{
226+
Namespace: string(*gwClass.Spec.ParametersRef.Namespace),
227+
Name: gwClass.Spec.ParametersRef.Name,
228+
}, lbConfig); err != nil {
229+
return client.IgnoreNotFound(err)
230+
}
231+
// remove finalizer if it exists and it not in use
232+
if k8s.HasFinalizer(lbConfig, shared_constants.LoadBalancerConfigurationFinalizer) && !isLBConfigInUse(ctx, lbConfig, gw, gwClass, k8sClient, controllerName) {
233+
if err := manager.RemoveFinalizers(ctx, lbConfig, shared_constants.LoadBalancerConfigurationFinalizer); err != nil {
234+
return err
235+
}
236+
}
237+
}
238+
239+
// remove finalizer from lbConfig - gateway
240+
if gw.Spec.Infrastructure != nil && gw.Spec.Infrastructure.ParametersRef != nil && string(gw.Spec.Infrastructure.ParametersRef.Kind) == constants.LoadBalancerConfiguration {
241+
lbConfig := &elbv2gw.LoadBalancerConfiguration{}
242+
if err := k8sClient.Get(ctx, types.NamespacedName{
243+
Namespace: gw.Namespace,
244+
Name: gw.Spec.Infrastructure.ParametersRef.Name,
245+
}, lbConfig); err != nil {
246+
return client.IgnoreNotFound(err)
247+
}
248+
// remove finalizer if it exists and it is not in use
249+
if k8s.HasFinalizer(lbConfig, shared_constants.LoadBalancerConfigurationFinalizer) && !isLBConfigInUse(ctx, lbConfig, gw, gwClass, k8sClient, controllerName) {
250+
if err := manager.RemoveFinalizers(ctx, lbConfig, shared_constants.LoadBalancerConfigurationFinalizer); err != nil {
251+
return err
252+
}
253+
}
254+
}
255+
return nil
256+
}
257+
258+
func isLBConfigInUse(ctx context.Context, lbConfig *elbv2gw.LoadBalancerConfiguration, gw *gwv1.Gateway, gwClass *gwv1.GatewayClass, k8sClient client.Client, controllerName string) bool {
259+
// check if lbConfig is referred by any other gateway
260+
gwsUsingLBConfig := eventhandlers.GetImpactedGatewaysFromLbConfig(ctx, k8sClient, lbConfig, controllerName)
261+
for _, gwUsingLBConfig := range gwsUsingLBConfig {
262+
if gwUsingLBConfig.Name != gw.Name || gwUsingLBConfig.Namespace != gw.Namespace {
263+
return true
264+
}
265+
}
266+
267+
// check if lbConfig is referred by any other gatewayClass
268+
gwClassesUsingLBConfig := eventhandlers.GetImpactedGatewayClassesFromLbConfig(ctx, k8sClient, lbConfig, sets.New(controllerName))
269+
return len(gwClassesUsingLBConfig) > 0
270+
}

0 commit comments

Comments
 (0)