From 266f676844000202af06d62b9574e5c8a90e83a2 Mon Sep 17 00:00:00 2001 From: bobz965 Date: Tue, 20 Feb 2024 14:23:29 +0800 Subject: [PATCH 1/6] fix: should use domain-qualified finalizer name Signed-off-by: bobz965 --- pkg/controller/init.go | 2 +- pkg/controller/ip.go | 4 ++-- pkg/controller/ovn_dnat.go | 4 ++-- pkg/controller/ovn_eip.go | 4 ++-- pkg/controller/ovn_fip.go | 4 ++-- pkg/controller/ovn_snat.go | 4 ++-- pkg/controller/qos_policy.go | 6 +++--- pkg/controller/subnet.go | 6 +++--- pkg/controller/vip.go | 6 +++--- pkg/controller/vpc_nat_gw_eip.go | 6 +++--- pkg/controller/vpc_nat_gw_nat.go | 18 +++++++++--------- pkg/util/const.go | 2 +- test/e2e/kube-ovn/subnet/subnet.go | 16 ++++++++-------- 13 files changed, 41 insertions(+), 41 deletions(-) diff --git a/pkg/controller/init.go b/pkg/controller/init.go index b89fa91e207..43ef8dc0649 100644 --- a/pkg/controller/init.go +++ b/pkg/controller/init.go @@ -578,7 +578,7 @@ func (c *Controller) syncIPCR() error { for _, ipCR := range ips { ip := ipCR.DeepCopy() - if ip.DeletionTimestamp != nil && slices.Contains(ip.Finalizers, util.ControllerName) { + if ip.DeletionTimestamp != nil && slices.Contains(ip.Finalizers, util.FinalizerName) { klog.Infof("enqueue update for deleting ip %s", ip.Name) c.updateIPQueue.Add(ip.Name) } diff --git a/pkg/controller/ip.go b/pkg/controller/ip.go index acea7b7fe6a..793a43dcd65 100644 --- a/pkg/controller/ip.go +++ b/pkg/controller/ip.go @@ -323,7 +323,7 @@ func (c *Controller) handleUpdateIP(key string) error { klog.Infof("ip cr %s release ipam pod key %s from subnet %s", cachedIP.Name, podKey, cachedIP.Spec.Subnet) c.ipam.ReleaseAddressByPod(podKey, cachedIP.Spec.Subnet) } - if err = c.handleDelIPFinalizer(cachedIP, util.ControllerName); err != nil { + if err = c.handleDelIPFinalizer(cachedIP, util.FinalizerName); err != nil { klog.Errorf("failed to handle del ip finalizer %v", err) return err } @@ -535,7 +535,7 @@ func (c *Controller) createOrUpdateIPCR(ipCRName, podName, ip, mac, subnetName, } } - if err := c.handleAddIPFinalizer(ipCR, util.ControllerName); err != nil { + if err := c.handleAddIPFinalizer(ipCR, util.FinalizerName); err != nil { klog.Errorf("failed to handle add ip finalizer %v", err) return err } diff --git a/pkg/controller/ovn_dnat.go b/pkg/controller/ovn_dnat.go index 77b6bbf5f89..4dc04b4060b 100644 --- a/pkg/controller/ovn_dnat.go +++ b/pkg/controller/ovn_dnat.go @@ -301,7 +301,7 @@ func (c *Controller) handleAddOvnDnatRule(key string) error { return err } - if err := c.handleAddOvnDnatFinalizer(cachedDnat, util.ControllerName); err != nil { + if err := c.handleAddOvnDnatFinalizer(cachedDnat, util.FinalizerName); err != nil { klog.Errorf("failed to add finalizer for ovn dnat %s, %v", cachedDnat.Name, err) return err } @@ -348,7 +348,7 @@ func (c *Controller) handleDelOvnDnatRule(key string) error { return err } } - if err = c.handleDelOvnDnatFinalizer(cachedDnat, util.ControllerName); err != nil { + if err = c.handleDelOvnDnatFinalizer(cachedDnat, util.FinalizerName); err != nil { klog.Errorf("failed to remove finalizer for ovn dnat %s, %v", cachedDnat.Name, err) return err } diff --git a/pkg/controller/ovn_eip.go b/pkg/controller/ovn_eip.go index a18fcc465d0..3e69c97ba48 100644 --- a/pkg/controller/ovn_eip.go +++ b/pkg/controller/ovn_eip.go @@ -268,7 +268,7 @@ func (c *Controller) handleAddOvnEip(key string) error { return err } } - if err = c.handleAddOvnEipFinalizer(cachedEip, util.ControllerName); err != nil { + if err = c.handleAddOvnEipFinalizer(cachedEip, util.FinalizerName); err != nil { klog.Errorf("failed to add finalizer for ovn eip, %v", err) return err } @@ -343,7 +343,7 @@ func (c *Controller) handleDelOvnEip(key string) error { } } - if err = c.handleDelOvnEipFinalizer(eip, util.ControllerName); err != nil { + if err = c.handleDelOvnEipFinalizer(eip, util.FinalizerName); err != nil { klog.Errorf("failed to handle remove ovn eip finalizer , %v", err) return err } diff --git a/pkg/controller/ovn_fip.go b/pkg/controller/ovn_fip.go index a78a6d37b1b..358c12255f3 100644 --- a/pkg/controller/ovn_fip.go +++ b/pkg/controller/ovn_fip.go @@ -296,7 +296,7 @@ func (c *Controller) handleAddOvnFip(key string) error { return err } - if err = c.handleAddOvnFipFinalizer(cachedFip, util.ControllerName); err != nil { + if err = c.handleAddOvnFipFinalizer(cachedFip, util.FinalizerName); err != nil { klog.Errorf("failed to add finalizer for ovn fip, %v", err) return err } @@ -463,7 +463,7 @@ func (c *Controller) handleDelOvnFip(key string) error { return err } } - if err = c.handleDelOvnFipFinalizer(cachedFip, util.ControllerName); err != nil { + if err = c.handleDelOvnFipFinalizer(cachedFip, util.FinalizerName); err != nil { klog.Errorf("failed to remove finalizer for ovn fip %s, %v", cachedFip.Name, err) return err } diff --git a/pkg/controller/ovn_snat.go b/pkg/controller/ovn_snat.go index a49b3af2a1a..3cdd6642419 100644 --- a/pkg/controller/ovn_snat.go +++ b/pkg/controller/ovn_snat.go @@ -257,7 +257,7 @@ func (c *Controller) handleAddOvnSnatRule(key string) error { klog.Errorf("failed to create snat, %v", err) return err } - if err := c.handleAddOvnSnatFinalizer(cachedSnat, util.ControllerName); err != nil { + if err := c.handleAddOvnSnatFinalizer(cachedSnat, util.FinalizerName); err != nil { klog.Errorf("failed to add finalizer for ovn snat %s, %v", cachedSnat.Name, err) return err } @@ -419,7 +419,7 @@ func (c *Controller) handleDelOvnSnatRule(key string) error { return err } } - if err = c.handleDelOvnSnatFinalizer(cachedSnat, util.ControllerName); err != nil { + if err = c.handleDelOvnSnatFinalizer(cachedSnat, util.FinalizerName); err != nil { klog.Errorf("failed to remove finalizer for ovn snat %s, %v", cachedSnat.Name, err) return err } diff --git a/pkg/controller/qos_policy.go b/pkg/controller/qos_policy.go index a0f667f55cd..5e27a056893 100644 --- a/pkg/controller/qos_policy.go +++ b/pkg/controller/qos_policy.go @@ -265,7 +265,7 @@ func (c *Controller) handleDelQoSPoliciesFinalizer(key string) error { return nil } newQoSPolicies := cachedQoSPolicies.DeepCopy() - controllerutil.RemoveFinalizer(newQoSPolicies, util.ControllerName) + controllerutil.RemoveFinalizer(newQoSPolicies, util.FinalizerName) patch, err := util.GenerateMergePatchPayload(cachedQoSPolicies, newQoSPolicies) if err != nil { klog.Errorf("failed to generate patch payload for qos '%s', %v", cachedQoSPolicies.Name, err) @@ -521,12 +521,12 @@ func (c *Controller) handleAddQoSPolicyFinalizer(key string) error { return err } if cachedQoSPolicy.DeletionTimestamp.IsZero() { - if slices.Contains(cachedQoSPolicy.Finalizers, util.ControllerName) { + if slices.Contains(cachedQoSPolicy.Finalizers, util.FinalizerName) { return nil } } newQoSPolicy := cachedQoSPolicy.DeepCopy() - controllerutil.AddFinalizer(newQoSPolicy, util.ControllerName) + controllerutil.AddFinalizer(newQoSPolicy, util.FinalizerName) patch, err := util.GenerateMergePatchPayload(cachedQoSPolicy, newQoSPolicy) if err != nil { klog.Errorf("failed to generate patch payload for qos '%s', %v", cachedQoSPolicy.Name, err) diff --git a/pkg/controller/subnet.go b/pkg/controller/subnet.go index f95994d18f2..7faba89d912 100644 --- a/pkg/controller/subnet.go +++ b/pkg/controller/subnet.go @@ -480,9 +480,9 @@ func checkAndUpdateExcludeIPs(subnet *kubeovnv1.Subnet) bool { } func (c *Controller) handleSubnetFinalizer(subnet *kubeovnv1.Subnet) (bool, error) { - if subnet.DeletionTimestamp.IsZero() && !slices.Contains(subnet.Finalizers, util.ControllerName) { + if subnet.DeletionTimestamp.IsZero() && !slices.Contains(subnet.Finalizers, util.FinalizerName) { newSubnet := subnet.DeepCopy() - newSubnet.Finalizers = append(newSubnet.Finalizers, util.ControllerName) + newSubnet.Finalizers = append(newSubnet.Finalizers, util.FinalizerName) patch, err := util.GenerateMergePatchPayload(subnet, newSubnet) if err != nil { klog.Errorf("failed to generate patch payload for subnet '%s', %v", subnet.Name, err) @@ -506,7 +506,7 @@ func (c *Controller) handleSubnetFinalizer(subnet *kubeovnv1.Subnet) (bool, erro u2oInterconnIP := subnet.Status.U2OInterconnectionIP if !subnet.DeletionTimestamp.IsZero() && (usingIPs == 0 || (usingIPs == 1 && u2oInterconnIP != "")) { newSubnet := subnet.DeepCopy() - newSubnet.Finalizers = util.RemoveString(newSubnet.Finalizers, util.ControllerName) + newSubnet.Finalizers = util.RemoveString(newSubnet.Finalizers, util.FinalizerName) patch, err := util.GenerateMergePatchPayload(subnet, newSubnet) if err != nil { klog.Errorf("failed to generate patch payload for subnet '%s', %v", subnet.Name, err) diff --git a/pkg/controller/vip.go b/pkg/controller/vip.go index 3657168f562..5e4745393e5 100644 --- a/pkg/controller/vip.go +++ b/pkg/controller/vip.go @@ -649,12 +649,12 @@ func (c *Controller) handleAddVipFinalizer(key string) error { return err } if cachedVip.DeletionTimestamp.IsZero() { - if slices.Contains(cachedVip.Finalizers, util.ControllerName) { + if slices.Contains(cachedVip.Finalizers, util.FinalizerName) { return nil } } newVip := cachedVip.DeepCopy() - controllerutil.AddFinalizer(newVip, util.ControllerName) + controllerutil.AddFinalizer(newVip, util.FinalizerName) patch, err := util.GenerateMergePatchPayload(cachedVip, newVip) if err != nil { klog.Errorf("failed to generate patch payload for ovn eip '%s', %v", cachedVip.Name, err) @@ -684,7 +684,7 @@ func (c *Controller) handleDelVipFinalizer(key string) error { return nil } newVip := cachedVip.DeepCopy() - controllerutil.RemoveFinalizer(newVip, util.ControllerName) + controllerutil.RemoveFinalizer(newVip, util.FinalizerName) patch, err := util.GenerateMergePatchPayload(cachedVip, newVip) if err != nil { klog.Errorf("failed to generate patch payload for ovn eip '%s', %v", cachedVip.Name, err) diff --git a/pkg/controller/vpc_nat_gw_eip.go b/pkg/controller/vpc_nat_gw_eip.go index 017a4eacfdb..fa1efbef561 100644 --- a/pkg/controller/vpc_nat_gw_eip.go +++ b/pkg/controller/vpc_nat_gw_eip.go @@ -760,12 +760,12 @@ func (c *Controller) handleAddIptablesEipFinalizer(key string) error { return err } if cachedIptablesEip.DeletionTimestamp.IsZero() { - if slices.Contains(cachedIptablesEip.Finalizers, util.ControllerName) { + if slices.Contains(cachedIptablesEip.Finalizers, util.FinalizerName) { return nil } } newIptablesEip := cachedIptablesEip.DeepCopy() - controllerutil.AddFinalizer(newIptablesEip, util.ControllerName) + controllerutil.AddFinalizer(newIptablesEip, util.FinalizerName) patch, err := util.GenerateMergePatchPayload(cachedIptablesEip, newIptablesEip) if err != nil { klog.Errorf("failed to generate patch payload for iptables eip '%s', %v", cachedIptablesEip.Name, err) @@ -795,7 +795,7 @@ func (c *Controller) handleDelIptablesEipFinalizer(key string) error { return nil } newIptablesEip := cachedIptablesEip.DeepCopy() - controllerutil.RemoveFinalizer(newIptablesEip, util.ControllerName) + controllerutil.RemoveFinalizer(newIptablesEip, util.FinalizerName) patch, err := util.GenerateMergePatchPayload(cachedIptablesEip, newIptablesEip) if err != nil { klog.Errorf("failed to generate patch payload for iptables eip '%s', %v", cachedIptablesEip.Name, err) diff --git a/pkg/controller/vpc_nat_gw_nat.go b/pkg/controller/vpc_nat_gw_nat.go index 44328254394..ce1a021f2c6 100644 --- a/pkg/controller/vpc_nat_gw_nat.go +++ b/pkg/controller/vpc_nat_gw_nat.go @@ -1054,12 +1054,12 @@ func (c *Controller) handleAddIptablesFipFinalizer(key string) error { return err } if cachedIptablesFip.DeletionTimestamp.IsZero() { - if slices.Contains(cachedIptablesFip.Finalizers, util.ControllerName) { + if slices.Contains(cachedIptablesFip.Finalizers, util.FinalizerName) { return nil } } newIptablesFip := cachedIptablesFip.DeepCopy() - controllerutil.AddFinalizer(newIptablesFip, util.ControllerName) + controllerutil.AddFinalizer(newIptablesFip, util.FinalizerName) patch, err := util.GenerateMergePatchPayload(cachedIptablesFip, newIptablesFip) if err != nil { klog.Errorf("failed to generate patch payload for iptables fip '%s', %v", cachedIptablesFip.Name, err) @@ -1089,7 +1089,7 @@ func (c *Controller) handleDelIptablesFipFinalizer(key string) error { return nil } newIptablesFip := cachedIptablesFip.DeepCopy() - controllerutil.RemoveFinalizer(newIptablesFip, util.ControllerName) + controllerutil.RemoveFinalizer(newIptablesFip, util.FinalizerName) patch, err := util.GenerateMergePatchPayload(cachedIptablesFip, newIptablesFip) if err != nil { klog.Errorf("failed to generate patch payload for iptables fip '%s', %v", cachedIptablesFip.Name, err) @@ -1116,12 +1116,12 @@ func (c *Controller) handleAddIptablesDnatFinalizer(key string) error { return err } if cachedIptablesDnat.DeletionTimestamp.IsZero() { - if slices.Contains(cachedIptablesDnat.Finalizers, util.ControllerName) { + if slices.Contains(cachedIptablesDnat.Finalizers, util.FinalizerName) { return nil } } newIptablesDnat := cachedIptablesDnat.DeepCopy() - controllerutil.AddFinalizer(newIptablesDnat, util.ControllerName) + controllerutil.AddFinalizer(newIptablesDnat, util.FinalizerName) patch, err := util.GenerateMergePatchPayload(cachedIptablesDnat, newIptablesDnat) if err != nil { klog.Errorf("failed to generate patch payload for iptables dnat '%s', %v", cachedIptablesDnat.Name, err) @@ -1151,7 +1151,7 @@ func (c *Controller) handleDelIptablesDnatFinalizer(key string) error { return nil } newIptablesDnat := cachedIptablesDnat.DeepCopy() - controllerutil.RemoveFinalizer(newIptablesDnat, util.ControllerName) + controllerutil.RemoveFinalizer(newIptablesDnat, util.FinalizerName) patch, err := util.GenerateMergePatchPayload(cachedIptablesDnat, newIptablesDnat) if err != nil { klog.Errorf("failed to generate patch payload for iptables dnat '%s', %v", cachedIptablesDnat.Name, err) @@ -1229,12 +1229,12 @@ func (c *Controller) handleAddIptablesSnatFinalizer(key string) error { return err } if cachedIptablesSnat.DeletionTimestamp.IsZero() { - if slices.Contains(cachedIptablesSnat.Finalizers, util.ControllerName) { + if slices.Contains(cachedIptablesSnat.Finalizers, util.FinalizerName) { return nil } } newIptablesSnat := cachedIptablesSnat.DeepCopy() - controllerutil.AddFinalizer(newIptablesSnat, util.ControllerName) + controllerutil.AddFinalizer(newIptablesSnat, util.FinalizerName) patch, err := util.GenerateMergePatchPayload(cachedIptablesSnat, newIptablesSnat) if err != nil { klog.Errorf("failed to generate patch payload for iptables snat '%s', %v", cachedIptablesSnat.Name, err) @@ -1264,7 +1264,7 @@ func (c *Controller) handleDelIptablesSnatFinalizer(key string) error { return nil } newIptablesSnat := cachedIptablesSnat.DeepCopy() - controllerutil.RemoveFinalizer(newIptablesSnat, util.ControllerName) + controllerutil.RemoveFinalizer(newIptablesSnat, util.FinalizerName) patch, err := util.GenerateMergePatchPayload(cachedIptablesSnat, newIptablesSnat) if err != nil { klog.Errorf("failed to generate patch payload for iptables snat '%s', %v", cachedIptablesSnat.Name, err) diff --git a/pkg/util/const.go b/pkg/util/const.go index 30cc990e313..d81be7d658f 100644 --- a/pkg/util/const.go +++ b/pkg/util/const.go @@ -3,7 +3,7 @@ package util const ( CniTypeName = "kube-ovn" - ControllerName = "kube-ovn-controller" + FinalizerName = "kubeovn.io/finalizer" AllocatedAnnotation = "ovn.kubernetes.io/allocated" RoutedAnnotation = "ovn.kubernetes.io/routed" diff --git a/test/e2e/kube-ovn/subnet/subnet.go b/test/e2e/kube-ovn/subnet/subnet.go index cddea940ee4..395de008ea4 100644 --- a/test/e2e/kube-ovn/subnet/subnet.go +++ b/test/e2e/kube-ovn/subnet/subnet.go @@ -148,7 +148,7 @@ var _ = framework.Describe("[group:subnet]", func() { subnet = subnetClient.CreateSync(subnet) ginkgo.By("Validating subnet finalizers") - framework.ExpectContainElement(subnet.Finalizers, util.ControllerName) + framework.ExpectContainElement(subnet.Finalizers, util.FinalizerName) ginkgo.By("Validating subnet spec fields") framework.ExpectFalse(subnet.Spec.Default) @@ -206,7 +206,7 @@ var _ = framework.Describe("[group:subnet]", func() { subnet = subnetClient.CreateSync(subnet) ginkgo.By("Validating subnet finalizers") - framework.ExpectContainElement(subnet.ObjectMeta.Finalizers, util.ControllerName) + framework.ExpectContainElement(subnet.ObjectMeta.Finalizers, util.FinalizerName) ginkgo.By("Validating subnet spec fields") framework.ExpectFalse(subnet.Spec.Default) @@ -251,7 +251,7 @@ var _ = framework.Describe("[group:subnet]", func() { subnet = subnetClient.CreateSync(subnet) ginkgo.By("Validating subnet finalizers") - framework.ExpectContainElement(subnet.ObjectMeta.Finalizers, util.ControllerName) + framework.ExpectContainElement(subnet.ObjectMeta.Finalizers, util.FinalizerName) ginkgo.By("Validating subnet spec fields") framework.ExpectFalse(subnet.Spec.Default) @@ -301,7 +301,7 @@ var _ = framework.Describe("[group:subnet]", func() { subnet = subnetClient.CreateSync(subnet) ginkgo.By("Validating subnet finalizers") - framework.ExpectContainElement(subnet.Finalizers, util.ControllerName) + framework.ExpectContainElement(subnet.Finalizers, util.FinalizerName) ginkgo.By("Validating subnet spec fields") framework.ExpectFalse(subnet.Spec.Default) @@ -345,7 +345,7 @@ var _ = framework.Describe("[group:subnet]", func() { subnet = subnetClient.CreateSync(subnet) ginkgo.By("Validating subnet finalizers") - framework.ExpectContainElement(subnet.Finalizers, util.ControllerName) + framework.ExpectContainElement(subnet.Finalizers, util.FinalizerName) ginkgo.By("Validating subnet spec fields") framework.ExpectFalse(subnet.Spec.Default) @@ -388,7 +388,7 @@ var _ = framework.Describe("[group:subnet]", func() { subnet = subnetClient.PatchSync(subnet, modifiedSubnet) ginkgo.By("Validating subnet finalizers") - framework.ExpectContainElement(subnet.ObjectMeta.Finalizers, util.ControllerName) + framework.ExpectContainElement(subnet.ObjectMeta.Finalizers, util.FinalizerName) ginkgo.By("Validating subnet spec fields") framework.ExpectFalse(subnet.Spec.Default) @@ -444,7 +444,7 @@ var _ = framework.Describe("[group:subnet]", func() { subnet = subnetClient.CreateSync(subnet) ginkgo.By("Validating subnet finalizers") - framework.ExpectContainElement(subnet.Finalizers, util.ControllerName) + framework.ExpectContainElement(subnet.Finalizers, util.FinalizerName) ginkgo.By("Validating centralized subnet with active-standby mode") framework.ExpectFalse(subnet.Spec.EnableEcmp) @@ -947,7 +947,7 @@ var _ = framework.Describe("[group:subnet]", func() { subnet = subnetClient.CreateSync(subnet) ginkgo.By("Validating subnet load-balancer records exist") - framework.ExpectContainElement(subnet.Finalizers, util.ControllerName) + framework.ExpectContainElement(subnet.Finalizers, util.FinalizerName) execCmd := "kubectl ko nbctl --format=csv --data=bare --no-heading --columns=load_balancer find logical-switch " + fmt.Sprintf("name=%s", subnetName) output, err := exec.Command("bash", "-c", execCmd).CombinedOutput() framework.ExpectNoError(err) From 53fe97265b81030dec58830ed2a90e1c73fc4c9f Mon Sep 17 00:00:00 2001 From: bobz965 Date: Tue, 20 Feb 2024 17:04:55 +0800 Subject: [PATCH 2/6] fix: migrate the finalizer field during initialization Signed-off-by: bobz965 --- pkg/controller/controller.go | 4 ++ pkg/controller/init.go | 53 +++++++++++++++ pkg/controller/ip.go | 37 +++++++++++ pkg/controller/ovn_dnat.go | 36 +++++++++++ pkg/controller/ovn_eip.go | 36 +++++++++++ pkg/controller/ovn_fip.go | 36 +++++++++++ pkg/controller/ovn_snat.go | 37 +++++++++++ pkg/controller/qos_policy.go | 36 +++++++++++ pkg/controller/subnet.go | 37 +++++++++++ pkg/controller/vip.go | 37 +++++++++++ pkg/controller/vpc_nat_gw_eip.go | 36 +++++++++++ pkg/controller/vpc_nat_gw_nat.go | 108 +++++++++++++++++++++++++++++++ pkg/util/const.go | 3 +- 13 files changed, 495 insertions(+), 1 deletion(-) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 8bd4d3e15da..473563f8341 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -764,6 +764,10 @@ func (c *Controller) Run(ctx context.Context) { util.LogFatalAndExit(err, "failed to sync crd ips") } + if err := c.syncFinalizers(); err != nil { + util.LogFatalAndExit(err, "failed to initialize crd finalizers") + } + if err := c.InitIPAM(); err != nil { util.LogFatalAndExit(err, "failed to initialize ipam") } diff --git a/pkg/controller/init.go b/pkg/controller/init.go index 43ef8dc0649..5a76ff9e11f 100644 --- a/pkg/controller/init.go +++ b/pkg/controller/init.go @@ -827,3 +827,56 @@ func (c *Controller) initNodeChassis() error { } return nil } + +func (c *Controller) syncFinalizers() error { + // migrate depreciated finalizer to new finalizer + if err := c.syncIPFinalizer(); err != nil { + klog.Errorf("failed to sync ip finalizer: %v", err) + return err + } + if err := c.syncOvnDnatFinalizer(); err != nil { + klog.Errorf("failed to sync ovn dnat finalizer: %v", err) + return err + } + if err := c.syncOvnEipFinalizer(); err != nil { + klog.Errorf("failed to sync ovn eip finalizer: %v", err) + return err + } + if err := c.syncOvnFipFinalizer(); err != nil { + klog.Errorf("failed to sync ovn fip finalizer: %v", err) + return err + } + if err := c.syncOvnSnatFinalizer(); err != nil { + klog.Errorf("failed to sync ovn snat finalizer: %v", err) + return err + } + if err := c.syncQoSPolicyFinalizer(); err != nil { + klog.Errorf("failed to sync qos policy finalizer: %v", err) + return err + } + if err := c.syncSubnetFinalizer(); err != nil { + klog.Errorf("failed to sync subnet finalizer: %v", err) + return err + } + if err := c.syncVipFinalizer(); err != nil { + klog.Errorf("failed to sync vip finalizer: %v", err) + return err + } + if err := c.syncIptablesEipFinalizer(); err != nil { + klog.Errorf("failed to sync iptables eip finalizer: %v", err) + return err + } + if err := c.syncIptablesFipFinalizer(); err != nil { + klog.Errorf("failed to sync iptables fip finalizer: %v", err) + return err + } + if err := c.syncIptablesDnatFinalizer(); err != nil { + klog.Errorf("failed to sync iptables dnat finalizer: %v", err) + return err + } + if err := c.syncIptablesSnatFinalizer(); err != nil { + klog.Errorf("failed to sync iptables snat finalizer: %v", err) + return err + } + return nil +} diff --git a/pkg/controller/ip.go b/pkg/controller/ip.go index 793a43dcd65..8295b195993 100644 --- a/pkg/controller/ip.go +++ b/pkg/controller/ip.go @@ -11,6 +11,7 @@ import ( k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/tools/cache" @@ -341,6 +342,42 @@ func (c *Controller) handleDelIP(ip *kubeovnv1.IP) error { return nil } +func (c *Controller) syncIPFinalizer() error { + // migrate depreciated finalizer to new finalizer + ips, err := c.ipsLister.List(labels.Everything()) + if err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to list ips, %v", err) + return err + } + for _, cachedIP := range ips { + if len(cachedIP.Finalizers) == 0 { + continue + } + if slices.Contains(cachedIP.Finalizers, util.DepreciatedFinalizerName) { + newIP := cachedIP.DeepCopy() + controllerutil.RemoveFinalizer(newIP, util.DepreciatedFinalizerName) + controllerutil.AddFinalizer(newIP, util.FinalizerName) + patch, err := util.GenerateMergePatchPayload(cachedIP, newIP) + if err != nil { + klog.Errorf("failed to generate patch payload for ip %s, %v", newIP.Name, err) + return err + } + if _, err := c.config.KubeOvnClient.KubeovnV1().IPs().Patch(context.Background(), newIP.Name, + types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to sync finalizer for ip %s, %v", newIP.Name, err) + return err + } + } + } + return nil +} + func (c *Controller) handleAddIPFinalizer(cachedIP *kubeovnv1.IP, finalizer string) error { if cachedIP.DeletionTimestamp.IsZero() { if slices.Contains(cachedIP.Finalizers, finalizer) { diff --git a/pkg/controller/ovn_dnat.go b/pkg/controller/ovn_dnat.go index 4dc04b4060b..82b948af95d 100644 --- a/pkg/controller/ovn_dnat.go +++ b/pkg/controller/ovn_dnat.go @@ -660,6 +660,42 @@ func (c *Controller) DelDnatRule(vpcName, dnatName, externalIP, externalPort str return nil } +func (c *Controller) syncOvnDnatFinalizer() error { + // migrate depreciated finalizer to new finalizer + dnats, err := c.ovnDnatRulesLister.List(labels.Everything()) + if err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to list dnats, %v", err) + return err + } + for _, cachedDnat := range dnats { + if len(cachedDnat.Finalizers) == 0 { + continue + } + if slices.Contains(cachedDnat.Finalizers, util.DepreciatedFinalizerName) { + newDnat := cachedDnat.DeepCopy() + controllerutil.RemoveFinalizer(newDnat, util.DepreciatedFinalizerName) + controllerutil.AddFinalizer(newDnat, util.FinalizerName) + patch, err := util.GenerateMergePatchPayload(cachedDnat, newDnat) + if err != nil { + klog.Errorf("failed to generate patch payload for dnat %s, %v", newDnat.Name, err) + return err + } + if _, err := c.config.KubeOvnClient.KubeovnV1().OvnDnatRules().Patch(context.Background(), newDnat.Name, + types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to sync finalizer for dnat %s, %v", newDnat.Name, err) + return err + } + } + } + return nil +} + func (c *Controller) handleAddOvnDnatFinalizer(cachedDnat *kubeovnv1.OvnDnatRule, finalizer string) error { if cachedDnat.DeletionTimestamp.IsZero() { if slices.Contains(cachedDnat.Finalizers, finalizer) { diff --git a/pkg/controller/ovn_eip.go b/pkg/controller/ovn_eip.go index 3e69c97ba48..ca20edc7a56 100644 --- a/pkg/controller/ovn_eip.go +++ b/pkg/controller/ovn_eip.go @@ -584,6 +584,42 @@ func (c *Controller) natLabelAndAnnoOvnEip(eipName, natName, vpcName string) err return err } +func (c *Controller) syncOvnEipFinalizer() error { + // migrate depreciated finalizer to new finalizer + eips, err := c.ovnEipsLister.List(labels.Everything()) + if err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to list eips, %v", err) + return err + } + for _, cachedEip := range eips { + if len(cachedEip.Finalizers) == 0 { + continue + } + if slices.Contains(cachedEip.Finalizers, util.DepreciatedFinalizerName) { + newEip := cachedEip.DeepCopy() + controllerutil.RemoveFinalizer(newEip, util.DepreciatedFinalizerName) + controllerutil.AddFinalizer(newEip, util.FinalizerName) + patch, err := util.GenerateMergePatchPayload(cachedEip, newEip) + if err != nil { + klog.Errorf("failed to generate patch payload for eip %s, %v", newEip.Name, err) + return err + } + if _, err := c.config.KubeOvnClient.KubeovnV1().OvnEips().Patch(context.Background(), newEip.Name, + types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to sync finalizer for eip %s, %v", newEip.Name, err) + return err + } + } + } + return nil +} + func (c *Controller) handleAddOvnEipFinalizer(cachedEip *kubeovnv1.OvnEip, finalizer string) error { if cachedEip.DeletionTimestamp.IsZero() { if slices.Contains(cachedEip.Finalizers, finalizer) { diff --git a/pkg/controller/ovn_fip.go b/pkg/controller/ovn_fip.go index 358c12255f3..d239a026fca 100644 --- a/pkg/controller/ovn_fip.go +++ b/pkg/controller/ovn_fip.go @@ -599,6 +599,42 @@ func (c *Controller) GetOvnEip(eipName string) (*kubeovnv1.OvnEip, error) { return cachedEip, nil } +func (c *Controller) syncOvnFipFinalizer() error { + // migrate depreciated finalizer to new finalizer + fips, err := c.ovnFipsLister.List(labels.Everything()) + if err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to list fips, %v", err) + return err + } + for _, cachedFip := range fips { + if len(cachedFip.Finalizers) == 0 { + continue + } + if slices.Contains(cachedFip.Finalizers, util.DepreciatedFinalizerName) { + newFip := cachedFip.DeepCopy() + controllerutil.RemoveFinalizer(newFip, util.DepreciatedFinalizerName) + controllerutil.AddFinalizer(newFip, util.FinalizerName) + patch, err := util.GenerateMergePatchPayload(cachedFip, newFip) + if err != nil { + klog.Errorf("failed to generate patch payload for fip %s, %v", newFip.Name, err) + return err + } + if _, err := c.config.KubeOvnClient.KubeovnV1().OvnFips().Patch(context.Background(), newFip.Name, + types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to sync finalizer for fip %s, %v", newFip.Name, err) + return err + } + } + } + return nil +} + func (c *Controller) handleAddOvnFipFinalizer(cachedFip *kubeovnv1.OvnFip, finalizer string) error { if cachedFip.DeletionTimestamp.IsZero() { if slices.Contains(cachedFip.Finalizers, finalizer) { diff --git a/pkg/controller/ovn_snat.go b/pkg/controller/ovn_snat.go index 3cdd6642419..a218a48b0a5 100644 --- a/pkg/controller/ovn_snat.go +++ b/pkg/controller/ovn_snat.go @@ -8,6 +8,7 @@ import ( k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/tools/cache" @@ -542,6 +543,42 @@ func (c *Controller) ovnSnatChangeEip(snat *kubeovnv1.OvnSnatRule, eip *kubeovnv return false } +func (c *Controller) syncOvnSnatFinalizer() error { + // migrate depreciated finalizer to new finalizer + snats, err := c.ovnSnatRulesLister.List(labels.Everything()) + if err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to list snats, %v", err) + return err + } + for _, cachedSnat := range snats { + if len(cachedSnat.Finalizers) == 0 { + continue + } + if slices.Contains(cachedSnat.Finalizers, util.DepreciatedFinalizerName) { + newSnat := cachedSnat.DeepCopy() + controllerutil.RemoveFinalizer(newSnat, util.DepreciatedFinalizerName) + controllerutil.AddFinalizer(newSnat, util.FinalizerName) + patch, err := util.GenerateMergePatchPayload(cachedSnat, newSnat) + if err != nil { + klog.Errorf("failed to generate patch payload for snat %s, %v", newSnat.Name, err) + return err + } + if _, err := c.config.KubeOvnClient.KubeovnV1().OvnSnatRules().Patch(context.Background(), newSnat.Name, + types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to sync finalizer for snat %s, %v", newSnat.Name, err) + return err + } + } + } + return nil +} + func (c *Controller) handleAddOvnSnatFinalizer(cachedSnat *kubeovnv1.OvnSnatRule, finalizer string) error { if cachedSnat.DeletionTimestamp.IsZero() { if slices.Contains(cachedSnat.Finalizers, finalizer) { diff --git a/pkg/controller/qos_policy.go b/pkg/controller/qos_policy.go index 5e27a056893..72f6e40f6f9 100644 --- a/pkg/controller/qos_policy.go +++ b/pkg/controller/qos_policy.go @@ -282,6 +282,42 @@ func (c *Controller) handleDelQoSPoliciesFinalizer(key string) error { return nil } +func (c *Controller) syncQoSPolicyFinalizer() error { + // migrate depreciated finalizer to new finalizer + qosPolicies, err := c.qosPoliciesLister.List(labels.Everything()) + if err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to list policy, %v", err) + return err + } + for _, cachedPolicy := range qosPolicies { + if len(cachedPolicy.Finalizers) == 0 { + continue + } + if slices.Contains(cachedPolicy.Finalizers, util.DepreciatedFinalizerName) { + newPolicy := cachedPolicy.DeepCopy() + controllerutil.RemoveFinalizer(newPolicy, util.DepreciatedFinalizerName) + controllerutil.AddFinalizer(newPolicy, util.FinalizerName) + patch, err := util.GenerateMergePatchPayload(cachedPolicy, newPolicy) + if err != nil { + klog.Errorf("failed to generate patch payload for policy %s, %v", newPolicy.Name, err) + return err + } + if _, err := c.config.KubeOvnClient.KubeovnV1().QoSPolicies().Patch(context.Background(), newPolicy.Name, + types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to sync finalizer for policy %s, %v", newPolicy.Name, err) + return err + } + } + } + return nil +} + func diffQoSPolicyBandwidthLimitRules(oldList, newList kubeovnv1.QoSPolicyBandwidthLimitRules) (added, deleted, updated kubeovnv1.QoSPolicyBandwidthLimitRules) { added = kubeovnv1.QoSPolicyBandwidthLimitRules{} deleted = kubeovnv1.QoSPolicyBandwidthLimitRules{} diff --git a/pkg/controller/subnet.go b/pkg/controller/subnet.go index 7faba89d912..c2965414e54 100644 --- a/pkg/controller/subnet.go +++ b/pkg/controller/subnet.go @@ -20,6 +20,7 @@ import ( utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/tools/cache" "k8s.io/klog/v2" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1" "github.com/kubeovn/kube-ovn/pkg/ipam" @@ -479,6 +480,42 @@ func checkAndUpdateExcludeIPs(subnet *kubeovnv1.Subnet) bool { return changed } +func (c *Controller) syncSubnetFinalizer() error { + // migrate depreciated finalizer to new finalizer + subnets, err := c.subnetsLister.List(labels.Everything()) + if err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to list subnets, %v", err) + return err + } + for _, cachedSubnet := range subnets { + if len(cachedSubnet.Finalizers) == 0 { + continue + } + if slices.Contains(cachedSubnet.Finalizers, util.DepreciatedFinalizerName) { + newSubnet := cachedSubnet.DeepCopy() + controllerutil.RemoveFinalizer(newSubnet, util.DepreciatedFinalizerName) + controllerutil.AddFinalizer(newSubnet, util.FinalizerName) + patch, err := util.GenerateMergePatchPayload(cachedSubnet, newSubnet) + if err != nil { + klog.Errorf("failed to generate patch payload for subnet %s, %v", newSubnet.Name, err) + return err + } + if _, err := c.config.KubeOvnClient.KubeovnV1().Subnets().Patch(context.Background(), newSubnet.Name, + types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to sync finalizer for subnet %s, %v", newSubnet.Name, err) + return err + } + } + } + return nil +} + func (c *Controller) handleSubnetFinalizer(subnet *kubeovnv1.Subnet) (bool, error) { if subnet.DeletionTimestamp.IsZero() && !slices.Contains(subnet.Finalizers, util.FinalizerName) { newSubnet := subnet.DeepCopy() diff --git a/pkg/controller/vip.go b/pkg/controller/vip.go index 5e4745393e5..74078be7f04 100644 --- a/pkg/controller/vip.go +++ b/pkg/controller/vip.go @@ -10,6 +10,7 @@ import ( k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/tools/cache" @@ -700,3 +701,39 @@ func (c *Controller) handleDelVipFinalizer(key string) error { } return nil } + +func (c *Controller) syncVipFinalizer() error { + // migrate depreciated finalizer to new finalizer + vips, err := c.virtualIpsLister.List(labels.Everything()) + if err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to list vips, %v", err) + return err + } + for _, cachedVip := range vips { + if len(cachedVip.Finalizers) == 0 { + continue + } + if slices.Contains(cachedVip.Finalizers, util.DepreciatedFinalizerName) { + newVip := cachedVip.DeepCopy() + controllerutil.RemoveFinalizer(newVip, util.DepreciatedFinalizerName) + controllerutil.AddFinalizer(newVip, util.FinalizerName) + patch, err := util.GenerateMergePatchPayload(cachedVip, newVip) + if err != nil { + klog.Errorf("failed to generate patch payload for vip %s, %v", newVip.Name, err) + return err + } + if _, err := c.config.KubeOvnClient.KubeovnV1().Vips().Patch(context.Background(), newVip.Name, + types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to sync finalizer for vip %s, %v", newVip.Name, err) + return err + } + } + } + return nil +} diff --git a/pkg/controller/vpc_nat_gw_eip.go b/pkg/controller/vpc_nat_gw_eip.go index fa1efbef561..f47c55ea665 100644 --- a/pkg/controller/vpc_nat_gw_eip.go +++ b/pkg/controller/vpc_nat_gw_eip.go @@ -750,6 +750,42 @@ func (c *Controller) createOrUpdateEipCR(key, v4ip, v6ip, mac, natGwDp, qos, ext return nil } +func (c *Controller) syncIptablesEipFinalizer() error { + // migrate depreciated finalizer to new finalizer + eips, err := c.iptablesEipsLister.List(labels.Everything()) + if err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to list eips, %v", err) + return err + } + for _, cachedEip := range eips { + if len(cachedEip.Finalizers) == 0 { + continue + } + if slices.Contains(cachedEip.Finalizers, util.DepreciatedFinalizerName) { + newEip := cachedEip.DeepCopy() + controllerutil.RemoveFinalizer(newEip, util.DepreciatedFinalizerName) + controllerutil.AddFinalizer(newEip, util.FinalizerName) + patch, err := util.GenerateMergePatchPayload(cachedEip, newEip) + if err != nil { + klog.Errorf("failed to generate patch payload for eip %s, %v", newEip.Name, err) + return err + } + if _, err := c.config.KubeOvnClient.KubeovnV1().IptablesEIPs().Patch(context.Background(), newEip.Name, + types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to sync finalizer for eip %s, %v", newEip.Name, err) + return err + } + } + } + return nil +} + func (c *Controller) handleAddIptablesEipFinalizer(key string) error { cachedIptablesEip, err := c.iptablesEipsLister.Get(key) if err != nil { diff --git a/pkg/controller/vpc_nat_gw_nat.go b/pkg/controller/vpc_nat_gw_nat.go index ce1a021f2c6..b5895f725e7 100644 --- a/pkg/controller/vpc_nat_gw_nat.go +++ b/pkg/controller/vpc_nat_gw_nat.go @@ -1044,6 +1044,42 @@ func (c *Controller) handleDelIptablesSnatRule(key string) error { return nil } +func (c *Controller) syncIptablesFipFinalizer() error { + // migrate depreciated finalizer to new finalizer + fips, err := c.iptablesFipsLister.List(labels.Everything()) + if err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to list fips, %v", err) + return err + } + for _, cachedFip := range fips { + if len(cachedFip.Finalizers) == 0 { + continue + } + if slices.Contains(cachedFip.Finalizers, util.DepreciatedFinalizerName) { + newFip := cachedFip.DeepCopy() + controllerutil.RemoveFinalizer(newFip, util.DepreciatedFinalizerName) + controllerutil.AddFinalizer(newFip, util.FinalizerName) + patch, err := util.GenerateMergePatchPayload(cachedFip, newFip) + if err != nil { + klog.Errorf("failed to generate patch payload for fip %s, %v", newFip.Name, err) + return err + } + if _, err := c.config.KubeOvnClient.KubeovnV1().IptablesFIPRules().Patch(context.Background(), newFip.Name, + types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to sync finalizer for fip %s, %v", newFip.Name, err) + return err + } + } + } + return nil +} + func (c *Controller) handleAddIptablesFipFinalizer(key string) error { cachedIptablesFip, err := c.iptablesFipsLister.Get(key) if err != nil { @@ -1106,6 +1142,42 @@ func (c *Controller) handleDelIptablesFipFinalizer(key string) error { return nil } +func (c *Controller) syncIptablesDnatFinalizer() error { + // migrate depreciated finalizer to new finalizer + dnats, err := c.iptablesDnatRulesLister.List(labels.Everything()) + if err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to list dnats, %v", err) + return err + } + for _, cachedDnat := range dnats { + if len(cachedDnat.Finalizers) == 0 { + continue + } + if slices.Contains(cachedDnat.Finalizers, util.DepreciatedFinalizerName) { + newDnat := cachedDnat.DeepCopy() + controllerutil.RemoveFinalizer(newDnat, util.DepreciatedFinalizerName) + controllerutil.AddFinalizer(newDnat, util.FinalizerName) + patch, err := util.GenerateMergePatchPayload(cachedDnat, newDnat) + if err != nil { + klog.Errorf("failed to generate patch payload for dnat %s, %v", newDnat.Name, err) + return err + } + if _, err := c.config.KubeOvnClient.KubeovnV1().IptablesDnatRules().Patch(context.Background(), newDnat.Name, + types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to sync finalizer for dnat %s, %v", newDnat.Name, err) + return err + } + } + } + return nil +} + func (c *Controller) handleAddIptablesDnatFinalizer(key string) error { cachedIptablesDnat, err := c.iptablesDnatRulesLister.Get(key) if err != nil { @@ -1219,6 +1291,42 @@ func (c *Controller) patchFipLabel(key string, eip *kubeovnv1.IptablesEIP) error return nil } +func (c *Controller) syncIptablesSnatFinalizer() error { + // migrate depreciated finalizer to new finalizer + snats, err := c.iptablesSnatRulesLister.List(labels.Everything()) + if err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to list snats, %v", err) + return err + } + for _, cachedSnat := range snats { + if len(cachedSnat.Finalizers) == 0 { + continue + } + if slices.Contains(cachedSnat.Finalizers, util.DepreciatedFinalizerName) { + newSnat := cachedSnat.DeepCopy() + controllerutil.RemoveFinalizer(newSnat, util.DepreciatedFinalizerName) + controllerutil.AddFinalizer(newSnat, util.FinalizerName) + patch, err := util.GenerateMergePatchPayload(cachedSnat, newSnat) + if err != nil { + klog.Errorf("failed to generate patch payload for dnat %s, %v", newSnat.Name, err) + return err + } + if _, err := c.config.KubeOvnClient.KubeovnV1().IptablesSnatRules().Patch(context.Background(), newSnat.Name, + types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to sync finalizer for dnat %s, %v", newSnat.Name, err) + return err + } + } + } + return nil +} + func (c *Controller) handleAddIptablesSnatFinalizer(key string) error { cachedIptablesSnat, err := c.iptablesSnatRulesLister.Get(key) if err != nil { diff --git a/pkg/util/const.go b/pkg/util/const.go index d81be7d658f..86d3f7c558e 100644 --- a/pkg/util/const.go +++ b/pkg/util/const.go @@ -3,7 +3,8 @@ package util const ( CniTypeName = "kube-ovn" - FinalizerName = "kubeovn.io/finalizer" + DepreciatedFinalizerName = "kube-ovn-controller" + FinalizerName = "kubeovn.io/finalizer" AllocatedAnnotation = "ovn.kubernetes.io/allocated" RoutedAnnotation = "ovn.kubernetes.io/routed" From 795817e19f4ec64fb81e754afcad02cc4e2a9275 Mon Sep 17 00:00:00 2001 From: bobz965 Date: Tue, 20 Feb 2024 17:11:33 +0800 Subject: [PATCH 3/6] add log Signed-off-by: bobz965 --- pkg/controller/init.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/controller/init.go b/pkg/controller/init.go index 5a76ff9e11f..69e687dd9bd 100644 --- a/pkg/controller/init.go +++ b/pkg/controller/init.go @@ -830,6 +830,7 @@ func (c *Controller) initNodeChassis() error { func (c *Controller) syncFinalizers() error { // migrate depreciated finalizer to new finalizer + klog.Info("start to sync finalizers") if err := c.syncIPFinalizer(); err != nil { klog.Errorf("failed to sync ip finalizer: %v", err) return err @@ -878,5 +879,6 @@ func (c *Controller) syncFinalizers() error { klog.Errorf("failed to sync iptables snat finalizer: %v", err) return err } + klog.Info("sync finalizers done") return nil } From 69ea00beb7f8c6ee46c0b501459b277c80b26347 Mon Sep 17 00:00:00 2001 From: bobz965 Date: Wed, 21 Feb 2024 11:01:07 +0800 Subject: [PATCH 4/6] Update pkg/util/const.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: 张祖建 --- pkg/util/const.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/util/const.go b/pkg/util/const.go index 86d3f7c558e..e68dd8173a0 100644 --- a/pkg/util/const.go +++ b/pkg/util/const.go @@ -4,7 +4,7 @@ const ( CniTypeName = "kube-ovn" DepreciatedFinalizerName = "kube-ovn-controller" - FinalizerName = "kubeovn.io/finalizer" + KubeOVNControllerFinalizer = "kubeovn.io/kube-ovn-controller" AllocatedAnnotation = "ovn.kubernetes.io/allocated" RoutedAnnotation = "ovn.kubernetes.io/routed" From 944382acb840e18e9871507e178de76783a0a7ed Mon Sep 17 00:00:00 2001 From: bobz965 Date: Wed, 21 Feb 2024 11:34:29 +0800 Subject: [PATCH 5/6] optimize Signed-off-by: bobz965 --- pkg/controller/init.go | 19 ++++++- pkg/controller/ip.go | 26 ++++----- pkg/controller/ovn_dnat.go | 23 ++++---- pkg/controller/ovn_eip.go | 23 ++++---- pkg/controller/ovn_fip.go | 26 ++++----- pkg/controller/ovn_snat.go | 26 ++++----- pkg/controller/qos_policy.go | 26 ++++----- pkg/controller/subnet.go | 27 ++++------ pkg/controller/vip.go | 26 ++++----- pkg/controller/vpc_nat_gw_eip.go | 28 ++++------ pkg/controller/vpc_nat_gw_nat.go | 84 ++++++++++++------------------ pkg/util/const.go | 4 +- test/e2e/kube-ovn/subnet/subnet.go | 16 +++--- 13 files changed, 152 insertions(+), 202 deletions(-) diff --git a/pkg/controller/init.go b/pkg/controller/init.go index 69e687dd9bd..6cc2f6d49e5 100644 --- a/pkg/controller/init.go +++ b/pkg/controller/init.go @@ -15,6 +15,8 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" "k8s.io/klog/v2" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1" "github.com/kubeovn/kube-ovn/pkg/ovs" @@ -578,7 +580,7 @@ func (c *Controller) syncIPCR() error { for _, ipCR := range ips { ip := ipCR.DeepCopy() - if ip.DeletionTimestamp != nil && slices.Contains(ip.Finalizers, util.FinalizerName) { + if ip.DeletionTimestamp != nil && slices.Contains(ip.Finalizers, util.KubeOVNControllerFinalizer) { klog.Infof("enqueue update for deleting ip %s", ip.Name) c.updateIPQueue.Add(ip.Name) } @@ -882,3 +884,18 @@ func (c *Controller) syncFinalizers() error { klog.Info("sync finalizers done") return nil } + +func (c *Controller) ReplaceFinalizer(cachedObj client.Object) ([]byte, error) { + if slices.Contains(cachedObj.GetFinalizers(), util.DepreciatedFinalizerName) { + newObj := cachedObj.DeepCopyObject().(client.Object) + controllerutil.RemoveFinalizer(newObj, util.DepreciatedFinalizerName) + controllerutil.AddFinalizer(newObj, util.KubeOVNControllerFinalizer) + patch, err := util.GenerateMergePatchPayload(cachedObj, newObj) + if err != nil { + klog.Errorf("failed to generate patch payload for %s, %v", newObj.GetName(), err) + return nil, err + } + return patch, nil + } + return nil, nil +} diff --git a/pkg/controller/ip.go b/pkg/controller/ip.go index 8295b195993..83003856098 100644 --- a/pkg/controller/ip.go +++ b/pkg/controller/ip.go @@ -324,7 +324,7 @@ func (c *Controller) handleUpdateIP(key string) error { klog.Infof("ip cr %s release ipam pod key %s from subnet %s", cachedIP.Name, podKey, cachedIP.Spec.Subnet) c.ipam.ReleaseAddressByPod(podKey, cachedIP.Spec.Subnet) } - if err = c.handleDelIPFinalizer(cachedIP, util.FinalizerName); err != nil { + if err = c.handleDelIPFinalizer(cachedIP, util.KubeOVNControllerFinalizer); err != nil { klog.Errorf("failed to handle del ip finalizer %v", err) return err } @@ -353,24 +353,18 @@ func (c *Controller) syncIPFinalizer() error { return err } for _, cachedIP := range ips { - if len(cachedIP.Finalizers) == 0 { - continue - } - if slices.Contains(cachedIP.Finalizers, util.DepreciatedFinalizerName) { - newIP := cachedIP.DeepCopy() - controllerutil.RemoveFinalizer(newIP, util.DepreciatedFinalizerName) - controllerutil.AddFinalizer(newIP, util.FinalizerName) - patch, err := util.GenerateMergePatchPayload(cachedIP, newIP) - if err != nil { - klog.Errorf("failed to generate patch payload for ip %s, %v", newIP.Name, err) - return err - } - if _, err := c.config.KubeOvnClient.KubeovnV1().IPs().Patch(context.Background(), newIP.Name, + patch, err := c.ReplaceFinalizer(cachedIP) + if err != nil { + klog.Errorf("failed to sync finalizer for ip %s, %v", cachedIP.Name, err) + return err + } + if patch != nil { + if _, err := c.config.KubeOvnClient.KubeovnV1().IPs().Patch(context.Background(), cachedIP.Name, types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil { if k8serrors.IsNotFound(err) { return nil } - klog.Errorf("failed to sync finalizer for ip %s, %v", newIP.Name, err) + klog.Errorf("failed to sync finalizer for ip %s, %v", cachedIP.Name, err) return err } } @@ -572,7 +566,7 @@ func (c *Controller) createOrUpdateIPCR(ipCRName, podName, ip, mac, subnetName, } } - if err := c.handleAddIPFinalizer(ipCR, util.FinalizerName); err != nil { + if err := c.handleAddIPFinalizer(ipCR, util.KubeOVNControllerFinalizer); err != nil { klog.Errorf("failed to handle add ip finalizer %v", err) return err } diff --git a/pkg/controller/ovn_dnat.go b/pkg/controller/ovn_dnat.go index 82b948af95d..1a788e7a8b5 100644 --- a/pkg/controller/ovn_dnat.go +++ b/pkg/controller/ovn_dnat.go @@ -301,7 +301,7 @@ func (c *Controller) handleAddOvnDnatRule(key string) error { return err } - if err := c.handleAddOvnDnatFinalizer(cachedDnat, util.FinalizerName); err != nil { + if err := c.handleAddOvnDnatFinalizer(cachedDnat, util.KubeOVNControllerFinalizer); err != nil { klog.Errorf("failed to add finalizer for ovn dnat %s, %v", cachedDnat.Name, err) return err } @@ -348,7 +348,7 @@ func (c *Controller) handleDelOvnDnatRule(key string) error { return err } } - if err = c.handleDelOvnDnatFinalizer(cachedDnat, util.FinalizerName); err != nil { + if err = c.handleDelOvnDnatFinalizer(cachedDnat, util.KubeOVNControllerFinalizer); err != nil { klog.Errorf("failed to remove finalizer for ovn dnat %s, %v", cachedDnat.Name, err) return err } @@ -674,21 +674,18 @@ func (c *Controller) syncOvnDnatFinalizer() error { if len(cachedDnat.Finalizers) == 0 { continue } - if slices.Contains(cachedDnat.Finalizers, util.DepreciatedFinalizerName) { - newDnat := cachedDnat.DeepCopy() - controllerutil.RemoveFinalizer(newDnat, util.DepreciatedFinalizerName) - controllerutil.AddFinalizer(newDnat, util.FinalizerName) - patch, err := util.GenerateMergePatchPayload(cachedDnat, newDnat) - if err != nil { - klog.Errorf("failed to generate patch payload for dnat %s, %v", newDnat.Name, err) - return err - } - if _, err := c.config.KubeOvnClient.KubeovnV1().OvnDnatRules().Patch(context.Background(), newDnat.Name, + patch, err := c.ReplaceFinalizer(cachedDnat) + if err != nil { + klog.Errorf("failed to sync finalizer for dnat %s, %v", cachedDnat.Name, err) + return err + } + if patch != nil { + if _, err := c.config.KubeOvnClient.KubeovnV1().OvnDnatRules().Patch(context.Background(), cachedDnat.Name, types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil { if k8serrors.IsNotFound(err) { return nil } - klog.Errorf("failed to sync finalizer for dnat %s, %v", newDnat.Name, err) + klog.Errorf("failed to sync finalizer for dnat %s, %v", cachedDnat.Name, err) return err } } diff --git a/pkg/controller/ovn_eip.go b/pkg/controller/ovn_eip.go index ca20edc7a56..e0bfd060d66 100644 --- a/pkg/controller/ovn_eip.go +++ b/pkg/controller/ovn_eip.go @@ -268,7 +268,7 @@ func (c *Controller) handleAddOvnEip(key string) error { return err } } - if err = c.handleAddOvnEipFinalizer(cachedEip, util.FinalizerName); err != nil { + if err = c.handleAddOvnEipFinalizer(cachedEip, util.KubeOVNControllerFinalizer); err != nil { klog.Errorf("failed to add finalizer for ovn eip, %v", err) return err } @@ -343,7 +343,7 @@ func (c *Controller) handleDelOvnEip(key string) error { } } - if err = c.handleDelOvnEipFinalizer(eip, util.FinalizerName); err != nil { + if err = c.handleDelOvnEipFinalizer(eip, util.KubeOVNControllerFinalizer); err != nil { klog.Errorf("failed to handle remove ovn eip finalizer , %v", err) return err } @@ -598,21 +598,18 @@ func (c *Controller) syncOvnEipFinalizer() error { if len(cachedEip.Finalizers) == 0 { continue } - if slices.Contains(cachedEip.Finalizers, util.DepreciatedFinalizerName) { - newEip := cachedEip.DeepCopy() - controllerutil.RemoveFinalizer(newEip, util.DepreciatedFinalizerName) - controllerutil.AddFinalizer(newEip, util.FinalizerName) - patch, err := util.GenerateMergePatchPayload(cachedEip, newEip) - if err != nil { - klog.Errorf("failed to generate patch payload for eip %s, %v", newEip.Name, err) - return err - } - if _, err := c.config.KubeOvnClient.KubeovnV1().OvnEips().Patch(context.Background(), newEip.Name, + patch, err := c.ReplaceFinalizer(cachedEip) + if err != nil { + klog.Errorf("failed to sync finalizer for eip %s, %v", cachedEip.Name, err) + return err + } + if patch != nil { + if _, err := c.config.KubeOvnClient.KubeovnV1().OvnEips().Patch(context.Background(), cachedEip.Name, types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil { if k8serrors.IsNotFound(err) { return nil } - klog.Errorf("failed to sync finalizer for eip %s, %v", newEip.Name, err) + klog.Errorf("failed to sync finalizer for eip %s, %v", cachedEip.Name, err) return err } } diff --git a/pkg/controller/ovn_fip.go b/pkg/controller/ovn_fip.go index d239a026fca..2a867ec87ba 100644 --- a/pkg/controller/ovn_fip.go +++ b/pkg/controller/ovn_fip.go @@ -296,7 +296,7 @@ func (c *Controller) handleAddOvnFip(key string) error { return err } - if err = c.handleAddOvnFipFinalizer(cachedFip, util.FinalizerName); err != nil { + if err = c.handleAddOvnFipFinalizer(cachedFip, util.KubeOVNControllerFinalizer); err != nil { klog.Errorf("failed to add finalizer for ovn fip, %v", err) return err } @@ -463,7 +463,7 @@ func (c *Controller) handleDelOvnFip(key string) error { return err } } - if err = c.handleDelOvnFipFinalizer(cachedFip, util.FinalizerName); err != nil { + if err = c.handleDelOvnFipFinalizer(cachedFip, util.KubeOVNControllerFinalizer); err != nil { klog.Errorf("failed to remove finalizer for ovn fip %s, %v", cachedFip.Name, err) return err } @@ -610,24 +610,18 @@ func (c *Controller) syncOvnFipFinalizer() error { return err } for _, cachedFip := range fips { - if len(cachedFip.Finalizers) == 0 { - continue - } - if slices.Contains(cachedFip.Finalizers, util.DepreciatedFinalizerName) { - newFip := cachedFip.DeepCopy() - controllerutil.RemoveFinalizer(newFip, util.DepreciatedFinalizerName) - controllerutil.AddFinalizer(newFip, util.FinalizerName) - patch, err := util.GenerateMergePatchPayload(cachedFip, newFip) - if err != nil { - klog.Errorf("failed to generate patch payload for fip %s, %v", newFip.Name, err) - return err - } - if _, err := c.config.KubeOvnClient.KubeovnV1().OvnFips().Patch(context.Background(), newFip.Name, + patch, err := c.ReplaceFinalizer(cachedFip) + if err != nil { + klog.Errorf("failed to sync finalizer for fip %s, %v", cachedFip.Name, err) + return err + } + if patch != nil { + if _, err := c.config.KubeOvnClient.KubeovnV1().OvnFips().Patch(context.Background(), cachedFip.Name, types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil { if k8serrors.IsNotFound(err) { return nil } - klog.Errorf("failed to sync finalizer for fip %s, %v", newFip.Name, err) + klog.Errorf("failed to sync finalizer for fip %s, %v", cachedFip.Name, err) return err } } diff --git a/pkg/controller/ovn_snat.go b/pkg/controller/ovn_snat.go index a218a48b0a5..1be94d1f31e 100644 --- a/pkg/controller/ovn_snat.go +++ b/pkg/controller/ovn_snat.go @@ -258,7 +258,7 @@ func (c *Controller) handleAddOvnSnatRule(key string) error { klog.Errorf("failed to create snat, %v", err) return err } - if err := c.handleAddOvnSnatFinalizer(cachedSnat, util.FinalizerName); err != nil { + if err := c.handleAddOvnSnatFinalizer(cachedSnat, util.KubeOVNControllerFinalizer); err != nil { klog.Errorf("failed to add finalizer for ovn snat %s, %v", cachedSnat.Name, err) return err } @@ -420,7 +420,7 @@ func (c *Controller) handleDelOvnSnatRule(key string) error { return err } } - if err = c.handleDelOvnSnatFinalizer(cachedSnat, util.FinalizerName); err != nil { + if err = c.handleDelOvnSnatFinalizer(cachedSnat, util.KubeOVNControllerFinalizer); err != nil { klog.Errorf("failed to remove finalizer for ovn snat %s, %v", cachedSnat.Name, err) return err } @@ -554,24 +554,18 @@ func (c *Controller) syncOvnSnatFinalizer() error { return err } for _, cachedSnat := range snats { - if len(cachedSnat.Finalizers) == 0 { - continue - } - if slices.Contains(cachedSnat.Finalizers, util.DepreciatedFinalizerName) { - newSnat := cachedSnat.DeepCopy() - controllerutil.RemoveFinalizer(newSnat, util.DepreciatedFinalizerName) - controllerutil.AddFinalizer(newSnat, util.FinalizerName) - patch, err := util.GenerateMergePatchPayload(cachedSnat, newSnat) - if err != nil { - klog.Errorf("failed to generate patch payload for snat %s, %v", newSnat.Name, err) - return err - } - if _, err := c.config.KubeOvnClient.KubeovnV1().OvnSnatRules().Patch(context.Background(), newSnat.Name, + patch, err := c.ReplaceFinalizer(cachedSnat) + if err != nil { + klog.Errorf("failed to sync finalizer for snat %s, %v", cachedSnat.Name, err) + return err + } + if patch != nil { + if _, err := c.config.KubeOvnClient.KubeovnV1().OvnSnatRules().Patch(context.Background(), cachedSnat.Name, types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil { if k8serrors.IsNotFound(err) { return nil } - klog.Errorf("failed to sync finalizer for snat %s, %v", newSnat.Name, err) + klog.Errorf("failed to sync finalizer for snat %s, %v", cachedSnat.Name, err) return err } } diff --git a/pkg/controller/qos_policy.go b/pkg/controller/qos_policy.go index 72f6e40f6f9..86e46ad72cd 100644 --- a/pkg/controller/qos_policy.go +++ b/pkg/controller/qos_policy.go @@ -265,7 +265,7 @@ func (c *Controller) handleDelQoSPoliciesFinalizer(key string) error { return nil } newQoSPolicies := cachedQoSPolicies.DeepCopy() - controllerutil.RemoveFinalizer(newQoSPolicies, util.FinalizerName) + controllerutil.RemoveFinalizer(newQoSPolicies, util.KubeOVNControllerFinalizer) patch, err := util.GenerateMergePatchPayload(cachedQoSPolicies, newQoSPolicies) if err != nil { klog.Errorf("failed to generate patch payload for qos '%s', %v", cachedQoSPolicies.Name, err) @@ -293,24 +293,18 @@ func (c *Controller) syncQoSPolicyFinalizer() error { return err } for _, cachedPolicy := range qosPolicies { - if len(cachedPolicy.Finalizers) == 0 { - continue + patch, err := c.ReplaceFinalizer(cachedPolicy) + if err != nil { + klog.Errorf("failed to sync finalizer for policy %s, %v", cachedPolicy.Name, err) + return err } - if slices.Contains(cachedPolicy.Finalizers, util.DepreciatedFinalizerName) { - newPolicy := cachedPolicy.DeepCopy() - controllerutil.RemoveFinalizer(newPolicy, util.DepreciatedFinalizerName) - controllerutil.AddFinalizer(newPolicy, util.FinalizerName) - patch, err := util.GenerateMergePatchPayload(cachedPolicy, newPolicy) - if err != nil { - klog.Errorf("failed to generate patch payload for policy %s, %v", newPolicy.Name, err) - return err - } - if _, err := c.config.KubeOvnClient.KubeovnV1().QoSPolicies().Patch(context.Background(), newPolicy.Name, + if patch != nil { + if _, err := c.config.KubeOvnClient.KubeovnV1().QoSPolicies().Patch(context.Background(), cachedPolicy.Name, types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil { if k8serrors.IsNotFound(err) { return nil } - klog.Errorf("failed to sync finalizer for policy %s, %v", newPolicy.Name, err) + klog.Errorf("failed to sync finalizer for policy %s, %v", cachedPolicy.Name, err) return err } } @@ -557,12 +551,12 @@ func (c *Controller) handleAddQoSPolicyFinalizer(key string) error { return err } if cachedQoSPolicy.DeletionTimestamp.IsZero() { - if slices.Contains(cachedQoSPolicy.Finalizers, util.FinalizerName) { + if slices.Contains(cachedQoSPolicy.Finalizers, util.KubeOVNControllerFinalizer) { return nil } } newQoSPolicy := cachedQoSPolicy.DeepCopy() - controllerutil.AddFinalizer(newQoSPolicy, util.FinalizerName) + controllerutil.AddFinalizer(newQoSPolicy, util.KubeOVNControllerFinalizer) patch, err := util.GenerateMergePatchPayload(cachedQoSPolicy, newQoSPolicy) if err != nil { klog.Errorf("failed to generate patch payload for qos '%s', %v", cachedQoSPolicy.Name, err) diff --git a/pkg/controller/subnet.go b/pkg/controller/subnet.go index c2965414e54..a9567cd7e92 100644 --- a/pkg/controller/subnet.go +++ b/pkg/controller/subnet.go @@ -20,7 +20,6 @@ import ( utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/tools/cache" "k8s.io/klog/v2" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1" "github.com/kubeovn/kube-ovn/pkg/ipam" @@ -491,24 +490,18 @@ func (c *Controller) syncSubnetFinalizer() error { return err } for _, cachedSubnet := range subnets { - if len(cachedSubnet.Finalizers) == 0 { - continue + patch, err := c.ReplaceFinalizer(cachedSubnet) + if err != nil { + klog.Errorf("failed to sync finalizer for subnet %s, %v", cachedSubnet.Name, err) + return err } - if slices.Contains(cachedSubnet.Finalizers, util.DepreciatedFinalizerName) { - newSubnet := cachedSubnet.DeepCopy() - controllerutil.RemoveFinalizer(newSubnet, util.DepreciatedFinalizerName) - controllerutil.AddFinalizer(newSubnet, util.FinalizerName) - patch, err := util.GenerateMergePatchPayload(cachedSubnet, newSubnet) - if err != nil { - klog.Errorf("failed to generate patch payload for subnet %s, %v", newSubnet.Name, err) - return err - } - if _, err := c.config.KubeOvnClient.KubeovnV1().Subnets().Patch(context.Background(), newSubnet.Name, + if patch != nil { + if _, err := c.config.KubeOvnClient.KubeovnV1().Subnets().Patch(context.Background(), cachedSubnet.Name, types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil { if k8serrors.IsNotFound(err) { return nil } - klog.Errorf("failed to sync finalizer for subnet %s, %v", newSubnet.Name, err) + klog.Errorf("failed to sync finalizer for subnet %s, %v", cachedSubnet.Name, err) return err } } @@ -517,9 +510,9 @@ func (c *Controller) syncSubnetFinalizer() error { } func (c *Controller) handleSubnetFinalizer(subnet *kubeovnv1.Subnet) (bool, error) { - if subnet.DeletionTimestamp.IsZero() && !slices.Contains(subnet.Finalizers, util.FinalizerName) { + if subnet.DeletionTimestamp.IsZero() && !slices.Contains(subnet.Finalizers, util.KubeOVNControllerFinalizer) { newSubnet := subnet.DeepCopy() - newSubnet.Finalizers = append(newSubnet.Finalizers, util.FinalizerName) + newSubnet.Finalizers = append(newSubnet.Finalizers, util.KubeOVNControllerFinalizer) patch, err := util.GenerateMergePatchPayload(subnet, newSubnet) if err != nil { klog.Errorf("failed to generate patch payload for subnet '%s', %v", subnet.Name, err) @@ -543,7 +536,7 @@ func (c *Controller) handleSubnetFinalizer(subnet *kubeovnv1.Subnet) (bool, erro u2oInterconnIP := subnet.Status.U2OInterconnectionIP if !subnet.DeletionTimestamp.IsZero() && (usingIPs == 0 || (usingIPs == 1 && u2oInterconnIP != "")) { newSubnet := subnet.DeepCopy() - newSubnet.Finalizers = util.RemoveString(newSubnet.Finalizers, util.FinalizerName) + newSubnet.Finalizers = util.RemoveString(newSubnet.Finalizers, util.KubeOVNControllerFinalizer) patch, err := util.GenerateMergePatchPayload(subnet, newSubnet) if err != nil { klog.Errorf("failed to generate patch payload for subnet '%s', %v", subnet.Name, err) diff --git a/pkg/controller/vip.go b/pkg/controller/vip.go index 74078be7f04..55d9edbc0e3 100644 --- a/pkg/controller/vip.go +++ b/pkg/controller/vip.go @@ -650,12 +650,12 @@ func (c *Controller) handleAddVipFinalizer(key string) error { return err } if cachedVip.DeletionTimestamp.IsZero() { - if slices.Contains(cachedVip.Finalizers, util.FinalizerName) { + if slices.Contains(cachedVip.Finalizers, util.KubeOVNControllerFinalizer) { return nil } } newVip := cachedVip.DeepCopy() - controllerutil.AddFinalizer(newVip, util.FinalizerName) + controllerutil.AddFinalizer(newVip, util.KubeOVNControllerFinalizer) patch, err := util.GenerateMergePatchPayload(cachedVip, newVip) if err != nil { klog.Errorf("failed to generate patch payload for ovn eip '%s', %v", cachedVip.Name, err) @@ -685,7 +685,7 @@ func (c *Controller) handleDelVipFinalizer(key string) error { return nil } newVip := cachedVip.DeepCopy() - controllerutil.RemoveFinalizer(newVip, util.FinalizerName) + controllerutil.RemoveFinalizer(newVip, util.KubeOVNControllerFinalizer) patch, err := util.GenerateMergePatchPayload(cachedVip, newVip) if err != nil { klog.Errorf("failed to generate patch payload for ovn eip '%s', %v", cachedVip.Name, err) @@ -713,24 +713,18 @@ func (c *Controller) syncVipFinalizer() error { return err } for _, cachedVip := range vips { - if len(cachedVip.Finalizers) == 0 { - continue + patch, err := c.ReplaceFinalizer(cachedVip) + if err != nil { + klog.Errorf("failed to sync finalizer for vip %s, %v", cachedVip.Name, err) + return err } - if slices.Contains(cachedVip.Finalizers, util.DepreciatedFinalizerName) { - newVip := cachedVip.DeepCopy() - controllerutil.RemoveFinalizer(newVip, util.DepreciatedFinalizerName) - controllerutil.AddFinalizer(newVip, util.FinalizerName) - patch, err := util.GenerateMergePatchPayload(cachedVip, newVip) - if err != nil { - klog.Errorf("failed to generate patch payload for vip %s, %v", newVip.Name, err) - return err - } - if _, err := c.config.KubeOvnClient.KubeovnV1().Vips().Patch(context.Background(), newVip.Name, + if patch != nil { + if _, err := c.config.KubeOvnClient.KubeovnV1().Vips().Patch(context.Background(), cachedVip.Name, types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil { if k8serrors.IsNotFound(err) { return nil } - klog.Errorf("failed to sync finalizer for vip %s, %v", newVip.Name, err) + klog.Errorf("failed to sync finalizer for vip %s, %v", cachedVip.Name, err) return err } } diff --git a/pkg/controller/vpc_nat_gw_eip.go b/pkg/controller/vpc_nat_gw_eip.go index f47c55ea665..dd8fbdd936e 100644 --- a/pkg/controller/vpc_nat_gw_eip.go +++ b/pkg/controller/vpc_nat_gw_eip.go @@ -761,24 +761,18 @@ func (c *Controller) syncIptablesEipFinalizer() error { return err } for _, cachedEip := range eips { - if len(cachedEip.Finalizers) == 0 { - continue - } - if slices.Contains(cachedEip.Finalizers, util.DepreciatedFinalizerName) { - newEip := cachedEip.DeepCopy() - controllerutil.RemoveFinalizer(newEip, util.DepreciatedFinalizerName) - controllerutil.AddFinalizer(newEip, util.FinalizerName) - patch, err := util.GenerateMergePatchPayload(cachedEip, newEip) - if err != nil { - klog.Errorf("failed to generate patch payload for eip %s, %v", newEip.Name, err) - return err - } - if _, err := c.config.KubeOvnClient.KubeovnV1().IptablesEIPs().Patch(context.Background(), newEip.Name, + patch, err := c.ReplaceFinalizer(cachedEip) + if err != nil { + klog.Errorf("failed to sync finalizer for eip %s, %v", cachedEip.Name, err) + return err + } + if patch != nil { + if _, err := c.config.KubeOvnClient.KubeovnV1().IptablesEIPs().Patch(context.Background(), cachedEip.Name, types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil { if k8serrors.IsNotFound(err) { return nil } - klog.Errorf("failed to sync finalizer for eip %s, %v", newEip.Name, err) + klog.Errorf("failed to sync finalizer for eip %s, %v", cachedEip.Name, err) return err } } @@ -796,12 +790,12 @@ func (c *Controller) handleAddIptablesEipFinalizer(key string) error { return err } if cachedIptablesEip.DeletionTimestamp.IsZero() { - if slices.Contains(cachedIptablesEip.Finalizers, util.FinalizerName) { + if slices.Contains(cachedIptablesEip.Finalizers, util.KubeOVNControllerFinalizer) { return nil } } newIptablesEip := cachedIptablesEip.DeepCopy() - controllerutil.AddFinalizer(newIptablesEip, util.FinalizerName) + controllerutil.AddFinalizer(newIptablesEip, util.KubeOVNControllerFinalizer) patch, err := util.GenerateMergePatchPayload(cachedIptablesEip, newIptablesEip) if err != nil { klog.Errorf("failed to generate patch payload for iptables eip '%s', %v", cachedIptablesEip.Name, err) @@ -831,7 +825,7 @@ func (c *Controller) handleDelIptablesEipFinalizer(key string) error { return nil } newIptablesEip := cachedIptablesEip.DeepCopy() - controllerutil.RemoveFinalizer(newIptablesEip, util.FinalizerName) + controllerutil.RemoveFinalizer(newIptablesEip, util.KubeOVNControllerFinalizer) patch, err := util.GenerateMergePatchPayload(cachedIptablesEip, newIptablesEip) if err != nil { klog.Errorf("failed to generate patch payload for iptables eip '%s', %v", cachedIptablesEip.Name, err) diff --git a/pkg/controller/vpc_nat_gw_nat.go b/pkg/controller/vpc_nat_gw_nat.go index b5895f725e7..1ac75aded9f 100644 --- a/pkg/controller/vpc_nat_gw_nat.go +++ b/pkg/controller/vpc_nat_gw_nat.go @@ -1055,24 +1055,18 @@ func (c *Controller) syncIptablesFipFinalizer() error { return err } for _, cachedFip := range fips { - if len(cachedFip.Finalizers) == 0 { - continue - } - if slices.Contains(cachedFip.Finalizers, util.DepreciatedFinalizerName) { - newFip := cachedFip.DeepCopy() - controllerutil.RemoveFinalizer(newFip, util.DepreciatedFinalizerName) - controllerutil.AddFinalizer(newFip, util.FinalizerName) - patch, err := util.GenerateMergePatchPayload(cachedFip, newFip) - if err != nil { - klog.Errorf("failed to generate patch payload for fip %s, %v", newFip.Name, err) - return err - } - if _, err := c.config.KubeOvnClient.KubeovnV1().IptablesFIPRules().Patch(context.Background(), newFip.Name, + patch, err := c.ReplaceFinalizer(cachedFip) + if err != nil { + klog.Errorf("failed to sync finalizer for fip %s, %v", cachedFip.Name, err) + return err + } + if patch != nil { + if _, err := c.config.KubeOvnClient.KubeovnV1().IptablesFIPRules().Patch(context.Background(), cachedFip.Name, types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil { if k8serrors.IsNotFound(err) { return nil } - klog.Errorf("failed to sync finalizer for fip %s, %v", newFip.Name, err) + klog.Errorf("failed to sync finalizer for fip %s, %v", cachedFip.Name, err) return err } } @@ -1090,12 +1084,12 @@ func (c *Controller) handleAddIptablesFipFinalizer(key string) error { return err } if cachedIptablesFip.DeletionTimestamp.IsZero() { - if slices.Contains(cachedIptablesFip.Finalizers, util.FinalizerName) { + if slices.Contains(cachedIptablesFip.Finalizers, util.KubeOVNControllerFinalizer) { return nil } } newIptablesFip := cachedIptablesFip.DeepCopy() - controllerutil.AddFinalizer(newIptablesFip, util.FinalizerName) + controllerutil.AddFinalizer(newIptablesFip, util.KubeOVNControllerFinalizer) patch, err := util.GenerateMergePatchPayload(cachedIptablesFip, newIptablesFip) if err != nil { klog.Errorf("failed to generate patch payload for iptables fip '%s', %v", cachedIptablesFip.Name, err) @@ -1125,7 +1119,7 @@ func (c *Controller) handleDelIptablesFipFinalizer(key string) error { return nil } newIptablesFip := cachedIptablesFip.DeepCopy() - controllerutil.RemoveFinalizer(newIptablesFip, util.FinalizerName) + controllerutil.RemoveFinalizer(newIptablesFip, util.KubeOVNControllerFinalizer) patch, err := util.GenerateMergePatchPayload(cachedIptablesFip, newIptablesFip) if err != nil { klog.Errorf("failed to generate patch payload for iptables fip '%s', %v", cachedIptablesFip.Name, err) @@ -1153,24 +1147,18 @@ func (c *Controller) syncIptablesDnatFinalizer() error { return err } for _, cachedDnat := range dnats { - if len(cachedDnat.Finalizers) == 0 { - continue - } - if slices.Contains(cachedDnat.Finalizers, util.DepreciatedFinalizerName) { - newDnat := cachedDnat.DeepCopy() - controllerutil.RemoveFinalizer(newDnat, util.DepreciatedFinalizerName) - controllerutil.AddFinalizer(newDnat, util.FinalizerName) - patch, err := util.GenerateMergePatchPayload(cachedDnat, newDnat) - if err != nil { - klog.Errorf("failed to generate patch payload for dnat %s, %v", newDnat.Name, err) - return err - } - if _, err := c.config.KubeOvnClient.KubeovnV1().IptablesDnatRules().Patch(context.Background(), newDnat.Name, + patch, err := c.ReplaceFinalizer(cachedDnat) + if err != nil { + klog.Errorf("failed to sync finalizer for dnat %s, %v", cachedDnat.Name, err) + return err + } + if patch != nil { + if _, err := c.config.KubeOvnClient.KubeovnV1().IptablesDnatRules().Patch(context.Background(), cachedDnat.Name, types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil { if k8serrors.IsNotFound(err) { return nil } - klog.Errorf("failed to sync finalizer for dnat %s, %v", newDnat.Name, err) + klog.Errorf("failed to sync finalizer for dnat %s, %v", cachedDnat.Name, err) return err } } @@ -1188,12 +1176,12 @@ func (c *Controller) handleAddIptablesDnatFinalizer(key string) error { return err } if cachedIptablesDnat.DeletionTimestamp.IsZero() { - if slices.Contains(cachedIptablesDnat.Finalizers, util.FinalizerName) { + if slices.Contains(cachedIptablesDnat.Finalizers, util.KubeOVNControllerFinalizer) { return nil } } newIptablesDnat := cachedIptablesDnat.DeepCopy() - controllerutil.AddFinalizer(newIptablesDnat, util.FinalizerName) + controllerutil.AddFinalizer(newIptablesDnat, util.KubeOVNControllerFinalizer) patch, err := util.GenerateMergePatchPayload(cachedIptablesDnat, newIptablesDnat) if err != nil { klog.Errorf("failed to generate patch payload for iptables dnat '%s', %v", cachedIptablesDnat.Name, err) @@ -1223,7 +1211,7 @@ func (c *Controller) handleDelIptablesDnatFinalizer(key string) error { return nil } newIptablesDnat := cachedIptablesDnat.DeepCopy() - controllerutil.RemoveFinalizer(newIptablesDnat, util.FinalizerName) + controllerutil.RemoveFinalizer(newIptablesDnat, util.KubeOVNControllerFinalizer) patch, err := util.GenerateMergePatchPayload(cachedIptablesDnat, newIptablesDnat) if err != nil { klog.Errorf("failed to generate patch payload for iptables dnat '%s', %v", cachedIptablesDnat.Name, err) @@ -1302,24 +1290,18 @@ func (c *Controller) syncIptablesSnatFinalizer() error { return err } for _, cachedSnat := range snats { - if len(cachedSnat.Finalizers) == 0 { - continue - } - if slices.Contains(cachedSnat.Finalizers, util.DepreciatedFinalizerName) { - newSnat := cachedSnat.DeepCopy() - controllerutil.RemoveFinalizer(newSnat, util.DepreciatedFinalizerName) - controllerutil.AddFinalizer(newSnat, util.FinalizerName) - patch, err := util.GenerateMergePatchPayload(cachedSnat, newSnat) - if err != nil { - klog.Errorf("failed to generate patch payload for dnat %s, %v", newSnat.Name, err) - return err - } - if _, err := c.config.KubeOvnClient.KubeovnV1().IptablesSnatRules().Patch(context.Background(), newSnat.Name, + patch, err := c.ReplaceFinalizer(cachedSnat) + if err != nil { + klog.Errorf("failed to sync finalizer for snat %s, %v", cachedSnat.Name, err) + return err + } + if patch != nil { + if _, err := c.config.KubeOvnClient.KubeovnV1().IptablesSnatRules().Patch(context.Background(), cachedSnat.Name, types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil { if k8serrors.IsNotFound(err) { return nil } - klog.Errorf("failed to sync finalizer for dnat %s, %v", newSnat.Name, err) + klog.Errorf("failed to sync finalizer for dnat %s, %v", cachedSnat.Name, err) return err } } @@ -1337,12 +1319,12 @@ func (c *Controller) handleAddIptablesSnatFinalizer(key string) error { return err } if cachedIptablesSnat.DeletionTimestamp.IsZero() { - if slices.Contains(cachedIptablesSnat.Finalizers, util.FinalizerName) { + if slices.Contains(cachedIptablesSnat.Finalizers, util.KubeOVNControllerFinalizer) { return nil } } newIptablesSnat := cachedIptablesSnat.DeepCopy() - controllerutil.AddFinalizer(newIptablesSnat, util.FinalizerName) + controllerutil.AddFinalizer(newIptablesSnat, util.KubeOVNControllerFinalizer) patch, err := util.GenerateMergePatchPayload(cachedIptablesSnat, newIptablesSnat) if err != nil { klog.Errorf("failed to generate patch payload for iptables snat '%s', %v", cachedIptablesSnat.Name, err) @@ -1372,7 +1354,7 @@ func (c *Controller) handleDelIptablesSnatFinalizer(key string) error { return nil } newIptablesSnat := cachedIptablesSnat.DeepCopy() - controllerutil.RemoveFinalizer(newIptablesSnat, util.FinalizerName) + controllerutil.RemoveFinalizer(newIptablesSnat, util.KubeOVNControllerFinalizer) patch, err := util.GenerateMergePatchPayload(cachedIptablesSnat, newIptablesSnat) if err != nil { klog.Errorf("failed to generate patch payload for iptables snat '%s', %v", cachedIptablesSnat.Name, err) diff --git a/pkg/util/const.go b/pkg/util/const.go index e68dd8173a0..6a391d89f4f 100644 --- a/pkg/util/const.go +++ b/pkg/util/const.go @@ -3,8 +3,8 @@ package util const ( CniTypeName = "kube-ovn" - DepreciatedFinalizerName = "kube-ovn-controller" - KubeOVNControllerFinalizer = "kubeovn.io/kube-ovn-controller" + DepreciatedFinalizerName = "kube-ovn-controller" + KubeOVNControllerFinalizer = "kubeovn.io/kube-ovn-controller" AllocatedAnnotation = "ovn.kubernetes.io/allocated" RoutedAnnotation = "ovn.kubernetes.io/routed" diff --git a/test/e2e/kube-ovn/subnet/subnet.go b/test/e2e/kube-ovn/subnet/subnet.go index 395de008ea4..8c784c3ba88 100644 --- a/test/e2e/kube-ovn/subnet/subnet.go +++ b/test/e2e/kube-ovn/subnet/subnet.go @@ -148,7 +148,7 @@ var _ = framework.Describe("[group:subnet]", func() { subnet = subnetClient.CreateSync(subnet) ginkgo.By("Validating subnet finalizers") - framework.ExpectContainElement(subnet.Finalizers, util.FinalizerName) + framework.ExpectContainElement(subnet.Finalizers, util.KubeOVNControllerFinalizer) ginkgo.By("Validating subnet spec fields") framework.ExpectFalse(subnet.Spec.Default) @@ -206,7 +206,7 @@ var _ = framework.Describe("[group:subnet]", func() { subnet = subnetClient.CreateSync(subnet) ginkgo.By("Validating subnet finalizers") - framework.ExpectContainElement(subnet.ObjectMeta.Finalizers, util.FinalizerName) + framework.ExpectContainElement(subnet.ObjectMeta.Finalizers, util.KubeOVNControllerFinalizer) ginkgo.By("Validating subnet spec fields") framework.ExpectFalse(subnet.Spec.Default) @@ -251,7 +251,7 @@ var _ = framework.Describe("[group:subnet]", func() { subnet = subnetClient.CreateSync(subnet) ginkgo.By("Validating subnet finalizers") - framework.ExpectContainElement(subnet.ObjectMeta.Finalizers, util.FinalizerName) + framework.ExpectContainElement(subnet.ObjectMeta.Finalizers, util.KubeOVNControllerFinalizer) ginkgo.By("Validating subnet spec fields") framework.ExpectFalse(subnet.Spec.Default) @@ -301,7 +301,7 @@ var _ = framework.Describe("[group:subnet]", func() { subnet = subnetClient.CreateSync(subnet) ginkgo.By("Validating subnet finalizers") - framework.ExpectContainElement(subnet.Finalizers, util.FinalizerName) + framework.ExpectContainElement(subnet.Finalizers, util.KubeOVNControllerFinalizer) ginkgo.By("Validating subnet spec fields") framework.ExpectFalse(subnet.Spec.Default) @@ -345,7 +345,7 @@ var _ = framework.Describe("[group:subnet]", func() { subnet = subnetClient.CreateSync(subnet) ginkgo.By("Validating subnet finalizers") - framework.ExpectContainElement(subnet.Finalizers, util.FinalizerName) + framework.ExpectContainElement(subnet.Finalizers, util.KubeOVNControllerFinalizer) ginkgo.By("Validating subnet spec fields") framework.ExpectFalse(subnet.Spec.Default) @@ -388,7 +388,7 @@ var _ = framework.Describe("[group:subnet]", func() { subnet = subnetClient.PatchSync(subnet, modifiedSubnet) ginkgo.By("Validating subnet finalizers") - framework.ExpectContainElement(subnet.ObjectMeta.Finalizers, util.FinalizerName) + framework.ExpectContainElement(subnet.ObjectMeta.Finalizers, util.KubeOVNControllerFinalizer) ginkgo.By("Validating subnet spec fields") framework.ExpectFalse(subnet.Spec.Default) @@ -444,7 +444,7 @@ var _ = framework.Describe("[group:subnet]", func() { subnet = subnetClient.CreateSync(subnet) ginkgo.By("Validating subnet finalizers") - framework.ExpectContainElement(subnet.Finalizers, util.FinalizerName) + framework.ExpectContainElement(subnet.Finalizers, util.KubeOVNControllerFinalizer) ginkgo.By("Validating centralized subnet with active-standby mode") framework.ExpectFalse(subnet.Spec.EnableEcmp) @@ -947,7 +947,7 @@ var _ = framework.Describe("[group:subnet]", func() { subnet = subnetClient.CreateSync(subnet) ginkgo.By("Validating subnet load-balancer records exist") - framework.ExpectContainElement(subnet.Finalizers, util.FinalizerName) + framework.ExpectContainElement(subnet.Finalizers, util.KubeOVNControllerFinalizer) execCmd := "kubectl ko nbctl --format=csv --data=bare --no-heading --columns=load_balancer find logical-switch " + fmt.Sprintf("name=%s", subnetName) output, err := exec.Command("bash", "-c", execCmd).CombinedOutput() framework.ExpectNoError(err) From 45de7924fc6d0ede3c4be28ce305cae6621e9ebc Mon Sep 17 00:00:00 2001 From: bobz965 Date: Wed, 21 Feb 2024 17:06:22 +0800 Subject: [PATCH 6/6] optimize Signed-off-by: bobz965 --- pkg/controller/init.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/init.go b/pkg/controller/init.go index 6cc2f6d49e5..d943e15047e 100644 --- a/pkg/controller/init.go +++ b/pkg/controller/init.go @@ -886,7 +886,7 @@ func (c *Controller) syncFinalizers() error { } func (c *Controller) ReplaceFinalizer(cachedObj client.Object) ([]byte, error) { - if slices.Contains(cachedObj.GetFinalizers(), util.DepreciatedFinalizerName) { + if controllerutil.ContainsFinalizer(cachedObj, util.DepreciatedFinalizerName) { newObj := cachedObj.DeepCopyObject().(client.Object) controllerutil.RemoveFinalizer(newObj, util.DepreciatedFinalizerName) controllerutil.AddFinalizer(newObj, util.KubeOVNControllerFinalizer)