From a5063c80775c8d106c5c61ed8db0f55883fddd9e Mon Sep 17 00:00:00 2001 From: bobz965 Date: Tue, 28 Nov 2023 14:05:50 +0800 Subject: [PATCH] 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 --- Makefile | 4 ++-- pkg/controller/gc.go | 28 +++++++++++++++++++------ pkg/controller/node.go | 2 +- pkg/controller/ovn_eip.go | 2 +- pkg/controller/pod.go | 4 ++-- pkg/controller/subnet.go | 4 ++-- pkg/controller/vip.go | 4 ++-- pkg/controller/vpc_nat_gw_eip.go | 9 ++++---- pkg/ipam/ipam.go | 12 ++++++++--- test/unittest/ipam/ipam.go | 30 +++++++++++++-------------- test/unittest/ipam_bench/ipam_test.go | 4 ++-- 11 files changed, 63 insertions(+), 40 deletions(-) diff --git a/Makefile b/Makefile index ae0f0a427f4..d4fd6f9c2d3 100644 --- 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 index 35e965c7398..a26ee18b364 100644 --- 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 index 442e5860969..4a7055a8238 100644 --- 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 index eae4773fd53..411f43c2070 100644 --- 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 index fd8174f3017..15f1e2eafa6 100644 --- 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 index d8f398c0aaf..f28d8ab49ef 100644 --- 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 index ed9a958920c..730ad78c939 100644 --- 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 index d15fc10f17f..98c82db9ec5 100644 --- a/pkg/controller/vpc_nat_gw_eip.go +++ b/pkg/controller/vpc_nat_gw_eip.go @@ -3,6 +3,9 @@ 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" @@ -10,9 +13,7 @@ import ( 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 index 449b9819218..9d99e7b34f2 100644 --- 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 index 6227d3b75c7..ee4c8e0553b 100644 --- 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 index f6f177eb886..68c46360b12 100644 --- 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, "") } } })