Ovn nat 1 (#3095)

* add ovn nat gw webhook

* simplify ovn eip fip finalizer

* add shared eip e2e

* fix name

* auto managed external routes

* add normal route and bfd route  test e2e

* vpc enable auto manage normal static routes

charts/templates/kube-ovn-crd.yaml

@@ -868,6 +868,9 @@ spec:
       - jsonPath: .status.type
         name: Type
         type: string
+      - jsonPath: .status.nat
+        name: Nat
+        type: string
       - jsonPath: .status.ready
         name: Ready
         type: boolean
@@ -880,6 +883,8 @@ spec:
               properties:
                 type:
                   type: string
+                nat:
+                  type: string
                 ready:
                   type: boolean
                 v4Ip:

dist/images/install.sh

@@ -1407,6 +1407,9 @@ spec:
       - jsonPath: .status.type
         name: Type
         type: string
+      - jsonPath: .status.nat
+        name: Nat
+        type: string
       - jsonPath: .status.ready
         name: Ready
         type: boolean
@@ -1419,6 +1422,8 @@ spec:
               properties:
                 type:
                   type: string
+                nat:
+                  type: string
                 ready:
                   type: boolean
                 v4Ip:

pkg/apis/kubeovn/v1/types.go

@@ -949,7 +949,10 @@ type OvnEipSpec struct {
 	V6Ip           string `json:"v6Ip"`
 	MacAddress     string `json:"macAddress"`
 	Type           string `json:"type"`
-	// usage type: fip, snat, lrp, node external gw
+	// usage type: eip, lrp, node external gw
+	// eip: only used by nat, fip, snat, dnat, all the nat type will record int the eip status
+	// lrp: logical router port
+	// node external gw: is lsp, in the case of bfd session between lrp and lsp, the lsp is on the node as external gateway
 }
 
 // OvnEipCondition describes the state of an object at a certain point.
@@ -964,6 +967,7 @@ type OvnEipStatus struct {
 	Conditions []OvnEipCondition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"`
 
 	Type       string `json:"type" patchStrategy:"merge"`
+	Nat        string `json:"nat" patchStrategy:"merge"`
 	Ready      bool   `json:"ready" patchStrategy:"merge"`
 	V4Ip       string `json:"v4Ip" patchStrategy:"merge"`
 	V6Ip       string `json:"v6Ip" patchStrategy:"merge"`

pkg/controller/external-gw.go

@@ -293,11 +293,6 @@ func (c *Controller) updateDefaultVpcExternal(enableExternal bool) error {
 			klog.Errorf("failed to patch vpc %s, %v", c.config.ClusterRouter, err)
 			return err
 		}
-		lrpEipName := fmt.Sprintf("%s-%s", c.config.ClusterRouter, c.config.ExternalGatewaySwitch)
-		if err := c.patchLrpOvnEipEnableBfdLabel(lrpEipName, vpc.Spec.EnableBfd); err != nil {
-			klog.Errorf("failed to patch label for lrp %s, %v", lrpEipName, err)
-			return err
-		}
 	}
 	return nil
 }

pkg/controller/node.go

