diff --git a/pkg/controller/ovn_dnat.go b/pkg/controller/ovn_dnat.go index 53e89eee1bd..34372d30f7f 100644 --- a/pkg/controller/ovn_dnat.go +++ b/pkg/controller/ovn_dnat.go @@ -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" @@ -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) } @@ -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) } @@ -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) } @@ -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) @@ -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 } @@ -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) @@ -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) { @@ -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 } @@ -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) @@ -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 @@ -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 +} diff --git a/pkg/controller/ovn_eip.go b/pkg/controller/ovn_eip.go index d1ce97555a1..ef9d406975e 100644 --- a/pkg/controller/ovn_eip.go +++ b/pkg/controller/ovn_eip.go @@ -3,7 +3,6 @@ package controller import ( "context" "encoding/json" - "errors" "fmt" "reflect" "strings" @@ -41,7 +40,6 @@ 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 { @@ -49,10 +47,11 @@ func (c *Controller) enqueueUpdateOvnEip(old, new interface{}) { 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) @@ -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() { @@ -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 @@ -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 == "" { @@ -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 } diff --git a/pkg/controller/ovn_fip.go b/pkg/controller/ovn_fip.go index 0da5c12f6c1..ff8250bf548 100644 --- a/pkg/controller/ovn_fip.go +++ b/pkg/controller/ovn_fip.go @@ -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/ovsdb/ovnnb" @@ -28,7 +29,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) } @@ -39,13 +40,18 @@ func (c *Controller) enqueueUpdateOvnFip(old, new interface{}) { utilruntime.HandleError(err) return } - 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 len(newFip.Finalizers) == 0 { + // avoid delete twice + return + } else { + klog.Infof("enqueue del ovn fip %s", key) + c.delOvnFipQueue.Add(key) + return + } } + oldFip := old.(*kubeovnv1.OvnFip) 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) @@ -66,7 +72,7 @@ func (c *Controller) enqueueDelOvnFip(obj interface{}) { utilruntime.HandleError(err) return } - klog.V(3).Infof("enqueue del ovn fip %s", key) + klog.Infof("enqueue del ovn fip %s", key) c.delOvnFipQueue.Add(key) } @@ -202,7 +208,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) @@ -276,7 +282,7 @@ func (c *Controller) handleAddOvnFip(key string) error { klog.Errorf("failed to patch status for fip %s, %v", key, err) 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 } @@ -287,6 +293,12 @@ func (c *Controller) handleAddOvnFip(key string) error { klog.Errorf("failed to create v4 fip, %v", err) return err } + + if err = c.handleAddOvnFipFinalizer(cachedFip, util.ControllerName); err != nil { + klog.Errorf("failed to add finalizer for ovn fip, %v", err) + return err + } + // patch fip eip relationship if err = c.natLabelAndAnnoOvnEip(eipName, cachedFip.Name, vpcName); err != nil { klog.Errorf("failed to label fip '%s' in eip %s, %v", cachedFip.Name, eipName, err) @@ -316,7 +328,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) @@ -380,7 +392,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 @@ -411,7 +423,7 @@ func (c *Controller) handleUpdateOvnFip(key string) error { } func (c *Controller) handleDelOvnFip(key string) error { - klog.V(3).Infof("handle del ovn fip %s", key) + klog.Infof("handle del ovn fip %s", key) cachedFip, err := c.ovnFipsLister.Get(key) if err != nil { if k8serrors.IsNotFound(err) { @@ -420,19 +432,21 @@ func (c *Controller) handleDelOvnFip(key string) error { klog.Error(err) return err } - eipName := cachedFip.Spec.OvnEip - if len(eipName) == 0 { - klog.Errorf("failed to delete ovn fip, should set eip") - } - // ovn delete fip + // ovn delete fip nat 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 { - klog.Errorf("failed to delete fip, %v", err) + klog.Errorf("failed to delete fip %s, %v", key, err) return err } } + if err = c.handleDelOvnFipFinalizer(cachedFip, util.ControllerName); err != nil { + klog.Errorf("failed to remove finalizer for ovn fip %s, %v", cachedFip.Name, err) + return err + } // reset eip - c.resetOvnEipQueue.Add(cachedFip.Spec.OvnEip) + if cachedFip.Spec.OvnEip != "" { + c.resetOvnEipQueue.Add(cachedFip.Spec.OvnEip) + } return nil } @@ -558,3 +572,50 @@ func (c *Controller) GetOvnEip(eipName string) (*kubeovnv1.OvnEip, error) { } return cachedEip, nil } + +func (c *Controller) handleAddOvnFipFinalizer(cachedFip *kubeovnv1.OvnFip, finalizer string) error { + if cachedFip.DeletionTimestamp.IsZero() { + if util.ContainsString(cachedFip.Finalizers, finalizer) { + return nil + } + } + newFip := cachedFip.DeepCopy() + controllerutil.AddFinalizer(newFip, finalizer) + patch, err := util.GenerateMergePatchPayload(cachedFip, newFip) + if err != nil { + klog.Errorf("failed to generate patch payload for ovn fip '%s', %v", cachedFip.Name, err) + return err + } + if _, err := c.config.KubeOvnClient.KubeovnV1().OvnFips().Patch(context.Background(), cachedFip.Name, + types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to add finalizer for ovn fip '%s', %v", cachedFip.Name, err) + return err + } + return nil +} + +func (c *Controller) handleDelOvnFipFinalizer(cachedFip *kubeovnv1.OvnFip, finalizer string) error { + if len(cachedFip.Finalizers) == 0 { + return nil + } + var err error + newFip := cachedFip.DeepCopy() + controllerutil.RemoveFinalizer(newFip, finalizer) + patch, err := util.GenerateMergePatchPayload(cachedFip, newFip) + if err != nil { + klog.Errorf("failed to generate patch payload for ovn fip '%s', %v", cachedFip.Name, err) + return err + } + if _, err := c.config.KubeOvnClient.KubeovnV1().OvnFips().Patch(context.Background(), cachedFip.Name, + types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to remove finalizer from ovn fip '%s', %v", cachedFip.Name, err) + return err + } + return nil +} diff --git a/pkg/controller/ovn_snat.go b/pkg/controller/ovn_snat.go index 47faa0a55ed..bacd2d02191 100644 --- a/pkg/controller/ovn_snat.go +++ b/pkg/controller/ovn_snat.go @@ -11,6 +11,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/ovsdb/ovnnb" @@ -35,13 +36,18 @@ func (c *Controller) enqueueUpdateOvnSnatRule(old, new interface{}) { utilruntime.HandleError(err) return } - 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 len(newSnat.Finalizers) == 0 { + // avoid delete twice + return + } else { + klog.Infof("enqueue del ovn snat %s", key) + c.delOvnSnatRuleQueue.Add(key) + return + } } + oldSnat := old.(*kubeovnv1.OvnSnatRule) if oldSnat.Spec.OvnEip != newSnat.Spec.OvnEip { // enqueue to reset eip to be clean c.resetOvnEipQueue.Add(oldSnat.Spec.OvnEip) @@ -62,7 +68,7 @@ func (c *Controller) enqueueDelOvnSnatRule(obj interface{}) { utilruntime.HandleError(err) return } - klog.V(3).Infof("enqueue del ovn snat %s", key) + klog.Infof("enqueue del ovn snat %s", key) c.delOvnSnatRuleQueue.Add(key) } @@ -184,7 +190,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") @@ -241,7 +247,7 @@ func (c *Controller) handleAddOvnSnatRule(key string) error { } // create snat - 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 } @@ -250,6 +256,10 @@ func (c *Controller) handleAddOvnSnatRule(key string) error { klog.Errorf("failed to create snat, %v", err) return err } + if err := c.handleAddOvnSnatFinalizer(cachedSnat, util.ControllerName); err != nil { + klog.Errorf("failed to add finalizer for ovn snat %s, %v", cachedSnat.Name, err) + return err + } if err = c.natLabelAndAnnoOvnEip(eipName, cachedSnat.Name, vpcName); err != nil { klog.Errorf("failed to label snat '%s' in eip %s, %v", cachedSnat.Name, eipName, err) return err @@ -278,7 +288,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") @@ -290,20 +300,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 @@ -344,7 +340,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 @@ -372,7 +368,31 @@ func (c *Controller) handleUpdateOvnSnatRule(key string) error { } func (c *Controller) handleDelOvnSnatRule(key string) error { - klog.V(3).Infof("deleted ovn snat %s", key) + klog.Infof("handle delete ovn snat %s", key) + cachedSnat, err := c.ovnSnatRulesLister.Get(key) + if err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Error(err) + return err + } + // 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 %s, %v", key, err) + return err + } + } + if err = c.handleDelOvnSnatFinalizer(cachedSnat, util.ControllerName); err != nil { + klog.Errorf("failed to remove finalizer for ovn snat %s, %v", cachedSnat.Name, err) + return err + } + // reset eip + if cachedSnat.Spec.OvnEip != "" { + c.resetOvnEipQueue.Add(cachedSnat.Spec.OvnEip) + } return nil } @@ -484,3 +504,50 @@ func (c *Controller) ovnSnatChangeEip(snat *kubeovnv1.OvnSnatRule, eip *kubeovnv } return false } + +func (c *Controller) handleAddOvnSnatFinalizer(cachedSnat *kubeovnv1.OvnSnatRule, finalizer string) error { + if cachedSnat.DeletionTimestamp.IsZero() { + if util.ContainsString(cachedSnat.Finalizers, finalizer) { + return nil + } + } + newSnat := cachedSnat.DeepCopy() + controllerutil.AddFinalizer(newSnat, finalizer) + patch, err := util.GenerateMergePatchPayload(cachedSnat, newSnat) + if err != nil { + klog.Errorf("failed to generate patch payload for ovn snat '%s', %v", cachedSnat.Name, err) + return err + } + if _, err := c.config.KubeOvnClient.KubeovnV1().OvnSnatRules().Patch(context.Background(), cachedSnat.Name, + types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to add finalizer for ovn snat '%s', %v", cachedSnat.Name, err) + return err + } + return nil +} + +func (c *Controller) handleDelOvnSnatFinalizer(cachedSnat *kubeovnv1.OvnSnatRule, finalizer string) error { + if len(cachedSnat.Finalizers) == 0 { + return nil + } + var err error + newSnat := cachedSnat.DeepCopy() + controllerutil.RemoveFinalizer(newSnat, finalizer) + patch, err := util.GenerateMergePatchPayload(cachedSnat, newSnat) + if err != nil { + klog.Errorf("failed to generate patch payload for ovn snat '%s', %v", cachedSnat.Name, err) + return err + } + if _, err := c.config.KubeOvnClient.KubeovnV1().OvnSnatRules().Patch(context.Background(), cachedSnat.Name, + types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to remove finalizer from ovn snat '%s', %v", cachedSnat.Name, err) + return err + } + return nil +} diff --git a/pkg/controller/vip.go b/pkg/controller/vip.go index c47479dd290..68d5b01cb43 100644 --- a/pkg/controller/vip.go +++ b/pkg/controller/vip.go @@ -28,7 +28,7 @@ func (c *Controller) enqueueAddVirtualIp(obj interface{}) { utilruntime.HandleError(err) return } - klog.V(3).Infof("enqueue add vip %s", key) + klog.Infof("enqueue add vip %s", key) c.addVirtualIpQueue.Add(key) } @@ -46,7 +46,7 @@ func (c *Controller) enqueueUpdateVirtualIp(old, new interface{}) { oldVip.Spec.ParentMac != newVip.Spec.ParentMac || oldVip.Spec.ParentV4ip != newVip.Spec.ParentV4ip || oldVip.Spec.V4ip != newVip.Spec.V4ip { - klog.V(3).Infof("enqueue update vip %s", key) + klog.Infof("enqueue update vip %s", key) c.updateVirtualIpQueue.Add(key) } } @@ -58,7 +58,7 @@ func (c *Controller) enqueueDelVirtualIp(obj interface{}) { utilruntime.HandleError(err) return } - klog.V(3).Infof("enqueue del vip %s", key) + klog.Infof("enqueue del vip %s", key) vip := obj.(*kubeovnv1.Vip) c.delVirtualIpQueue.Add(vip) } @@ -298,7 +298,7 @@ func (c *Controller) handleUpdateVirtualIp(key string) error { } func (c *Controller) handleDelVirtualIp(vip *kubeovnv1.Vip) error { - klog.Infof("delete vip %s", vip.Name) + klog.Infof("handle delete vip %s", vip.Name) // TODO:// clean vip in its parent port aap list if vip.Spec.Type == util.SwitchLBRuleVip { subnet, err := c.subnetsLister.Get(vip.Spec.Subnet) diff --git a/pkg/util/const.go b/pkg/util/const.go index 83ad8a71a28..1885ef1c94b 100644 --- a/pkg/util/const.go +++ b/pkg/util/const.go @@ -20,7 +20,6 @@ const ( FipNameAnnotation = "ovn.kubernetes.io/fip_name" FipEnableAnnotation = "ovn.kubernetes.io/enable_fip" FipFinalizer = "ovn.kubernetes.io/fip" - OvnEipFinalizer = "ovn.kubernetes.io/ovn_eip" VipAnnotation = "ovn.kubernetes.io/vip" ChassisAnnotation = "ovn.kubernetes.io/chassis" diff --git a/test/e2e/ovn-vpc-nat-gw/e2e_test.go b/test/e2e/ovn-vpc-nat-gw/e2e_test.go index bb807c0800c..c5584e2a856 100644 --- a/test/e2e/ovn-vpc-nat-gw/e2e_test.go +++ b/test/e2e/ovn-vpc-nat-gw/e2e_test.go @@ -307,12 +307,21 @@ var _ = framework.Describe("[group:ovn-vpc-nat-gw]", func() { ovnEipClient.DeleteSync(dnatEipName) ginkgo.By("Deleting ovn eip " + snatEipName) ovnEipClient.DeleteSync(snatEipName) - ginkgo.By("Deleting ovn share eip " + sharedEipName) - ovnEipClient.DeleteSync(sharedEipName) - ginkgo.By("Deleting ovn vip " + arpProxyVip1Name) + ginkgo.By("Deleting ovn arp proxy vip " + arpProxyVip1Name) vipClient.DeleteSync(arpProxyVip1Name) - ginkgo.By("Deleting ovn vip " + arpProxyVip2Name) + ginkgo.By("Deleting ovn arp proxy vip " + arpProxyVip2Name) + + // clean up share eip case resource + ginkgo.By("Deleting share ovn dnat " + sharedEipDnatName) + ovnDnatRuleClient.DeleteSync(sharedEipDnatName) + ginkgo.By("Deleting share ovn fip " + sharedEipFipShoudOkName) + ovnFipClient.DeleteSync(sharedEipFipShoudOkName) + ginkgo.By("Deleting share ovn fip " + sharedEipFipShoudFailName) + ovnFipClient.DeleteSync(sharedEipFipShoudFailName) + ginkgo.By("Deleting share ovn snat " + sharedEipSnatName) + ovnSnatRuleClient.DeleteSync(sharedEipSnatName) + vipClient.DeleteSync(arpProxyVip2Name) ginkgo.By("Deleting ovn vip " + dnatVipName) vipClient.DeleteSync(dnatVipName) @@ -323,16 +332,13 @@ var _ = framework.Describe("[group:ovn-vpc-nat-gw]", func() { ginkgo.By("Deleting subnet " + noBfdSubnetName) subnetClient.DeleteSync(noBfdSubnetName) - ginkgo.By("Deleting subnet " + bfdSubnetName) subnetClient.DeleteSync(bfdSubnetName) - ginkgo.By("Deleting underlay subnet " + underlaySubnetName) subnetClient.DeleteSync(underlaySubnetName) ginkgo.By("Deleting no bfd custom vpc " + noBfdVpcName) vpcClient.DeleteSync(noBfdVpcName) - ginkgo.By("Deleting bfd custom vpc " + bfdVpcName) vpcClient.DeleteSync(bfdVpcName) @@ -489,16 +495,6 @@ var _ = framework.Describe("[group:ovn-vpc-nat-gw]", func() { framework.ExpectContainSubstring(vlanSubnetGw, route.NextHopIP) } - // clean up share eip case resource - ginkgo.By("Deleting share ovn fip " + sharedEipFipShoudOkName) - ovnFipClient.DeleteSync(sharedEipFipShoudOkName) - ginkgo.By("Deleting share ovn fip " + sharedEipFipShoudFailName) - ovnFipClient.DeleteSync(sharedEipFipShoudFailName) - ginkgo.By("Deleting share ovn dnat " + sharedEipDnatName) - ovnDnatRuleClient.DeleteSync(sharedEipDnatName) - ginkgo.By("Deleting share ovn snat " + sharedEipSnatName) - ovnSnatRuleClient.DeleteSync(sharedEipSnatName) - ginkgo.By("2. Creating custom vpc enable external and bfd") for _, nodeName := range nodeNames { ginkgo.By("Creating ovn node-ext-gw type eip on node " + nodeName)