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

…nic (#3451) * fix: ipam clean all po nic ip address and mac even if just delete a nic Signed-off-by: bobz965 <[email protected]>
kubeovn · Nov 28, 2023 · a5063c8 · a5063c8
1 parent 1dd86ea
commit a5063c8
Show file tree

Hide file tree

Showing 11 changed files with 63 additions and 40 deletions.
diff --git a/Makefile b/Makefile
@@ -567,8 +567,8 @@ lint-windows:
 
 .PHONY: scan
 scan:
-	trivy image --exit-code=1 --ignore-unfixed --security-checks vuln $(REGISTRY)/kube-ovn:$(RELEASE_TAG)
-	trivy image --exit-code=1 --ignore-unfixed --security-checks vuln $(REGISTRY)/vpc-nat-gateway:$(RELEASE_TAG)
+	trivy image --exit-code=1 --ignore-unfixed --scanners vuln $(REGISTRY)/kube-ovn:$(RELEASE_TAG)
+	trivy image --exit-code=1 --ignore-unfixed --scanners vuln $(REGISTRY)/vpc-nat-gateway:$(RELEASE_TAG)
 
 .PHONY: ut
 ut:

diff --git a/pkg/controller/gc.go b/pkg/controller/gc.go
@@ -350,15 +350,31 @@ func (c *Controller) markAndCleanLSP() error {
 			klog.Errorf("failed to delete lsp %s, %v", lsp, 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
+		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
+		}
+		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

diff --git a/pkg/controller/node.go b/pkg/controller/node.go
@@ -490,7 +490,7 @@ func (c *Controller) handleDeleteNode(key string) error {
 		return err
 	}
 
-	c.ipam.ReleaseAddressByPod(portName)
+	c.ipam.ReleaseAddressByPod(portName, c.config.NodeSwitch)
 
 	providerNetworks, err := c.providerNetworksLister.List(labels.Everything())
 	if err != nil && !k8serrors.IsNotFound(err) {

diff --git a/pkg/controller/ovn_eip.go b/pkg/controller/ovn_eip.go
@@ -299,7 +299,7 @@ func (c *Controller) handleResetOvnEip(key string) error {
 
 func (c *Controller) handleDelOvnEip(key string) error {
 	klog.V(3).Infof("release ovn eip %s", key)
-	c.ipam.ReleaseAddressByPod(key)
+	c.ipam.ReleaseAddressByPod(key, "")
 	return nil
 }
 

diff --git a/pkg/controller/pod.go b/pkg/controller/pod.go
@@ -830,8 +830,8 @@ func (c *Controller) handleDeletePod(pod *v1.Pod) error {
 			c.syncSgPortsQueue.Add(sg)
 		}
 	}
-
-	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 {

diff --git a/pkg/controller/subnet.go b/pkg/controller/subnet.go
@@ -1565,7 +1565,7 @@ 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)
+				c.ipam.ReleaseAddressByPod(u2oInterconnName, subnet.Name)
 			}
 
 			v4ip, v6ip, _, err = c.acquireStaticIpAddress(subnet.Name, u2oInterconnName, u2oInterconnLrpName, subnet.Spec.U2OInterconnectionIP)
@@ -1594,7 +1594,7 @@ 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)
+			c.ipam.ReleaseAddressByPod(u2oInterconnName, subnet.Name)
 			subnet.Status.U2OInterconnectionIP = ""
 
 			if err := c.config.KubeOvnClient.KubeovnV1().IPs().Delete(context.Background(), u2oInterconnName, metav1.DeleteOptions{}); err != nil {

diff --git a/pkg/controller/vip.go b/pkg/controller/vip.go
@@ -262,7 +262,7 @@ func (c *Controller) handleUpdateVirtualIp(key string) error {
 
 func (c *Controller) handleDelVirtualIp(key string) error {
 	klog.V(3).Infof("release vip %s", key)
-	c.ipam.ReleaseAddressByPod(key)
+	c.ipam.ReleaseAddressByPod(key, "")
 	return nil
 }
 
@@ -468,7 +468,7 @@ func (c *Controller) podReuseVip(key, portName string, isStsPod 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
 }

diff --git a/pkg/controller/vpc_nat_gw_eip.go b/pkg/controller/vpc_nat_gw_eip.go
@@ -3,16 +3,17 @@ package controller
 import (
 	"context"
 	"fmt"
+	"net"
+	"strings"
+
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/types"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/klog/v2"
-	"net"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
-	"strings"
 
 	kubeovnv1 "github.com/kubeovn/kube-ovn/pkg/apis/kubeovn/v1"
 	"github.com/kubeovn/kube-ovn/pkg/ovs"
@@ -393,8 +394,8 @@ 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.V(3).Infof("delete vpc nat eip %s", key)
+	c.ipam.ReleaseAddressByPod(key, "")
 	return nil
 }
 

diff --git a/pkg/ipam/ipam.go b/pkg/ipam/ipam.go
@@ -111,11 +111,17 @@ func checkAndAppendIpsForDual(ips []IP, mac string, podName string, nicName stri
 	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)
+		}
 	}
 }
 

