fix: ipam clean all pod nic ip address and mac even if just delete a nic

Signed-off-by: bobz965 <[email protected]>

pkg/controller/gc.go

@@ -353,20 +353,31 @@ func (c *Controller) markAndCleanLSP() error {
 		}
 
 		klog.Infof("gc logical switch port %s", lsp.Name)
-		if err := c.OVNNbClient.DeleteLogicalSwitchPort(lsp.Name); err != nil {
-			klog.Errorf("failed to delete lsp %s: %v", lsp.Name, err)
+		ipCr, err := c.config.KubeOvnClient.KubeovnV1().IPs().Get(context.Background(), lsp.Name, metav1.GetOptions{})
+		if err != nil {
+			if k8serrors.IsNotFound(err) {
+				// ip cr not found, skip lsp gc
+				continue
+			}
+			klog.Errorf("failed to get ip %s, %v", lsp.Name, err)
 			return err
 		}
-
-		if err := c.config.KubeOvnClient.KubeovnV1().IPs().Delete(context.Background(), lsp.Name, metav1.DeleteOptions{}); err != nil {
-			if !k8serrors.IsNotFound(err) {
-				klog.Errorf("failed to delete ip %s, %v", lsp.Name, err)
-				return err
+		klog.Infof("gc ip %s", ipCr.Name)
+		if err := c.config.KubeOvnClient.KubeovnV1().IPs().Delete(context.Background(), ipCr.Name, metav1.DeleteOptions{}); err != nil {
+			if k8serrors.IsNotFound(err) {
+				// ip cr not found, skip lsp gc
+				continue
 			}
+			klog.Errorf("failed to delete ip %s, %v", ipCr.Name, err)
+			return err
+		}
+		if ipCr.Spec.Subnet == "" {
+			klog.Errorf("ip %s has no subnet", ipCr.Name)
+			// ip cr no subnet, skip lsp gc
+			continue
 		}
-
 		if key := lsp.ExternalIDs["pod"]; key != "" {
-			c.ipam.ReleaseAddressByPod(key)
+			c.ipam.ReleaseAddressByPod(key, ipCr.Spec.Subnet)
 		}
 	}
 	lastNoPodLSP = noPodLSP

pkg/controller/node.go

@@ -516,7 +516,8 @@ func (c *Controller) handleDeleteNode(key string) error {
 		return err
 	}
 
-	c.ipam.ReleaseAddressByPod(portName)
+	klog.Infof("release node port %s", portName)
+	c.ipam.ReleaseAddressByPod(portName, c.config.NodeSwitch)
 
 	providerNetworks, err := c.providerNetworksLister.List(labels.Everything())
 	if err != nil && !k8serrors.IsNotFound(err) {

pkg/controller/ovn_eip.go

@@ -358,7 +358,7 @@ func (c *Controller) handleDelOvnEip(key string) error {
 		}
 	}
 
-	c.ipam.ReleaseAddressByPod(eip.Name)
+	c.ipam.ReleaseAddressByPod(eip.Name, eip.Spec.ExternalSubnet)
 	c.updateSubnetStatusQueue.Add(eip.Spec.ExternalSubnet)
 	return nil
 }

pkg/controller/pod.go

@@ -568,7 +568,8 @@ func (c *Controller) changeVMSubnet(vmName, namespace, providerName, subnetName
 					return err
 				}
 			}
-			c.ipam.ReleaseAddressByPod(key)
+			klog.Infof("after changing subnet, release ip %s", ipName)
+			c.ipam.ReleaseAddressByPod(key, ipCr.Spec.Subnet)
 		}
 	}
 	return nil
@@ -1082,8 +1083,8 @@ func (c *Controller) handleDeletePod(key string) error {
 			return err
 		}
 	}
