Merge pull request #503 from xagent003/arjun/nodeIpamFixes

Fixes for node slice IPAM

pkg/allocate/allocate.go

@@ -50,8 +50,8 @@ func AssignIP(ipamConf types.RangeConfiguration, reservelist []types.IPReservati
 }
 
 // DeallocateIP removes allocation from reserve list. Returns the updated reserve list and the deallocated IP.
-func DeallocateIP(reservelist []types.IPReservation, containerID string) ([]types.IPReservation, net.IP) {
-	index := getMatchingIPReservationIndex(reservelist, containerID)
+func DeallocateIP(reservelist []types.IPReservation, containerID, ifName string) ([]types.IPReservation, net.IP) {
+	index := getMatchingIPReservationIndex(reservelist, containerID, ifName)
 	if index < 0 {
 		// Allocation not found. Return the original reserve list and nil IP.
 		return reservelist, nil
@@ -63,9 +63,9 @@ func DeallocateIP(reservelist []types.IPReservation, containerID string) ([]type
 	return removeIdxFromSlice(reservelist, index), ip
 }
 
-func getMatchingIPReservationIndex(reservelist []types.IPReservation, id string) int {
+func getMatchingIPReservationIndex(reservelist []types.IPReservation, id, ifName string) int {
 	for idx, v := range reservelist {
-		if v.ContainerID == id {
+		if v.ContainerID == id && v.IfName == ifName {
 			return idx
 		}
 	}

pkg/iphelpers/iphelpers.go

@@ -34,15 +34,15 @@ func CompareIPs(ipX net.IP, ipY net.IP) int {
 func DivideRangeBySize(inputNetwork string, sliceSizeString string) ([]string, error) {
 	// Remove "/" from the start of the sliceSize
 	sliceSizeString = strings.TrimPrefix(sliceSizeString, "/")
-	
+
 	sliceSize, err := strconv.Atoi(sliceSizeString)
 	if err != nil {
 		fmt.Println("Error:", err)
 		return nil, nil
 	}
 	ip, ipNet, err := net.ParseCIDR(inputNetwork)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("error parsing CIDR %s: %v", inputNetwork, err)
 	}
 	if !ip.Equal(ipNet.IP) {
 		return nil, errors.New("netCIDR is not a valid network address")

pkg/node-controller/controller.go

@@ -132,24 +132,19 @@ func NewController(
 	nadInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
 		AddFunc: c.onNadEvent,
 		UpdateFunc: func(old, cur interface{}) {
+			oldNad := old.(*cncfV1.NetworkAttachmentDefinition)
+			newNad := cur.(*cncfV1.NetworkAttachmentDefinition)
+			if newNad.ResourceVersion == oldNad.ResourceVersion {
+				logger.Info("update for NAD with same resource version")
+				return
+			}
 			c.onNadEvent(cur)
 		},
 		DeleteFunc: c.onNadEvent,
 	})
 
 	nodeInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
-		AddFunc: c.requeueNADs,
-		UpdateFunc: func(old, cur interface{}) {
-			c.requeueNADs(cur)
-		},
-		DeleteFunc: c.requeueNADs,
-	})
-
-	nodeSlicePoolInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
-		AddFunc: c.requeueNADs,
-		UpdateFunc: func(old, cur interface{}) {
-			c.requeueNADs(cur)
-		},
+		AddFunc:    c.requeueNADs,
 		DeleteFunc: c.requeueNADs,
 	})
 
@@ -185,6 +180,7 @@ func (c *Controller) onNadEvent(obj interface{}) {
 // in this case we get all applicable NADs for the node rather than requeuing all
 // same applies to other node event handlers
 func (c *Controller) requeueNADs(obj interface{}) {
+	klog.Infof("handling requeueNADs")
 	nadlist, err := c.nadLister.List(labels.Everything())
 	if err != nil {
 		utilruntime.HandleError(fmt.Errorf("couldn't get network-attachment-definition list from informer: %v", err))
@@ -414,6 +410,7 @@ func (c *Controller) syncHandler(ctx context.Context, key string) error {
 		logger.Info(fmt.Sprintf("final allocations: %v", allocations))
 		_, err = c.whereaboutsclientset.WhereaboutsV1alpha1().NodeSlicePools(c.whereaboutsNamespace).Create(ctx, nodeslice, metav1.CreateOptions{})
 		if err != nil {
+			logger.Error(err, "failed to create nodeslicepool")
 			return err
 		}
 	} else {
@@ -431,9 +428,10 @@ func (c *Controller) syncHandler(ctx context.Context, key string) error {
 		// node slice currently exists
 		if currentNodeSlicePool.Spec.SliceSize != ipamConf.NodeSliceSize ||
 			currentNodeSlicePool.Spec.Range != ipamConf.IPRanges[0].Range {
-			logger.Info("network-attachment-definition range or slice size changed, re-allocating node slices")
+			logger.Info("network-attachment-definition range or slice size changed, re-allocating node slices",
+				"new range", ipamConf.IPRanges[0].Range, "new slice size", ipamConf.NodeSliceSize)
 			// slices have changed so redo the slicing and reassign nodes
-			subnets, err := iphelpers.DivideRangeBySize(ipamConf.Range, ipamConf.NodeSliceSize)
+			subnets, err := iphelpers.DivideRangeBySize(ipamConf.IPRanges[0].Range, ipamConf.NodeSliceSize)
 			if err != nil {
 				return err
 			}
@@ -452,6 +450,10 @@ func (c *Controller) syncHandler(ctx context.Context, key string) error {
 				assignNodeToSlice(allocations, node.Name)
 			}
 
+			nodeslice.Spec = v1alpha1.NodeSlicePoolSpec{
+				Range:     ipamConf.IPRanges[0].Range,
+				SliceSize: ipamConf.NodeSliceSize,
+			}
 			nodeslice.Status = v1alpha1.NodeSlicePoolStatus{
 				Allocations: allocations,
 			}

pkg/storage/kubernetes/ipam.go

@@ -607,7 +607,7 @@ func IPManagementKubernetesUpdate(ctx context.Context, mode int, ipam *Kubernete
 					return newips, err
 				}
 				ipRange = whereaboutstypes.RangeConfiguration{
-					Range:      nodeSliceRange,
+					Range:      ipRange.Range,
 					RangeStart: rangeStart,
 					RangeEnd:   rangeEnd,
 				}
@@ -658,7 +658,7 @@ func IPManagementKubernetesUpdate(ctx context.Context, mode int, ipam *Kubernete
 				}
 
 			case whereaboutstypes.Deallocate:
-				updatedreservelist, ipforoverlappingrangeupdate = allocate.DeallocateIP(reservelist, ipam.containerID)
+				updatedreservelist, ipforoverlappingrangeupdate = allocate.DeallocateIP(reservelist, ipam.containerID, ipam.IfName)
 				if ipforoverlappingrangeupdate == nil {
 					// Do not fail if allocation was not found.
 					logging.Debugf("Failed to find allocation for container ID: %s", ipam.containerID)