Skip to content

Commit 515bdb7

Browse files
zbb88888zbb88888
committed
fix ovn nat not clean (#3139)
* fix ovn nat not clean
1 parent f225c66 commit 515bdb7

File tree

7 files changed

+284
-108
lines changed

7 files changed

+284
-108
lines changed

‎pkg/controller/ovn_dnat.go

+77-24
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
1515
"k8s.io/client-go/tools/cache"
1616
"k8s.io/klog/v2"
17+
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
1718

1819
kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1"
1920
"github.com/kubeovn/kube-ovn/pkg/util"
@@ -26,7 +27,7 @@ func (c *Controller) enqueueAddOvnDnatRule(obj interface{}) {
2627
utilruntime.HandleError(err)
2728
return
2829
}
29-
klog.V(3).Infof("enqueue add ovn dnat %s", key)
30+
klog.Infof("enqueue add ovn dnat %s", key)
3031
c.addOvnDnatRuleQueue.Add(key)
3132
}
3233

@@ -37,15 +38,18 @@ func (c *Controller) enqueueUpdateOvnDnatRule(old, new interface{}) {
3738
utilruntime.HandleError(err)
3839
return
3940
}
40-
41-
oldDnat := old.(*kubeovnv1.OvnDnatRule)
4241
newDnat := new.(*kubeovnv1.OvnDnatRule)
4342
if !newDnat.DeletionTimestamp.IsZero() {
44-
klog.Infof("enqueue del ovn dnat %s", key)
45-
c.delOvnDnatRuleQueue.Add(key)
46-
return
43+
if len(newDnat.Finalizers) == 0 {
44+
// avoid delete twice
45+
return
46+
} else {
47+
klog.Infof("enqueue del ovn dnat %s", key)
48+
c.delOvnDnatRuleQueue.Add(key)
49+
return
50+
}
4751
}
48-
52+
oldDnat := old.(*kubeovnv1.OvnDnatRule)
4953
if oldDnat.Spec.OvnEip != newDnat.Spec.OvnEip {
5054
c.resetOvnEipQueue.Add(oldDnat.Spec.OvnEip)
5155
}
@@ -67,7 +71,7 @@ func (c *Controller) enqueueDelOvnDnatRule(obj interface{}) {
6771
utilruntime.HandleError(err)
6872
return
6973
}
70-
klog.V(3).Infof("enqueue delete ovn dnat %s", key)
74+
klog.Infof("enqueue delete ovn dnat %s", key)
7175
c.delOvnDnatRuleQueue.Add(key)
7276
}
7377

