From 7b28d1a601ecd567227f598bf933dbd8c0eea6b1 Mon Sep 17 00:00:00 2001 From: Icarus9913 Date: Wed, 1 Nov 2023 17:17:26 +0800 Subject: [PATCH] fix ipam block with ip address used out Signed-off-by: Icarus9913 --- pkg/ippoolmanager/ippool_manager.go | 40 +++++++++++------ pkg/ippoolmanager/ippool_manager_test.go | 56 ++++++++++++++++++++++++ pkg/ippoolmanager/utils.go | 14 ++++++ pkg/ippoolmanager/utils_test.go | 45 +++++++++++++++++++ 4 files changed, 141 insertions(+), 14 deletions(-) diff --git a/pkg/ippoolmanager/ippool_manager.go b/pkg/ippoolmanager/ippool_manager.go index 0c3bd7dd05..b8e23b2b9d 100644 --- a/pkg/ippoolmanager/ippool_manager.go +++ b/pkg/ippoolmanager/ippool_manager.go @@ -140,6 +140,20 @@ func (im *ipPoolManager) AllocateIP(ctx context.Context, poolName, nic string, p } func (im *ipPoolManager) genRandomIP(ctx context.Context, nic string, ipPool *spiderpoolv2beta1.SpiderIPPool, pod *corev1.Pod, podController types.PodTopController) (net.IP, error) { + logger := logutils.FromContext(ctx) + + var tmpPod *corev1.Pod + if im.config.EnableKubevirtStaticIP && podController.APIVersion == kubevirtv1.SchemeGroupVersion.String() && podController.Kind == constant.KindKubevirtVMI { + tmpPod = pod.DeepCopy() + tmpPod.SetName(podController.Name) + } else { + tmpPod = pod + } + key, err := cache.MetaNamespaceKeyFunc(tmpPod) + if err != nil { + return nil, err + } + reservedIPs, err := im.rIPManager.AssembleReservedIPs(ctx, *ipPool.Spec.IPVersion) if err != nil { return nil, err @@ -166,22 +180,20 @@ func (im *ipPoolManager) genRandomIP(ctx context.Context, nic string, ipPool *sp availableIPs := spiderpoolip.IPsDiffSet(totalIPs, append(reservedIPs, usedIPs...), false) if len(availableIPs) == 0 { - return nil, constant.ErrIPUsedOut - } - resIP := availableIPs[0] - - var tmpPod *corev1.Pod - if im.config.EnableKubevirtStaticIP && podController.APIVersion == kubevirtv1.SchemeGroupVersion.String() && podController.Kind == constant.KindKubevirtVMI { - tmpPod = pod.DeepCopy() - tmpPod.SetName(podController.Name) - } else { - tmpPod = pod - } + // traverse the usedIPs to find the previous allocated IPs if there be + // reference issue: https://github.com/spidernet-io/spiderpool/issues/2517 + allocatedIPFromRecords, hasFound := findAllocatedIPFromRecords(allocatedRecords, nic, key, string(pod.UID)) + if !hasFound { + return nil, constant.ErrIPUsedOut + } - key, err := cache.MetaNamespaceKeyFunc(tmpPod) - if err != nil { - return nil, err + availableIPs, err = spiderpoolip.ParseIPRange(*ipPool.Spec.IPVersion, allocatedIPFromRecords) + if nil != err { + return nil, err + } + logger.Sugar().Warnf("find previous IP '%s' from IPPool '%s' recorded IP allocations", allocatedIPFromRecords, ipPool.Name) } + resIP := availableIPs[0] if allocatedRecords == nil { allocatedRecords = spiderpoolv2beta1.PoolIPAllocations{} diff --git a/pkg/ippoolmanager/ippool_manager_test.go b/pkg/ippoolmanager/ippool_manager_test.go index 2415de070c..28ef298400 100644 --- a/pkg/ippoolmanager/ippool_manager_test.go +++ b/pkg/ippoolmanager/ippool_manager_test.go @@ -5,6 +5,7 @@ package ippoolmanager_test import ( "context" + "encoding/json" "fmt" "net" "sync/atomic" @@ -19,6 +20,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/uuid" + "k8s.io/client-go/tools/cache" "k8s.io/utils/pointer" kubevirtv1 "kubevirt.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -410,6 +412,60 @@ var _ = Describe("IPPoolManager", Label("ippool_manager_test"), func() { Expect(res.Gateway).To(Equal(gateway)) Expect(res.Vlan).To(Equal(vlan)) }) + + It("allocate IP address from the previous records", func() { + mockRIPManager.EXPECT(). + AssembleReservedIPs(gomock.Eq(ctx), gomock.Eq(constant.IPv4)). + Return(nil, nil). + Times(1) + + ipVersion := constant.IPv4 + allocatedIP := "172.18.40.40/24" + gateway := "172.18.40.1" + vlan := int64(0) + + ip, ipNet, err := net.ParseCIDR(allocatedIP) + Expect(err).NotTo(HaveOccurred()) + + ipPoolT.Spec.IPVersion = pointer.Int64(ipVersion) + ipPoolT.Spec.Subnet = ipNet.String() + ipPoolT.Spec.IPs = append(ipPoolT.Spec.IPs, ip.String()) + ipPoolT.Spec.Gateway = pointer.String(gateway) + ipPoolT.Spec.Vlan = pointer.Int64(vlan) + + key, err := cache.MetaNamespaceKeyFunc(podT) + Expect(err).NotTo(HaveOccurred()) + + records := spiderpoolv2beta1.PoolIPAllocations{ + ip.String(): spiderpoolv2beta1.PoolIPAllocation{ + NIC: nic, + NamespacedName: key, + PodUID: string(podT.UID), + }, + } + allocatedIPs, err := json.Marshal(records) + Expect(err).NotTo(HaveOccurred()) + + ipPoolT.Status = spiderpoolv2beta1.IPPoolStatus{ + AllocatedIPs: pointer.String(string(allocatedIPs)), + TotalIPCount: pointer.Int64(1), + AllocatedIPCount: pointer.Int64(1), + } + + err = fakeClient.Create(ctx, ipPoolT) + Expect(err).NotTo(HaveOccurred()) + err = tracker.Add(ipPoolT) + Expect(err).NotTo(HaveOccurred()) + + res, err := ipPoolManager.AllocateIP(ctx, ipPoolName, nic, podT, spiderpooltypes.PodTopController{}) + Expect(err).NotTo(HaveOccurred()) + Expect(*res.Nic).To(Equal(nic)) + Expect(*res.Version).To(Equal(ipVersion)) + Expect(*res.Address).To(Equal(allocatedIP)) + Expect(res.IPPool).To(Equal(ipPoolT.Name)) + Expect(res.Gateway).To(Equal(gateway)) + Expect(res.Vlan).To(Equal(vlan)) + }) }) Describe("ReleaseIP", func() { diff --git a/pkg/ippoolmanager/utils.go b/pkg/ippoolmanager/utils.go index 47d9235d26..8ae9ffcd97 100644 --- a/pkg/ippoolmanager/utils.go +++ b/pkg/ippoolmanager/utils.go @@ -123,3 +123,17 @@ func (b ByPoolPriority) Less(i, j int) bool { return false } + +// findAllocatedIPFromRecords try to find pod NIC previous allocated IP from the IPPool.Status.AllocatedIPs +// this function serves for the issue: https://github.com/spidernet-io/spiderpool/issues/2517 +func findAllocatedIPFromRecords(allocatedRecords spiderpoolv2beta1.PoolIPAllocations, nic, namespacedName, podUID string) (previousIP string, hasFound bool) { + for tmpIP, poolIPAllocation := range allocatedRecords { + if poolIPAllocation.NIC == nic && + poolIPAllocation.NamespacedName == namespacedName && + poolIPAllocation.PodUID == podUID { + return tmpIP, true + } + } + + return "", false +} diff --git a/pkg/ippoolmanager/utils_test.go b/pkg/ippoolmanager/utils_test.go index 4249a65dde..8e517f6244 100644 --- a/pkg/ippoolmanager/utils_test.go +++ b/pkg/ippoolmanager/utils_test.go @@ -11,6 +11,7 @@ import ( appsv1 "k8s.io/api/apps/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" types2 "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/uuid" "github.com/spidernet-io/spiderpool/pkg/constant" spiderpoolv2beta1 "github.com/spidernet-io/spiderpool/pkg/k8s/apis/spiderpool.spidernet.io/v2beta1" @@ -290,4 +291,48 @@ var _ = Describe("IPPoolManager-utils", Label("ippool_manager_utils"), func() { Expect(byPoolPriority).Should(Equal(ByPoolPriority{pool2, pool1})) }) }) + + Context("Test findAllocatedIPFromRecords", func() { + var allocatedRecords spiderpoolv2beta1.PoolIPAllocations + var ip, nic, namespacedName, podUID string + + BeforeEach(func() { + ip = "172.18.40.40" + nic = "eth0" + namespacedName = "default/testPod" + podUID = string(uuid.NewUUID()) + allocatedRecords = spiderpoolv2beta1.PoolIPAllocations{ + ip: spiderpoolv2beta1.PoolIPAllocation{ + NIC: nic, + NamespacedName: namespacedName, + PodUID: podUID, + }, + } + }) + + It("find previous allocated IP in the records", func() { + _, hasFound := findAllocatedIPFromRecords(allocatedRecords, nic, namespacedName, podUID) + Expect(hasFound).To(BeTrue()) + }) + + It("no previous allocated IP in the records", func() { + _, hasFound := findAllocatedIPFromRecords(allocatedRecords, "eth1", "kube-system/testPod1", string(uuid.NewUUID())) + Expect(hasFound).To(BeFalse()) + }) + + It("no previous allocated IP in records due to different nic", func() { + _, hasFound := findAllocatedIPFromRecords(allocatedRecords, "eth1", namespacedName, podUID) + Expect(hasFound).To(BeFalse()) + }) + + It("no previous allocated IP in records due to different NamespacedName", func() { + _, hasFound := findAllocatedIPFromRecords(allocatedRecords, nic, "kube-system/testPod1", podUID) + Expect(hasFound).To(BeFalse()) + }) + + It("no previous allocated IP in records due to different podUID", func() { + _, hasFound := findAllocatedIPFromRecords(allocatedRecords, nic, namespacedName, string(uuid.NewUUID())) + Expect(hasFound).To(BeFalse()) + }) + }) })