Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix ovn nat not clean #3139

Merged
merged 4 commits into from
Aug 16, 2023
Merged

fix ovn nat not clean #3139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3b5d53e
fix ovn nat not clean
bobz965 Aug 14, 2023
2b25faf
fix eip nat clean
bobz965 Aug 15, 2023
1d7c24b
fix e2e
bobz965 Aug 15, 2023
01b389a
fix log
bobz965 Aug 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 77 additions & 24 deletions pkg/controller/ovn_dnat.go
View file
Open in desktop
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
Open in desktop
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
Loading