From a9fdbf92ea09a2cfff184ecae4c72c632f0d213b Mon Sep 17 00:00:00 2001 From: Longchuanzheng Date: Fri, 4 Aug 2023 09:05:22 +0800 Subject: [PATCH] Fix relevant annotations are not deleted in hotnoplug nic process (#3108) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix relevant annotations are not deleted in hotnoplug nic process Signed-off-by: zhuanlan * change error log and update pod when patch Signed-off-by: zhuanlan * Determine whether the pod has changed before updating the pod Signed-off-by: zhuanlan * Apply suggestions from code review Thanks :) Co-authored-by: 张祖建 * Return patch result to use the latest object later Signed-off-by: zhuanlan --------- Signed-off-by: zhuanlan Co-authored-by: 张祖建 --- pkg/controller/pod.go | 39 ++++++++++++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/pkg/controller/pod.go b/pkg/controller/pod.go index fcd7ebdb595..24944151dcb 100644 --- a/pkg/controller/pod.go +++ b/pkg/controller/pod.go @@ -579,11 +579,15 @@ func (c *Controller) handleAddOrUpdatePod(key string) (err error) { } // check and do hotnoplug nic - if err = c.syncKubeOvnNet(pod, podNets); err != nil { + if cachedPod, err = c.syncKubeOvnNet(cachedPod, pod, podNets); err != nil { klog.Errorf("failed to sync pod nets %v", err) return err } - + if cachedPod == nil { + // pod has been deleted + return nil + } + pod = cachedPod.DeepCopy() // check if allocate subnet is need. also allocate subnet when hotplug nic needAllocatePodNets := needAllocateSubnets(pod, podNets) if len(needAllocatePodNets) != 0 { @@ -1080,7 +1084,7 @@ func (c *Controller) handleUpdatePodSecurity(key string) error { } return nil } -func (c *Controller) syncKubeOvnNet(pod *v1.Pod, podNets []*kubeovnNet) error { +func (c *Controller) syncKubeOvnNet(cachedPod, pod *v1.Pod, podNets []*kubeovnNet) (*v1.Pod, error) { podName := c.getNameByPod(pod) key := fmt.Sprintf("%s/%s", pod.Namespace, podName) targetPortNameList := strset.NewWithSize(len(podNets)) @@ -1096,7 +1100,7 @@ func (c *Controller) syncKubeOvnNet(pod *v1.Pod, podNets []*kubeovnNet) error { ports, err := c.ovnClient.ListNormalLogicalSwitchPorts(true, map[string]string{"pod": key}) if err != nil { klog.Errorf("failed to list lsps of pod '%s', %v", pod.Name, err) - return err + return nil, err } for _, port := range ports { @@ -1113,7 +1117,7 @@ func (c *Controller) syncKubeOvnNet(pod *v1.Pod, podNets []*kubeovnNet) error { } if len(portsNeedToDel) == 0 { - return nil + return pod, nil } for _, portNeedDel := range portsNeedToDel { @@ -1124,25 +1128,42 @@ func (c *Controller) syncKubeOvnNet(pod *v1.Pod, podNets []*kubeovnNet) error { if err := c.ovnClient.DeleteLogicalSwitchPort(portNeedDel); err != nil { klog.Errorf("failed to delete lsp %s, %v", portNeedDel, err) - return err + return nil, err } if err := c.config.KubeOvnClient.KubeovnV1().IPs().Delete(context.Background(), portNeedDel, metav1.DeleteOptions{}); err != nil { if !k8serrors.IsNotFound(err) { klog.Errorf("failed to delete ip %s, %v", portNeedDel, err) - return err + return nil, err } } } for _, providerName := range annotationsNeedToDel { for annotationKey := range pod.Annotations { - if strings.HasPrefix(key, providerName) { + if strings.HasPrefix(annotationKey, providerName) { delete(pod.Annotations, annotationKey) } } } + if len(cachedPod.Annotations) == len(pod.Annotations) { + return pod, nil + } + patch, err := util.GenerateMergePatchPayload(cachedPod, pod) + if err != nil { + klog.Errorf("failed to generate patch payload for pod '%s', %v", pod.Name, err) + return nil, err + } + patchedPod, err := c.config.KubeClient.CoreV1().Pods(pod.Namespace).Patch(context.Background(), pod.Name, + types.MergePatchType, patch, metav1.PatchOptions{}, "") + if err != nil { + if k8serrors.IsNotFound(err) { + return nil, nil + } + klog.Errorf("failed to delete useless annotations for pod %s: %v", pod.Name, err) + return nil, err + } - return nil + return patchedPod.DeepCopy(), nil } func isStatefulSetPod(pod *v1.Pod) (bool, string) {