Added kubeReserved calculation support for mixed instance NodeGroups

[elementalvoid]

Description

Support for calculating CPU, RAM, and Storage reservations via
kubeReserved was added to eksctl in v0.16. However, it only supported
NodeGroups with a single instance type. ASGs can have multiple
instance types and were not included in the calculation.

This change enables calculation of kubeReserved for mixed instances too.
Since multiple instance types are supported it calculates the reservation
based on the smallest instance type in the distribution.

Additional kubeReserved tests covering unknown innstance types. The new tests
ensure that unknown instance types are properly ignored during calculation
of kubeReserved.

Checklist

• Added tests that cover your change (if possible)
• Added/modified documentation as required (such as the README.md, or the userdocs directory)
• Manually tested
• Added labels for change area (e.g. area/nodegroup), target version (e.g. version/0.12.0) and kind (e.g. kind/improvement)
• Make sure the title of the PR is a good description that can go into the release notes

[martina-if]

Hi @elementalvoid thank you for this PR. The code looks good in general and I only had one minor comment. There is a change that we need to make though and it's regarding the mentioning of fleets. Please revise the PR and make sure there is no mention of it since eksctl uses ASGs instead.

[martina-if]

Perhaps these can be extracted into a func setKubereserved(*obj, *info) to avoid duplicating these in lines 138-143

[elementalvoid]

👍 I created the new function but the signature is different than suggested. Rather than passing in the kubeReserved document and mutating it, I pass only the info and return the required document. Let me know if this works!

[elementalvoid]

Thanks @martina-if! I squashed my commits together to keep things clean and to reword the original message removing Fleet mentions there too.

Separately, GitHub status is warning about being out of date from the base branch. Do you want me to rebase or leave it as is?

[martina-if]

Hi @elementalvoid thanks for the changes. They look good except one minor thing on the docs side. Yes, please rebase the changes on top of the latest master.

Also, can you post here proof of manual testing of this feature? That helps us merge it faster and keeps instructions on how to test new features.

[elementalvoid]

I've rebased onto the current master branch and found/replaced the remaining Fleet mention in the docs.

Here's the manual test run:

The test NodeGroup config

This is my minimized config -- I removed all the extraneous config specific to my test cluster.

nodeGroups:
  - name: worker-eksctl-kubereserved-test
    minSize: 1
    instancesDistribution:
      instanceTypes:
        - t2.large
        - t3.large
        - t3a.large
        - m4.large
        - m5.large
        - m5ad.large
        - m5d.large
        - m5n.large

Create the NodeGroup

❯ eksctl create nodegroup --config-file clusters/staging/us-east-1/c10/eks_cluster.yaml
[ℹ]  eksctl version 0.24.0-dev
[ℹ]  using region us-east-1
[ℹ]  4 existing nodegroup(s) (...REDACTED...) will be excluded
[ℹ]  nodegroup "worker-eksctl-kubereserved-test" will use "ami-0d960646974cf9e5b" [AmazonLinux2/1.16]
[ℹ]  using EC2 key pair "...REDACTED..."
[ℹ]  1 nodegroup (worker-eksctl-kubereserved-test) was included (based on the include/exclude rules)
[ℹ]  combined exclude rules: ...REDACTED...
[ℹ]  4 nodegroups (...REDACTED...) were excluded (based on the include/exclude rules)
[ℹ]  will create a CloudFormation stack for each of 1 nodegroups in cluster "c10"
[ℹ]  2 sequential tasks: { fix cluster compatibility, 1 task: { 1 task: { create nodegroup "worker-eksctl-kubereserved-test" } } }
[ℹ]  checking cluster stack for missing resources
[ℹ]  cluster stack has all required resources
[ℹ]  building nodegroup stack "eksctl-c10-nodegroup-worker-eksctl-kubereserved-test"
[ℹ]  deploying stack "eksctl-c10-nodegroup-worker-eksctl-kubereserved-test"
[ℹ]  no tasks
[ℹ]  adding identity "arn:aws:iam::REDACTED:role/eksctl-c10-nodegroup-worker-NodeInstanceRole-REDACTED" to auth ConfigMap
[✔]  created 1 nodegroup(s) in cluster "c10"
[✔]  created 0 managed nodegroup(s) in cluster "c10"
[ℹ]  checking security group configuration for all nodegroups
[ℹ]  all nodegroups have up-to-date configuration

Check that we have an instance

❯ kubectl get node -l 'alpha.eksctl.io/nodegroup-name=worker-eksctl-kubereserved-test' --label-columns 'beta.kubernetes.io/instance-type'
NAME                             STATUS   ROLES    AGE   VERSION              INSTANCE-TYPE
ip-10-192-234-205.ec2.internal   Ready    <none>   68s   v1.16.8-eks-fd1ea7   t3a.large

Find and validate the kubeReserved

❯ kubectl proxy --port=8001 &

❯ export NODE='ip-10-192-234-205.ec2.internal'; curl -sSL "http://localhost:8001/api/v1/nodes/${NODE}/proxy/configz" | jq '.kubeletconfig.kubeReserved'
{
  "cpu": "70m",
  "ephemeral-storage": "1Gi",
  "memory": "1843Mi"
}

Since the t2/t3 are the smallest instances in the NodeGroup at 8Gb of RAM and 2 CPU this is the expected reservation.

[whereisaaron]

Hi @elementalvoid @martina-if looking at this PR and the website documentation, I am confused about the behavior of kubeReserved. Originally nodes were being deployed by eksctl with no kubelet reservations at all. Then kubeletExtraConfig was introduced and we could specify reservations.

However this PR makes it sound like eksctl will now calculate a sensible default reservation? How do you trigger that behavior? Just don't specify kubeletExtraConfig any more? If there is a sensible default now, perhaps the documentation could mention this, as the current wording still I think implies no reservation occurs if you don't configure it manually with kubeletExtraConfig.

[elementalvoid]

Hi @whereisaaron! @martina-if linked a related issue above -- an ask to improve documentation on this feature.

Automatic kubeReserved calculation was introduced in eksctl 0.17. The implementation has attempted to track that of the bootstrap.sh script embedded in the EKS AMI.

So long as you do not specify the kubeReserved setting then eksctl will automatically calculate the setting based on the instance type(s) in use.

What my PR did was expand that automatic calculation to include ASGs with multiple instance types as the original implementation only calculated them for single instance type ASGs.

I hope that helps.

[martina-if]

Thanks @elementalvoid for the reply, and thanks @whereisaaron for the feedback, this is very useful since it helps us assign the right priority to things. I've raised the priority of #2091. Hopefully we will be able to include it in next week's planning.