From bef1b5b6cb08592bb3b03c87abaf668858ed86ab Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Sun, 5 Jan 2020 18:09:56 +0200 Subject: [PATCH 1/2] 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 --- 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 | 1 - .../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, 14 insertions(+), 20 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..84a92585492f0 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 @@ -1040,7 +1040,6 @@ "Properties": { "MaxSize": "2", "MinSize": "1", - "DesiredCapacity": "2", "LaunchConfigurationName": { "Ref": "ClusterDefaultCapacityLaunchConfig72790CF7" }, 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 2beb4a8e36e1f..9119422895ac2 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 @@ -941,8 +941,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 512e0ec0ce350..19d42cb6931d3 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 @@ -941,8 +941,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 08a9eb106491d..b3cc25d7822b7 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(); }, From 10339f366cc689dab7eff7d443765123952ade50 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Mon, 6 Jan 2020 14:03:08 +0200 Subject: [PATCH 2/2] Update integ.eks-cluster.defaults.expected.json --- .../aws-eks/test/integ.eks-cluster.defaults.expected.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 84a92585492f0..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,7 +1039,7 @@ "Type": "AWS::AutoScaling::AutoScalingGroup", "Properties": { "MaxSize": "2", - "MinSize": "1", + "MinSize": "2", "LaunchConfigurationName": { "Ref": "ClusterDefaultCapacityLaunchConfig72790CF7" }, @@ -1379,4 +1379,4 @@ "Default": "/aws/service/eks/optimized-ami/1.14/amazon-linux-2/recommended/image_id" } } -} \ No newline at end of file +}