From ee414a8212845ccc0e2a5cce99a53e7286d9258a Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Fri, 1 Oct 2021 17:17:54 -0500 Subject: [PATCH 01/14] fix ipampoolmonitor starting before receiving a nodenetworkconfig Signed-off-by: Evan Baker --- cns/api.go | 6 +- cns/client/client_test.go | 19 +- cns/fakes/ipampoolmonitorfake.go | 12 +- cns/fakes/requestcontrollerfake.go | 3 +- cns/ipampoolmonitor/ipampoolmonitor.go | 159 ++++++++-------- cns/ipampoolmonitor/ipampoolmonitor_test.go | 194 ++++++++++---------- cns/restserver/internalapi.go | 9 +- cns/restserver/internalapi_test.go | 84 +++++++-- cns/restserver/ipam_test.go | 17 +- cns/service/main.go | 2 +- 10 files changed, 293 insertions(+), 212 deletions(-) diff --git a/cns/api.go b/cns/api.go index fdaa6d6509..37e3775e2e 100644 --- a/cns/api.go +++ b/cns/api.go @@ -215,14 +215,14 @@ type NodeConfiguration struct { type IPAMPoolMonitor interface { Start(ctx context.Context, poolMonitorRefreshMilliseconds int) error - Update(scalar v1alpha.Scaler, spec v1alpha.NodeNetworkConfigSpec) + Update(nnc *v1alpha.NodeNetworkConfig) GetStateSnapshot() IpamPoolMonitorStateSnapshot } // IpamPoolMonitorStateSnapshot struct to expose state values for IPAMPoolMonitor struct type IpamPoolMonitorStateSnapshot struct { - MinimumFreeIps int64 - MaximumFreeIps int64 + MinimumFreeIps int + MaximumFreeIps int UpdatingIpsNotInUseCount int CachedNNC v1alpha.NodeNetworkConfig } diff --git a/cns/client/client_test.go b/cns/client/client_test.go index d55b820f1b..ffaa2b76d4 100644 --- a/cns/client/client_test.go +++ b/cns/client/client_test.go @@ -97,9 +97,18 @@ func addTestStateToRestServer(t *testing.T, secondaryIps []string) { t.Fatalf("Failed to createNetworkContainerRequest, req: %+v, err: %d", req, returnCode) } - svc.IPAMPoolMonitor.Update( - fakes.NewFakeScalar(releasePercent, requestPercent, batchSize), - fakes.NewFakeNodeNetworkConfigSpec(initPoolSize)) + svc.IPAMPoolMonitor.Update(&v1alpha.NodeNetworkConfig{ + Status: v1alpha.NodeNetworkConfigStatus{ + Scaler: v1alpha.Scaler{ + BatchSize: batchSize, + ReleaseThresholdPercent: releasePercent, + RequestThresholdPercent: requestPercent, + }, + }, + Spec: v1alpha.NodeNetworkConfigSpec{ + RequestedIPCount: initPoolSize, + }, + }) } func getIPNetFromResponse(resp *cns.IPConfigResponse) (net.IPNet, error) { @@ -188,7 +197,7 @@ func TestMain(m *testing.M) { }, }, } - svc.IPAMPoolMonitor = &fakes.IPAMPoolMonitorFake{FakeMinimumIps: 10, FakeMaximumIps: 20, FakeIpsNotInUseCount: 13, FakecachedNNC: fakeNNC} + svc.IPAMPoolMonitor = &fakes.IPAMPoolMonitorFake{FakeIpsNotInUseCount: 13, FakecachedNNC: &fakeNNC} if err != nil { logger.Errorf("Failed to create CNS object, err:%v.\n", err) @@ -1406,4 +1415,4 @@ func TestGetHTTPServiceData(t *testing.T) { assert.Equal(t, tt.want, got) }) } -} \ No newline at end of file +} diff --git a/cns/fakes/ipampoolmonitorfake.go b/cns/fakes/ipampoolmonitorfake.go index adde22342a..cbb528ae83 100644 --- a/cns/fakes/ipampoolmonitorfake.go +++ b/cns/fakes/ipampoolmonitorfake.go @@ -11,17 +11,17 @@ import ( ) type IPAMPoolMonitorFake struct { - FakeMinimumIps int - FakeMaximumIps int FakeIpsNotInUseCount int - FakecachedNNC v1alpha.NodeNetworkConfig + FakecachedNNC *v1alpha.NodeNetworkConfig } func (ipm *IPAMPoolMonitorFake) Start(ctx context.Context, poolMonitorRefreshMilliseconds int) error { return nil } -func (ipm *IPAMPoolMonitorFake) Update(scalar v1alpha.Scaler, spec v1alpha.NodeNetworkConfigSpec) {} +func (ipm *IPAMPoolMonitorFake) Update(nnc *v1alpha.NodeNetworkConfig) { + ipm.FakecachedNNC = nnc +} func (ipm *IPAMPoolMonitorFake) Reconcile() error { return nil @@ -29,9 +29,7 @@ func (ipm *IPAMPoolMonitorFake) Reconcile() error { func (ipm *IPAMPoolMonitorFake) GetStateSnapshot() cns.IpamPoolMonitorStateSnapshot { return cns.IpamPoolMonitorStateSnapshot{ - MinimumFreeIps: int64(ipm.FakeMinimumIps), - MaximumFreeIps: int64(ipm.FakeMaximumIps), UpdatingIpsNotInUseCount: ipm.FakeIpsNotInUseCount, - CachedNNC: ipm.FakecachedNNC, + CachedNNC: *ipm.FakecachedNNC, } } diff --git a/cns/fakes/requestcontrollerfake.go b/cns/fakes/requestcontrollerfake.go index ab7e3870d1..2447d4548c 100644 --- a/cns/fakes/requestcontrollerfake.go +++ b/cns/fakes/requestcontrollerfake.go @@ -114,8 +114,7 @@ func (rc *RequestControllerFake) Reconcile(removePendingReleaseIPs bool) error { } // update - rc.cnscli.PoolMonitor.Update(rc.NNC.Status.Scaler, rc.NNC.Spec) - + rc.cnscli.PoolMonitor.Update(rc.NNC) return nil } diff --git a/cns/ipampoolmonitor/ipampoolmonitor.go b/cns/ipampoolmonitor/ipampoolmonitor.go index 54ad7a998f..f77e086ac0 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor.go +++ b/cns/ipampoolmonitor/ipampoolmonitor.go @@ -18,57 +18,63 @@ type nodeNetworkConfigSpecUpdater interface { UpdateSpec(context.Context, *v1alpha.NodeNetworkConfigSpec) (*v1alpha.NodeNetworkConfig, error) } -type CNSIPAMPoolMonitor struct { - MaximumFreeIps int64 - MinimumFreeIps int64 - cachedNNC v1alpha.NodeNetworkConfig +type PoolMon struct { + nnc *v1alpha.NodeNetworkConfig + nnccli nodeNetworkConfigSpecUpdater httpService cns.HTTPService - mu sync.RWMutex - scalarUnits v1alpha.Scaler updatingIpsNotInUseCount int - nnccli nodeNetworkConfigSpecUpdater + initialized chan interface{} + nncSource chan v1alpha.NodeNetworkConfig + once sync.Once } -func NewCNSIPAMPoolMonitor(httpService cns.HTTPService, nnccli nodeNetworkConfigSpecUpdater) *CNSIPAMPoolMonitor { +func NewPoolMon(httpService cns.HTTPService, nnccli nodeNetworkConfigSpecUpdater) *PoolMon { logger.Printf("NewCNSIPAMPoolMonitor: Create IPAM Pool Monitor") - return &CNSIPAMPoolMonitor{ + return &PoolMon{ httpService: httpService, nnccli: nnccli, + initialized: make(chan interface{}), + nncSource: make(chan v1alpha.NodeNetworkConfig), } } -func (pm *CNSIPAMPoolMonitor) Start(ctx context.Context, poolMonitorRefreshMilliseconds int) error { +func (pm *PoolMon) Start(ctx context.Context, poolMonitorRefreshMilliseconds int) error { logger.Printf("[ipam-pool-monitor] Starting CNS IPAM Pool Monitor") ticker := time.NewTicker(time.Duration(poolMonitorRefreshMilliseconds) * time.Millisecond) for { + // block until something happens select { case <-ctx.Done(): return fmt.Errorf("[ipam-pool-monitor] CNS IPAM Pool Monitor received cancellation signal") case <-ticker.C: - err := pm.Reconcile(ctx) - if err != nil { - logger.Printf("[ipam-pool-monitor] Reconcile failed with err %v", err) - } + // block on ticks until we have initialized + <-pm.initialized + case nnc := <-pm.nncSource: + pm.nnc = &nnc + } + err := pm.reconcile(ctx) + if err != nil { + logger.Printf("[ipam-pool-monitor] Reconcile failed with err %v", err) } } } -func (pm *CNSIPAMPoolMonitor) Reconcile(ctx context.Context) error { +func (pm *PoolMon) reconcile(ctx context.Context) error { cnsPodIPConfigCount := len(pm.httpService.GetPodIPConfigState()) pendingProgramCount := len(pm.httpService.GetPendingProgramIPConfigs()) // TODO: add pending program count to real cns allocatedPodIPCount := len(pm.httpService.GetAllocatedIPConfigs()) pendingReleaseIPCount := len(pm.httpService.GetPendingReleaseIPConfigs()) availableIPConfigCount := len(pm.httpService.GetAvailableIPConfigs()) // TODO: add pending allocation count to real cns - requestedIPConfigCount := pm.cachedNNC.Spec.RequestedIPCount + requestedIPConfigCount := pm.nnc.Spec.RequestedIPCount unallocatedIPConfigCount := cnsPodIPConfigCount - allocatedPodIPCount freeIPConfigCount := requestedIPConfigCount - int64(allocatedPodIPCount) - batchSize := pm.getBatchSize() // Use getters in case customer changes batchsize manually - maxIPCount := pm.getMaxIPCount() + batchSize := pm.nnc.Status.Scaler.BatchSize + maxIPCount := pm.nnc.Status.Scaler.MaxIPCount - 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) + msg := fmt.Sprintf("[ipam-pool-monitor] Pool Size: %v, Goal Size: %v, BatchSize: %v, MaxIPCount: %v, Allocated: %v, Available: %v, Pending Release: %v, Free: %v, Pending Program: %v", + cnsPodIPConfigCount, pm.nnc.Spec.RequestedIPCount, batchSize, maxIPCount, allocatedPodIPCount, availableIPConfigCount, pendingReleaseIPCount, freeIPConfigCount, pendingProgramCount) ipamAllocatedIPCount.Set(float64(allocatedPodIPCount)) ipamAvailableIPCount.Set(float64(availableIPConfigCount)) @@ -83,8 +89,8 @@ func (pm *CNSIPAMPoolMonitor) Reconcile(ctx context.Context) error { switch { // pod count is increasing - case freeIPConfigCount < pm.MinimumFreeIps: - if pm.cachedNNC.Spec.RequestedIPCount == maxIPCount { + case freeIPConfigCount < int64(calculateMinFreeIPs(*pm.nnc)): + if pm.nnc.Spec.RequestedIPCount == maxIPCount { // If we're already at the maxIPCount, don't try to increase return nil } @@ -93,13 +99,13 @@ func (pm *CNSIPAMPoolMonitor) Reconcile(ctx context.Context) error { return pm.increasePoolSize(ctx) // pod count is decreasing - case freeIPConfigCount >= pm.MaximumFreeIps: + case freeIPConfigCount >= int64(calculateMaxFreeIPs(*pm.nnc)): 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 len(pm.cachedNNC.Spec.IPsNotInUse) != pendingReleaseIPCount: + case len(pm.nnc.Spec.IPsNotInUse) != pendingReleaseIPCount: logger.Printf("[ipam-pool-monitor] Removing Pending Release IP's from CRD...%s ", msg) return pm.cleanPendingRelease(ctx) @@ -112,16 +118,13 @@ func (pm *CNSIPAMPoolMonitor) Reconcile(ctx context.Context) error { return nil } -func (pm *CNSIPAMPoolMonitor) increasePoolSize(ctx context.Context) error { - pm.mu.Lock() - defer pm.mu.Unlock() - +func (pm *PoolMon) increasePoolSize(ctx context.Context) error { tempNNCSpec := pm.createNNCSpecForCRD() // Query the max IP count - maxIPCount := pm.getMaxIPCount() + maxIPCount := pm.nnc.Status.Scaler.MaxIPCount previouslyRequestedIPCount := tempNNCSpec.RequestedIPCount - batchSize := pm.getBatchSize() + batchSize := pm.nnc.Status.Scaler.BatchSize tempNNCSpec.RequestedIPCount += batchSize if tempNNCSpec.RequestedIPCount > maxIPCount { @@ -147,22 +150,19 @@ func (pm *CNSIPAMPoolMonitor) increasePoolSize(ctx context.Context) error { // start an alloc timer metric.StartPoolIncreaseTimer(int(batchSize)) // save the updated state to cachedSpec - pm.cachedNNC.Spec = tempNNCSpec + pm.nnc.Spec = tempNNCSpec return nil } -func (pm *CNSIPAMPoolMonitor) decreasePoolSize(ctx context.Context, existingPendingReleaseIPCount int) error { - pm.mu.Lock() - defer pm.mu.Unlock() - +func (pm *PoolMon) decreasePoolSize(ctx context.Context, existingPendingReleaseIPCount int) error { // mark n number of IP's as pending var newIpsMarkedAsPending bool var pendingIPAddresses map[string]cns.IPConfigurationStatus var updatedRequestedIPCount int64 // Ensure the updated requested IP count is a multiple of the batch size - previouslyRequestedIPCount := pm.cachedNNC.Spec.RequestedIPCount - batchSize := pm.getBatchSize() + previouslyRequestedIPCount := pm.nnc.Spec.RequestedIPCount + batchSize := pm.nnc.Status.Scaler.BatchSize modResult := previouslyRequestedIPCount % batchSize logger.Printf("[ipam-pool-monitor] Previously RequestedIP Count %v", previouslyRequestedIPCount) @@ -218,7 +218,7 @@ func (pm *CNSIPAMPoolMonitor) decreasePoolSize(ctx context.Context, existingPend metric.StartPoolDecreaseTimer(int(batchSize)) // save the updated state to cachedSpec - pm.cachedNNC.Spec = tempNNCSpec + pm.nnc.Spec = tempNNCSpec // clear the updatingPendingIpsNotInUse, as we have Updated the CRD logger.Printf("[ipam-pool-monitor] cleaning the updatingPendingIpsNotInUse, existing length %d", pm.updatingIpsNotInUseCount) @@ -229,10 +229,7 @@ func (pm *CNSIPAMPoolMonitor) decreasePoolSize(ctx context.Context, existingPend // cleanPendingRelease removes IPs from the cache and CRD if the request controller has reconciled // CNS state and the pending IP release map is empty. -func (pm *CNSIPAMPoolMonitor) cleanPendingRelease(ctx context.Context) error { - pm.mu.Lock() - defer pm.mu.Unlock() - +func (pm *PoolMon) cleanPendingRelease(ctx context.Context) error { tempNNCSpec := pm.createNNCSpecForCRD() _, err := pm.nnccli.UpdateSpec(ctx, &tempNNCSpec) @@ -244,16 +241,16 @@ func (pm *CNSIPAMPoolMonitor) cleanPendingRelease(ctx context.Context) error { logger.Printf("[ipam-pool-monitor] cleanPendingRelease: UpdateCRDSpec succeeded for spec %+v", tempNNCSpec) // save the updated state to cachedSpec - pm.cachedNNC.Spec = tempNNCSpec + pm.nnc.Spec = tempNNCSpec return nil } // createNNCSpecForCRD translates CNS's map of IPs to be released and requested IP count into an NNC Spec. -func (pm *CNSIPAMPoolMonitor) createNNCSpecForCRD() v1alpha.NodeNetworkConfigSpec { +func (pm *PoolMon) createNNCSpecForCRD() v1alpha.NodeNetworkConfigSpec { var spec v1alpha.NodeNetworkConfigSpec // Update the count from cached spec - spec.RequestedIPCount = pm.cachedNNC.Spec.RequestedIPCount + spec.RequestedIPCount = pm.nnc.Spec.RequestedIPCount // Get All Pending IPs from CNS and populate it again. pendingIPs := pm.httpService.GetPendingReleaseIPConfigs() @@ -264,53 +261,61 @@ func (pm *CNSIPAMPoolMonitor) createNNCSpecForCRD() v1alpha.NodeNetworkConfigSpe return spec } -// UpdatePoolLimitsTransacted called by request controller on reconcile to set the batch size limits -func (pm *CNSIPAMPoolMonitor) Update(scalar v1alpha.Scaler, spec v1alpha.NodeNetworkConfigSpec) { - pm.mu.Lock() - defer pm.mu.Unlock() - - pm.scalarUnits = scalar - - pm.MinimumFreeIps = int64(float64(pm.getBatchSize()) * (float64(pm.scalarUnits.RequestThresholdPercent) / 100)) - pm.MaximumFreeIps = int64(float64(pm.getBatchSize()) * (float64(pm.scalarUnits.ReleaseThresholdPercent) / 100)) +// GetStateSnapshot gets a snapshot of the IPAMPoolMonitor struct. +func (pm *PoolMon) GetStateSnapshot() cns.IpamPoolMonitorStateSnapshot { + nnc := *pm.nnc + return cns.IpamPoolMonitorStateSnapshot{ + MinimumFreeIps: calculateMinFreeIPs(nnc), + MaximumFreeIps: calculateMaxFreeIPs(nnc), + UpdatingIpsNotInUseCount: pm.updatingIpsNotInUseCount, + CachedNNC: nnc, + } +} - pm.cachedNNC.Spec = spec +// Update ingests a NodeNetworkConfig, clamping some values to ensure they are legal and then +// pushing it to the PoolMonitor's source channel. +// As a side effect, marks the PoolMonitor as initialized, if it is not already. +func (pm *PoolMon) Update(nnc *v1alpha.NodeNetworkConfig) { + clampMaxIPs(nnc) + clampBatchSize(nnc) // if the nnc has conveged, observe the pool scaling latency (if any) allocatedIPs := len(pm.httpService.GetPodIPConfigState()) - len(pm.httpService.GetPendingReleaseIPConfigs()) - if int(pm.cachedNNC.Spec.RequestedIPCount) == allocatedIPs { + if int(nnc.Spec.RequestedIPCount) == allocatedIPs { // observe elapsed duration for IP pool scaling metric.ObserverPoolScaleLatency() } - logger.Printf("[ipam-pool-monitor] Update spec %+v, pm.MinimumFreeIps %d, pm.MaximumFreeIps %d", - pm.cachedNNC.Spec, pm.MinimumFreeIps, pm.MaximumFreeIps) + // defer closing the init channel to signify that we have received at least one NodeNetworkConfig. + defer pm.once.Do(func() { close(pm.initialized) }) + pm.nncSource <- *nnc } -func (pm *CNSIPAMPoolMonitor) getMaxIPCount() int64 { - if pm.scalarUnits.MaxIPCount == 0 { - pm.scalarUnits.MaxIPCount = defaultMaxIPCount +func clampMaxIPs(nnc *v1alpha.NodeNetworkConfig) { + if nnc.Status.Scaler.MaxIPCount == 0 { + nnc.Status.Scaler.MaxIPCount = defaultMaxIPCount } - return pm.scalarUnits.MaxIPCount } -func (pm *CNSIPAMPoolMonitor) getBatchSize() int64 { - maxIPCount := pm.getMaxIPCount() - if pm.scalarUnits.BatchSize > maxIPCount { - pm.scalarUnits.BatchSize = maxIPCount +func clampBatchSize(nnc *v1alpha.NodeNetworkConfig) { + if nnc.Status.Scaler.BatchSize < 1 { + nnc.Status.Scaler.BatchSize = 1 + } + if nnc.Status.Scaler.BatchSize > nnc.Status.Scaler.MaxIPCount { + nnc.Status.Scaler.BatchSize = nnc.Status.Scaler.MaxIPCount } - return pm.scalarUnits.BatchSize } -// GetStateSnapshot gets a snapshot of the IPAMPoolMonitor struct. -func (pm *CNSIPAMPoolMonitor) GetStateSnapshot() cns.IpamPoolMonitorStateSnapshot { - pm.mu.Lock() - defer pm.mu.Unlock() +//nolint:gocritic // ignore hugeparam +func calculateMaxFreeIPs(nnc v1alpha.NodeNetworkConfig) int { + batchSize := nnc.Status.Scaler.BatchSize + requestThreshold := nnc.Status.Scaler.RequestThresholdPercent + return int(float64(batchSize) * (float64(requestThreshold) / 100)) //nolint:gomnd // it's a percent +} - return cns.IpamPoolMonitorStateSnapshot{ - MinimumFreeIps: pm.MinimumFreeIps, - MaximumFreeIps: pm.MaximumFreeIps, - UpdatingIpsNotInUseCount: pm.updatingIpsNotInUseCount, - CachedNNC: pm.cachedNNC, - } +//nolint:gocritic // ignore hugeparam +func calculateMinFreeIPs(nnc v1alpha.NodeNetworkConfig) int { + batchSize := nnc.Status.Scaler.BatchSize + releaseThreshold := nnc.Status.Scaler.ReleaseThresholdPercent + return int(float64(batchSize) * (float64(releaseThreshold) / 100)) //nolint:gomnd // it's a percent } diff --git a/cns/ipampoolmonitor/ipampoolmonitor_test.go b/cns/ipampoolmonitor/ipampoolmonitor_test.go index cf39ecf5c1..1ffe9d7a66 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor_test.go +++ b/cns/ipampoolmonitor/ipampoolmonitor_test.go @@ -2,7 +2,6 @@ package ipampoolmonitor import ( "context" - "log" "testing" "github.com/Azure/azure-container-networking/cns/fakes" @@ -24,7 +23,7 @@ func initFakes(t *testing.T, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent int, - maxPodIPCount int64) (*fakes.HTTPServiceFake, *fakes.RequestControllerFake, *CNSIPAMPoolMonitor) { + maxPodIPCount int64) (*fakes.HTTPServiceFake, *fakes.RequestControllerFake, *PoolMon) { logger.InitLogger("testlogs", 0, 0, "./") scalarUnits := v1alpha.Scaler{ @@ -38,8 +37,7 @@ func initFakes(t *testing.T, fakecns := fakes.NewHTTPServiceFake() fakerc := fakes.NewRequestControllerFake(fakecns, scalarUnits, subnetaddresspace, initialIPConfigCount) - poolmonitor := NewCNSIPAMPoolMonitor(fakecns, &fakeNodeNetworkConfigUpdater{fakerc.NNC}) - + poolmonitor := NewPoolMon(fakecns, &fakeNodeNetworkConfigUpdater{fakerc.NNC}) fakecns.PoolMonitor = poolmonitor err := fakerc.Reconcile(true) @@ -73,16 +71,16 @@ func TestPoolSizeIncrease(t *testing.T) { } // When poolmonitor reconcile is called, trigger increase and cache goal state - err = poolmonitor.Reconcile(context.Background()) + err = poolmonitor.reconcile(context.Background()) if err != nil { t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) } // ensure pool monitor has reached quorum with cns - if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(initialIPConfigCount+(1*batchSize)) { + if poolmonitor.nnc.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())) + "actual %v", poolmonitor.nnc.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) } // request controller reconciles, carves new IP's from the test subnet and adds to CNS state @@ -93,16 +91,16 @@ func TestPoolSizeIncrease(t *testing.T) { // when poolmonitor reconciles again here, the IP count will be within the thresholds // so no CRD update and nothing pending - err = poolmonitor.Reconcile(context.Background()) + err = poolmonitor.reconcile(context.Background()) if err != nil { t.Fatalf("Failed to reconcile pool monitor after request controller updates CNS state: %v", err) } // ensure pool monitor has reached quorum with cns - if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(initialIPConfigCount+(1*batchSize)) { + if poolmonitor.nnc.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())) + "actual %v", poolmonitor.nnc.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) } // make sure IPConfig state size reflects the new pool size @@ -113,7 +111,7 @@ func TestPoolSizeIncrease(t *testing.T) { t.Logf("Pool size %v, Target pool size %v, "+ "Allocated IP's %v, ", len(fakecns.GetPodIPConfigState()), - poolmonitor.cachedNNC.Spec.RequestedIPCount, len(fakecns.GetAllocatedIPConfigs())) + poolmonitor.nnc.Spec.RequestedIPCount, len(fakecns.GetAllocatedIPConfigs())) } func TestPoolIncreaseDoesntChangeWhenIncreaseIsAlreadyInProgress(t *testing.T) { @@ -139,7 +137,7 @@ func TestPoolIncreaseDoesntChangeWhenIncreaseIsAlreadyInProgress(t *testing.T) { } // When poolmonitor reconcile is called, trigger increase and cache goal state - err = poolmonitor.Reconcile(context.Background()) + err = poolmonitor.reconcile(context.Background()) if err != nil { t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) } @@ -151,15 +149,15 @@ func TestPoolIncreaseDoesntChangeWhenIncreaseIsAlreadyInProgress(t *testing.T) { } // poolmonitor reconciles, but doesn't actually update the CRD, because there is already a pending update - err = poolmonitor.Reconcile(context.Background()) + err = poolmonitor.reconcile(context.Background()) if err != nil { t.Fatalf("Failed to reconcile pool monitor after allocation ip increase with err: %v", err) } // ensure pool monitor has reached quorum with cns - if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(initialIPConfigCount+(1*batchSize)) { + if poolmonitor.nnc.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())) + " actual %v", poolmonitor.nnc.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) } // request controller reconciles, carves new IP's from the test subnet and adds to CNS state @@ -170,7 +168,7 @@ func TestPoolIncreaseDoesntChangeWhenIncreaseIsAlreadyInProgress(t *testing.T) { // when poolmonitor reconciles again here, the IP count will be within the thresholds // so no CRD update and nothing pending - err = poolmonitor.Reconcile(context.Background()) + err = poolmonitor.reconcile(context.Background()) if err != nil { t.Fatalf("Failed to reconcile pool monitor after request controller updates CNS state: %v", err) } @@ -182,13 +180,13 @@ func TestPoolIncreaseDoesntChangeWhenIncreaseIsAlreadyInProgress(t *testing.T) { } // ensure pool monitor has reached quorum with cns - if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(initialIPConfigCount+(1*batchSize)) { + if poolmonitor.nnc.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())) + "actual %v", poolmonitor.nnc.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())) + poolmonitor.nnc.Spec.RequestedIPCount, len(fakecns.GetAllocatedIPConfigs())) } func TestPoolSizeIncreaseIdempotency(t *testing.T) { @@ -207,8 +205,8 @@ func TestPoolSizeIncreaseIdempotency(t *testing.T) { releaseThresholdPercent, maxPodIPCount) - t.Logf("Minimum free IPs to request: %v", poolmonitor.MinimumFreeIps) - t.Logf("Maximum free IPs to release: %v", poolmonitor.MaximumFreeIps) + t.Logf("Minimum free IPs to request: %v", calculateMinFreeIPs(*poolmonitor.nnc)) + t.Logf("Maximum free IPs to release: %v", calculateMaxFreeIPs(*poolmonitor.nnc)) // increase number of allocated IP's in CNS err := fakecns.SetNumberOfAllocatedIPs(8) @@ -217,27 +215,27 @@ func TestPoolSizeIncreaseIdempotency(t *testing.T) { } // When poolmonitor reconcile is called, trigger increase and cache goal state - err = poolmonitor.Reconcile(context.Background()) + err = poolmonitor.reconcile(context.Background()) if err != nil { t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) } // ensure pool monitor has increased batch size - if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(initialIPConfigCount+(1*batchSize)) { + if poolmonitor.nnc.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())) + " actual %v", poolmonitor.nnc.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) } // reconcile pool monitor a second time, then verify requested ip count is still the same - err = poolmonitor.Reconcile(context.Background()) + err = poolmonitor.reconcile(context.Background()) if err != nil { t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) } // ensure pool monitor requested pool size is unchanged as request controller hasn't reconciled yet - if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(initialIPConfigCount+(1*batchSize)) { + if poolmonitor.nnc.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())) + " actual %v", poolmonitor.nnc.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) } } @@ -257,8 +255,8 @@ func TestPoolIncreasePastNodeLimit(t *testing.T) { releaseThresholdPercent, maxPodIPCount) - t.Logf("Minimum free IPs to request: %v", poolmonitor.MinimumFreeIps) - t.Logf("Maximum free IPs to release: %v", poolmonitor.MaximumFreeIps) + t.Logf("Minimum free IPs to request: %v", calculateMinFreeIPs(*poolmonitor.nnc)) + t.Logf("Maximum free IPs to release: %v", calculateMaxFreeIPs(*poolmonitor.nnc)) // increase number of allocated IP's in CNS err := fakecns.SetNumberOfAllocatedIPs(9) @@ -267,15 +265,15 @@ func TestPoolIncreasePastNodeLimit(t *testing.T) { } // When poolmonitor reconcile is called, trigger increase and cache goal state - err = poolmonitor.Reconcile(context.Background()) + err = poolmonitor.reconcile(context.Background()) if err != nil { t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) } // ensure pool monitor has only requested the max pod ip count - if poolmonitor.cachedNNC.Spec.RequestedIPCount != maxPodIPCount { + if poolmonitor.nnc.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) + "has been reached", poolmonitor.nnc.Spec.RequestedIPCount, maxPodIPCount) } } @@ -295,8 +293,8 @@ func TestPoolIncreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { releaseThresholdPercent, maxPodIPCount) - t.Logf("Minimum free IPs to request: %v", poolmonitor.MinimumFreeIps) - t.Logf("Maximum free IPs to release: %v", poolmonitor.MaximumFreeIps) + t.Logf("Minimum free IPs to request: %v", calculateMinFreeIPs(*poolmonitor.nnc)) + t.Logf("Maximum free IPs to release: %v", calculateMaxFreeIPs(*poolmonitor.nnc)) // increase number of allocated IP's in CNS err := fakecns.SetNumberOfAllocatedIPs(16) @@ -305,15 +303,15 @@ func TestPoolIncreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { } // When poolmonitor reconcile is called, trigger increase and cache goal state - err = poolmonitor.Reconcile(context.Background()) + err = poolmonitor.reconcile(context.Background()) if err != nil { t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) } // ensure pool monitor has only requested the max pod ip count - if poolmonitor.cachedNNC.Spec.RequestedIPCount != maxPodIPCount { + if poolmonitor.nnc.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) + "when the max has been reached", poolmonitor.nnc.Spec.RequestedIPCount, maxPodIPCount) } } @@ -330,9 +328,9 @@ func TestPoolIncreaseMaxIPCountSetToZero(t *testing.T) { _, _, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, initialMaxPodIPCount) - if poolmonitor.getMaxIPCount() != expectedMaxPodIPCount { + if poolmonitor.nnc.Status.Scaler.MaxIPCount != 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) + "when the MaxIPCount field in the CRD is zero", poolmonitor.nnc.Status.Scaler.MaxIPCount, expectedMaxPodIPCount) } } @@ -348,8 +346,8 @@ func TestPoolDecrease(t *testing.T) { 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) + t.Logf("Minimum free IPs to request: %v", calculateMinFreeIPs(*poolmonitor.nnc)) + t.Logf("Maximum free IPs to release: %v", calculateMaxFreeIPs(*poolmonitor.nnc)) // initial pool count is 20, set 15 of them to be allocated err := fakecns.SetNumberOfAllocatedIPs(15) @@ -358,7 +356,7 @@ func TestPoolDecrease(t *testing.T) { } // Pool monitor does nothing, as the current number of IP's falls in the threshold - err = poolmonitor.Reconcile(context.Background()) + err = poolmonitor.reconcile(context.Background()) if err != nil { t.Fatal(err) } @@ -370,15 +368,15 @@ func TestPoolDecrease(t *testing.T) { } // Pool monitor will adjust the spec so the pool size will be 1 batch size smaller - err = poolmonitor.Reconcile(context.Background()) + err = poolmonitor.reconcile(context.Background()) if err != nil { t.Fatal(err) } // ensure that the adjusted spec is smaller than the initial pool size - if len(poolmonitor.cachedNNC.Spec.IPsNotInUse) != (initialIPConfigCount - batchSize) { + if len(poolmonitor.nnc.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)) + (initialIPConfigCount - batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) } // reconcile the fake request controller @@ -407,8 +405,8 @@ func TestPoolSizeDecreaseWhenDecreaseHasAlreadyBeenRequested(t *testing.T) { 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) + t.Logf("Minimum free IPs to request: %v", calculateMinFreeIPs(*poolmonitor.nnc)) + t.Logf("Maximum free IPs to release: %v", calculateMaxFreeIPs(*poolmonitor.nnc)) // initial pool count is 30, set 25 of them to be allocated err := fakecns.SetNumberOfAllocatedIPs(5) @@ -417,21 +415,21 @@ func TestPoolSizeDecreaseWhenDecreaseHasAlreadyBeenRequested(t *testing.T) { } // Pool monitor does nothing, as the current number of IP's falls in the threshold - err = poolmonitor.Reconcile(context.Background()) + 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) } // Ensure the size of the requested spec is still the same - if len(poolmonitor.cachedNNC.Spec.IPsNotInUse) != (initialIPConfigCount - batchSize) { + if len(poolmonitor.nnc.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)) + " actual %v", (initialIPConfigCount - batchSize), len(poolmonitor.nnc.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) { + if poolmonitor.nnc.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)) + " actual %v", (initialIPConfigCount - batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) } // Update pods with IP count, ensure pool monitor stays the same until request controller reconciles @@ -441,17 +439,17 @@ func TestPoolSizeDecreaseWhenDecreaseHasAlreadyBeenRequested(t *testing.T) { } // Ensure the size of the requested spec is still the same - if len(poolmonitor.cachedNNC.Spec.IPsNotInUse) != (initialIPConfigCount - batchSize) { + if len(poolmonitor.nnc.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)) + (initialIPConfigCount - batchSize), len(poolmonitor.nnc.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) { + if poolmonitor.nnc.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)) + len(poolmonitor.nnc.Spec.IPsNotInUse)) } err = fakerc.Reconcile(true) @@ -459,15 +457,15 @@ func TestPoolSizeDecreaseWhenDecreaseHasAlreadyBeenRequested(t *testing.T) { t.Error(err) } - err = poolmonitor.Reconcile(context.Background()) + err = poolmonitor.reconcile(context.Background()) if err != nil { t.Errorf("Expected no pool monitor failure after request controller reconcile: %v", err) } // Ensure the spec doesn't have any IPsNotInUse after request controller has reconciled - if len(poolmonitor.cachedNNC.Spec.IPsNotInUse) != 0 { + if len(poolmonitor.nnc.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)) + (initialIPConfigCount - batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) } } @@ -487,8 +485,8 @@ func TestDecreaseAndIncreaseToSameCount(t *testing.T) { releaseThresholdPercent, maxPodIPCount) - log.Printf("Min free IP's %v", poolmonitor.MinimumFreeIps) - log.Printf("Max free IP %v", poolmonitor.MaximumFreeIps) + t.Logf("Minimum free IPs to request: %v", calculateMinFreeIPs(*poolmonitor.nnc)) + t.Logf("Maximum free IPs to release: %v", calculateMaxFreeIPs(*poolmonitor.nnc)) // initial pool count is 10, set 5 of them to be allocated err := fakecns.SetNumberOfAllocatedIPs(7) @@ -542,7 +540,7 @@ func TestDecreaseAndIncreaseToSameCount(t *testing.T) { t.Error(err) } - err = poolmonitor.Reconcile(context.Background()) + err = poolmonitor.reconcile(context.Background()) if err != nil { t.Errorf("Expected no pool monitor failure after request controller reconcile: %v", err) } @@ -565,8 +563,8 @@ func TestPoolSizeDecreaseToReallyLow(t *testing.T) { releaseThresholdPercent, maxPodIPCount) - log.Printf("Min free IP's %v", poolmonitor.MinimumFreeIps) - log.Printf("Max free IP %v", poolmonitor.MaximumFreeIps) + t.Logf("Minimum free IPs to request: %v", calculateMinFreeIPs(*poolmonitor.nnc)) + t.Logf("Maximum free IPs to release: %v", calculateMaxFreeIPs(*poolmonitor.nnc)) // initial pool count is 30, set 23 of them to be allocated err := fakecns.SetNumberOfAllocatedIPs(23) @@ -575,7 +573,7 @@ func TestPoolSizeDecreaseToReallyLow(t *testing.T) { } // Pool monitor does nothing, as the current number of IP's falls in the threshold - err = poolmonitor.Reconcile(context.Background()) + 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) } @@ -588,40 +586,40 @@ func TestPoolSizeDecreaseToReallyLow(t *testing.T) { // Pool monitor does nothing, as the current number of IP's falls in the threshold t.Logf("Reconcile after Allocated count from 33 -> 3, Exepected free count = 10") - err = poolmonitor.Reconcile(context.Background()) + 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) } // Ensure the size of the requested spec is still the same - if len(poolmonitor.cachedNNC.Spec.IPsNotInUse) != batchSize { + if len(poolmonitor.nnc.Spec.IPsNotInUse) != batchSize { t.Fatalf("Expected IP's not in use is not correct, expected %v, actual %v", - batchSize, len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) + batchSize, len(poolmonitor.nnc.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) { + if poolmonitor.nnc.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)) + "actual %v", (initialIPConfigCount - batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) } // Reconcile again, it should release the second batch t.Logf("Reconcile again - 2, Exepected free count = 20") - err = poolmonitor.Reconcile(context.Background()) + 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) } // Ensure the size of the requested spec is still the same - if len(poolmonitor.cachedNNC.Spec.IPsNotInUse) != batchSize*2 { + if len(poolmonitor.nnc.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)) + len(poolmonitor.nnc.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)) { + if poolmonitor.nnc.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)) + "actual %v", (initialIPConfigCount - batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) } t.Logf("Update Request Controller") @@ -630,15 +628,15 @@ func TestPoolSizeDecreaseToReallyLow(t *testing.T) { t.Error(err) } - err = poolmonitor.Reconcile(context.Background()) + err = poolmonitor.reconcile(context.Background()) if err != nil { t.Errorf("Expected no pool monitor failure after request controller reconcile: %v", err) } // Ensure the spec doesn't have any IPsNotInUse after request controller has reconciled - if len(poolmonitor.cachedNNC.Spec.IPsNotInUse) != 0 { + if len(poolmonitor.nnc.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)) + (initialIPConfigCount - batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) } } @@ -660,15 +658,15 @@ func TestDecreaseAfterNodeLimitReached(t *testing.T) { releaseThresholdPercent, maxPodIPCount) - t.Logf("Minimum free IPs to request: %v", poolmonitor.MinimumFreeIps) - t.Logf("Maximum free IPs to release: %v", poolmonitor.MaximumFreeIps) + t.Logf("Minimum free IPs to request: %v", calculateMinFreeIPs(*poolmonitor.nnc)) + t.Logf("Maximum free IPs to release: %v", calculateMaxFreeIPs(*poolmonitor.nnc)) err := fakecns.SetNumberOfAllocatedIPs(20) if err != nil { t.Error(err) } - err = poolmonitor.Reconcile(context.Background()) + 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) } @@ -679,23 +677,23 @@ func TestDecreaseAfterNodeLimitReached(t *testing.T) { t.Error(err) } - err = poolmonitor.Reconcile(context.Background()) + 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) } // Ensure poolmonitor asked for a multiple of batch size - if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(expectedRequestedIP) { + if poolmonitor.nnc.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) + poolmonitor.nnc.Spec.RequestedIPCount) } // Ensure we minused by the mod result - if len(poolmonitor.cachedNNC.Spec.IPsNotInUse) != expectedDecreaseIP { + if len(poolmonitor.nnc.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)) + "the max before scale down, but got %v", expectedDecreaseIP, len(poolmonitor.nnc.Spec.IPsNotInUse)) } } @@ -715,8 +713,8 @@ func TestPoolDecreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { releaseThresholdPercent, maxPodIPCount) - t.Logf("Minimum free IPs to request: %v", poolmonitor.MinimumFreeIps) - t.Logf("Maximum free IPs to release: %v", poolmonitor.MaximumFreeIps) + t.Logf("Minimum free IPs to request: %v", calculateMinFreeIPs(*poolmonitor.nnc)) + t.Logf("Maximum free IPs to release: %v", calculateMaxFreeIPs(*poolmonitor.nnc)) // increase number of allocated IP's in CNS err := fakecns.SetNumberOfAllocatedIPs(30) @@ -725,7 +723,7 @@ func TestPoolDecreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { } // When poolmonitor reconcile is called, trigger increase and cache goal state - err = poolmonitor.Reconcile(context.Background()) + err = poolmonitor.reconcile(context.Background()) if err != nil { t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) } @@ -736,39 +734,39 @@ func TestPoolDecreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { t.Error(err) } - err = poolmonitor.Reconcile(context.Background()) + 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) } // ensure pool monitor has only requested the max pod ip count - if poolmonitor.cachedNNC.Spec.RequestedIPCount != maxPodIPCount { + if poolmonitor.nnc.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) + "has been reached", poolmonitor.nnc.Spec.RequestedIPCount, maxPodIPCount) } } func ReconcileAndValidate(ctx context.Context, t *testing.T, - poolmonitor *CNSIPAMPoolMonitor, + poolmonitor *PoolMon, expectedRequestCount, expectedIpsNotInUse int) { - err := poolmonitor.Reconcile(context.Background()) + 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 != int64(expectedRequestCount) { + if poolmonitor.nnc.Spec.RequestedIPCount != int64(expectedRequestCount) { t.Fatalf("RequestIPCount not same, expected %v, actual %v", expectedRequestCount, - poolmonitor.cachedNNC.Spec.RequestedIPCount) + poolmonitor.nnc.Spec.RequestedIPCount) } // Ensure there is no pending release ips - if len(poolmonitor.cachedNNC.Spec.IPsNotInUse) != expectedIpsNotInUse { + if len(poolmonitor.nnc.Spec.IPsNotInUse) != expectedIpsNotInUse { t.Fatalf("Expected IP's not in use, expected %v, actual %v", expectedIpsNotInUse, - len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) + len(poolmonitor.nnc.Spec.IPsNotInUse)) } } diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index 4b7c949923..faa04cb562 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -211,8 +211,7 @@ func (service *HTTPRestService) SyncHostNCVersion(ctx context.Context, channelMo // This API will be called by CNS RequestController on CRD update. func (service *HTTPRestService) ReconcileNCState( - ncRequest *cns.CreateNetworkContainerRequest, podInfoByIP map[string]cns.PodInfo, scalar v1alpha.Scaler, - spec v1alpha.NodeNetworkConfigSpec) types.ResponseCode { + ncRequest *cns.CreateNetworkContainerRequest, podInfoByIP map[string]cns.PodInfo, nnc *v1alpha.NodeNetworkConfig) types.ResponseCode { logger.Printf("Reconciling NC state with podInfo %+v", podInfoByIP) // check if ncRequest is null, then return as there is no CRD state yet if ncRequest == nil { @@ -225,7 +224,7 @@ func (service *HTTPRestService) ReconcileNCState( if returnCode != types.Success { return returnCode } - service.IPAMPoolMonitor.Update(scalar, spec) + service.IPAMPoolMonitor.Update(nnc) // now parse the secondaryIP list, if it exists in PodInfo list, then allocate that ip for _, secIpConfig := range ncRequest.SecondaryIPConfigs { @@ -254,9 +253,9 @@ func (service *HTTPRestService) ReconcileNCState( } } - err := service.MarkExistingIPsAsPending(spec.IPsNotInUse) + err := service.MarkExistingIPsAsPending(nnc.Spec.IPsNotInUse) if err != nil { - logger.Errorf("[Azure CNS] Error. Failed to mark IP's as pending %v", spec.IPsNotInUse) + logger.Errorf("[Azure CNS] Error. Failed to mark IP's as pending %v", nnc.Spec.IPsNotInUse) return types.UnexpectedError } diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index 2e8609acdf..a9be924bcf 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -13,8 +13,8 @@ import ( "time" "github.com/Azure/azure-container-networking/cns" - "github.com/Azure/azure-container-networking/cns/fakes" "github.com/Azure/azure-container-networking/cns/types" + "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" "github.com/google/uuid" ) @@ -200,7 +200,18 @@ func TestReconcileNCWithEmptyState(t *testing.T) { expectedNcCount := len(svc.state.ContainerStatus) expectedAllocatedPods := make(map[string]cns.PodInfo) - returnCode := svc.ReconcileNCState(nil, expectedAllocatedPods, fakes.NewFakeScalar(releasePercent, requestPercent, batchSize), fakes.NewFakeNodeNetworkConfigSpec(initPoolSize)) + returnCode := svc.ReconcileNCState(nil, expectedAllocatedPods, &v1alpha.NodeNetworkConfig{ + Status: v1alpha.NodeNetworkConfigStatus{ + Scaler: v1alpha.Scaler{ + BatchSize: batchSize, + ReleaseThresholdPercent: releasePercent, + RequestThresholdPercent: requestPercent, + }, + }, + Spec: v1alpha.NodeNetworkConfigSpec{ + RequestedIPCount: initPoolSize, + }, + }) if returnCode != types.Success { t.Errorf("Unexpected failure on reconcile with no state %d", returnCode) } @@ -231,7 +242,18 @@ func TestReconcileNCWithExistingState(t *testing.T) { } expectedNcCount := len(svc.state.ContainerStatus) - returnCode := svc.ReconcileNCState(req, expectedAllocatedPods, fakes.NewFakeScalar(releasePercent, requestPercent, batchSize), fakes.NewFakeNodeNetworkConfigSpec(initPoolSize)) + returnCode := svc.ReconcileNCState(req, expectedAllocatedPods, &v1alpha.NodeNetworkConfig{ + Status: v1alpha.NodeNetworkConfigStatus{ + Scaler: v1alpha.Scaler{ + BatchSize: batchSize, + ReleaseThresholdPercent: releasePercent, + RequestThresholdPercent: requestPercent, + }, + }, + Spec: v1alpha.NodeNetworkConfigSpec{ + RequestedIPCount: initPoolSize, + }, + }) if returnCode != types.Success { t.Errorf("Unexpected failure on reconcile with no state %d", returnCode) } @@ -264,7 +286,18 @@ func TestReconcileNCWithExistingStateFromInterfaceID(t *testing.T) { } expectedNcCount := len(svc.state.ContainerStatus) - returnCode := svc.ReconcileNCState(req, expectedAllocatedPods, fakes.NewFakeScalar(releasePercent, requestPercent, batchSize), fakes.NewFakeNodeNetworkConfigSpec(initPoolSize)) + returnCode := svc.ReconcileNCState(req, expectedAllocatedPods, &v1alpha.NodeNetworkConfig{ + Status: v1alpha.NodeNetworkConfigStatus{ + Scaler: v1alpha.Scaler{ + BatchSize: batchSize, + ReleaseThresholdPercent: releasePercent, + RequestThresholdPercent: requestPercent, + }, + }, + Spec: v1alpha.NodeNetworkConfigSpec{ + RequestedIPCount: initPoolSize, + }, + }) if returnCode != types.Success { t.Errorf("Unexpected failure on reconcile with no state %d", returnCode) } @@ -296,7 +329,18 @@ func TestReconcileNCWithSystemPods(t *testing.T) { expectedAllocatedPods["192.168.0.1"] = cns.NewPodInfo("", "", "systempod", "kube-system") expectedNcCount := len(svc.state.ContainerStatus) - returnCode := svc.ReconcileNCState(req, expectedAllocatedPods, fakes.NewFakeScalar(releasePercent, requestPercent, batchSize), fakes.NewFakeNodeNetworkConfigSpec(initPoolSize)) + returnCode := svc.ReconcileNCState(req, expectedAllocatedPods, &v1alpha.NodeNetworkConfig{ + Status: v1alpha.NodeNetworkConfigStatus{ + Scaler: v1alpha.Scaler{ + BatchSize: batchSize, + ReleaseThresholdPercent: releasePercent, + RequestThresholdPercent: requestPercent, + }, + }, + Spec: v1alpha.NodeNetworkConfigSpec{ + RequestedIPCount: initPoolSize, + }, + }) if returnCode != types.Success { t.Errorf("Unexpected failure on reconcile with no state %d", returnCode) } @@ -382,9 +426,18 @@ func createAndValidateNCRequest(t *testing.T, secondaryIPConfigs map[string]cns. if returnCode != 0 { t.Fatalf("Failed to createNetworkContainerRequest, req: %+v, err: %d", req, returnCode) } - svc.IPAMPoolMonitor.Update( - fakes.NewFakeScalar(releasePercent, requestPercent, batchSize), - fakes.NewFakeNodeNetworkConfigSpec(initPoolSize)) + svc.IPAMPoolMonitor.Update(&v1alpha.NodeNetworkConfig{ + Status: v1alpha.NodeNetworkConfigStatus{ + Scaler: v1alpha.Scaler{ + BatchSize: batchSize, + ReleaseThresholdPercent: releasePercent, + RequestThresholdPercent: requestPercent, + }, + }, + Spec: v1alpha.NodeNetworkConfigSpec{ + RequestedIPCount: initPoolSize, + }, + }) validateNetworkRequest(t, *req) } @@ -546,9 +599,18 @@ func createNCReqInternal(t *testing.T, secondaryIPConfigs map[string]cns.Seconda if returnCode != 0 { t.Fatalf("Failed to createNetworkContainerRequest, req: %+v, err: %d", req, returnCode) } - svc.IPAMPoolMonitor.Update( - fakes.NewFakeScalar(releasePercent, requestPercent, batchSize), - fakes.NewFakeNodeNetworkConfigSpec(initPoolSize)) + svc.IPAMPoolMonitor.Update(&v1alpha.NodeNetworkConfig{ + Status: v1alpha.NodeNetworkConfigStatus{ + Scaler: v1alpha.Scaler{ + BatchSize: batchSize, + ReleaseThresholdPercent: releasePercent, + RequestThresholdPercent: requestPercent, + }, + }, + Spec: v1alpha.NodeNetworkConfigSpec{ + RequestedIPCount: initPoolSize, + }, + }) return *req } diff --git a/cns/restserver/ipam_test.go b/cns/restserver/ipam_test.go index db9259743d..634d4c0491 100644 --- a/cns/restserver/ipam_test.go +++ b/cns/restserver/ipam_test.go @@ -11,6 +11,7 @@ import ( "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/common" "github.com/Azure/azure-container-networking/cns/fakes" + "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" ) var ( @@ -613,9 +614,19 @@ func TestIPAMMarkIPAsPendingWithPendingProgrammingIPs(t *testing.T) { t.Fatalf("Failed to createNetworkContainerRequest, req: %+v, err: %d", req, returnCode) } svc.IPAMPoolMonitor.Update( - fakes.NewFakeScalar(releasePercent, requestPercent, batchSize), - fakes.NewFakeNodeNetworkConfigSpec(initPoolSize)) - + &v1alpha.NodeNetworkConfig{ + Status: v1alpha.NodeNetworkConfigStatus{ + Scaler: v1alpha.Scaler{ + BatchSize: batchSize, + ReleaseThresholdPercent: releasePercent, + RequestThresholdPercent: requestPercent, + }, + }, + Spec: v1alpha.NodeNetworkConfigSpec{ + RequestedIPCount: initPoolSize, + }, + }, + ) // Release pending programming IPs ips, err := svc.MarkIPAsPendingRelease(2) if err != nil { diff --git a/cns/service/main.go b/cns/service/main.go index 7a6fdfa146..edb09fa5d2 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -872,7 +872,7 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn scopedcli := kubecontroller.NewScopedClient(nnccli, types.NamespacedName{Namespace: "kube-system", Name: nodeName}) // initialize the ipam pool monitor - httpRestServiceImplementation.IPAMPoolMonitor = ipampoolmonitor.NewCNSIPAMPoolMonitor(httpRestServiceImplementation, scopedcli) + httpRestServiceImplementation.IPAMPoolMonitor = ipampoolmonitor.NewPoolMon(httpRestServiceImplementation, scopedcli) err = initCNS(ctx, scopedcli, httpRestServiceImplementation) if err != nil { return errors.Wrap(err, "failed to initialize CNS state") From 062d96dc9afc977722e7387767a395407c7a4ec7 Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Tue, 5 Oct 2021 23:24:17 -0500 Subject: [PATCH 02/14] update test Signed-off-by: Evan Baker --- cns/client/client_test.go | 12 ++++++------ cns/ipampoolmonitor/ipampoolmonitor_test.go | 2 ++ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/cns/client/client_test.go b/cns/client/client_test.go index ffaa2b76d4..51fd1d6562 100644 --- a/cns/client/client_test.go +++ b/cns/client/client_test.go @@ -39,8 +39,8 @@ const ( gatewayIp = "10.0.0.1" subnetPrfixLength = 24 dockerContainerType = cns.Docker - releasePercent = 50 - requestPercent = 100 + releasePercent = 150 + requestPercent = 50 batchSize = 10 initPoolSize = 10 ) @@ -176,8 +176,8 @@ func TestMain(m *testing.M) { Status: v1alpha.NodeNetworkConfigStatus{ Scaler: v1alpha.Scaler{ BatchSize: 10, - ReleaseThresholdPercent: 50, - RequestThresholdPercent: 40, + ReleaseThresholdPercent: 150, + RequestThresholdPercent: 50, }, NetworkContainers: []v1alpha.NetworkContainer{ { @@ -366,8 +366,8 @@ func TestCNSClientDebugAPI(t *testing.T) { // check for cached NNC Status struct values assert.EqualValues(t, 10, testIpamPoolMonitor.CachedNNC.Status.Scaler.BatchSize, "IPAMPoolMonitor cached NNC Status is not reflecting the initial set values") - assert.EqualValues(t, 50, testIpamPoolMonitor.CachedNNC.Status.Scaler.ReleaseThresholdPercent, "IPAMPoolMonitor cached NNC Status is not reflecting the initial set values") - assert.EqualValues(t, 40, testIpamPoolMonitor.CachedNNC.Status.Scaler.RequestThresholdPercent, "IPAMPoolMonitor cached NNC Status is not reflecting the initial set values") + assert.EqualValues(t, 150, testIpamPoolMonitor.CachedNNC.Status.Scaler.ReleaseThresholdPercent, "IPAMPoolMonitor cached NNC Status is not reflecting the initial set values") + assert.EqualValues(t, 50, testIpamPoolMonitor.CachedNNC.Status.Scaler.RequestThresholdPercent, "IPAMPoolMonitor cached NNC Status is not reflecting the initial set values") assert.Len(t, testIpamPoolMonitor.CachedNNC.Status.NetworkContainers, 1, "Expected only one Network Container in the list") t.Logf("In-memory Data: ") diff --git a/cns/ipampoolmonitor/ipampoolmonitor_test.go b/cns/ipampoolmonitor/ipampoolmonitor_test.go index 1ffe9d7a66..516306beb7 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor_test.go +++ b/cns/ipampoolmonitor/ipampoolmonitor_test.go @@ -38,6 +38,8 @@ func initFakes(t *testing.T, fakerc := fakes.NewRequestControllerFake(fakecns, scalarUnits, subnetaddresspace, initialIPConfigCount) poolmonitor := NewPoolMon(fakecns, &fakeNodeNetworkConfigUpdater{fakerc.NNC}) + poolmonitor.once.Do(func() { close(poolmonitor.initialized) }) + fakecns.PoolMonitor = poolmonitor err := fakerc.Reconcile(true) From 3a5721d18e36ff5232e845e05ee867826722a39e Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Wed, 6 Oct 2021 17:52:51 -0500 Subject: [PATCH 03/14] update packaging of ipampool and tests --- cns/client/client_test.go | 19 +++++--- cns/fakes/ipampoolmonitorfake.go | 35 --------------- cns/ipampool/fake.go | 37 ++++++++++++++++ cns/{ipampoolmonitor => ipampool}/metrics.go | 2 +- .../monitor.go} | 37 ++++++++-------- .../monitor_test.go} | 44 +++++++++---------- cns/restserver/api_test.go | 3 +- cns/restserver/ipam_test.go | 3 +- cns/service/main.go | 4 +- 9 files changed, 97 insertions(+), 87 deletions(-) delete mode 100644 cns/fakes/ipampoolmonitorfake.go create mode 100644 cns/ipampool/fake.go rename cns/{ipampoolmonitor => ipampool}/metrics.go (98%) rename cns/{ipampoolmonitor/ipampoolmonitor.go => ipampool/monitor.go} (91%) rename cns/{ipampoolmonitor/ipampoolmonitor_test.go => ipampool/monitor_test.go} (95%) diff --git a/cns/client/client_test.go b/cns/client/client_test.go index 51fd1d6562..bd6eab1bc8 100644 --- a/cns/client/client_test.go +++ b/cns/client/client_test.go @@ -18,6 +18,7 @@ import ( "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/common" "github.com/Azure/azure-container-networking/cns/fakes" + "github.com/Azure/azure-container-networking/cns/ipampool" "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/cns/restserver" "github.com/Azure/azure-container-networking/cns/types" @@ -98,15 +99,20 @@ func addTestStateToRestServer(t *testing.T, secondaryIps []string) { } svc.IPAMPoolMonitor.Update(&v1alpha.NodeNetworkConfig{ + Spec: v1alpha.NodeNetworkConfigSpec{ + RequestedIPCount: 16, + IPsNotInUse: []string{"abc"}, + }, Status: v1alpha.NodeNetworkConfigStatus{ Scaler: v1alpha.Scaler{ BatchSize: batchSize, ReleaseThresholdPercent: releasePercent, RequestThresholdPercent: requestPercent, + MaxIPCount: 250, + }, + NetworkContainers: []v1alpha.NetworkContainer{ + {}, }, - }, - Spec: v1alpha.NodeNetworkConfigSpec{ - RequestedIPCount: initPoolSize, }, }) } @@ -178,6 +184,7 @@ func TestMain(m *testing.M) { BatchSize: 10, ReleaseThresholdPercent: 150, RequestThresholdPercent: 50, + MaxIPCount: 250, }, NetworkContainers: []v1alpha.NetworkContainer{ { @@ -197,7 +204,7 @@ func TestMain(m *testing.M) { }, }, } - svc.IPAMPoolMonitor = &fakes.IPAMPoolMonitorFake{FakeIpsNotInUseCount: 13, FakecachedNNC: &fakeNNC} + svc.IPAMPoolMonitor = &ipampool.Fake{IPsNotInUseCount: 13, NodeNetworkConfig: &fakeNNC} if err != nil { logger.Errorf("Failed to create CNS object, err:%v.\n", err) @@ -356,8 +363,8 @@ func TestCNSClientDebugAPI(t *testing.T) { assert.GreaterOrEqual(t, len(inmemory.HTTPRestServiceData.PodIPConfigState), 1, "PodIpConfigState with at least 1 entry expected") testIpamPoolMonitor := inmemory.HTTPRestServiceData.IPAMPoolMonitor - assert.EqualValues(t, 10, testIpamPoolMonitor.MinimumFreeIps, "IPAMPoolMonitor state is not reflecting the initial set values") - assert.EqualValues(t, 20, testIpamPoolMonitor.MaximumFreeIps, "IPAMPoolMonitor state is not reflecting the initial set values") + assert.EqualValues(t, 5, testIpamPoolMonitor.MinimumFreeIps, "IPAMPoolMonitor state is not reflecting the initial set values") + assert.EqualValues(t, 15, testIpamPoolMonitor.MaximumFreeIps, "IPAMPoolMonitor state is not reflecting the initial set values") assert.Equal(t, 13, testIpamPoolMonitor.UpdatingIpsNotInUseCount, "IPAMPoolMonitor state is not reflecting the initial set values") // check for cached NNC Spec struct values diff --git a/cns/fakes/ipampoolmonitorfake.go b/cns/fakes/ipampoolmonitorfake.go deleted file mode 100644 index cbb528ae83..0000000000 --- a/cns/fakes/ipampoolmonitorfake.go +++ /dev/null @@ -1,35 +0,0 @@ -//go:build !ignore_uncovered -// +build !ignore_uncovered - -package fakes - -import ( - "context" - - "github.com/Azure/azure-container-networking/cns" - "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" -) - -type IPAMPoolMonitorFake struct { - FakeIpsNotInUseCount int - FakecachedNNC *v1alpha.NodeNetworkConfig -} - -func (ipm *IPAMPoolMonitorFake) Start(ctx context.Context, poolMonitorRefreshMilliseconds int) error { - return nil -} - -func (ipm *IPAMPoolMonitorFake) Update(nnc *v1alpha.NodeNetworkConfig) { - ipm.FakecachedNNC = nnc -} - -func (ipm *IPAMPoolMonitorFake) Reconcile() error { - return nil -} - -func (ipm *IPAMPoolMonitorFake) GetStateSnapshot() cns.IpamPoolMonitorStateSnapshot { - return cns.IpamPoolMonitorStateSnapshot{ - UpdatingIpsNotInUseCount: ipm.FakeIpsNotInUseCount, - CachedNNC: *ipm.FakecachedNNC, - } -} diff --git a/cns/ipampool/fake.go b/cns/ipampool/fake.go new file mode 100644 index 0000000000..6299ee2c98 --- /dev/null +++ b/cns/ipampool/fake.go @@ -0,0 +1,37 @@ +//go:build !ignore_uncovered +// +build !ignore_uncovered + +package ipampool + +import ( + "context" + + "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" +) + +type Fake struct { + IPsNotInUseCount int + NodeNetworkConfig *v1alpha.NodeNetworkConfig +} + +func (*Fake) Start(ctx context.Context, poolMonitorRefreshMilliseconds int) error { + return nil +} + +func (f *Fake) Update(nnc *v1alpha.NodeNetworkConfig) { + f.NodeNetworkConfig = nnc +} + +func (*Fake) Reconcile() error { + return nil +} + +func (f *Fake) GetStateSnapshot() cns.IpamPoolMonitorStateSnapshot { + return cns.IpamPoolMonitorStateSnapshot{ + MaximumFreeIps: CalculateMaxFreeIPs(*f.NodeNetworkConfig), + MinimumFreeIps: CalculateMinFreeIPs(*f.NodeNetworkConfig), + UpdatingIpsNotInUseCount: f.IPsNotInUseCount, + CachedNNC: *f.NodeNetworkConfig, + } +} diff --git a/cns/ipampoolmonitor/metrics.go b/cns/ipampool/metrics.go similarity index 98% rename from cns/ipampoolmonitor/metrics.go rename to cns/ipampool/metrics.go index 7c9b8ce901..a553cdbaf6 100644 --- a/cns/ipampoolmonitor/metrics.go +++ b/cns/ipampool/metrics.go @@ -1,4 +1,4 @@ -package ipampoolmonitor +package ipampool import ( "github.com/prometheus/client_golang/prometheus" diff --git a/cns/ipampoolmonitor/ipampoolmonitor.go b/cns/ipampool/monitor.go similarity index 91% rename from cns/ipampoolmonitor/ipampoolmonitor.go rename to cns/ipampool/monitor.go index f77e086ac0..b94160bd59 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor.go +++ b/cns/ipampool/monitor.go @@ -1,4 +1,4 @@ -package ipampoolmonitor +package ipampool import ( "context" @@ -18,7 +18,7 @@ type nodeNetworkConfigSpecUpdater interface { UpdateSpec(context.Context, *v1alpha.NodeNetworkConfigSpec) (*v1alpha.NodeNetworkConfig, error) } -type PoolMon struct { +type Monitor struct { nnc *v1alpha.NodeNetworkConfig nnccli nodeNetworkConfigSpecUpdater httpService cns.HTTPService @@ -28,9 +28,8 @@ type PoolMon struct { once sync.Once } -func NewPoolMon(httpService cns.HTTPService, nnccli nodeNetworkConfigSpecUpdater) *PoolMon { - logger.Printf("NewCNSIPAMPoolMonitor: Create IPAM Pool Monitor") - return &PoolMon{ +func NewMonitor(httpService cns.HTTPService, nnccli nodeNetworkConfigSpecUpdater) *Monitor { + return &Monitor{ httpService: httpService, nnccli: nnccli, initialized: make(chan interface{}), @@ -38,7 +37,7 @@ func NewPoolMon(httpService cns.HTTPService, nnccli nodeNetworkConfigSpecUpdater } } -func (pm *PoolMon) Start(ctx context.Context, poolMonitorRefreshMilliseconds int) error { +func (pm *Monitor) Start(ctx context.Context, poolMonitorRefreshMilliseconds int) error { logger.Printf("[ipam-pool-monitor] Starting CNS IPAM Pool Monitor") ticker := time.NewTicker(time.Duration(poolMonitorRefreshMilliseconds) * time.Millisecond) @@ -61,7 +60,7 @@ func (pm *PoolMon) Start(ctx context.Context, poolMonitorRefreshMilliseconds int } } -func (pm *PoolMon) reconcile(ctx context.Context) error { +func (pm *Monitor) reconcile(ctx context.Context) error { cnsPodIPConfigCount := len(pm.httpService.GetPodIPConfigState()) pendingProgramCount := len(pm.httpService.GetPendingProgramIPConfigs()) // TODO: add pending program count to real cns allocatedPodIPCount := len(pm.httpService.GetAllocatedIPConfigs()) @@ -89,7 +88,7 @@ func (pm *PoolMon) reconcile(ctx context.Context) error { switch { // pod count is increasing - case freeIPConfigCount < int64(calculateMinFreeIPs(*pm.nnc)): + case freeIPConfigCount < int64(CalculateMinFreeIPs(*pm.nnc)): if pm.nnc.Spec.RequestedIPCount == maxIPCount { // If we're already at the maxIPCount, don't try to increase return nil @@ -99,7 +98,7 @@ func (pm *PoolMon) reconcile(ctx context.Context) error { return pm.increasePoolSize(ctx) // pod count is decreasing - case freeIPConfigCount >= int64(calculateMaxFreeIPs(*pm.nnc)): + case freeIPConfigCount >= int64(CalculateMaxFreeIPs(*pm.nnc)): logger.Printf("[ipam-pool-monitor] Decreasing pool size...%s ", msg) return pm.decreasePoolSize(ctx, pendingReleaseIPCount) @@ -118,7 +117,7 @@ func (pm *PoolMon) reconcile(ctx context.Context) error { return nil } -func (pm *PoolMon) increasePoolSize(ctx context.Context) error { +func (pm *Monitor) increasePoolSize(ctx context.Context) error { tempNNCSpec := pm.createNNCSpecForCRD() // Query the max IP count @@ -154,7 +153,7 @@ func (pm *PoolMon) increasePoolSize(ctx context.Context) error { return nil } -func (pm *PoolMon) decreasePoolSize(ctx context.Context, existingPendingReleaseIPCount int) error { +func (pm *Monitor) decreasePoolSize(ctx context.Context, existingPendingReleaseIPCount int) error { // mark n number of IP's as pending var newIpsMarkedAsPending bool var pendingIPAddresses map[string]cns.IPConfigurationStatus @@ -229,7 +228,7 @@ func (pm *PoolMon) decreasePoolSize(ctx context.Context, existingPendingReleaseI // cleanPendingRelease removes IPs from the cache and CRD if the request controller has reconciled // CNS state and the pending IP release map is empty. -func (pm *PoolMon) cleanPendingRelease(ctx context.Context) error { +func (pm *Monitor) cleanPendingRelease(ctx context.Context) error { tempNNCSpec := pm.createNNCSpecForCRD() _, err := pm.nnccli.UpdateSpec(ctx, &tempNNCSpec) @@ -246,7 +245,7 @@ func (pm *PoolMon) cleanPendingRelease(ctx context.Context) error { } // createNNCSpecForCRD translates CNS's map of IPs to be released and requested IP count into an NNC Spec. -func (pm *PoolMon) createNNCSpecForCRD() v1alpha.NodeNetworkConfigSpec { +func (pm *Monitor) createNNCSpecForCRD() v1alpha.NodeNetworkConfigSpec { var spec v1alpha.NodeNetworkConfigSpec // Update the count from cached spec @@ -262,11 +261,11 @@ func (pm *PoolMon) createNNCSpecForCRD() v1alpha.NodeNetworkConfigSpec { } // GetStateSnapshot gets a snapshot of the IPAMPoolMonitor struct. -func (pm *PoolMon) GetStateSnapshot() cns.IpamPoolMonitorStateSnapshot { +func (pm *Monitor) GetStateSnapshot() cns.IpamPoolMonitorStateSnapshot { nnc := *pm.nnc return cns.IpamPoolMonitorStateSnapshot{ - MinimumFreeIps: calculateMinFreeIPs(nnc), - MaximumFreeIps: calculateMaxFreeIPs(nnc), + MinimumFreeIps: CalculateMinFreeIPs(nnc), + MaximumFreeIps: CalculateMaxFreeIPs(nnc), UpdatingIpsNotInUseCount: pm.updatingIpsNotInUseCount, CachedNNC: nnc, } @@ -275,7 +274,7 @@ func (pm *PoolMon) GetStateSnapshot() cns.IpamPoolMonitorStateSnapshot { // Update ingests a NodeNetworkConfig, clamping some values to ensure they are legal and then // pushing it to the PoolMonitor's source channel. // As a side effect, marks the PoolMonitor as initialized, if it is not already. -func (pm *PoolMon) Update(nnc *v1alpha.NodeNetworkConfig) { +func (pm *Monitor) Update(nnc *v1alpha.NodeNetworkConfig) { clampMaxIPs(nnc) clampBatchSize(nnc) @@ -307,14 +306,14 @@ func clampBatchSize(nnc *v1alpha.NodeNetworkConfig) { } //nolint:gocritic // ignore hugeparam -func calculateMaxFreeIPs(nnc v1alpha.NodeNetworkConfig) int { +func CalculateMinFreeIPs(nnc v1alpha.NodeNetworkConfig) int { batchSize := nnc.Status.Scaler.BatchSize requestThreshold := nnc.Status.Scaler.RequestThresholdPercent return int(float64(batchSize) * (float64(requestThreshold) / 100)) //nolint:gomnd // it's a percent } //nolint:gocritic // ignore hugeparam -func calculateMinFreeIPs(nnc v1alpha.NodeNetworkConfig) int { +func CalculateMaxFreeIPs(nnc v1alpha.NodeNetworkConfig) int { batchSize := nnc.Status.Scaler.BatchSize releaseThreshold := nnc.Status.Scaler.ReleaseThresholdPercent return int(float64(batchSize) * (float64(releaseThreshold) / 100)) //nolint:gomnd // it's a percent diff --git a/cns/ipampoolmonitor/ipampoolmonitor_test.go b/cns/ipampool/monitor_test.go similarity index 95% rename from cns/ipampoolmonitor/ipampoolmonitor_test.go rename to cns/ipampool/monitor_test.go index 516306beb7..5a3d82f0b8 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor_test.go +++ b/cns/ipampool/monitor_test.go @@ -1,4 +1,4 @@ -package ipampoolmonitor +package ipampool import ( "context" @@ -23,7 +23,7 @@ func initFakes(t *testing.T, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent int, - maxPodIPCount int64) (*fakes.HTTPServiceFake, *fakes.RequestControllerFake, *PoolMon) { + maxPodIPCount int64) (*fakes.HTTPServiceFake, *fakes.RequestControllerFake, *Monitor) { logger.InitLogger("testlogs", 0, 0, "./") scalarUnits := v1alpha.Scaler{ @@ -37,7 +37,7 @@ func initFakes(t *testing.T, fakecns := fakes.NewHTTPServiceFake() fakerc := fakes.NewRequestControllerFake(fakecns, scalarUnits, subnetaddresspace, initialIPConfigCount) - poolmonitor := NewPoolMon(fakecns, &fakeNodeNetworkConfigUpdater{fakerc.NNC}) + poolmonitor := NewMonitor(fakecns, &fakeNodeNetworkConfigUpdater{fakerc.NNC}) poolmonitor.once.Do(func() { close(poolmonitor.initialized) }) fakecns.PoolMonitor = poolmonitor @@ -207,8 +207,8 @@ func TestPoolSizeIncreaseIdempotency(t *testing.T) { releaseThresholdPercent, maxPodIPCount) - t.Logf("Minimum free IPs to request: %v", calculateMinFreeIPs(*poolmonitor.nnc)) - t.Logf("Maximum free IPs to release: %v", calculateMaxFreeIPs(*poolmonitor.nnc)) + t.Logf("Minimum free IPs to request: %v", CalculateMinFreeIPs(*poolmonitor.nnc)) + t.Logf("Maximum free IPs to release: %v", CalculateMaxFreeIPs(*poolmonitor.nnc)) // increase number of allocated IP's in CNS err := fakecns.SetNumberOfAllocatedIPs(8) @@ -257,8 +257,8 @@ func TestPoolIncreasePastNodeLimit(t *testing.T) { releaseThresholdPercent, maxPodIPCount) - t.Logf("Minimum free IPs to request: %v", calculateMinFreeIPs(*poolmonitor.nnc)) - t.Logf("Maximum free IPs to release: %v", calculateMaxFreeIPs(*poolmonitor.nnc)) + t.Logf("Minimum free IPs to request: %v", CalculateMinFreeIPs(*poolmonitor.nnc)) + t.Logf("Maximum free IPs to release: %v", CalculateMaxFreeIPs(*poolmonitor.nnc)) // increase number of allocated IP's in CNS err := fakecns.SetNumberOfAllocatedIPs(9) @@ -295,8 +295,8 @@ func TestPoolIncreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { releaseThresholdPercent, maxPodIPCount) - t.Logf("Minimum free IPs to request: %v", calculateMinFreeIPs(*poolmonitor.nnc)) - t.Logf("Maximum free IPs to release: %v", calculateMaxFreeIPs(*poolmonitor.nnc)) + t.Logf("Minimum free IPs to request: %v", CalculateMinFreeIPs(*poolmonitor.nnc)) + t.Logf("Maximum free IPs to release: %v", CalculateMaxFreeIPs(*poolmonitor.nnc)) // increase number of allocated IP's in CNS err := fakecns.SetNumberOfAllocatedIPs(16) @@ -348,8 +348,8 @@ func TestPoolDecrease(t *testing.T) { fakecns, fakerc, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) - t.Logf("Minimum free IPs to request: %v", calculateMinFreeIPs(*poolmonitor.nnc)) - t.Logf("Maximum free IPs to release: %v", calculateMaxFreeIPs(*poolmonitor.nnc)) + t.Logf("Minimum free IPs to request: %v", CalculateMinFreeIPs(*poolmonitor.nnc)) + t.Logf("Maximum free IPs to release: %v", CalculateMaxFreeIPs(*poolmonitor.nnc)) // initial pool count is 20, set 15 of them to be allocated err := fakecns.SetNumberOfAllocatedIPs(15) @@ -407,8 +407,8 @@ func TestPoolSizeDecreaseWhenDecreaseHasAlreadyBeenRequested(t *testing.T) { fakecns, fakerc, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) - t.Logf("Minimum free IPs to request: %v", calculateMinFreeIPs(*poolmonitor.nnc)) - t.Logf("Maximum free IPs to release: %v", calculateMaxFreeIPs(*poolmonitor.nnc)) + t.Logf("Minimum free IPs to request: %v", CalculateMinFreeIPs(*poolmonitor.nnc)) + t.Logf("Maximum free IPs to release: %v", CalculateMaxFreeIPs(*poolmonitor.nnc)) // initial pool count is 30, set 25 of them to be allocated err := fakecns.SetNumberOfAllocatedIPs(5) @@ -487,8 +487,8 @@ func TestDecreaseAndIncreaseToSameCount(t *testing.T) { releaseThresholdPercent, maxPodIPCount) - t.Logf("Minimum free IPs to request: %v", calculateMinFreeIPs(*poolmonitor.nnc)) - t.Logf("Maximum free IPs to release: %v", calculateMaxFreeIPs(*poolmonitor.nnc)) + t.Logf("Minimum free IPs to request: %v", CalculateMinFreeIPs(*poolmonitor.nnc)) + t.Logf("Maximum free IPs to release: %v", CalculateMaxFreeIPs(*poolmonitor.nnc)) // initial pool count is 10, set 5 of them to be allocated err := fakecns.SetNumberOfAllocatedIPs(7) @@ -565,8 +565,8 @@ func TestPoolSizeDecreaseToReallyLow(t *testing.T) { releaseThresholdPercent, maxPodIPCount) - t.Logf("Minimum free IPs to request: %v", calculateMinFreeIPs(*poolmonitor.nnc)) - t.Logf("Maximum free IPs to release: %v", calculateMaxFreeIPs(*poolmonitor.nnc)) + t.Logf("Minimum free IPs to request: %v", CalculateMinFreeIPs(*poolmonitor.nnc)) + t.Logf("Maximum free IPs to release: %v", CalculateMaxFreeIPs(*poolmonitor.nnc)) // initial pool count is 30, set 23 of them to be allocated err := fakecns.SetNumberOfAllocatedIPs(23) @@ -660,8 +660,8 @@ func TestDecreaseAfterNodeLimitReached(t *testing.T) { releaseThresholdPercent, maxPodIPCount) - t.Logf("Minimum free IPs to request: %v", calculateMinFreeIPs(*poolmonitor.nnc)) - t.Logf("Maximum free IPs to release: %v", calculateMaxFreeIPs(*poolmonitor.nnc)) + t.Logf("Minimum free IPs to request: %v", CalculateMinFreeIPs(*poolmonitor.nnc)) + t.Logf("Maximum free IPs to release: %v", CalculateMaxFreeIPs(*poolmonitor.nnc)) err := fakecns.SetNumberOfAllocatedIPs(20) if err != nil { @@ -715,8 +715,8 @@ func TestPoolDecreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { releaseThresholdPercent, maxPodIPCount) - t.Logf("Minimum free IPs to request: %v", calculateMinFreeIPs(*poolmonitor.nnc)) - t.Logf("Maximum free IPs to release: %v", calculateMaxFreeIPs(*poolmonitor.nnc)) + t.Logf("Minimum free IPs to request: %v", CalculateMinFreeIPs(*poolmonitor.nnc)) + t.Logf("Maximum free IPs to release: %v", CalculateMaxFreeIPs(*poolmonitor.nnc)) // increase number of allocated IP's in CNS err := fakecns.SetNumberOfAllocatedIPs(30) @@ -750,7 +750,7 @@ func TestPoolDecreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { func ReconcileAndValidate(ctx context.Context, t *testing.T, - poolmonitor *PoolMon, + poolmonitor *Monitor, expectedRequestCount, expectedIpsNotInUse int) { err := poolmonitor.reconcile(context.Background()) diff --git a/cns/restserver/api_test.go b/cns/restserver/api_test.go index 6577681123..4bff22fa19 100644 --- a/cns/restserver/api_test.go +++ b/cns/restserver/api_test.go @@ -20,6 +20,7 @@ import ( "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/common" "github.com/Azure/azure-container-networking/cns/fakes" + "github.com/Azure/azure-container-networking/cns/ipampool" "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/cns/nmagentclient" "github.com/Azure/azure-container-networking/cns/types" @@ -925,7 +926,7 @@ func startService() error { return err } - svc.IPAMPoolMonitor = &fakes.IPAMPoolMonitorFake{} + svc.IPAMPoolMonitor = &ipampool.Fake{} if service != nil { // Create empty azure-cns.json. CNS should start successfully by deleting this file diff --git a/cns/restserver/ipam_test.go b/cns/restserver/ipam_test.go index 634d4c0491..30b223eb39 100644 --- a/cns/restserver/ipam_test.go +++ b/cns/restserver/ipam_test.go @@ -11,6 +11,7 @@ import ( "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/common" "github.com/Azure/azure-container-networking/cns/fakes" + "github.com/Azure/azure-container-networking/cns/ipampool" "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" ) @@ -37,7 +38,7 @@ func getTestService() *HTTPRestService { var config common.ServiceConfig httpsvc, _ := NewHTTPRestService(&config, fakes.NewFakeImdsClient(), fakes.NewFakeNMAgentClient()) svc = httpsvc.(*HTTPRestService) - svc.IPAMPoolMonitor = &fakes.IPAMPoolMonitorFake{} + svc.IPAMPoolMonitor = &ipampool.Fake{} setOrchestratorTypeInternal(cns.KubernetesCRD) return svc diff --git a/cns/service/main.go b/cns/service/main.go index edb09fa5d2..1a779a6032 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -28,7 +28,7 @@ import ( "github.com/Azure/azure-container-networking/cns/configuration" "github.com/Azure/azure-container-networking/cns/hnsclient" "github.com/Azure/azure-container-networking/cns/imdsclient" - "github.com/Azure/azure-container-networking/cns/ipampoolmonitor" + "github.com/Azure/azure-container-networking/cns/ipampool" "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/cns/multitenantcontroller" "github.com/Azure/azure-container-networking/cns/multitenantcontroller/multitenantoperator" @@ -872,7 +872,7 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn scopedcli := kubecontroller.NewScopedClient(nnccli, types.NamespacedName{Namespace: "kube-system", Name: nodeName}) // initialize the ipam pool monitor - httpRestServiceImplementation.IPAMPoolMonitor = ipampoolmonitor.NewPoolMon(httpRestServiceImplementation, scopedcli) + httpRestServiceImplementation.IPAMPoolMonitor = ipampool.NewMonitor(httpRestServiceImplementation, scopedcli) err = initCNS(ctx, scopedcli, httpRestServiceImplementation) if err != nil { return errors.Wrap(err, "failed to initialize CNS state") From 53d2695dab351c938e2da859bd7203f427b1c7d6 Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Wed, 6 Oct 2021 18:15:13 -0500 Subject: [PATCH 04/14] add big scale-up to e2e to trigger pool scaling Signed-off-by: Evan Baker --- cns/ipampool/monitor.go | 4 ++++ test/integration/k8s_test.go | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/cns/ipampool/monitor.go b/cns/ipampool/monitor.go index b94160bd59..852c2e99c8 100644 --- a/cns/ipampool/monitor.go +++ b/cns/ipampool/monitor.go @@ -305,6 +305,8 @@ func clampBatchSize(nnc *v1alpha.NodeNetworkConfig) { } } +// CalculateMinFreeIPs calculates the minimum free IP quantity based on the Scaler +// in the passed NodeNetworkConfig. //nolint:gocritic // ignore hugeparam func CalculateMinFreeIPs(nnc v1alpha.NodeNetworkConfig) int { batchSize := nnc.Status.Scaler.BatchSize @@ -312,6 +314,8 @@ func CalculateMinFreeIPs(nnc v1alpha.NodeNetworkConfig) int { return int(float64(batchSize) * (float64(requestThreshold) / 100)) //nolint:gomnd // it's a percent } +// CalculateMaxFreeIPs calculates the maximum free IP quantity based on the Scaler +// in the passed NodeNetworkConfig. //nolint:gocritic // ignore hugeparam func CalculateMaxFreeIPs(nnc v1alpha.NodeNetworkConfig) int { batchSize := nnc.Status.Scaler.BatchSize diff --git a/test/integration/k8s_test.go b/test/integration/k8s_test.go index 71ee58c0fd..8566e066a5 100644 --- a/test/integration/k8s_test.go +++ b/test/integration/k8s_test.go @@ -142,7 +142,7 @@ func TestPodScaling(t *testing.T) { } }) - counts := []int{15, 5, 15} + counts := []int{15, 5, 15, 150, 15} for _, c := range counts { count := c From 80e1ea8db4c267da9d812345b1862f82141d905a Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Thu, 7 Oct 2021 13:40:01 -0500 Subject: [PATCH 05/14] update pool monitor ifaces Signed-off-by: Evan Baker --- cns/client/client_test.go | 2 +- cns/ipampool/fake.go | 13 ++++------ cns/restserver/api_test.go | 2 +- cns/restserver/ipam_test.go | 2 +- cns/service/main.go | 8 +++--- cns/singletenantcontroller/reconciler.go | 6 ++--- cns/singletenantcontroller/reconciler_test.go | 26 +++++++++---------- 7 files changed, 28 insertions(+), 31 deletions(-) diff --git a/cns/client/client_test.go b/cns/client/client_test.go index bd6eab1bc8..29a149c41b 100644 --- a/cns/client/client_test.go +++ b/cns/client/client_test.go @@ -204,7 +204,7 @@ func TestMain(m *testing.M) { }, }, } - svc.IPAMPoolMonitor = &ipampool.Fake{IPsNotInUseCount: 13, NodeNetworkConfig: &fakeNNC} + svc.IPAMPoolMonitor = &ipampool.MonitorFake{IPsNotInUseCount: 13, NodeNetworkConfig: &fakeNNC} if err != nil { logger.Errorf("Failed to create CNS object, err:%v.\n", err) diff --git a/cns/ipampool/fake.go b/cns/ipampool/fake.go index 6299ee2c98..92f770a236 100644 --- a/cns/ipampool/fake.go +++ b/cns/ipampool/fake.go @@ -1,6 +1,3 @@ -//go:build !ignore_uncovered -// +build !ignore_uncovered - package ipampool import ( @@ -10,24 +7,24 @@ import ( "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" ) -type Fake struct { +type MonitorFake struct { IPsNotInUseCount int NodeNetworkConfig *v1alpha.NodeNetworkConfig } -func (*Fake) Start(ctx context.Context, poolMonitorRefreshMilliseconds int) error { +func (*MonitorFake) Start(ctx context.Context, poolMonitorRefreshMilliseconds int) error { return nil } -func (f *Fake) Update(nnc *v1alpha.NodeNetworkConfig) { +func (f *MonitorFake) Update(nnc *v1alpha.NodeNetworkConfig) { f.NodeNetworkConfig = nnc } -func (*Fake) Reconcile() error { +func (*MonitorFake) Reconcile() error { return nil } -func (f *Fake) GetStateSnapshot() cns.IpamPoolMonitorStateSnapshot { +func (f *MonitorFake) GetStateSnapshot() cns.IpamPoolMonitorStateSnapshot { return cns.IpamPoolMonitorStateSnapshot{ MaximumFreeIps: CalculateMaxFreeIPs(*f.NodeNetworkConfig), MinimumFreeIps: CalculateMinFreeIPs(*f.NodeNetworkConfig), diff --git a/cns/restserver/api_test.go b/cns/restserver/api_test.go index 4bff22fa19..e0176985bd 100644 --- a/cns/restserver/api_test.go +++ b/cns/restserver/api_test.go @@ -926,7 +926,7 @@ func startService() error { return err } - svc.IPAMPoolMonitor = &ipampool.Fake{} + svc.IPAMPoolMonitor = &ipampool.MonitorFake{} if service != nil { // Create empty azure-cns.json. CNS should start successfully by deleting this file diff --git a/cns/restserver/ipam_test.go b/cns/restserver/ipam_test.go index 30b223eb39..f0ff82f57e 100644 --- a/cns/restserver/ipam_test.go +++ b/cns/restserver/ipam_test.go @@ -38,7 +38,7 @@ func getTestService() *HTTPRestService { var config common.ServiceConfig httpsvc, _ := NewHTTPRestService(&config, fakes.NewFakeImdsClient(), fakes.NewFakeNMAgentClient()) svc = httpsvc.(*HTTPRestService) - svc.IPAMPoolMonitor = &ipampool.Fake{} + svc.IPAMPoolMonitor = &ipampool.MonitorFake{} setOrchestratorTypeInternal(cns.KubernetesCRD) return svc diff --git a/cns/service/main.go b/cns/service/main.go index 1a779a6032..7d3da2179c 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -784,7 +784,7 @@ type nodeNetworkConfigGetter interface { } type ncStateReconciler interface { - ReconcileNCState(ncRequest *cns.CreateNetworkContainerRequest, podInfoByIP map[string]cns.PodInfo, scalar v1alpha.Scaler, spec v1alpha.NodeNetworkConfigSpec) cnstypes.ResponseCode + ReconcileNCState(ncRequest *cns.CreateNetworkContainerRequest, podInfoByIP map[string]cns.PodInfo, nnc *v1alpha.NodeNetworkConfig) cnstypes.ResponseCode } // TODO(rbtr) where should this live?? @@ -800,7 +800,7 @@ func initCNS(ctx context.Context, cli nodeNetworkConfigGetter, ncReconciler ncSt // If instance of crd is not found, pass nil to CNSClient if client.IgnoreNotFound(err) == nil { - err = restserver.ResponseCodeToError(ncReconciler.ReconcileNCState(nil, nil, nnc.Status.Scaler, nnc.Spec)) + err = restserver.ResponseCodeToError(ncReconciler.ReconcileNCState(nil, nil, nnc)) return errors.Wrap(err, "failed to reconcile NC state") } @@ -810,7 +810,7 @@ func initCNS(ctx context.Context, cli nodeNetworkConfigGetter, ncReconciler ncSt // If there are no NCs, pass nil to CNSClient if len(nnc.Status.NetworkContainers) == 0 { - err = restserver.ResponseCodeToError(ncReconciler.ReconcileNCState(nil, nil, nnc.Status.Scaler, nnc.Spec)) + err = restserver.ResponseCodeToError(ncReconciler.ReconcileNCState(nil, nil, nnc)) return errors.Wrap(err, "failed to reconcile NC state") } @@ -833,7 +833,7 @@ func initCNS(ctx context.Context, cli nodeNetworkConfigGetter, ncReconciler ncSt // errors.Wrap provides additional context, and return nil if the err input arg is nil // Call cnsclient init cns passing those two things. - err = restserver.ResponseCodeToError(ncReconciler.ReconcileNCState(&ncRequest, podInfoByIP, nnc.Status.Scaler, nnc.Spec)) + err = restserver.ResponseCodeToError(ncReconciler.ReconcileNCState(&ncRequest, podInfoByIP, nnc)) return errors.Wrap(err, "err in CNS reconciliation") } diff --git a/cns/singletenantcontroller/reconciler.go b/cns/singletenantcontroller/reconciler.go index 99249efb8a..748196c4dc 100644 --- a/cns/singletenantcontroller/reconciler.go +++ b/cns/singletenantcontroller/reconciler.go @@ -23,7 +23,7 @@ type cnsClient interface { } type ipamPoolMonitorClient interface { - Update(scalar v1alpha.Scaler, spec v1alpha.NodeNetworkConfigSpec) + Update(*v1alpha.NodeNetworkConfig) } type nncGetter interface { @@ -81,7 +81,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco return reconcile.Result{}, errors.Wrap(err, "failed to create or update network container") } - r.ipampoolmonitorcli.Update(nnc.Status.Scaler, nnc.Spec) + r.ipampoolmonitorcli.Update(nnc) // record assigned IPs metric assignedIPs.Set(float64(len(nnc.Status.NetworkContainers[0].IPAssignments))) @@ -103,7 +103,7 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager, nodeName string) error { return nodeName == object.GetName() })). WithEventFilter(predicate.Funcs{ - // check that the generation is the same - status changes don't update generation. + // check that the generation is the same - status changes don't update generation.a UpdateFunc: func(ue event.UpdateEvent) bool { return ue.ObjectOld.GetGeneration() == ue.ObjectNew.GetGeneration() }, diff --git a/cns/singletenantcontroller/reconciler_test.go b/cns/singletenantcontroller/reconciler_test.go index c82ee24755..9e396c3ae2 100644 --- a/cns/singletenantcontroller/reconciler_test.go +++ b/cns/singletenantcontroller/reconciler_test.go @@ -18,15 +18,14 @@ import ( ) type cnsClientState struct { - req *cns.CreateNetworkContainerRequest - scaler v1alpha.Scaler - spec v1alpha.NodeNetworkConfigSpec + req *cns.CreateNetworkContainerRequest + nnc *v1alpha.NodeNetworkConfig } type mockCNSClient struct { state cnsClientState createOrUpdateNC func(*cns.CreateNetworkContainerRequest) cnstypes.ResponseCode - update func(v1alpha.Scaler, v1alpha.NodeNetworkConfigSpec) + update func(*v1alpha.NodeNetworkConfig) } //nolint:gocritic // ignore hugeParam pls @@ -35,10 +34,9 @@ func (m *mockCNSClient) CreateOrUpdateNetworkContainerInternal(req *cns.CreateNe return m.createOrUpdateNC(req) } -func (m *mockCNSClient) Update(scaler v1alpha.Scaler, spec v1alpha.NodeNetworkConfigSpec) { - m.state.scaler = scaler - m.state.spec = spec - m.update(scaler, spec) +func (m *mockCNSClient) Update(nnc *v1alpha.NodeNetworkConfig) { + m.state.nnc = nnc + m.update(nnc) } type mockNCGetter struct { @@ -133,14 +131,16 @@ func TestReconcile(t *testing.T) { createOrUpdateNC: func(*cns.CreateNetworkContainerRequest) cnstypes.ResponseCode { return cnstypes.Success }, - update: func(v1alpha.Scaler, v1alpha.NodeNetworkConfigSpec) {}, + update: func(*v1alpha.NodeNetworkConfig) {}, }, wantErr: false, wantCNSClientState: cnsClientState{ - req: &validRequest, - scaler: validStatus.Scaler, - spec: v1alpha.NodeNetworkConfigSpec{ - RequestedIPCount: 1, + req: &validRequest, + nnc: &v1alpha.NodeNetworkConfig{ + Status: validStatus, + Spec: v1alpha.NodeNetworkConfigSpec{ + RequestedIPCount: 1, + }, }, }, }, From 6b04e9085e306c3e12d2adae08a1061cb50dfb92 Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Thu, 7 Oct 2021 18:11:31 -0500 Subject: [PATCH 06/14] fix tests Signed-off-by: Evan Baker --- cns/api.go | 2 +- cns/fakes/cnsfake.go | 27 +- cns/ipampool/fake.go | 2 +- cns/ipampool/monitor.go | 48 +- cns/ipampool/monitor_test.go | 901 ++++++++++++++++++++++------------- cns/service/main.go | 7 +- 6 files changed, 611 insertions(+), 376 deletions(-) diff --git a/cns/api.go b/cns/api.go index 37e3775e2e..816849fa12 100644 --- a/cns/api.go +++ b/cns/api.go @@ -214,7 +214,7 @@ type NodeConfiguration struct { } type IPAMPoolMonitor interface { - Start(ctx context.Context, poolMonitorRefreshMilliseconds int) error + Start(ctx context.Context) error Update(nnc *v1alpha.NodeNetworkConfig) GetStateSnapshot() IpamPoolMonitorStateSnapshot } diff --git a/cns/fakes/cnsfake.go b/cns/fakes/cnsfake.go index 1c86d1cd2b..7fb9f9dfc9 100644 --- a/cns/fakes/cnsfake.go +++ b/cns/fakes/cnsfake.go @@ -13,42 +13,27 @@ import ( "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/common" "github.com/Azure/azure-container-networking/cns/types" - "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" ) type StringStack struct { - lock sync.Mutex // you don't have to do this if you don't want thread safety + sync.Mutex items []string } -func NewFakeScalar(releaseThreshold, requestThreshold, batchSize int) v1alpha.Scaler { - return v1alpha.Scaler{ - BatchSize: int64(batchSize), - ReleaseThresholdPercent: int64(releaseThreshold), - RequestThresholdPercent: int64(requestThreshold), - } -} - -func NewFakeNodeNetworkConfigSpec(requestedIPCount int) v1alpha.NodeNetworkConfigSpec { - return v1alpha.NodeNetworkConfigSpec{ - RequestedIPCount: int64(requestedIPCount), - } -} - func NewStack() *StringStack { - return &StringStack{sync.Mutex{}, make([]string, 0)} + return &StringStack{items: make([]string, 0)} } func (stack *StringStack) Push(v string) { - stack.lock.Lock() - defer stack.lock.Unlock() + stack.Lock() + defer stack.Unlock() stack.items = append(stack.items, v) } func (stack *StringStack) Pop() (string, error) { - stack.lock.Lock() - defer stack.lock.Unlock() + stack.Lock() + defer stack.Unlock() length := len(stack.items) if length == 0 { diff --git a/cns/ipampool/fake.go b/cns/ipampool/fake.go index 92f770a236..be221bc242 100644 --- a/cns/ipampool/fake.go +++ b/cns/ipampool/fake.go @@ -12,7 +12,7 @@ type MonitorFake struct { NodeNetworkConfig *v1alpha.NodeNetworkConfig } -func (*MonitorFake) Start(ctx context.Context, poolMonitorRefreshMilliseconds int) error { +func (*MonitorFake) Start(ctx context.Context) error { return nil } diff --git a/cns/ipampool/monitor.go b/cns/ipampool/monitor.go index 852c2e99c8..2235e743ae 100644 --- a/cns/ipampool/monitor.go +++ b/cns/ipampool/monitor.go @@ -10,15 +10,23 @@ import ( "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/cns/metric" "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" + "github.com/pkg/errors" ) -const defaultMaxIPCount = int64(250) +const ( + DefaultRefreshDelay = 1 * time.Second +) type nodeNetworkConfigSpecUpdater interface { UpdateSpec(context.Context, *v1alpha.NodeNetworkConfigSpec) (*v1alpha.NodeNetworkConfig, error) } +type Options struct { + RefreshDelay time.Duration +} + type Monitor struct { + opts *Options nnc *v1alpha.NodeNetworkConfig nnccli nodeNetworkConfigSpecUpdater httpService cns.HTTPService @@ -28,8 +36,12 @@ type Monitor struct { once sync.Once } -func NewMonitor(httpService cns.HTTPService, nnccli nodeNetworkConfigSpecUpdater) *Monitor { +func NewMonitor(httpService cns.HTTPService, nnccli nodeNetworkConfigSpecUpdater, opts *Options) *Monitor { + if opts.RefreshDelay < 1 { + opts.RefreshDelay = DefaultRefreshDelay + } return &Monitor{ + opts: opts, httpService: httpService, nnccli: nnccli, initialized: make(chan interface{}), @@ -37,16 +49,17 @@ func NewMonitor(httpService cns.HTTPService, nnccli nodeNetworkConfigSpecUpdater } } -func (pm *Monitor) Start(ctx context.Context, poolMonitorRefreshMilliseconds int) error { +func (pm *Monitor) Start(ctx context.Context) error { logger.Printf("[ipam-pool-monitor] Starting CNS IPAM Pool Monitor") - ticker := time.NewTicker(time.Duration(poolMonitorRefreshMilliseconds) * time.Millisecond) + ticker := time.NewTicker(pm.opts.RefreshDelay) + defer ticker.Stop() for { // block until something happens select { case <-ctx.Done(): - return fmt.Errorf("[ipam-pool-monitor] CNS IPAM Pool Monitor received cancellation signal") + return errors.Wrap(ctx.Err(), "pool monitor context closed") case <-ticker.C: // block on ticks until we have initialized <-pm.initialized @@ -275,8 +288,7 @@ func (pm *Monitor) GetStateSnapshot() cns.IpamPoolMonitorStateSnapshot { // pushing it to the PoolMonitor's source channel. // As a side effect, marks the PoolMonitor as initialized, if it is not already. func (pm *Monitor) Update(nnc *v1alpha.NodeNetworkConfig) { - clampMaxIPs(nnc) - clampBatchSize(nnc) + clampScaler(nnc) // if the nnc has conveged, observe the pool scaling latency (if any) allocatedIPs := len(pm.httpService.GetPodIPConfigState()) - len(pm.httpService.GetPendingReleaseIPConfigs()) @@ -290,19 +302,29 @@ func (pm *Monitor) Update(nnc *v1alpha.NodeNetworkConfig) { pm.nncSource <- *nnc } -func clampMaxIPs(nnc *v1alpha.NodeNetworkConfig) { - if nnc.Status.Scaler.MaxIPCount == 0 { - nnc.Status.Scaler.MaxIPCount = defaultMaxIPCount +// clampScaler makes sure that the values stored in the scaler are sane. +// we usually expect these to be correctly set for us, but we could crash +// without these checks. if they are incorrectly set, there will be some weird +// IP pool behavior for a while until the nnc reconciler corrects the state. +func clampScaler(nnc *v1alpha.NodeNetworkConfig) { + if nnc.Status.Scaler.MaxIPCount < 1 { + nnc.Status.Scaler.MaxIPCount = 1 } -} - -func clampBatchSize(nnc *v1alpha.NodeNetworkConfig) { if nnc.Status.Scaler.BatchSize < 1 { nnc.Status.Scaler.BatchSize = 1 } if nnc.Status.Scaler.BatchSize > nnc.Status.Scaler.MaxIPCount { nnc.Status.Scaler.BatchSize = nnc.Status.Scaler.MaxIPCount } + if nnc.Status.Scaler.RequestThresholdPercent < 1 { + nnc.Status.Scaler.RequestThresholdPercent = 1 + } + if nnc.Status.Scaler.RequestThresholdPercent > 100 { + nnc.Status.Scaler.RequestThresholdPercent = 100 + } + if nnc.Status.Scaler.ReleaseThresholdPercent < nnc.Status.Scaler.RequestThresholdPercent+100 { + nnc.Status.Scaler.ReleaseThresholdPercent = nnc.Status.Scaler.RequestThresholdPercent + 100 //nolint:gomnd // it's a percent + } } // CalculateMinFreeIPs calculates the minimum free IP quantity based on the Scaler diff --git a/cns/ipampool/monitor_test.go b/cns/ipampool/monitor_test.go index 5a3d82f0b8..c419799078 100644 --- a/cns/ipampool/monitor_test.go +++ b/cns/ipampool/monitor_test.go @@ -3,10 +3,13 @@ package ipampool import ( "context" "testing" + "time" + "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/fakes" "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" + "github.com/stretchr/testify/assert" ) type fakeNodeNetworkConfigUpdater struct { @@ -18,68 +21,437 @@ func (f *fakeNodeNetworkConfigUpdater) UpdateSpec(ctx context.Context, spec *v1a return f.nnc, nil } -func initFakes(t *testing.T, - batchSize, - initialIPConfigCount, - requestThresholdPercent, - releaseThresholdPercent int, - maxPodIPCount int64) (*fakes.HTTPServiceFake, *fakes.RequestControllerFake, *Monitor) { +type directUpdatePoolMonitor struct { + m *Monitor + cns.IPAMPoolMonitor +} + +func (d *directUpdatePoolMonitor) Update(nnc *v1alpha.NodeNetworkConfig) { + d.m.nnc = nnc +} + +type state struct { + allocatedIPCount int + batchSize int + ipConfigCount int + maxIPCount int + releaseThresholdPercent int + requestThresholdPercent int +} + +func initFakes(initState state) (*fakes.HTTPServiceFake, *fakes.RequestControllerFake, *Monitor) { logger.InitLogger("testlogs", 0, 0, "./") scalarUnits := v1alpha.Scaler{ - BatchSize: int64(batchSize), - RequestThresholdPercent: int64(requestThresholdPercent), - ReleaseThresholdPercent: int64(releaseThresholdPercent), - MaxIPCount: int64(maxPodIPCount), + BatchSize: int64(initState.batchSize), + RequestThresholdPercent: int64(initState.requestThresholdPercent), + ReleaseThresholdPercent: int64(initState.releaseThresholdPercent), + MaxIPCount: int64(initState.maxIPCount), } subnetaddresspace := "10.0.0.0/8" fakecns := fakes.NewHTTPServiceFake() - fakerc := fakes.NewRequestControllerFake(fakecns, scalarUnits, subnetaddresspace, initialIPConfigCount) + fakerc := fakes.NewRequestControllerFake(fakecns, scalarUnits, subnetaddresspace, initState.ipConfigCount) - poolmonitor := NewMonitor(fakecns, &fakeNodeNetworkConfigUpdater{fakerc.NNC}) - poolmonitor.once.Do(func() { close(poolmonitor.initialized) }) + poolmonitor := NewMonitor(fakecns, &fakeNodeNetworkConfigUpdater{fakerc.NNC}, &Options{RefreshDelay: 100 * time.Second}) - fakecns.PoolMonitor = poolmonitor - - err := fakerc.Reconcile(true) - if err != nil { - t.Fatalf("Failed to reconcile fake requestcontroller with err: %v", err) - } + fakecns.PoolMonitor = &directUpdatePoolMonitor{m: poolmonitor} + _ = fakecns.SetNumberOfAllocatedIPs(initState.allocatedIPCount) return fakecns, fakerc, poolmonitor } +// func TestReconcile(t *testing.T) { +// tests := []struct { +// name string +// init state +// want state +// }{ +// { +// name: "increase", +// init: state{ +// initState.batchSize: 10, +// allocatedIPCount: 8, +// ipConfigCount: 10, +// requestThresholdPercent: 50, +// releaseThresholdPercent: 150, +// maxIPCount: 30, +// }, +// want: state{ +// allocatedIPCount: 8, +// ipConfigCount: 20, +// }, +// }, +// { +// name: "increase idempotency", +// init: state{ +// initState.batchSize: 10, +// allocatedIPCount: 8, +// ipConfigCount: 10, +// requestThresholdPercent: 50, +// releaseThresholdPercent: 150, +// maxIPCount: 30, +// }, +// want: state{ +// allocatedIPCount: 9, +// ipConfigCount: 20, +// }, +// }, +// { +// name: "increase cap at node limit", +// init: state{ +// initState.batchSize: 16, +// allocatedIPCount: 9, +// ipConfigCount: 16, +// requestThresholdPercent: 50, +// releaseThresholdPercent: 150, +// maxIPCount: 30, +// }, +// want: state{ +// allocatedIPCount: 9, +// ipConfigCount: 30, +// }, +// }, +// { +// name: "increase cap batch at node limit", +// init: state{ +// initState.batchSize: 50, +// allocatedIPCount: 16, +// ipConfigCount: 16, +// requestThresholdPercent: 50, +// releaseThresholdPercent: 150, +// maxIPCount: 30, +// }, +// want: state{ +// allocatedIPCount: 16, +// ipConfigCount: 30, +// }, +// }, +// { +// name: "decrease", +// init: state{ +// initState.batchSize: 10, +// allocatedIPCount: 15, +// ipConfigCount: 20, +// requestThresholdPercent: 50, +// releaseThresholdPercent: 150, +// maxIPCount: 30, +// }, +// want: state{ +// allocatedIPCount: 4, +// ipConfigCount: 10, +// }, +// }, +// { +// name: "decrease in-progress idempotency", +// init: state{ +// initState.batchSize: 10, +// allocatedIPCount: 5, +// ipConfigCount: 20, +// requestThresholdPercent: 50, +// releaseThresholdPercent: 150, +// maxIPCount: 30, +// }, +// want: state{ +// allocatedIPCount: 6, +// ipConfigCount: 10, +// }, +// }, +// { +// name: "decrease multiple batches", +// init: state{ +// initState.batchSize: 10, +// allocatedIPCount: 23, +// ipConfigCount: 30, +// requestThresholdPercent: 50, +// releaseThresholdPercent: 150, +// maxIPCount: 30, +// }, +// want: state{ +// allocatedIPCount: 3, +// ipConfigCount: 10, +// }, +// }, + +// { +// name: "decrease to zero", +// init: state{ +// initState.batchSize: 10, +// allocatedIPCount: 23, +// ipConfigCount: 30, +// requestThresholdPercent: 50, +// releaseThresholdPercent: 150, +// maxIPCount: 30, +// }, +// want: state{ +// allocatedIPCount: 0, +// ipConfigCount: 10, +// }, +// }, +// } +// for _, tt := range tests { +// tt := tt +// t.Run(tt.name, func(t *testing.T) { +// fakecns, fakerc, poolmonitor := initFakes(tt.init) +// require.NoError(t, fakerc.Reconcile(true)) +// require.NoError(t, poolmonitor.reconcile(context.Background())) +// require.NoError(t, fakecns.SetNumberOfAllocatedIPs(tt.want.allocatedIPCount)) + +// // reconcile until CNS and pool monitor have converged +// for tt.want.ipConfigCount != len(fakecns.GetPodIPConfigState()) || tt.want.ipConfigCount != int(poolmonitor.nnc.Spec.RequestedIPCount) { +// // request controller reconciles, carves new IPs from the test subnet and adds to CNS state +// require.NoError(t, fakerc.Reconcile(true)) +// // pool monitor reconciles, makes new pool size requests +// require.NoError(t, poolmonitor.reconcile(context.Background())) +// } + +// require.Zero(t, len(fakecns.GetPendingReleaseIPConfigs())) +// }) +// } +// } + +// func TestPoolSizeDecreaseToReallyLow(t *testing.T) { +// var ( +// initState.batchSize = 10 +// initialAllocated = 23 +// initState.ipConfigCount = 30 +// requestThresholdPercent = 30 +// releaseThresholdPercent = 100 +// maxIPCount = 30 +// ) + +// fakecns, fakerc, poolmonitor := initFakes(t, +// initState.batchSize, +// initState.ipConfigCount, +// requestThresholdPercent, +// releaseThresholdPercent, +// maxIPCount, +// initialAllocated, +// ) + +// // Pool monitor does nothing, as the current number of IP's falls in the threshold +// assert.NoError(t, poolmonitor.reconcile(context.Background())) + +// // Now Drop the Allocated count to really low, say 3. This should trigger release in 2 batches +// assert.NoError(t, fakecns.SetNumberOfAllocatedIPs(3)) + +// // Pool monitor does nothing, as the current number of IP's falls in the threshold +// t.Logf("Reconcile after Allocated count from 33 -> 3, Exepected free count = 10") +// assert.NoError(t, poolmonitor.reconcile(context.Background())) + +// // Ensure the size of the requested spec is still the same +// if len(poolmonitor.nnc.Spec.IPsNotInUse) != initState.batchSize { +// t.Fatalf("Expected IP's not in use is not correct, expected %v, actual %v", +// initState.batchSize, len(poolmonitor.nnc.Spec.IPsNotInUse)) +// } + +// // Ensure the request ipcount is now one batch size smaller than the initial IP count +// if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.ipConfigCount-initState.batchSize) { +// t.Fatalf("Expected pool size to be one batch size smaller after reconcile, expected %v, "+ +// "actual %v", (initState.ipConfigCount - initState.batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) +// } + +// // Reconcile again, it should release the second batch +// t.Logf("Reconcile again - 2, Exepected free count = 20") +// assert.NoError(t, poolmonitor.reconcile(context.Background())) + +// // Ensure the size of the requested spec is still the same +// if len(poolmonitor.nnc.Spec.IPsNotInUse) != initState.batchSize*2 { +// t.Fatalf("Expected IP's not in use is not correct, expected %v, actual %v", initState.batchSize*2, +// len(poolmonitor.nnc.Spec.IPsNotInUse)) +// } + +// // Ensure the request ipcount is now one batch size smaller than the inital IP count +// if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.ipConfigCount-(initState.batchSize*2)) { +// t.Fatalf("Expected pool size to be one batch size smaller after reconcile, expected %v, "+ +// "actual %v", (initState.ipConfigCount - initState.batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) +// } + +// t.Logf("Update Request Controller") +// assert.NoError(t, fakerc.Reconcile(true)) + +// assert.NoError(t, poolmonitor.reconcile(context.Background())) + +// // Ensure the spec doesn't have any IPsNotInUse after request controller has reconciled +// if len(poolmonitor.nnc.Spec.IPsNotInUse) != 0 { +// t.Fatalf("Expected IP's not in use to be 0 after reconcile, expected %v, actual %v", +// (initState.ipConfigCount - initState.batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) +// } +// } + +// func TestDecreaseAfterNodeLimitReached(t *testing.T) { +// var ( +// initState.batchSize = 16 +// initialAllocated = 20 +// initState.ipConfigCount = 30 +// requestThresholdPercent = 50 +// releaseThresholdPercent = 150 +// maxIPCount = 30 +// expectedRequestedIP = 16 +// expectedDecreaseIP = int(maxIPCount) % initState.batchSize +// ) + +// fakecns, _, poolmonitor := initFakes(t, +// initState.batchSize, +// initState.ipConfigCount, +// requestThresholdPercent, +// releaseThresholdPercent, +// maxIPCount, +// initialAllocated, +// ) + +// assert.NoError(t, poolmonitor.reconcile(context.Background())) + +// // Trigger a batch release +// assert.NoError(t, fakecns.SetNumberOfAllocatedIPs(5)) + +// assert.NoError(t, poolmonitor.reconcile(context.Background())) + +// // Ensure poolmonitor asked for a multiple of batch size +// if poolmonitor.nnc.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, maxIPCount, +// poolmonitor.nnc.Spec.RequestedIPCount) +// } + +// // Ensure we minused by the mod result +// if len(poolmonitor.nnc.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.nnc.Spec.IPsNotInUse)) +// } +// } + +// func TestPoolDecreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { +// var ( +// initState.batchSize = 31 +// initialAllocated = 30 +// initState.ipConfigCount = 30 +// requestThresholdPercent = 50 +// releaseThresholdPercent = 150 +// maxIPCount = 30 +// ) + +// fakecns, _, poolmonitor := initFakes(t, +// initState.batchSize, +// initState.ipConfigCount, +// requestThresholdPercent, +// releaseThresholdPercent, +// maxIPCount, +// initialAllocated, +// ) + +// // When poolmonitor reconcile is called, trigger increase and cache goal state +// assert.NoError(t, poolmonitor.reconcile(context.Background())) + +// // Trigger a batch release +// assert.NoError(t, fakecns.SetNumberOfAllocatedIPs(1)) + +// assert.NoError(t, poolmonitor.reconcile(context.Background())) + +// // ensure pool monitor has only requested the max pod ip count +// if poolmonitor.nnc.Spec.RequestedIPCount != int64(maxIPCount) { +// t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) when the max "+ +// "has been reached", poolmonitor.nnc.Spec.RequestedIPCount, maxIPCount) +// } +// } + +// func ReconcileAndValidate(ctx context.Context, t *testing.T, poolmonitor *Monitor, expectedRequestCount, expectedIpsNotInUse int) { +// assert.NoError(t, poolmonitor.reconcile(context.Background())) + +// // Increased the new count to be 20 +// if poolmonitor.nnc.Spec.RequestedIPCount != int64(expectedRequestCount) { +// t.Fatalf("RequestIPCount not same, expected %v, actual %v", +// expectedRequestCount, +// poolmonitor.nnc.Spec.RequestedIPCount) +// } + +// // Ensure there is no pending release ips +// if len(poolmonitor.nnc.Spec.IPsNotInUse) != expectedIpsNotInUse { +// t.Fatalf("Expected IP's not in use, expected %v, actual %v", +// expectedIpsNotInUse, +// len(poolmonitor.nnc.Spec.IPsNotInUse)) +// } +// } + +// func TestDecreaseAndIncreaseToSameCount(t *testing.T) { +// var ( +// initState.batchSize = 10 +// initialAllocated = 7 +// initState.ipConfigCount = 10 +// requestThresholdPercent = 50 +// releaseThresholdPercent = 150 +// maxIPCount = 30 +// ) + +// fakecns, fakerc, poolmonitor := initFakes(t, +// initState.batchSize, +// initState.ipConfigCount, +// requestThresholdPercent, +// releaseThresholdPercent, +// maxIPCount, +// initialAllocated, +// ) + +// // Pool monitor will increase the count to 20 +// t.Logf("Scaleup: Increase pool size to 20") +// ReconcileAndValidate(context.Background(), t, 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 +// assert.NoError(t, fakecns.SetNumberOfAllocatedIPs(0)) + +// t.Logf("Scaledown: Decrease pool size to 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 +// t.Logf("Scaleup: pool size back to 20 without updating the PodIpState for previous scale down") +// assert.NoError(t, fakecns.SetNumberOfAllocatedIPs(7)) +// ReconcileAndValidate(context.Background(), t, poolmonitor, 20, 10) + +// // Update the IPConfig count and dont remove the pending IPs +// t.Logf("Reconcile with PodIPState") +// assert.NoError(t, fakerc.Reconcile(false)) + +// // reconcile again +// t.Logf("Reconcole with pool monitor again, it should not cleanup ipsnotinuse") +// ReconcileAndValidate(context.Background(), t, poolmonitor, 20, 10) + +// t.Logf("Now update podipconfig state") +// assert.NoError(t, fakerc.Reconcile(true)) + +// assert.NoError(t, poolmonitor.reconcile(context.Background())) +// ReconcileAndValidate(context.Background(), t, poolmonitor, 20, 0) +// } + func TestPoolSizeIncrease(t *testing.T) { - var ( - batchSize = 10 - initialIPConfigCount = 10 - requestThresholdPercent = 30 - releaseThresholdPercent = 150 - maxPodIPCount = int64(30) - ) - - fakecns, fakerc, poolmonitor := initFakes(t, - batchSize, - initialIPConfigCount, - requestThresholdPercent, - releaseThresholdPercent, - maxPodIPCount) - - // increase number of allocated IP's in CNS - err := fakecns.SetNumberOfAllocatedIPs(8) - if err != nil { - t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) + initState := state{ + batchSize: 10, + allocatedIPCount: 8, + ipConfigCount: 10, + requestThresholdPercent: 50, + releaseThresholdPercent: 150, + maxIPCount: 30, } + fakecns, fakerc, poolmonitor := initFakes(initState) + assert.NoError(t, fakerc.Reconcile(true)) + // When poolmonitor reconcile is called, trigger increase and cache goal state - err = poolmonitor.reconcile(context.Background()) + err := poolmonitor.reconcile(context.Background()) if err != nil { t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) } // ensure pool monitor has reached quorum with cns - if poolmonitor.nnc.Spec.RequestedIPCount != int64(initialIPConfigCount+(1*batchSize)) { + if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.ipConfigCount+(1*initState.batchSize)) { t.Fatalf("Pool monitor target IP count doesn't match CNS pool state "+ "after reconcile: %v, "+ "actual %v", poolmonitor.nnc.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) @@ -99,16 +471,16 @@ func TestPoolSizeIncrease(t *testing.T) { } // ensure pool monitor has reached quorum with cns - if poolmonitor.nnc.Spec.RequestedIPCount != int64(initialIPConfigCount+(1*batchSize)) { + if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.ipConfigCount+(1*initState.batchSize)) { t.Fatalf("Pool monitor target IP count doesn't "+ "match CNS pool state after reconcile: %v, "+ "actual %v", poolmonitor.nnc.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) } // make sure IPConfig state size reflects the new pool size - if len(fakecns.GetPodIPConfigState()) != initialIPConfigCount+(1*batchSize) { + if len(fakecns.GetPodIPConfigState()) != initState.ipConfigCount+(1*initState.batchSize) { t.Fatalf("CNS Pod IPConfig state count doesn't "+ - "match, expected: %v, actual %v", len(fakecns.GetPodIPConfigState()), initialIPConfigCount+(1*batchSize)) + "match, expected: %v, actual %v", len(fakecns.GetPodIPConfigState()), initState.ipConfigCount+(1*initState.batchSize)) } t.Logf("Pool size %v, Target pool size %v, "+ @@ -117,29 +489,20 @@ func TestPoolSizeIncrease(t *testing.T) { } func TestPoolIncreaseDoesntChangeWhenIncreaseIsAlreadyInProgress(t *testing.T) { - var ( - batchSize = 10 - initialIPConfigCount = 10 - requestThresholdPercent = 30 - releaseThresholdPercent = 150 - maxPodIPCount = int64(30) - ) - - fakecns, fakerc, poolmonitor := initFakes(t, - batchSize, - initialIPConfigCount, - requestThresholdPercent, - releaseThresholdPercent, - maxPodIPCount) - - // increase number of allocated IP's in CNS - err := fakecns.SetNumberOfAllocatedIPs(8) - if err != nil { - t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) + initState := state{ + batchSize: 10, + allocatedIPCount: 8, + ipConfigCount: 10, + requestThresholdPercent: 30, + releaseThresholdPercent: 150, + maxIPCount: 30, } + fakecns, fakerc, poolmonitor := initFakes(initState) + assert.NoError(t, fakerc.Reconcile(true)) + // When poolmonitor reconcile is called, trigger increase and cache goal state - err = poolmonitor.reconcile(context.Background()) + err := poolmonitor.reconcile(context.Background()) if err != nil { t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) } @@ -157,7 +520,7 @@ func TestPoolIncreaseDoesntChangeWhenIncreaseIsAlreadyInProgress(t *testing.T) { } // ensure pool monitor has reached quorum with cns - if poolmonitor.nnc.Spec.RequestedIPCount != int64(initialIPConfigCount+(1*batchSize)) { + if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.ipConfigCount+(1*initState.batchSize)) { t.Fatalf("Pool monitor target IP count doesn't match CNS pool state after reconcile: %v,"+ " actual %v", poolmonitor.nnc.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) } @@ -176,13 +539,13 @@ func TestPoolIncreaseDoesntChangeWhenIncreaseIsAlreadyInProgress(t *testing.T) { } // make sure IPConfig state size reflects the new pool size - if len(fakecns.GetPodIPConfigState()) != initialIPConfigCount+(1*batchSize) { + if len(fakecns.GetPodIPConfigState()) != initState.ipConfigCount+(1*initState.batchSize) { t.Fatalf("CNS Pod IPConfig state count doesn't match, expected: %v, actual %v", - len(fakecns.GetPodIPConfigState()), initialIPConfigCount+(1*batchSize)) + len(fakecns.GetPodIPConfigState()), initState.ipConfigCount+(1*initState.batchSize)) } // ensure pool monitor has reached quorum with cns - if poolmonitor.nnc.Spec.RequestedIPCount != int64(initialIPConfigCount+(1*batchSize)) { + if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.ipConfigCount+(1*initState.batchSize)) { t.Fatalf("Pool monitor target IP count doesn't match CNS pool state after reconcile: %v, "+ "actual %v", poolmonitor.nnc.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) } @@ -192,38 +555,26 @@ func TestPoolIncreaseDoesntChangeWhenIncreaseIsAlreadyInProgress(t *testing.T) { } func TestPoolSizeIncreaseIdempotency(t *testing.T) { - var ( - batchSize = 10 - initialIPConfigCount = 10 - requestThresholdPercent = 30 - releaseThresholdPercent = 150 - maxPodIPCount = int64(30) - ) - - fakecns, _, poolmonitor := initFakes(t, - batchSize, - initialIPConfigCount, - requestThresholdPercent, - releaseThresholdPercent, - maxPodIPCount) - - t.Logf("Minimum free IPs to request: %v", CalculateMinFreeIPs(*poolmonitor.nnc)) - t.Logf("Maximum free IPs to release: %v", CalculateMaxFreeIPs(*poolmonitor.nnc)) - - // increase number of allocated IP's in CNS - err := fakecns.SetNumberOfAllocatedIPs(8) - if err != nil { - t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) + initState := state{ + batchSize: 10, + allocatedIPCount: 8, + ipConfigCount: 10, + requestThresholdPercent: 30, + releaseThresholdPercent: 150, + maxIPCount: 30, } + fakecns, fakerc, poolmonitor := initFakes(initState) + assert.NoError(t, fakerc.Reconcile(true)) + // When poolmonitor reconcile is called, trigger increase and cache goal state - err = poolmonitor.reconcile(context.Background()) + err := poolmonitor.reconcile(context.Background()) if err != nil { t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) } // ensure pool monitor has increased batch size - if poolmonitor.nnc.Spec.RequestedIPCount != int64(initialIPConfigCount+(1*batchSize)) { + if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.ipConfigCount+(1*initState.batchSize)) { t.Fatalf("Pool monitor target IP count doesn't match CNS pool state after reconcile: %v,"+ " actual %v", poolmonitor.nnc.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) } @@ -235,203 +586,127 @@ func TestPoolSizeIncreaseIdempotency(t *testing.T) { } // ensure pool monitor requested pool size is unchanged as request controller hasn't reconciled yet - if poolmonitor.nnc.Spec.RequestedIPCount != int64(initialIPConfigCount+(1*batchSize)) { + if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.ipConfigCount+(1*initState.batchSize)) { t.Fatalf("Pool monitor target IP count doesn't match CNS pool state after reconcile: %v,"+ " actual %v", poolmonitor.nnc.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) } } func TestPoolIncreasePastNodeLimit(t *testing.T) { - var ( - batchSize = 16 - initialIPConfigCount = 16 - requestThresholdPercent = 50 - releaseThresholdPercent = 150 - maxPodIPCount = int64(30) - ) - - fakecns, _, poolmonitor := initFakes(t, - batchSize, - initialIPConfigCount, - requestThresholdPercent, - releaseThresholdPercent, - maxPodIPCount) - - t.Logf("Minimum free IPs to request: %v", CalculateMinFreeIPs(*poolmonitor.nnc)) - t.Logf("Maximum free IPs to release: %v", CalculateMaxFreeIPs(*poolmonitor.nnc)) - - // increase number of allocated IP's in CNS - err := fakecns.SetNumberOfAllocatedIPs(9) - if err != nil { - t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) + initState := state{ + batchSize: 16, + allocatedIPCount: 9, + ipConfigCount: 16, + requestThresholdPercent: 50, + releaseThresholdPercent: 150, + maxIPCount: 30, } + _, fakerc, poolmonitor := initFakes(initState) + assert.NoError(t, fakerc.Reconcile(true)) + // When poolmonitor reconcile is called, trigger increase and cache goal state - err = poolmonitor.reconcile(context.Background()) + err := poolmonitor.reconcile(context.Background()) if err != nil { t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) } // ensure pool monitor has only requested the max pod ip count - if poolmonitor.nnc.Spec.RequestedIPCount != maxPodIPCount { + if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.maxIPCount) { t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) when the max "+ - "has been reached", poolmonitor.nnc.Spec.RequestedIPCount, maxPodIPCount) + "has been reached", poolmonitor.nnc.Spec.RequestedIPCount, initState.maxIPCount) } } func TestPoolIncreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { - var ( - batchSize = 50 - initialIPConfigCount = 16 - requestThresholdPercent = 50 - releaseThresholdPercent = 150 - maxPodIPCount = int64(30) - ) - - fakecns, _, poolmonitor := initFakes(t, - batchSize, - initialIPConfigCount, - requestThresholdPercent, - releaseThresholdPercent, - maxPodIPCount) - - t.Logf("Minimum free IPs to request: %v", CalculateMinFreeIPs(*poolmonitor.nnc)) - t.Logf("Maximum free IPs to release: %v", CalculateMaxFreeIPs(*poolmonitor.nnc)) - - // increase number of allocated IP's in CNS - err := fakecns.SetNumberOfAllocatedIPs(16) - if err != nil { - t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) + initState := state{ + batchSize: 50, + allocatedIPCount: 16, + ipConfigCount: 16, + requestThresholdPercent: 50, + releaseThresholdPercent: 150, + maxIPCount: 30, } + _, fakerc, poolmonitor := initFakes(initState) + assert.NoError(t, fakerc.Reconcile(true)) + // When poolmonitor reconcile is called, trigger increase and cache goal state - err = poolmonitor.reconcile(context.Background()) + err := poolmonitor.reconcile(context.Background()) if err != nil { t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) } // ensure pool monitor has only requested the max pod ip count - if poolmonitor.nnc.Spec.RequestedIPCount != maxPodIPCount { + if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.maxIPCount) { t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) "+ - "when the max has been reached", poolmonitor.nnc.Spec.RequestedIPCount, maxPodIPCount) - } -} - -func TestPoolIncreaseMaxIPCountSetToZero(t *testing.T) { - var ( - batchSize = 16 - initialIPConfigCount = 16 - requestThresholdPercent = 50 - releaseThresholdPercent = 150 - initialMaxPodIPCount = int64(0) - expectedMaxPodIPCount = defaultMaxIPCount - ) - - _, _, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, - requestThresholdPercent, releaseThresholdPercent, initialMaxPodIPCount) - - if poolmonitor.nnc.Status.Scaler.MaxIPCount != 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.nnc.Status.Scaler.MaxIPCount, expectedMaxPodIPCount) + "when the max has been reached", poolmonitor.nnc.Spec.RequestedIPCount, initState.maxIPCount) } } func TestPoolDecrease(t *testing.T) { - var ( - batchSize = 10 - initialIPConfigCount = 20 - requestThresholdPercent = 30 - releaseThresholdPercent = 150 - maxPodIPCount = int64(30) - ) - - fakecns, fakerc, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, - requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) - - t.Logf("Minimum free IPs to request: %v", CalculateMinFreeIPs(*poolmonitor.nnc)) - t.Logf("Maximum free IPs to release: %v", CalculateMaxFreeIPs(*poolmonitor.nnc)) - - // initial pool count is 20, set 15 of them to be allocated - err := fakecns.SetNumberOfAllocatedIPs(15) - if err != nil { - t.Fatal(err) + initState := state{ + batchSize: 10, + ipConfigCount: 20, + allocatedIPCount: 15, + requestThresholdPercent: 50, + releaseThresholdPercent: 150, + maxIPCount: 30, } + fakecns, fakerc, poolmonitor := initFakes(initState) + assert.NoError(t, fakerc.Reconcile(true)) + // Pool monitor does nothing, as the current number of IP's falls in the threshold - err = poolmonitor.reconcile(context.Background()) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, poolmonitor.reconcile(context.Background())) // Decrease the number of allocated IP's down to 5. This should trigger a scale down - err = fakecns.SetNumberOfAllocatedIPs(4) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, fakecns.SetNumberOfAllocatedIPs(4)) // Pool monitor will adjust the spec so the pool size will be 1 batch size smaller - err = poolmonitor.reconcile(context.Background()) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, poolmonitor.reconcile(context.Background())) // ensure that the adjusted spec is smaller than the initial pool size - if len(poolmonitor.nnc.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.nnc.Spec.IPsNotInUse)) - } + assert.Equal(t, (initState.ipConfigCount - initState.batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse), "expected pool size to be smaller after reconcile") // reconcile the fake request controller - err = fakerc.Reconcile(true) - if err != nil { - t.Fatal(err) - } + assert.NoError(t, fakerc.Reconcile(true)) // 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())) - } + assert.Zero(t, len(fakecns.GetPendingReleaseIPConfigs()), "expected 0 PendingReleaseIPConfigs") } func TestPoolSizeDecreaseWhenDecreaseHasAlreadyBeenRequested(t *testing.T) { - var ( - batchSize = 10 - initialIPConfigCount = 20 - requestThresholdPercent = 30 - releaseThresholdPercent = 100 - maxPodIPCount = int64(30) - ) - - fakecns, fakerc, poolmonitor := initFakes(t, batchSize, initialIPConfigCount, - requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) - - t.Logf("Minimum free IPs to request: %v", CalculateMinFreeIPs(*poolmonitor.nnc)) - t.Logf("Maximum free IPs to release: %v", CalculateMaxFreeIPs(*poolmonitor.nnc)) - - // initial pool count is 30, set 25 of them to be allocated - err := fakecns.SetNumberOfAllocatedIPs(5) - if err != nil { - t.Error(err) + initState := state{ + batchSize: 10, + allocatedIPCount: 5, + ipConfigCount: 20, + requestThresholdPercent: 30, + releaseThresholdPercent: 100, + maxIPCount: 30, } + fakecns, fakerc, poolmonitor := initFakes(initState) + assert.NoError(t, fakerc.Reconcile(true)) + // Pool monitor does nothing, as the current number of IP's falls in the threshold - err = poolmonitor.reconcile(context.Background()) + 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) } // Ensure the size of the requested spec is still the same - if len(poolmonitor.nnc.Spec.IPsNotInUse) != (initialIPConfigCount - batchSize) { + if len(poolmonitor.nnc.Spec.IPsNotInUse) != (initState.ipConfigCount - initState.batchSize) { t.Fatalf("Expected IP's not in use be one batch size smaller after reconcile, expected %v,"+ - " actual %v", (initialIPConfigCount - batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) + " actual %v", (initState.ipConfigCount - initState.batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) } // Ensure the request ipcount is now one batch size smaller than the inital IP count - if poolmonitor.nnc.Spec.RequestedIPCount != int64(initialIPConfigCount-batchSize) { + if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.ipConfigCount-initState.batchSize) { t.Fatalf("Expected pool size to be one batch size smaller after reconcile, expected %v,"+ - " actual %v", (initialIPConfigCount - batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) + " actual %v", (initState.ipConfigCount - initState.batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) } // Update pods with IP count, ensure pool monitor stays the same until request controller reconciles @@ -441,16 +716,16 @@ func TestPoolSizeDecreaseWhenDecreaseHasAlreadyBeenRequested(t *testing.T) { } // Ensure the size of the requested spec is still the same - if len(poolmonitor.nnc.Spec.IPsNotInUse) != (initialIPConfigCount - batchSize) { + if len(poolmonitor.nnc.Spec.IPsNotInUse) != (initState.ipConfigCount - initState.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.nnc.Spec.IPsNotInUse)) + (initState.ipConfigCount - initState.batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) } // Ensure the request ipcount is now one batch size smaller than the inital IP count - if poolmonitor.nnc.Spec.RequestedIPCount != int64(initialIPConfigCount-batchSize) { + if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.ipConfigCount-initState.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), + " existing call, expected %v, actual %v", (initState.ipConfigCount - initState.batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) } @@ -467,42 +742,30 @@ func TestPoolSizeDecreaseWhenDecreaseHasAlreadyBeenRequested(t *testing.T) { // Ensure the spec doesn't have any IPsNotInUse after request controller has reconciled if len(poolmonitor.nnc.Spec.IPsNotInUse) != 0 { t.Fatalf("Expected IP's not in use to be 0 after reconcile, expected %v, actual %v", - (initialIPConfigCount - batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) + (initState.ipConfigCount - initState.batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) } } func TestDecreaseAndIncreaseToSameCount(t *testing.T) { - var ( - batchSize = 10 - initialIPConfigCount = 10 - requestThresholdPercent = 50 - releaseThresholdPercent = 150 - maxPodIPCount = int64(30) - ) - - fakecns, fakerc, poolmonitor := initFakes(t, - batchSize, - initialIPConfigCount, - requestThresholdPercent, - releaseThresholdPercent, - maxPodIPCount) - - t.Logf("Minimum free IPs to request: %v", CalculateMinFreeIPs(*poolmonitor.nnc)) - t.Logf("Maximum free IPs to release: %v", CalculateMaxFreeIPs(*poolmonitor.nnc)) - - // initial pool count is 10, set 5 of them to be allocated - err := fakecns.SetNumberOfAllocatedIPs(7) - if err != nil { - t.Error(err) + initState := state{ + batchSize: 10, + allocatedIPCount: 7, + ipConfigCount: 10, + requestThresholdPercent: 50, + releaseThresholdPercent: 150, + maxIPCount: 30, } + fakecns, fakerc, poolmonitor := initFakes(initState) + assert.NoError(t, fakerc.Reconcile(true)) + // Pool monitor will increase the count to 20 t.Logf("Scaleup: Increase pool size to 20") ReconcileAndValidate(context.Background(), t, poolmonitor, 20, 0) // Update the IPConfig state t.Logf("Reconcile with PodIPState") - err = fakerc.Reconcile(true) + err := fakerc.Reconcile(true) if err != nil { t.Error(err) } @@ -550,32 +813,20 @@ func TestDecreaseAndIncreaseToSameCount(t *testing.T) { } func TestPoolSizeDecreaseToReallyLow(t *testing.T) { - var ( - batchSize = 10 - initialIPConfigCount = 30 - requestThresholdPercent = 30 - releaseThresholdPercent = 100 - maxPodIPCount = int64(30) - ) - - fakecns, fakerc, poolmonitor := initFakes(t, - batchSize, - initialIPConfigCount, - requestThresholdPercent, - releaseThresholdPercent, - maxPodIPCount) - - t.Logf("Minimum free IPs to request: %v", CalculateMinFreeIPs(*poolmonitor.nnc)) - t.Logf("Maximum free IPs to release: %v", CalculateMaxFreeIPs(*poolmonitor.nnc)) - - // initial pool count is 30, set 23 of them to be allocated - err := fakecns.SetNumberOfAllocatedIPs(23) - if err != nil { - t.Error(err) + initState := state{ + batchSize: 10, + allocatedIPCount: 23, + ipConfigCount: 30, + requestThresholdPercent: 30, + releaseThresholdPercent: 100, + maxIPCount: 30, } + fakecns, fakerc, poolmonitor := initFakes(initState) + assert.NoError(t, fakerc.Reconcile(true)) + // Pool monitor does nothing, as the current number of IP's falls in the threshold - err = poolmonitor.reconcile(context.Background()) + 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) } @@ -594,15 +845,15 @@ func TestPoolSizeDecreaseToReallyLow(t *testing.T) { } // Ensure the size of the requested spec is still the same - if len(poolmonitor.nnc.Spec.IPsNotInUse) != batchSize { + if len(poolmonitor.nnc.Spec.IPsNotInUse) != initState.batchSize { t.Fatalf("Expected IP's not in use is not correct, expected %v, actual %v", - batchSize, len(poolmonitor.nnc.Spec.IPsNotInUse)) + initState.batchSize, len(poolmonitor.nnc.Spec.IPsNotInUse)) } - // Ensure the request ipcount is now one batch size smaller than the inital IP count - if poolmonitor.nnc.Spec.RequestedIPCount != int64(initialIPConfigCount-batchSize) { + // Ensure the request ipcount is now one batch size smaller than the initial IP count + if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.ipConfigCount-initState.batchSize) { t.Fatalf("Expected pool size to be one batch size smaller after reconcile, expected %v, "+ - "actual %v", (initialIPConfigCount - batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) + "actual %v", (initState.ipConfigCount - initState.batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) } // Reconcile again, it should release the second batch @@ -613,15 +864,15 @@ func TestPoolSizeDecreaseToReallyLow(t *testing.T) { } // Ensure the size of the requested spec is still the same - if len(poolmonitor.nnc.Spec.IPsNotInUse) != batchSize*2 { - t.Fatalf("Expected IP's not in use is not correct, expected %v, actual %v", batchSize*2, + if len(poolmonitor.nnc.Spec.IPsNotInUse) != initState.batchSize*2 { + t.Fatalf("Expected IP's not in use is not correct, expected %v, actual %v", initState.batchSize*2, len(poolmonitor.nnc.Spec.IPsNotInUse)) } // Ensure the request ipcount is now one batch size smaller than the inital IP count - if poolmonitor.nnc.Spec.RequestedIPCount != int64(initialIPConfigCount-(batchSize*2)) { + if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.ipConfigCount-(initState.batchSize*2)) { t.Fatalf("Expected pool size to be one batch size smaller after reconcile, expected %v, "+ - "actual %v", (initialIPConfigCount - batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) + "actual %v", (initState.ipConfigCount - initState.batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) } t.Logf("Update Request Controller") @@ -638,37 +889,25 @@ func TestPoolSizeDecreaseToReallyLow(t *testing.T) { // Ensure the spec doesn't have any IPsNotInUse after request controller has reconciled if len(poolmonitor.nnc.Spec.IPsNotInUse) != 0 { t.Fatalf("Expected IP's not in use to be 0 after reconcile, expected %v, actual %v", - (initialIPConfigCount - batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) + (initState.ipConfigCount - initState.batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) } } func TestDecreaseAfterNodeLimitReached(t *testing.T) { - var ( - batchSize = 16 - initialIPConfigCount = 30 - requestThresholdPercent = 50 - releaseThresholdPercent = 150 - maxPodIPCount = int64(30) - expectedRequestedIP = 16 - expectedDecreaseIP = int(maxPodIPCount) % batchSize - ) - - fakecns, _, poolmonitor := initFakes(t, - batchSize, - initialIPConfigCount, - requestThresholdPercent, - releaseThresholdPercent, - maxPodIPCount) - - t.Logf("Minimum free IPs to request: %v", CalculateMinFreeIPs(*poolmonitor.nnc)) - t.Logf("Maximum free IPs to release: %v", CalculateMaxFreeIPs(*poolmonitor.nnc)) - - err := fakecns.SetNumberOfAllocatedIPs(20) - if err != nil { - t.Error(err) - } + initState := state{ + batchSize: 16, + allocatedIPCount: 20, + ipConfigCount: 30, + requestThresholdPercent: 50, + releaseThresholdPercent: 150, + maxIPCount: 30, + } + expectedRequestedIP := 16 + expectedDecreaseIP := int(initState.maxIPCount) % initState.batchSize + fakecns, fakerc, poolmonitor := initFakes(initState) + assert.NoError(t, fakerc.Reconcile(true)) - err = poolmonitor.reconcile(context.Background()) + 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) } @@ -687,7 +926,7 @@ func TestDecreaseAfterNodeLimitReached(t *testing.T) { // Ensure poolmonitor asked for a multiple of batch size if poolmonitor.nnc.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, + "(max pod limit) but got %v", expectedRequestedIP, initState.maxIPCount, poolmonitor.nnc.Spec.RequestedIPCount) } @@ -700,32 +939,20 @@ func TestDecreaseAfterNodeLimitReached(t *testing.T) { } func TestPoolDecreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { - var ( - batchSize = 31 - initialIPConfigCount = 30 - requestThresholdPercent = 50 - releaseThresholdPercent = 150 - maxPodIPCount = int64(30) - ) - - fakecns, _, poolmonitor := initFakes(t, - batchSize, - initialIPConfigCount, - requestThresholdPercent, - releaseThresholdPercent, - maxPodIPCount) - - t.Logf("Minimum free IPs to request: %v", CalculateMinFreeIPs(*poolmonitor.nnc)) - t.Logf("Maximum free IPs to release: %v", CalculateMaxFreeIPs(*poolmonitor.nnc)) - - // increase number of allocated IP's in CNS - err := fakecns.SetNumberOfAllocatedIPs(30) - if err != nil { - t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) + initState := state{ + batchSize: 31, + allocatedIPCount: 30, + ipConfigCount: 30, + requestThresholdPercent: 50, + releaseThresholdPercent: 150, + maxIPCount: 30, } + fakecns, fakerc, poolmonitor := initFakes(initState) + assert.NoError(t, fakerc.Reconcile(true)) + // When poolmonitor reconcile is called, trigger increase and cache goal state - err = poolmonitor.reconcile(context.Background()) + err := poolmonitor.reconcile(context.Background()) if err != nil { t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) } @@ -742,9 +969,9 @@ func TestPoolDecreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { } // ensure pool monitor has only requested the max pod ip count - if poolmonitor.nnc.Spec.RequestedIPCount != maxPodIPCount { + if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.maxIPCount) { t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) when the max "+ - "has been reached", poolmonitor.nnc.Spec.RequestedIPCount, maxPodIPCount) + "has been reached", poolmonitor.nnc.Spec.RequestedIPCount, initState.maxIPCount) } } diff --git a/cns/service/main.go b/cns/service/main.go index 7d3da2179c..12099a42b5 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -872,7 +872,8 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn scopedcli := kubecontroller.NewScopedClient(nnccli, types.NamespacedName{Namespace: "kube-system", Name: nodeName}) // initialize the ipam pool monitor - httpRestServiceImplementation.IPAMPoolMonitor = ipampool.NewMonitor(httpRestServiceImplementation, scopedcli) + poolMonitor := ipampool.NewMonitor(httpRestServiceImplementation, scopedcli, &ipampool.Options{RefreshDelay: poolIPAMRefreshRateInMilliseconds}) + httpRestServiceImplementation.IPAMPoolMonitor = poolMonitor err = initCNS(ctx, scopedcli, httpRestServiceImplementation) if err != nil { return errors.Wrap(err, "failed to initialize CNS state") @@ -911,7 +912,7 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn logger.Printf("Starting IPAM Pool Monitor") go func() { for { - if err := httpRestServiceImplementation.IPAMPoolMonitor.Start(ctx, poolIPAMRefreshRateInMilliseconds); err != nil { + if err := poolMonitor.Start(ctx); err != nil { logger.Errorf("[Azure CNS] Failed to start pool monitor with err: %v", err) // todo: add a CNS metric to count # of failures } else { @@ -927,7 +928,7 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn logger.Printf("Starting SyncHostNCVersion") go func() { // Periodically poll vfp programmed NC version from NMAgent - tickerChannel := time.Tick(cnsconfig.SyncHostNCVersionIntervalMs * time.Millisecond) + tickerChannel := time.Tick(cnsconfig.SyncHostNCVersionIntervalMs) for { select { case <-tickerChannel: From 0c74193785e9332ca0c2958b2cd82221beac5f3b Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Fri, 8 Oct 2021 16:06:49 -0500 Subject: [PATCH 07/14] cleanup and delint Signed-off-by: Evan Baker --- cns/ipampool/monitor.go | 3 +- cns/ipampool/monitor_test.go | 392 +---------------------------------- 2 files changed, 11 insertions(+), 384 deletions(-) diff --git a/cns/ipampool/monitor.go b/cns/ipampool/monitor.go index 2235e743ae..333e773b5d 100644 --- a/cns/ipampool/monitor.go +++ b/cns/ipampool/monitor.go @@ -14,6 +14,7 @@ import ( ) const ( + // DefaultRefreshDelay pool monitor poll delay default in seconds. DefaultRefreshDelay = 1 * time.Second ) @@ -319,7 +320,7 @@ func clampScaler(nnc *v1alpha.NodeNetworkConfig) { if nnc.Status.Scaler.RequestThresholdPercent < 1 { nnc.Status.Scaler.RequestThresholdPercent = 1 } - if nnc.Status.Scaler.RequestThresholdPercent > 100 { + if nnc.Status.Scaler.RequestThresholdPercent > 100 { //nolint:gomnd // it's a percent nnc.Status.Scaler.RequestThresholdPercent = 100 } if nnc.Status.Scaler.ReleaseThresholdPercent < nnc.Status.Scaler.RequestThresholdPercent+100 { diff --git a/cns/ipampool/monitor_test.go b/cns/ipampool/monitor_test.go index c419799078..b97cacf81a 100644 --- a/cns/ipampool/monitor_test.go +++ b/cns/ipampool/monitor_test.go @@ -61,376 +61,6 @@ func initFakes(initState state) (*fakes.HTTPServiceFake, *fakes.RequestControlle return fakecns, fakerc, poolmonitor } -// func TestReconcile(t *testing.T) { -// tests := []struct { -// name string -// init state -// want state -// }{ -// { -// name: "increase", -// init: state{ -// initState.batchSize: 10, -// allocatedIPCount: 8, -// ipConfigCount: 10, -// requestThresholdPercent: 50, -// releaseThresholdPercent: 150, -// maxIPCount: 30, -// }, -// want: state{ -// allocatedIPCount: 8, -// ipConfigCount: 20, -// }, -// }, -// { -// name: "increase idempotency", -// init: state{ -// initState.batchSize: 10, -// allocatedIPCount: 8, -// ipConfigCount: 10, -// requestThresholdPercent: 50, -// releaseThresholdPercent: 150, -// maxIPCount: 30, -// }, -// want: state{ -// allocatedIPCount: 9, -// ipConfigCount: 20, -// }, -// }, -// { -// name: "increase cap at node limit", -// init: state{ -// initState.batchSize: 16, -// allocatedIPCount: 9, -// ipConfigCount: 16, -// requestThresholdPercent: 50, -// releaseThresholdPercent: 150, -// maxIPCount: 30, -// }, -// want: state{ -// allocatedIPCount: 9, -// ipConfigCount: 30, -// }, -// }, -// { -// name: "increase cap batch at node limit", -// init: state{ -// initState.batchSize: 50, -// allocatedIPCount: 16, -// ipConfigCount: 16, -// requestThresholdPercent: 50, -// releaseThresholdPercent: 150, -// maxIPCount: 30, -// }, -// want: state{ -// allocatedIPCount: 16, -// ipConfigCount: 30, -// }, -// }, -// { -// name: "decrease", -// init: state{ -// initState.batchSize: 10, -// allocatedIPCount: 15, -// ipConfigCount: 20, -// requestThresholdPercent: 50, -// releaseThresholdPercent: 150, -// maxIPCount: 30, -// }, -// want: state{ -// allocatedIPCount: 4, -// ipConfigCount: 10, -// }, -// }, -// { -// name: "decrease in-progress idempotency", -// init: state{ -// initState.batchSize: 10, -// allocatedIPCount: 5, -// ipConfigCount: 20, -// requestThresholdPercent: 50, -// releaseThresholdPercent: 150, -// maxIPCount: 30, -// }, -// want: state{ -// allocatedIPCount: 6, -// ipConfigCount: 10, -// }, -// }, -// { -// name: "decrease multiple batches", -// init: state{ -// initState.batchSize: 10, -// allocatedIPCount: 23, -// ipConfigCount: 30, -// requestThresholdPercent: 50, -// releaseThresholdPercent: 150, -// maxIPCount: 30, -// }, -// want: state{ -// allocatedIPCount: 3, -// ipConfigCount: 10, -// }, -// }, - -// { -// name: "decrease to zero", -// init: state{ -// initState.batchSize: 10, -// allocatedIPCount: 23, -// ipConfigCount: 30, -// requestThresholdPercent: 50, -// releaseThresholdPercent: 150, -// maxIPCount: 30, -// }, -// want: state{ -// allocatedIPCount: 0, -// ipConfigCount: 10, -// }, -// }, -// } -// for _, tt := range tests { -// tt := tt -// t.Run(tt.name, func(t *testing.T) { -// fakecns, fakerc, poolmonitor := initFakes(tt.init) -// require.NoError(t, fakerc.Reconcile(true)) -// require.NoError(t, poolmonitor.reconcile(context.Background())) -// require.NoError(t, fakecns.SetNumberOfAllocatedIPs(tt.want.allocatedIPCount)) - -// // reconcile until CNS and pool monitor have converged -// for tt.want.ipConfigCount != len(fakecns.GetPodIPConfigState()) || tt.want.ipConfigCount != int(poolmonitor.nnc.Spec.RequestedIPCount) { -// // request controller reconciles, carves new IPs from the test subnet and adds to CNS state -// require.NoError(t, fakerc.Reconcile(true)) -// // pool monitor reconciles, makes new pool size requests -// require.NoError(t, poolmonitor.reconcile(context.Background())) -// } - -// require.Zero(t, len(fakecns.GetPendingReleaseIPConfigs())) -// }) -// } -// } - -// func TestPoolSizeDecreaseToReallyLow(t *testing.T) { -// var ( -// initState.batchSize = 10 -// initialAllocated = 23 -// initState.ipConfigCount = 30 -// requestThresholdPercent = 30 -// releaseThresholdPercent = 100 -// maxIPCount = 30 -// ) - -// fakecns, fakerc, poolmonitor := initFakes(t, -// initState.batchSize, -// initState.ipConfigCount, -// requestThresholdPercent, -// releaseThresholdPercent, -// maxIPCount, -// initialAllocated, -// ) - -// // Pool monitor does nothing, as the current number of IP's falls in the threshold -// assert.NoError(t, poolmonitor.reconcile(context.Background())) - -// // Now Drop the Allocated count to really low, say 3. This should trigger release in 2 batches -// assert.NoError(t, fakecns.SetNumberOfAllocatedIPs(3)) - -// // Pool monitor does nothing, as the current number of IP's falls in the threshold -// t.Logf("Reconcile after Allocated count from 33 -> 3, Exepected free count = 10") -// assert.NoError(t, poolmonitor.reconcile(context.Background())) - -// // Ensure the size of the requested spec is still the same -// if len(poolmonitor.nnc.Spec.IPsNotInUse) != initState.batchSize { -// t.Fatalf("Expected IP's not in use is not correct, expected %v, actual %v", -// initState.batchSize, len(poolmonitor.nnc.Spec.IPsNotInUse)) -// } - -// // Ensure the request ipcount is now one batch size smaller than the initial IP count -// if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.ipConfigCount-initState.batchSize) { -// t.Fatalf("Expected pool size to be one batch size smaller after reconcile, expected %v, "+ -// "actual %v", (initState.ipConfigCount - initState.batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) -// } - -// // Reconcile again, it should release the second batch -// t.Logf("Reconcile again - 2, Exepected free count = 20") -// assert.NoError(t, poolmonitor.reconcile(context.Background())) - -// // Ensure the size of the requested spec is still the same -// if len(poolmonitor.nnc.Spec.IPsNotInUse) != initState.batchSize*2 { -// t.Fatalf("Expected IP's not in use is not correct, expected %v, actual %v", initState.batchSize*2, -// len(poolmonitor.nnc.Spec.IPsNotInUse)) -// } - -// // Ensure the request ipcount is now one batch size smaller than the inital IP count -// if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.ipConfigCount-(initState.batchSize*2)) { -// t.Fatalf("Expected pool size to be one batch size smaller after reconcile, expected %v, "+ -// "actual %v", (initState.ipConfigCount - initState.batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) -// } - -// t.Logf("Update Request Controller") -// assert.NoError(t, fakerc.Reconcile(true)) - -// assert.NoError(t, poolmonitor.reconcile(context.Background())) - -// // Ensure the spec doesn't have any IPsNotInUse after request controller has reconciled -// if len(poolmonitor.nnc.Spec.IPsNotInUse) != 0 { -// t.Fatalf("Expected IP's not in use to be 0 after reconcile, expected %v, actual %v", -// (initState.ipConfigCount - initState.batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) -// } -// } - -// func TestDecreaseAfterNodeLimitReached(t *testing.T) { -// var ( -// initState.batchSize = 16 -// initialAllocated = 20 -// initState.ipConfigCount = 30 -// requestThresholdPercent = 50 -// releaseThresholdPercent = 150 -// maxIPCount = 30 -// expectedRequestedIP = 16 -// expectedDecreaseIP = int(maxIPCount) % initState.batchSize -// ) - -// fakecns, _, poolmonitor := initFakes(t, -// initState.batchSize, -// initState.ipConfigCount, -// requestThresholdPercent, -// releaseThresholdPercent, -// maxIPCount, -// initialAllocated, -// ) - -// assert.NoError(t, poolmonitor.reconcile(context.Background())) - -// // Trigger a batch release -// assert.NoError(t, fakecns.SetNumberOfAllocatedIPs(5)) - -// assert.NoError(t, poolmonitor.reconcile(context.Background())) - -// // Ensure poolmonitor asked for a multiple of batch size -// if poolmonitor.nnc.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, maxIPCount, -// poolmonitor.nnc.Spec.RequestedIPCount) -// } - -// // Ensure we minused by the mod result -// if len(poolmonitor.nnc.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.nnc.Spec.IPsNotInUse)) -// } -// } - -// func TestPoolDecreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { -// var ( -// initState.batchSize = 31 -// initialAllocated = 30 -// initState.ipConfigCount = 30 -// requestThresholdPercent = 50 -// releaseThresholdPercent = 150 -// maxIPCount = 30 -// ) - -// fakecns, _, poolmonitor := initFakes(t, -// initState.batchSize, -// initState.ipConfigCount, -// requestThresholdPercent, -// releaseThresholdPercent, -// maxIPCount, -// initialAllocated, -// ) - -// // When poolmonitor reconcile is called, trigger increase and cache goal state -// assert.NoError(t, poolmonitor.reconcile(context.Background())) - -// // Trigger a batch release -// assert.NoError(t, fakecns.SetNumberOfAllocatedIPs(1)) - -// assert.NoError(t, poolmonitor.reconcile(context.Background())) - -// // ensure pool monitor has only requested the max pod ip count -// if poolmonitor.nnc.Spec.RequestedIPCount != int64(maxIPCount) { -// t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) when the max "+ -// "has been reached", poolmonitor.nnc.Spec.RequestedIPCount, maxIPCount) -// } -// } - -// func ReconcileAndValidate(ctx context.Context, t *testing.T, poolmonitor *Monitor, expectedRequestCount, expectedIpsNotInUse int) { -// assert.NoError(t, poolmonitor.reconcile(context.Background())) - -// // Increased the new count to be 20 -// if poolmonitor.nnc.Spec.RequestedIPCount != int64(expectedRequestCount) { -// t.Fatalf("RequestIPCount not same, expected %v, actual %v", -// expectedRequestCount, -// poolmonitor.nnc.Spec.RequestedIPCount) -// } - -// // Ensure there is no pending release ips -// if len(poolmonitor.nnc.Spec.IPsNotInUse) != expectedIpsNotInUse { -// t.Fatalf("Expected IP's not in use, expected %v, actual %v", -// expectedIpsNotInUse, -// len(poolmonitor.nnc.Spec.IPsNotInUse)) -// } -// } - -// func TestDecreaseAndIncreaseToSameCount(t *testing.T) { -// var ( -// initState.batchSize = 10 -// initialAllocated = 7 -// initState.ipConfigCount = 10 -// requestThresholdPercent = 50 -// releaseThresholdPercent = 150 -// maxIPCount = 30 -// ) - -// fakecns, fakerc, poolmonitor := initFakes(t, -// initState.batchSize, -// initState.ipConfigCount, -// requestThresholdPercent, -// releaseThresholdPercent, -// maxIPCount, -// initialAllocated, -// ) - -// // Pool monitor will increase the count to 20 -// t.Logf("Scaleup: Increase pool size to 20") -// ReconcileAndValidate(context.Background(), t, 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 -// assert.NoError(t, fakecns.SetNumberOfAllocatedIPs(0)) - -// t.Logf("Scaledown: Decrease pool size to 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 -// t.Logf("Scaleup: pool size back to 20 without updating the PodIpState for previous scale down") -// assert.NoError(t, fakecns.SetNumberOfAllocatedIPs(7)) -// ReconcileAndValidate(context.Background(), t, poolmonitor, 20, 10) - -// // Update the IPConfig count and dont remove the pending IPs -// t.Logf("Reconcile with PodIPState") -// assert.NoError(t, fakerc.Reconcile(false)) - -// // reconcile again -// t.Logf("Reconcole with pool monitor again, it should not cleanup ipsnotinuse") -// ReconcileAndValidate(context.Background(), t, poolmonitor, 20, 10) - -// t.Logf("Now update podipconfig state") -// assert.NoError(t, fakerc.Reconcile(true)) - -// assert.NoError(t, poolmonitor.reconcile(context.Background())) -// ReconcileAndValidate(context.Background(), t, poolmonitor, 20, 0) -// } - func TestPoolSizeIncrease(t *testing.T) { initState := state{ batchSize: 10, @@ -703,7 +333,7 @@ func TestPoolSizeDecreaseWhenDecreaseHasAlreadyBeenRequested(t *testing.T) { " actual %v", (initState.ipConfigCount - initState.batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) } - // Ensure the request ipcount is now one batch size smaller than the inital IP count + // Ensure the request ipcount is now one batch size smaller than the initial IP count if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.ipConfigCount-initState.batchSize) { t.Fatalf("Expected pool size to be one batch size smaller after reconcile, expected %v,"+ " actual %v", (initState.ipConfigCount - initState.batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) @@ -722,7 +352,7 @@ func TestPoolSizeDecreaseWhenDecreaseHasAlreadyBeenRequested(t *testing.T) { (initState.ipConfigCount - initState.batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) } - // Ensure the request ipcount is now one batch size smaller than the inital IP count + // Ensure the request ipcount is now one batch size smaller than the initial IP count if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.ipConfigCount-initState.batchSize) { t.Fatalf("Expected pool size to be one batch size smaller after reconcile, and not change after"+ " existing call, expected %v, actual %v", (initState.ipConfigCount - initState.batchSize), @@ -761,7 +391,7 @@ func TestDecreaseAndIncreaseToSameCount(t *testing.T) { // Pool monitor will increase the count to 20 t.Logf("Scaleup: Increase pool size to 20") - ReconcileAndValidate(context.Background(), t, poolmonitor, 20, 0) + ReconcileAndValidate(t, poolmonitor, 20, 0) // Update the IPConfig state t.Logf("Reconcile with PodIPState") @@ -777,7 +407,7 @@ func TestDecreaseAndIncreaseToSameCount(t *testing.T) { } t.Logf("Scaledown: Decrease pool size to 10") - ReconcileAndValidate(context.Background(), t, poolmonitor, 10, 10) + ReconcileAndValidate(t, poolmonitor, 10, 10) // Increase it back to 20 // initial pool count is 10, set 5 of them to be allocated @@ -786,7 +416,7 @@ func TestDecreaseAndIncreaseToSameCount(t *testing.T) { if err != nil { t.Error(err) } - ReconcileAndValidate(context.Background(), t, poolmonitor, 20, 10) + ReconcileAndValidate(t, poolmonitor, 20, 10) // Update the IPConfig count and dont remove the pending IPs t.Logf("Reconcile with PodIPState") @@ -797,7 +427,7 @@ func TestDecreaseAndIncreaseToSameCount(t *testing.T) { // reconcile again t.Logf("Reconcole with pool monitor again, it should not cleanup ipsnotinuse") - ReconcileAndValidate(context.Background(), t, poolmonitor, 20, 10) + ReconcileAndValidate(t, poolmonitor, 20, 10) t.Logf("Now update podipconfig state") err = fakerc.Reconcile(true) @@ -809,7 +439,7 @@ func TestDecreaseAndIncreaseToSameCount(t *testing.T) { if err != nil { t.Errorf("Expected no pool monitor failure after request controller reconcile: %v", err) } - ReconcileAndValidate(context.Background(), t, poolmonitor, 20, 0) + ReconcileAndValidate(t, poolmonitor, 20, 0) } func TestPoolSizeDecreaseToReallyLow(t *testing.T) { @@ -869,7 +499,7 @@ func TestPoolSizeDecreaseToReallyLow(t *testing.T) { len(poolmonitor.nnc.Spec.IPsNotInUse)) } - // Ensure the request ipcount is now one batch size smaller than the inital IP count + // Ensure the request ipcount is now one batch size smaller than the initial IP count if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.ipConfigCount-(initState.batchSize*2)) { t.Fatalf("Expected pool size to be one batch size smaller after reconcile, expected %v, "+ "actual %v", (initState.ipConfigCount - initState.batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) @@ -975,11 +605,7 @@ func TestPoolDecreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { } } -func ReconcileAndValidate(ctx context.Context, - t *testing.T, - poolmonitor *Monitor, - expectedRequestCount, - expectedIpsNotInUse int) { +func ReconcileAndValidate(t *testing.T, poolmonitor *Monitor, 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) From e68903bcd859f0279ee2ba75d1570c2e3cb19770 Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Mon, 11 Oct 2021 14:32:41 -0500 Subject: [PATCH 08/14] start ipampoolmonitor early for init reconcile Signed-off-by: Evan Baker --- cns/service/main.go | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/cns/service/main.go b/cns/service/main.go index 12099a42b5..2db6bbf08c 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -874,6 +874,13 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn // initialize the ipam pool monitor poolMonitor := ipampool.NewMonitor(httpRestServiceImplementation, scopedcli, &ipampool.Options{RefreshDelay: poolIPAMRefreshRateInMilliseconds}) httpRestServiceImplementation.IPAMPoolMonitor = poolMonitor + logger.Printf("Starting IPAM Pool Monitor") + go func() { + if e := poolMonitor.Start(ctx); e != nil { + logger.Errorf("[Azure CNS] Failed to start pool monitor with err: %v", e) + } + }() + err = initCNS(ctx, scopedcli, httpRestServiceImplementation) if err != nil { return errors.Wrap(err, "failed to initialize CNS state") @@ -909,22 +916,6 @@ func InitializeCRDState(ctx context.Context, httpRestService cns.HTTPService, cn } }() - logger.Printf("Starting IPAM Pool Monitor") - go func() { - for { - if err := poolMonitor.Start(ctx); err != nil { - logger.Errorf("[Azure CNS] Failed to start pool monitor with err: %v", err) - // todo: add a CNS metric to count # of failures - } else { - logger.Printf("[Azure CNS] Exiting IPAM Pool Monitor") - return - } - - // Retry after 1sec - time.Sleep(time.Second) - } - }() - logger.Printf("Starting SyncHostNCVersion") go func() { // Periodically poll vfp programmed NC version from NMAgent From 6d31aa620bbc9e0973105c501513f2ccce1cc9fd Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Mon, 11 Oct 2021 16:06:23 -0500 Subject: [PATCH 09/14] remove cached nnc in pool monitor entirely Signed-off-by: Evan Baker --- cns/ipampool/fake.go | 4 +- cns/ipampool/monitor.go | 91 ++++++++++++++++--------------- cns/ipampool/monitor_test.go | 100 +++++++++++++++++------------------ 3 files changed, 101 insertions(+), 94 deletions(-) diff --git a/cns/ipampool/fake.go b/cns/ipampool/fake.go index be221bc242..1cb2624de2 100644 --- a/cns/ipampool/fake.go +++ b/cns/ipampool/fake.go @@ -26,8 +26,8 @@ func (*MonitorFake) Reconcile() error { func (f *MonitorFake) GetStateSnapshot() cns.IpamPoolMonitorStateSnapshot { return cns.IpamPoolMonitorStateSnapshot{ - MaximumFreeIps: CalculateMaxFreeIPs(*f.NodeNetworkConfig), - MinimumFreeIps: CalculateMinFreeIPs(*f.NodeNetworkConfig), + MaximumFreeIps: CalculateMaxFreeIPs(f.NodeNetworkConfig.Status.Scaler), + MinimumFreeIps: CalculateMinFreeIPs(f.NodeNetworkConfig.Status.Scaler), UpdatingIpsNotInUseCount: f.IPsNotInUseCount, CachedNNC: *f.NodeNetworkConfig, } diff --git a/cns/ipampool/monitor.go b/cns/ipampool/monitor.go index 333e773b5d..ca12eacce7 100644 --- a/cns/ipampool/monitor.go +++ b/cns/ipampool/monitor.go @@ -28,7 +28,8 @@ type Options struct { type Monitor struct { opts *Options - nnc *v1alpha.NodeNetworkConfig + spec v1alpha.NodeNetworkConfigSpec + scaler v1alpha.Scaler nnccli nodeNetworkConfigSpecUpdater httpService cns.HTTPService updatingIpsNotInUseCount int @@ -65,7 +66,8 @@ func (pm *Monitor) Start(ctx context.Context) error { // block on ticks until we have initialized <-pm.initialized case nnc := <-pm.nncSource: - pm.nnc = &nnc + pm.spec = nnc.Spec + pm.scaler = nnc.Status.Scaler } err := pm.reconcile(ctx) if err != nil { @@ -80,14 +82,14 @@ func (pm *Monitor) reconcile(ctx context.Context) error { allocatedPodIPCount := len(pm.httpService.GetAllocatedIPConfigs()) pendingReleaseIPCount := len(pm.httpService.GetPendingReleaseIPConfigs()) availableIPConfigCount := len(pm.httpService.GetAvailableIPConfigs()) // TODO: add pending allocation count to real cns - requestedIPConfigCount := pm.nnc.Spec.RequestedIPCount + requestedIPConfigCount := pm.spec.RequestedIPCount unallocatedIPConfigCount := cnsPodIPConfigCount - allocatedPodIPCount freeIPConfigCount := requestedIPConfigCount - int64(allocatedPodIPCount) - batchSize := pm.nnc.Status.Scaler.BatchSize - maxIPCount := pm.nnc.Status.Scaler.MaxIPCount + batchSize := pm.scaler.BatchSize + maxIPCount := pm.scaler.MaxIPCount msg := fmt.Sprintf("[ipam-pool-monitor] Pool Size: %v, Goal Size: %v, BatchSize: %v, MaxIPCount: %v, Allocated: %v, Available: %v, Pending Release: %v, Free: %v, Pending Program: %v", - cnsPodIPConfigCount, pm.nnc.Spec.RequestedIPCount, batchSize, maxIPCount, allocatedPodIPCount, availableIPConfigCount, pendingReleaseIPCount, freeIPConfigCount, pendingProgramCount) + cnsPodIPConfigCount, pm.spec.RequestedIPCount, batchSize, maxIPCount, allocatedPodIPCount, availableIPConfigCount, pendingReleaseIPCount, freeIPConfigCount, pendingProgramCount) ipamAllocatedIPCount.Set(float64(allocatedPodIPCount)) ipamAvailableIPCount.Set(float64(availableIPConfigCount)) @@ -102,8 +104,8 @@ func (pm *Monitor) reconcile(ctx context.Context) error { switch { // pod count is increasing - case freeIPConfigCount < int64(CalculateMinFreeIPs(*pm.nnc)): - if pm.nnc.Spec.RequestedIPCount == maxIPCount { + case freeIPConfigCount < int64(CalculateMinFreeIPs(pm.scaler)): + if pm.spec.RequestedIPCount == maxIPCount { // If we're already at the maxIPCount, don't try to increase return nil } @@ -112,13 +114,13 @@ func (pm *Monitor) reconcile(ctx context.Context) error { return pm.increasePoolSize(ctx) // pod count is decreasing - case freeIPConfigCount >= int64(CalculateMaxFreeIPs(*pm.nnc)): + case freeIPConfigCount >= int64(CalculateMaxFreeIPs(pm.scaler)): 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 len(pm.nnc.Spec.IPsNotInUse) != pendingReleaseIPCount: + case len(pm.spec.IPsNotInUse) != pendingReleaseIPCount: logger.Printf("[ipam-pool-monitor] Removing Pending Release IP's from CRD...%s ", msg) return pm.cleanPendingRelease(ctx) @@ -135,9 +137,9 @@ func (pm *Monitor) increasePoolSize(ctx context.Context) error { tempNNCSpec := pm.createNNCSpecForCRD() // Query the max IP count - maxIPCount := pm.nnc.Status.Scaler.MaxIPCount + maxIPCount := pm.scaler.MaxIPCount previouslyRequestedIPCount := tempNNCSpec.RequestedIPCount - batchSize := pm.nnc.Status.Scaler.BatchSize + batchSize := pm.scaler.BatchSize tempNNCSpec.RequestedIPCount += batchSize if tempNNCSpec.RequestedIPCount > maxIPCount { @@ -163,7 +165,7 @@ func (pm *Monitor) increasePoolSize(ctx context.Context) error { // start an alloc timer metric.StartPoolIncreaseTimer(int(batchSize)) // save the updated state to cachedSpec - pm.nnc.Spec = tempNNCSpec + pm.spec = tempNNCSpec return nil } @@ -174,8 +176,8 @@ func (pm *Monitor) decreasePoolSize(ctx context.Context, existingPendingReleaseI var updatedRequestedIPCount int64 // Ensure the updated requested IP count is a multiple of the batch size - previouslyRequestedIPCount := pm.nnc.Spec.RequestedIPCount - batchSize := pm.nnc.Status.Scaler.BatchSize + previouslyRequestedIPCount := pm.spec.RequestedIPCount + batchSize := pm.scaler.BatchSize modResult := previouslyRequestedIPCount % batchSize logger.Printf("[ipam-pool-monitor] Previously RequestedIP Count %v", previouslyRequestedIPCount) @@ -231,7 +233,7 @@ func (pm *Monitor) decreasePoolSize(ctx context.Context, existingPendingReleaseI metric.StartPoolDecreaseTimer(int(batchSize)) // save the updated state to cachedSpec - pm.nnc.Spec = tempNNCSpec + pm.spec = tempNNCSpec // clear the updatingPendingIpsNotInUse, as we have Updated the CRD logger.Printf("[ipam-pool-monitor] cleaning the updatingPendingIpsNotInUse, existing length %d", pm.updatingIpsNotInUseCount) @@ -254,7 +256,7 @@ func (pm *Monitor) cleanPendingRelease(ctx context.Context) error { logger.Printf("[ipam-pool-monitor] cleanPendingRelease: UpdateCRDSpec succeeded for spec %+v", tempNNCSpec) // save the updated state to cachedSpec - pm.nnc.Spec = tempNNCSpec + pm.spec = tempNNCSpec return nil } @@ -263,7 +265,7 @@ func (pm *Monitor) createNNCSpecForCRD() v1alpha.NodeNetworkConfigSpec { var spec v1alpha.NodeNetworkConfigSpec // Update the count from cached spec - spec.RequestedIPCount = pm.nnc.Spec.RequestedIPCount + spec.RequestedIPCount = pm.spec.RequestedIPCount // Get All Pending IPs from CNS and populate it again. pendingIPs := pm.httpService.GetPendingReleaseIPConfigs() @@ -276,12 +278,17 @@ func (pm *Monitor) createNNCSpecForCRD() v1alpha.NodeNetworkConfigSpec { // GetStateSnapshot gets a snapshot of the IPAMPoolMonitor struct. func (pm *Monitor) GetStateSnapshot() cns.IpamPoolMonitorStateSnapshot { - nnc := *pm.nnc + scaler, spec := pm.scaler, pm.spec return cns.IpamPoolMonitorStateSnapshot{ - MinimumFreeIps: CalculateMinFreeIPs(nnc), - MaximumFreeIps: CalculateMaxFreeIPs(nnc), + MinimumFreeIps: CalculateMinFreeIPs(scaler), + MaximumFreeIps: CalculateMaxFreeIPs(scaler), UpdatingIpsNotInUseCount: pm.updatingIpsNotInUseCount, - CachedNNC: nnc, + CachedNNC: v1alpha.NodeNetworkConfig{ + Spec: spec, + Status: v1alpha.NodeNetworkConfigStatus{ + Scaler: scaler, + }, + }, } } @@ -289,7 +296,7 @@ func (pm *Monitor) GetStateSnapshot() cns.IpamPoolMonitorStateSnapshot { // pushing it to the PoolMonitor's source channel. // As a side effect, marks the PoolMonitor as initialized, if it is not already. func (pm *Monitor) Update(nnc *v1alpha.NodeNetworkConfig) { - clampScaler(nnc) + clampScaler(&nnc.Status.Scaler) // if the nnc has conveged, observe the pool scaling latency (if any) allocatedIPs := len(pm.httpService.GetPodIPConfigState()) - len(pm.httpService.GetPendingReleaseIPConfigs()) @@ -307,41 +314,41 @@ func (pm *Monitor) Update(nnc *v1alpha.NodeNetworkConfig) { // we usually expect these to be correctly set for us, but we could crash // without these checks. if they are incorrectly set, there will be some weird // IP pool behavior for a while until the nnc reconciler corrects the state. -func clampScaler(nnc *v1alpha.NodeNetworkConfig) { - if nnc.Status.Scaler.MaxIPCount < 1 { - nnc.Status.Scaler.MaxIPCount = 1 +func clampScaler(scaler *v1alpha.Scaler) { + if scaler.MaxIPCount < 1 { + scaler.MaxIPCount = 1 } - if nnc.Status.Scaler.BatchSize < 1 { - nnc.Status.Scaler.BatchSize = 1 + if scaler.BatchSize < 1 { + scaler.BatchSize = 1 } - if nnc.Status.Scaler.BatchSize > nnc.Status.Scaler.MaxIPCount { - nnc.Status.Scaler.BatchSize = nnc.Status.Scaler.MaxIPCount + if scaler.BatchSize > scaler.MaxIPCount { + scaler.BatchSize = scaler.MaxIPCount } - if nnc.Status.Scaler.RequestThresholdPercent < 1 { - nnc.Status.Scaler.RequestThresholdPercent = 1 + if scaler.RequestThresholdPercent < 1 { + scaler.RequestThresholdPercent = 1 } - if nnc.Status.Scaler.RequestThresholdPercent > 100 { //nolint:gomnd // it's a percent - nnc.Status.Scaler.RequestThresholdPercent = 100 + if scaler.RequestThresholdPercent > 100 { //nolint:gomnd // it's a percent + scaler.RequestThresholdPercent = 100 } - if nnc.Status.Scaler.ReleaseThresholdPercent < nnc.Status.Scaler.RequestThresholdPercent+100 { - nnc.Status.Scaler.ReleaseThresholdPercent = nnc.Status.Scaler.RequestThresholdPercent + 100 //nolint:gomnd // it's a percent + if scaler.ReleaseThresholdPercent < scaler.RequestThresholdPercent+100 { + scaler.ReleaseThresholdPercent = scaler.RequestThresholdPercent + 100 //nolint:gomnd // it's a percent } } // CalculateMinFreeIPs calculates the minimum free IP quantity based on the Scaler // in the passed NodeNetworkConfig. //nolint:gocritic // ignore hugeparam -func CalculateMinFreeIPs(nnc v1alpha.NodeNetworkConfig) int { - batchSize := nnc.Status.Scaler.BatchSize - requestThreshold := nnc.Status.Scaler.RequestThresholdPercent +func CalculateMinFreeIPs(scaler v1alpha.Scaler) int { + batchSize := scaler.BatchSize + requestThreshold := scaler.RequestThresholdPercent return int(float64(batchSize) * (float64(requestThreshold) / 100)) //nolint:gomnd // it's a percent } // CalculateMaxFreeIPs calculates the maximum free IP quantity based on the Scaler // in the passed NodeNetworkConfig. //nolint:gocritic // ignore hugeparam -func CalculateMaxFreeIPs(nnc v1alpha.NodeNetworkConfig) int { - batchSize := nnc.Status.Scaler.BatchSize - releaseThreshold := nnc.Status.Scaler.ReleaseThresholdPercent +func CalculateMaxFreeIPs(scaler v1alpha.Scaler) int { + batchSize := scaler.BatchSize + releaseThreshold := scaler.ReleaseThresholdPercent return int(float64(batchSize) * (float64(releaseThreshold) / 100)) //nolint:gomnd // it's a percent } diff --git a/cns/ipampool/monitor_test.go b/cns/ipampool/monitor_test.go index b97cacf81a..ca6d7bf1a8 100644 --- a/cns/ipampool/monitor_test.go +++ b/cns/ipampool/monitor_test.go @@ -27,7 +27,7 @@ type directUpdatePoolMonitor struct { } func (d *directUpdatePoolMonitor) Update(nnc *v1alpha.NodeNetworkConfig) { - d.m.nnc = nnc + d.m.scaler, d.m.spec = nnc.Status.Scaler, nnc.Spec } type state struct { @@ -81,10 +81,10 @@ func TestPoolSizeIncrease(t *testing.T) { } // ensure pool monitor has reached quorum with cns - if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.ipConfigCount+(1*initState.batchSize)) { + if poolmonitor.spec.RequestedIPCount != int64(initState.ipConfigCount+(1*initState.batchSize)) { t.Fatalf("Pool monitor target IP count doesn't match CNS pool state "+ "after reconcile: %v, "+ - "actual %v", poolmonitor.nnc.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) + "actual %v", poolmonitor.spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) } // request controller reconciles, carves new IP's from the test subnet and adds to CNS state @@ -101,10 +101,10 @@ func TestPoolSizeIncrease(t *testing.T) { } // ensure pool monitor has reached quorum with cns - if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.ipConfigCount+(1*initState.batchSize)) { + if poolmonitor.spec.RequestedIPCount != int64(initState.ipConfigCount+(1*initState.batchSize)) { t.Fatalf("Pool monitor target IP count doesn't "+ "match CNS pool state after reconcile: %v, "+ - "actual %v", poolmonitor.nnc.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) + "actual %v", poolmonitor.spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) } // make sure IPConfig state size reflects the new pool size @@ -115,7 +115,7 @@ func TestPoolSizeIncrease(t *testing.T) { t.Logf("Pool size %v, Target pool size %v, "+ "Allocated IP's %v, ", len(fakecns.GetPodIPConfigState()), - poolmonitor.nnc.Spec.RequestedIPCount, len(fakecns.GetAllocatedIPConfigs())) + poolmonitor.spec.RequestedIPCount, len(fakecns.GetAllocatedIPConfigs())) } func TestPoolIncreaseDoesntChangeWhenIncreaseIsAlreadyInProgress(t *testing.T) { @@ -150,9 +150,9 @@ func TestPoolIncreaseDoesntChangeWhenIncreaseIsAlreadyInProgress(t *testing.T) { } // ensure pool monitor has reached quorum with cns - if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.ipConfigCount+(1*initState.batchSize)) { + if poolmonitor.spec.RequestedIPCount != int64(initState.ipConfigCount+(1*initState.batchSize)) { t.Fatalf("Pool monitor target IP count doesn't match CNS pool state after reconcile: %v,"+ - " actual %v", poolmonitor.nnc.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) + " actual %v", poolmonitor.spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) } // request controller reconciles, carves new IP's from the test subnet and adds to CNS state @@ -175,13 +175,13 @@ func TestPoolIncreaseDoesntChangeWhenIncreaseIsAlreadyInProgress(t *testing.T) { } // ensure pool monitor has reached quorum with cns - if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.ipConfigCount+(1*initState.batchSize)) { + if poolmonitor.spec.RequestedIPCount != int64(initState.ipConfigCount+(1*initState.batchSize)) { t.Fatalf("Pool monitor target IP count doesn't match CNS pool state after reconcile: %v, "+ - "actual %v", poolmonitor.nnc.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) + "actual %v", poolmonitor.spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) } t.Logf("Pool size %v, Target pool size %v, Allocated IP's %v, ", len(fakecns.GetPodIPConfigState()), - poolmonitor.nnc.Spec.RequestedIPCount, len(fakecns.GetAllocatedIPConfigs())) + poolmonitor.spec.RequestedIPCount, len(fakecns.GetAllocatedIPConfigs())) } func TestPoolSizeIncreaseIdempotency(t *testing.T) { @@ -204,9 +204,9 @@ func TestPoolSizeIncreaseIdempotency(t *testing.T) { } // ensure pool monitor has increased batch size - if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.ipConfigCount+(1*initState.batchSize)) { + if poolmonitor.spec.RequestedIPCount != int64(initState.ipConfigCount+(1*initState.batchSize)) { t.Fatalf("Pool monitor target IP count doesn't match CNS pool state after reconcile: %v,"+ - " actual %v", poolmonitor.nnc.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) + " actual %v", poolmonitor.spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) } // reconcile pool monitor a second time, then verify requested ip count is still the same @@ -216,9 +216,9 @@ func TestPoolSizeIncreaseIdempotency(t *testing.T) { } // ensure pool monitor requested pool size is unchanged as request controller hasn't reconciled yet - if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.ipConfigCount+(1*initState.batchSize)) { + if poolmonitor.spec.RequestedIPCount != int64(initState.ipConfigCount+(1*initState.batchSize)) { t.Fatalf("Pool monitor target IP count doesn't match CNS pool state after reconcile: %v,"+ - " actual %v", poolmonitor.nnc.Spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) + " actual %v", poolmonitor.spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) } } @@ -242,9 +242,9 @@ func TestPoolIncreasePastNodeLimit(t *testing.T) { } // ensure pool monitor has only requested the max pod ip count - if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.maxIPCount) { + if poolmonitor.spec.RequestedIPCount != int64(initState.maxIPCount) { t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) when the max "+ - "has been reached", poolmonitor.nnc.Spec.RequestedIPCount, initState.maxIPCount) + "has been reached", poolmonitor.spec.RequestedIPCount, initState.maxIPCount) } } @@ -268,9 +268,9 @@ func TestPoolIncreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { } // ensure pool monitor has only requested the max pod ip count - if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.maxIPCount) { + if poolmonitor.spec.RequestedIPCount != int64(initState.maxIPCount) { t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) "+ - "when the max has been reached", poolmonitor.nnc.Spec.RequestedIPCount, initState.maxIPCount) + "when the max has been reached", poolmonitor.spec.RequestedIPCount, initState.maxIPCount) } } @@ -297,7 +297,7 @@ func TestPoolDecrease(t *testing.T) { assert.NoError(t, poolmonitor.reconcile(context.Background())) // ensure that the adjusted spec is smaller than the initial pool size - assert.Equal(t, (initState.ipConfigCount - initState.batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse), "expected pool size to be smaller after reconcile") + assert.Equal(t, (initState.ipConfigCount - initState.batchSize), len(poolmonitor.spec.IPsNotInUse), "expected pool size to be smaller after reconcile") // reconcile the fake request controller assert.NoError(t, fakerc.Reconcile(true)) @@ -328,15 +328,15 @@ func TestPoolSizeDecreaseWhenDecreaseHasAlreadyBeenRequested(t *testing.T) { } // Ensure the size of the requested spec is still the same - if len(poolmonitor.nnc.Spec.IPsNotInUse) != (initState.ipConfigCount - initState.batchSize) { + if len(poolmonitor.spec.IPsNotInUse) != (initState.ipConfigCount - initState.batchSize) { t.Fatalf("Expected IP's not in use be one batch size smaller after reconcile, expected %v,"+ - " actual %v", (initState.ipConfigCount - initState.batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) + " actual %v", (initState.ipConfigCount - initState.batchSize), len(poolmonitor.spec.IPsNotInUse)) } // Ensure the request ipcount is now one batch size smaller than the initial IP count - if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.ipConfigCount-initState.batchSize) { + if poolmonitor.spec.RequestedIPCount != int64(initState.ipConfigCount-initState.batchSize) { t.Fatalf("Expected pool size to be one batch size smaller after reconcile, expected %v,"+ - " actual %v", (initState.ipConfigCount - initState.batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) + " actual %v", (initState.ipConfigCount - initState.batchSize), len(poolmonitor.spec.IPsNotInUse)) } // Update pods with IP count, ensure pool monitor stays the same until request controller reconciles @@ -346,17 +346,17 @@ func TestPoolSizeDecreaseWhenDecreaseHasAlreadyBeenRequested(t *testing.T) { } // Ensure the size of the requested spec is still the same - if len(poolmonitor.nnc.Spec.IPsNotInUse) != (initState.ipConfigCount - initState.batchSize) { + if len(poolmonitor.spec.IPsNotInUse) != (initState.ipConfigCount - initState.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", - (initState.ipConfigCount - initState.batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) + (initState.ipConfigCount - initState.batchSize), len(poolmonitor.spec.IPsNotInUse)) } // Ensure the request ipcount is now one batch size smaller than the initial IP count - if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.ipConfigCount-initState.batchSize) { + if poolmonitor.spec.RequestedIPCount != int64(initState.ipConfigCount-initState.batchSize) { t.Fatalf("Expected pool size to be one batch size smaller after reconcile, and not change after"+ " existing call, expected %v, actual %v", (initState.ipConfigCount - initState.batchSize), - len(poolmonitor.nnc.Spec.IPsNotInUse)) + len(poolmonitor.spec.IPsNotInUse)) } err = fakerc.Reconcile(true) @@ -370,9 +370,9 @@ func TestPoolSizeDecreaseWhenDecreaseHasAlreadyBeenRequested(t *testing.T) { } // Ensure the spec doesn't have any IPsNotInUse after request controller has reconciled - if len(poolmonitor.nnc.Spec.IPsNotInUse) != 0 { + if len(poolmonitor.spec.IPsNotInUse) != 0 { t.Fatalf("Expected IP's not in use to be 0 after reconcile, expected %v, actual %v", - (initState.ipConfigCount - initState.batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) + (initState.ipConfigCount - initState.batchSize), len(poolmonitor.spec.IPsNotInUse)) } } @@ -475,15 +475,15 @@ func TestPoolSizeDecreaseToReallyLow(t *testing.T) { } // Ensure the size of the requested spec is still the same - if len(poolmonitor.nnc.Spec.IPsNotInUse) != initState.batchSize { + if len(poolmonitor.spec.IPsNotInUse) != initState.batchSize { t.Fatalf("Expected IP's not in use is not correct, expected %v, actual %v", - initState.batchSize, len(poolmonitor.nnc.Spec.IPsNotInUse)) + initState.batchSize, len(poolmonitor.spec.IPsNotInUse)) } // Ensure the request ipcount is now one batch size smaller than the initial IP count - if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.ipConfigCount-initState.batchSize) { + if poolmonitor.spec.RequestedIPCount != int64(initState.ipConfigCount-initState.batchSize) { t.Fatalf("Expected pool size to be one batch size smaller after reconcile, expected %v, "+ - "actual %v", (initState.ipConfigCount - initState.batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) + "actual %v", (initState.ipConfigCount - initState.batchSize), len(poolmonitor.spec.IPsNotInUse)) } // Reconcile again, it should release the second batch @@ -494,15 +494,15 @@ func TestPoolSizeDecreaseToReallyLow(t *testing.T) { } // Ensure the size of the requested spec is still the same - if len(poolmonitor.nnc.Spec.IPsNotInUse) != initState.batchSize*2 { + if len(poolmonitor.spec.IPsNotInUse) != initState.batchSize*2 { t.Fatalf("Expected IP's not in use is not correct, expected %v, actual %v", initState.batchSize*2, - len(poolmonitor.nnc.Spec.IPsNotInUse)) + len(poolmonitor.spec.IPsNotInUse)) } // Ensure the request ipcount is now one batch size smaller than the initial IP count - if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.ipConfigCount-(initState.batchSize*2)) { + if poolmonitor.spec.RequestedIPCount != int64(initState.ipConfigCount-(initState.batchSize*2)) { t.Fatalf("Expected pool size to be one batch size smaller after reconcile, expected %v, "+ - "actual %v", (initState.ipConfigCount - initState.batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) + "actual %v", (initState.ipConfigCount - initState.batchSize), len(poolmonitor.spec.IPsNotInUse)) } t.Logf("Update Request Controller") @@ -517,9 +517,9 @@ func TestPoolSizeDecreaseToReallyLow(t *testing.T) { } // Ensure the spec doesn't have any IPsNotInUse after request controller has reconciled - if len(poolmonitor.nnc.Spec.IPsNotInUse) != 0 { + if len(poolmonitor.spec.IPsNotInUse) != 0 { t.Fatalf("Expected IP's not in use to be 0 after reconcile, expected %v, actual %v", - (initState.ipConfigCount - initState.batchSize), len(poolmonitor.nnc.Spec.IPsNotInUse)) + (initState.ipConfigCount - initState.batchSize), len(poolmonitor.spec.IPsNotInUse)) } } @@ -554,17 +554,17 @@ func TestDecreaseAfterNodeLimitReached(t *testing.T) { } // Ensure poolmonitor asked for a multiple of batch size - if poolmonitor.nnc.Spec.RequestedIPCount != int64(expectedRequestedIP) { + if poolmonitor.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, initState.maxIPCount, - poolmonitor.nnc.Spec.RequestedIPCount) + poolmonitor.spec.RequestedIPCount) } // Ensure we minused by the mod result - if len(poolmonitor.nnc.Spec.IPsNotInUse) != expectedDecreaseIP { + if len(poolmonitor.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.nnc.Spec.IPsNotInUse)) + "the max before scale down, but got %v", expectedDecreaseIP, len(poolmonitor.spec.IPsNotInUse)) } } @@ -599,9 +599,9 @@ func TestPoolDecreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { } // ensure pool monitor has only requested the max pod ip count - if poolmonitor.nnc.Spec.RequestedIPCount != int64(initState.maxIPCount) { + if poolmonitor.spec.RequestedIPCount != int64(initState.maxIPCount) { t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) when the max "+ - "has been reached", poolmonitor.nnc.Spec.RequestedIPCount, initState.maxIPCount) + "has been reached", poolmonitor.spec.RequestedIPCount, initState.maxIPCount) } } @@ -612,16 +612,16 @@ func ReconcileAndValidate(t *testing.T, poolmonitor *Monitor, expectedRequestCou } // Increased the new count to be 20 - if poolmonitor.nnc.Spec.RequestedIPCount != int64(expectedRequestCount) { + if poolmonitor.spec.RequestedIPCount != int64(expectedRequestCount) { t.Fatalf("RequestIPCount not same, expected %v, actual %v", expectedRequestCount, - poolmonitor.nnc.Spec.RequestedIPCount) + poolmonitor.spec.RequestedIPCount) } // Ensure there is no pending release ips - if len(poolmonitor.nnc.Spec.IPsNotInUse) != expectedIpsNotInUse { + if len(poolmonitor.spec.IPsNotInUse) != expectedIpsNotInUse { t.Fatalf("Expected IP's not in use, expected %v, actual %v", expectedIpsNotInUse, - len(poolmonitor.nnc.Spec.IPsNotInUse)) + len(poolmonitor.spec.IPsNotInUse)) } } From ea14b4d33c87c716ea56d415a2cd9c5abae5bcd9 Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Mon, 11 Oct 2021 16:38:58 -0500 Subject: [PATCH 10/14] soft-block on not-initialized-monitor Signed-off-by: Evan Baker --- cns/ipampool/monitor.go | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/cns/ipampool/monitor.go b/cns/ipampool/monitor.go index ca12eacce7..7c42ed8622 100644 --- a/cns/ipampool/monitor.go +++ b/cns/ipampool/monitor.go @@ -58,17 +58,24 @@ func (pm *Monitor) Start(ctx context.Context) error { defer ticker.Stop() for { - // block until something happens + // proceed when things happen: select { - case <-ctx.Done(): + case <-ctx.Done(): // calling context has closed, we'll exit. return errors.Wrap(ctx.Err(), "pool monitor context closed") - case <-ticker.C: - // block on ticks until we have initialized - <-pm.initialized - case nnc := <-pm.nncSource: + case <-ticker.C: // attempt to reconcile every tick. + select { + case <-pm.initialized: // this blocks until we have initialized + // if we have initialized and enter this case, we proceed out of the select and continue to reconcile. + default: + // if we have NOT initialized and enter this case, we continue out of this iteration and let the for loop begin again. + continue + } + case nnc := <-pm.nncSource: // received a new NodeNetworkConfig, extract the data from it and re-recancile. pm.spec = nnc.Spec pm.scaler = nnc.Status.Scaler + pm.once.Do(func() { close(pm.initialized) }) // close the init channel the first time we receive a NodeNetworkConfig. } + // if control has flowed through the select(s) to this point, we can now reconcile. err := pm.reconcile(ctx) if err != nil { logger.Printf("[ipam-pool-monitor] Reconcile failed with err %v", err) @@ -304,9 +311,6 @@ func (pm *Monitor) Update(nnc *v1alpha.NodeNetworkConfig) { // observe elapsed duration for IP pool scaling metric.ObserverPoolScaleLatency() } - - // defer closing the init channel to signify that we have received at least one NodeNetworkConfig. - defer pm.once.Do(func() { close(pm.initialized) }) pm.nncSource <- *nnc } From bc2a862034dd2289f94c458b50f9accce00da62c Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Mon, 11 Oct 2021 19:41:12 -0500 Subject: [PATCH 11/14] test scaler calculations Signed-off-by: Evan Baker --- cns/ipampool/monitor.go | 12 ++++++++--- cns/ipampool/monitor_test.go | 39 ++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/cns/ipampool/monitor.go b/cns/ipampool/monitor.go index 7c42ed8622..4f917f5f94 100644 --- a/cns/ipampool/monitor.go +++ b/cns/ipampool/monitor.go @@ -16,6 +16,8 @@ import ( const ( // DefaultRefreshDelay pool monitor poll delay default in seconds. DefaultRefreshDelay = 1 * time.Second + // DefaultMaxIPs default maximum allocatable IPs + DefaultMaxPods = 250 ) type nodeNetworkConfigSpecUpdater interface { @@ -24,6 +26,7 @@ type nodeNetworkConfigSpecUpdater interface { type Options struct { RefreshDelay time.Duration + MaxIPs int } type Monitor struct { @@ -42,6 +45,9 @@ func NewMonitor(httpService cns.HTTPService, nnccli nodeNetworkConfigSpecUpdater if opts.RefreshDelay < 1 { opts.RefreshDelay = DefaultRefreshDelay } + if opts.MaxIPs < 1 { + opts.MaxIPs = 250 + } return &Monitor{ opts: opts, httpService: httpService, @@ -303,7 +309,7 @@ func (pm *Monitor) GetStateSnapshot() cns.IpamPoolMonitorStateSnapshot { // pushing it to the PoolMonitor's source channel. // As a side effect, marks the PoolMonitor as initialized, if it is not already. func (pm *Monitor) Update(nnc *v1alpha.NodeNetworkConfig) { - clampScaler(&nnc.Status.Scaler) + pm.clampScaler(&nnc.Status.Scaler) // if the nnc has conveged, observe the pool scaling latency (if any) allocatedIPs := len(pm.httpService.GetPodIPConfigState()) - len(pm.httpService.GetPendingReleaseIPConfigs()) @@ -318,9 +324,9 @@ func (pm *Monitor) Update(nnc *v1alpha.NodeNetworkConfig) { // we usually expect these to be correctly set for us, but we could crash // without these checks. if they are incorrectly set, there will be some weird // IP pool behavior for a while until the nnc reconciler corrects the state. -func clampScaler(scaler *v1alpha.Scaler) { +func (pm *Monitor) clampScaler(scaler *v1alpha.Scaler) { if scaler.MaxIPCount < 1 { - scaler.MaxIPCount = 1 + scaler.MaxIPCount = int64(pm.opts.MaxIPs) } if scaler.BatchSize < 1 { scaler.BatchSize = 1 diff --git a/cns/ipampool/monitor_test.go b/cns/ipampool/monitor_test.go index ca6d7bf1a8..6279d998ec 100644 --- a/cns/ipampool/monitor_test.go +++ b/cns/ipampool/monitor_test.go @@ -625,3 +625,42 @@ func ReconcileAndValidate(t *testing.T, poolmonitor *Monitor, expectedRequestCou len(poolmonitor.spec.IPsNotInUse)) } } + +func TestCalculateIPs(t *testing.T) { + tests := []struct { + name string + in v1alpha.Scaler + wantMinFree int + wantMaxFree int + }{ + { + name: "good", + in: v1alpha.Scaler{ + BatchSize: 16, + RequestThresholdPercent: 50, + ReleaseThresholdPercent: 150, + MaxIPCount: 250, + }, + wantMinFree: 8, + wantMaxFree: 24, + }, + { + name: "good", + in: v1alpha.Scaler{ + BatchSize: 16, + RequestThresholdPercent: 100, + ReleaseThresholdPercent: 200, + MaxIPCount: 250, + }, + wantMinFree: 16, + wantMaxFree: 32, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.wantMinFree, CalculateMinFreeIPs(tt.in)) + assert.Equal(t, tt.wantMaxFree, CalculateMaxFreeIPs(tt.in)) + }) + } +} From 6bc40bbbeac0be575ca9b1b7cf974fb6b972e7ef Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Tue, 12 Oct 2021 14:13:36 -0500 Subject: [PATCH 12/14] update e2e scaling and checks Signed-off-by: Evan Baker --- test/integration/goldpinger/utils.go | 7 ++++--- test/integration/k8s_test.go | 19 ++++++++----------- test/integration/setup_test.go | 2 +- 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/test/integration/goldpinger/utils.go b/test/integration/goldpinger/utils.go index ad9bd14e0d..7c0a769306 100644 --- a/test/integration/goldpinger/utils.go +++ b/test/integration/goldpinger/utils.go @@ -1,3 +1,4 @@ +//go:build integration // +build integration package goldpinger @@ -62,10 +63,10 @@ func (c ClusterStats) PrintStats() { format := "cluster stats - " + "nodes in use: %d, " + "pod count: %d, " + - "pod health percentage: %2.2f, " + - "ping health percentage: %2.2f\n" + "pod health: %d/%d (%2.2f), " + + "ping health percentage: %d/%d (%2.2f)\n" podHealthPct := (float64(len(healthyPods)) / float64(podCount)) * 100 pingHealthPct := (float64(healthyPingCount) / float64(pingCount)) * 100 - fmt.Printf(format, len(nodes), podCount, podHealthPct, pingHealthPct) + fmt.Printf(format, len(nodes), podCount, len(healthyPods), podCount, podHealthPct, healthyPingCount, pingCount, pingHealthPct) } diff --git a/test/integration/k8s_test.go b/test/integration/k8s_test.go index 8566e066a5..8facf12d27 100644 --- a/test/integration/k8s_test.go +++ b/test/integration/k8s_test.go @@ -35,9 +35,8 @@ const ( gpServiceAccountPath = gpFolder + "/service-account.yaml" gpDaemonset = gpFolder + "/daemonset.yaml" gpDeployment = gpFolder + "/deployment.yaml" - - retryAttempts = 20 - retryDelaySec = 5 * time.Second + retryAttempts = 60 + retryDelaySec = 5 * time.Second ) var ( @@ -45,6 +44,7 @@ var ( kubeconfig = flag.String("test-kubeconfig", filepath.Join(homedir.HomeDir(), ".kube", "config"), "(optional) absolute path to the kubeconfig file") delegatedSubnetID = flag.String("delegated-subnet-id", "", "delegated subnet id for node labeling") delegatedSubnetName = flag.String("subnet-name", "", "subnet name for node labeling") + gpPodScaleCounts = []int{3, 15, 150, 3} ) func shouldLabelNodes() bool { @@ -142,12 +142,10 @@ func TestPodScaling(t *testing.T) { } }) - counts := []int{15, 5, 15, 150, 15} - - for _, c := range counts { + for _, c := range gpPodScaleCounts { count := c t.Run(fmt.Sprintf("replica count %d", count), func(t *testing.T) { - replicaCtx, cancel := context.WithTimeout(ctx, 60*time.Second) + replicaCtx, cancel := context.WithTimeout(ctx, (retryAttempts+1)*retryDelaySec) defer cancel() if err := updateReplicaCount(t, replicaCtx, deploymentsClient, deployment.Name, count); err != nil { @@ -192,7 +190,7 @@ func TestPodScaling(t *testing.T) { } t.Run("all pods can ping each other", func(t *testing.T) { - clusterCheckCtx, cancel := context.WithTimeout(ctx, 10*time.Minute) + clusterCheckCtx, cancel := context.WithTimeout(ctx, 20*time.Minute) defer cancel() clusterCheckFn := func() error { pf, err := NewPortForwarder(restConfig) @@ -215,7 +213,7 @@ func TestPodScaling(t *testing.T) { return nil } if err := defaultRetrier.Do(portForwardCtx, portForwardFn); err != nil { - t.Fatalf("could not start port forward within %v: %v", retryDelaySec.String(), err) + t.Fatalf("could not start port forward within %v: %v", retryDelaySec.String(), err) } defer streamHandle.Stop() @@ -235,8 +233,7 @@ func TestPodScaling(t *testing.T) { return errors.New("not all pings are healthy") } - retrier := retry.Retrier{Attempts: 5, Delay: 20 * time.Second} - if err := retrier.Do(clusterCheckCtx, clusterCheckFn); err != nil { + if err := defaultRetrier.Do(clusterCheckCtx, clusterCheckFn); err != nil { t.Fatalf("cluster could not reach healthy state: %v", err) } diff --git a/test/integration/setup_test.go b/test/integration/setup_test.go index a520c94e62..f000493f7f 100644 --- a/test/integration/setup_test.go +++ b/test/integration/setup_test.go @@ -180,7 +180,7 @@ func installCNIManagerDaemonset(ctx context.Context, clientset *kubernetes.Clien cni.Spec.Template.Spec.Containers[0].Image = getImageString(image, imageTag) cniDaemonsetClient := clientset.AppsV1().DaemonSets(cni.Namespace) - log.Printf("Installing CNI with image %s", cni.Spec.Template.Spec.Containers[0].Image) + log.Printf("Installing CNI with image %s", cni.Spec.Template.Spec.Containers[0].Image) if err = mustCreateDaemonset(ctx, cniDaemonsetClient, cni); err != nil { return nil, err From a2e17aa80bb5d10af93679fcdc4d45a03eafb16d Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Tue, 12 Oct 2021 15:23:43 -0500 Subject: [PATCH 13/14] put monitor fake back in fakes and cache free min/max Signed-off-by: Evan Baker --- cns/client/client_test.go | 3 +- cns/{ipampool/fake.go => fakes/monitor.go} | 9 ++- cns/ipampool/monitor.go | 69 +++++++++++----------- cns/restserver/api_test.go | 3 +- cns/restserver/ipam_test.go | 3 +- 5 files changed, 45 insertions(+), 42 deletions(-) rename cns/{ipampool/fake.go => fakes/monitor.go} (59%) diff --git a/cns/client/client_test.go b/cns/client/client_test.go index 29a149c41b..a6ea4793ea 100644 --- a/cns/client/client_test.go +++ b/cns/client/client_test.go @@ -18,7 +18,6 @@ import ( "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/common" "github.com/Azure/azure-container-networking/cns/fakes" - "github.com/Azure/azure-container-networking/cns/ipampool" "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/cns/restserver" "github.com/Azure/azure-container-networking/cns/types" @@ -204,7 +203,7 @@ func TestMain(m *testing.M) { }, }, } - svc.IPAMPoolMonitor = &ipampool.MonitorFake{IPsNotInUseCount: 13, NodeNetworkConfig: &fakeNNC} + svc.IPAMPoolMonitor = &fakes.MonitorFake{IPsNotInUseCount: 13, NodeNetworkConfig: &fakeNNC} if err != nil { logger.Errorf("Failed to create CNS object, err:%v.\n", err) diff --git a/cns/ipampool/fake.go b/cns/fakes/monitor.go similarity index 59% rename from cns/ipampool/fake.go rename to cns/fakes/monitor.go index 1cb2624de2..d9087f20b5 100644 --- a/cns/ipampool/fake.go +++ b/cns/fakes/monitor.go @@ -1,4 +1,7 @@ -package ipampool +//go:build !ignore_uncovered +// +build !ignore_uncovered + +package fakes import ( "context" @@ -26,8 +29,8 @@ func (*MonitorFake) Reconcile() error { func (f *MonitorFake) GetStateSnapshot() cns.IpamPoolMonitorStateSnapshot { return cns.IpamPoolMonitorStateSnapshot{ - MaximumFreeIps: CalculateMaxFreeIPs(f.NodeNetworkConfig.Status.Scaler), - MinimumFreeIps: CalculateMinFreeIPs(f.NodeNetworkConfig.Status.Scaler), + MaximumFreeIps: int(float64(f.NodeNetworkConfig.Status.Scaler.BatchSize) * (float64(f.NodeNetworkConfig.Status.Scaler.ReleaseThresholdPercent) / 100)), //nolint:gomnd // it's a percent + MinimumFreeIps: int(float64(f.NodeNetworkConfig.Status.Scaler.BatchSize) * (float64(f.NodeNetworkConfig.Status.Scaler.RequestThresholdPercent) / 100)), //nolint:gomnd // it's a percent UpdatingIpsNotInUseCount: f.IPsNotInUseCount, CachedNNC: *f.NodeNetworkConfig, } diff --git a/cns/ipampool/monitor.go b/cns/ipampool/monitor.go index 4f917f5f94..245b7331bc 100644 --- a/cns/ipampool/monitor.go +++ b/cns/ipampool/monitor.go @@ -17,28 +17,35 @@ const ( // DefaultRefreshDelay pool monitor poll delay default in seconds. DefaultRefreshDelay = 1 * time.Second // DefaultMaxIPs default maximum allocatable IPs - DefaultMaxPods = 250 + DefaultMaxIPs = 250 ) type nodeNetworkConfigSpecUpdater interface { UpdateSpec(context.Context, *v1alpha.NodeNetworkConfigSpec) (*v1alpha.NodeNetworkConfig, error) } +// poolState is the Monitor's view of the IP pool. +type poolState struct { + minFreeCount int + maxFreeCount int + notInUseCount int +} + type Options struct { RefreshDelay time.Duration MaxIPs int } type Monitor struct { - opts *Options - spec v1alpha.NodeNetworkConfigSpec - scaler v1alpha.Scaler - nnccli nodeNetworkConfigSpecUpdater - httpService cns.HTTPService - updatingIpsNotInUseCount int - initialized chan interface{} - nncSource chan v1alpha.NodeNetworkConfig - once sync.Once + opts *Options + spec v1alpha.NodeNetworkConfigSpec + scaler v1alpha.Scaler + state poolState + nnccli nodeNetworkConfigSpecUpdater + httpService cns.HTTPService + initialized chan interface{} + nncSource chan v1alpha.NodeNetworkConfig + once sync.Once } func NewMonitor(httpService cns.HTTPService, nnccli nodeNetworkConfigSpecUpdater, opts *Options) *Monitor { @@ -46,7 +53,7 @@ func NewMonitor(httpService cns.HTTPService, nnccli nodeNetworkConfigSpecUpdater opts.RefreshDelay = DefaultRefreshDelay } if opts.MaxIPs < 1 { - opts.MaxIPs = 250 + opts.MaxIPs = DefaultMaxIPs } return &Monitor{ opts: opts, @@ -76,9 +83,10 @@ func (pm *Monitor) Start(ctx context.Context) error { // if we have NOT initialized and enter this case, we continue out of this iteration and let the for loop begin again. continue } - case nnc := <-pm.nncSource: // received a new NodeNetworkConfig, extract the data from it and re-recancile. + case nnc := <-pm.nncSource: // received a new NodeNetworkConfig, extract the data from it and re-reconcile. pm.spec = nnc.Spec pm.scaler = nnc.Status.Scaler + pm.state.minFreeCount, pm.state.maxFreeCount = CalculateMinFreeIPs(pm.scaler), CalculateMaxFreeIPs(pm.scaler) pm.once.Do(func() { close(pm.initialized) }) // close the init channel the first time we receive a NodeNetworkConfig. } // if control has flowed through the select(s) to this point, we can now reconcile. @@ -117,7 +125,7 @@ func (pm *Monitor) reconcile(ctx context.Context) error { switch { // pod count is increasing - case freeIPConfigCount < int64(CalculateMinFreeIPs(pm.scaler)): + case freeIPConfigCount < int64(pm.state.minFreeCount): if pm.spec.RequestedIPCount == maxIPCount { // If we're already at the maxIPCount, don't try to increase return nil @@ -127,7 +135,7 @@ func (pm *Monitor) reconcile(ctx context.Context) error { return pm.increasePoolSize(ctx) // pod count is decreasing - case freeIPConfigCount >= int64(CalculateMaxFreeIPs(pm.scaler)): + case freeIPConfigCount >= int64(pm.state.maxFreeCount): logger.Printf("[ipam-pool-monitor] Decreasing pool size...%s ", msg) return pm.decreasePoolSize(ctx, pendingReleaseIPCount) @@ -210,8 +218,8 @@ func (pm *Monitor) decreasePoolSize(ctx context.Context, existingPendingReleaseI logger.Printf("[ipam-pool-monitor] updatedRequestedIPCount %v", updatedRequestedIPCount) - if pm.updatingIpsNotInUseCount == 0 || - pm.updatingIpsNotInUseCount < existingPendingReleaseIPCount { + if pm.state.notInUseCount == 0 || + pm.state.notInUseCount < existingPendingReleaseIPCount { logger.Printf("[ipam-pool-monitor] Marking IPs as PendingRelease, ipsToBeReleasedCount %d", int(decreaseIPCountBy)) var err error pendingIPAddresses, err = pm.httpService.MarkIPAsPendingRelease(int(decreaseIPCountBy)) @@ -226,11 +234,11 @@ func (pm *Monitor) decreasePoolSize(ctx context.Context, existingPendingReleaseI if newIpsMarkedAsPending { // cache the updatingPendingRelease so that we dont re-set new IPs to PendingRelease in case UpdateCRD call fails - pm.updatingIpsNotInUseCount = len(tempNNCSpec.IPsNotInUse) + pm.state.notInUseCount = len(tempNNCSpec.IPsNotInUse) } logger.Printf("[ipam-pool-monitor] Releasing IPCount in this batch %d, updatingPendingIpsNotInUse count %d", - len(pendingIPAddresses), pm.updatingIpsNotInUseCount) + len(pendingIPAddresses), pm.state.notInUseCount) tempNNCSpec.RequestedIPCount -= int64(len(pendingIPAddresses)) logger.Printf("[ipam-pool-monitor] Decreasing pool size, Current Pool Size: %v, 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)) @@ -249,8 +257,8 @@ func (pm *Monitor) decreasePoolSize(ctx context.Context, existingPendingReleaseI pm.spec = tempNNCSpec // clear the updatingPendingIpsNotInUse, as we have Updated the CRD - logger.Printf("[ipam-pool-monitor] cleaning the updatingPendingIpsNotInUse, existing length %d", pm.updatingIpsNotInUseCount) - pm.updatingIpsNotInUseCount = 0 + logger.Printf("[ipam-pool-monitor] cleaning the updatingPendingIpsNotInUse, existing length %d", pm.state.notInUseCount) + pm.state.notInUseCount = 0 return nil } @@ -291,11 +299,11 @@ func (pm *Monitor) createNNCSpecForCRD() v1alpha.NodeNetworkConfigSpec { // GetStateSnapshot gets a snapshot of the IPAMPoolMonitor struct. func (pm *Monitor) GetStateSnapshot() cns.IpamPoolMonitorStateSnapshot { - scaler, spec := pm.scaler, pm.spec + scaler, spec, state := pm.scaler, pm.spec, pm.state return cns.IpamPoolMonitorStateSnapshot{ - MinimumFreeIps: CalculateMinFreeIPs(scaler), - MaximumFreeIps: CalculateMaxFreeIPs(scaler), - UpdatingIpsNotInUseCount: pm.updatingIpsNotInUseCount, + MinimumFreeIps: state.minFreeCount, + MaximumFreeIps: state.maxFreeCount, + UpdatingIpsNotInUseCount: state.notInUseCount, CachedNNC: v1alpha.NodeNetworkConfig{ Spec: spec, Status: v1alpha.NodeNetworkConfigStatus{ @@ -306,12 +314,11 @@ func (pm *Monitor) GetStateSnapshot() cns.IpamPoolMonitorStateSnapshot { } // Update ingests a NodeNetworkConfig, clamping some values to ensure they are legal and then -// pushing it to the PoolMonitor's source channel. -// As a side effect, marks the PoolMonitor as initialized, if it is not already. +// pushing it to the Monitor's source channel. func (pm *Monitor) Update(nnc *v1alpha.NodeNetworkConfig) { pm.clampScaler(&nnc.Status.Scaler) - // if the nnc has conveged, observe the pool scaling latency (if any) + // if the nnc has converged, observe the pool scaling latency (if any). allocatedIPs := len(pm.httpService.GetPodIPConfigState()) - len(pm.httpService.GetPendingReleaseIPConfigs()) if int(nnc.Spec.RequestedIPCount) == allocatedIPs { // observe elapsed duration for IP pool scaling @@ -349,16 +356,12 @@ func (pm *Monitor) clampScaler(scaler *v1alpha.Scaler) { // in the passed NodeNetworkConfig. //nolint:gocritic // ignore hugeparam func CalculateMinFreeIPs(scaler v1alpha.Scaler) int { - batchSize := scaler.BatchSize - requestThreshold := scaler.RequestThresholdPercent - return int(float64(batchSize) * (float64(requestThreshold) / 100)) //nolint:gomnd // it's a percent + return int(float64(scaler.BatchSize) * (float64(scaler.RequestThresholdPercent) / 100)) //nolint:gomnd // it's a percent } // CalculateMaxFreeIPs calculates the maximum free IP quantity based on the Scaler // in the passed NodeNetworkConfig. //nolint:gocritic // ignore hugeparam func CalculateMaxFreeIPs(scaler v1alpha.Scaler) int { - batchSize := scaler.BatchSize - releaseThreshold := scaler.ReleaseThresholdPercent - return int(float64(batchSize) * (float64(releaseThreshold) / 100)) //nolint:gomnd // it's a percent + return int(float64(scaler.BatchSize) * (float64(scaler.ReleaseThresholdPercent) / 100)) //nolint:gomnd // it's a percent } diff --git a/cns/restserver/api_test.go b/cns/restserver/api_test.go index e0176985bd..e1566918f5 100644 --- a/cns/restserver/api_test.go +++ b/cns/restserver/api_test.go @@ -20,7 +20,6 @@ import ( "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/common" "github.com/Azure/azure-container-networking/cns/fakes" - "github.com/Azure/azure-container-networking/cns/ipampool" "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/cns/nmagentclient" "github.com/Azure/azure-container-networking/cns/types" @@ -926,7 +925,7 @@ func startService() error { return err } - svc.IPAMPoolMonitor = &ipampool.MonitorFake{} + svc.IPAMPoolMonitor = &fakes.MonitorFake{} if service != nil { // Create empty azure-cns.json. CNS should start successfully by deleting this file diff --git a/cns/restserver/ipam_test.go b/cns/restserver/ipam_test.go index f0ff82f57e..9ab3ae20d0 100644 --- a/cns/restserver/ipam_test.go +++ b/cns/restserver/ipam_test.go @@ -11,7 +11,6 @@ import ( "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/common" "github.com/Azure/azure-container-networking/cns/fakes" - "github.com/Azure/azure-container-networking/cns/ipampool" "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" ) @@ -38,7 +37,7 @@ func getTestService() *HTTPRestService { var config common.ServiceConfig httpsvc, _ := NewHTTPRestService(&config, fakes.NewFakeImdsClient(), fakes.NewFakeNMAgentClient()) svc = httpsvc.(*HTTPRestService) - svc.IPAMPoolMonitor = &ipampool.MonitorFake{} + svc.IPAMPoolMonitor = &fakes.MonitorFake{} setOrchestratorTypeInternal(cns.KubernetesCRD) return svc From 1d7f8c588715cf57cbb1913e94bb9c106dcd494f Mon Sep 17 00:00:00 2001 From: Evan Baker Date: Tue, 12 Oct 2021 17:20:38 -0500 Subject: [PATCH 14/14] testify Signed-off-by: Evan Baker --- cns/ipampool/monitor_test.go | 379 ++++++++--------------------------- 1 file changed, 79 insertions(+), 300 deletions(-) diff --git a/cns/ipampool/monitor_test.go b/cns/ipampool/monitor_test.go index 6279d998ec..698c72b469 100644 --- a/cns/ipampool/monitor_test.go +++ b/cns/ipampool/monitor_test.go @@ -28,6 +28,7 @@ type directUpdatePoolMonitor struct { func (d *directUpdatePoolMonitor) Update(nnc *v1alpha.NodeNetworkConfig) { d.m.scaler, d.m.spec = nnc.Status.Scaler, nnc.Spec + d.m.state.minFreeCount, d.m.state.maxFreeCount = CalculateMinFreeIPs(d.m.scaler), CalculateMaxFreeIPs(d.m.scaler) } type state struct { @@ -75,47 +76,23 @@ func TestPoolSizeIncrease(t *testing.T) { assert.NoError(t, fakerc.Reconcile(true)) // When poolmonitor reconcile is called, trigger increase and cache goal state - err := poolmonitor.reconcile(context.Background()) - if err != nil { - t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) - } + assert.NoError(t, poolmonitor.reconcile(context.Background())) // ensure pool monitor has reached quorum with cns - if poolmonitor.spec.RequestedIPCount != int64(initState.ipConfigCount+(1*initState.batchSize)) { - t.Fatalf("Pool monitor target IP count doesn't match CNS pool state "+ - "after reconcile: %v, "+ - "actual %v", poolmonitor.spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) - } + assert.Equal(t, int64(initState.ipConfigCount+(1*initState.batchSize)), poolmonitor.spec.RequestedIPCount) // request controller reconciles, carves new IP's from the test subnet and adds to CNS state - err = fakerc.Reconcile(true) - if err != nil { - t.Fatalf("Failed to reconcile fake requestcontroller with err: %v", err) - } + assert.NoError(t, fakerc.Reconcile(true)) // when poolmonitor reconciles again here, the IP count will be within the thresholds // so no CRD update and nothing pending - err = poolmonitor.reconcile(context.Background()) - if err != nil { - t.Fatalf("Failed to reconcile pool monitor after request controller updates CNS state: %v", err) - } + assert.NoError(t, poolmonitor.reconcile(context.Background())) // ensure pool monitor has reached quorum with cns - if poolmonitor.spec.RequestedIPCount != int64(initState.ipConfigCount+(1*initState.batchSize)) { - t.Fatalf("Pool monitor target IP count doesn't "+ - "match CNS pool state after reconcile: %v, "+ - "actual %v", poolmonitor.spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) - } + assert.Equal(t, int64(initState.ipConfigCount+(1*initState.batchSize)), poolmonitor.spec.RequestedIPCount) // make sure IPConfig state size reflects the new pool size - if len(fakecns.GetPodIPConfigState()) != initState.ipConfigCount+(1*initState.batchSize) { - t.Fatalf("CNS Pod IPConfig state count doesn't "+ - "match, expected: %v, actual %v", len(fakecns.GetPodIPConfigState()), initState.ipConfigCount+(1*initState.batchSize)) - } - - t.Logf("Pool size %v, Target pool size %v, "+ - "Allocated IP's %v, ", len(fakecns.GetPodIPConfigState()), - poolmonitor.spec.RequestedIPCount, len(fakecns.GetAllocatedIPConfigs())) + assert.Len(t, fakecns.GetPodIPConfigState(), initState.ipConfigCount+(1*initState.batchSize)) } func TestPoolIncreaseDoesntChangeWhenIncreaseIsAlreadyInProgress(t *testing.T) { @@ -132,56 +109,29 @@ func TestPoolIncreaseDoesntChangeWhenIncreaseIsAlreadyInProgress(t *testing.T) { assert.NoError(t, fakerc.Reconcile(true)) // When poolmonitor reconcile is called, trigger increase and cache goal state - err := poolmonitor.reconcile(context.Background()) - if err != nil { - t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) - } + assert.NoError(t, poolmonitor.reconcile(context.Background())) - // increase number of allocated IP's in CNS, within allocatable size but still inside trigger threshold, - err = fakecns.SetNumberOfAllocatedIPs(9) - if err != nil { - t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) - } + // increase number of allocated IP's in CNS, within allocatable size but still inside trigger threshold + assert.NoError(t, fakecns.SetNumberOfAllocatedIPs(9)) // poolmonitor reconciles, but doesn't actually update the CRD, because there is already a pending update - err = poolmonitor.reconcile(context.Background()) - if err != nil { - t.Fatalf("Failed to reconcile pool monitor after allocation ip increase with err: %v", err) - } + assert.NoError(t, poolmonitor.reconcile(context.Background())) // ensure pool monitor has reached quorum with cns - if poolmonitor.spec.RequestedIPCount != int64(initState.ipConfigCount+(1*initState.batchSize)) { - t.Fatalf("Pool monitor target IP count doesn't match CNS pool state after reconcile: %v,"+ - " actual %v", poolmonitor.spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) - } + assert.Equal(t, int64(initState.ipConfigCount+(1*initState.batchSize)), poolmonitor.spec.RequestedIPCount) // request controller reconciles, carves new IP's from the test subnet and adds to CNS state - err = fakerc.Reconcile(true) - if err != nil { - t.Fatalf("Failed to reconcile fake requestcontroller with err: %v", err) - } + assert.NoError(t, fakerc.Reconcile(true)) // when poolmonitor reconciles again here, the IP count will be within the thresholds // so no CRD update and nothing pending - err = poolmonitor.reconcile(context.Background()) - if err != nil { - t.Fatalf("Failed to reconcile pool monitor after request controller updates CNS state: %v", err) - } + assert.NoError(t, poolmonitor.reconcile(context.Background())) // make sure IPConfig state size reflects the new pool size - if len(fakecns.GetPodIPConfigState()) != initState.ipConfigCount+(1*initState.batchSize) { - t.Fatalf("CNS Pod IPConfig state count doesn't match, expected: %v, actual %v", - len(fakecns.GetPodIPConfigState()), initState.ipConfigCount+(1*initState.batchSize)) - } + assert.Len(t, fakecns.GetPodIPConfigState(), initState.ipConfigCount+(1*initState.batchSize)) // ensure pool monitor has reached quorum with cns - if poolmonitor.spec.RequestedIPCount != int64(initState.ipConfigCount+(1*initState.batchSize)) { - t.Fatalf("Pool monitor target IP count doesn't match CNS pool state after reconcile: %v, "+ - "actual %v", poolmonitor.spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) - } - - t.Logf("Pool size %v, Target pool size %v, Allocated IP's %v, ", len(fakecns.GetPodIPConfigState()), - poolmonitor.spec.RequestedIPCount, len(fakecns.GetAllocatedIPConfigs())) + assert.Equal(t, int64(initState.ipConfigCount+(1*initState.batchSize)), poolmonitor.spec.RequestedIPCount) } func TestPoolSizeIncreaseIdempotency(t *testing.T) { @@ -194,32 +144,20 @@ func TestPoolSizeIncreaseIdempotency(t *testing.T) { maxIPCount: 30, } - fakecns, fakerc, poolmonitor := initFakes(initState) + _, fakerc, poolmonitor := initFakes(initState) assert.NoError(t, fakerc.Reconcile(true)) // When poolmonitor reconcile is called, trigger increase and cache goal state - err := poolmonitor.reconcile(context.Background()) - if err != nil { - t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) - } + assert.NoError(t, poolmonitor.reconcile(context.Background())) // ensure pool monitor has increased batch size - if poolmonitor.spec.RequestedIPCount != int64(initState.ipConfigCount+(1*initState.batchSize)) { - t.Fatalf("Pool monitor target IP count doesn't match CNS pool state after reconcile: %v,"+ - " actual %v", poolmonitor.spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) - } + assert.Equal(t, int64(initState.ipConfigCount+(1*initState.batchSize)), poolmonitor.spec.RequestedIPCount) // reconcile pool monitor a second time, then verify requested ip count is still the same - err = poolmonitor.reconcile(context.Background()) - if err != nil { - t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) - } + assert.NoError(t, poolmonitor.reconcile(context.Background())) // ensure pool monitor requested pool size is unchanged as request controller hasn't reconciled yet - if poolmonitor.spec.RequestedIPCount != int64(initState.ipConfigCount+(1*initState.batchSize)) { - t.Fatalf("Pool monitor target IP count doesn't match CNS pool state after reconcile: %v,"+ - " actual %v", poolmonitor.spec.RequestedIPCount, len(fakecns.GetPodIPConfigState())) - } + assert.Equal(t, int64(initState.ipConfigCount+(1*initState.batchSize)), poolmonitor.spec.RequestedIPCount) } func TestPoolIncreasePastNodeLimit(t *testing.T) { @@ -236,16 +174,10 @@ func TestPoolIncreasePastNodeLimit(t *testing.T) { assert.NoError(t, fakerc.Reconcile(true)) // When poolmonitor reconcile is called, trigger increase and cache goal state - err := poolmonitor.reconcile(context.Background()) - if err != nil { - t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) - } + assert.NoError(t, poolmonitor.reconcile(context.Background())) // ensure pool monitor has only requested the max pod ip count - if poolmonitor.spec.RequestedIPCount != int64(initState.maxIPCount) { - t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) when the max "+ - "has been reached", poolmonitor.spec.RequestedIPCount, initState.maxIPCount) - } + assert.Equal(t, int64(initState.maxIPCount), poolmonitor.spec.RequestedIPCount) } func TestPoolIncreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { @@ -262,16 +194,10 @@ func TestPoolIncreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { assert.NoError(t, fakerc.Reconcile(true)) // When poolmonitor reconcile is called, trigger increase and cache goal state - err := poolmonitor.reconcile(context.Background()) - if err != nil { - t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) - } + assert.NoError(t, poolmonitor.reconcile(context.Background())) // ensure pool monitor has only requested the max pod ip count - if poolmonitor.spec.RequestedIPCount != int64(initState.maxIPCount) { - t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) "+ - "when the max has been reached", poolmonitor.spec.RequestedIPCount, initState.maxIPCount) - } + assert.Equal(t, int64(initState.maxIPCount), poolmonitor.spec.RequestedIPCount) } func TestPoolDecrease(t *testing.T) { @@ -297,7 +223,7 @@ func TestPoolDecrease(t *testing.T) { assert.NoError(t, poolmonitor.reconcile(context.Background())) // ensure that the adjusted spec is smaller than the initial pool size - assert.Equal(t, (initState.ipConfigCount - initState.batchSize), len(poolmonitor.spec.IPsNotInUse), "expected pool size to be smaller after reconcile") + assert.Len(t, poolmonitor.spec.IPsNotInUse, initState.ipConfigCount-initState.batchSize) // reconcile the fake request controller assert.NoError(t, fakerc.Reconcile(true)) @@ -305,7 +231,7 @@ func TestPoolDecrease(t *testing.T) { // 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. - assert.Zero(t, len(fakecns.GetPendingReleaseIPConfigs()), "expected 0 PendingReleaseIPConfigs") + assert.Empty(t, fakecns.GetPendingReleaseIPConfigs()) } func TestPoolSizeDecreaseWhenDecreaseHasAlreadyBeenRequested(t *testing.T) { @@ -322,58 +248,29 @@ func TestPoolSizeDecreaseWhenDecreaseHasAlreadyBeenRequested(t *testing.T) { assert.NoError(t, fakerc.Reconcile(true)) // Pool monitor does nothing, as the current number of IP's falls in the threshold - 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) - } + assert.NoError(t, poolmonitor.reconcile(context.Background())) // Ensure the size of the requested spec is still the same - if len(poolmonitor.spec.IPsNotInUse) != (initState.ipConfigCount - initState.batchSize) { - t.Fatalf("Expected IP's not in use be one batch size smaller after reconcile, expected %v,"+ - " actual %v", (initState.ipConfigCount - initState.batchSize), len(poolmonitor.spec.IPsNotInUse)) - } + assert.Len(t, poolmonitor.spec.IPsNotInUse, initState.ipConfigCount-initState.batchSize) // Ensure the request ipcount is now one batch size smaller than the initial IP count - if poolmonitor.spec.RequestedIPCount != int64(initState.ipConfigCount-initState.batchSize) { - t.Fatalf("Expected pool size to be one batch size smaller after reconcile, expected %v,"+ - " actual %v", (initState.ipConfigCount - initState.batchSize), len(poolmonitor.spec.IPsNotInUse)) - } + assert.Equal(t, int64(initState.ipConfigCount-initState.batchSize), poolmonitor.spec.RequestedIPCount) // Update pods with IP count, ensure pool monitor stays the same until request controller reconciles - err = fakecns.SetNumberOfAllocatedIPs(6) - if err != nil { - t.Error(err) - } + assert.NoError(t, fakecns.SetNumberOfAllocatedIPs(6)) // Ensure the size of the requested spec is still the same - if len(poolmonitor.spec.IPsNotInUse) != (initState.ipConfigCount - initState.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", - (initState.ipConfigCount - initState.batchSize), len(poolmonitor.spec.IPsNotInUse)) - } + assert.Len(t, poolmonitor.spec.IPsNotInUse, initState.ipConfigCount-initState.batchSize) // Ensure the request ipcount is now one batch size smaller than the initial IP count - if poolmonitor.spec.RequestedIPCount != int64(initState.ipConfigCount-initState.batchSize) { - t.Fatalf("Expected pool size to be one batch size smaller after reconcile, and not change after"+ - " existing call, expected %v, actual %v", (initState.ipConfigCount - initState.batchSize), - len(poolmonitor.spec.IPsNotInUse)) - } + assert.Equal(t, int64(initState.ipConfigCount-initState.batchSize), poolmonitor.spec.RequestedIPCount) - err = fakerc.Reconcile(true) - if err != nil { - t.Error(err) - } + assert.NoError(t, fakerc.Reconcile(true)) - err = poolmonitor.reconcile(context.Background()) - if err != nil { - t.Errorf("Expected no pool monitor failure after request controller reconcile: %v", err) - } + assert.NoError(t, poolmonitor.reconcile(context.Background())) // Ensure the spec doesn't have any IPsNotInUse after request controller has reconciled - if len(poolmonitor.spec.IPsNotInUse) != 0 { - t.Fatalf("Expected IP's not in use to be 0 after reconcile, expected %v, actual %v", - (initState.ipConfigCount - initState.batchSize), len(poolmonitor.spec.IPsNotInUse)) - } + assert.Empty(t, poolmonitor.spec.IPsNotInUse) } func TestDecreaseAndIncreaseToSameCount(t *testing.T) { @@ -388,58 +285,37 @@ func TestDecreaseAndIncreaseToSameCount(t *testing.T) { fakecns, fakerc, poolmonitor := initFakes(initState) assert.NoError(t, fakerc.Reconcile(true)) - - // Pool monitor will increase the count to 20 - t.Logf("Scaleup: Increase pool size to 20") - ReconcileAndValidate(t, poolmonitor, 20, 0) + assert.NoError(t, poolmonitor.reconcile(context.Background())) + assert.Equal(t, int64(20), poolmonitor.spec.RequestedIPCount) + assert.Empty(t, poolmonitor.spec.IPsNotInUse) // Update the IPConfig state - t.Logf("Reconcile with PodIPState") - err := fakerc.Reconcile(true) - if err != nil { - t.Error(err) - } + assert.NoError(t, fakerc.Reconcile(true)) // Release all IPs - err = fakecns.SetNumberOfAllocatedIPs(0) - if err != nil { - t.Error(err) - } - - t.Logf("Scaledown: Decrease pool size to 10") - ReconcileAndValidate(t, poolmonitor, 10, 10) + assert.NoError(t, fakecns.SetNumberOfAllocatedIPs(0)) + assert.NoError(t, poolmonitor.reconcile(context.Background())) + assert.Equal(t, int64(10), poolmonitor.spec.RequestedIPCount) + assert.Len(t, poolmonitor.spec.IPsNotInUse, 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, poolmonitor, 20, 10) + assert.NoError(t, fakecns.SetNumberOfAllocatedIPs(7)) + assert.NoError(t, poolmonitor.reconcile(context.Background())) + assert.Equal(t, int64(20), poolmonitor.spec.RequestedIPCount) + assert.Len(t, poolmonitor.spec.IPsNotInUse, 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, poolmonitor, 20, 10) - - t.Logf("Now update podipconfig state") - err = fakerc.Reconcile(true) - if err != nil { - t.Error(err) - } + assert.NoError(t, fakerc.Reconcile(false)) + assert.NoError(t, poolmonitor.reconcile(context.Background())) + assert.Equal(t, int64(20), poolmonitor.spec.RequestedIPCount) + assert.Len(t, poolmonitor.spec.IPsNotInUse, 10) - err = poolmonitor.reconcile(context.Background()) - if err != nil { - t.Errorf("Expected no pool monitor failure after request controller reconcile: %v", err) - } - ReconcileAndValidate(t, poolmonitor, 20, 0) + assert.NoError(t, fakerc.Reconcile(true)) + assert.NoError(t, poolmonitor.reconcile(context.Background())) + assert.NoError(t, poolmonitor.reconcile(context.Background())) + assert.Equal(t, int64(20), poolmonitor.spec.RequestedIPCount) + assert.Empty(t, poolmonitor.spec.IPsNotInUse) } func TestPoolSizeDecreaseToReallyLow(t *testing.T) { @@ -456,71 +332,32 @@ func TestPoolSizeDecreaseToReallyLow(t *testing.T) { assert.NoError(t, fakerc.Reconcile(true)) // Pool monitor does nothing, as the current number of IP's falls in the threshold - 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) - } + assert.NoError(t, poolmonitor.reconcile(context.Background())) // Now Drop the Allocated count to really low, say 3. This should trigger release in 2 batches - err = fakecns.SetNumberOfAllocatedIPs(3) - if err != nil { - t.Error(err) - } + assert.NoError(t, fakecns.SetNumberOfAllocatedIPs(3)) // Pool monitor does nothing, as the current number of IP's falls in the threshold - t.Logf("Reconcile after Allocated count from 33 -> 3, Exepected free count = 10") - 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) - } + assert.NoError(t, poolmonitor.reconcile(context.Background())) // Ensure the size of the requested spec is still the same - if len(poolmonitor.spec.IPsNotInUse) != initState.batchSize { - t.Fatalf("Expected IP's not in use is not correct, expected %v, actual %v", - initState.batchSize, len(poolmonitor.spec.IPsNotInUse)) - } + assert.Len(t, poolmonitor.spec.IPsNotInUse, initState.batchSize) // Ensure the request ipcount is now one batch size smaller than the initial IP count - if poolmonitor.spec.RequestedIPCount != int64(initState.ipConfigCount-initState.batchSize) { - t.Fatalf("Expected pool size to be one batch size smaller after reconcile, expected %v, "+ - "actual %v", (initState.ipConfigCount - initState.batchSize), len(poolmonitor.spec.IPsNotInUse)) - } + assert.Equal(t, int64(initState.ipConfigCount-initState.batchSize), poolmonitor.spec.RequestedIPCount) // Reconcile again, it should release the second batch - t.Logf("Reconcile again - 2, Exepected free count = 20") - 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) - } + assert.NoError(t, poolmonitor.reconcile(context.Background())) // Ensure the size of the requested spec is still the same - if len(poolmonitor.spec.IPsNotInUse) != initState.batchSize*2 { - t.Fatalf("Expected IP's not in use is not correct, expected %v, actual %v", initState.batchSize*2, - len(poolmonitor.spec.IPsNotInUse)) - } + assert.Len(t, poolmonitor.spec.IPsNotInUse, initState.batchSize*2) // Ensure the request ipcount is now one batch size smaller than the initial IP count - if poolmonitor.spec.RequestedIPCount != int64(initState.ipConfigCount-(initState.batchSize*2)) { - t.Fatalf("Expected pool size to be one batch size smaller after reconcile, expected %v, "+ - "actual %v", (initState.ipConfigCount - initState.batchSize), len(poolmonitor.spec.IPsNotInUse)) - } - - t.Logf("Update Request Controller") - err = fakerc.Reconcile(true) - if err != nil { - t.Error(err) - } + assert.Equal(t, int64(initState.ipConfigCount-(initState.batchSize*2)), poolmonitor.spec.RequestedIPCount) - err = poolmonitor.reconcile(context.Background()) - if err != nil { - t.Errorf("Expected no pool monitor failure after request controller reconcile: %v", err) - } - - // Ensure the spec doesn't have any IPsNotInUse after request controller has reconciled - if len(poolmonitor.spec.IPsNotInUse) != 0 { - t.Fatalf("Expected IP's not in use to be 0 after reconcile, expected %v, actual %v", - (initState.ipConfigCount - initState.batchSize), len(poolmonitor.spec.IPsNotInUse)) - } + assert.NoError(t, fakerc.Reconcile(true)) + assert.NoError(t, poolmonitor.reconcile(context.Background())) + assert.Empty(t, poolmonitor.spec.IPsNotInUse) } func TestDecreaseAfterNodeLimitReached(t *testing.T) { @@ -532,40 +369,18 @@ func TestDecreaseAfterNodeLimitReached(t *testing.T) { releaseThresholdPercent: 150, maxIPCount: 30, } - expectedRequestedIP := 16 - expectedDecreaseIP := int(initState.maxIPCount) % initState.batchSize fakecns, fakerc, poolmonitor := initFakes(initState) assert.NoError(t, fakerc.Reconcile(true)) - 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) - } + assert.NoError(t, poolmonitor.reconcile(context.Background())) // Trigger a batch release - err = fakecns.SetNumberOfAllocatedIPs(5) - if err != nil { - t.Error(err) - } - - 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) - } + assert.NoError(t, fakecns.SetNumberOfAllocatedIPs(5)) + assert.NoError(t, poolmonitor.reconcile(context.Background())) // Ensure poolmonitor asked for a multiple of batch size - if poolmonitor.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, initState.maxIPCount, - poolmonitor.spec.RequestedIPCount) - } - - // Ensure we minused by the mod result - if len(poolmonitor.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.spec.IPsNotInUse)) - } + assert.Equal(t, int64(16), poolmonitor.spec.RequestedIPCount) + assert.Len(t, poolmonitor.spec.IPsNotInUse, int(initState.maxIPCount%initState.batchSize)) } func TestPoolDecreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { @@ -582,48 +397,12 @@ func TestPoolDecreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { assert.NoError(t, fakerc.Reconcile(true)) // When poolmonitor reconcile is called, trigger increase and cache goal state - err := poolmonitor.reconcile(context.Background()) - if err != nil { - t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) - } + assert.NoError(t, poolmonitor.reconcile(context.Background())) // Trigger a batch release - err = fakecns.SetNumberOfAllocatedIPs(1) - if err != nil { - t.Error(err) - } - - 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) - } - - // ensure pool monitor has only requested the max pod ip count - if poolmonitor.spec.RequestedIPCount != int64(initState.maxIPCount) { - t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) when the max "+ - "has been reached", poolmonitor.spec.RequestedIPCount, initState.maxIPCount) - } -} - -func ReconcileAndValidate(t *testing.T, poolmonitor *Monitor, 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.spec.RequestedIPCount != int64(expectedRequestCount) { - t.Fatalf("RequestIPCount not same, expected %v, actual %v", - expectedRequestCount, - poolmonitor.spec.RequestedIPCount) - } - - // Ensure there is no pending release ips - if len(poolmonitor.spec.IPsNotInUse) != expectedIpsNotInUse { - t.Fatalf("Expected IP's not in use, expected %v, actual %v", - expectedIpsNotInUse, - len(poolmonitor.spec.IPsNotInUse)) - } + assert.NoError(t, fakecns.SetNumberOfAllocatedIPs(1)) + assert.NoError(t, poolmonitor.reconcile(context.Background())) + assert.Equal(t, int64(initState.maxIPCount), poolmonitor.spec.RequestedIPCount) } func TestCalculateIPs(t *testing.T) { @@ -634,7 +413,7 @@ func TestCalculateIPs(t *testing.T) { wantMaxFree int }{ { - name: "good", + name: "normal", in: v1alpha.Scaler{ BatchSize: 16, RequestThresholdPercent: 50, @@ -645,7 +424,7 @@ func TestCalculateIPs(t *testing.T) { wantMaxFree: 24, }, { - name: "good", + name: "200%", in: v1alpha.Scaler{ BatchSize: 16, RequestThresholdPercent: 100,