Repository: keikoproj/instance-manager
Pull request #244 (2021-02-10 19:50:04)

feature: add support for calculating custom networking max pods
This PR adds support for automatic calculation of the max pods bootstrap argument. This is useful for EKS clusters where custom networking is enabled, because the default maxPods value is not correct. See https://docs.aws.amazon.com/eks/latest/userguide/cni-custom-network.html for more information about this limitation. 


Signed-off-by: Jonah Back <jonah@jonahback.com>

codecov[bot]: # [Codecov](https://codecov.io/gh/keikoproj/instance-manager/pull/244?src=pr&el=h1) Report
> Merging [#244](https://codecov.io/gh/keikoproj/instance-manager/pull/244?src=pr&el=desc) (4658966) into [master](https://codecov.io/gh/keikoproj/instance-manager/commit/c1420c9593d875502b729c71d55dda9e39c3ef88?el=desc) (c1420c9) will **decrease** coverage by `0.06%`.
> The diff coverage is `85.71%`.

[![Impacted file tree graph](https://codecov.io/gh/keikoproj/instance-manager/pull/244/graphs/tree.svg?width=650&height=150&src=pr&token=IJbjmSBliL)](https://codecov.io/gh/keikoproj/instance-manager/pull/244?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #244      +/-   ##
==========================================
- Coverage   88.00%   87.94%   -0.07%     
==========================================
  Files          17       17              
  Lines        2210     2231      +21     
==========================================
+ Hits         1945     1962      +17     
- Misses        161      163       +2     
- Partials      104      106       +2     
```


| [Impacted Files](https://codecov.io/gh/keikoproj/instance-manager/pull/244?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [controllers/provisioners/eks/eks.go](https://codecov.io/gh/keikoproj/instance-manager/pull/244/diff?src=pr&el=tree#diff-Y29udHJvbGxlcnMvcHJvdmlzaW9uZXJzL2Vrcy9la3MuZ28=) | `84.21% <ø> (ø)` | |
| [controllers/provisioners/eks/helpers.go](https://codecov.io/gh/keikoproj/instance-manager/pull/244/diff?src=pr&el=tree#diff-Y29udHJvbGxlcnMvcHJvdmlzaW9uZXJzL2Vrcy9oZWxwZXJzLmdv) | `90.51% <85.71%> (-0.34%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/keikoproj/instance-manager/pull/244?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/keikoproj/instance-manager/pull/244?src=pr&el=footer). Last update [c1420c9...4658966](https://codecov.io/gh/keikoproj/instance-manager/pull/244?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/controllers/providers/aws/aws.go b/controllers/providers/aws/aws.go
index abb48e3..ebdbc2f 100644
--- a/controllers/providers/aws/aws.go
+++ b/controllers/providers/aws/aws.go
@@ -1323,3 +1323,12 @@ func GetScalingConfigName(group *autoscaling.Group) string {
 	}
 	return configName
 }
+
+func GetInstanceTypeNetworkInfo(instanceTypes []*ec2.InstanceTypeInfo, instanceType string) *ec2.NetworkInfo {
+	for _, instanceTypeInfo := range instanceTypes {
+		if aws.StringValue(instanceTypeInfo.InstanceType) == instanceType {
+			return instanceTypeInfo.NetworkInfo
+		}
+	}
+	return nil
+}
diff --git a/controllers/provisioners/eks/cloud.go b/controllers/provisioners/eks/cloud.go
index 1c87a89..f14b469 100644
--- a/controllers/provisioners/eks/cloud.go
+++ b/controllers/provisioners/eks/cloud.go
@@ -54,6 +54,7 @@ type DiscoveredState struct {
 	Cluster              *eks.Cluster
 	VPCId                string
 	InstancePool         InstancePoolSpec
+	InstanceTypeInfo     []*ec2.InstanceTypeInfo
 }
 
 func (ctx *EksInstanceGroupContext) CloudDiscovery() error {
@@ -205,6 +206,12 @@ func (ctx *EksInstanceGroupContext) CloudDiscovery() error {
 	status.SetCurrentMin(int(aws.Int64Value(targetScalingGroup.MinSize)))
 	status.SetCurrentMax(int(aws.Int64Value(targetScalingGroup.MaxSize)))
 
+	instanceTypes, err := ctx.AwsWorker.DescribeInstanceTypes()
+	if err != nil {
+		return errors.Wrap(err, "failed to discover instance types")
+	}
+	state.SetInstanceTypeInfo(instanceTypes)
+
 	if spec.IsLaunchConfiguration() {
 
 		state.ScalingConfiguration, err = scaling.NewLaunchConfiguration(instanceGroup.NamespacedName(), ctx.AwsWorker, &scaling.DiscoverConfigurationInput{
@@ -230,11 +237,6 @@ func (ctx *EksInstanceGroupContext) CloudDiscovery() error {
 		if err != nil {
 			return errors.Wrap(err, "failed to discover launch templates")
 		}
-		instanceTypes, err := ctx.AwsWorker.DescribeInstanceTypes()
-		if err != nil {
-			return errors.Wrap(err, "failed to discover launch templates")
-		}
-
 		var (
 			pool             = subFamilyFlexiblePool(offerings, instanceTypes)
 			resource         = state.ScalingConfiguration.Resource()
@@ -352,6 +354,19 @@ func (d *DiscoveredState) GetInstanceProfile() *iam.InstanceProfile {
 	}
 	return &iam.InstanceProfile{}
 }
+
+func (d *DiscoveredState) SetInstanceTypeInfo(instanceTypeInfo []*ec2.InstanceTypeInfo) {
+	if instanceTypeInfo != nil {
+		d.InstanceTypeInfo = instanceTypeInfo
+	}
+}
+func (d *DiscoveredState) GetInstanceTypeInfo() []*ec2.InstanceTypeInfo {
+	if d.InstanceTypeInfo != nil {
+		return d.InstanceTypeInfo
+	}
+	return []*ec2.InstanceTypeInfo{}
+}
+
 func (d *DiscoveredState) GetRole() *iam.Role {
 	if d.IAMRole != nil {
 		return d.IAMRole
diff --git a/controllers/provisioners/eks/eks.go b/controllers/provisioners/eks/eks.go
index 3eb181d..233f66b 100644
--- a/controllers/provisioners/eks/eks.go
+++ b/controllers/provisioners/eks/eks.go
@@ -35,6 +35,8 @@ const (
 	OverrideDefaultLabelsAnnotationKey  = "instancemgr.keikoproj.io/default-labels"
 	OsFamilyAnnotation                  = "instancemgr.keikoproj.io/os-family"
 	ClusterAutoscalerEnabledAnnotation  = "instancemgr.keikoproj.io/cluster-autoscaler-enabled"
+	CustomNetworkingEnabledAnnotation   = "instancemgr.keikoproj.io/custom-networking-enabled"
+	CustomNetworkingHostPodsAnnotation  = "instancemgr.keikoproj.io/custom-networking-host-pods"
 
 	OsFamilyWindows      = "windows"
 	OsFamilyBottleRocket = "bottlerocket"
diff --git a/controllers/provisioners/eks/helpers.go b/controllers/provisioners/eks/helpers.go
index 3af3ad7..631fbf1 100644
--- a/controllers/provisioners/eks/helpers.go
+++ b/controllers/provisioners/eks/helpers.go
@@ -108,9 +108,10 @@ func (ctx *EksInstanceGroupContext) GetBasicUserData(clusterName, args string, k
 		osFamily         = ctx.GetOsFamily()
 		nodeLabels       = ctx.GetComputedLabels()
 		nodeTaints       = configuration.GetTaints()
-		bootstrapOptions = configuration.GetBootstrapOptions()
+		bootstrapOptions = ctx.GetComputedBootstrapOptions()
 	)
 	var maxPods int64 = 0
+
 	if bootstrapOptions != nil {
 		maxPods = bootstrapOptions.MaxPods
 	}
@@ -457,14 +458,43 @@ func (ctx *EksInstanceGroupContext) GetLabelList() []string {
 	return labelList
 }
 
-func (ctx *EksInstanceGroupContext) GetBootstrapArgs() string {
+func (ctx *EksInstanceGroupContext) GetComputedBootstrapOptions() *v1alpha1.BootstrapOptions {
 	var (
 		instanceGroup = ctx.GetInstanceGroup()
+		state         = ctx.GetDiscoveredState()
 		configuration = instanceGroup.GetEKSConfiguration()
 	)
+	if instanceGroup.GetAnnotations()[CustomNetworkingEnabledAnnotation] == "true" {
+		hostNetworkPods, err := strconv.ParseInt(instanceGroup.GetAnnotations()[CustomNetworkingHostPodsAnnotation], 10, 64)
+		if err != nil {
+			hostNetworkPods = 2 //Default on EKS. Kube-Proxy and AWS VPC CNI
+		}
+
+		instanceTypeNetworkInfo := awsprovider.GetInstanceTypeNetworkInfo(state.GetInstanceTypeInfo(), configuration.InstanceType)
+		maxPods := (*instanceTypeNetworkInfo.MaximumNetworkInterfaces-1)*
+			(*instanceTypeNetworkInfo.Ipv4AddressesPerInterface-1) + hostNetworkPods
+		if configuration.BootstrapOptions == nil {
+			return &v1alpha1.BootstrapOptions{
+				MaxPods: maxPods,
+			}
+		} else {
+			bootstrapOptions := *configuration.BootstrapOptions
+			if bootstrapOptions.MaxPods == 0 {
+				bootstrapOptions.MaxPods = maxPods
+			}
+			return &bootstrapOptions
+		}
+	}
+	return configuration.BootstrapOptions
+}
+
+func (ctx *EksInstanceGroupContext) GetBootstrapArgs() string {
+	var (
+		bootstrapOptions = ctx.GetComputedBootstrapOptions()
+	)
 	var sb strings.Builder
 
-	if configuration.BootstrapOptions != nil && configuration.BootstrapOptions.MaxPods > 0 {
+	if bootstrapOptions != nil && bootstrapOptions.MaxPods > 0 {
 		sb.WriteString("--use-max-pods false ")
 	}
 	sb.WriteString(fmt.Sprintf("--kubelet-extra-args '%v' ", ctx.GetKubeletExtraArgs()))
@@ -473,17 +503,18 @@ func (ctx *EksInstanceGroupContext) GetBootstrapArgs() string {
 
 func (ctx *EksInstanceGroupContext) GetKubeletExtraArgs() string {
 	var (
-		instanceGroup = ctx.GetInstanceGroup()
-		configuration = instanceGroup.GetEKSConfiguration()
-		bootstrapArgs = configuration.GetBootstrapArguments()
+		instanceGroup    = ctx.GetInstanceGroup()
+		configuration    = instanceGroup.GetEKSConfiguration()
+		bootstrapArgs    = configuration.GetBootstrapArguments()
+		bootstrapOptions = ctx.GetComputedBootstrapOptions()
 	)
 
 	labelsFlag := fmt.Sprintf("--node-labels=%v", strings.Join(ctx.GetLabelList(), ","))
 	taintsFlag := fmt.Sprintf("--register-with-taints=%v", strings.Join(ctx.GetTaintList(), ","))
 	var sb strings.Builder
 	sb.WriteString(fmt.Sprintf("%v %v %v ", labelsFlag, taintsFlag, bootstrapArgs))
-	if configuration.BootstrapOptions != nil && configuration.BootstrapOptions.MaxPods > 0 {
-		sb.WriteString(fmt.Sprintf("--max-pods=%v ", configuration.BootstrapOptions.MaxPods))
+	if bootstrapOptions != nil && bootstrapOptions.MaxPods > 0 {
+		sb.WriteString(fmt.Sprintf("--max-pods=%v ", bootstrapOptions.MaxPods))
 	}
 	return sb.String()
 }
diff --git a/docs/EKS.md b/docs/EKS.md
index f4dad42..152a99e 100644
--- a/docs/EKS.md
+++ b/docs/EKS.md
@@ -547,3 +547,5 @@ Individual namespaces can opt-out by adding the annotation `instancemgr.keikopro
 |instancemgr.keikoproj.io/cluster-autoscaler-enabled|InstanceGroup|"true"|setting this annotation to true will add the relevant cluster-autoscaler EC2 tags according to cluster name, taints, and labels|
 |instancemgr.keikoproj.io/os-family|InstanceGroup|either "windows", "bottlerocket", or "amazonlinux2" (default)|this is required if you are running a windows or bottlerocket based AMI, by default the controller will try to bootstrap an amazonlinux2 AMI|
 |instancemgr.keikoproj.io/default-labels|InstanceGroup|comma-seprarated key-value string e.g. "label1=value1,label2=value2"|allows overriding the default node labels added by the controller, by default the role label is added depending on the cluster version|
+|instancemgr.keikoproj.io/custom-networking-enabled|InstanceGroup|"true"|setting this annotation to true will automatically calculate the correct setting for max pods and pass it to the kubelet|
+|instancemgr.keikoproj.io/custom-networking-host-pods|InstanceGroup|"2"|setting this annotation increases the number of max pods on nodes with custom networking, due to the fact that hostNetwork pods do not use an additional IP address |


Test patch:
diff --git a/controllers/provisioners/eks/eks_test.go b/controllers/provisioners/eks/eks_test.go
index f3c46e8..c6ef5fb 100644
--- a/controllers/provisioners/eks/eks_test.go
+++ b/controllers/provisioners/eks/eks_test.go
@@ -134,6 +134,7 @@ func MockInstanceGroup() *v1alpha1.InstanceGroup {
 				MinSize: 1,
 				EKSConfiguration: &v1alpha1.EKSConfiguration{
 					EksClusterName: "my-cluster",
+					InstanceType:   "m5.large",
 					SuspendedProcesses: []string{
 						"AZRebalance",
 					},
diff --git a/controllers/provisioners/eks/helpers_test.go b/controllers/provisioners/eks/helpers_test.go
index 14c74c6..c98c279 100644
--- a/controllers/provisioners/eks/helpers_test.go
+++ b/controllers/provisioners/eks/helpers_test.go
@@ -81,6 +81,97 @@ func TestAutoscalerTags(t *testing.T) {
 
 }
 
+func TestCustomNetworkingMaxPods(t *testing.T) {
+	var (
+		k       = MockKubernetesClientSet()
+		ig      = MockInstanceGroup()
+		asgMock = NewAutoScalingMocker()
+		iamMock = NewIamMocker()
+		eksMock = NewEksMocker()
+		ec2Mock = NewEc2Mocker()
+	)
+
+	w := MockAwsWorker(asgMock, iamMock, eksMock, ec2Mock)
+
+	ig.Annotations = map[string]string{
+		ClusterAutoscalerEnabledAnnotation: "true",
+		CustomNetworkingHostPodsAnnotation: "2",
+		CustomNetworkingEnabledAnnotation:  "true",
+	}
+
+	ctx := MockContext(ig, k, w)
+	ctx.GetDiscoveredState().SetInstanceTypeInfo([]*ec2.InstanceTypeInfo{
+		&ec2.InstanceTypeInfo{
+			InstanceType: aws.String("m5.large"),
+			NetworkInfo: &ec2.NetworkInfo{
+				MaximumNetworkInterfaces:  aws.Int64(3),
+				Ipv4AddressesPerInterface: aws.Int64(10),
+			},
+		},
+	})
+
+	userData := ctx.GetBasicUserData("foo", ctx.GetBootstrapArgs(), "", UserDataPayload{}, []MountOpts{})
+	basicUserDataDecoded, _ := base64.StdEncoding.DecodeString(userData)
+	basicUserDataString := string(basicUserDataDecoded)
+	if !strings.Contains(basicUserDataString, "--max-pods=20") {
+		t.Fail()
+	}
+
+	tests := []struct {
+		bootstrapOptions *v1alpha1.BootstrapOptions
+		annotations      map[string]string
+		expectedMaxPods  string
+	}{
+		{
+			annotations: map[string]string{
+				ClusterAutoscalerEnabledAnnotation: "true",
+				CustomNetworkingHostPodsAnnotation: "2",
+				CustomNetworkingEnabledAnnotation:  "true",
+			},
+			bootstrapOptions: &v1alpha1.BootstrapOptions{MaxPods: 22},
+			expectedMaxPods:  "--max-pods=22",
+		},
+		{
+			annotations: map[string]string{
+				ClusterAutoscalerEnabledAnnotation: "true",
+				CustomNetworkingHostPodsAnnotation: "2",
+				CustomNetworkingEnabledAnnotation:  "true",
+			},
+			bootstrapOptions: &v1alpha1.BootstrapOptions{MaxPods: 0},
+			expectedMaxPods:  "--max-pods=20",
+		},
+		{
+			annotations: map[string]string{
+				ClusterAutoscalerEnabledAnnotation: "true",
+				CustomNetworkingHostPodsAnnotation: "2",
+				CustomNetworkingEnabledAnnotation:  "true",
+			},
+			bootstrapOptions: nil,
+			expectedMaxPods:  "--max-pods=20",
+		},
+		{
+			annotations: map[string]string{
+				ClusterAutoscalerEnabledAnnotation: "true",
+				CustomNetworkingHostPodsAnnotation: "",
+				CustomNetworkingEnabledAnnotation:  "true",
+			},
+			bootstrapOptions: nil,
+			expectedMaxPods:  "--max-pods=20",
+		},
+	}
+
+	for _, tc := range tests {
+		ctx.InstanceGroup.GetEKSConfiguration().BootstrapOptions = tc.bootstrapOptions
+		ctx.InstanceGroup.Annotations = tc.annotations
+		userData = ctx.GetBasicUserData("foo", ctx.GetBootstrapArgs(), "", UserDataPayload{}, []MountOpts{})
+		basicUserDataDecoded, _ = base64.StdEncoding.DecodeString(userData)
+		basicUserDataString = string(basicUserDataDecoded)
+		if !strings.Contains(basicUserDataString, tc.expectedMaxPods) {
+			t.Fail()
+		}
+	}
+}
+
 func TestResolveSecurityGroups(t *testing.T) {
 	var (
 		g       = gomega.NewGomegaWithT(t)