failed to delete ovn-fip or ovn-snat (#2534)

* fix ovn-eip delete

* modify

* modify

---------

Co-authored-by: yl4811 <yl4811@yealink.com>

pkg/controller/ovn_eip.go

@@ -3,6 +3,7 @@ package controller
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"strconv"
 	"time"
@@ -262,7 +263,7 @@ func (c *Controller) handleAddOvnEip(key string) error {
 		klog.Errorf("failed to count ovn eip '%s' in subnet, %v", cachedEip.Name, err)
 		return err
 	}
-	if err = c.handleAddOvnEipFinalizer(cachedEip); err != nil {
+	if err = c.handleAddOvnEipFinalizer(cachedEip, util.ControllerName); err != nil {
 		klog.Errorf("failed to add finalizer for ovn eip, %v", err)
 		return err
 	}
@@ -336,6 +337,13 @@ func (c *Controller) handleDelOvnEip(key string) error {
 		}
 		return err
 	}
+
+	if len(cachedEip.Finalizers) > 1 {
+		err := errors.New("eip is referenced, it cannot be deleted directly")
+		klog.Errorf("failed to delete eip %s, %v", key, err)
+		return err
+	}
+
 	if cachedEip.Spec.Type == util.NodeExtGwUsingEip {
 		if err := c.ovnClient.DeleteLogicalSwitchPort(cachedEip.Name); err != nil {
 			klog.Errorf("failed to delete lsp %s, %v", cachedEip.Name, err)
@@ -350,7 +358,7 @@ func (c *Controller) handleDelOvnEip(key string) error {
 		}
 	}
 
-	if err = c.handleDelOvnEipFinalizer(cachedEip); err != nil {
+	if err = c.handleDelOvnEipFinalizer(cachedEip, util.ControllerName); err != nil {
 		klog.Errorf("failed to remove finalizer from ovn eip, %v", err)
 		return err
 	}
@@ -598,14 +606,14 @@ func (c *Controller) natLabelAndAnnoOvnEip(eipName, natName, vpcName string) err
 	return err
 }
 
-func (c *Controller) handleAddOvnEipFinalizer(cachedEip *kubeovnv1.OvnEip) error {
+func (c *Controller) handleAddOvnEipFinalizer(cachedEip *kubeovnv1.OvnEip, finalizer string) error {
 	if cachedEip.DeletionTimestamp.IsZero() {
-		if util.ContainsString(cachedEip.Finalizers, util.ControllerName) {
+		if util.ContainsString(cachedEip.Finalizers, finalizer) {
 			return nil
 		}
 	}
 	newEip := cachedEip.DeepCopy()
-	controllerutil.AddFinalizer(newEip, util.ControllerName)
+	controllerutil.AddFinalizer(newEip, finalizer)
 	patch, err := util.GenerateMergePatchPayload(cachedEip, newEip)
 	if err != nil {
 		klog.Errorf("failed to generate patch payload for ovn eip '%s', %v", cachedEip.Name, err)
@@ -622,12 +630,12 @@ func (c *Controller) handleAddOvnEipFinalizer(cachedEip *kubeovnv1.OvnEip) error
 	return nil
 }
 
-func (c *Controller) handleDelOvnEipFinalizer(cachedEip *kubeovnv1.OvnEip) error {
+func (c *Controller) handleDelOvnEipFinalizer(cachedEip *kubeovnv1.OvnEip, finalizer string) error {
 	if len(cachedEip.Finalizers) == 0 {
 		return nil
 	}
 	newEip := cachedEip.DeepCopy()
-	controllerutil.RemoveFinalizer(newEip, util.ControllerName)
+	controllerutil.RemoveFinalizer(newEip, finalizer)
 	patch, err := util.GenerateMergePatchPayload(cachedEip, newEip)
 	if err != nil {
 		klog.Errorf("failed to generate patch payload for ovn eip '%s', %v", cachedEip.Name, err)

pkg/controller/ovn_fip.go

@@ -236,7 +236,7 @@ func (c *Controller) handleAddOvnFip(key string) error {
 		err = fmt.Errorf("failed to create fip %s, eip '%s' is using by other fip %s", key, eipName, cachedEip.Annotations[util.VpcNatAnnotation])
 		return err
 	}
-	if err = c.handleAddOvnEipFinalizer(cachedEip); err != nil {
+	if err = c.handleAddOvnEipFinalizer(cachedEip, util.OvnFipUseEipFinalizer); err != nil {
 		klog.Errorf("failed to add finalizer for ovn eip, %v", err)
 		return err
 	}
@@ -394,7 +394,7 @@ func (c *Controller) handleDelOvnFip(key string) error {
 	}
 	//  reset eip
 	c.resetOvnEipQueue.Add(cachedFip.Spec.OvnEip)
-	if err = c.handleDelOvnEipFinalizer(cachedEip); err != nil {
+	if err = c.handleDelOvnEipFinalizer(cachedEip, util.OvnFipUseEipFinalizer); err != nil {
 		klog.Errorf("failed to handle remove finalizer from ovn eip, %v", err)
 		return err
 	}

pkg/controller/ovn_snat.go

@@ -229,7 +229,7 @@ func (c *Controller) handleAddOvnSnatRule(key string) error {
 		klog.Errorf("failed to add finalizer for ovn snat, %v", err)
 		return err
 	}
-	if err = c.handleAddOvnEipFinalizer(cachedEip); err != nil {
+	if err = c.handleAddOvnEipFinalizer(cachedEip, util.OvnSnatUseEipFinalizer); err != nil {
 		klog.Errorf("failed to add finalizer for ovn eip, %v", err)
 		return err
 	}
@@ -285,7 +285,7 @@ func (c *Controller) handleUpdateOvnSnatRule(key string) error {
 			klog.Errorf("failed to handle finalizer for snat %s, %v", key, err)
 			return err
 		}
-		if err = c.handleDelOvnEipFinalizer(cachedEip); err != nil {
+		if err = c.handleDelOvnEipFinalizer(cachedEip, util.OvnSnatUseEipFinalizer); err != nil {
 			klog.Errorf("failed to handle finalizer for eip %s, %v", key, err)
 			return err
 		}

pkg/controller/vpc.go

@@ -837,7 +837,7 @@ func (c *Controller) handleAddVpcExternal(key string) error {
 	if err != nil {
 		return err
 	}
-	if err = c.handleAddOvnEipFinalizer(cachedEip); err != nil {
+	if err = c.handleAddOvnEipFinalizer(cachedEip, util.OvnLrpUseEipFinalizer); err != nil {
 		klog.Errorf("failed to add finalizer for ovn eip, %v", err)
 		return err
 	}
@@ -915,7 +915,7 @@ func (c *Controller) handleDelVpcExternal(key string) error {
 		}
 		return err
 	}
-	if err = c.handleDelOvnEipFinalizer(cachedEip); err != nil {
+	if err = c.handleDelOvnEipFinalizer(cachedEip, util.OvnLrpUseEipFinalizer); err != nil {
 		klog.Errorf("failed to del finalizer for ovn eip, %v", err)
 		return err
 	}

pkg/util/const.go

@@ -23,6 +23,10 @@ const (
 	VipAnnotation        = "ovn.kubernetes.io/vip"
 	ChassisAnnotation    = "ovn.kubernetes.io/chassis"
 
+	OvnFipUseEipFinalizer  = "ovn.kubernetes.io/ovn_fip"
+	OvnSnatUseEipFinalizer = "ovn.kubernetes.io/ovn_snat"
+	OvnLrpUseEipFinalizer  = "ovn.kubernetes.io/ovn_lrp"
+
 	ExternalIpAnnotation         = "ovn.kubernetes.io/external_ip"
 	ExternalMacAnnotation        = "ovn.kubernetes.io/external_mac"
 	ExternalCidrAnnotation       = "ovn.kubernetes.io/external_cidr"