@@ -212,8 +216,7 @@ func (c *Controller) handleAddOvnDnatRule(key string) error {
212216
// already ok
213217
return nil
214218
}
215-
klog.V(3).Infof("handle add dnat %s", key)
216-
219+
klog.Infof("handle add dnat %s", key)
217220
var internalV4Ip, mac, subnetName string
218221
if cachedDnat.Spec.IpType == util.Vip {
219222
internalVip, err := c.virtualIpsLister.Get(cachedDnat.Spec.IpName)
@@ -279,7 +282,7 @@ func (c *Controller) handleAddOvnDnatRule(key string) error {
279282
return err
280283
}
281284

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

296+
if err := c.handleAddOvnDnatFinalizer(cachedDnat, util.ControllerName); err != nil {
297+
klog.Errorf("failed to add finalizer for ovn dnat %s, %v", cachedDnat.Name, err)
298+
return err
299+
}
300+
293301
// patch dnat eip relationship
294302
if err = c.natLabelAndAnnoOvnEip(eipName, cachedDnat.Name, vpcName); err != nil {
295303
klog.Errorf("failed to label dnat '%s' in eip %s, %v", cachedDnat.Name, eipName, err)
@@ -316,7 +324,7 @@ func (c *Controller) handleAddOvnDnatRule(key string) error {
316324
}
317325

318326
func (c *Controller) handleDelOvnDnatRule(key string) error {
319-
klog.V(3).Infof("handle del ovn dnat %s", key)
327+
klog.Infof("handle delete ovn dnat %s", key)
320328
cachedDnat, err := c.ovnDnatRulesLister.Get(key)
321329
if err != nil {
322330
if k8serrors.IsNotFound(err) {
@@ -325,22 +333,20 @@ func (c *Controller) handleDelOvnDnatRule(key string) error {
325333
klog.Error(err)
326334
return err
327335
}
328-
329-
eipName := cachedDnat.Spec.OvnEip
330-
if len(eipName) == 0 {
331-
err := fmt.Errorf("failed to create fip rule, should set eip")
332-
klog.Error(err)
333-
return err
334-
}
335-
336336
if cachedDnat.Status.Vpc != "" && cachedDnat.Status.V4Eip != "" && cachedDnat.Status.ExternalPort != "" {
337337
if err = c.DelDnatRule(cachedDnat.Status.Vpc, cachedDnat.Name,
338338
cachedDnat.Status.V4Eip, cachedDnat.Status.ExternalPort); err != nil {
339-
klog.Errorf("failed to delete dnat, %v", err)
339+
klog.Errorf("failed to delete dnat %s, %v", key, err)
340340
return err
341341
}
342342
}
343-
c.resetOvnEipQueue.Add(cachedDnat.Spec.OvnEip)
343+
if err = c.handleDelOvnDnatFinalizer(cachedDnat, util.ControllerName); err != nil {
344+
klog.Errorf("failed to remove finalizer for ovn dnat %s, %v", cachedDnat.Name, err)
345+
return err
346+
}
347+
if cachedDnat.Spec.OvnEip != "" {
348+
c.resetOvnEipQueue.Add(cachedDnat.Spec.OvnEip)
349+
}
344350
return nil
345351
}
346352

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

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

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

613619
return nil
614620
}
621+
622+
func (c *Controller) handleAddOvnDnatFinalizer(cachedDnat *kubeovnv1.OvnDnatRule, finalizer string) error {
623+
if cachedDnat.DeletionTimestamp.IsZero() {
624+
if util.ContainsString(cachedDnat.Finalizers, finalizer) {
625+
return nil
626+
}
627+
}
628+
newDnat := cachedDnat.DeepCopy()
629+
controllerutil.AddFinalizer(newDnat, finalizer)
630+
patch, err := util.GenerateMergePatchPayload(cachedDnat, newDnat)
631+
if err != nil {
632+
klog.Errorf("failed to generate patch payload for ovn dnat '%s', %v", cachedDnat.Name, err)
633+
return err
634+
}
635+
if _, err := c.config.KubeOvnClient.KubeovnV1().OvnDnatRules().Patch(context.Background(), cachedDnat.Name,
636+
types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil {
637+
if k8serrors.IsNotFound(err) {
638+
return nil
639+
}
640+
klog.Errorf("failed to add finalizer for ovn dnat '%s', %v", cachedDnat.Name, err)
641+
return err
642+
}
643+
return nil
644+
}
645+
646+
func (c *Controller) handleDelOvnDnatFinalizer(cachedDnat *kubeovnv1.OvnDnatRule, finalizer string) error {
647+
if len(cachedDnat.Finalizers) == 0 {
648+
return nil
649+
}
650+
var err error
651+
newDnat := cachedDnat.DeepCopy()
652+
controllerutil.RemoveFinalizer(newDnat, finalizer)
653+
patch, err := util.GenerateMergePatchPayload(cachedDnat, newDnat)
654+
if err != nil {
655+
klog.Errorf("failed to generate patch payload for ovn dnat '%s', %v", cachedDnat.Name, err)
656+
return err
657+
}
658+
if _, err := c.config.KubeOvnClient.KubeovnV1().OvnDnatRules().Patch(context.Background(), cachedDnat.Name,
659+
types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil {
660+
if k8serrors.IsNotFound(err) {
661+
return nil
662+
}
663+
klog.Errorf("failed to remove finalizer from ovn dnat '%s', %v", cachedDnat.Name, err)
664+
return err
665+
}
666+
return nil
667+
}

‎pkg/controller/ovn_eip.go

+20-20
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package controller
33
import (
44
"context"
55
"encoding/json"
6-
"errors"
76
"fmt"
87
"reflect"
98
"strings"
@@ -41,18 +40,18 @@ func (c *Controller) enqueueUpdateOvnEip(old, new interface{}) {
4140
utilruntime.HandleError(err)
4241
return
4342
}
44-
oldEip := old.(*kubeovnv1.OvnEip)
4543
newEip := new.(*kubeovnv1.OvnEip)
4644
if newEip.DeletionTimestamp != nil {
4745
if len(newEip.Finalizers) == 0 {
4846
// avoid delete eip twice
4947
return
5048
} else {
5149
klog.Infof("enqueue del ovn eip %s", key)
52-
c.delOvnEipQueue.Add(newEip)
50+
c.delOvnEipQueue.Add(key)
5351
return
5452
}
5553
}
54+
oldEip := old.(*kubeovnv1.OvnEip)
5655
if oldEip.Spec.V4Ip != "" && oldEip.Spec.V4Ip != newEip.Spec.V4Ip ||
5756
oldEip.Spec.MacAddress != "" && oldEip.Spec.MacAddress != newEip.Spec.MacAddress {
5857
klog.Infof("not support change ip or mac for eip %s", key)
@@ -74,8 +73,7 @@ func (c *Controller) enqueueDelOvnEip(obj interface{}) {
7473
return
7574
}
7675
klog.Infof("enqueue del ovn eip %s", key)
77-
eip := obj.(*kubeovnv1.OvnEip)
78-
c.delOvnEipQueue.Add(eip)
76+
c.delOvnEipQueue.Add(key)
7977
}
8078

8179
func (c *Controller) runAddOvnEipWorker() {
@@ -189,16 +187,16 @@ func (c *Controller) processNextDeleteOvnEipWorkItem() bool {
189187
}
190188
err := func(obj interface{}) error {
191189
defer c.delOvnEipQueue.Done(obj)
192-
var eip *kubeovnv1.OvnEip
190+
var key string
193191
var ok bool
194-
if eip, ok = obj.(*kubeovnv1.OvnEip); !ok {
192+
if key, ok = obj.(string); !ok {
195193
c.delOvnEipQueue.Forget(obj)
196194
utilruntime.HandleError(fmt.Errorf("expected ovn eip in workqueue but got %#v", obj))
197195
return nil
198196
}
199-
if err := c.handleDelOvnEip(eip); err != nil {
200-
c.delOvnEipQueue.AddRateLimited(obj)
201-
return fmt.Errorf("error syncing '%s': %s, requeuing", eip.Name, err.Error())
197+
if err := c.handleDelOvnEip(key); err != nil {
198+
c.delOvnEipQueue.AddRateLimited(key)
199+
return fmt.Errorf("error syncing '%s': %s, requeuing", key, err.Error())
202200
}
203201
c.delOvnEipQueue.Forget(obj)
204202
return nil
@@ -285,7 +283,7 @@ func (c *Controller) handleUpdateOvnEip(key string) error {
285283
klog.Error(err)
286284
return err
287285
}
288-
klog.V(3).Infof("handle update ovn eip %s", cachedEip.Name)
286+
klog.Infof("handle update ovn eip %s", cachedEip.Name)
289287
if !cachedEip.DeletionTimestamp.IsZero() {
290288
subnetName := cachedEip.Spec.ExternalSubnet
291289
if subnetName == "" {
@@ -332,28 +330,30 @@ func (c *Controller) handleResetOvnEip(key string) error {
332330
return nil
333331
}
334332

335-
func (c *Controller) handleDelOvnEip(eip *kubeovnv1.OvnEip) error {
336-
klog.V(3).Infof("handle del ovn eip %s", eip.Name)
337-
if len(eip.Finalizers) > 1 {
338-
err := errors.New("eip is referenced, it cannot be deleted directly")
339-
klog.Errorf("failed to delete eip %s, %v", eip.Name, err)
333+
func (c *Controller) handleDelOvnEip(key string) error {
334+
klog.Infof("handle del ovn eip %s", key)
335+
eip, err := c.ovnEipsLister.Get(key)
336+
if err != nil {
337+
if k8serrors.IsNotFound(err) {
338+
return nil
339+
}
340+
klog.Error(err)
340341
return err
341342
}
342-
343-
if err := c.handleDelOvnEipFinalizer(eip, util.OvnEipFinalizer); err != nil {
343+
if err = c.handleDelOvnEipFinalizer(eip, util.ControllerName); err != nil {
344344
klog.Errorf("failed to handle remove ovn eip finalizer , %v", err)
345345
return err
346346
}
347347

348348
if eip.Spec.Type == util.Lsp {
349-
if err := c.ovnClient.DeleteLogicalSwitchPort(eip.Name); err != nil {
349+
if err = c.ovnClient.DeleteLogicalSwitchPort(eip.Name); err != nil {
350350
klog.Errorf("failed to delete lsp %s, %v", eip.Name, err)
351351
return err
352352
}
353353
}
354354

355355
if eip.Spec.Type == util.Lrp {
356-
if err := c.ovnClient.DeleteLogicalRouterPort(eip.Name); err != nil {
356+
if err = c.ovnClient.DeleteLogicalRouterPort(eip.Name); err != nil {
357357
klog.Errorf("failed to delete lrp %s, %v", eip.Name, err)
358358
return err
359359
}

0 commit comments

Comments
 (0)