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 1 commit
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
58 changes: 19 additions & 39 deletions pkg/controller/ovn_dnat.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,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 @@ -40,11 +40,6 @@ func (c *Controller) enqueueUpdateOvnDnatRule(old, new interface{}) {

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 oldDnat.Spec.OvnEip != newDnat.Spec.OvnEip {
c.resetOvnEipQueue.Add(oldDnat.Spec.OvnEip)
Expand All @@ -67,8 +62,9 @@ func (c *Controller) enqueueDelOvnDnatRule(obj interface{}) {
utilruntime.HandleError(err)
return
}
klog.V(3).Infof("enqueue delete ovn dnat %s", key)
c.delOvnDnatRuleQueue.Add(key)
klog.Infof("enqueue delete ovn dnat %s", key)
dnat := obj.(*kubeovnv1.OvnDnatRule)
c.delOvnDnatRuleQueue.Add(dnat)
bobz965 marked this conversation as resolved.
Show resolved Hide resolved
}

func (c *Controller) runAddOvnDnatRuleWorker() {
Expand Down Expand Up @@ -154,16 +150,16 @@ func (c *Controller) processNextDeleteOvnDnatRuleWorkItem() bool {

err := func(obj interface{}) error {
defer c.delOvnDnatRuleQueue.Done(obj)
var key string
var dnat *kubeovnv1.OvnDnatRule
var ok bool
if key, ok = obj.(string); !ok {
if dnat, ok = obj.(*kubeovnv1.OvnDnatRule); !ok {
c.delOvnDnatRuleQueue.Forget(obj)
utilruntime.HandleError(fmt.Errorf("expected string in workqueue but got %#v", obj))
return nil
}
if err := c.handleDelOvnDnatRule(key); err != nil {
c.delOvnDnatRuleQueue.AddRateLimited(key)
return fmt.Errorf("error syncing '%s': %s, requeuing", key, err.Error())
if err := c.handleDelOvnDnatRule(dnat); err != nil {
c.delOvnDnatRuleQueue.AddRateLimited(obj)
return fmt.Errorf("error syncing '%s': %s, requeuing", dnat.Name, err.Error())
}
c.delOvnDnatRuleQueue.Forget(obj)
return nil
Expand Down Expand Up @@ -212,8 +208,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 @@ -315,32 +310,17 @@ func (c *Controller) handleAddOvnDnatRule(key string) error {
return nil
}

func (c *Controller) handleDelOvnDnatRule(key string) error {
klog.V(3).Infof("handle del ovn dnat %s", key)
cachedDnat, err := c.ovnDnatRulesLister.Get(key)
if err != nil {
if k8serrors.IsNotFound(err) {
return nil
}
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 {
func (c *Controller) handleDelOvnDnatRule(dnat *kubeovnv1.OvnDnatRule) error {
if dnat.Status.Vpc != "" && dnat.Status.V4Eip != "" && dnat.Status.ExternalPort != "" {
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
}
}
c.resetOvnEipQueue.Add(cachedDnat.Spec.OvnEip)
if dnat.Spec.OvnEip != "" {
c.resetOvnEipQueue.Add(dnat.Spec.OvnEip)
}
return nil
}

Expand All @@ -354,7 +334,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 +401,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
52 changes: 19 additions & 33 deletions pkg/controller/ovn_fip.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func (c *Controller) enqueueAddOvnFip(obj interface{}) {
utilruntime.HandleError(err)
return
}
klog.V(3).Infof("enqueue add ovn fip %s", key)
klog.Infof("enqueue add ovn fip %s", key)
c.addOvnFipQueue.Add(key)
}

Expand All @@ -41,11 +41,6 @@ func (c *Controller) enqueueUpdateOvnFip(old, new interface{}) {
}
oldFip := old.(*kubeovnv1.OvnFip)
newFip := new.(*kubeovnv1.OvnFip)
if !newFip.DeletionTimestamp.IsZero() {
klog.V(3).Infof("enqueue del ovn fip %s", key)
c.delOvnFipQueue.Add(key)
return
}
if oldFip.Spec.OvnEip != newFip.Spec.OvnEip {
// enqueue to reset eip to be clean
klog.Infof("enqueue reset old ovn eip %s", oldFip.Spec.OvnEip)
Expand All @@ -66,8 +61,9 @@ func (c *Controller) enqueueDelOvnFip(obj interface{}) {
utilruntime.HandleError(err)
return
}
klog.V(3).Infof("enqueue del ovn fip %s", key)
c.delOvnFipQueue.Add(key)
klog.Infof("enqueue del ovn fip %s", key)
fip := obj.(*kubeovnv1.OvnFip)
c.delOvnFipQueue.Add(fip)
}

func (c *Controller) runAddOvnFipWorker() {
Expand Down Expand Up @@ -150,16 +146,16 @@ func (c *Controller) processNextDeleteOvnFipWorkItem() bool {
}
err := func(obj interface{}) error {
defer c.delOvnFipQueue.Done(obj)
var key string
var fip *kubeovnv1.OvnFip
var ok bool
if key, ok = obj.(string); !ok {
if fip, ok = obj.(*kubeovnv1.OvnFip); !ok {
c.delOvnFipQueue.Forget(obj)
utilruntime.HandleError(fmt.Errorf("expected fip in workqueue but got %#v", obj))
return nil
}
if err := c.handleDelOvnFip(key); err != nil {
c.delOvnFipQueue.AddRateLimited(key)
return fmt.Errorf("error syncing '%s': %s, requeuing", key, err.Error())
if err := c.handleDelOvnFip(fip); err != nil {
c.delOvnFipQueue.AddRateLimited(obj)
return fmt.Errorf("error syncing '%s': %s, requeuing", fip.Name, err.Error())
}
c.delOvnFipQueue.Forget(obj)
return nil
Expand Down Expand Up @@ -202,7 +198,7 @@ func (c *Controller) handleAddOvnFip(key string) error {
// already ok
return nil
}
klog.V(3).Infof("handle add fip %s", key)
klog.Infof("handle add fip %s", key)
var internalV4Ip, mac, subnetName string
if cachedFip.Spec.IpType == util.Vip {
internalVip, err := c.virtualIpsLister.Get(cachedFip.Spec.IpName)
Expand Down Expand Up @@ -316,7 +312,7 @@ func (c *Controller) handleUpdateOvnFip(key string) error {
}
return err
}
klog.V(3).Infof("handle update fip %s", key)
klog.Infof("handle update fip %s", key)
var internalV4Ip, mac, subnetName string
if cachedFip.Spec.IpType == util.Vip {
internalVip, err := c.virtualIpsLister.Get(cachedFip.Spec.IpName)
Expand Down Expand Up @@ -380,7 +376,7 @@ func (c *Controller) handleUpdateOvnFip(key string) error {
fip := cachedFip.DeepCopy()
// fip change eip
if c.ovnFipChangeEip(fip, cachedEip) {
klog.V(3).Infof("fip change ip, old ip '%s', new ip %s", fip.Status.V4Ip, cachedEip.Status.V4Ip)
klog.Infof("fip change ip, old ip '%s', new ip %s", fip.Status.V4Ip, cachedEip.Status.V4Ip)
if err = c.ovnClient.DeleteNat(vpcName, ovnnb.NATTypeDNATAndSNAT, fip.Status.V4Ip, internalV4Ip); err != nil {
klog.Errorf("failed to create fip, %v", err)
return err
Expand Down Expand Up @@ -410,29 +406,19 @@ func (c *Controller) handleUpdateOvnFip(key string) error {
return nil
}

func (c *Controller) handleDelOvnFip(key string) error {
klog.V(3).Infof("handle del ovn fip %s", key)
cachedFip, err := c.ovnFipsLister.Get(key)
if err != nil {
if k8serrors.IsNotFound(err) {
return nil
}
klog.Error(err)
return err
}
eipName := cachedFip.Spec.OvnEip
if len(eipName) == 0 {
klog.Errorf("failed to delete ovn fip, should set eip")
}
func (c *Controller) handleDelOvnFip(fip *kubeovnv1.OvnFip) error {
klog.Infof("handle del ovn fip %s", fip.Name)
// ovn delete fip
if cachedFip.Status.Vpc != "" && cachedFip.Status.V4Eip != "" && cachedFip.Status.V4Ip != "" {
if err = c.ovnClient.DeleteNat(cachedFip.Status.Vpc, ovnnb.NATTypeDNATAndSNAT, cachedFip.Status.V4Eip, cachedFip.Status.V4Ip); err != nil {
if fip.Status.Vpc != "" && fip.Status.V4Eip != "" && fip.Status.V4Ip != "" {
if err := c.ovnClient.DeleteNat(fip.Status.Vpc, ovnnb.NATTypeDNATAndSNAT, fip.Status.V4Eip, fip.Status.V4Ip); err != nil {
klog.Errorf("failed to delete fip, %v", err)
return err
}
}
// reset eip
c.resetOvnEipQueue.Add(cachedFip.Spec.OvnEip)
if fip.Spec.OvnEip != "" {
c.resetOvnEipQueue.Add(fip.Spec.OvnEip)
}
return nil
}

Expand Down
55 changes: 24 additions & 31 deletions pkg/controller/ovn_snat.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,6 @@ func (c *Controller) enqueueUpdateOvnSnatRule(old, new interface{}) {
}
oldSnat := old.(*kubeovnv1.OvnSnatRule)
newSnat := new.(*kubeovnv1.OvnSnatRule)
if !newSnat.DeletionTimestamp.IsZero() {
klog.V(3).Infof("enqueue reset old ovn eip %s", oldSnat.Spec.OvnEip)
c.updateOvnSnatRuleQueue.Add(key)
return
}
if oldSnat.Spec.OvnEip != newSnat.Spec.OvnEip {
// enqueue to reset eip to be clean
c.resetOvnEipQueue.Add(oldSnat.Spec.OvnEip)
Expand All @@ -62,8 +57,9 @@ func (c *Controller) enqueueDelOvnSnatRule(obj interface{}) {
utilruntime.HandleError(err)
return
}
klog.V(3).Infof("enqueue del ovn snat %s", key)
c.delOvnSnatRuleQueue.Add(key)
klog.Infof("enqueue del ovn snat %s", key)
snat := obj.(*kubeovnv1.OvnSnatRule)
c.delOvnSnatRuleQueue.Add(snat)
}

func (c *Controller) runAddOvnSnatRuleWorker() {
Expand Down Expand Up @@ -149,16 +145,16 @@ func (c *Controller) processNextDeleteOvnSnatRuleWorkItem() bool {

err := func(obj interface{}) error {
defer c.delOvnSnatRuleQueue.Done(obj)
var key string
var snat *kubeovnv1.OvnSnatRule
var ok bool
if key, ok = obj.(string); !ok {
if snat, ok = obj.(*kubeovnv1.OvnSnatRule); !ok {
c.delOvnSnatRuleQueue.Forget(obj)
utilruntime.HandleError(fmt.Errorf("expected snat in workqueue but got %#v", obj))
return nil
}
if err := c.handleDelOvnSnatRule(key); err != nil {
c.delOvnSnatRuleQueue.AddRateLimited(key)
return fmt.Errorf("error syncing '%s': %s, requeuing", key, err.Error())
if err := c.handleDelOvnSnatRule(snat); err != nil {
c.delOvnSnatRuleQueue.AddRateLimited(obj)
return fmt.Errorf("error syncing '%s': %s, requeuing", snat.Name, err.Error())
}
c.delOvnSnatRuleQueue.Forget(obj)
return nil
Expand All @@ -184,7 +180,7 @@ func (c *Controller) handleAddOvnSnatRule(key string) error {
// already ok
return nil
}
klog.V(3).Infof("handle add ovn snat %s", key)
klog.Infof("handle add ovn snat %s", key)
eipName := cachedSnat.Spec.OvnEip
if len(eipName) == 0 {
err := fmt.Errorf("failed to create fip rule, should set eip")
Expand Down Expand Up @@ -278,7 +274,7 @@ func (c *Controller) handleUpdateOvnSnatRule(key string) error {
klog.Error(err)
return err
}
klog.V(3).Infof("handle update ovn snat %s", key)
klog.Infof("handle update ovn snat %s", key)
eipName := cachedSnat.Spec.OvnEip
if len(eipName) == 0 {
err := fmt.Errorf("failed to create fip rule, should set eip")
Expand All @@ -290,20 +286,6 @@ func (c *Controller) handleUpdateOvnSnatRule(key string) error {
klog.Errorf("failed to get eip, %v", err)
return err
}
// should delete
if !cachedSnat.DeletionTimestamp.IsZero() {
klog.V(3).Infof("ovn delete snat %s", key)
// ovn delete snat
if cachedSnat.Status.Vpc != "" && cachedSnat.Status.V4Eip != "" && cachedSnat.Status.V4IpCidr != "" {
if err = c.ovnClient.DeleteNat(cachedSnat.Status.Vpc, ovnnb.NATTypeSNAT, cachedSnat.Status.V4Eip, cachedSnat.Status.V4IpCidr); err != nil {
klog.Errorf("failed to delete snat, %v", err)
return err
}
}
// reset eip
c.resetOvnEipQueue.Add(cachedSnat.Spec.OvnEip)
return nil
}

if cachedEip.Spec.Type == util.Lsp {
// eip is using by ecmp nexthop lsp, nat can not use
Expand Down Expand Up @@ -344,7 +326,7 @@ func (c *Controller) handleUpdateOvnSnatRule(key string) error {
}
// snat change eip
if c.ovnSnatChangeEip(cachedSnat, cachedEip) {
klog.V(3).Infof("snat change ip, old ip %s, new ip %s", cachedEip.Status.V4Ip, cachedEip.Spec.V4Ip)
klog.Infof("snat change ip, old ip %s, new ip %s", cachedEip.Status.V4Ip, cachedEip.Spec.V4Ip)
if err = c.ovnClient.DeleteNat(vpcName, ovnnb.NATTypeSNAT, cachedEip.Status.V4Ip, v4IpCidr); err != nil {
klog.Errorf("failed to delte snat, %v", err)
return err
Expand All @@ -371,8 +353,19 @@ func (c *Controller) handleUpdateOvnSnatRule(key string) error {
return nil
}

func (c *Controller) handleDelOvnSnatRule(key string) error {
klog.V(3).Infof("deleted ovn snat %s", key)
func (c *Controller) handleDelOvnSnatRule(snat *kubeovnv1.OvnSnatRule) error {
klog.Infof("deleted ovn snat %s", snat.Name)
// ovn delete snat
if snat.Status.Vpc != "" && snat.Status.V4Eip != "" && snat.Status.V4IpCidr != "" {
if err := c.ovnClient.DeleteNat(snat.Status.Vpc, ovnnb.NATTypeSNAT, snat.Status.V4Eip, snat.Status.V4IpCidr); err != nil {
klog.Errorf("failed to delete snat, %v", err)
return err
}
}
// reset eip
if snat.Spec.OvnEip != "" {
c.resetOvnEipQueue.Add(snat.Spec.OvnEip)
}
return nil
}

Expand Down
Loading