Add Windows secondary IP mode configurable options for managing IP ad…

…dress allocation aws#443
tzifudzi · Jul 19, 2024 · 9c7bc6e · 9c7bc6e
1 parent 020d23a
commit 9c7bc6e
Show file tree

Hide file tree

Showing 14 changed files with 915 additions and 154 deletions.
diff --git a/controllers/core/configmap_controller.go b/controllers/core/configmap_controller.go
@@ -46,8 +46,8 @@ type ConfigMapReconciler struct {
 	Condition                         condition.Conditions
 	curWinIPAMEnabledCond             bool
 	curWinPrefixDelegationEnabledCond bool
-	curWinPDWarmIPTarget              int
-	curWinPDMinIPTarget               int
+	curWinWarmIPTarget                int
+	curWinMinIPTarget                 int
 	curWinPDWarmPrefixTarget          int
 	Context                           context.Context
 }
@@ -116,21 +116,34 @@ func (r *ConfigMapReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		isPrefixFlagUpdated = true
 	}
 
-	// Check if configurations for Windows prefix delegation have changed
-	var isPDConfigUpdated bool
-	warmIPTarget, minIPTarget, warmPrefixTarget := config.ParseWinPDTargets(r.Log, configmap)
-	if r.curWinPDWarmIPTarget != warmIPTarget || r.curWinPDMinIPTarget != minIPTarget || r.curWinPDWarmPrefixTarget != warmPrefixTarget {
-		r.curWinPDWarmIPTarget = warmIPTarget
-		r.curWinPDMinIPTarget = minIPTarget
+	// Check if Windows IP target configurations in ConfigMap have changed
+	var isWinIPConfigsUpdated bool
+
+	warmIPTarget, minIPTarget, warmPrefixTarget, isPDEnabled := config.ParseWinIPTargetConfigs(r.Log, configmap)
+	var winMinIPTargetUpdated = r.curWinMinIPTarget != minIPTarget
+	var winWarmIPTargetUpdated = r.curWinWarmIPTarget != warmIPTarget
+	var winPDWarmPrefixTargetUpdated = r.curWinPDWarmPrefixTarget != warmPrefixTarget
+	if winWarmIPTargetUpdated || winMinIPTargetUpdated {
+		r.curWinWarmIPTarget = warmIPTarget
+		r.curWinMinIPTarget = minIPTarget
+		isWinIPConfigsUpdated = true
+	}
+	if isPDEnabled && winPDWarmPrefixTargetUpdated {
 		r.curWinPDWarmPrefixTarget = warmPrefixTarget
-		logger.Info("updated PD configs from configmap", config.WarmIPTarget, r.curWinPDWarmIPTarget,
-			config.MinimumIPTarget, r.curWinPDMinIPTarget, config.WarmPrefixTarget, r.curWinPDWarmPrefixTarget)
-
-		isPDConfigUpdated = true
+		isWinIPConfigsUpdated = true
+	}
+	if isWinIPConfigsUpdated {
+		logger.Info(
+			"Detected update in Windows IP configuration parameter values in ConfigMap",
+			config.WinWarmIPTarget, r.curWinWarmIPTarget,
+			config.WinMinimumIPTarget, r.curWinMinIPTarget,
+			config.WinWarmPrefixTarget, r.curWinPDWarmPrefixTarget,
+			config.EnableWindowsPrefixDelegationKey, isPDEnabled,
+		)
 	}
 
-	// Flag is updated, update all nodes
-	if isIPAMFlagUpdated || isPrefixFlagUpdated || isPDConfigUpdated {
+	var nodesRequireUpdate = isIPAMFlagUpdated || isPrefixFlagUpdated || isWinIPConfigsUpdated
+	if nodesRequireUpdate {
 		err := UpdateNodesOnConfigMapChanges(r.K8sAPI, r.NodeManager)
 		if err != nil {
 			// Error in updating nodes

diff --git a/controllers/core/configmap_controller_test.go b/controllers/core/configmap_controller_test.go
@@ -16,14 +16,9 @@ package controllers
 import (
 	"context"
 	"errors"
+	"strconv"
 	"testing"
 
-	mock_condition "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/condition"
-	mock_k8s "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/k8s"
-	mock_node "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/node"
-	mock_manager "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/node/manager"
-	"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/config"
-	cooldown "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider/branch/cooldown"
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
@@ -35,18 +30,35 @@ import (
 	fakeClient "sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+
+	mock_condition "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/condition"
+	mock_k8s "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/k8s"
+	mock_node "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/node"
+	mock_manager "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/node/manager"
+	"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/config"
+	cooldown "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider/branch/cooldown"
 )
 
 var (
 	mockConfigMap = &corev1.ConfigMap{
 		TypeMeta:   metav1.TypeMeta{},
 		ObjectMeta: metav1.ObjectMeta{Name: config.VpcCniConfigMapName, Namespace: config.KubeSystemNamespace},
-		Data:       map[string]string{config.EnableWindowsIPAMKey: "true", config.EnableWindowsPrefixDelegationKey: "true"},
+		Data: map[string]string{
+			config.EnableWindowsIPAMKey:             "true",
+			config.EnableWindowsPrefixDelegationKey: "true",
+			config.WinMinimumIPTarget:               strconv.Itoa(config.IPv4DefaultWinMinIPTarget),
+			config.WinWarmIPTarget:                  strconv.Itoa(config.IPv4DefaultWinWarmIPTarget),
+		},
 	}
 	mockConfigMapPD = &corev1.ConfigMap{
 		TypeMeta:   metav1.TypeMeta{},
 		ObjectMeta: metav1.ObjectMeta{Name: config.VpcCniConfigMapName, Namespace: config.KubeSystemNamespace},
-		Data:       map[string]string{config.EnableWindowsIPAMKey: "false", config.EnableWindowsPrefixDelegationKey: "true"},
+		Data: map[string]string{
+			config.EnableWindowsIPAMKey:             "false",
+			config.EnableWindowsPrefixDelegationKey: "true",
+			config.WinMinimumIPTarget:               strconv.Itoa(config.IPv4PDDefaultMinIPTargetSize),
+			config.WinWarmIPTarget:                  strconv.Itoa(config.IPv4PDDefaultWarmIPTargetSize),
+		},
 	}
 	mockConfigMapReq = reconcile.Request{
 		NamespacedName: types.NamespacedName{
@@ -89,11 +101,13 @@ func NewConfigMapMock(ctrl *gomock.Controller, mockObjects ...client.Object) Con
 	return ConfigMapMock{
 		MockNodeManager: mockNodeManager,
 		ConfigMapReconciler: &ConfigMapReconciler{
-			Client:      client,
-			Log:         zap.New(),
-			NodeManager: mockNodeManager,
-			K8sAPI:      mockK8sWrapper,
-			Condition:   mockCondition,
+			Client:             client,
+			Log:                zap.New(),
+			NodeManager:        mockNodeManager,
+			K8sAPI:             mockK8sWrapper,
+			Condition:          mockCondition,
+			curWinMinIPTarget:  config.IPv4DefaultWinMinIPTarget,
+			curWinWarmIPTarget: config.IPv4DefaultWinWarmIPTarget,
 		},
 		MockNode:                   mockNode,
 		MockK8sAPI:                 mockK8sWrapper,
@@ -127,7 +141,7 @@ func Test_Reconcile_ConfigMap_PD_Disabled_If_IPAM_Disabled(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
 
-	mock := NewConfigMapMock(ctrl, mockConfigMapPD)
+	mock := NewConfigMapMock(ctrl, mockConfigMap)
 	mock.MockCondition.EXPECT().IsWindowsIPAMEnabled().Return(false)
 	mock.MockCondition.EXPECT().IsWindowsPrefixDelegationEnabled().Return(false)
 	mock.MockK8sAPI.EXPECT().GetConfigMap(config.VpcCniConfigMapName, config.KubeSystemNamespace).Return(createCoolDownMockCM("30"), nil).AnyTimes()

diff --git a/pkg/config/loader.go b/pkg/config/loader.go
@@ -28,13 +28,14 @@ const (
 	// Default Configuration for Pod ENI resource type
 	PodENIDefaultWorker = 30
 
-	// Default Configuration for IPv4 resource type
-	IPv4DefaultWorker  = 2
-	IPv4DefaultWPSize  = 3
-	IPv4DefaultMaxDev  = 1
-	IPv4DefaultResSize = 0
-
-	// Default Configuration for IPv4 prefix resource type
+	// Default Windows Configuration for IPv4 resource type
+	IPv4DefaultWinWorkerCount  = 2
+	IPv4DefaultWinWarmIPTarget = 3
+	IPv4DefaultWinMinIPTarget  = 3
+	IPv4DefaultWinMaxDev       = 0
+	IPv4DefaultWinResSize      = 0
+
+	// Default Windows Configuration for IPv4 prefix resource type
 	IPv4PDDefaultWorker               = 2
 	IPv4PDDefaultWPSize               = 1
 	IPv4PDDefaultMaxDev               = 0
@@ -70,26 +71,45 @@ func LoadResourceConfig() map[string]ResourceConfig {
 func LoadResourceConfigFromConfigMap(log logr.Logger, vpcCniConfigMap *v1.ConfigMap) map[string]ResourceConfig {
 	resourceConfig := getDefaultResourceConfig()
 
-	warmIPTarget, minIPTarget, warmPrefixTarget := ParseWinPDTargets(log, vpcCniConfigMap)
+	warmIPTarget, minIPTarget, warmPrefixTarget, isPDEnabled := ParseWinIPTargetConfigs(log, vpcCniConfigMap)
 
 	// If no PD configuration is set in configMap or none is valid, return default resource config
 	if warmIPTarget == 0 && minIPTarget == 0 && warmPrefixTarget == 0 {
 		return resourceConfig
 	}
 
-	resourceConfig[ResourceNameIPAddressFromPrefix].WarmPoolConfig.WarmIPTarget = warmIPTarget
-	resourceConfig[ResourceNameIPAddressFromPrefix].WarmPoolConfig.MinIPTarget = minIPTarget
-	resourceConfig[ResourceNameIPAddressFromPrefix].WarmPoolConfig.WarmPrefixTarget = warmPrefixTarget
+	if isPDEnabled {
+		resourceConfig[ResourceNameIPAddressFromPrefix].WarmPoolConfig.WarmIPTarget = warmIPTarget
+		resourceConfig[ResourceNameIPAddressFromPrefix].WarmPoolConfig.MinIPTarget = minIPTarget
+		resourceConfig[ResourceNameIPAddressFromPrefix].WarmPoolConfig.WarmPrefixTarget = warmPrefixTarget
+	} else {
+		resourceConfig[ResourceNameIPAddress].WarmPoolConfig.WarmIPTarget = warmIPTarget
+		resourceConfig[ResourceNameIPAddress].WarmPoolConfig.MinIPTarget = minIPTarget
+		resourceConfig[ResourceNameIPAddress].WarmPoolConfig.WarmPrefixTarget = warmPrefixTarget // ignore warm prefix in secondary IP mode
+	}
 
 	return resourceConfig
 }
 
-// ParseWinPDTargets parses config map for Windows prefix delegation configurations set by users
-func ParseWinPDTargets(log logr.Logger, vpcCniConfigMap *v1.ConfigMap) (warmIPTarget int, minIPTarget int, warmPrefixTarget int) {
-	warmIPTarget, minIPTarget, warmPrefixTarget = 0, 0, 0
-
+// ParseWinIPTargetConfigs parses Windows IP target configuration parameters in the amazon-vpc-cni ConfigMap
+// If all three config parameter values (warm-ip-target, min-ip-target, warm-prefix-target) are 0 or unset, or config map does not exist,
+// then default values for warm-ip-target and min-ip-target will be set.
+func ParseWinIPTargetConfigs(log logr.Logger, vpcCniConfigMap *v1.ConfigMap) (warmIPTarget int, minIPTarget int, warmPrefixTarget int, isPDEnabled bool) {
 	if vpcCniConfigMap.Data == nil {
-		return warmIPTarget, minIPTarget, warmPrefixTarget
+		warmIPTarget = IPv4DefaultWinWarmIPTarget
+		minIPTarget = IPv4DefaultWinMinIPTarget
+		log.V(1).Info(
+			"No ConfigMap data found, falling back to using default values",
+			"minIPTarget", minIPTarget,
+			"warmIPTarget", warmIPTarget,
+		)
+		return warmIPTarget, minIPTarget, 0, false
+	}
+
+	isPDEnabled, err := strconv.ParseBool(vpcCniConfigMap.Data[EnableWindowsPrefixDelegationKey])
+	if err != nil {
+		log.V(1).Info("Could not parse prefix delegation flag from ConfigMap, falling back to using secondary IP mode")
+		isPDEnabled = false
 	}
 
 	warmIPTargetStr, foundWarmIP := vpcCniConfigMap.Data[WarmIPTarget]
@@ -105,36 +125,67 @@ func ParseWinPDTargets(log logr.Logger, vpcCniConfigMap *v1.ConfigMap) (warmIPTa
 		warmPrefixTargetStr, foundWarmPrefix = vpcCniConfigMap.Data[WinWarmPrefixTarget]
 	}
 
-	// If no configuration is found, return 0
-	if !foundWarmIP && !foundMinIP && !foundWarmPrefix {
-		return warmIPTarget, minIPTarget, warmPrefixTarget
-	}
-
+	// If warm IP target config value is not found, or there is an error parsing it, the value will be set to zero
 	if foundWarmIP {
 		warmIPTargetInt, err := strconv.Atoi(warmIPTargetStr)
 		if err != nil {
-			log.Error(err, "failed to parse warm ip target", "warm ip target", warmIPTargetStr)
+			log.V(1).Info("could not parse warm ip target, defaulting to zero", "warm ip target", warmIPTargetStr)
+			warmIPTarget = 0
 		} else {
 			warmIPTarget = warmIPTargetInt
 		}
+	} else {
+		log.V(1).Info("could not find warm ip target in ConfigMap, defaulting to zero")
+		warmIPTarget = 0
 	}
+
+	// If min IP target config value is not found, or there is an error parsing it, the value will be set to zero
 	if foundMinIP {
 		minIPTargetInt, err := strconv.Atoi(minIPTargetStr)
 		if err != nil {
-			log.Error(err, "failed to parse minimum ip target", "minimum ip target", minIPTargetStr)
+			log.V(1).Info("could not parse minimum ip target, defaulting to zero", "minimum ip target", minIPTargetStr)
+			minIPTarget = 0
 		} else {
 			minIPTarget = minIPTargetInt
 		}
+	} else {
+		log.V(1).Info("could not find minimum ip target in ConfigMap, defaulting to zero")
+		minIPTarget = 0
 	}
-	if foundWarmPrefix {
+
+	// If PD is enabled and warm prefix target config value is not found, or there is an error parsing it, the value will be set to zero
+	if !isPDEnabled && foundWarmPrefix {
+		log.V(1).Info("warm prefix configuration not supported in secondary IP mode, will ignore warm prefix configuration")
+		warmPrefixTarget = 0
+	} else if isPDEnabled && foundWarmPrefix {
 		warmPrefixTargetInt, err := strconv.Atoi(warmPrefixTargetStr)
 		if err != nil {
-			log.Error(err, "failed to parse warm prefix target", "warm prefix target", warmPrefixTargetStr)
+			log.Error(err, "failed to parse warm prefix target, defaulting to zero", "warm prefix target", warmPrefixTargetStr)
+			warmPrefixTarget = 0
 		} else {
 			warmPrefixTarget = warmPrefixTargetInt
 		}
+	} else if isPDEnabled && !foundWarmPrefix {
+		log.V(1).Info("could not find warm prefix target in ConfigMap, defaulting to zero")
+		warmPrefixTarget = 0
+	}
+
+	if warmIPTarget == 0 && minIPTarget == 0 {
+		if isPDEnabled && warmPrefixTarget == 0 {
+			minIPTarget = IPv4PDDefaultMinIPTargetSize
+			warmIPTarget = IPv4PDDefaultWarmIPTargetSize
+		} else if !isPDEnabled {
+			minIPTarget = IPv4DefaultWinMinIPTarget
+			warmIPTarget = IPv4DefaultWinWarmIPTarget
+		}
+		log.V(1).Info(
+			"No valid configuration values for warm-ip-target, min-ip-target and warm-prefix-target found in ConfigMap, falling back to using default values",
+			"minIPTarget", minIPTarget,
+			"warmIPTarget", warmIPTarget,
+		)
 	}
-	return warmIPTarget, minIPTarget, warmPrefixTarget
+
+	return warmIPTarget, minIPTarget, warmPrefixTarget, isPDEnabled
 }
 
 // getDefaultResourceConfig returns the default Resource Configuration.
@@ -153,13 +204,15 @@ func getDefaultResourceConfig() map[string]ResourceConfig {
 
 	// Create default configuration for IPv4 Resource
 	ipV4WarmPoolConfig := WarmPoolConfig{
-		DesiredSize:  IPv4DefaultWPSize,
-		MaxDeviation: IPv4DefaultMaxDev,
-		ReservedSize: IPv4DefaultResSize,
+		DesiredSize:  IPv4DefaultWinWarmIPTarget,
+		WarmIPTarget: IPv4DefaultWinWarmIPTarget,
+		MinIPTarget:  IPv4DefaultWinMinIPTarget,
+		MaxDeviation: IPv4DefaultWinMaxDev,
+		ReservedSize: IPv4DefaultWinResSize,
 	}
 	ipV4Config := ResourceConfig{
 		Name:           ResourceNameIPAddress,
-		WorkerCount:    IPv4DefaultWorker,
+		WorkerCount:    IPv4DefaultWinWorkerCount,
 		SupportedOS:    map[string]bool{OSWindows: true, OSLinux: false},
 		WarmPoolConfig: &ipV4WarmPoolConfig,
 	}