From 2652bcfec145a821df9b5677a758ea422dd4824a Mon Sep 17 00:00:00 2001 From: bobz965 Date: Tue, 29 Nov 2022 16:16:40 +0800 Subject: [PATCH] fix: slow vip finalizer operation (#2092) Co-authored-by: zhangbingbing --- dist/images/install.sh | 8 +- pkg/controller/controller.go | 6 +- pkg/controller/init.go | 2 +- pkg/controller/pod.go | 2 +- pkg/controller/vip.go | 165 +++++++++++++++++------------------ 5 files changed, 91 insertions(+), 92 deletions(-) diff --git a/dist/images/install.sh b/dist/images/install.sh index 038d35c7618..ff1995945e2 100755 --- a/dist/images/install.sh +++ b/dist/images/install.sh @@ -1190,19 +1190,19 @@ spec: additionalPrinterColumns: - name: V4IP type: string - jsonPath: .spec.v4ip + jsonPath: .status.v4ip - name: PV4IP type: string jsonPath: .spec.parentV4ip - name: Mac type: string - jsonPath: .spec.macAddress + jsonPath: .status.mac - name: PMac type: string - jsonPath: .spec.ParentMac + jsonPath: .spec.parentMac - name: V6IP type: string - jsonPath: .spec.v6ip + jsonPath: .status.v6ip - name: PV6IP type: string jsonPath: .spec.parentV6ip diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 75bfdea961f..21e26a72d14 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -295,9 +295,9 @@ func NewController(config *Configuration) *Controller { virtualIpsLister: virtualIpInformer.Lister(), virtualIpsSynced: virtualIpInformer.Informer().HasSynced, - addVirtualIpQueue: workqueue.NewNamedRateLimitingQueue(custCrdRateLimiter, "addVirtualIp"), - updateVirtualIpQueue: workqueue.NewNamedRateLimitingQueue(custCrdRateLimiter, "updateVirtualIp"), - delVirtualIpQueue: workqueue.NewNamedRateLimitingQueue(custCrdRateLimiter, "delVirtualIp"), + addVirtualIpQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "addVirtualIp"), + updateVirtualIpQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "updateVirtualIp"), + delVirtualIpQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "delVirtualIp"), iptablesEipsLister: iptablesEipInformer.Lister(), iptablesEipSynced: iptablesEipInformer.Informer().HasSynced, diff --git a/pkg/controller/init.go b/pkg/controller/init.go index 2ba6271762d..2a0abebc11d 100644 --- a/pkg/controller/init.go +++ b/pkg/controller/init.go @@ -404,7 +404,7 @@ func (c *Controller) InitIPAM() error { } else { ipamKey = vip.Name } - if _, _, _, err = c.ipam.GetStaticAddress(ipamKey, vip.Name, vip.Spec.V4ip, vip.Spec.MacAddress, vip.Spec.Subnet, false); err != nil { + if _, _, _, err = c.ipam.GetStaticAddress(ipamKey, vip.Name, vip.Status.V4ip, vip.Status.Mac, vip.Spec.Subnet, true); err != nil { klog.Errorf("failed to init ipam from vip cr %s: %v", vip.Name, err) } } diff --git a/pkg/controller/pod.go b/pkg/controller/pod.go index 69e946e3ff9..e5552d93bb7 100644 --- a/pkg/controller/pod.go +++ b/pkg/controller/pod.go @@ -1229,7 +1229,7 @@ func (c *Controller) acquireAddress(pod *v1.Pod, podNet *kubeovnNet) (string, st if err = c.podReuseVip(vipName, portName, isStsPod); err != nil { return "", "", "", podNet.Subnet, err } - return vip.Spec.V4ip, vip.Spec.V6ip, vip.Spec.MacAddress, podNet.Subnet, nil + return vip.Status.V4ip, vip.Status.V6ip, vip.Status.Mac, podNet.Subnet, nil } macStr := pod.Annotations[fmt.Sprintf(util.MacAddressAnnotationTemplate, podNet.ProviderName)] diff --git a/pkg/controller/vip.go b/pkg/controller/vip.go index 0473ec1b7df..314f8ae89cc 100644 --- a/pkg/controller/vip.go +++ b/pkg/controller/vip.go @@ -6,7 +6,6 @@ import ( "fmt" "net" "strings" - "time" kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1" "github.com/kubeovn/kube-ovn/pkg/ovs" @@ -17,6 +16,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" ) func (c *Controller) enqueueAddVirtualIp(obj interface{}) { @@ -183,9 +183,9 @@ func (c *Controller) handleAddVirtualIp(key string) error { // already ok return nil } + klog.V(3).Infof("handle add vip %s", key) vip := cachedVip.DeepCopy() - klog.V(3).Infof("handle add vip %s", vip.Name) - var sourceV4Ip, v4ip, v6ip, mac, subnetName, parentV4ip, parentV6ip, parentMac string + var sourceV4Ip, v4ip, v6ip, mac, subnetName string subnetName = vip.Spec.Subnet if subnetName == "" { return fmt.Errorf("failed to create vip '%s', subnet should be set", key) @@ -205,6 +205,7 @@ func (c *Controller) handleAddVirtualIp(key string) error { if err != nil { return err } + var parentV4ip, parentV6ip, parentMac string if vip.Spec.ParentMac != "" { parentV4ip = vip.Spec.ParentV4ip parentV6ip = vip.Spec.ParentV6ip @@ -214,10 +215,6 @@ func (c *Controller) handleAddVirtualIp(key string) error { klog.Errorf("failed to create or update vip '%s', %v", vip.Name, err) return err } - if _, err = c.handleVipFinalizer(vip); err != nil { - klog.Errorf("failed to handle vip finalizer, %v", err) - return err - } if err = c.subnetCountIp(subnet); err != nil { klog.Errorf("failed to count vip '%s' in subnet, %v", vip.Name, err) return err @@ -237,25 +234,23 @@ func (c *Controller) handleUpdateVirtualIp(key string) error { // should delete if !vip.DeletionTimestamp.IsZero() { // TODO:// clean vip in its parent port aap list - // klog.V(3).Infof("todo:// remove this vip '%s' from its parent port aap list", key) - v4ip := "" - ready := false - if err = c.patchVipStatus(key, v4ip, ready); err != nil { - return err - } - vip.Status.Ready = false - if _, err = c.handleVipFinalizer(vip); err != nil { + if err = c.handleDelVipFinalizer(key); err != nil { klog.Errorf("failed to handle vip finalizer %v", err) return err } return nil } - if vip.Status.Mac == "" || - vip.Status.Mac != vip.Spec.MacAddress || - vip.Status.Pmac != vip.Spec.ParentMac { + // not support change ip + if vip.Status.Mac != "" && vip.Status.Mac != vip.Spec.MacAddress || + vip.Status.V4ip != "" && vip.Status.V4ip != vip.Spec.V4ip || + vip.Status.V6ip != "" && vip.Status.V6ip != vip.Spec.V6ip { + err = fmt.Errorf("not support change ip of vip") + klog.Errorf("%v", err) + return err + } + // should update + if vip.Status.Mac == "" && vip.Spec.MacAddress == "" { // TODO:// add vip in its parent port aap list - // klog.V(3).Infof("todo:// add this vip '%s' into its parent port aap list", key) - if err = c.createOrUpdateCrdVip(key, vip.Namespace, vip.Spec.Subnet, vip.Spec.V6ip, vip.Spec.V6ip, vip.Spec.MacAddress, vip.Spec.ParentV4ip, vip.Spec.ParentV6ip, vip.Spec.MacAddress); err != nil { @@ -265,11 +260,10 @@ func (c *Controller) handleUpdateVirtualIp(key string) error { if err = c.patchVipStatus(key, vip.Spec.V4ip, ready); err != nil { return err } - } - vip.Status.Ready = true - if _, err = c.handleVipFinalizer(vip); err != nil { - klog.Errorf("failed to handle vip finalizer %v", err) - return err + if err = c.handleAddVipFinalizer(key); err != nil { + klog.Errorf("failed to handle vip finalizer %v", err) + return err + } } return nil } @@ -340,7 +334,7 @@ func (c *Controller) createOrUpdateCrdVip(key, ns, subnet, v4ip, v6ip, mac, pV4i vipCr, err := c.config.KubeOvnClient.KubeovnV1().Vips().Get(context.Background(), key, metav1.GetOptions{}) if err != nil { if k8serrors.IsNotFound(err) { - _, err := c.config.KubeOvnClient.KubeovnV1().Vips().Create(context.Background(), &kubeovnv1.Vip{ + if _, err := c.config.KubeOvnClient.KubeovnV1().Vips().Create(context.Background(), &kubeovnv1.Vip{ ObjectMeta: metav1.ObjectMeta{ Name: key, Labels: map[string]string{ @@ -359,18 +353,7 @@ func (c *Controller) createOrUpdateCrdVip(key, ns, subnet, v4ip, v6ip, mac, pV4i ParentV6ip: pV6ip, ParentMac: pmac, }, - Status: kubeovnv1.VipStatus{ - Ready: true, - V4ip: v4ip, - V6ip: v6ip, - Mac: mac, - Pv4ip: pV4ip, - Pv6ip: pV6ip, - Pmac: pmac, - }, - }, metav1.CreateOptions{}) - - if err != nil { + }, metav1.CreateOptions{}); err != nil { errMsg := fmt.Errorf("failed to create crd vip '%s', %v", key, err) klog.Error(errMsg) return errMsg @@ -384,9 +367,7 @@ func (c *Controller) createOrUpdateCrdVip(key, ns, subnet, v4ip, v6ip, mac, pV4i vip := vipCr.DeepCopy() if vip.Spec.MacAddress == "" && mac != "" { // vip not support to update, just delete and create - vip.ObjectMeta.Namespace = ns vip.Spec.Namespace = ns - vip.Spec.Subnet = subnet vip.Spec.V4ip = v4ip vip.Spec.V6ip = v6ip vip.Spec.MacAddress = mac @@ -401,7 +382,6 @@ func (c *Controller) createOrUpdateCrdVip(key, ns, subnet, v4ip, v6ip, mac, pV4i vip.Status.Pv4ip = pV4ip vip.Status.Pv6ip = pV6ip vip.Status.Pmac = pmac - if _, err := c.config.KubeOvnClient.KubeovnV1().Vips().Update(context.Background(), vip, metav1.UpdateOptions{}); err != nil { errMsg := fmt.Errorf("failed to update vip '%s', %v", key, err) klog.Error(errMsg) @@ -437,47 +417,6 @@ func (c *Controller) createOrUpdateCrdVip(key, ns, subnet, v4ip, v6ip, mac, pV4i return nil } -func (c *Controller) handleVipFinalizer(vip *kubeovnv1.Vip) (bool, error) { - if vip.DeletionTimestamp.IsZero() && !util.ContainsString(vip.Finalizers, util.ControllerName) { - if len(vip.Finalizers) != 0 { - return false, nil - } - vip.Finalizers = append(vip.Finalizers, util.ControllerName) - raw, _ := json.Marshal(vip.Finalizers) - patchPayloadTemplate := `[{ "op": "add", "path": "/metadata/finalizers", "value": %s }]` - patchPayload := fmt.Sprintf(patchPayloadTemplate, raw) - if _, err := c.config.KubeOvnClient.KubeovnV1().Vips().Patch(context.Background(), vip.Name, types.JSONPatchType, []byte(patchPayload), metav1.PatchOptions{}); err != nil { - if k8serrors.IsNotFound(err) { - return true, nil - } - klog.Errorf("failed to add finalizer for vip '%s', %v", vip.Name, err) - return false, err - } - // wait local cache ready - time.Sleep(2 * time.Second) - return false, nil - } - - if !vip.DeletionTimestamp.IsZero() && !vip.Status.Ready { - if len(vip.Finalizers) == 0 { - return true, nil - } - vip.Finalizers = util.RemoveString(vip.Finalizers, util.ControllerName) - raw, _ := json.Marshal(vip.Finalizers) - patchPayloadTemplate := `[{ "op": "remove", "path": "/metadata/finalizers", "value": %s }]` - patchPayload := fmt.Sprintf(patchPayloadTemplate, raw) - if _, err := c.config.KubeOvnClient.KubeovnV1().Vips().Patch(context.Background(), vip.Name, types.JSONPatchType, []byte(patchPayload), metav1.PatchOptions{}); err != nil { - if k8serrors.IsNotFound(err) { - return true, nil - } - klog.Errorf("failed to remove finalizer from vip '%s', %v", vip.Name, err) - return false, err - } - return true, nil - } - return false, nil -} - func (c *Controller) patchVipStatus(key, v4ip string, ready bool) error { oriVip, err := c.virtualIpsLister.Get(key) if err != nil { @@ -572,10 +511,70 @@ func (c *Controller) releaseVip(key string) error { klog.Errorf("failed to patch label for vip '%s', %v", vip.Name, err) return err } - if _, _, _, err = c.ipam.GetStaticAddress(key, vip.Name, vip.Spec.V4ip, vip.Spec.MacAddress, vip.Spec.Subnet, false); err != nil { + if _, _, _, err = c.ipam.GetStaticAddress(key, vip.Name, vip.Status.V4ip, vip.Status.Mac, vip.Spec.Subnet, false); err != nil { klog.Errorf("failed to recover IPAM from VIP CR %s: %v", vip.Name, err) } c.updateSubnetStatusQueue.Add(vip.Spec.Subnet) } return nil } + +func (c *Controller) handleAddVipFinalizer(key string) error { + cachedVip, err := c.virtualIpsLister.Get(key) + if err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + return err + } + if cachedVip.DeletionTimestamp.IsZero() { + if util.ContainsString(cachedVip.Finalizers, util.ControllerName) { + return nil + } + } + newVip := cachedVip.DeepCopy() + controllerutil.AddFinalizer(newVip, util.ControllerName) + patch, err := util.GenerateMergePatchPayload(cachedVip, newVip) + if err != nil { + klog.Errorf("failed to generate patch payload for ovn eip '%s', %v", cachedVip.Name, err) + return err + } + if _, err := c.config.KubeOvnClient.KubeovnV1().Vips().Patch(context.Background(), cachedVip.Name, + types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to add finalizer for vip '%s', %v", cachedVip.Name, err) + return err + } + return nil +} + +func (c *Controller) handleDelVipFinalizer(key string) error { + cachedVip, err := c.virtualIpsLister.Get(key) + if err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + return err + } + if len(cachedVip.Finalizers) == 0 { + return nil + } + newVip := cachedVip.DeepCopy() + controllerutil.RemoveFinalizer(newVip, util.ControllerName) + patch, err := util.GenerateMergePatchPayload(cachedVip, newVip) + if err != nil { + klog.Errorf("failed to generate patch payload for ovn eip '%s', %v", cachedVip.Name, err) + return err + } + if _, err := c.config.KubeOvnClient.KubeovnV1().Vips().Patch(context.Background(), cachedVip.Name, + types.MergePatchType, patch, metav1.PatchOptions{}, ""); err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to remove finalizer from vip '%s', %v", cachedVip.Name, err) + return err + } + return nil +}