From a883fed02a520068221c91ea3755cf63dd493f4e Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Mon, 6 Jan 2020 16:49:29 +0200 Subject: [PATCH] fix(eks): default capacity uses desiredCapacity which is an anti-pattern (#5651) * 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> --- packages/@aws-cdk/aws-eks/README.md | 4 ++-- packages/@aws-cdk/aws-eks/lib/cluster.ts | 6 +++--- .../aws-eks/test/integ.eks-cluster.defaults.expected.json | 5 ++--- .../test/integ.eks-cluster.kubectl-disabled.expected.json | 1 - .../aws-eks/test/integ.eks-cluster.kubectl-disabled.ts | 2 +- .../aws-eks/test/integ.eks-cluster.lit.expected.json | 1 - packages/@aws-cdk/aws-eks/test/integ.eks-cluster.lit.ts | 2 +- .../@aws-cdk/aws-eks/test/integ.eks-helm.lit.expected.json | 3 +-- packages/@aws-cdk/aws-eks/test/integ.eks-helm.lit.ts | 2 +- .../aws-eks/test/integ.eks-kubectl.lit.expected.json | 3 +-- packages/@aws-cdk/aws-eks/test/integ.eks-kubectl.lit.ts | 2 +- packages/@aws-cdk/aws-eks/test/integ.eks-spot.expected.json | 3 +-- packages/@aws-cdk/aws-eks/test/test.cluster.ts | 4 ++-- 13 files changed, 16 insertions(+), 22 deletions(-) diff --git a/packages/@aws-cdk/aws-eks/README.md b/packages/@aws-cdk/aws-eks/README.md index c4c7d3caa1f97..7217ecaefb357 100644 --- 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 index 04ed3df18954a..bff8969dbee17 100644 --- 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 index 3d033347d03a4..2207f0f2c7739 100644 --- 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" } } -} \ No newline at end of file +} 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 index 794734b570960..337644a485dae 100644 --- 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 index 5cdab928f90cf..cffa45e2fb4ee 100644 --- 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 index 9cf9a0ae81c26..f378b3772d256 100644 --- 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 index 5ab3956b1ba96..e899a4443ea11 100644 --- 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 index 9dd72f5a51058..69ad72f71a9a0 100644 --- 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 index 35113435a740b..dab62b18d39b0 100644 --- 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 index aca6817f17b21..4634525d3c961 100644 --- 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 index 16821dc79cfd3..1a5b1c7b5bbe8 100644 --- 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 index 747826f71b065..c73d59116b98f 100644 --- 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 index f49727726b24c..fa1ef52331973 100644 --- 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(); },