fix(eks): default capacity uses desiredCapacity which is an anti-pattern (#5651)

Elad Ben-Israel · mergify[bot] · mergify[bot] · commit a883fed02a52 · 2020-01-06T14:49:29.000Z
* fix(eks): default capacity uses desiredCapacity which is an anti-pattern As described in #5215, `desiredCapacity` is not the recommended way to configure an auto scaling group since it will cause the ASG to reset the number of nodes in every CloudFormation deployment. Since EKS's default capacity uses `desiredCapacity` instead of `minCapacity`, as of #5507 this would emit a warning: "desiredCapacity has been configured. Be aware this will reset the size of your AutoScalingGroup on every deployment". This change modifies the behavior of the default capacity such that it will configure the ASG using `minCapacity` instead of `desiredCapacity` as recommended by ASG. Fixes #5650 * Update integ.eks-cluster.defaults.expected.json Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
diff --git a/packages/@aws-cdk/aws-eks/README.md b/packages/@aws-cdk/aws-eks/README.md
@@ -88,7 +88,7 @@ You can add customized capacity through `cluster.addCapacity()` or
 ```ts
 cluster.addCapacity('frontend-nodes', {
   instanceType: new ec2.InstanceType('t2.medium'),
-  desiredCapacity: 3,
+  minCapacity: 3,
   vpcSubnets: { subnetType: ec2.SubnetType.PUBLIC }
 });
 ```
@@ -125,7 +125,7 @@ you can use `kubeletExtraArgs` to add custom node labels or taints.
 // up to ten spot instances
 cluster.addCapacity('spot', {
   instanceType: new ec2.InstanceType('t3.large'),
-  desiredCapacity: 2,
+  minCapacity: 2,
   bootstrapOptions: {
     kubeletExtraArgs: '--node-labels foo=bar,goo=far',
     awsApiRetryAttempts: 5
diff --git a/packages/@aws-cdk/aws-eks/lib/cluster.ts b/packages/@aws-cdk/aws-eks/lib/cluster.ts
@@ -401,10 +401,10 @@ export class Cluster extends Resource implements ICluster {
     }
 
     // allocate default capacity if non-zero (or default).
-    const desiredCapacity = props.defaultCapacity === undefined ? DEFAULT_CAPACITY_COUNT : props.defaultCapacity;
-    if (desiredCapacity > 0) {
+    const minCapacity = props.defaultCapacity === undefined ? DEFAULT_CAPACITY_COUNT : props.defaultCapacity;
+    if (minCapacity > 0) {
       const instanceType = props.defaultCapacityInstance || DEFAULT_CAPACITY_TYPE;
-      this.defaultCapacity = this.addCapacity('DefaultCapacity', { instanceType, desiredCapacity });
+      this.defaultCapacity = this.addCapacity('DefaultCapacity', { instanceType, minCapacity });
     }
 
     const outputConfigCommand = props.outputConfigCommand === undefined ? true : props.outputConfigCommand;
diff --git a/packages/@aws-cdk/aws-eks/test/integ.eks-cluster.defaults.expected.json b/packages/@aws-cdk/aws-eks/test/integ.eks-cluster.defaults.expected.json
@@ -1039,8 +1039,7 @@
       "Type": "AWS::AutoScaling::AutoScalingGroup",
       "Properties": {
         "MaxSize": "2",
-        "MinSize": "1",
-        "DesiredCapacity": "2",
+        "MinSize": "2",
         "LaunchConfigurationName": {
           "Ref": "ClusterDefaultCapacityLaunchConfig72790CF7"
         },
@@ -1380,4 +1379,4 @@
       "Default": "/aws/service/eks/optimized-ami/1.14/amazon-linux-2/recommended/image_id"
     }
   }
-}
+}
diff --git a/packages/@aws-cdk/aws-eks/test/integ.eks-cluster.kubectl-disabled.expected.json b/packages/@aws-cdk/aws-eks/test/integ.eks-cluster.kubectl-disabled.expected.json
@@ -941,7 +941,6 @@
       "Properties": {
         "MaxSize": "1",
         "MinSize": "1",
-        "DesiredCapacity": "1",
         "LaunchConfigurationName": {
           "Ref": "EKSClusterNodesLaunchConfig921F1106"
         },
diff --git a/packages/@aws-cdk/aws-eks/test/integ.eks-cluster.kubectl-disabled.ts b/packages/@aws-cdk/aws-eks/test/integ.eks-cluster.kubectl-disabled.ts
@@ -18,7 +18,7 @@ class EksClusterStack extends TestStack {
 
     cluster.addCapacity('Nodes', {
       instanceType: new ec2.InstanceType('t2.medium'),
-      desiredCapacity: 1,  // Raise this number to add more nodes
+      minCapacity: 1,  // Raise this number to add more nodes
     });
     /// !hide
   }
diff --git a/packages/@aws-cdk/aws-eks/test/integ.eks-cluster.lit.expected.json b/packages/@aws-cdk/aws-eks/test/integ.eks-cluster.lit.expected.json
@@ -1040,7 +1040,6 @@
       "Properties": {
         "MaxSize": "1",
         "MinSize": "1",
-        "DesiredCapacity": "1",
         "LaunchConfigurationName": {
           "Ref": "EKSClusterNodesLaunchConfig921F1106"
         },
diff --git a/packages/@aws-cdk/aws-eks/test/integ.eks-cluster.lit.ts b/packages/@aws-cdk/aws-eks/test/integ.eks-cluster.lit.ts
@@ -17,7 +17,7 @@ class EksClusterStack extends TestStack {
 
     cluster.addCapacity('Nodes', {
       instanceType: new ec2.InstanceType('t2.medium'),
-      desiredCapacity: 1,  // Raise this number to add more nodes
+      minCapacity: 1,  // Raise this number to add more nodes
     });
     /// !hide
   }
diff --git a/packages/@aws-cdk/aws-eks/test/integ.eks-helm.lit.expected.json b/packages/@aws-cdk/aws-eks/test/integ.eks-helm.lit.expected.json
@@ -948,8 +948,7 @@
         "Type": "AWS::AutoScaling::AutoScalingGroup",
         "Properties": {
           "MaxSize": "3",
-          "MinSize": "1",
-          "DesiredCapacity": "3",
+          "MinSize": "3",
           "LaunchConfigurationName": {
             "Ref": "cluster22NodesLaunchConfig184BF3BA"
           },
diff --git a/packages/@aws-cdk/aws-eks/test/integ.eks-helm.lit.ts b/packages/@aws-cdk/aws-eks/test/integ.eks-helm.lit.ts
@@ -38,7 +38,7 @@ class ClusterStack extends TestStack {
     // automatically be mapped via aws-auth to allow nodes to join the cluster.
     this.cluster.addCapacity('Nodes', {
       instanceType: new ec2.InstanceType('t2.medium'),
-      desiredCapacity: 3,
+      minCapacity: 3,
     });
 
     // add two Helm charts to the cluster. This will be the Kubernetes dashboard and the Nginx Ingress Controller
diff --git a/packages/@aws-cdk/aws-eks/test/integ.eks-kubectl.lit.expected.json b/packages/@aws-cdk/aws-eks/test/integ.eks-kubectl.lit.expected.json
@@ -948,8 +948,7 @@
         "Type": "AWS::AutoScaling::AutoScalingGroup",
         "Properties": {
           "MaxSize": "3",
-          "MinSize": "1",
-          "DesiredCapacity": "3",
+          "MinSize": "3",
           "LaunchConfigurationName": {
             "Ref": "cluster22NodesLaunchConfig184BF3BA"
           },
diff --git a/packages/@aws-cdk/aws-eks/test/integ.eks-kubectl.lit.ts b/packages/@aws-cdk/aws-eks/test/integ.eks-kubectl.lit.ts
@@ -38,7 +38,7 @@ class ClusterStack extends TestStack {
     // automatically be mapped via aws-auth to allow nodes to join the cluster.
     this.cluster.addCapacity('Nodes', {
       instanceType: new ec2.InstanceType('t2.medium'),
-      desiredCapacity: 3,
+      minCapacity: 3,
     });
 
     // add an arbitrary k8s manifest to the cluster. This will `kubectl apply`
diff --git a/packages/@aws-cdk/aws-eks/test/integ.eks-spot.expected.json b/packages/@aws-cdk/aws-eks/test/integ.eks-spot.expected.json
@@ -871,8 +871,7 @@
       "Type": "AWS::AutoScaling::AutoScalingGroup",
       "Properties": {
         "MaxSize": "2",
-        "MinSize": "1",
-        "DesiredCapacity": "2",
+        "MinSize": "2",
         "LaunchConfigurationName": {
           "Ref": "myClusterDefaultCapacityLaunchConfigCF6D4B81"
         },
diff --git a/packages/@aws-cdk/aws-eks/test/test.cluster.ts b/packages/@aws-cdk/aws-eks/test/test.cluster.ts
@@ -55,7 +55,7 @@ export = {
 
       // THEN
       test.ok(cluster.defaultCapacity);
-      expect(stack).to(haveResource('AWS::AutoScaling::AutoScalingGroup', { DesiredCapacity: '2' }));
+      expect(stack).to(haveResource('AWS::AutoScaling::AutoScalingGroup', { MinSize: '2', MaxSize: '2' }));
       expect(stack).to(haveResource('AWS::AutoScaling::LaunchConfiguration', { InstanceType: 'm5.large' }));
       test.done();
     },
@@ -72,7 +72,7 @@ export = {
 
       // THEN
       test.ok(cluster.defaultCapacity);
-      expect(stack).to(haveResource('AWS::AutoScaling::AutoScalingGroup', { DesiredCapacity: '10' }));
+      expect(stack).to(haveResource('AWS::AutoScaling::AutoScalingGroup', { MinSize: '10', MaxSize: '10' }));
       expect(stack).to(haveResource('AWS::AutoScaling::LaunchConfiguration', { InstanceType: 'm2.xlarge' }));
       test.done();
     },

Original file line number	Diff line number	Diff line change
`@@ -401,10 +401,10 @@ export class Cluster extends Resource implements ICluster {`
`401`	`401`	`}`
`402`	`402`
`403`	`403`	`// allocate default capacity if non-zero (or default).`
`404`		`- const desiredCapacity = props.defaultCapacity === undefined ? DEFAULT_CAPACITY_COUNT : props.defaultCapacity;`
`405`		`- if (desiredCapacity > 0) {`
	`404`	`+ const minCapacity = props.defaultCapacity === undefined ? DEFAULT_CAPACITY_COUNT : props.defaultCapacity;`
	`405`	`+ if (minCapacity > 0) {`
`406`	`406`	`const instanceType = props.defaultCapacityInstance \|\| DEFAULT_CAPACITY_TYPE;`
`407`		`- this.defaultCapacity = this.addCapacity('DefaultCapacity', { instanceType, desiredCapacity });`
	`407`	`+ this.defaultCapacity = this.addCapacity('DefaultCapacity', { instanceType, minCapacity });`
`408`	`408`	`}`
`409`	`409`
`410`	`410`	`const outputConfigCommand = props.outputConfigCommand === undefined ? true : props.outputConfigCommand;`
Original file line number	Diff line number	Diff line change
`@@ -18,7 +18,7 @@ class EksClusterStack extends TestStack {`
`18`	`18`
`19`	`19`	`cluster.addCapacity('Nodes', {`
`20`	`20`	`instanceType: new ec2.InstanceType('t2.medium'),`
`21`		`- desiredCapacity: 1, // Raise this number to add more nodes`
	`21`	`+ minCapacity: 1, // Raise this number to add more nodes`
`22`	`22`	`});`
`23`	`23`	`/// !hide`
`24`	`24`	`}`
Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,7 @@ class EksClusterStack extends TestStack {`
`17`	`17`
`18`	`18`	`cluster.addCapacity('Nodes', {`
`19`	`19`	`instanceType: new ec2.InstanceType('t2.medium'),`
`20`		`- desiredCapacity: 1, // Raise this number to add more nodes`
	`20`	`+ minCapacity: 1, // Raise this number to add more nodes`
`21`	`21`	`});`
`22`	`22`	`/// !hide`
`23`	`23`	`}`