From 4991e89000fdf9d88d188caaaa305fe8393eebc6 Mon Sep 17 00:00:00 2001 From: lou-lan Date: Thu, 7 Mar 2024 17:26:46 +0800 Subject: [PATCH] Refactor RemoveFinalizers to improve code reusability Signed-off-by: lou-lan (cherry picked from commit a6b5f3197fc8664689958a194968c760f91f7ffb) --- pkg/controller/tunnel/egress_tunnel.go | 10 +--- pkg/controller/tunnel/egress_tunnel_test.go | 31 ---------- pkg/egressgateway/egress_gateway.go | 23 ++------ pkg/utils/finalizers.go | 17 ++++++ pkg/utils/finalizers_test.go | 63 +++++++++++++++++++++ 5 files changed, 85 insertions(+), 59 deletions(-) create mode 100644 pkg/utils/finalizers.go create mode 100644 pkg/utils/finalizers_test.go diff --git a/pkg/controller/tunnel/egress_tunnel.go b/pkg/controller/tunnel/egress_tunnel.go index cc964ccbe..7bb575c91 100644 --- a/pkg/controller/tunnel/egress_tunnel.go +++ b/pkg/controller/tunnel/egress_tunnel.go @@ -133,7 +133,7 @@ func (r *egReconciler) reconcileEGN(ctx context.Context, req reconcile.Request, // For the existence of Node, when the user manually deletes EgressTunnel, // we first release the EgressTunnel and then regenerate it. err := r.releaseEgressTunnel(*egresstunnel, log, func() error { - cleanFinalizers(egresstunnel) + utils.RemoveFinalizers(&egresstunnel.ObjectMeta, egressTunnelFinalizers) err = r.client.Update(context.Background(), egresstunnel) if err != nil { return err @@ -156,14 +156,6 @@ func (r *egReconciler) reconcileEGN(ctx context.Context, req reconcile.Request, return reconcile.Result{Requeue: false}, nil } -func cleanFinalizers(node *egressv1.EgressTunnel) { - for i, item := range node.Finalizers { - if item == egressTunnelFinalizers { - node.Finalizers = append(node.Finalizers[:i], node.Finalizers[i+1:]...) - } - } -} - // reconcileNode reconcile node // not goal: // - add node diff --git a/pkg/controller/tunnel/egress_tunnel_test.go b/pkg/controller/tunnel/egress_tunnel_test.go index 4b83b28cf..7f0469614 100644 --- a/pkg/controller/tunnel/egress_tunnel_test.go +++ b/pkg/controller/tunnel/egress_tunnel_test.go @@ -194,37 +194,6 @@ func TestEgressTunnelCtrlForNode(t *testing.T) { } } -func TestCleanFinalizers(t *testing.T) { - tests := []struct { - name string - node *egressv1.EgressTunnel - wantFinalizers []string - }{ - { - name: "remove egressTunnelFinalizers", - node: &egressv1.EgressTunnel{ - ObjectMeta: v1.ObjectMeta{ - Finalizers: []string{ - "keep-this", - "keep-that", - "egressgateway.spidernet.io/egresstunnel", - }, - }, - }, - wantFinalizers: []string{"keep-this", "keep-that"}, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - cleanFinalizers(tc.node) - if !slicesEqual(tc.node.Finalizers, tc.wantFinalizers) { - t.Errorf("cleanFinalizers() got = %v, want %v", tc.node.Finalizers, tc.wantFinalizers) - } - }) - } -} - func slicesEqual(a, b []string) bool { if len(a) != len(b) { return false diff --git a/pkg/egressgateway/egress_gateway.go b/pkg/egressgateway/egress_gateway.go index 15b7af345..fd4c75531 100644 --- a/pkg/egressgateway/egress_gateway.go +++ b/pkg/egressgateway/egress_gateway.go @@ -30,7 +30,6 @@ import ( egress "github.com/spidernet-io/egressgateway/pkg/k8s/apis/v1beta1" "github.com/spidernet-io/egressgateway/pkg/utils" "github.com/spidernet-io/egressgateway/pkg/utils/ip" - "github.com/spidernet-io/egressgateway/pkg/utils/slice" ) type egnReconciler struct { @@ -438,9 +437,8 @@ func (r *egnReconciler) reconcileGateway(ctx context.Context, req reconcile.Requ return reconcile.Result{Requeue: true}, err } if count == 0 && egw.Name != "" { - log.Info("remove the egressGatewayFinalizer") - removeEgressGatewayFinalizer(egw) log.V(1).Info("remove the egressGatewayFinalizer", "ObjectMeta", egw.ObjectMeta) + utils.RemoveFinalizers(&egw.ObjectMeta, egressGatewayFinalizers) err = r.client.Update(ctx, egw) if err != nil { log.Error(err, "remove the egressGatewayFinalizer", "ObjectMeta", egw.ObjectMeta) @@ -1278,13 +1276,6 @@ func countGatewayIP(egw *egress.EgressGateway) (ipv4sFree, ipv6sFree, ipv4sTotal return } -// removeEgressGatewayFinalizer if the egress gateway is being deleted -func removeEgressGatewayFinalizer(egw *egress.EgressGateway) { - if containsEgressGatewayFinalizer(egw, egressGatewayFinalizers) { - egw.Finalizers = slice.RemoveElement(egw.Finalizers, egressGatewayFinalizers) - } -} - func getPolicyCountByGatewayName(ctx context.Context, client client.Client, name string) (int, error) { var num int @@ -1313,15 +1304,6 @@ func getPolicyCountByGatewayName(ctx context.Context, client client.Client, name return num, nil } -func containsEgressGatewayFinalizer(gateway *egress.EgressGateway, finalizer string) bool { - for _, f := range gateway.ObjectMeta.Finalizers { - if f == finalizer { - return true - } - } - return false -} - type egressPolicyPredicate struct{} func (p egressPolicyPredicate) Create(_ event.CreateEvent) bool { return true } @@ -1375,6 +1357,9 @@ func (p egressGatewayPredicate) Update(updateEvent event.UpdateEvent) bool { if !ok { return false } + if !reflect.DeepEqual(oldObj.ObjectMeta, newObj.ObjectMeta) { + return true + } if !reflect.DeepEqual(oldObj.Spec, newObj.Spec) { return true } diff --git a/pkg/utils/finalizers.go b/pkg/utils/finalizers.go new file mode 100644 index 000000000..8357cdbc3 --- /dev/null +++ b/pkg/utils/finalizers.go @@ -0,0 +1,17 @@ +// Copyright 2022 Authors of spidernet-io +// SPDX-License-Identifier: Apache-2.0 + +package utils + +import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + +func RemoveFinalizers(objMeta *metav1.ObjectMeta, finalizers ...string) { + for _, f := range finalizers { + for i := len(objMeta.Finalizers) - 1; i >= 0; i-- { + if objMeta.Finalizers[i] == f { + objMeta.Finalizers = append(objMeta.Finalizers[:i], objMeta.Finalizers[i+1:]...) + break + } + } + } +} diff --git a/pkg/utils/finalizers_test.go b/pkg/utils/finalizers_test.go new file mode 100644 index 000000000..18b4bd10f --- /dev/null +++ b/pkg/utils/finalizers_test.go @@ -0,0 +1,63 @@ +// Copyright 2022 Authors of spidernet-io +// SPDX-License-Identifier: Apache-2.0 + +package utils + +import ( + "reflect" + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// TestRemoveFinalizers tests the RemoveFinalizers function. +func TestRemoveFinalizers(t *testing.T) { + tests := []struct { + name string + objMeta metav1.ObjectMeta + finalizers []string + want []string + }{ + { + name: "remove single finalizer", + objMeta: metav1.ObjectMeta{ + Finalizers: []string{"finalize1", "finalize2", "finalize3"}, + }, + finalizers: []string{"finalize2"}, + want: []string{"finalize1", "finalize3"}, + }, + { + name: "remove multiple finalizers", + objMeta: metav1.ObjectMeta{ + Finalizers: []string{"finalize1", "finalize2", "finalize3", "finalize4"}, + }, + finalizers: []string{"finalize2", "finalize4"}, + want: []string{"finalize1", "finalize3"}, + }, + { + name: "remove non-existent finalizer", + objMeta: metav1.ObjectMeta{ + Finalizers: []string{"finalize1", "finalize2"}, + }, + finalizers: []string{"finalize3"}, + want: []string{"finalize1", "finalize2"}, + }, + { + name: "remove all finalizers", + objMeta: metav1.ObjectMeta{ + Finalizers: []string{"finalize1", "finalize2"}, + }, + finalizers: []string{"finalize1", "finalize2"}, + want: []string{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + RemoveFinalizers(&tt.objMeta, tt.finalizers...) + if !reflect.DeepEqual(tt.objMeta.Finalizers, tt.want) { + t.Errorf("RemoveFinalizers() got = %v, want %v", tt.objMeta.Finalizers, tt.want) + } + }) + } +}