-
-	c.ipam.ReleaseAddressByPod(key)
+	klog.Infof("release all ip address for deleting pod %s", key)
+	c.ipam.ReleaseAddressByPod(key, "")
 
 	podNets, err := c.getPodKubeovnNets(pod)
 	if err != nil {

pkg/controller/subnet.go

@@ -1920,7 +1920,8 @@ func (c *Controller) reconcileU2OInterconnectionIP(subnet *kubeovnv1.Subnet) err
 			}
 		} else if subnet.Spec.U2OInterconnectionIP != "" && subnet.Status.U2OInterconnectionIP != subnet.Spec.U2OInterconnectionIP {
 			if subnet.Status.U2OInterconnectionIP != "" {
-				c.ipam.ReleaseAddressByPod(u2oInterconnName)
+				klog.Infof("release underlay to overlay interconnection ip address %s for subnet %s", subnet.Status.U2OInterconnectionIP, subnet.Name)
+				c.ipam.ReleaseAddressByPod(u2oInterconnName, subnet.Name)
 			}
 
 			v4ip, v6ip, _, err = c.acquireStaticIPAddress(subnet.Name, u2oInterconnName, u2oInterconnLrpName, subnet.Spec.U2OInterconnectionIP)
@@ -1948,7 +1949,8 @@ func (c *Controller) reconcileU2OInterconnectionIP(subnet *kubeovnv1.Subnet) err
 		}
 	} else if subnet.Status.U2OInterconnectionIP != "" {
 		u2oInterconnName := fmt.Sprintf(util.U2OInterconnName, subnet.Spec.Vpc, subnet.Name)
-		c.ipam.ReleaseAddressByPod(u2oInterconnName)
+		klog.Infof("release underlay to overlay interconnection ip address %s for subnet %s", subnet.Status.U2OInterconnectionIP, subnet.Name)
+		c.ipam.ReleaseAddressByPod(u2oInterconnName, subnet.Name)
 		subnet.Status.U2OInterconnectionIP = ""
 
 		if err := c.config.KubeOvnClient.KubeovnV1().IPs().Delete(context.Background(), u2oInterconnName, metav1.DeleteOptions{}); err != nil {

pkg/controller/vip.go

@@ -358,7 +358,7 @@ func (c *Controller) handleDelVirtualIP(vip *kubeovnv1.Vip) error {
 		klog.Errorf("delete virtual logical switch port %s from logical switch %s: %v", vip.Name, vip.Spec.Subnet, err)
 		return err
 	}
-	c.ipam.ReleaseAddressByPod(vip.Name)
+	c.ipam.ReleaseAddressByPod(vip.Name, vip.Spec.Subnet)
 	c.updateSubnetStatusQueue.Add(vip.Spec.Subnet)
 	return nil
 }
@@ -635,7 +635,7 @@ func (c *Controller) podReuseVip(key, portName string, keepVIP bool) error {
 		klog.Errorf("failed to patch label for vip '%s', %v", vip.Name, err)
 		return err
 	}
-	c.ipam.ReleaseAddressByPod(key)
+	c.ipam.ReleaseAddressByPod(key, vip.Spec.Subnet)
 	c.updateSubnetStatusQueue.Add(vip.Spec.Subnet)
 	return nil
 }

pkg/controller/vpc_nat_gw_eip.go

@@ -314,13 +314,9 @@ func (c *Controller) handleUpdateIptablesEip(key string) error {
 				return err
 			}
 		}
-		if err = c.handleDelIptablesEipFinalizer(key); err != nil {
-			klog.Errorf("failed to handle del finalizer for eip %s, %v", key, err)
-			return err
-		}
 		return nil
 	}
