Add Windows secondary IP mode configurable options (#443)

#443

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

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()

docs/windows/secondary_ip_mode_config_options.md

@@ -0,0 +1,63 @@
+# Configuration options when using secondary IP addresses Windows
+
+We provide multiple configuration options that allow you to fine-tune the IP address allocation behavior on Windows
+nodes using the secondary IP address mode. These configuration options can be set in the `amazon-vpc-cni` ConfigMap in
+the `kube-system` namespace.
+
+- `windows-warm-ip-target` → The total number of IP addresses that should be allocated to each Windows node in excess of
+  the current need at any given time. The excess IPs can be used by newly launched pods, which aids in faster pod
+  startup times since there is no wait time for additional IP addresses to be allocated. The VPC Resource Controller
+  will attempt to ensure that this excess desired threshold is always met.
+
+  Defaults to 3 if unspecified or invalid. Must be greater than or equal to 1.
+
+  For example, if no pods were running on a given Windows node, and if you set `windows-warm-ip-target` to 5, the VPC
+  Resource Controller will aim to ensure that each Windows node always has at least 5 IP addresses in excess, ready for
+  use, allocated to its ENI. If 2 pods are scheduled on the node, the controller will allocate 2 additional IP addresses
+  to the ENI, maintaining the 5 warm IP address target.
+
+- `windows-minimum-ip-target` → Defaults to 3 if unspecified or invalid. The minimum number of IP addresses, both in use
+  by running pods and available as warm IPs, that should be allocated to each Windows node at any given time. The
+  controller will attempt to ensure that this minimum threshold is always met.
+
+  Defaults to 3 if unspecified or invalid. Must be greater than or equal to 0.
+
+  For example, if no pods were running on a given Windows node, and if you set `windows-minimum-ip-target` to 10, the
+  VPC Resource Controller will aim to ensure that the total number of IP addresses on the Windows node should be at
+  least 10. Therefore, before pods are scheduled, there should be at least 10 IP addresses available. If 5 pods are
+  scheduled on a given node, they will consume 5 of the 10 available IPs. The VPC Resource Controller will keep 5 the
+  remaining available IPs available in addition to the 5 already in use to meet the target of 10.
+
+### Considerations while using the above configuration options
+
+- These configuration options only apply when the VPC Resource Controller is operating in the secondary IP mode. They do
+  not affect the prefix delegation mode. More explicitly, if `enable-windows-prefix-delegation` is set to false, or is
+  not specified, then the VPC Resource Controller operates in secondary IP mode.
+- Setting either `windows-warm-ip-target` or `windows-minimum-ip-target` to a negative value will result in the
+  respective default value being used.
+- If the values of `windows-warm-ip-target` or `windows-minimum-ip-target` are set such that the maximum node IP
+  capacity would be exceeded, the controller will limit the allocation to the maximum capacity possible.
+- The `warm-prefix-target` configuration option will be ignored when using the secondary IP mode, as it only applies to
+  the prefix delegation mode.
+- If `windows-warm-ip-target` is set to 0, the system will implicitly set `windows-warm-ip-target` to 1. This is
+  because on-demand IP allocation whereby an IP is allocated on the Windows node as the pods are scheduled is currently
+  not supported. Implicitly Setting `windows-warm-ip-target` to 1 ensures the minimum acceptable non-zero value is set
+  since the `windows-warm-ip-target` should always be at least 1.
+- The configuration options `warm-ip-target` and `minimum-ip-target` are deprecated in favor of the new
+  options `windows-warm-ip-target` and `windows-minimum-ip-target`.
+
+### Examples
+
+| `windows-warm-ip-target` | `windows-minimum-ip-target` | Running Pods | Total Allocated IPs | Warm IPs |
+|--------------------------|-----------------------------|--------------|---------------------|----------|
+| 1                        | 0                           | 0            | 1                   | 1        |
+| 1                        | 0                           | 5            | 6                   | 1        |
+| 5                        | 0                           | 0            | 5                   | 5        |
+| 1                        | 1                           | 0            | 1                   | 1        |
+| 1                        | 1                           | 1            | 2                   | 1        |
+| 1                        | 3                           | 3            | 4                   | 1        |
+| 1                        | 3                           | 5            | 6                   | 1        |
+| 5                        | 10                          | 0            | 10                  | 10       |
+| 10                       | 10                          | 0            | 10                  | 10       |
+| 10                       | 10                          | 10           | 20                  | 10       |
+| 15                       | 10                          | 10           | 25                  | 15       |

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,74 @@ 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
+
+			// Handle secondary IP mode scenario where WarmIPTarget is explicitly configured to zero
+			// In such a case there must always be 1 warm IP to ensure that the warmpool is never empty
+			if !isPDEnabled && warmIPTarget == 0 {
+				log.V(1).Info("Explicitly setting WarmIPTarget zero value not supported in secondary IP mode, will override with 1")
+				warmIPTarget = 1
+			}
 		}
+	} 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
 	}
-	return warmIPTarget, minIPTarget, warmPrefixTarget
+
+	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, isPDEnabled
 }
 
 // getDefaultResourceConfig returns the default Resource Configuration.
@@ -153,13 +211,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,
 	}