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.

Sign up for GitHub

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

Allow consuming TF code to alter wait_for_capacity_timeout. #23

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Allow consuming TF code to alter wait_for_capacity_timeout. #23

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a85fa85
Use name_prefix so we can create_before_destroy.
Sep 22, 2017
79726b3
Add wait_for_capacity_timeout option.
joestump Sep 25, 2017
5800d1b
Remove timeouts as it's not the setting we're looking for.
joestump Sep 25, 2017
c2731d1
Merge pull request #1 from carmeraco/name-prefix
joestump Sep 25, 2017
271baef
Merge branch 'master' into asg-timeouts
joestump Sep 25, 2017
62b465e
Fix formatting.
joestump Sep 26, 2017
ee7f1e6
Fix formatting.
joestump Sep 26, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 28 additions & 14 deletions main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,19 +52,32 @@ resource "aws_launch_configuration" "ecs" {
}

resource "aws_autoscaling_group" "ecs" {
name = "asg-${aws_launch_configuration.ecs.name}"
vpc_zone_identifier = ["${var.subnet_id}"]
launch_configuration = "${aws_launch_configuration.ecs.name}"
min_size = "${var.min_servers}"
max_size = "${var.max_servers}"
desired_capacity = "${var.servers}"
termination_policies = ["OldestLaunchConfiguration", "ClosestToNextInstanceHour", "Default"]

tags = [{
key = "Name"
value = "${var.name} ${var.tagName}"
propagate_at_launch = true
}]
name_prefix = "asg-${aws_launch_configuration.ecs.name}-"
Copy link
Member

Choose a reason for hiding this comment

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

I think this change will mean we'll need to cut a new major version release. Am I reading that right? Meaning, if someone upgrades to this version of the module, it'll tear down and recreate the autoscaling groups on them? (I haven't had a chance to pull this code and test)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are correct that it'll tear down any existing ASGs and bring them backup with prefixed names.

vpc_zone_identifier = ["${var.subnet_id}"]
launch_configuration = "${aws_launch_configuration.ecs.name}"
min_size = "${var.min_servers}"
max_size = "${var.max_servers}"
desired_capacity = "${var.servers}"
wait_for_capacity_timeout = "${var.wait_for_capacity_timeout}"
Copy link
Member

Choose a reason for hiding this comment

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

Does this add a similar function to heartbeat_timeout ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really. It's how long TF will wait for a capacity change before timing out. I added it because we had an ASG I wanted to take from 10 to 1 and it timed out.


termination_policies = [
"OldestLaunchConfiguration",
"ClosestToNextInstanceHour",
"Default",
]

tags = [
{
key = "Name"
value = "${var.name} ${var.tagName}"
propagate_at_launch = true
},
{
key = "Terraform"
Copy link
Member

Choose a reason for hiding this comment

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

Cool!

value = "yes"
propagate_at_launch = true
},
]

tags = ["${var.extra_tags}"]

Expand Down Expand Up @@ -100,7 +113,8 @@ resource "aws_security_group" "ecs" {
}

tags {
Name = "ecs-sg-${var.name}"
Name = "ecs-sg-${var.name}"
Terraform = "yes"
Copy link
Member

Choose a reason for hiding this comment

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

I like these tags, one though, what would you think about the tags reading "true" rather then "yes" ?

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 have zero preference. We use this pattern internally, but happy to follow whatever patterns the community/maintainers prefer.

Copy link
Member

Choose a reason for hiding this comment

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

cool, lets switch this to true and I'll merge PR in.

}
}

Expand Down
5 changes: 5 additions & 0 deletions variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,14 @@ variable "enable_agents" {
}

variable "extra_tags" {
type = "list"
default = []
}

variable "wait_for_capacity_timeout" {
default = "10m"
}

variable "heartbeat_timeout" {
description = "Heartbeat Timeout setting for how long it takes for the graceful shutodwn hook takes to timeout. This is useful when deploying clustered applications like consul that benifit from having a deploy between autoscaling create/destroy actions. Defaults to 180"
default = "180"
Expand Down