-	klog.V(3).Infof("handle update eip %s", key)
+	klog.Infof("handle update eip %s", key)
 	// eip change ip
 	if c.eipChangeIP(cachedEip) {
 		err := fmt.Errorf("not support eip change ip, old ip '%s', new ip '%s'", cachedEip.Status.IP, cachedEip.Spec.V4ip)
@@ -400,8 +396,20 @@ func (c *Controller) handleUpdateIptablesEip(key string) error {
 }
 
 func (c *Controller) handleDelIptablesEip(key string) error {
-	c.ipam.ReleaseAddressByPod(key)
-	klog.V(3).Infof("deleted vpc nat eip %s", key)
+	klog.Infof("handle delete iptables eip %s", key)
+	eip, err := c.iptablesEipsLister.Get(key)
+	if err != nil {
+		if k8serrors.IsNotFound(err) {
+			return nil
+		}
+		klog.Error(err)
+		return err
+	}
+	if err = c.handleDelIptablesEipFinalizer(key); err != nil {
+		klog.Errorf("failed to handle del finalizer for eip %s, %v", key, err)
+		return err
+	}
+	c.ipam.ReleaseAddressByPod(key, eip.Spec.ExternalSubnet)
 	return nil
 }
 

pkg/ipam/ipam.go

@@ -139,11 +139,17 @@ func checkAndAppendIpsForDual(ips []IP, mac, podName, nicName string, subnet *Su
 	return newIps, err
 }
 
-func (ipam *IPAM) ReleaseAddressByPod(podName string) {
+func (ipam *IPAM) ReleaseAddressByPod(podName, subnetName string) {
 	ipam.mutex.RLock()
 	defer ipam.mutex.RUnlock()
-	for _, subnet := range ipam.Subnets {
-		subnet.ReleaseAddress(podName)
+	if subnetName != "" {
+		if subnet, ok := ipam.Subnets[subnetName]; ok {
+			subnet.ReleaseAddress(podName)
+		}
+	} else {
+		for _, subnet := range ipam.Subnets {
+			subnet.ReleaseAddress(podName)
+		}
 	}
 }
 

test/unittest/ipam/ipam.go

@@ -111,7 +111,7 @@ var _ = Describe("[IPAM]", func() {
 				Expect(err).Should(MatchError(ipam.ErrConflict))
 
 				By("release pod with multiple nics")
-				im.ReleaseAddressByPod(pod2)
+				im.ReleaseAddressByPod(pod2, "")
 				ip2, err := ipam.NewIP(freeIP2)
 				Expect(err).ShouldNot(HaveOccurred())
 				ip3, err := ipam.NewIP(freeIP3)
@@ -120,7 +120,7 @@ var _ = Describe("[IPAM]", func() {
 				Expect(im.Subnets[subnetName].IPPools[""].V4Released.Contains(ip3)).Should(BeTrue())
 
 				By("release pod with single nic")
-				im.ReleaseAddressByPod(pod1)
+				im.ReleaseAddressByPod(pod1, "")
 				ip1, err := ipam.NewIP(freeIP1)
 				Expect(err).ShouldNot(HaveOccurred())
 				Expect(im.Subnets[subnetName].IPPools[""].V4Released.Contains(ip1)).To(BeTrue())
@@ -166,12 +166,12 @@ var _ = Describe("[IPAM]", func() {
 				Expect(err).ShouldNot(HaveOccurred())
 				Expect(ip).To(Equal("10.16.0.1"))
 
-				im.ReleaseAddressByPod("pod1.ns")
+				im.ReleaseAddressByPod("pod1.ns", "")
 				ip, _, _, err = im.GetRandomAddress("pod1.ns", "pod1.ns", nil, subnetName, "", nil, true)
 				Expect(err).ShouldNot(HaveOccurred())
 				Expect(ip).To(Equal("10.16.0.2"))
 
-				im.ReleaseAddressByPod("pod1.ns")
+				im.ReleaseAddressByPod("pod1.ns", "")
 				ip, _, _, err = im.GetRandomAddress("pod1.ns", "pod1.ns", nil, subnetName, "", nil, true)
 				Expect(err).ShouldNot(HaveOccurred())
 				Expect(ip).To(Equal("10.16.0.1"))
@@ -186,7 +186,7 @@ var _ = Describe("[IPAM]", func() {
 				Expect(err).ShouldNot(HaveOccurred())
 				Expect(ip).To(Equal("10.16.0.1"))
 
-				im.ReleaseAddressByPod("pod1.ns")
+				im.ReleaseAddressByPod("pod1.ns", "")
 				err = im.AddOrUpdateSubnet(subnetName, "10.16.0.0/30", v4Gw, []string{"10.16.0.1..10.16.0.2"})
 				Expect(err).ShouldNot(HaveOccurred())
 
@@ -266,7 +266,7 @@ var _ = Describe("[IPAM]", func() {
 				Expect(err).Should(MatchError(ipam.ErrConflict))
 
 				By("release pod with multiple nics")
-				im.ReleaseAddressByPod(pod2)
+				im.ReleaseAddressByPod(pod2, "")
 				ip2, err := ipam.NewIP(freeIP2)
 				Expect(err).ShouldNot(HaveOccurred())
 				ip3, err := ipam.NewIP(freeIP3)
@@ -275,7 +275,7 @@ var _ = Describe("[IPAM]", func() {
 				Expect(im.Subnets[subnetName].IPPools[""].V6Released.Contains(ip3)).Should(BeTrue())
 
 				By("release pod with single nic")
-				im.ReleaseAddressByPod(pod1)
+				im.ReleaseAddressByPod(pod1, "")
 				ip1, err := ipam.NewIP(freeIP1)
 				Expect(err).ShouldNot(HaveOccurred())
 				Expect(im.Subnets[subnetName].IPPools[""].V6Released.Contains(ip1)).Should(BeTrue())
@@ -321,12 +321,12 @@ var _ = Describe("[IPAM]", func() {
 				Expect(err).ShouldNot(HaveOccurred())
 				Expect(ip).To(Equal("fd00::1"))
 
-				im.ReleaseAddressByPod("pod1.ns")
+				im.ReleaseAddressByPod("pod1.ns", "")
 				_, ip, _, err = im.GetRandomAddress("pod1.ns", "pod1.ns", nil, subnetName, "", nil, true)
 				Expect(err).ShouldNot(HaveOccurred())
 				Expect(ip).To(Equal("fd00::2"))
 
-				im.ReleaseAddressByPod("pod1.ns")
+				im.ReleaseAddressByPod("pod1.ns", "")
 				_, ip, _, err = im.GetRandomAddress("pod1.ns", "pod1.ns", nil, subnetName, "", nil, true)
 				Expect(err).ShouldNot(HaveOccurred())
 				Expect(ip).To(Equal("fd00::1"))
@@ -341,7 +341,7 @@ var _ = Describe("[IPAM]", func() {
 				Expect(err).ShouldNot(HaveOccurred())
 				Expect(ip).To(Equal("fd00::1"))
 
-				im.ReleaseAddressByPod("pod1.ns")
+				im.ReleaseAddressByPod("pod1.ns", "")
 				err = im.AddOrUpdateSubnet(subnetName, "fd00::/126", v6Gw, []string{"fd00::1..fd00::2"})
 				Expect(err).ShouldNot(HaveOccurred())
 
@@ -440,7 +440,7 @@ var _ = Describe("[IPAM]", func() {
 				Expect(err).Should(MatchError(ipam.ErrConflict))
 
 				By("release pod with multiple nics")
-				im.ReleaseAddressByPod(pod2)
+				im.ReleaseAddressByPod(pod2, "")
 				ip42, err := ipam.NewIP(freeIP42)
 				Expect(err).ShouldNot(HaveOccurred())
 				ip43, err := ipam.NewIP(freeIP43)
@@ -455,7 +455,7 @@ var _ = Describe("[IPAM]", func() {
 				Expect(im.Subnets[subnetName].IPPools[""].V6Released.Contains(ip63)).Should(BeTrue())
 
 				By("release pod with single nic")
-				im.ReleaseAddressByPod(pod1)
+				im.ReleaseAddressByPod(pod1, "")
 				ip41, err := ipam.NewIP(freeIP41)
 				Expect(err).ShouldNot(HaveOccurred())
 				ip61, err := ipam.NewIP(freeIP61)
@@ -504,13 +504,13 @@ var _ = Describe("[IPAM]", func() {
 				Expect(ipv4).To(Equal("10.16.0.1"))
 				Expect(ipv6).To(Equal("fd00::1"))
 
-				im.ReleaseAddressByPod("pod1.ns")
+				im.ReleaseAddressByPod("pod1.ns", "")
 				ipv4, ipv6, _, err = im.GetRandomAddress("pod1.ns", "pod1.ns", nil, subnetName, "", nil, true)
 				Expect(err).ShouldNot(HaveOccurred())
 				Expect(ipv4).To(Equal("10.16.0.2"))
 				Expect(ipv6).To(Equal("fd00::2"))
 
-				im.ReleaseAddressByPod("pod1.ns")
+				im.ReleaseAddressByPod("pod1.ns", "")
 				ipv4, ipv6, _, err = im.GetRandomAddress("pod1.ns", "pod1.ns", nil, subnetName, "", nil, true)
 				Expect(err).ShouldNot(HaveOccurred())
 				Expect(ipv4).To(Equal("10.16.0.1"))
@@ -527,7 +527,7 @@ var _ = Describe("[IPAM]", func() {
 				Expect(ipv4).To(Equal("10.16.0.1"))
 				Expect(ipv6).To(Equal("fd00::1"))
 
-				im.ReleaseAddressByPod("pod1.ns")
+				im.ReleaseAddressByPod("pod1.ns", "")
 				err = im.AddOrUpdateSubnet(subnetName, "10.16.0.2/30,fd00::/126", dualGw, []string{"10.16.0.1..10.16.0.2", "fd00::1..fd00::2"})
 				Expect(err).ShouldNot(HaveOccurred())
 

test/unittest/ipam_bench/ipam_test.go

@@ -234,7 +234,7 @@ func delPodAddressCapacity(b *testing.B, im *ipam.IPAM, isTimeTrace bool) {
 			fmt.Printf("delete %d to %d pods spent time %ds \n", n+1-step, n, currentTime-startTime)
 			startTime = currentTime
 		}
-		im.ReleaseAddressByPod(podName)
+		im.ReleaseAddressByPod(podName, "")
 	}
 }
 
@@ -332,7 +332,7 @@ func benchmarkAllocFreeAddrParallel(b *testing.B, podNumber int, protocol string
 			}
 			for n := 0; n < podNumber; n++ {
 				podName := fmt.Sprintf("pod%d_%d", key, n)
-				im.ReleaseAddressByPod(podName)
+				im.ReleaseAddressByPod(podName, "")
 			}
 		}
 	})