Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(aws-ecs): instance autoscaling and drain hook #1192

Merged
merged 3 commits into from
Nov 19, 2018

Conversation

rix0rrr
Copy link
Contributor

@rix0rrr rix0rrr commented Nov 16, 2018

Make it easy to configure EC2 instance autoscaling for your cluster,
and automatically add a Lifecylce Hook Lambda that will delay
instance termination until all ECS tasks have drained from the instance.

Fixes #1162.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.

Make it easy to configure EC2 instance autoscaling for your cluster,
and automatically add a Lifecylce Hook Lambda that will delay
instance termination until all ECS tasks have drained from the instance.

Fixes #1162.
@rix0rrr
Copy link
Contributor Author

rix0rrr commented Nov 16, 2018

cc @clareliguori and @SoManyHs.

I've tested this a bit with the amazon/amazon-ecs-sample integ test and manually adding and removing instances, but in some cases even though my instance was set to "DRAINING" the task would not be stopped and eventually the hook timed out and the instance was just killed without the task properly draining.

Was that a scheduling fluke, or do the tasks need draining support, or have I been doing something wrong?

Setup and Lambda code has been copied and slighly adjusted from:

https://github.com/aws-samples/ecs-refarch-cloudformation/blob/master/infrastructure/lifecyclehook.yaml

@clareliguori
Copy link
Member

The scheduler will attempt to start a new task on a different instance before stopping the one on the draining instance. Check that you had enough spare capacity, new tasks started successfully, etc

@clareliguori
Copy link
Member

cc @PaulMaddox

@rix0rrr
Copy link
Contributor Author

rix0rrr commented Nov 17, 2018

It was running elsewhere too

@rix0rrr
Copy link
Contributor Author

rix0rrr commented Nov 17, 2018

But my most important objective right now is to verify the code is ok

@clareliguori
Copy link
Member

LGTM

packages/@aws-cdk/aws-autoscaling/lib/lifecycle-hook.ts Outdated Show resolved Hide resolved
// Invoke Lambda via SNS Topic
const topic = new sns.Topic(this, 'Topic');
const fn = new lambda.Function(this, 'Function', {
code: lambda.Code.directory(path.join(__dirname, 'lambda-source')),
Copy link
Contributor

Choose a reason for hiding this comment

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

  • .directory is deprecated (use .asset)
  • Also, for L2s, if possible, I think we should prefer inline code instead of assets to reduce the surface area

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I totally agree!

@rix0rrr
Copy link
Contributor Author

rix0rrr commented Nov 19, 2018

Waiting for the build...

@rix0rrr rix0rrr merged commit 811462e into master Nov 19, 2018
@rix0rrr rix0rrr deleted the huijbers/ecs-autoscaling branch November 19, 2018 10:37
@NGL321 NGL321 added the contribution/core This is a PR that came from AWS. label Sep 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EC2 instance autoscaling with ECS lifecycle hook
4 participants