Return existing reservation if podRef matched - rebase

Co-Authored-By: Arjun Baindur <[email protected]>

pkg/allocate/allocate.go

@@ -100,9 +100,9 @@ func IterateForAssignment(ipnet net.IPNet, rangeStart net.IP, rangeEnd net.IP, r
 		rangeStart, rangeEnd, ipnet, firstIP, lastIP)
 
 	// Build reserved map.
-	reserved := make(map[string]bool)
+	reserved := make(map[string]string)
 	for _, r := range reserveList {
-		reserved[r.IP.String()] = true
+		reserved[r.IP.String()] = r.PodRef
 	}
 
 	// Build excluded list, "192.168.2.229/30", "192.168.1.229/30".
@@ -119,8 +119,11 @@ func IterateForAssignment(ipnet net.IPNet, rangeStart net.IP, rangeEnd net.IP, r
 	// within ipnet, and make sure that ip is smaller than lastIP.
 	for ip := firstIP; ipnet.Contains(ip) && iphelpers.CompareIPs(ip, lastIP) <= 0; ip = iphelpers.IncIP(ip) {
 		// If already reserved, skip it.
-		if reserved[ip.String()] {
-			continue
+		if reserved[ip.String()] != "" {
+			if reserved[ip.String()] != podRef {
+				continue
+			}
+			logging.Debugf("Found existing reservation %v with matching podRef %s", ip.String(), podRef)
 		}
 		// If this IP is within the range of one of the excluded subnets, jump to the exluded subnet's broadcast address
 		// and skip.

pkg/allocate/allocate_test.go

@@ -415,4 +415,24 @@ var _ = Describe("Allocation operations", func() {
 			})
 		})
 	})
+
+
+	It("can IterateForAssignment on an IPv4 address for existing pod Allocation and return same IP", func() {
+
+		firstip, ipnet, err := net.ParseCIDR("192.168.1.1/24")
+		Expect(err).NotTo(HaveOccurred())
+
+		// figure out the range start.
+		calculatedrangestart := net.ParseIP(firstip.Mask(ipnet.Mask).String())
+
+		var ipres []types.IPReservation
+		var exrange []string
+		podRef := "hello/world-0"
+		newip, _, err := IterateForAssignment(*ipnet, calculatedrangestart, nil, ipres, exrange, "0xdeadbeef", podRef )
+		Expect(err).NotTo(HaveOccurred())
+		newipsec, _, err := IterateForAssignment(*ipnet, calculatedrangestart, nil, ipres, exrange, "0xdeadbeef", podRef )
+		Expect(err).NotTo(HaveOccurred())
+		Expect(fmt.Sprint(newip)).To(Equal(fmt.Sprint(newipsec)))
+
+	})
 })

pkg/storage/kubernetes/ipam.go

@@ -200,13 +200,13 @@ func (i *KubernetesIPAM) GetOverlappingRangeStore() (storage.OverlappingRangeSto
 // IsAllocatedInOverlappingRange checks for IP addresses to see if they're allocated cluster wide, for overlapping
 // ranges.
 func (c *KubernetesOverlappingRangeStore) IsAllocatedInOverlappingRange(ctx context.Context, ip net.IP,
-	networkName string) (bool, error) {
+	networkName string , podRef string) (bool, error) {
 	normalizedIP := normalizeIP(ip, networkName)
 
 	logging.Debugf("OverlappingRangewide allocation check; normalized IP: %q, IP: %q, networkName: %q",
 		normalizedIP, ip, networkName)
 
-	_, err := c.client.WhereaboutsV1alpha1().OverlappingRangeIPReservations(c.namespace).Get(ctx, normalizedIP, metav1.GetOptions{})
+	clusteripres, err := c.client.WhereaboutsV1alpha1().OverlappingRangeIPReservations(c.namespace).Get(ctx, normalizedIP, metav1.GetOptions{})
 	if err != nil && errors.IsNotFound(err) {
 		// cluster ip reservation does not exist, this appears to be good news.
 		// logging.Debugf("IP %v is not reserved cluster wide, allowing.", ip)
@@ -216,6 +216,11 @@ func (c *KubernetesOverlappingRangeStore) IsAllocatedInOverlappingRange(ctx cont
 		return false, fmt.Errorf("k8s get OverlappingRangeIPReservation error: %s", err)
 	}
 
+	if clusteripres.Spec.PodRef == podRef {
+		logging.Debugf("IP %v matches existing podRef %s", ip, podRef)
+		return false, nil
+	}
+
 	logging.Debugf("Normalized IP is reserved; normalized IP: %q, IP: %q, networkName: %q",
 		normalizedIP, ip, networkName)
 	return true, nil
@@ -245,6 +250,21 @@ func (c *KubernetesOverlappingRangeStore) UpdateOverlappingRangeAllocation(ctx c
 		_, err = c.client.WhereaboutsV1alpha1().OverlappingRangeIPReservations(c.namespace).Create(
 			ctx, clusteripres, metav1.CreateOptions{})
 
+		if errors.IsAlreadyExists(err) {
+			logging.Debugf("clusteripres already exists, updating with %v", clusteripres.Spec)
+			// first get the existing object resourceVersion and then update it  https://github.com/kubernetes/kubernetes/issues/70674
+			clusteripresorig, errorig := c.client.WhereaboutsV1alpha1().OverlappingRangeIPReservations(c.namespace).Get(ctx, normalizedIP, metav1.GetOptions{})
+			if errorig != nil {
+				err=errorig
+			} else {
+				clusteripres.SetResourceVersion(clusteripresorig.GetResourceVersion())
+				_, err = c.client.WhereaboutsV1alpha1().OverlappingRangeIPReservations(c.namespace).Update(ctx, clusteripres, metav1.UpdateOptions{})
+			}
+
+
+		}
+
+
 	case whereaboutstypes.Deallocate:
 		verb = "deallocate"
 		err = c.client.WhereaboutsV1alpha1().OverlappingRangeIPReservations(c.namespace).Delete(ctx, clusteripres.GetName(), metav1.DeleteOptions{})
@@ -525,7 +545,7 @@ func IPManagementKubernetesUpdate(ctx context.Context, mode int, ipam *Kubernete
 				// And we try again.
 				if ipamConf.OverlappingRanges {
 					isAllocated, err := overlappingrangestore.IsAllocatedInOverlappingRange(requestCtx, newip.IP,
-						ipamConf.NetworkName)
+						ipamConf.NetworkName, podRef)
 					if err != nil {
 						logging.Errorf("Error checking overlappingrange allocation: %v", err)
 						return newips, err

pkg/storage/storage.go

@@ -33,7 +33,7 @@ type Store interface {
 
 // OverlappingRangeStore is an interface for wrapping overlappingrange storage options
 type OverlappingRangeStore interface {
-	IsAllocatedInOverlappingRange(ctx context.Context, ip net.IP, networkName string) (bool, error)
+	IsAllocatedInOverlappingRange(ctx context.Context, ip net.IP, networkName string, podRef string) (bool, error)
 	UpdateOverlappingRangeAllocation(ctx context.Context, mode int, ip net.IP, containerID string, podRef,
 		networkName string) error
 }