Add configuration check for env WARM_IP_TARGET and MINIMUM_IP_TARGET

README.md

@@ -323,8 +323,8 @@ Specifies the number of total IP addresses that the `ipamd` daemon should attemp
 addresses to keep available at all times, it sets a target number for a floor on how many total IP addresses are allocated. Setting to a
 non-positive value is same as setting this to 0 or not setting the variable.
 
-`MINIMUM_IP_TARGET` is for pre-scaling, `WARM_IP_TARGET` is for dynamic scaling. For example, suppose a cluster has an
-expected pod density of approximately 30 pods per node. If `WARM_IP_TARGET` is set to 30 to ensure there are enough IPs
+`MINIMUM_IP_TARGET` is for pre-scaling, `WARM_IP_TARGET` is for dynamic scaling, and when `MINIMUM_IP_TARGET` is set, it is required that `WARM_IP_TARGET` is also set to positive value. 
+For example, suppose a cluster has an expected pod density of approximately 30 pods per node. If `WARM_IP_TARGET` is set to 30 to ensure there are enough IPs
 allocated up front by the CNI, then 30 pods are deployed to the node, the CNI will allocate an additional 30 IPs, for
 a total of 60, accelerating IP exhaustion in the relevant subnets. If instead `MINIMUM_IP_TARGET` is set to 30 and
 `WARM_IP_TARGET` to 2, after the 30 pods are deployed the CNI would allocate an additional 2 IPs. This still provides
