Skip to content

Commit

Permalink
fix ovn nat not clean (#3139)
Browse files Browse the repository at this point in the history 
* fix ovn nat not clean
  • Loading branch information
@bobz965
bobz965 committed Aug 16, 2023
1 parent f225c66 commit 515bdb7
Show file tree
Hide file tree
Showing 7 changed files with 284 additions and 108 deletions.
101 changes: 77 additions & 24 deletions pkg/controller/ovn_dnat.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,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/util"
Expand All @@ -26,7 +27,7 @@ func (c *Controller) enqueueAddOvnDnatRule(obj interface{}) {
utilruntime.HandleError(err)
return
}
klog.V(3).Infof("enqueue add ovn dnat %s", key)
klog.Infof("enqueue add ovn dnat %s", key)
c.addOvnDnatRuleQueue.Add(key)
}

Expand All @@ -37,15 +38,18 @@ func (c *Controller) enqueueUpdateOvnDnatRule(old, new interface{}) {
utilruntime.HandleError(err)
return
}

oldDnat := old.(*kubeovnv1.OvnDnatRule)
newDnat := new.(*kubeovnv1.OvnDnatRule)
if !newDnat.DeletionTimestamp.IsZero() {
klog.Infof("enqueue del ovn dnat %s", key)
c.delOvnDnatRuleQueue.Add(key)
return
if len(newDnat.Finalizers) == 0 {
// avoid delete twice
return
} else {
klog.Infof("enqueue del ovn dnat %s", key)
c.delOvnDnatRuleQueue.Add(key)
return
}
}

oldDnat := old.(*kubeovnv1.OvnDnatRule)
if oldDnat.Spec.OvnEip != newDnat.Spec.OvnEip {
c.resetOvnEipQueue.Add(oldDnat.Spec.OvnEip)
}
Expand All @@ -67,7 +71,7 @@ func (c *Controller) enqueueDelOvnDnatRule(obj interface{}) {
utilruntime.HandleError(err)
return
}
klog.V(3).Infof("enqueue delete ovn dnat %s", key)
klog.Infof("enqueue delete ovn dnat %s", key)
c.delOvnDnatRuleQueue.Add(key)
}

Expand Down Expand Up @@ -212,8 +216,7 @@ func (c *Controller) handleAddOvnDnatRule(key string) error {
// already ok
return nil
}
klog.V(3).Infof("handle add dnat %s", key)

