Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default WARM_PREFIX_TARGET to 1 #1515

Merged
merged 2 commits into from
Jun 17, 2021
Merged

Default WARM_PREFIX_TARGET to 1 #1515

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6903ddc
Handle prefix target 0
jayanthvn Jun 17, 2021
112a8b8
pr commets
jayanthvn Jun 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 5 additions & 16 deletions pkg/ipamd/ipamd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ const (

//envWarmPrefixTarget is used to keep a /28 prefix in warm pool.
envWarmPrefixTarget = "WARM_PREFIX_TARGET"
noWarmPrefixTarget = 0
defaultWarmPrefixTarget = 0
)

var log = logger.Get()
Expand Down Expand Up @@ -961,17 +961,17 @@ func getWarmPrefixTarget() int {
inputStr, found := os.LookupEnv(envWarmPrefixTarget)

if !found {
return noWarmPrefixTarget
return defaultWarmPrefixTarget
}

if input, err := strconv.Atoi(inputStr); err == nil {
if input < 0 {
return noWarmPrefixTarget
return defaultWarmPrefixTarget
}
log.Debugf("Using WARM_PREFIX_TARGET %v", input)
return input
}
return noWarmPrefixTarget
return defaultWarmPrefixTarget
}

func logPoolStats(total int, used int, maxAddrsPerENI int, Ipv4PrefixDelegation bool) {
Expand Down Expand Up @@ -1039,19 +1039,8 @@ func (c *IPAMContext) shouldRemoveExtraPrefixes() int {
available := total - used

freePrefixes := c.dataStore.GetFreePrefixes()
_, numIPsPerPrefix, _ := datastore.GetPrefixDelegationDefaults()

over = max(freePrefixes-c.warmPrefixTarget, 0)

//if warm prefix target is 0, we should not make available to reach 0 since reconciler will enter into a churn
// of allocating and freeing prefixes similar to issue. This will add continous EC2 calls on nodes which have
// no prefixes used. Hence better to handle it.
// [ENI allocates/frees in loop with custom networking and WARM_ENI_TARGET=0 #1451]
// But Will sync up internally with the team and see if need to even support WARM_ENI_TARGET = 0 and WARM_PREFIX_TARGET = 0
if over > 0 && ((available - (over * numIPsPerPrefix)) <= 0) && c.warmPrefixTarget == 0 {
log.Debugf("Warm prefix target is 0 and we will go below available hence capping to have atleast 1 free prefix")
over -= 1
}
logPoolStats(total, used, c.maxIPsPerENI, c.enableIpv4PrefixDelegation)
log.Debugf("shouldRemoveExtraPrefixes available %d over %d warm_prefix_target %d", available, over, c.warmPrefixTarget)

Expand Down Expand Up @@ -1807,7 +1796,7 @@ func (c *IPAMContext) isDatastorePoolTooHigh() bool {
}

func (c *IPAMContext) warmPrefixTargetDefined() bool {
return c.warmPrefixTarget >= noWarmPrefixTarget && c.enableIpv4PrefixDelegation
return c.warmPrefixTarget >= defaultWarmPrefixTarget && c.enableIpv4PrefixDelegation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will we ever change defaultWarmPrefixTarget to a value than 0?
If we change it to be same thing like 2, then this read as c.warmPrefixTarget >= 2. I feel this should be something like c.warmPrefixTarget >= 0

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

defaultWarmPrefixTarget is 1 in this PR and also setting just WARM_PREFIX_TARGET = 0 won't be supported. If someone sets WARM_IP_TARGET = num and WARM_PREFIX_TARGET = 0 or any num then WARM_IP_TARGET will take precedence.

}

//DeallocCidrs frees IPs and Prefixes from EC2
Expand Down
28 changes: 28 additions & 0 deletions scripts/entrypoint.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,36 @@ log_in_json()
printf '{"level":"%s","ts":"%s","caller":"%s","msg":"%s"}\n' "$LOGTYPE" "$TIMESTAMP" "$FILENAME" "$MSG"
}

unsupported_prefix_target_conf()
{
if [ "${WARM_PREFIX_TARGET}" == "0" ] && [ "${WARM_IP_TARGET}" == "0" ] && [ "${MINIMUM_IP_TARGET}" == "0" ];then
true
else
false
fi
}

is_prefix_delegation_enabled()
{
if [ "${ENABLE_PREFIX_DELEGATION}" == "true" ]; then
true
else
false
fi
}

validate_env_var()
{
log_in_json info "Validating env variables ..."
if [[ "${AWS_VPC_K8S_PLUGIN_LOG_FILE,,}" == "stdout" ]]; then
log_in_json error "AWS_VPC_K8S_PLUGIN_LOG_FILE cannot be set to stdout"
exit 1
fi

if is_prefix_delegation_enabled && unsupported_prefix_target_conf ; then
log_in_json error "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"
exit 1
fi
}

# Check for all the required binaries before we go forward
Expand All @@ -63,6 +86,11 @@ AWS_VPC_K8S_PLUGIN_LOG_FILE=${AWS_VPC_K8S_PLUGIN_LOG_FILE:-"/var/log/aws-routed-
AWS_VPC_K8S_PLUGIN_LOG_LEVEL=${AWS_VPC_K8S_PLUGIN_LOG_LEVEL:-"Debug"}

AWS_VPC_K8S_CNI_CONFIGURE_RPFILTER=${AWS_VPC_K8S_CNI_CONFIGURE_RPFILTER:-"true"}
ENABLE_PREFIX_DELEGATION=${ENABLE_PREFIX_DELEGATION:-"false"}
WARM_IP_TARGET=${WARM_IP_TARGET:-"0"}
MINIMUM_IP_TARGET=${MINIMUM_IP_TARGET:-"0"}
WARM_PREFIX_TARGET=${WARM_PREFIX_TARGET:-"0"}


validate_env_var

Expand Down