From 919665cc9fc574ef83f9b2814c3428e5a008cf03 Mon Sep 17 00:00:00 2001 From: neaggarw Date: Thu, 5 Aug 2021 21:15:34 -0700 Subject: [PATCH 01/10] Fix CNS Scale Up/down fix to avoid cleanup pending ToBeDeleted ips --- cns/fakes/requestcontrollerfake.go | 14 +-- cns/ipampoolmonitor/ipampoolmonitor.go | 25 ++--- cns/ipampoolmonitor/ipampoolmonitor_test.go | 104 ++++++++++++++++++-- cns/restserver/util.go | 7 +- 4 files changed, 121 insertions(+), 29 deletions(-) diff --git a/cns/fakes/requestcontrollerfake.go b/cns/fakes/requestcontrollerfake.go index 0dcaccaef8..f7979847f9 100644 --- a/cns/fakes/requestcontrollerfake.go +++ b/cns/fakes/requestcontrollerfake.go @@ -37,6 +37,7 @@ func NewRequestControllerFake(cnsService *HTTPServiceFake, scalar nnc.Scaler, su rc.ip, _, _ = net.ParseCIDR(subnetAddressSpace) rc.CarveIPConfigsAndAddToStatusAndCNS(numberOfIPConfigs) + rc.cachedCRD.Spec.RequestedIPCount = int64(numberOfIPConfigs) return rc } @@ -62,7 +63,6 @@ func (rc *RequestControllerFake) CarveIPConfigsAndAddToStatusAndCNS(numberOfIPCo } rc.fakecns.IPStateManager.AddIPConfigs(cnsIPConfigs) - rc.cachedCRD.Spec.RequestedIPCount = int64(len(cnsIPConfigs)) return cnsIPConfigs } @@ -88,7 +88,7 @@ func remove(slice []nnc.IPAssignment, s int) []nnc.IPAssignment { return append(slice[:s], slice[s+1:]...) } -func (rc *RequestControllerFake) Reconcile() error { +func (rc *RequestControllerFake) Reconcile(removePendingReleaseIPs bool) error { diff := int(rc.cachedCRD.Spec.RequestedIPCount) - len(rc.fakecns.GetPodIPConfigState()) @@ -115,11 +115,13 @@ func (rc *RequestControllerFake) Reconcile() error { } - // remove ipconfig from CNS - rc.fakecns.IPStateManager.RemovePendingReleaseIPConfigs(rc.cachedCRD.Spec.IPsNotInUse) - // empty the not in use ip's from the spec - rc.cachedCRD.Spec.IPsNotInUse = []string{} + // rc.cachedCRD.Spec.IPsNotInUse = []string{} + } + + // remove ipconfig from CNS + if removePendingReleaseIPs { + rc.fakecns.IPStateManager.RemovePendingReleaseIPConfigs(rc.cachedCRD.Spec.IPsNotInUse) } // update diff --git a/cns/ipampoolmonitor/ipampoolmonitor.go b/cns/ipampoolmonitor/ipampoolmonitor.go index 1d2bd60bb8..6adc95904c 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor.go +++ b/cns/ipampoolmonitor/ipampoolmonitor.go @@ -80,13 +80,13 @@ func (pm *CNSIPAMPoolMonitor) Reconcile(ctx context.Context) error { return pm.increasePoolSize(ctx) // pod count is decreasing - case freeIPConfigCount > pm.MaximumFreeIps: + case freeIPConfigCount >= pm.MaximumFreeIps: logger.Printf("[ipam-pool-monitor] Decreasing pool size...%s ", msg) return pm.decreasePoolSize(ctx, pendingReleaseIPCount) // CRD has reconciled CNS state, and target spec is now the same size as the state // free to remove the IP's from the CRD - case pm.pendingRelease && int(pm.cachedNNC.Spec.RequestedIPCount) == cnsPodIPConfigCount: + case len(pm.cachedNNC.Spec.IPsNotInUse) != pendingReleaseIPCount: logger.Printf("[ipam-pool-monitor] Removing Pending Release IP's from CRD...%s ", msg) return pm.cleanPendingRelease(ctx) @@ -105,7 +105,7 @@ func (pm *CNSIPAMPoolMonitor) increasePoolSize(ctx context.Context) error { var err error var tempNNCSpec nnc.NodeNetworkConfigSpec - tempNNCSpec, err = pm.createNNCSpecForCRD(false) + tempNNCSpec, err = pm.createNNCSpecForCRD() if err != nil { return err } @@ -187,7 +187,7 @@ func (pm *CNSIPAMPoolMonitor) decreasePoolSize(ctx context.Context, existingPend } var tempNNCSpec nnc.NodeNetworkConfigSpec - tempNNCSpec, err = pm.createNNCSpecForCRD(false) + tempNNCSpec, err = pm.createNNCSpecForCRD() if err != nil { return err } @@ -229,7 +229,7 @@ func (pm *CNSIPAMPoolMonitor) cleanPendingRelease(ctx context.Context) error { var err error var tempNNCSpec nnc.NodeNetworkConfigSpec - tempNNCSpec, err = pm.createNNCSpecForCRD(true) + tempNNCSpec, err = pm.createNNCSpecForCRD() if err != nil { return err } @@ -249,7 +249,7 @@ func (pm *CNSIPAMPoolMonitor) cleanPendingRelease(ctx context.Context) error { } // CNSToCRDSpec translates CNS's map of Ips to be released and requested ip count into a CRD Spec -func (pm *CNSIPAMPoolMonitor) createNNCSpecForCRD(resetNotInUseList bool) (nnc.NodeNetworkConfigSpec, error) { +func (pm *CNSIPAMPoolMonitor) createNNCSpecForCRD() (nnc.NodeNetworkConfigSpec, error) { var ( spec nnc.NodeNetworkConfigSpec ) @@ -257,15 +257,10 @@ func (pm *CNSIPAMPoolMonitor) createNNCSpecForCRD(resetNotInUseList bool) (nnc.N // DUpdate the count from cached spec spec.RequestedIPCount = pm.cachedNNC.Spec.RequestedIPCount - // Discard the ToBeDeleted list if requested. This happens if DNC has cleaned up the pending ips and CNS has also updated its state. - if resetNotInUseList == true { - spec.IPsNotInUse = make([]string, 0) - } else { - // Get All Pending IPs from CNS and populate it again. - pendingIps := pm.httpService.GetPendingReleaseIPConfigs() - for _, pendingIp := range pendingIps { - spec.IPsNotInUse = append(spec.IPsNotInUse, pendingIp.ID) - } + // Get All Pending IPs from CNS and populate it again. + pendingIps := pm.httpService.GetPendingReleaseIPConfigs() + for _, pendingIp := range pendingIps { + spec.IPsNotInUse = append(spec.IPsNotInUse, pendingIp.ID) } return spec, nil diff --git a/cns/ipampoolmonitor/ipampoolmonitor_test.go b/cns/ipampoolmonitor/ipampoolmonitor_test.go index d20ba00d4d..3b9ca56946 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor_test.go +++ b/cns/ipampoolmonitor/ipampoolmonitor_test.go @@ -28,7 +28,7 @@ func initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, release fakecns.PoolMonitor = poolmonitor - fakerc.Reconcile() + fakerc.Reconcile(true) return fakecns, fakerc, poolmonitor } @@ -62,7 +62,7 @@ func TestPoolSizeIncrease(t *testing.T) { } // request controller reconciles, carves new IP's from the test subnet and adds to CNS state - err = fakerc.Reconcile() + err = fakerc.Reconcile(true) if err != nil { t.Fatalf("Failed to reconcile fake requestcontroller with err: %v", err) } @@ -128,7 +128,7 @@ func TestPoolIncreaseDoesntChangeWhenIncreaseIsAlreadyInProgress(t *testing.T) { } // request controller reconciles, carves new IP's from the test subnet and adds to CNS state - err = fakerc.Reconcile() + err = fakerc.Reconcile(true) if err != nil { t.Fatalf("Failed to reconcile fake requestcontroller with err: %v", err) } @@ -321,7 +321,7 @@ func TestPoolDecrease(t *testing.T) { } // reconcile the fake request controller - err = fakerc.Reconcile() + err = fakerc.Reconcile(true) if err != nil { t.Fatal(err) } @@ -386,7 +386,7 @@ func TestPoolSizeDecreaseWhenDecreaseHasAlreadyBeenRequested(t *testing.T) { t.Fatalf("Expected pool size to be one batch size smaller after reconcile, and not change after existing call, expected %v, actual %v", (initialIPConfigCount - batchSize), len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) } - err = fakerc.Reconcile() + err = fakerc.Reconcile(true) if err != nil { t.Error(err) } @@ -402,6 +402,81 @@ func TestPoolSizeDecreaseWhenDecreaseHasAlreadyBeenRequested(t *testing.T) { } } +func TestDecreaseAndIncreaseToSameCount(t *testing.T) { + var ( + batchSize = 10 + initialIPConfigCount = 10 + requestThresholdPercent = 50 + releaseThresholdPercent = 150 + maxPodIPCount = int64(30) + ) + + fakecns, fakerc, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) + + log.Printf("Min free IP's %v", poolmonitor.MinimumFreeIps) + log.Printf("Max free IP %v", poolmonitor.MaximumFreeIps) + + // initial pool count is 10, set 5 of them to be allocated + err := fakecns.SetNumberOfAllocatedIPs(7) + if err != nil { + t.Error(err) + } + + // Pool monitor will increase the count to 20 + t.Logf("Scaleup: Increase pool size to 20") + ReconcileAndValidate(t, context.Background(), poolmonitor, 20, 0) + + // Update the IPConfig state + t.Logf("Reconcile with PodIPState") + err = fakerc.Reconcile(true) + if err != nil { + t.Error(err) + } + + // Release all IPs + err = fakecns.SetNumberOfAllocatedIPs(0) + if err != nil { + t.Error(err) + } + + t.Logf("Scaledown: Decrease pool size to 10") + ReconcileAndValidate(t, context.Background(), poolmonitor, 10, 10) + + // Increase it back to 20 + // initial pool count is 10, set 5 of them to be allocated + t.Logf("Scaleup: pool size back to 20 without updating the PodIpState for previous scale down") + err = fakecns.SetNumberOfAllocatedIPs(7) + if err != nil { + t.Error(err) + } + ReconcileAndValidate(t, context.Background(), poolmonitor, 20, 10) + + // Update the IPConfig count and dont remove the pending IPs + t.Logf("Reconcile with PodIPState") + err = fakerc.Reconcile(false) + if err != nil { + t.Error(err) + } + + // reconcile again + t.Logf("Reconcole with pool monitor again, it should not cleanup ipsnotinuse") + ReconcileAndValidate(t, context.Background(), poolmonitor, 20, 10) + + t.Logf("Now update podipconfig state") + err = fakerc.Reconcile(true) + if err != nil { + t.Error(err) + } + + err = poolmonitor.Reconcile(context.Background()) + if err != nil { + t.Errorf("Expected no pool monitor failure after request controller reconcile: %v", err) + } + ReconcileAndValidate(t, context.Background(), poolmonitor, 20, 0) +} + + + func TestPoolSizeDecreaseToReallyLow(t *testing.T) { var ( batchSize = 10 @@ -469,7 +544,7 @@ func TestPoolSizeDecreaseToReallyLow(t *testing.T) { } t.Logf("Update Request Controller") - err = fakerc.Reconcile() + err = fakerc.Reconcile(true) if err != nil { t.Error(err) } @@ -575,3 +650,20 @@ func TestPoolDecreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) when the max has been reached", poolmonitor.cachedNNC.Spec.RequestedIPCount, maxPodIPCount) } } + +func ReconcileAndValidate(t *testing.T, ctx context.Context, poolmonitor *CNSIPAMPoolMonitor, expectedRequestCount int64, expectedIpsNotInUse int) { + err := poolmonitor.Reconcile(context.Background()) + if err != nil { + t.Errorf("Expected pool monitor to not fail after CNS set number of allocated IP's %v", err) + } + + // Increased the new count to be 20 + if poolmonitor.cachedNNC.Spec.RequestedIPCount != expectedRequestCount { + t.Fatalf("ScaleUp: Expected pool size failed, expected %v, actual %v", expectedRequestCount, poolmonitor.cachedNNC.Spec.RequestedIPCount) + } + + // Ensure there is no pending release ips + if len(poolmonitor.cachedNNC.Spec.IPsNotInUse) != expectedIpsNotInUse { + t.Fatalf("Expected IP's not in use be one batch size smaller after reconcile, expected %v, actual %v", expectedIpsNotInUse, len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) + } +} \ No newline at end of file diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 6e27f72279..ebdaba4457 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -269,10 +269,13 @@ func (service *HTTPRestService) addIPConfigStateUntransacted(ncId string, hostVe ipconfig.NCVersion = existingIPConfig.NCVersion ipconfigs[ipId] = ipconfig } - logger.Printf("[Azure-Cns] Set IP %s version to %d, programmed host nc version is %d", ipconfig.IPAddress, ipconfig.NCVersion, hostVersion) - if _, exists := service.PodIPConfigState[ipId]; exists { + + if ipState, exists := service.PodIPConfigState[ipId]; exists { + logger.Printf("[Azure-Cns] Set ipId %s, IP %s version to %d, programmed host nc version is %d, ipState: %+v", ipId, ipconfig.IPAddress, ipconfig.NCVersion, hostVersion, ipState) continue } + + logger.Printf("[Azure-Cns] Set ipId %s, IP %s version to %d, programmed host nc version is %d", ipId, ipconfig.IPAddress, ipconfig.NCVersion, hostVersion) // Using the updated NC version attached with IP to compare with latest nmagent version and determine IP statues. // When reconcile, service.PodIPConfigState doens't exist, rebuild it with the help of NC version attached with IP. var newIPCNSStatus string From 3f8f55cf5b3e6eb0e4688e908fac0c367ea87922 Mon Sep 17 00:00:00 2001 From: neaggarw Date: Thu, 5 Aug 2021 21:29:11 -0700 Subject: [PATCH 02/10] remove pendingRelease bool flag --- cns/ipampoolmonitor/ipampoolmonitor.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/cns/ipampoolmonitor/ipampoolmonitor.go b/cns/ipampoolmonitor/ipampoolmonitor.go index 6adc95904c..b7d6d0c3b5 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor.go +++ b/cns/ipampoolmonitor/ipampoolmonitor.go @@ -22,7 +22,6 @@ type CNSIPAMPoolMonitor struct { cachedNNC nnc.NodeNetworkConfig httpService cns.HTTPService mu sync.RWMutex - pendingRelease bool rc singletenantcontroller.RequestController scalarUnits nnc.Scaler updatingIpsNotInUseCount int @@ -31,7 +30,6 @@ type CNSIPAMPoolMonitor struct { func NewCNSIPAMPoolMonitor(httpService cns.HTTPService, rc singletenantcontroller.RequestController) *CNSIPAMPoolMonitor { logger.Printf("NewCNSIPAMPoolMonitor: Create IPAM Pool Monitor") return &CNSIPAMPoolMonitor{ - pendingRelease: false, httpService: httpService, rc: rc, } @@ -212,7 +210,6 @@ func (pm *CNSIPAMPoolMonitor) decreasePoolSize(ctx context.Context, existingPend // save the updated state to cachedSpec pm.cachedNNC.Spec = tempNNCSpec - pm.pendingRelease = true // clear the updatingPendingIpsNotInUse, as we have Updated the CRD logger.Printf("[ipam-pool-monitor] cleaning the updatingPendingIpsNotInUse, existing length %d", pm.updatingIpsNotInUseCount) @@ -244,7 +241,6 @@ func (pm *CNSIPAMPoolMonitor) cleanPendingRelease(ctx context.Context) error { // save the updated state to cachedSpec pm.cachedNNC.Spec = tempNNCSpec - pm.pendingRelease = false return nil } From 8b7a4cd1b469661934ec3d8d29860fcf59b76ca6 Mon Sep 17 00:00:00 2001 From: neaggarw Date: Fri, 6 Aug 2021 13:30:20 -0700 Subject: [PATCH 03/10] Incorporate feedback --- cns/ipampoolmonitor/ipampoolmonitor.go | 25 +++------ cns/ipampoolmonitor/ipampoolmonitor_test.go | 56 +++++++++++++-------- 2 files changed, 43 insertions(+), 38 deletions(-) diff --git a/cns/ipampoolmonitor/ipampoolmonitor.go b/cns/ipampoolmonitor/ipampoolmonitor.go index b7d6d0c3b5..6ff87f5daf 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor.go +++ b/cns/ipampoolmonitor/ipampoolmonitor.go @@ -103,10 +103,7 @@ func (pm *CNSIPAMPoolMonitor) increasePoolSize(ctx context.Context) error { var err error var tempNNCSpec nnc.NodeNetworkConfigSpec - tempNNCSpec, err = pm.createNNCSpecForCRD() - if err != nil { - return err - } + tempNNCSpec = pm.createNNCSpecForCRD() // Query the max IP count maxIPCount := pm.getMaxIPCount() @@ -185,10 +182,7 @@ func (pm *CNSIPAMPoolMonitor) decreasePoolSize(ctx context.Context, existingPend } var tempNNCSpec nnc.NodeNetworkConfigSpec - tempNNCSpec, err = pm.createNNCSpecForCRD() - if err != nil { - return err - } + tempNNCSpec = pm.createNNCSpecForCRD() if newIpsMarkedAsPending { // cache the updatingPendingRelease so that we dont re-set new IPs to PendingRelease in case UpdateCRD call fails @@ -226,10 +220,7 @@ func (pm *CNSIPAMPoolMonitor) cleanPendingRelease(ctx context.Context) error { var err error var tempNNCSpec nnc.NodeNetworkConfigSpec - tempNNCSpec, err = pm.createNNCSpecForCRD() - if err != nil { - return err - } + tempNNCSpec = pm.createNNCSpecForCRD() err = pm.rc.UpdateCRDSpec(ctx, tempNNCSpec) if err != nil { @@ -245,7 +236,7 @@ func (pm *CNSIPAMPoolMonitor) cleanPendingRelease(ctx context.Context) error { } // CNSToCRDSpec translates CNS's map of Ips to be released and requested ip count into a CRD Spec -func (pm *CNSIPAMPoolMonitor) createNNCSpecForCRD() (nnc.NodeNetworkConfigSpec, error) { +func (pm *CNSIPAMPoolMonitor) createNNCSpecForCRD() (nnc.NodeNetworkConfigSpec) { var ( spec nnc.NodeNetworkConfigSpec ) @@ -254,12 +245,12 @@ func (pm *CNSIPAMPoolMonitor) createNNCSpecForCRD() (nnc.NodeNetworkConfigSpec, spec.RequestedIPCount = pm.cachedNNC.Spec.RequestedIPCount // Get All Pending IPs from CNS and populate it again. - pendingIps := pm.httpService.GetPendingReleaseIPConfigs() - for _, pendingIp := range pendingIps { - spec.IPsNotInUse = append(spec.IPsNotInUse, pendingIp.ID) + pendingIPs := pm.httpService.GetPendingReleaseIPConfigs() + for _, pendingIP := range pendingIPs { + spec.IPsNotInUse = append(spec.IPsNotInUse, pendingIP.ID) } - return spec, nil + return spec } // UpdatePoolLimitsTransacted called by request controller on reconcile to set the batch size limits diff --git a/cns/ipampoolmonitor/ipampoolmonitor_test.go b/cns/ipampoolmonitor/ipampoolmonitor_test.go index 3b9ca56946..eba5f35039 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor_test.go +++ b/cns/ipampoolmonitor/ipampoolmonitor_test.go @@ -10,7 +10,7 @@ import ( nnc "github.com/Azure/azure-container-networking/nodenetworkconfig/api/v1alpha" ) -func initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent int, maxPodIPCount int64) (*fakes.HTTPServiceFake, *fakes.RequestControllerFake, *CNSIPAMPoolMonitor) { +func initFakes(t *testing.T, batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent int, maxPodIPCount int64) (*fakes.HTTPServiceFake, *fakes.RequestControllerFake, *CNSIPAMPoolMonitor) { logger.InitLogger("testlogs", 0, 0, "./") scalarUnits := nnc.Scaler{ @@ -28,7 +28,10 @@ func initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, release fakecns.PoolMonitor = poolmonitor - fakerc.Reconcile(true) + err := fakerc.Reconcile(true) + if err != nil { + t.Fatalf("Failed to reconcile fake requestcontroller with err: %v", err) + } return fakecns, fakerc, poolmonitor } @@ -42,7 +45,7 @@ func TestPoolSizeIncrease(t *testing.T) { maxPodIPCount = int64(30) ) - fakecns, fakerc, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) + fakecns, fakerc, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) // increase number of allocated IP's in CNS err := fakecns.SetNumberOfAllocatedIPs(8) @@ -96,7 +99,7 @@ func TestPoolIncreaseDoesntChangeWhenIncreaseIsAlreadyInProgress(t *testing.T) { maxPodIPCount = int64(30) ) - fakecns, fakerc, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) + fakecns, fakerc, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) // increase number of allocated IP's in CNS err := fakecns.SetNumberOfAllocatedIPs(8) @@ -162,7 +165,7 @@ func TestPoolSizeIncreaseIdempotency(t *testing.T) { maxPodIPCount = int64(30) ) - fakecns, _, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) + fakecns, _, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) t.Logf("Minimum free IPs to request: %v", poolmonitor.MinimumFreeIps) t.Logf("Maximum free IPs to release: %v", poolmonitor.MaximumFreeIps) @@ -205,7 +208,7 @@ func TestPoolIncreasePastNodeLimit(t *testing.T) { maxPodIPCount = int64(30) ) - fakecns, _, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) + fakecns, _, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) t.Logf("Minimum free IPs to request: %v", poolmonitor.MinimumFreeIps) t.Logf("Maximum free IPs to release: %v", poolmonitor.MaximumFreeIps) @@ -237,7 +240,7 @@ func TestPoolIncreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { maxPodIPCount = int64(30) ) - fakecns, _, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) + fakecns, _, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) t.Logf("Minimum free IPs to request: %v", poolmonitor.MinimumFreeIps) t.Logf("Maximum free IPs to release: %v", poolmonitor.MaximumFreeIps) @@ -270,7 +273,7 @@ func TestPoolIncreaseMaxIPCountSetToZero(t *testing.T) { expectedMaxPodIPCount = defaultMaxIPCount ) - _, _, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, initialMaxPodIPCount) + _, _, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, initialMaxPodIPCount) if poolmonitor.getMaxIPCount() != expectedMaxPodIPCount { t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) when the MaxIPCount field in the CRD is zero", poolmonitor.getMaxIPCount(), expectedMaxPodIPCount) @@ -286,7 +289,7 @@ func TestPoolDecrease(t *testing.T) { maxPodIPCount = int64(30) ) - fakecns, fakerc, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) + fakecns, fakerc, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) log.Printf("Min free IP's %v", poolmonitor.MinimumFreeIps) log.Printf("Max free IP %v", poolmonitor.MaximumFreeIps) @@ -343,7 +346,7 @@ func TestPoolSizeDecreaseWhenDecreaseHasAlreadyBeenRequested(t *testing.T) { maxPodIPCount = int64(30) ) - fakecns, fakerc, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) + fakecns, fakerc, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) log.Printf("Min free IP's %v", poolmonitor.MinimumFreeIps) log.Printf("Max free IP %v", poolmonitor.MaximumFreeIps) @@ -411,7 +414,12 @@ func TestDecreaseAndIncreaseToSameCount(t *testing.T) { maxPodIPCount = int64(30) ) - fakecns, fakerc, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) + fakecns, fakerc, poolmonitor := initFakes(t, + batchSize, + initialIPConfigCount, + requestThresholdPercent, + releaseThresholdPercent, + maxPodIPCount) log.Printf("Min free IP's %v", poolmonitor.MinimumFreeIps) log.Printf("Max free IP %v", poolmonitor.MaximumFreeIps) @@ -475,8 +483,6 @@ func TestDecreaseAndIncreaseToSameCount(t *testing.T) { ReconcileAndValidate(t, context.Background(), poolmonitor, 20, 0) } - - func TestPoolSizeDecreaseToReallyLow(t *testing.T) { var ( batchSize = 10 @@ -486,7 +492,7 @@ func TestPoolSizeDecreaseToReallyLow(t *testing.T) { maxPodIPCount = int64(30) ) - fakecns, fakerc, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) + fakecns, fakerc, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) log.Printf("Min free IP's %v", poolmonitor.MinimumFreeIps) log.Printf("Max free IP %v", poolmonitor.MaximumFreeIps) @@ -571,7 +577,7 @@ func TestDecreaseAfterNodeLimitReached(t *testing.T) { expectedDecreaseIP = int(maxPodIPCount) % batchSize ) - fakecns, _, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) + fakecns, _, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) t.Logf("Minimum free IPs to request: %v", poolmonitor.MinimumFreeIps) t.Logf("Maximum free IPs to release: %v", poolmonitor.MaximumFreeIps) @@ -617,7 +623,7 @@ func TestPoolDecreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { maxPodIPCount = int64(30) ) - fakecns, _, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) + fakecns, _, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) t.Logf("Minimum free IPs to request: %v", poolmonitor.MinimumFreeIps) t.Logf("Maximum free IPs to release: %v", poolmonitor.MaximumFreeIps) @@ -651,19 +657,27 @@ func TestPoolDecreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { } } -func ReconcileAndValidate(t *testing.T, ctx context.Context, poolmonitor *CNSIPAMPoolMonitor, expectedRequestCount int64, expectedIpsNotInUse int) { +func ReconcileAndValidate(t *testing.T, + ctx context.Context, + poolmonitor *CNSIPAMPoolMonitor, + expectedRequestCount, + expectedIpsNotInUse int) { err := poolmonitor.Reconcile(context.Background()) if err != nil { t.Errorf("Expected pool monitor to not fail after CNS set number of allocated IP's %v", err) } // Increased the new count to be 20 - if poolmonitor.cachedNNC.Spec.RequestedIPCount != expectedRequestCount { - t.Fatalf("ScaleUp: Expected pool size failed, expected %v, actual %v", expectedRequestCount, poolmonitor.cachedNNC.Spec.RequestedIPCount) + if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(expectedRequestCount) { + t.Fatalf("RequestIPCount not same, expected %v, actual %v", + expectedRequestCount, + poolmonitor.cachedNNC.Spec.RequestedIPCount) } // Ensure there is no pending release ips if len(poolmonitor.cachedNNC.Spec.IPsNotInUse) != expectedIpsNotInUse { - t.Fatalf("Expected IP's not in use be one batch size smaller after reconcile, expected %v, actual %v", expectedIpsNotInUse, len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) + t.Fatalf("Expected IP's not in use, expected %v, actual %v", + expectedIpsNotInUse, + len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) } -} \ No newline at end of file +} From 906f5914a8749d307858a7b6816921545e495cff Mon Sep 17 00:00:00 2001 From: neaggarw Date: Fri, 6 Aug 2021 14:28:07 -0700 Subject: [PATCH 04/10] incorporate lint failures --- cns/ipampoolmonitor/ipampoolmonitor.go | 6 +-- cns/ipampoolmonitor/ipampoolmonitor_test.go | 49 ++++++++++++++++----- 2 files changed, 42 insertions(+), 13 deletions(-) diff --git a/cns/ipampoolmonitor/ipampoolmonitor.go b/cns/ipampoolmonitor/ipampoolmonitor.go index 6ff87f5daf..dd751bc544 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor.go +++ b/cns/ipampoolmonitor/ipampoolmonitor.go @@ -30,8 +30,8 @@ type CNSIPAMPoolMonitor struct { func NewCNSIPAMPoolMonitor(httpService cns.HTTPService, rc singletenantcontroller.RequestController) *CNSIPAMPoolMonitor { logger.Printf("NewCNSIPAMPoolMonitor: Create IPAM Pool Monitor") return &CNSIPAMPoolMonitor{ - httpService: httpService, - rc: rc, + httpService: httpService, + rc: rc, } } @@ -236,7 +236,7 @@ func (pm *CNSIPAMPoolMonitor) cleanPendingRelease(ctx context.Context) error { } // CNSToCRDSpec translates CNS's map of Ips to be released and requested ip count into a CRD Spec -func (pm *CNSIPAMPoolMonitor) createNNCSpecForCRD() (nnc.NodeNetworkConfigSpec) { +func (pm *CNSIPAMPoolMonitor) createNNCSpecForCRD() nnc.NodeNetworkConfigSpec { var ( spec nnc.NodeNetworkConfigSpec ) diff --git a/cns/ipampoolmonitor/ipampoolmonitor_test.go b/cns/ipampoolmonitor/ipampoolmonitor_test.go index eba5f35039..c68366b6ab 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor_test.go +++ b/cns/ipampoolmonitor/ipampoolmonitor_test.go @@ -10,7 +10,12 @@ import ( nnc "github.com/Azure/azure-container-networking/nodenetworkconfig/api/v1alpha" ) -func initFakes(t *testing.T, batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent int, maxPodIPCount int64) (*fakes.HTTPServiceFake, *fakes.RequestControllerFake, *CNSIPAMPoolMonitor) { +func initFakes(t *testing.T, + batchSize, + initialIPConfigCount, + requestThresholdPercent, + releaseThresholdPercent int, + maxPodIPCount int64) (*fakes.HTTPServiceFake, *fakes.RequestControllerFake, *CNSIPAMPoolMonitor) { logger.InitLogger("testlogs", 0, 0, "./") scalarUnits := nnc.Scaler{ @@ -99,7 +104,12 @@ func TestPoolIncreaseDoesntChangeWhenIncreaseIsAlreadyInProgress(t *testing.T) { maxPodIPCount = int64(30) ) - fakecns, fakerc, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) + fakecns, fakerc, poolmonitor := initFakes(t, + batchSize, + initialIPConfigCount, + requestThresholdPercent, + releaseThresholdPercent, + maxPodIPCount) // increase number of allocated IP's in CNS err := fakecns.SetNumberOfAllocatedIPs(8) @@ -165,7 +175,12 @@ func TestPoolSizeIncreaseIdempotency(t *testing.T) { maxPodIPCount = int64(30) ) - fakecns, _, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) + fakecns, _, poolmonitor := initFakes(t, + batchSize, + initialIPConfigCount, + requestThresholdPercent, + releaseThresholdPercent, + maxPodIPCount) t.Logf("Minimum free IPs to request: %v", poolmonitor.MinimumFreeIps) t.Logf("Maximum free IPs to release: %v", poolmonitor.MaximumFreeIps) @@ -208,7 +223,12 @@ func TestPoolIncreasePastNodeLimit(t *testing.T) { maxPodIPCount = int64(30) ) - fakecns, _, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) + fakecns, _, poolmonitor := initFakes(t, + batchSize, + initialIPConfigCount, + requestThresholdPercent, + releaseThresholdPercent, + maxPodIPCount) t.Logf("Minimum free IPs to request: %v", poolmonitor.MinimumFreeIps) t.Logf("Maximum free IPs to release: %v", poolmonitor.MaximumFreeIps) @@ -240,7 +260,12 @@ func TestPoolIncreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { maxPodIPCount = int64(30) ) - fakecns, _, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) + fakecns, _, poolmonitor := initFakes(t, + batchSize, + initialIPConfigCount, + requestThresholdPercent, + releaseThresholdPercent, + maxPodIPCount) t.Logf("Minimum free IPs to request: %v", poolmonitor.MinimumFreeIps) t.Logf("Maximum free IPs to release: %v", poolmonitor.MaximumFreeIps) @@ -273,10 +298,12 @@ func TestPoolIncreaseMaxIPCountSetToZero(t *testing.T) { expectedMaxPodIPCount = defaultMaxIPCount ) - _, _, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, initialMaxPodIPCount) + _, _, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, + requestThresholdPercent, releaseThresholdPercent, initialMaxPodIPCount) if poolmonitor.getMaxIPCount() != expectedMaxPodIPCount { - t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) when the MaxIPCount field in the CRD is zero", poolmonitor.getMaxIPCount(), expectedMaxPodIPCount) + t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) " + + "when the MaxIPCount field in the CRD is zero", poolmonitor.getMaxIPCount(), expectedMaxPodIPCount) } } @@ -289,7 +316,8 @@ func TestPoolDecrease(t *testing.T) { maxPodIPCount = int64(30) ) - fakecns, fakerc, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) + fakecns, fakerc, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, + requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) log.Printf("Min free IP's %v", poolmonitor.MinimumFreeIps) log.Printf("Max free IP %v", poolmonitor.MaximumFreeIps) @@ -346,7 +374,8 @@ func TestPoolSizeDecreaseWhenDecreaseHasAlreadyBeenRequested(t *testing.T) { maxPodIPCount = int64(30) ) - fakecns, fakerc, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) + fakecns, fakerc, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, + requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) log.Printf("Min free IP's %v", poolmonitor.MinimumFreeIps) log.Printf("Max free IP %v", poolmonitor.MaximumFreeIps) @@ -492,7 +521,7 @@ func TestPoolSizeDecreaseToReallyLow(t *testing.T) { maxPodIPCount = int64(30) ) - fakecns, fakerc, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) + fakecns, fakerc, poolmonitor := exit(t, batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) log.Printf("Min free IP's %v", poolmonitor.MinimumFreeIps) log.Printf("Max free IP %v", poolmonitor.MaximumFreeIps) From 0f04270916b00762ede108417616d1373fd02298 Mon Sep 17 00:00:00 2001 From: neaggarw Date: Fri, 6 Aug 2021 16:07:43 -0700 Subject: [PATCH 05/10] fix more lint issues --- cns/ipampoolmonitor/ipampoolmonitor_test.go | 7 ++++++- cns/restserver/util.go | 16 ++++++++-------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/cns/ipampoolmonitor/ipampoolmonitor_test.go b/cns/ipampoolmonitor/ipampoolmonitor_test.go index c68366b6ab..24adb58e48 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor_test.go +++ b/cns/ipampoolmonitor/ipampoolmonitor_test.go @@ -521,7 +521,12 @@ func TestPoolSizeDecreaseToReallyLow(t *testing.T) { maxPodIPCount = int64(30) ) - fakecns, fakerc, poolmonitor := exit(t, batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) + fakecns, fakerc, poolmonitor := initFakes(t, + batchSize, + initialIPConfigCount, + requestThresholdPercent, + releaseThresholdPercent, + maxPodIPCount) log.Printf("Min free IP's %v", poolmonitor.MinimumFreeIps) log.Printf("Max free IP %v", poolmonitor.MaximumFreeIps) diff --git a/cns/restserver/util.go b/cns/restserver/util.go index ebdaba4457..70f87cee12 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -262,20 +262,20 @@ func (service *HTTPRestService) updateIPConfigsStateUntransacted( // acquire/release the service lock. func (service *HTTPRestService) addIPConfigStateUntransacted(ncId string, hostVersion int, ipconfigs, existingSecondaryIPConfigs map[string]cns.SecondaryIPConfig) { // add ipconfigs to state - for ipId, ipconfig := range ipconfigs { + for ipID, ipconfig := range ipconfigs { // New secondary IP configs has new NC version however, CNS don't want to override existing IPs'with new NC version // Set it back to previous NC version if IP already exist. - if existingIPConfig, existsInPreviousIPConfig := existingSecondaryIPConfigs[ipId]; existsInPreviousIPConfig { + if existingIPConfig, existsInPreviousIPConfig := existingSecondaryIPConfigs[ipID]; existsInPreviousIPConfig { ipconfig.NCVersion = existingIPConfig.NCVersion - ipconfigs[ipId] = ipconfig + ipconfigs[ipID] = ipconfig } - if ipState, exists := service.PodIPConfigState[ipId]; exists { - logger.Printf("[Azure-Cns] Set ipId %s, IP %s version to %d, programmed host nc version is %d, ipState: %+v", ipId, ipconfig.IPAddress, ipconfig.NCVersion, hostVersion, ipState) + if ipState, exists := service.PodIPConfigState[ipID]; exists { + logger.Printf("[Azure-Cns] Set ipId %s, IP %s version to %d, programmed host nc version is %d, ipState: %+v", ipID, ipconfig.IPAddress, ipconfig.NCVersion, hostVersion, ipState) continue } - logger.Printf("[Azure-Cns] Set ipId %s, IP %s version to %d, programmed host nc version is %d", ipId, ipconfig.IPAddress, ipconfig.NCVersion, hostVersion) + logger.Printf("[Azure-Cns] Set ipId %s, IP %s version to %d, programmed host nc version is %d", ipID, ipconfig.IPAddress, ipconfig.NCVersion, hostVersion) // Using the updated NC version attached with IP to compare with latest nmagent version and determine IP statues. // When reconcile, service.PodIPConfigState doens't exist, rebuild it with the help of NC version attached with IP. var newIPCNSStatus string @@ -287,14 +287,14 @@ func (service *HTTPRestService) addIPConfigStateUntransacted(ncId string, hostVe // add the new State ipconfigStatus := cns.IPConfigurationStatus{ NCID: ncId, - ID: ipId, + ID: ipID, IPAddress: ipconfig.IPAddress, State: newIPCNSStatus, PodInfo: nil, } logger.Printf("[Azure-Cns] Add IP %s as %s", ipconfig.IPAddress, newIPCNSStatus) - service.PodIPConfigState[ipId] = ipconfigStatus + service.PodIPConfigState[ipID] = ipconfigStatus // Todo Update batch API and maintain the count } From 12c981b0d836f2089926ee613e47cec884d88e98 Mon Sep 17 00:00:00 2001 From: neaggarw Date: Fri, 6 Aug 2021 16:24:13 -0700 Subject: [PATCH 06/10] nit fixes again for lint errors --- cns/ipampoolmonitor/ipampoolmonitor_test.go | 109 ++++++++++++++------ 1 file changed, 79 insertions(+), 30 deletions(-) diff --git a/cns/ipampoolmonitor/ipampoolmonitor_test.go b/cns/ipampoolmonitor/ipampoolmonitor_test.go index 24adb58e48..8021f1177d 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor_test.go +++ b/cns/ipampoolmonitor/ipampoolmonitor_test.go @@ -50,7 +50,12 @@ func TestPoolSizeIncrease(t *testing.T) { maxPodIPCount = int64(30) ) - fakecns, fakerc, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) + fakecns, fakerc, poolmonitor := initFakes(t, + batchSize, + initialIPConfigCount, + requestThresholdPercent, + releaseThresholdPercent, + maxPodIPCount) // increase number of allocated IP's in CNS err := fakecns.SetNumberOfAllocatedIPs(8) @@ -66,7 +71,9 @@ func TestPoolSizeIncrease(t *testing.T) { // ensure pool monitor has reached quorum with cns if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(initialIPConfigCount+(1*batchSize)) { - t.Fatalf("Pool monitor target IP count doesn't match CNS pool state after reconcile: %v, actual %v", poolmonitor.cachedNNC.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) + t.Fatalf("Pool monitor target IP count doesn't match CNS pool state " + + "after reconcile: %v, " + + "actual %v", poolmonitor.cachedNNC.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) } // request controller reconciles, carves new IP's from the test subnet and adds to CNS state @@ -84,15 +91,20 @@ func TestPoolSizeIncrease(t *testing.T) { // ensure pool monitor has reached quorum with cns if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(initialIPConfigCount+(1*batchSize)) { - t.Fatalf("Pool monitor target IP count doesn't match CNS pool state after reconcile: %v, actual %v", poolmonitor.cachedNNC.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) + t.Fatalf("Pool monitor target IP count doesn't " + + "match CNS pool state after reconcile: %v, " + + "actual %v", poolmonitor.cachedNNC.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) } // make sure IPConfig state size reflects the new pool size if len(fakecns.GetPodIPConfigState()) != initialIPConfigCount+(1*batchSize) { - t.Fatalf("CNS Pod IPConfig state count doesn't match, expected: %v, actual %v", len(fakecns.GetPodIPConfigState()), initialIPConfigCount+(1*batchSize)) + t.Fatalf("CNS Pod IPConfig state count doesn't " + + "match, expected: %v, actual %v", len(fakecns.GetPodIPConfigState()), initialIPConfigCount+(1*batchSize)) } - t.Logf("Pool size %v, Target pool size %v, Allocated IP's %v, ", len(fakecns.GetPodIPConfigState()), poolmonitor.cachedNNC.Spec.RequestedIPCount, len(fakecns.GetAllocatedIPConfigs())) + t.Logf("Pool size %v, Target pool size %v, " + + "Allocated IP's %v, ", len(fakecns.GetPodIPConfigState()), + poolmonitor.cachedNNC.Spec.RequestedIPCount, len(fakecns.GetAllocatedIPConfigs())) } func TestPoolIncreaseDoesntChangeWhenIncreaseIsAlreadyInProgress(t *testing.T) { @@ -137,7 +149,8 @@ func TestPoolIncreaseDoesntChangeWhenIncreaseIsAlreadyInProgress(t *testing.T) { // ensure pool monitor has reached quorum with cns if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(initialIPConfigCount+(1*batchSize)) { - t.Fatalf("Pool monitor target IP count doesn't match CNS pool state after reconcile: %v, actual %v", poolmonitor.cachedNNC.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) + t.Fatalf("Pool monitor target IP count doesn't match CNS pool state after reconcile: %v," + + " actual %v", poolmonitor.cachedNNC.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) } // request controller reconciles, carves new IP's from the test subnet and adds to CNS state @@ -155,15 +168,18 @@ func TestPoolIncreaseDoesntChangeWhenIncreaseIsAlreadyInProgress(t *testing.T) { // make sure IPConfig state size reflects the new pool size if len(fakecns.GetPodIPConfigState()) != initialIPConfigCount+(1*batchSize) { - t.Fatalf("CNS Pod IPConfig state count doesn't match, expected: %v, actual %v", len(fakecns.GetPodIPConfigState()), initialIPConfigCount+(1*batchSize)) + t.Fatalf("CNS Pod IPConfig state count doesn't match, expected: %v, actual %v", + len(fakecns.GetPodIPConfigState()), initialIPConfigCount+(1*batchSize)) } // ensure pool monitor has reached quorum with cns if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(initialIPConfigCount+(1*batchSize)) { - t.Fatalf("Pool monitor target IP count doesn't match CNS pool state after reconcile: %v, actual %v", poolmonitor.cachedNNC.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) + t.Fatalf("Pool monitor target IP count doesn't match CNS pool state after reconcile: %v, " + + "actual %v", poolmonitor.cachedNNC.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) } - t.Logf("Pool size %v, Target pool size %v, Allocated IP's %v, ", len(fakecns.GetPodIPConfigState()), poolmonitor.cachedNNC.Spec.RequestedIPCount, len(fakecns.GetAllocatedIPConfigs())) + t.Logf("Pool size %v, Target pool size %v, Allocated IP's %v, ", len(fakecns.GetPodIPConfigState()), + poolmonitor.cachedNNC.Spec.RequestedIPCount, len(fakecns.GetAllocatedIPConfigs())) } func TestPoolSizeIncreaseIdempotency(t *testing.T) { @@ -199,7 +215,8 @@ func TestPoolSizeIncreaseIdempotency(t *testing.T) { // ensure pool monitor has increased batch size if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(initialIPConfigCount+(1*batchSize)) { - t.Fatalf("Pool monitor target IP count doesn't match CNS pool state after reconcile: %v, actual %v", poolmonitor.cachedNNC.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) + t.Fatalf("Pool monitor target IP count doesn't match CNS pool state after reconcile: %v," + + " actual %v", poolmonitor.cachedNNC.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) } // reconcile pool monitor a second time, then verify requested ip count is still the same @@ -210,7 +227,8 @@ func TestPoolSizeIncreaseIdempotency(t *testing.T) { // ensure pool monitor requested pool size is unchanged as request controller hasn't reconciled yet if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(initialIPConfigCount+(1*batchSize)) { - t.Fatalf("Pool monitor target IP count doesn't match CNS pool state after reconcile: %v, actual %v", poolmonitor.cachedNNC.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) + t.Fatalf("Pool monitor target IP count doesn't match CNS pool state after reconcile: %v," + + " actual %v", poolmonitor.cachedNNC.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) } } @@ -247,7 +265,8 @@ func TestPoolIncreasePastNodeLimit(t *testing.T) { // ensure pool monitor has only requested the max pod ip count if poolmonitor.cachedNNC.Spec.RequestedIPCount != maxPodIPCount { - t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) when the max has been reached", poolmonitor.cachedNNC.Spec.RequestedIPCount, maxPodIPCount) + t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) when the max " + + "has been reached", poolmonitor.cachedNNC.Spec.RequestedIPCount, maxPodIPCount) } } @@ -284,7 +303,8 @@ func TestPoolIncreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { // ensure pool monitor has only requested the max pod ip count if poolmonitor.cachedNNC.Spec.RequestedIPCount != maxPodIPCount { - t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) when the max has been reached", poolmonitor.cachedNNC.Spec.RequestedIPCount, maxPodIPCount) + t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) " + + "when the max has been reached", poolmonitor.cachedNNC.Spec.RequestedIPCount, maxPodIPCount) } } @@ -348,7 +368,8 @@ func TestPoolDecrease(t *testing.T) { // ensure that the adjusted spec is smaller than the initial pool size if len(poolmonitor.cachedNNC.Spec.IPsNotInUse) != (initialIPConfigCount - batchSize) { - t.Fatalf("Expected pool size to be one batch size smaller after reconcile, expected %v, actual %v", (initialIPConfigCount - batchSize), len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) + t.Fatalf("Expected pool size to be one batch size smaller after reconcile, expected %v, actual %v", + (initialIPConfigCount - batchSize), len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) } // reconcile the fake request controller @@ -359,7 +380,8 @@ func TestPoolDecrease(t *testing.T) { // Ensure the size of the requested spec is still the same if len(poolmonitor.cachedNNC.Spec.IPsNotInUse) != 0 { - t.Fatalf("Expected IPsNotInUse to be 0 after request controller reconcile, actual %v", poolmonitor.cachedNNC.Spec.IPsNotInUse) + t.Fatalf("Expected IPsNotInUse to be 0 after request controller reconcile, " + + "actual %v", poolmonitor.cachedNNC.Spec.IPsNotInUse) } return @@ -394,12 +416,14 @@ func TestPoolSizeDecreaseWhenDecreaseHasAlreadyBeenRequested(t *testing.T) { // Ensure the size of the requested spec is still the same if len(poolmonitor.cachedNNC.Spec.IPsNotInUse) != (initialIPConfigCount - batchSize) { - t.Fatalf("Expected IP's not in use be one batch size smaller after reconcile, expected %v, actual %v", (initialIPConfigCount - batchSize), len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) + t.Fatalf("Expected IP's not in use be one batch size smaller after reconcile, expected %v," + + " actual %v", (initialIPConfigCount - batchSize), len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) } // Ensure the request ipcount is now one batch size smaller than the inital IP count if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(initialIPConfigCount-batchSize) { - t.Fatalf("Expected pool size to be one batch size smaller after reconcile, expected %v, actual %v", (initialIPConfigCount - batchSize), len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) + t.Fatalf("Expected pool size to be one batch size smaller after reconcile, expected %v," + + " actual %v", (initialIPConfigCount - batchSize), len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) } // Update pods with IP count, ensure pool monitor stays the same until request controller reconciles @@ -410,12 +434,16 @@ func TestPoolSizeDecreaseWhenDecreaseHasAlreadyBeenRequested(t *testing.T) { // Ensure the size of the requested spec is still the same if len(poolmonitor.cachedNNC.Spec.IPsNotInUse) != (initialIPConfigCount - batchSize) { - t.Fatalf("Expected IP's not in use to be one batch size smaller after reconcile, and not change after reconcile, expected %v, actual %v", (initialIPConfigCount - batchSize), len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) + t.Fatalf("Expected IP's not in use to be one batch size smaller after reconcile, and not change" + + " after reconcile, expected %v, actual %v", + (initialIPConfigCount - batchSize), len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) } // Ensure the request ipcount is now one batch size smaller than the inital IP count if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(initialIPConfigCount-batchSize) { - t.Fatalf("Expected pool size to be one batch size smaller after reconcile, and not change after existing call, expected %v, actual %v", (initialIPConfigCount - batchSize), len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) + t.Fatalf("Expected pool size to be one batch size smaller after reconcile, and not change after" + + " existing call, expected %v, actual %v", (initialIPConfigCount - batchSize), + len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) } err = fakerc.Reconcile(true) @@ -430,7 +458,8 @@ func TestPoolSizeDecreaseWhenDecreaseHasAlreadyBeenRequested(t *testing.T) { // Ensure the spec doesn't have any IPsNotInUse after request controller has reconciled if len(poolmonitor.cachedNNC.Spec.IPsNotInUse) != 0 { - t.Fatalf("Expected IP's not in use to be 0 after reconcile, expected %v, actual %v", (initialIPConfigCount - batchSize), len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) + t.Fatalf("Expected IP's not in use to be 0 after reconcile, expected %v, actual %v", + (initialIPConfigCount - batchSize), len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) } } @@ -558,12 +587,14 @@ func TestPoolSizeDecreaseToReallyLow(t *testing.T) { // Ensure the size of the requested spec is still the same if len(poolmonitor.cachedNNC.Spec.IPsNotInUse) != batchSize { - t.Fatalf("Expected IP's not in use is not correct, expected %v, actual %v", batchSize, len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) + t.Fatalf("Expected IP's not in use is not correct, expected %v, actual %v", + batchSize, len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) } // Ensure the request ipcount is now one batch size smaller than the inital IP count if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(initialIPConfigCount-batchSize) { - t.Fatalf("Expected pool size to be one batch size smaller after reconcile, expected %v, actual %v", (initialIPConfigCount - batchSize), len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) + t.Fatalf("Expected pool size to be one batch size smaller after reconcile, expected %v, " + + "actual %v", (initialIPConfigCount - batchSize), len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) } // Reconcile again, it should release the second batch @@ -575,12 +606,14 @@ func TestPoolSizeDecreaseToReallyLow(t *testing.T) { // Ensure the size of the requested spec is still the same if len(poolmonitor.cachedNNC.Spec.IPsNotInUse) != batchSize*2 { - t.Fatalf("Expected IP's not in use is not correct, expected %v, actual %v", batchSize*2, len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) + t.Fatalf("Expected IP's not in use is not correct, expected %v, actual %v", batchSize*2, + len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) } // Ensure the request ipcount is now one batch size smaller than the inital IP count if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(initialIPConfigCount-(batchSize*2)) { - t.Fatalf("Expected pool size to be one batch size smaller after reconcile, expected %v, actual %v", (initialIPConfigCount - batchSize), len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) + t.Fatalf("Expected pool size to be one batch size smaller after reconcile, expected %v, " + + "actual %v", (initialIPConfigCount - batchSize), len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) } t.Logf("Update Request Controller") @@ -596,7 +629,8 @@ func TestPoolSizeDecreaseToReallyLow(t *testing.T) { // Ensure the spec doesn't have any IPsNotInUse after request controller has reconciled if len(poolmonitor.cachedNNC.Spec.IPsNotInUse) != 0 { - t.Fatalf("Expected IP's not in use to be 0 after reconcile, expected %v, actual %v", (initialIPConfigCount - batchSize), len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) + t.Fatalf("Expected IP's not in use to be 0 after reconcile, expected %v, actual %v", + (initialIPConfigCount - batchSize), len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) } } @@ -611,7 +645,12 @@ func TestDecreaseAfterNodeLimitReached(t *testing.T) { expectedDecreaseIP = int(maxPodIPCount) % batchSize ) - fakecns, _, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) + fakecns, _, poolmonitor := initFakes(t, + batchSize, + initialIPConfigCount, + requestThresholdPercent, + releaseThresholdPercent, + maxPodIPCount) t.Logf("Minimum free IPs to request: %v", poolmonitor.MinimumFreeIps) t.Logf("Maximum free IPs to release: %v", poolmonitor.MaximumFreeIps) @@ -639,12 +678,16 @@ func TestDecreaseAfterNodeLimitReached(t *testing.T) { // Ensure poolmonitor asked for a multiple of batch size if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(expectedRequestedIP) { - t.Fatalf("Expected requested ips to be %v when scaling by 1 batch size down from %v (max pod limit) but got %v", expectedRequestedIP, maxPodIPCount, poolmonitor.cachedNNC.Spec.RequestedIPCount) + t.Fatalf("Expected requested ips to be %v when scaling by 1 batch size down from %v " + + "(max pod limit) but got %v", expectedRequestedIP, maxPodIPCount, + poolmonitor.cachedNNC.Spec.RequestedIPCount) } // Ensure we minused by the mod result if len(poolmonitor.cachedNNC.Spec.IPsNotInUse) != expectedDecreaseIP { - t.Fatalf("Expected to decrease requested IPs by %v (max pod count mod batchsize) to make the requested ip count a multiple of the batch size in the case of hitting the max before scale down, but got %v", expectedDecreaseIP, len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) + t.Fatalf("Expected to decrease requested IPs by %v (max pod count mod batchsize) to " + + "make the requested ip count a multiple of the batch size in the case of hitting " + + "the max before scale down, but got %v", expectedDecreaseIP, len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) } } @@ -657,7 +700,12 @@ func TestPoolDecreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { maxPodIPCount = int64(30) ) - fakecns, _, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) + fakecns, _, poolmonitor := initFakes(t, + batchSize, + initialIPConfigCount, + requestThresholdPercent, + releaseThresholdPercent, + maxPodIPCount) t.Logf("Minimum free IPs to request: %v", poolmonitor.MinimumFreeIps) t.Logf("Maximum free IPs to release: %v", poolmonitor.MaximumFreeIps) @@ -687,7 +735,8 @@ func TestPoolDecreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { // ensure pool monitor has only requested the max pod ip count if poolmonitor.cachedNNC.Spec.RequestedIPCount != maxPodIPCount { - t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) when the max has been reached", poolmonitor.cachedNNC.Spec.RequestedIPCount, maxPodIPCount) + t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) when the max " + + "has been reached", poolmonitor.cachedNNC.Spec.RequestedIPCount, maxPodIPCount) } } From 6c78a722f2ff1e0fc498a4daa26cc866bb698699 Mon Sep 17 00:00:00 2001 From: neaggarw Date: Fri, 6 Aug 2021 16:51:04 -0700 Subject: [PATCH 07/10] fixes for lint --- cns/ipampoolmonitor/ipampoolmonitor_test.go | 62 ++++++++++----------- cns/restserver/util.go | 21 ++++--- 2 files changed, 44 insertions(+), 39 deletions(-) diff --git a/cns/ipampoolmonitor/ipampoolmonitor_test.go b/cns/ipampoolmonitor/ipampoolmonitor_test.go index 8021f1177d..825dda0ba4 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor_test.go +++ b/cns/ipampoolmonitor/ipampoolmonitor_test.go @@ -71,8 +71,8 @@ func TestPoolSizeIncrease(t *testing.T) { // ensure pool monitor has reached quorum with cns if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(initialIPConfigCount+(1*batchSize)) { - t.Fatalf("Pool monitor target IP count doesn't match CNS pool state " + - "after reconcile: %v, " + + t.Fatalf("Pool monitor target IP count doesn't match CNS pool state "+ + "after reconcile: %v, "+ "actual %v", poolmonitor.cachedNNC.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) } @@ -91,18 +91,18 @@ func TestPoolSizeIncrease(t *testing.T) { // ensure pool monitor has reached quorum with cns if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(initialIPConfigCount+(1*batchSize)) { - t.Fatalf("Pool monitor target IP count doesn't " + - "match CNS pool state after reconcile: %v, " + + t.Fatalf("Pool monitor target IP count doesn't "+ + "match CNS pool state after reconcile: %v, "+ "actual %v", poolmonitor.cachedNNC.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) } // make sure IPConfig state size reflects the new pool size if len(fakecns.GetPodIPConfigState()) != initialIPConfigCount+(1*batchSize) { - t.Fatalf("CNS Pod IPConfig state count doesn't " + + t.Fatalf("CNS Pod IPConfig state count doesn't "+ "match, expected: %v, actual %v", len(fakecns.GetPodIPConfigState()), initialIPConfigCount+(1*batchSize)) } - t.Logf("Pool size %v, Target pool size %v, " + + t.Logf("Pool size %v, Target pool size %v, "+ "Allocated IP's %v, ", len(fakecns.GetPodIPConfigState()), poolmonitor.cachedNNC.Spec.RequestedIPCount, len(fakecns.GetAllocatedIPConfigs())) } @@ -149,7 +149,7 @@ func TestPoolIncreaseDoesntChangeWhenIncreaseIsAlreadyInProgress(t *testing.T) { // ensure pool monitor has reached quorum with cns if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(initialIPConfigCount+(1*batchSize)) { - t.Fatalf("Pool monitor target IP count doesn't match CNS pool state after reconcile: %v," + + t.Fatalf("Pool monitor target IP count doesn't match CNS pool state after reconcile: %v,"+ " actual %v", poolmonitor.cachedNNC.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) } @@ -174,7 +174,7 @@ func TestPoolIncreaseDoesntChangeWhenIncreaseIsAlreadyInProgress(t *testing.T) { // ensure pool monitor has reached quorum with cns if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(initialIPConfigCount+(1*batchSize)) { - t.Fatalf("Pool monitor target IP count doesn't match CNS pool state after reconcile: %v, " + + t.Fatalf("Pool monitor target IP count doesn't match CNS pool state after reconcile: %v, "+ "actual %v", poolmonitor.cachedNNC.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) } @@ -215,7 +215,7 @@ func TestPoolSizeIncreaseIdempotency(t *testing.T) { // ensure pool monitor has increased batch size if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(initialIPConfigCount+(1*batchSize)) { - t.Fatalf("Pool monitor target IP count doesn't match CNS pool state after reconcile: %v," + + t.Fatalf("Pool monitor target IP count doesn't match CNS pool state after reconcile: %v,"+ " actual %v", poolmonitor.cachedNNC.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) } @@ -227,7 +227,7 @@ func TestPoolSizeIncreaseIdempotency(t *testing.T) { // ensure pool monitor requested pool size is unchanged as request controller hasn't reconciled yet if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(initialIPConfigCount+(1*batchSize)) { - t.Fatalf("Pool monitor target IP count doesn't match CNS pool state after reconcile: %v," + + t.Fatalf("Pool monitor target IP count doesn't match CNS pool state after reconcile: %v,"+ " actual %v", poolmonitor.cachedNNC.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) } } @@ -265,7 +265,7 @@ func TestPoolIncreasePastNodeLimit(t *testing.T) { // ensure pool monitor has only requested the max pod ip count if poolmonitor.cachedNNC.Spec.RequestedIPCount != maxPodIPCount { - t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) when the max " + + t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) when the max "+ "has been reached", poolmonitor.cachedNNC.Spec.RequestedIPCount, maxPodIPCount) } } @@ -303,7 +303,7 @@ func TestPoolIncreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { // ensure pool monitor has only requested the max pod ip count if poolmonitor.cachedNNC.Spec.RequestedIPCount != maxPodIPCount { - t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) " + + t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) "+ "when the max has been reached", poolmonitor.cachedNNC.Spec.RequestedIPCount, maxPodIPCount) } } @@ -322,7 +322,7 @@ func TestPoolIncreaseMaxIPCountSetToZero(t *testing.T) { requestThresholdPercent, releaseThresholdPercent, initialMaxPodIPCount) if poolmonitor.getMaxIPCount() != expectedMaxPodIPCount { - t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) " + + t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) "+ "when the MaxIPCount field in the CRD is zero", poolmonitor.getMaxIPCount(), expectedMaxPodIPCount) } } @@ -380,7 +380,7 @@ func TestPoolDecrease(t *testing.T) { // Ensure the size of the requested spec is still the same if len(poolmonitor.cachedNNC.Spec.IPsNotInUse) != 0 { - t.Fatalf("Expected IPsNotInUse to be 0 after request controller reconcile, " + + t.Fatalf("Expected IPsNotInUse to be 0 after request controller reconcile, "+ "actual %v", poolmonitor.cachedNNC.Spec.IPsNotInUse) } @@ -416,13 +416,13 @@ func TestPoolSizeDecreaseWhenDecreaseHasAlreadyBeenRequested(t *testing.T) { // Ensure the size of the requested spec is still the same if len(poolmonitor.cachedNNC.Spec.IPsNotInUse) != (initialIPConfigCount - batchSize) { - t.Fatalf("Expected IP's not in use be one batch size smaller after reconcile, expected %v," + + t.Fatalf("Expected IP's not in use be one batch size smaller after reconcile, expected %v,"+ " actual %v", (initialIPConfigCount - batchSize), len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) } // Ensure the request ipcount is now one batch size smaller than the inital IP count if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(initialIPConfigCount-batchSize) { - t.Fatalf("Expected pool size to be one batch size smaller after reconcile, expected %v," + + t.Fatalf("Expected pool size to be one batch size smaller after reconcile, expected %v,"+ " actual %v", (initialIPConfigCount - batchSize), len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) } @@ -434,14 +434,14 @@ func TestPoolSizeDecreaseWhenDecreaseHasAlreadyBeenRequested(t *testing.T) { // Ensure the size of the requested spec is still the same if len(poolmonitor.cachedNNC.Spec.IPsNotInUse) != (initialIPConfigCount - batchSize) { - t.Fatalf("Expected IP's not in use to be one batch size smaller after reconcile, and not change" + + t.Fatalf("Expected IP's not in use to be one batch size smaller after reconcile, and not change"+ " after reconcile, expected %v, actual %v", (initialIPConfigCount - batchSize), len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) } // Ensure the request ipcount is now one batch size smaller than the inital IP count if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(initialIPConfigCount-batchSize) { - t.Fatalf("Expected pool size to be one batch size smaller after reconcile, and not change after" + + t.Fatalf("Expected pool size to be one batch size smaller after reconcile, and not change after"+ " existing call, expected %v, actual %v", (initialIPConfigCount - batchSize), len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) } @@ -490,7 +490,7 @@ func TestDecreaseAndIncreaseToSameCount(t *testing.T) { // Pool monitor will increase the count to 20 t.Logf("Scaleup: Increase pool size to 20") - ReconcileAndValidate(t, context.Background(), poolmonitor, 20, 0) + ReconcileAndValidate(context.Background(), t, poolmonitor, 20, 0) // Update the IPConfig state t.Logf("Reconcile with PodIPState") @@ -506,7 +506,7 @@ func TestDecreaseAndIncreaseToSameCount(t *testing.T) { } t.Logf("Scaledown: Decrease pool size to 10") - ReconcileAndValidate(t, context.Background(), poolmonitor, 10, 10) + ReconcileAndValidate(context.Background(), t, poolmonitor, 10, 10) // Increase it back to 20 // initial pool count is 10, set 5 of them to be allocated @@ -515,7 +515,7 @@ func TestDecreaseAndIncreaseToSameCount(t *testing.T) { if err != nil { t.Error(err) } - ReconcileAndValidate(t, context.Background(), poolmonitor, 20, 10) + ReconcileAndValidate(context.Background(), t, poolmonitor, 20, 10) // Update the IPConfig count and dont remove the pending IPs t.Logf("Reconcile with PodIPState") @@ -526,7 +526,7 @@ func TestDecreaseAndIncreaseToSameCount(t *testing.T) { // reconcile again t.Logf("Reconcole with pool monitor again, it should not cleanup ipsnotinuse") - ReconcileAndValidate(t, context.Background(), poolmonitor, 20, 10) + ReconcileAndValidate(context.Background(), t, poolmonitor, 20, 10) t.Logf("Now update podipconfig state") err = fakerc.Reconcile(true) @@ -538,7 +538,7 @@ func TestDecreaseAndIncreaseToSameCount(t *testing.T) { if err != nil { t.Errorf("Expected no pool monitor failure after request controller reconcile: %v", err) } - ReconcileAndValidate(t, context.Background(), poolmonitor, 20, 0) + ReconcileAndValidate(context.Background(), t, poolmonitor, 20, 0) } func TestPoolSizeDecreaseToReallyLow(t *testing.T) { @@ -593,7 +593,7 @@ func TestPoolSizeDecreaseToReallyLow(t *testing.T) { // Ensure the request ipcount is now one batch size smaller than the inital IP count if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(initialIPConfigCount-batchSize) { - t.Fatalf("Expected pool size to be one batch size smaller after reconcile, expected %v, " + + t.Fatalf("Expected pool size to be one batch size smaller after reconcile, expected %v, "+ "actual %v", (initialIPConfigCount - batchSize), len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) } @@ -612,7 +612,7 @@ func TestPoolSizeDecreaseToReallyLow(t *testing.T) { // Ensure the request ipcount is now one batch size smaller than the inital IP count if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(initialIPConfigCount-(batchSize*2)) { - t.Fatalf("Expected pool size to be one batch size smaller after reconcile, expected %v, " + + t.Fatalf("Expected pool size to be one batch size smaller after reconcile, expected %v, "+ "actual %v", (initialIPConfigCount - batchSize), len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) } @@ -678,15 +678,15 @@ func TestDecreaseAfterNodeLimitReached(t *testing.T) { // Ensure poolmonitor asked for a multiple of batch size if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(expectedRequestedIP) { - t.Fatalf("Expected requested ips to be %v when scaling by 1 batch size down from %v " + + t.Fatalf("Expected requested ips to be %v when scaling by 1 batch size down from %v "+ "(max pod limit) but got %v", expectedRequestedIP, maxPodIPCount, poolmonitor.cachedNNC.Spec.RequestedIPCount) } // Ensure we minused by the mod result if len(poolmonitor.cachedNNC.Spec.IPsNotInUse) != expectedDecreaseIP { - t.Fatalf("Expected to decrease requested IPs by %v (max pod count mod batchsize) to " + - "make the requested ip count a multiple of the batch size in the case of hitting " + + t.Fatalf("Expected to decrease requested IPs by %v (max pod count mod batchsize) to "+ + "make the requested ip count a multiple of the batch size in the case of hitting "+ "the max before scale down, but got %v", expectedDecreaseIP, len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) } } @@ -735,13 +735,13 @@ func TestPoolDecreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { // ensure pool monitor has only requested the max pod ip count if poolmonitor.cachedNNC.Spec.RequestedIPCount != maxPodIPCount { - t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) when the max " + + t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) when the max "+ "has been reached", poolmonitor.cachedNNC.Spec.RequestedIPCount, maxPodIPCount) } } -func ReconcileAndValidate(t *testing.T, - ctx context.Context, +func ReconcileAndValidate(ctx context.Context, + t *testing.T, poolmonitor *CNSIPAMPoolMonitor, expectedRequestCount, expectedIpsNotInUse int) { diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 70f87cee12..22b1067d4f 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -226,7 +226,7 @@ func (service *HTTPRestService) updateIPConfigsStateUntransacted( } // Validate TobeDeletedIps are ready to be deleted. - for ipId, _ := range tobeDeletedIpConfigs { + for ipId := range tobeDeletedIpConfigs { ipConfigStatus, exists := service.PodIPConfigState[ipId] if exists { // pod ip exists, validate if state is not allocated, else fail @@ -238,8 +238,8 @@ func (service *HTTPRestService) updateIPConfigsStateUntransacted( } // now actually remove the deletedIPs - for ipId, _ := range tobeDeletedIpConfigs { - returncode, errMsg := service.removeToBeDeletedIPStateUntransacted(ipId, true) + for ipId := range tobeDeletedIpConfigs { + returncode, errMsg := service.removeToBeDeletedIpsStateUntransacted(ipId, true) if returncode != types.Success { return returncode, errMsg } @@ -252,7 +252,8 @@ func (service *HTTPRestService) updateIPConfigsStateUntransacted( if hostNCVersionInInt, err = strconv.Atoi(hostVersion); err != nil { return types.UnsupportedNCVersion, fmt.Sprintf("Invalid hostVersion is %s, err:%s", hostVersion, err) } - service.addIPConfigStateUntransacted(req.NetworkContainerid, hostNCVersionInInt, req.SecondaryIPConfigs, existingSecondaryIPConfigs) + service.addIPConfigStateUntransacted(req.NetworkContainerid, hostNCVersionInInt, req.SecondaryIPConfigs, + existingSecondaryIPConfigs) return 0, "" } @@ -260,10 +261,12 @@ func (service *HTTPRestService) updateIPConfigsStateUntransacted( // addIPConfigStateUntransacted adds the IPConfigs to the PodIpConfigState map with Available state // If the IP is already added then it will be an idempotent call. Also note, caller will // acquire/release the service lock. -func (service *HTTPRestService) addIPConfigStateUntransacted(ncId string, hostVersion int, ipconfigs, existingSecondaryIPConfigs map[string]cns.SecondaryIPConfig) { +func (service *HTTPRestService) addIPConfigStateUntransacted(ncId string, hostVersion int, ipconfigs, + existingSecondaryIPConfigs map[string]cns.SecondaryIPConfig) { // add ipconfigs to state for ipID, ipconfig := range ipconfigs { - // New secondary IP configs has new NC version however, CNS don't want to override existing IPs'with new NC version + // New secondary IP configs has new NC version however, CNS don't want to override existing IPs'with new + //NC version // Set it back to previous NC version if IP already exist. if existingIPConfig, existsInPreviousIPConfig := existingSecondaryIPConfigs[ipID]; existsInPreviousIPConfig { ipconfig.NCVersion = existingIPConfig.NCVersion @@ -271,11 +274,13 @@ func (service *HTTPRestService) addIPConfigStateUntransacted(ncId string, hostVe } if ipState, exists := service.PodIPConfigState[ipID]; exists { - logger.Printf("[Azure-Cns] Set ipId %s, IP %s version to %d, programmed host nc version is %d, ipState: %+v", ipID, ipconfig.IPAddress, ipconfig.NCVersion, hostVersion, ipState) + logger.Printf("[Azure-Cns] Set ipId %s, IP %s version to %d, programmed host nc version is %d, "+ + "ipState: %+v", ipID, ipconfig.IPAddress, ipconfig.NCVersion, hostVersion, ipState) continue } - logger.Printf("[Azure-Cns] Set ipId %s, IP %s version to %d, programmed host nc version is %d", ipID, ipconfig.IPAddress, ipconfig.NCVersion, hostVersion) + logger.Printf("[Azure-Cns] Set ipId %s, IP %s version to %d, programmed host nc version is %d", + ipID, ipconfig.IPAddress, ipconfig.NCVersion, hostVersion) // Using the updated NC version attached with IP to compare with latest nmagent version and determine IP statues. // When reconcile, service.PodIPConfigState doens't exist, rebuild it with the help of NC version attached with IP. var newIPCNSStatus string From 751f9c7516de6ff28973bc7bd6c1d87d2b357fb6 Mon Sep 17 00:00:00 2001 From: neaggarw Date: Fri, 6 Aug 2021 17:06:09 -0700 Subject: [PATCH 08/10] fix comment spacing --- cns/restserver/util.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 22b1067d4f..d5790719c0 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -266,8 +266,7 @@ func (service *HTTPRestService) addIPConfigStateUntransacted(ncId string, hostVe // add ipconfigs to state for ipID, ipconfig := range ipconfigs { // New secondary IP configs has new NC version however, CNS don't want to override existing IPs'with new - //NC version - // Set it back to previous NC version if IP already exist. + // NC version. Set it back to previous NC version if IP already exist. if existingIPConfig, existsInPreviousIPConfig := existingSecondaryIPConfigs[ipID]; existsInPreviousIPConfig { ipconfig.NCVersion = existingIPConfig.NCVersion ipconfigs[ipID] = ipconfig From 73b78a28ccb5a5e9742c43e19692274e6c264867 Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Fri, 6 Aug 2021 17:37:03 -0700 Subject: [PATCH 09/10] fix final lints --- cns/restserver/util.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cns/restserver/util.go b/cns/restserver/util.go index d5790719c0..76d4c6e6ba 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -226,8 +226,8 @@ func (service *HTTPRestService) updateIPConfigsStateUntransacted( } // Validate TobeDeletedIps are ready to be deleted. - for ipId := range tobeDeletedIpConfigs { - ipConfigStatus, exists := service.PodIPConfigState[ipId] + for ipID := range tobeDeletedIpConfigs { + ipConfigStatus, exists := service.PodIPConfigState[ipID] if exists { // pod ip exists, validate if state is not allocated, else fail if ipConfigStatus.State == cns.Allocated { @@ -238,8 +238,8 @@ func (service *HTTPRestService) updateIPConfigsStateUntransacted( } // now actually remove the deletedIPs - for ipId := range tobeDeletedIpConfigs { - returncode, errMsg := service.removeToBeDeletedIpsStateUntransacted(ipId, true) + for ipID := range tobeDeletedIpConfigs { + returncode, errMsg := service.removeToBeDeletedIPStateUntransacted(ipID, true) if returncode != types.Success { return returncode, errMsg } @@ -261,7 +261,7 @@ func (service *HTTPRestService) updateIPConfigsStateUntransacted( // addIPConfigStateUntransacted adds the IPConfigs to the PodIpConfigState map with Available state // If the IP is already added then it will be an idempotent call. Also note, caller will // acquire/release the service lock. -func (service *HTTPRestService) addIPConfigStateUntransacted(ncId string, hostVersion int, ipconfigs, +func (service *HTTPRestService) addIPConfigStateUntransacted(ncID string, hostVersion int, ipconfigs, existingSecondaryIPConfigs map[string]cns.SecondaryIPConfig) { // add ipconfigs to state for ipID, ipconfig := range ipconfigs { @@ -290,7 +290,7 @@ func (service *HTTPRestService) addIPConfigStateUntransacted(ncId string, hostVe } // add the new State ipconfigStatus := cns.IPConfigurationStatus{ - NCID: ncId, + NCID: ncID, ID: ipID, IPAddress: ipconfig.IPAddress, State: newIPCNSStatus, From f8f4c669a055911bd4cf6da95ca88c47c78b3830 Mon Sep 17 00:00:00 2001 From: Matthew Long <61910737+thatmattlong@users.noreply.github.com> Date: Mon, 9 Aug 2021 15:54:19 -0700 Subject: [PATCH 10/10] fix broken ipampoolmonitor test --- cns/fakes/requestcontrollerfake.go | 3 --- cns/ipampoolmonitor/ipampoolmonitor_test.go | 11 +++++------ 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/cns/fakes/requestcontrollerfake.go b/cns/fakes/requestcontrollerfake.go index f7979847f9..7616bda300 100644 --- a/cns/fakes/requestcontrollerfake.go +++ b/cns/fakes/requestcontrollerfake.go @@ -114,9 +114,6 @@ func (rc *RequestControllerFake) Reconcile(removePendingReleaseIPs bool) error { rc.cachedCRD.Status.NetworkContainers[0].IPAssignments = remove(rc.cachedCRD.Status.NetworkContainers[0].IPAssignments, index) } - - // empty the not in use ip's from the spec - // rc.cachedCRD.Spec.IPsNotInUse = []string{} } // remove ipconfig from CNS diff --git a/cns/ipampoolmonitor/ipampoolmonitor_test.go b/cns/ipampoolmonitor/ipampoolmonitor_test.go index 825dda0ba4..24bc063480 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor_test.go +++ b/cns/ipampoolmonitor/ipampoolmonitor_test.go @@ -378,13 +378,12 @@ func TestPoolDecrease(t *testing.T) { t.Fatal(err) } - // Ensure the size of the requested spec is still the same - if len(poolmonitor.cachedNNC.Spec.IPsNotInUse) != 0 { - t.Fatalf("Expected IPsNotInUse to be 0 after request controller reconcile, "+ - "actual %v", poolmonitor.cachedNNC.Spec.IPsNotInUse) + // CNS won't actually clean up the IPsNotInUse until it changes the spec for some other reason (i.e. scale up) + // so instead we should just verify that the CNS state has no more PendingReleaseIPConfigs, + // and that they were cleaned up. + if len(fakecns.GetPendingReleaseIPConfigs()) != 0 { + t.Fatalf("expected 0 PendingReleaseIPConfigs, got %d", len(fakecns.GetPendingReleaseIPConfigs())) } - - return } func TestPoolSizeDecreaseWhenDecreaseHasAlreadyBeenRequested(t *testing.T) {