Skip to content

Commit

Permalink
fix(eks): default capacity uses desiredCapacity which is an anti-pattern
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Elad Ben-Israel committed Jan 5, 2020
1 parent 9472e09 commit bef1b5b
Show file tree
Hide file tree
Showing 13 changed files with 14 additions and 20 deletions.
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-eks/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
});
```
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-eks/lib/cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1040,7 +1040,6 @@
"Properties": {
"MaxSize": "2",
"MinSize": "1",
"DesiredCapacity": "2",
"LaunchConfigurationName": {
"Ref": "ClusterDefaultCapacityLaunchConfig72790CF7"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -941,7 +941,6 @@
"Properties": {
"MaxSize": "1",
"MinSize": "1",
"DesiredCapacity": "1",
"LaunchConfigurationName": {
"Ref": "EKSClusterNodesLaunchConfig921F1106"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1040,7 +1040,6 @@
"Properties": {
"MaxSize": "1",
"MinSize": "1",
"DesiredCapacity": "1",
"LaunchConfigurationName": {
"Ref": "EKSClusterNodesLaunchConfig921F1106"
},
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-eks/test/integ.eks-cluster.lit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -941,8 +941,7 @@
"Type": "AWS::AutoScaling::AutoScalingGroup",
"Properties": {
"MaxSize": "3",
"MinSize": "1",
"DesiredCapacity": "3",
"MinSize": "3",
"LaunchConfigurationName": {
"Ref": "cluster22NodesLaunchConfig184BF3BA"
},
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-eks/test/integ.eks-helm.lit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -941,8 +941,7 @@
"Type": "AWS::AutoScaling::AutoScalingGroup",
"Properties": {
"MaxSize": "3",
"MinSize": "1",
"DesiredCapacity": "3",
"MinSize": "3",
"LaunchConfigurationName": {
"Ref": "cluster22NodesLaunchConfig184BF3BA"
},
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-eks/test/integ.eks-kubectl.lit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
3 changes: 1 addition & 2 deletions packages/@aws-cdk/aws-eks/test/integ.eks-spot.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -871,8 +871,7 @@
"Type": "AWS::AutoScaling::AutoScalingGroup",
"Properties": {
"MaxSize": "2",
"MinSize": "1",
"DesiredCapacity": "2",
"MinSize": "2",
"LaunchConfigurationName": {
"Ref": "myClusterDefaultCapacityLaunchConfigCF6D4B81"
},
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-eks/test/test.cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
},
Expand All @@ -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();
},
Expand Down

0 comments on commit bef1b5b

Please sign in to comment.