@@ -333,6 +333,9 @@ elasticity, but uses roughly half as many IPs as using WARM_IP_TARGET alone (32
 This also improves the reliability of the EKS cluster by reducing the number of calls necessary to allocate or deallocate
 private IPs, which may be throttled, especially at scaling-related times.
 
+Setting both `WARM_IP_TARGET` and `MINIMUM_IP_TARGET` will override `WARM_PREFIX_TARGET` and `WARM_ENI_TARGET`. For a detailed explanation, see
+[`WARM_PREFIX_TARGET`, `WARM_IP_TARGET` and `MINIMUM_IP_TARGET`](https://github.com/aws/amazon-vpc-cni-k8s/blob/master/docs/prefix-and-ip-target.md).
+
 #### `MAX_ENI`
 
 Type: Integer

cmd/aws-vpc-cni/main.go

@@ -83,6 +83,9 @@ const (
 	defaultEnPrefixDelegation    = false
 	defaultIPCooldownPeriod      = 30
 	defaultDisablePodV6          = false
+	defaultWarmIPTarget          = 0
+	defaultMinIPTarget           = 0
+	defaultWarmPrefixTarget      = 0
 
 	envHostCniBinPath        = "HOST_CNI_BIN_PATH"
 	envHostCniConfDirPath    = "HOST_CNI_CONFDIR_PATH"
@@ -377,21 +380,35 @@ func validateEnvVars() bool {
 	}
 
 	// Validate that IP_COOLDOWN_PERIOD is a valid integer
-	ipCooldownPeriod, err, input := utils.GetIntFromStringEnvVar(envIPCooldownPeriod, defaultIPCooldownPeriod)
+	ipCooldownPeriod, err, ipCooldownPeriodStr := utils.GetIntEnvVar(envIPCooldownPeriod, defaultIPCooldownPeriod)
 	if err != nil || ipCooldownPeriod < 0 {
-		log.Errorf("IP_COOLDOWN_PERIOD MUST be a valid positive integer. %s is invalid", input)
+		log.Errorf("IP_COOLDOWN_PERIOD MUST be a valid positive integer. %s is invalid", ipCooldownPeriodStr)
 		return false
 	}
 
 	prefixDelegationEn := utils.GetBoolAsStringEnvVar(envEnPrefixDelegation, defaultEnPrefixDelegation)
-	warmIPTarget := utils.GetEnv(envWarmIPTarget, "0")
-	warmPrefixTarget := utils.GetEnv(envWarmPrefixTarget, "0")
-	minimumIPTarget := utils.GetEnv(envMinIPTarget, "0")
+	warmIPTarget, err, warmIPTargetStr := utils.GetIntEnvVar(envWarmIPTarget, defaultWarmIPTarget)
+	if err != nil {
+		log.Errorf("error when trying to get env WARM_IP_TARGET: %s; input is %v", err, warmIPTargetStr)
+		return false
+	}
+	warmPrefixTarget, err, warmPrefixTargetStr := utils.GetIntEnvVar(envWarmPrefixTarget, defaultWarmPrefixTarget)
+	if err != nil {
+		log.Errorf("error when trying to get env WARM_PREFIX_TARGET: %s; input is %v", err, warmPrefixTargetStr)
+		return false
+	}
+	minimumIPTarget, err, minimumIPTargetStr := utils.GetIntEnvVar(envMinIPTarget, defaultMinIPTarget)
+	if err != nil {
+		log.Errorf("error when trying to get env MINIMUM_IP_TARGET: %s; input is %v", err, minimumIPTargetStr)
+		return false
+	}
 
-	// Note that these string values should probably be cast to integers, but the comparison for values greater than 0 works either way
-	if prefixDelegationEn && (warmIPTarget <= "0" && warmPrefixTarget <= "0" && minimumIPTarget <= "0") {
+	if prefixDelegationEn && (warmIPTarget == 0 && warmPrefixTarget == 0 && minimumIPTarget == 0) {
 		log.Errorf("Setting WARM_PREFIX_TARGET = 0 is not supported while WARM_IP_TARGET/MINIMUM_IP_TARGET is not set. Please configure either one of the WARM_{PREFIX/IP}_TARGET or MINIMUM_IP_TARGET env variables")
 		return false
+	} else if warmIPTargetStr == "" && minimumIPTargetStr != "" {
+		log.Errorf("WARM_IP_TARGET MUST be set when MINIMUM_IP_TARGET is set. Please configure both environment variables.")
+		return false
 	}
 	return true
 }

pkg/ipamd/datastore/data_store.go

@@ -222,7 +222,7 @@ func (addr AddressInfo) Assigned() bool {
 
 // getCooldownPeriod returns the time duration in seconds configured by the IP_COOLDOWN_PERIOD env variable
 func getCooldownPeriod() time.Duration {
-	cooldownVal, err, _ := utils.GetIntFromStringEnvVar(envIPCooldownPeriod, 30)
+	cooldownVal, err, _ := utils.GetIntEnvVar(envIPCooldownPeriod, 30)
 	if err != nil {
 		return 30 * time.Second
 	}

utils/utils.go

@@ -22,7 +22,7 @@ func GetBoolAsStringEnvVar(env string, defaultVal bool) bool {
 }
 
 // Parse environment variable and return integer representation of string, or default value if environment variable is unset
-func GetIntFromStringEnvVar(env string, defaultVal int) (int, error, string) {
+func GetIntEnvVar(env string, defaultVal int) (int, error, string) {
 	if val, ok := os.LookupEnv(env); ok {
 		parsedVal, err := strconv.Atoi(val)
 		if err == nil {

utils/utils_test.go

@@ -49,19 +49,19 @@ func TestGetEnv(t *testing.T) {
 }
 
 // Validate that GetIntFromStringEnvVar runs against acceptable format input without error
-func TestGetIntFromStringEnvVar(t *testing.T) {
+func TestGetIntEnvVar(t *testing.T) {
 	// Test environment flag variable not set
-	tmp, _, _ := GetIntFromStringEnvVar(envInt, defaultIntEnv)
+	tmp, _, _ := GetIntEnvVar(envInt, defaultIntEnv)
 	assert.Equal(t, tmp, defaultIntEnv)
 
 	// Test basic Integer as string set with acceptable format
 	os.Setenv(envInt, "20")
-	tmp, _, _ = GetIntFromStringEnvVar(envInt, defaultIntEnv)
+	tmp, _, _ = GetIntEnvVar(envInt, defaultIntEnv)
 	assert.Equal(t, tmp, 20)
 
 	// Test basic Integer as string set with unacceptable format
 	os.Setenv(envInt, "2O")
 	defer os.Unsetenv(envInt)
-	tmp, _, _ = GetIntFromStringEnvVar(envInt, defaultIntEnv)
+	tmp, _, _ = GetIntEnvVar(envInt, defaultIntEnv)
 	assert.Equal(t, tmp, -1)
 }