feat: calculate the correct number of pods for custom ENIConfig

[js-timbirkett]

Issues:
Many issues over a few different projects related to:
#375

See also:
aws/amazon-vpc-cni-k8s#331
kubernetes/autoscaler#1366
awsdocs/amazon-eks-user-guide#72

Description of changes:
This moves from using a pre-calculated file listing instance types and max-pod values, to a file containing the ENI data for each instance type (max ENIs, max IPs per ENI) and calculates MAX_PODS from those values.

It adds an option to bootstrap.sh: --using-custom-eniconfig or env variable: USING_CUSTOM_ENICONFIG which will cause the correct MAX_PODS value to be calculated.

I'm not overly precious about the naming, or the way it's been implemented so feedback, discussion, or suggestions are most welcome 😸 - I'm just trying to solve a problem that has been biting us EKS AMI users for a while without overwriting files in user-data or maintaining a separate map of values.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[js-timbirkett]

The ENI data file was generated with:

import requests
import json

from bs4 import BeautifulSoup

table_id = 'w297aac21c13c15b5'

response = requests.get("https://docs.aws.amazon.com/AWSEC2/latest"
                        "/UserGuide/using-eni.html#AvailableIpPerENI")

parsed_html = BeautifulSoup(response.text, features="html.parser")

table = parsed_html.find('table', attrs={'id': table_id})

rows = table.find_all("tr")

for row in rows:
    cells = row.find_all("td")
    if len(cells) < 1:
        continue
    print('{} {} {}'.format(
        cells[0].text.strip(),
        cells[1].text.strip(),
        cells[2].text.strip()))

Which is sub-optimal... I'm certain that this could be retrieved from AWS APIs.

[galindro]

@js-timbirkett thank you very much for your great job! Could you please solve the conflicts?

[mogren]

This is related to a change I recently did in the AWS CNI: aws/amazon-vpc-cni-k8s/pull/1035/

I like this approach, and the generator code can definitely output ENIs and IPs per ENI instead of the max-pods value.

[js-timbirkett]

@galindro @mogren - Thanks I'll take a look at sorting this out soon :)

@mogren - is the generator code in: https://github.com/aws/amazon-vpc-cni-k8s/pull/1035/files used by anything else or just to produce the file in this project?

[mogren]

@js-timbirkett You are correct that eni-max-pods.txt is only used here. This file used to be updated manually, once EC2 added the DescribeInstanceTypes, we started generating the limits.

It might be worth generating a new file, like instance-eni-and-ip.txt or something like that.

[CharlieSu]

Any update on this issue? It would be great to have official support for custom ENIConfigs.

[enderv]

Just wanted to ping as well any update on this? This would be incredibly useful for a project I'm working on. Wouldn't mind helping out if needed

[crsantini]

Hey guys, thanks for putting this together. Any updates when the AMI will be released to be used with EKS? Anytime this year yet?

In addition, will the parameter "--using-custom-eniconfig" be available to be set for eks managed node groups?

[technotaff-nbs]

Any reason why this cannot be merged - it looks like a no-brainer from my analysis?

We are unable to move forward with Custom VPC CNI networking without this merge.

[ssanders1449]

As a workaround, I added the following to the User Data startup script in the launch configuration before calling /etc/eks/bootstrap.sh (see https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/using-eni.html#AvailableIpPerENI for details)

INSTANCE_TYPE=$(curl --silent http://169.254.169.254/latest/meta-data/instance-type)
REGION=$(curl -s http://169.254.169.254/latest/dynamic/instance-identity/document | jq .region -r)
NICS_IPS=$(aws ec2 describe-instance-types --region $REGION --filters Name=instance-type,Values=$INSTANCE_TYPE --query "InstanceTypes[].{MaxENI: NetworkInfo.MaximumNetworkInterfaces, IPv4addr: NetworkInfo.Ipv4AddressesPerInterface}" --output text)
if [ -n "$NICS_IPS" ]
then
MAX_PODS=$(echo $NICS_IPS|awk '{print ($2 - 1) * ($1 - 1) + 2}')
SKIP_MAX_PODS="--use-max-pods false"
MAX_PODS_KUBELET_EXTRA_ARGS="--max-pods=$MAX_PODS"
fi
/etc/eks/bootstrap.sh ... ${SKIP_MAX_PODS} --kubelet-extra-args "${MAX_PODS_KUBELET_EXTRA_ARGS}"

[technotaff-nbs]

Please please please merge this fix, all the heavy-lifting has been done for you.

[technotaff-nbs]

This looks good to me.

[splichy]

BTW why not use: https://github.com/awslabs/amazon-eks-ami/blob/master/files/max-pods-calculator.sh

[adammw]

@splichy is there any documentation on how max-pods-calculator.sh is supposed to work? i can't see anywhere where that would get called...

[vl-shopback]

why not merge?

[bryantbiggs]

if you want to call the max-pods-calculator.sh on nodes using the EKS optimized AMI you can do this within the user data:

MAX_PODS=$(/etc/eks/max-pods-calculator.sh \
			--instance-type-from-imds \
			--cni-version 1.11 \
			--cni-custom-networking-enabled \
			--cni-prefix-delegation-enabled \
)

Tailor the flags to suit your needs

[fawadkhaliq]

@M00nF1sh @jayanthvn any chance do you guys know this issue was resolved as part of another PR?

[jayanthvn]

@M00nF1sh @jayanthvn any chance do you guys know this issue was resolved as part of another PR?

Hi @fawadkhaliq - This script is added to compute the correct max pods with custom networking enabled. For managed node groups, it’s going to be handled automatically as long as you upgrade to CNI 1.9.X. MNG will run execute a version of this formula and set max pods https://github.com/awslabs/amazon-eks-ami/blob/master/files/max-pods-calculator.sh. For self managed the same script can be used and that will help generate the max pods - https://docs.amazonaws.cn/en_us/eks/latest/userguide/choosing-instance-type.html and https://docs.aws.amazon.com/eks/latest/userguide/cni-increase-ip-addresses.html. The script takes into account - CNI version, custom networking enabled/disabled, prefix delegation enabled/disabled, max ENI and so on. Please let me know if this is what you were looking for?

[fawadkhaliq]

@jayanthvn perfect, thanks! super helpful