fix: prevent ipampoolmonitor from reconciling before it receives a nodenetworkconfig

cns/api.go

@@ -214,15 +214,15 @@ type NodeConfiguration struct {
 }
 
 type IPAMPoolMonitor interface {
-	Start(ctx context.Context, poolMonitorRefreshMilliseconds int) error
-	Update(scalar v1alpha.Scaler, spec v1alpha.NodeNetworkConfigSpec)
+	Start(ctx context.Context) error
+	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
 }

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
 )
@@ -97,9 +97,23 @@ 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{
+		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{
+				{},
+			},
+		},
+	})
 }
 
 func getIPNetFromResponse(resp *cns.IPConfigResponse) (net.IPNet, error) {
@@ -167,8 +181,9 @@ func TestMain(m *testing.M) {
 		Status: v1alpha.NodeNetworkConfigStatus{
 			Scaler: v1alpha.Scaler{
 				BatchSize:               10,
-				ReleaseThresholdPercent: 50,
-				RequestThresholdPercent: 40,
+				ReleaseThresholdPercent: 150,
+				RequestThresholdPercent: 50,
+				MaxIPCount:              250,
 			},
 			NetworkContainers: []v1alpha.NetworkContainer{
 				{
@@ -188,7 +203,7 @@ func TestMain(m *testing.M) {
 			},
 		},
 	}
-	svc.IPAMPoolMonitor = &fakes.IPAMPoolMonitorFake{FakeMinimumIps: 10, FakeMaximumIps: 20, FakeIpsNotInUseCount: 13, FakecachedNNC: fakeNNC}
+	svc.IPAMPoolMonitor = &fakes.MonitorFake{IPsNotInUseCount: 13, NodeNetworkConfig: &fakeNNC}
 
 	if err != nil {
 		logger.Errorf("Failed to create CNS object, err:%v.\n", err)
@@ -347,8 +362,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
@@ -357,8 +372,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: ")
@@ -1406,4 +1421,4 @@ func TestGetHTTPServiceData(t *testing.T) {
 			assert.Equal(t, tt.want, got)
 		})
 	}
-}
+}

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 {

cns/fakes/monitor.go

@@ -0,0 +1,37 @@
+//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 MonitorFake struct {
+	IPsNotInUseCount  int
+	NodeNetworkConfig *v1alpha.NodeNetworkConfig
+}
+
+func (*MonitorFake) Start(ctx context.Context) error {
+	return nil
+}
+
+func (f *MonitorFake) Update(nnc *v1alpha.NodeNetworkConfig) {
+	f.NodeNetworkConfig = nnc
+}
+
+func (*MonitorFake) Reconcile() error {
+	return nil
+}
+
+func (f *MonitorFake) GetStateSnapshot() cns.IpamPoolMonitorStateSnapshot {
+	return cns.IpamPoolMonitorStateSnapshot{
+		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,
+	}
+}

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
 }
 

cns/ipampoolmonitor/metrics.go → cns/ipampool/metrics.go

@@ -1,4 +1,4 @@
-package ipampoolmonitor
+package ipampool
 
 import (
 	"github.com/prometheus/client_golang/prometheus"