@@ -1046,7 +1046,7 @@ func (c *Controller) addNodeGwStaticRoute() error {
 			if util.CheckProtocol(cidrBlock) != util.CheckProtocol(nextHop) {
 				continue
 			}
-			if err := c.ovnClient.AddLogicalRouterStaticRoute(c.config.ClusterRouter, util.MainRouteTable, ovnnb.LogicalRouterStaticRoutePolicyDstIP, cidrBlock, nextHop); err != nil {
+			if err := c.ovnClient.AddLogicalRouterStaticRoute(c.config.ClusterRouter, util.MainRouteTable, ovnnb.LogicalRouterStaticRoutePolicyDstIP, cidrBlock, nil, nextHop); err != nil {
 				klog.Errorf("failed to add static route for node gw: %v", err)
 				return err
 			}

pkg/controller/ovn_dnat.go

@@ -9,11 +9,11 @@ import (
 	"github.com/ovn-org/libovsdb/ovsdb"
 	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"
 	"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/util"
@@ -41,14 +41,9 @@ func (c *Controller) enqueueUpdateOvnDnatRule(old, new interface{}) {
 	oldDnat := old.(*kubeovnv1.OvnDnatRule)
 	newDnat := new.(*kubeovnv1.OvnDnatRule)
 	if !newDnat.DeletionTimestamp.IsZero() {
-		if len(newDnat.Finalizers) == 0 {
-			// avoid delete dnat twice
-			return
-		} else {
-			klog.V(3).Infof("enqueue del ovn dnat %s", key)
-			c.delOvnDnatRuleQueue.Add(key)
-			return
-		}
+		klog.Infof("enqueue del ovn dnat %s", key)
+		c.delOvnDnatRuleQueue.Add(key)
+		return
 	}
 
 	if oldDnat.Spec.OvnEip != newDnat.Spec.OvnEip {
@@ -60,7 +55,7 @@ func (c *Controller) enqueueUpdateOvnDnatRule(old, new interface{}) {
 		oldDnat.Spec.IpName != newDnat.Spec.IpName ||
 		oldDnat.Spec.InternalPort != newDnat.Spec.InternalPort ||
 		oldDnat.Spec.ExternalPort != newDnat.Spec.ExternalPort {
-		klog.V(3).Infof("enqueue update dnat %s", key)
+		klog.Infof("enqueue update dnat %s", key)
 		c.updateOvnDnatRuleQueue.Add(key)
 	}
 }
@@ -181,6 +176,27 @@ func (c *Controller) processNextDeleteOvnDnatRuleWorkItem() bool {
 	return true
 }
 
+func (c *Controller) isOvnDnatDuplicated(eipName, dnatName, externalPort string) (bool, error) {
+	// check if eip:external port already used
+	dnats, err := c.ovnDnatRulesLister.List(labels.SelectorFromSet(labels.Set{
+		util.VpcDnatEPortLabel: externalPort,
+	}))
+	if err != nil {
+		if !k8serrors.IsNotFound(err) {
+			return false, err
+		}
+	}
+	if len(dnats) != 0 {
+		for _, d := range dnats {
+			if d.Name != dnatName && d.Spec.OvnEip == eipName {
+				err = fmt.Errorf("failed to create dnat %s, duplicate, same eip %s, same external port '%s' is using by dnat %s", dnatName, eipName, externalPort, d.Name)
+				return true, err
+			}
+		}
+	}
+	return false, nil
+}
+
 func (c *Controller) handleAddOvnDnatRule(key string) error {
 	cachedDnat, err := c.ovnDnatRulesLister.Get(key)
 	if err != nil {
@@ -230,6 +246,16 @@ func (c *Controller) handleAddOvnDnatRule(key string) error {
 		return err
 	}
 
+	if cachedEip.Status.Type != "" && cachedEip.Status.Type != util.NatUsingEip {
+		err = fmt.Errorf("ovn eip %s type is not %s, can not use", cachedEip.Name, util.NatUsingEip)
+		return err
+	}
+
+	if _, err := c.isOvnDnatDuplicated(eipName, key, cachedDnat.Spec.ExternalPort); err != nil {
+		klog.Error("failed to create dnat %s, %v", cachedDnat.Name, err)
+		return err
+	}
+
 	subnet, err := c.subnetsLister.Get(subnetName)
 	if err != nil {
 		klog.Errorf("failed to get vpc subnet %s, %v", subnetName, err)
@@ -243,27 +269,17 @@ func (c *Controller) handleAddOvnDnatRule(key string) error {
 	}
 
 	vpcName := subnet.Spec.Vpc
-	if cachedEip.Status.Type != "" && cachedEip.Status.Type != util.DnatUsingEip {
-		err = fmt.Errorf("failed to create ovn dnat %s, eip '%s' is using by %s", key, eipName, cachedEip.Spec.Type)
-		return err
-	}
-
 	if err = c.patchOvnDnatStatus(key, vpcName, cachedEip.Status.V4Ip,
 		internalV4Ip, mac, false); err != nil {
 		klog.Errorf("failed to patch status for dnat %s, %v", key, err)
 		return err
 	}
 
-	if err = c.handleAddOvnEipFinalizer(cachedEip, util.OvnDnatUseEipFinalizer); err != nil {
+	if err = c.handleAddOvnEipFinalizer(cachedEip, util.OvnEipFinalizer); err != nil {
 		klog.Errorf("failed to add finalizer for ovn eip, %v", err)
 		return err
 	}
 
-	if err = c.handleAddOvnDnatFinalizer(cachedDnat); err != nil {
-		klog.Errorf("failed to handle finalizer for ovn dnat, %v", err)
-		return err
-	}
-
 	if err = c.AddDnatRule(vpcName, cachedDnat.Name, cachedEip.Status.V4Ip, internalV4Ip,
 		cachedDnat.Spec.ExternalPort, cachedDnat.Spec.InternalPort, cachedDnat.Spec.Protocol); err != nil {
 		klog.Errorf("failed to create v4 dnat, %v", err)
@@ -287,7 +303,7 @@ func (c *Controller) handleAddOvnDnatRule(key string) error {
 		return err
 	}
 
-	if err = c.patchOvnEipNat(eipName, util.DnatUsingEip); err != nil {
+	if err = c.patchOvnEipStatus(eipName, true); err != nil {
 		klog.Errorf("failed to patch status for eip %s, %v", key, err)
 		return err
 	}
@@ -307,33 +323,19 @@ func (c *Controller) handleDelOvnDnatRule(key string) error {
 
 	eipName := cachedDnat.Spec.OvnEip
 	if len(eipName) == 0 {
-		klog.Errorf("failed to delete ovn dnat, should set eip")
-	}
-
-	cachedEip, err := c.GetOvnEip(eipName)
-	if err != nil {
-		klog.Errorf("failed to get eip, %v", err)
+		err := fmt.Errorf("failed to create fip rule, should set eip")
+		klog.Error(err)
 		return err
 	}
+
 	if cachedDnat.Status.Vpc != "" && cachedDnat.Status.V4Eip != "" && cachedDnat.Status.ExternalPort != "" {
 		if err = c.DelDnatRule(cachedDnat.Status.Vpc, cachedDnat.Name,
 			cachedDnat.Status.V4Eip, cachedDnat.Status.ExternalPort); err != nil {
 			klog.Errorf("failed to delete dnat, %v", err)
 			return err
 		}
 	}
-
-	if err = c.handleDelOvnEipFinalizer(cachedEip, util.OvnDnatUseEipFinalizer); err != nil {
-		klog.Errorf("failed to handle remove finalizer from ovn eip, %v", err)
-		return err
-	}
 	c.resetOvnEipQueue.Add(cachedDnat.Spec.OvnEip)
-
-	if err = c.handleDelOvnDnatFinalizer(cachedDnat); err != nil {
-		klog.Errorf("failed to handle remove finalizer from ovn dnat, %v", err)
-		return err
-	}
-
 	return nil
 }
 
@@ -381,6 +383,16 @@ func (c *Controller) handleUpdateOvnDnatRule(key string) error {
 		return err
 	}
 
+	if cachedEip.Status.Type != "" && cachedEip.Status.Type != util.NatUsingEip {
+		err = fmt.Errorf("ovn eip %s type is not %s, can not use", cachedEip.Name, util.NatUsingEip)
+		return err
+	}
+
+	if _, err := c.isOvnDnatDuplicated(eipName, key, cachedDnat.Spec.ExternalPort); err != nil {
+		klog.Error("failed to update dnat %s, %v", cachedDnat.Name, err)
+		return err
+	}
+
 	subnet, err := c.subnetsLister.Get(subnetName)
 	if err != nil {
 		klog.Errorf("failed to get vpc subnet %s, %v", subnetName, err)
@@ -440,31 +452,6 @@ func (c *Controller) handleUpdateOvnDnatRule(key string) error {
 	return nil
 }
 
-func (c *Controller) handleAddOvnDnatFinalizer(cachedDnat *kubeovnv1.OvnDnatRule) error {
-	if cachedDnat.DeletionTimestamp.IsZero() {
-		if util.ContainsString(cachedDnat.Finalizers, util.ControllerName) {
-			return nil
-		}
-	}
-
-	newDnat := cachedDnat.DeepCopy()
-	controllerutil.AddFinalizer(newDnat, util.ControllerName)
-	patch, err := util.GenerateMergePatchPayload(cachedDnat, newDnat)
-	if err != nil {
-		return err
-	}
-
-	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 add finalizer for ovn dnat '%s', %v", cachedDnat.Name, err)
-		return err
-	}
-	return nil
-}
-
 func (c *Controller) patchOvnDnatAnnotations(key, eipName string) error {
 	oriDnat, err := c.ovnDnatRulesLister.Get(key)
 	if err != nil {
@@ -510,8 +497,31 @@ func (c *Controller) patchOvnDnatStatus(key, vpcName, v4Eip, podIp, podMac strin
 		}
 		return err
 	}
-
 	dnat := oriDnat.DeepCopy()
+	needUpdateLabel := false
+	var op string
+	if len(dnat.Labels) == 0 {
+		op = "add"
+		needUpdateLabel = true
+		dnat.Labels = map[string]string{
+			util.EipV4IpLabel: v4Eip,
+		}
+	} else if dnat.Labels[util.EipV4IpLabel] != v4Eip {
+		op = "replace"
+		needUpdateLabel = true
+		dnat.Labels[util.EipV4IpLabel] = v4Eip
+	}
+	if needUpdateLabel {
+		patchPayloadTemplate := `[{ "op": "%s", "path": "/metadata/labels", "value": %s }]`
+		raw, _ := json.Marshal(dnat.Labels)
+		patchPayload := fmt.Sprintf(patchPayloadTemplate, op, raw)
+		if _, err := c.config.KubeOvnClient.KubeovnV1().OvnDnatRules().Patch(context.Background(), dnat.Name,
+			types.JSONPatchType, []byte(patchPayload), metav1.PatchOptions{}); err != nil {
+			klog.Errorf("failed to patch label for ovn dnat %s, %v", dnat.Name, err)
+			return err
+		}
+	}
+
 	var changed bool
 	if dnat.Status.Ready != ready {
 		dnat.Status.Ready = ready
@@ -563,29 +573,6 @@ func (c *Controller) patchOvnDnatStatus(key, vpcName, v4Eip, podIp, podMac strin
 	return nil
 }
 
-func (c *Controller) handleDelOvnDnatFinalizer(cachedDnat *kubeovnv1.OvnDnatRule) error {
-	if len(cachedDnat.Finalizers) == 0 {
-		return nil
-	}
-
-	newDnat := cachedDnat.DeepCopy()
-	controllerutil.RemoveFinalizer(newDnat, util.ControllerName)
-	patch, err := util.GenerateMergePatchPayload(cachedDnat, newDnat)
-	if err != nil {
-		return err
-	}
-
-	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 remove finalizer from ovn dnat '%s', %v", cachedDnat.Name, err)
-		return err
-	}
-	return nil
-}
-
 func (c *Controller) AddDnatRule(vpcName, dnatName, externalIp, internalIp, externalPort, internalPort, protocol string) error {
 	externalEndpoint := net.JoinHostPort(externalIp, externalPort)
 	internalEndpoint := net.JoinHostPort(internalIp, internalPort)