diff --git a/test/unittest/ipam/ipam.go b/test/unittest/ipam/ipam.go
@@ -116,12 +116,12 @@ var _ = Describe("[IPAM]", func() {
 				Expect(err).Should(MatchError(ipam.ErrConflict))
 
 				By("release pod with multiple nics")
-				im.ReleaseAddressByPod(pod2)
+				im.ReleaseAddressByPod(pod2, "")
 				Expect(im.Subnets[subnetName].V4ReleasedIPList.Contains(ipam.IP(freeIp2))).Should(BeTrue())
 				Expect(im.Subnets[subnetName].V4ReleasedIPList.Contains(ipam.IP(freeIp3))).Should(BeTrue())
 
 				By("release pod with single nic")
-				im.ReleaseAddressByPod(pod1)
+				im.ReleaseAddressByPod(pod1, "")
 				Expect(im.Subnets[subnetName].V4ReleasedIPList.Contains(ipam.IP(freeIp1))).Should(BeTrue())
 
 				By("create new pod with released ips")
@@ -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", "", 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", "", 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,12 +266,12 @@ var _ = Describe("[IPAM]", func() {
 				Expect(err).Should(MatchError(ipam.ErrConflict))
 
 				By("release pod with multiple nics")
-				im.ReleaseAddressByPod(pod2)
+				im.ReleaseAddressByPod(pod2, "")
 				Expect(im.Subnets[subnetName].V6ReleasedIPList.Contains(ipam.IP(freeIp2))).Should(BeTrue())
 				Expect(im.Subnets[subnetName].V6ReleasedIPList.Contains(ipam.IP(freeIp3))).Should(BeTrue())
 
 				By("release pod with single nic")
-				im.ReleaseAddressByPod(pod1)
+				im.ReleaseAddressByPod(pod1, "")
 				Expect(im.Subnets[subnetName].V6ReleasedIPList.Contains(ipam.IP(freeIp1))).Should(BeTrue())
 
 				By("create new pod with released ips")
@@ -315,12 +315,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", "", 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", "", subnetName, nil, true)
 				Expect(err).ShouldNot(HaveOccurred())
 				Expect(ip).To(Equal("fd00::1"))
@@ -335,7 +335,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())
 
@@ -434,14 +434,14 @@ var _ = Describe("[IPAM]", func() {
 				Expect(err).Should(MatchError(ipam.ErrConflict))
 
 				By("release pod with multiple nics")
-				im.ReleaseAddressByPod(pod2)
+				im.ReleaseAddressByPod(pod2, "")
 				Expect(im.Subnets[subnetName].V4ReleasedIPList.Contains(ipam.IP(freeIp42))).Should(BeTrue())
 				Expect(im.Subnets[subnetName].V4ReleasedIPList.Contains(ipam.IP(freeIp43))).Should(BeTrue())
 				Expect(im.Subnets[subnetName].V6ReleasedIPList.Contains(ipam.IP(freeIp62))).Should(BeTrue())
 				Expect(im.Subnets[subnetName].V6ReleasedIPList.Contains(ipam.IP(freeIp63))).Should(BeTrue())
 
 				By("release pod with single nic")
-				im.ReleaseAddressByPod(pod1)
+				im.ReleaseAddressByPod(pod1, "")
 				Expect(im.Subnets[subnetName].V4ReleasedIPList.Contains(ipam.IP(freeIp41))).Should(BeTrue())
 				Expect(im.Subnets[subnetName].V6ReleasedIPList.Contains(ipam.IP(freeIp61))).Should(BeTrue())
 
@@ -487,13 +487,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", "", 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", "", subnetName, nil, true)
 				Expect(err).ShouldNot(HaveOccurred())
 				Expect(ipv4).To(Equal("10.16.0.1"))
@@ -510,7 +510,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())
 

diff --git a/test/unittest/ipam_bench/ipam_test.go b/test/unittest/ipam_bench/ipam_test.go
@@ -233,7 +233,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, "")
 	}
 }
 
@@ -331,7 +331,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, "")
 			}
 		}
 	})