feature: don't let CNS ask for more IPS than node limit (#843)

* feature: CNS to honor MaxIPCount in CRD for scaling events

Author: pjohnst5

cns/ipampoolmonitor/ipampoolmonitor.go

@@ -12,6 +12,10 @@ import (
 	nnc "github.com/Azure/azure-container-networking/nodenetworkconfig/api/v1alpha"
 )
 
+const (
+	defaultMaxIPCount = int64(250)
+)
+
 type CNSIPAMPoolMonitor struct {
 	pendingRelease bool
 
@@ -71,12 +75,21 @@ func (pm *CNSIPAMPoolMonitor) Reconcile() error {
 	pendingReleaseIPCount := len(pm.httpService.GetPendingReleaseIPConfigs())
 	availableIPConfigCount := len(pm.httpService.GetAvailableIPConfigs()) // TODO: add pending allocation count to real cns
 	freeIPConfigCount := pm.cachedNNC.Spec.RequestedIPCount - int64(allocatedPodIPCount)
-	msg := fmt.Sprintf("[ipam-pool-monitor] Pool Size: %v, Goal Size: %v, BatchSize: %v, MinFree: %v, MaxFree:%v, Allocated: %v, Available: %v, Pending Release: %v, Free: %v, Pending Program: %v",
-		cnsPodIPConfigCount, pm.cachedNNC.Spec.RequestedIPCount, pm.scalarUnits.BatchSize, pm.MinimumFreeIps, pm.MaximumFreeIps, allocatedPodIPCount, availableIPConfigCount, pendingReleaseIPCount, freeIPConfigCount, pendingProgramCount)
+	batchSize := pm.getBatchSize() //Use getters in case customer changes batchsize manually
+	maxIPCount := pm.getMaxIPCount()
+
+
+	msg := fmt.Sprintf("[ipam-pool-monitor] Pool Size: %v, Goal Size: %v, BatchSize: %v, MaxIPCount: %v, MinFree: %v, MaxFree:%v, Allocated: %v, Available: %v, Pending Release: %v, Free: %v, Pending Program: %v",
+		cnsPodIPConfigCount, pm.cachedNNC.Spec.RequestedIPCount, batchSize, maxIPCount, pm.MinimumFreeIps, pm.MaximumFreeIps, allocatedPodIPCount, availableIPConfigCount, pendingReleaseIPCount, freeIPConfigCount, pendingProgramCount)
 
 	switch {
 	// pod count is increasing
 	case freeIPConfigCount < pm.MinimumFreeIps:
+		if pm.cachedNNC.Spec.RequestedIPCount == maxIPCount {
+			// If we're already at the maxIPCount, don't try to increase
+			return nil
+		}
+
 		logger.Printf("[ipam-pool-monitor] Increasing pool size...%s ", msg)
 		return pm.increasePoolSize()
 
@@ -111,7 +124,24 @@ func (pm *CNSIPAMPoolMonitor) increasePoolSize() error {
 		return err
 	}
 
-	tempNNCSpec.RequestedIPCount += pm.scalarUnits.BatchSize
+	// Query the max IP count
+	maxIPCount := pm.getMaxIPCount()
+	previouslyRequestedIPCount := tempNNCSpec.RequestedIPCount
+	batchSize := pm.getBatchSize()
+
+	tempNNCSpec.RequestedIPCount += batchSize
+	if tempNNCSpec.RequestedIPCount > maxIPCount {
+		// We don't want to ask for more ips than the max
+		logger.Printf("[ipam-pool-monitor] Requested IP count (%v) is over max limit (%v), requesting max limit instead.", tempNNCSpec.RequestedIPCount, maxIPCount)
+		tempNNCSpec.RequestedIPCount = maxIPCount
+	}
+
+	// If the requested IP count is same as before, then don't do anything
+	if tempNNCSpec.RequestedIPCount == previouslyRequestedIPCount {
+		logger.Printf("[ipam-pool-monitor] Previously requested IP count %v is same as updated IP count %v, doing nothing", previouslyRequestedIPCount, tempNNCSpec.RequestedIPCount)
+		return nil
+	}
+
 	logger.Printf("[ipam-pool-monitor] Increasing pool size, Current Pool Size: %v, Updated Requested IP Count: %v, Pods with IP's:%v, ToBeDeleted Count: %v", len(pm.httpService.GetPodIPConfigState()), tempNNCSpec.RequestedIPCount, len(pm.httpService.GetAllocatedIPConfigs()), len(tempNNCSpec.IPsNotInUse))
 
 	err = pm.rc.UpdateCRDSpec(context.Background(), tempNNCSpec)
@@ -134,10 +164,35 @@ func (pm *CNSIPAMPoolMonitor) decreasePoolSize(existingPendingReleaseIPCount int
 	var err error
 	var newIpsMarkedAsPending bool
 	var pendingIpAddresses map[string]cns.IPConfigurationStatus
+	var updatedRequestedIPCount int64
+	var decreaseIPCountBy int64
+
+	// Ensure the updated requested IP count is a multiple of the batch size
+	previouslyRequestedIPCount := pm.cachedNNC.Spec.RequestedIPCount
+	batchSize := pm.getBatchSize()
+	modResult := previouslyRequestedIPCount % batchSize
+
+	logger.Printf("[ipam-pool-monitor] Previously RequestedIP Count %v", previouslyRequestedIPCount)
+	logger.Printf("[ipam-pool-monitor] Batch size : %v", batchSize)
+	logger.Printf("[ipam-pool-monitor] modResult of (previously requested IP count mod batch size) = %v", modResult)
+
+	if modResult != 0 {
+		// Example: previouscount = 25, batchsize = 10, 25 - 10 = 15, NOT a multiple of batchsize (10)
+		// Don't want that, so make requestedIPCount 20 (25 - (25 % 10)) so that it is a multiple of the batchsize (10)
+		updatedRequestedIPCount = previouslyRequestedIPCount - modResult
+	} else {
+		// Example: previouscount = 30, batchsize = 10, 30 - 10 = 20 which is multiple of batchsize (10) so all good 
+		updatedRequestedIPCount = previouslyRequestedIPCount - batchSize
+	}
+
+  decreaseIPCountBy = previouslyRequestedIPCount - updatedRequestedIPCount
+
+  logger.Printf("[ipam-pool-monitor] updatedRequestedIPCount %v", updatedRequestedIPCount)
+
 	if pm.updatingIpsNotInUseCount == 0 ||
 		pm.updatingIpsNotInUseCount < existingPendingReleaseIPCount {
-		logger.Printf("[ipam-pool-monitor] Marking IPs as PendingRelease, ipsToBeReleasedCount %d", int(pm.scalarUnits.BatchSize))
-		pendingIpAddresses, err = pm.httpService.MarkIPAsPendingRelease(int(pm.scalarUnits.BatchSize))
+		logger.Printf("[ipam-pool-monitor] Marking IPs as PendingRelease, ipsToBeReleasedCount %d", int(decreaseIPCountBy))
+		pendingIpAddresses, err = pm.httpService.MarkIPAsPendingRelease(int(decreaseIPCountBy))
 		if err != nil {
 			return err
 		}
@@ -237,8 +292,8 @@ func (pm *CNSIPAMPoolMonitor) Update(scalar nnc.Scaler, spec nnc.NodeNetworkConf
 
 	pm.scalarUnits = scalar
 
-	pm.MinimumFreeIps = int64(float64(pm.scalarUnits.BatchSize) * (float64(pm.scalarUnits.RequestThresholdPercent) / 100))
-	pm.MaximumFreeIps = int64(float64(pm.scalarUnits.BatchSize) * (float64(pm.scalarUnits.ReleaseThresholdPercent) / 100))
+	pm.MinimumFreeIps = int64(float64(pm.getBatchSize()) * (float64(pm.scalarUnits.RequestThresholdPercent) / 100))
+	pm.MaximumFreeIps = int64(float64(pm.getBatchSize()) * (float64(pm.scalarUnits.ReleaseThresholdPercent) / 100))
 
 	pm.cachedNNC.Spec = spec
 
@@ -248,6 +303,21 @@ func (pm *CNSIPAMPoolMonitor) Update(scalar nnc.Scaler, spec nnc.NodeNetworkConf
 	return nil
 }
 
+func (pm *CNSIPAMPoolMonitor) getMaxIPCount() int64 {
+	if pm.scalarUnits.MaxIPCount == 0 {
+		pm.scalarUnits.MaxIPCount = defaultMaxIPCount
+	}
+	return pm.scalarUnits.MaxIPCount
+}
+
+func (pm *CNSIPAMPoolMonitor) getBatchSize() int64 {
+	maxIPCount := pm.getMaxIPCount()
+	if pm.scalarUnits.BatchSize > maxIPCount {
+		pm.scalarUnits.BatchSize = maxIPCount
+	}
+	return pm.scalarUnits.BatchSize
+}
+
 //this function sets the values for state in IPAMPoolMonitor Struct
 func (pm *CNSIPAMPoolMonitor) GetStateSnapshot() cns.IpamPoolMonitorStateSnapshot {
 	pm.mu.Lock()