Merge pull request #2518 from Icarus9913/fix/wk/ipam-allocation

fix ipam block with ip address used out

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{}

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() {

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
+}

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())
+		})
+	})
 })