Skip to content

Commit

Permalink
fix(eks): impossible to define multiple spot capacities
Browse files Browse the repository at this point in the history
Install only a single instance of the spot termination handler helm chart, even if there are multiple capacities or ASGs the use spot instances.

Fixes #7136
Fixes #7524
  • Loading branch information
Elad Ben-Israel committed May 4, 2020
1 parent 2008b91 commit be6666b
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 12 deletions.
32 changes: 23 additions & 9 deletions packages/@aws-cdk/aws-eks/lib/cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,8 @@ export class Cluster extends Resource implements ICluster {
*/
private _awsAuth?: AwsAuth;

private _spotInterruptHandler?: HelmChart;

private readonly version: string | undefined;

/**
Expand Down Expand Up @@ -596,15 +598,7 @@ export class Cluster extends Resource implements ICluster {

// if this is an ASG with spot instances, install the spot interrupt handler (only if kubectl is enabled).
if (autoScalingGroup.spotPrice && this.kubectlEnabled) {
this.addChart('spot-interrupt-handler', {
chart: 'aws-node-termination-handler',
version: '0.7.3',
repository: 'https://aws.github.io/eks-charts',
namespace: 'kube-system',
values: {
'nodeSelector.lifecycle': LifecycleLabel.SPOT,
},
});
this.addSpotInterruptHandler();
}
}

Expand Down Expand Up @@ -692,6 +686,26 @@ export class Cluster extends Resource implements ICluster {
return this.stack.node.tryFindChild(uid) as KubectlProvider || new KubectlProvider(this.stack, uid);
}

/**
* Installs the AWS spot instance interrupt handler on the cluster if it's not
* already added.
*/
private addSpotInterruptHandler() {
if (!this._spotInterruptHandler) {
this._spotInterruptHandler = this.addChart('spot-interrupt-handler', {
chart: 'aws-node-termination-handler',
version: '0.7.3',
repository: 'https://aws.github.io/eks-charts',
namespace: 'kube-system',
values: {
'nodeSelector.lifecycle': LifecycleLabel.SPOT,
},
});
}

return this._spotInterruptHandler;
}

/**
* Opportunistically tag subnets with the required tags.
*
Expand Down
27 changes: 24 additions & 3 deletions packages/@aws-cdk/aws-eks/test/test.cluster.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { expect, haveResource, haveResourceLike, not } from '@aws-cdk/assert';
import { countResources, expect, haveResource, haveResourceLike, not } from '@aws-cdk/assert';
import * as ec2 from '@aws-cdk/aws-ec2';
import * as iam from '@aws-cdk/aws-iam';
import * as cdk from '@aws-cdk/core';
Expand Down Expand Up @@ -665,13 +665,13 @@ export = {
},

'if kubectl is enabled, the interrupt handler is added'(test: Test) {
// GIVEN
// GIVEN
const { stack } = testFixtureNoVpc();
const cluster = new eks.Cluster(stack, 'Cluster', { defaultCapacity: 0 });

// WHEN
cluster.addCapacity('MyCapcity', {
instanceType: new ec2.InstanceType('m3.xlargs'),
instanceType: new ec2.InstanceType('m3.xlarge'),
spotPrice: '0.01',
});

Expand All @@ -687,6 +687,27 @@ export = {
test.done();
},

'its possible to add two capacities with spot instances and only one stop handler will be installed'(test: Test) {
// GIVEN
const { stack } = testFixtureNoVpc();
const cluster = new eks.Cluster(stack, 'Cluster', { defaultCapacity: 0 });

// WHEN
cluster.addCapacity('Spot1', {
instanceType: new ec2.InstanceType('m3.xlarge'),
spotPrice: '0.01',
});

cluster.addCapacity('Spot2', {
instanceType: new ec2.InstanceType('m4.xlarge'),
spotPrice: '0.01',
});

// THEN
expect(stack).to(countResources(eks.HelmChart.RESOURCE_TYPE, 1));
test.done();
},

'if kubectl is disabled, interrupt handler is not added'(test: Test) {
// GIVEN
const { stack } = testFixtureNoVpc();
Expand Down

1 comment on commit be6666b

@pahud
Copy link
Contributor

@pahud pahud commented on be6666b May 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @eladb. I've learned something new again.

Please sign in to comment.