klog.Infof("handle add dnat %s", key)
var internalV4Ip, mac, subnetName string
if cachedDnat.Spec.IpType == util.Vip {
internalVip, err := c.virtualIpsLister.Get(cachedDnat.Spec.IpName)
Expand Down Expand Up @@ -279,7 +282,7 @@ func (c *Controller) handleAddOvnDnatRule(key string) error {
return err
}

if err = c.handleAddOvnEipFinalizer(cachedEip, util.OvnEipFinalizer); err != nil {
if err = c.handleAddOvnEipFinalizer(cachedEip, util.ControllerName); err != nil {
klog.Errorf("failed to add finalizer for ovn eip, %v", err)
return err
}
Expand All @@ -290,6 +293,11 @@ func (c *Controller) handleAddOvnDnatRule(key string) error {
return err
}

if err := c.handleAddOvnDnatFinalizer(cachedDnat, util.ControllerName); err != nil {
klog.Errorf("failed to add finalizer for ovn dnat %s, %v", cachedDnat.Name, err)
return err
}

// patch dnat eip relationship
if err = c.natLabelAndAnnoOvnEip(eipName, cachedDnat.Name, vpcName); err != nil {
klog.Errorf("failed to label dnat '%s' in eip %s, %v", cachedDnat.Name, eipName, err)
Expand All @@ -316,7 +324,7 @@ func (c *Controller) handleAddOvnDnatRule(key string) error {
}

func (c *Controller) handleDelOvnDnatRule(key string) error {
klog.V(3).Infof("handle del ovn dnat %s", key)
klog.Infof("handle delete ovn dnat %s", key)
cachedDnat, err := c.ovnDnatRulesLister.Get(key)
if err != nil {
if k8serrors.IsNotFound(err) {
Expand All @@ -325,22 +333,20 @@ func (c *Controller) handleDelOvnDnatRule(key string) error {
klog.Error(err)
return err
}

eipName := cachedDnat.Spec.OvnEip
if len(eipName) == 0 {
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)
klog.Errorf("failed to delete dnat %s, %v", key, err)
return err
}
}
c.resetOvnEipQueue.Add(cachedDnat.Spec.OvnEip)
if err = c.handleDelOvnDnatFinalizer(cachedDnat, util.ControllerName); err != nil {
klog.Errorf("failed to remove finalizer for ovn dnat %s, %v", cachedDnat.Name, err)
return err
}
if cachedDnat.Spec.OvnEip != "" {
c.resetOvnEipQueue.Add(cachedDnat.Spec.OvnEip)
}
return nil
}

Expand All @@ -354,7 +360,7 @@ func (c *Controller) handleUpdateOvnDnatRule(key string) error {
return err
}

klog.V(3).Infof("handle update dnat %s", key)
klog.Infof("handle update dnat %s", key)
var internalV4Ip, mac, subnetName string
if cachedDnat.Spec.IpType == util.Vip {
internalVip, err := c.virtualIpsLister.Get(cachedDnat.Spec.IpName)
Expand Down Expand Up @@ -421,7 +427,7 @@ func (c *Controller) handleUpdateOvnDnatRule(key string) error {

dnat := cachedDnat.DeepCopy()
if dnat.Status.Ready {
klog.V(3).Infof("dnat change ip, old ip '%s', new ip %s", dnat.Status.V4Ip, cachedEip.Status.V4Ip)
klog.Infof("dnat change ip, old ip '%s', new ip %s", dnat.Status.V4Ip, cachedEip.Status.V4Ip)
if err = c.DelDnatRule(dnat.Status.Vpc, dnat.Name, dnat.Status.V4Eip, dnat.Status.ExternalPort); err != nil {
klog.Errorf("failed to delete dnat, %v", err)
return err
Expand Down Expand Up @@ -612,3 +618,50 @@ func (c *Controller) DelDnatRule(vpcName, dnatName, externalIp, externalPort str

return nil
}

func (c *Controller) handleAddOvnDnatFinalizer(cachedDnat *kubeovnv1.OvnDnatRule, finalizer string) error {
if cachedDnat.DeletionTimestamp.IsZero() {
if util.ContainsString(cachedDnat.Finalizers, finalizer) {
return nil
}
}
newDnat := cachedDnat.DeepCopy()
controllerutil.AddFinalizer(newDnat, finalizer)
patch, err := util.GenerateMergePatchPayload(cachedDnat, newDnat)
if err != nil {
klog.Errorf("failed to generate patch payload for ovn dnat '%s', %v", cachedDnat.Name, err)
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) handleDelOvnDnatFinalizer(cachedDnat *kubeovnv1.OvnDnatRule, finalizer string) error {
if len(cachedDnat.Finalizers) == 0 {
return nil
}
var err error
newDnat := cachedDnat.DeepCopy()
controllerutil.RemoveFinalizer(newDnat, finalizer)
patch, err := util.GenerateMergePatchPayload(cachedDnat, newDnat)
if err != nil {
klog.Errorf("failed to generate patch payload for ovn dnat '%s', %v", cachedDnat.Name, err)
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
}
40 changes: 20 additions & 20 deletions pkg/controller/ovn_eip.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package controller
import (
"context"
"encoding/json"
"errors"
"fmt"
"reflect"
"strings"
Expand Down Expand Up @@ -41,18 +40,18 @@ func (c *Controller) enqueueUpdateOvnEip(old, new interface{}) {
utilruntime.HandleError(err)
return
}
oldEip := old.(*kubeovnv1.OvnEip)
newEip := new.(*kubeovnv1.OvnEip)
if newEip.DeletionTimestamp != nil {
if len(newEip.Finalizers) == 0 {
// avoid delete eip twice
return
} else {
klog.Infof("enqueue del ovn eip %s", key)
c.delOvnEipQueue.Add(newEip)
c.delOvnEipQueue.Add(key)
return
}
}
oldEip := old.(*kubeovnv1.OvnEip)
if oldEip.Spec.V4Ip != "" && oldEip.Spec.V4Ip != newEip.Spec.V4Ip ||
oldEip.Spec.MacAddress != "" && oldEip.Spec.MacAddress != newEip.Spec.MacAddress {
klog.Infof("not support change ip or mac for eip %s", key)
Expand All @@ -74,8 +73,7 @@ func (c *Controller) enqueueDelOvnEip(obj interface{}) {
return
}
klog.Infof("enqueue del ovn eip %s", key)
eip := obj.(*kubeovnv1.OvnEip)
c.delOvnEipQueue.Add(eip)
c.delOvnEipQueue.Add(key)
}

func (c *Controller) runAddOvnEipWorker() {
Expand Down Expand Up @@ -189,16 +187,16 @@ func (c *Controller) processNextDeleteOvnEipWorkItem() bool {
}
err := func(obj interface{}) error {
defer c.delOvnEipQueue.Done(obj)
var eip *kubeovnv1.OvnEip
var key string
var ok bool
if eip, ok = obj.(*kubeovnv1.OvnEip); !ok {
if key, ok = obj.(string); !ok {
c.delOvnEipQueue.Forget(obj)
utilruntime.HandleError(fmt.Errorf("expected ovn eip in workqueue but got %#v", obj))
return nil
}
if err := c.handleDelOvnEip(eip); err != nil {
c.delOvnEipQueue.AddRateLimited(obj)
return fmt.Errorf("error syncing '%s': %s, requeuing", eip.Name, err.Error())
if err := c.handleDelOvnEip(key); err != nil {
c.delOvnEipQueue.AddRateLimited(key)
return fmt.Errorf("error syncing '%s': %s, requeuing", key, err.Error())
}
c.delOvnEipQueue.Forget(obj)
return nil
Expand Down Expand Up @@ -285,7 +283,7 @@ func (c *Controller) handleUpdateOvnEip(key string) error {
klog.Error(err)
return err
}
klog.V(3).Infof("handle update ovn eip %s", cachedEip.Name)
klog.Infof("handle update ovn eip %s", cachedEip.Name)
if !cachedEip.DeletionTimestamp.IsZero() {
subnetName := cachedEip.Spec.ExternalSubnet
if subnetName == "" {
Expand Down Expand Up @@ -332,28 +330,30 @@ func (c *Controller) handleResetOvnEip(key string) error {
return nil
}

func (c *Controller) handleDelOvnEip(eip *kubeovnv1.OvnEip) error {
klog.V(3).Infof("handle del ovn eip %s", eip.Name)
if len(eip.Finalizers) > 1 {
err := errors.New("eip is referenced, it cannot be deleted directly")
klog.Errorf("failed to delete eip %s, %v", eip.Name, err)
func (c *Controller) handleDelOvnEip(key string) error {
klog.Infof("handle del ovn eip %s", key)
eip, err := c.ovnEipsLister.Get(key)
if err != nil {
if k8serrors.IsNotFound(err) {
return nil
}
klog.Error(err)
return err
}

if err := c.handleDelOvnEipFinalizer(eip, util.OvnEipFinalizer); err != nil {
if err = c.handleDelOvnEipFinalizer(eip, util.ControllerName); err != nil {
klog.Errorf("failed to handle remove ovn eip finalizer , %v", err)
return err
}

if eip.Spec.Type == util.Lsp {
if err := c.ovnClient.DeleteLogicalSwitchPort(eip.Name); err != nil {
if err = c.ovnClient.DeleteLogicalSwitchPort(eip.Name); err != nil {
klog.Errorf("failed to delete lsp %s, %v", eip.Name, err)
return err
}
}

if eip.Spec.Type == util.Lrp {
if err := c.ovnClient.DeleteLogicalRouterPort(eip.Name); err != nil {
if err = c.ovnClient.DeleteLogicalRouterPort(eip.Name); err != nil {
klog.Errorf("failed to delete lrp %s, %v", eip.Name, err)
return err
}
Expand Down
Loading

0 comments on commit 515bdb7

Please sign in to comment.