-
Notifications
You must be signed in to change notification settings - Fork 466
Feature/launch template #97
base: master
Are you sure you want to change the base?
Changes from all commits
ef23524
72aee03
0ee25b9
055132e
af10a1f
cbd44e2
ed5cebe
bb7a7a6
234f63b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,10 +11,47 @@ terraform { | |
# --------------------------------------------------------------------------------------------------------------------- | ||
|
||
resource "aws_autoscaling_group" "autoscaling_group" { | ||
count = "${var.asg_launch_mechanism == "launch_configuration" ? 1 : 0}" | ||
name_prefix = "${var.cluster_name}" | ||
|
||
launch_configuration = "${aws_launch_configuration.launch_configuration.name}" | ||
|
||
depends_on = ["aws_iam_instance_profile.instance_profile", "aws_launch_configuration.launch_configuration"] | ||
|
||
availability_zones = ["${var.availability_zones}"] | ||
vpc_zone_identifier = ["${var.subnet_ids}"] | ||
|
||
# Use a fixed-size cluster | ||
min_size = "${var.cluster_size}" | ||
max_size = "${var.cluster_size}" | ||
desired_capacity = "${var.cluster_size}" | ||
termination_policies = ["${var.termination_policies}"] | ||
|
||
health_check_type = "${var.health_check_type}" | ||
health_check_grace_period = "${var.health_check_grace_period}" | ||
wait_for_capacity_timeout = "${var.wait_for_capacity_timeout}" | ||
|
||
tags = ["${concat( | ||
var.cluster_extra_tags, | ||
list( | ||
map("key", var.cluster_tag_key, "value", var.cluster_name, "propagate_at_launch", true) | ||
) | ||
) | ||
}"] | ||
} | ||
|
||
# An alternate autoscaling group that uses a launch_template | ||
resource "aws_autoscaling_group" "lt_autoscaling_group" { | ||
count = "${var.asg_launch_mechanism == "launch_template" ? 1 : 0}" | ||
name_prefix = "${var.cluster_name}" | ||
|
||
launch_template { | ||
id = "${aws_launch_template.launch_template.id}" | ||
version = "${var.launch_template_version}" | ||
} | ||
|
||
depends_on = ["aws_iam_instance_profile.instance_profile", "aws_launch_template.launch_template"] | ||
|
||
availability_zones = ["${var.availability_zones}"] | ||
vpc_zone_identifier = ["${var.subnet_ids}"] | ||
|
||
|
@@ -29,10 +66,11 @@ resource "aws_autoscaling_group" "autoscaling_group" { | |
wait_for_capacity_timeout = "${var.wait_for_capacity_timeout}" | ||
|
||
tags = ["${concat( | ||
var.cluster_extra_tags, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why reverse the order of the tags? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't want var.cluster_extra_tagsto inadvertently change the value of the Name tag. |
||
list( | ||
map("key", var.cluster_tag_key, "value", var.cluster_name, "propagate_at_launch", true) | ||
), | ||
var.cluster_extra_tags) | ||
) | ||
) | ||
}"] | ||
} | ||
|
||
|
@@ -41,6 +79,7 @@ resource "aws_autoscaling_group" "autoscaling_group" { | |
# --------------------------------------------------------------------------------------------------------------------- | ||
|
||
resource "aws_launch_configuration" "launch_configuration" { | ||
count = "${var.asg_launch_mechanism == "launch_configuration" ? 1 : 0}" | ||
name_prefix = "${var.cluster_name}-" | ||
image_id = "${var.ami_id}" | ||
instance_type = "${var.instance_type}" | ||
|
@@ -72,6 +111,77 @@ resource "aws_launch_configuration" "launch_configuration" { | |
} | ||
} | ||
|
||
# --------------------------------------------------------------------------------------------------------------------- | ||
# CREATE LAUNCH TEMPLATE TO DEFINE WHAT RUNS ON EACH INSTANCE IN THE ASG | ||
# --------------------------------------------------------------------------------------------------------------------- | ||
|
||
data "aws_ami" "ami" { | ||
filter { | ||
name = "image-id" | ||
values = ["${var.ami_id}"] | ||
} | ||
} | ||
|
||
resource "aws_launch_template" "launch_template" { | ||
count = "${var.asg_launch_mechanism == "launch_template" ? 1 : 0}" | ||
name_prefix = "${var.cluster_name}-" | ||
image_id = "${var.ami_id}" | ||
instance_type = "${var.instance_type}" | ||
user_data = "${base64encode(var.user_data)}" | ||
|
||
depends_on = ["aws_iam_instance_profile.instance_profile"] | ||
|
||
iam_instance_profile { | ||
name = "${aws_iam_instance_profile.instance_profile.name}" | ||
} | ||
|
||
key_name = "${var.ssh_key_name}" | ||
|
||
# Don't use vpc_security_group_ids when network_interfaces includes security_groups. | ||
# vpc_security_group_ids = ["${concat(list(aws_security_group.lc_security_group.id), var.additional_security_group_ids)}"] | ||
|
||
placement { | ||
tenancy = "${var.tenancy}" | ||
} | ||
network_interfaces { | ||
associate_public_ip_address = "${var.associate_public_ip_address}" | ||
delete_on_termination = true | ||
security_groups = ["${concat(list(aws_security_group.lc_security_group.id), var.additional_security_group_ids)}"] | ||
} | ||
ebs_optimized = "${var.root_volume_ebs_optimized}" | ||
block_device_mappings { | ||
device_name = "${data.aws_ami.ami.root_device_name}" | ||
|
||
ebs { | ||
encrypted = "${var.root_volume_ebs_encryption}" | ||
volume_type = "${var.root_volume_type}" | ||
volume_size = "${var.root_volume_size}" | ||
delete_on_termination = "${var.root_volume_delete_on_termination}" | ||
} | ||
} | ||
tags = "${var.launch_template_tags}" | ||
tag_specifications { | ||
# Instance tags are already handled by the autoscaling group | ||
resource_type = "volume" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I must admit that this is a lot of extra code to maintain just to be able to tag EBS volumes. Is there no other way to do it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree. My hope is that others will find other reasons why launch_template is a better alternative for their usecases and can expand on what I've started here. The reason for the long delay between my initial submission & issue and this PR was due to the aws provider issue mentioned elsewhere. In the meantime, I was able to find a workaround via user-data script which inspects the instance & applies its tags to the instance volume. Of course, that requires that the instance be able to query its own tags and apply tags to its instance. So, yes, there is another way to do it but it's a hack. And, from what I gather, launch templates are the new hot thing and presumably the preferred option going forward. |
||
|
||
tags = "${merge( | ||
var.volume_extra_tags, | ||
map("key", var.cluster_tag_key, "value", var.cluster_name) | ||
) | ||
}" | ||
} | ||
# Important note: whenever using a launch configuration with an auto scaling group, you must set | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this true with a launch template too? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To be honest, I'm not entirely sure that it is (still?) true for launch_configuration. My code is uses the vault-cluster default (true) and I haven't made any special configuration for the other resources. |
||
# create_before_destroy = true. However, as soon as you set create_before_destroy = true in one resource, you must | ||
# also set it in every resource that it depends on, or you'll get an error about cyclic dependencies (especially when | ||
# removing resources). For more info, see: | ||
# | ||
# https://www.terraform.io/docs/providers/aws/r/launch_configuration.html | ||
# https://terraform.io/docs/configuration/resources.html | ||
lifecycle { | ||
create_before_destroy = true | ||
} | ||
} | ||
|
||
# --------------------------------------------------------------------------------------------------------------------- | ||
# CREATE A SECURITY GROUP TO CONTROL WHAT REQUESTS CAN GO IN AND OUT OF EACH EC2 INSTANCE | ||
# --------------------------------------------------------------------------------------------------------------------- | ||
|
@@ -88,7 +198,7 @@ resource "aws_security_group" "lc_security_group" { | |
create_before_destroy = true | ||
} | ||
|
||
tags = "${merge(map("Name", var.cluster_name), var.security_group_tags)}" | ||
tags = "${merge(var.security_group_tags, map("Name", var.cluster_name))}" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why reverse the order of the tags? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't want var.security_group_tags to inadvertently change the value of the Name tag. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, I'm tempted to say that's a feature, not a bug, as it allows the user full control over the |
||
} | ||
|
||
resource "aws_security_group_rule" "allow_ssh_inbound_from_cidr_blocks" { | ||
|
@@ -188,8 +298,9 @@ resource "aws_s3_bucket" "vault_storage" { | |
force_destroy = "${var.force_destroy_s3_bucket}" | ||
|
||
tags = "${merge( | ||
map("Description", "Used for secret storage with Vault. DO NOT DELETE this Bucket unless you know what you are doing."), | ||
var.s3_bucket_tags) | ||
var.s3_bucket_tags, | ||
map("Description", "Used for secret storage with Vault. DO NOT DELETE this Bucket unless you know what you are doing.") | ||
) | ||
}" | ||
} | ||
|
||
|
@@ -201,7 +312,8 @@ resource "aws_iam_role_policy" "vault_s3" { | |
} | ||
|
||
data "aws_iam_policy_document" "vault_s3" { | ||
count = "${var.enable_s3_backend ? 1 : 0}" | ||
count = "${var.enable_s3_backend ? 1 : 0}" | ||
|
||
statement { | ||
effect = "Allow" | ||
actions = ["s3:*"] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,7 @@ | ||
output "asg_name" { | ||
value = "${aws_autoscaling_group.autoscaling_group.name}" | ||
# This is safe because asg_launch_mechanism will only allow one of aws_autoscaling_group.autoscaling_group.* | ||
# or aws_autoscaling_group.lt_autoscaling_group.* to be non-empty. | ||
value = "${join("",concat(aws_autoscaling_group.autoscaling_group.*.name,aws_autoscaling_group.lt_autoscaling_group.*.name))}" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Our typical approach for this is: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah. Nice. Fixed. |
||
} | ||
|
||
output "cluster_tag_key" { | ||
|
@@ -11,11 +13,9 @@ output "cluster_tag_value" { | |
} | ||
|
||
output "cluster_size" { | ||
value = "${aws_autoscaling_group.autoscaling_group.desired_capacity}" | ||
} | ||
|
||
output "launch_config_name" { | ||
value = "${aws_launch_configuration.launch_configuration.name}" | ||
# This is safe because asg_launch_mechanism will only allow one of aws_autoscaling_group.autoscaling_group.* | ||
# or aws_autoscaling_group.lt_autoscaling_group.* to be non-empty. | ||
value = "${element(concat(aws_autoscaling_group.autoscaling_group.*.desired_capacity, aws_autoscaling_group.lt_autoscaling_group.*.desired_capacity), 0)}" | ||
} | ||
|
||
output "iam_role_arn" { | ||
|
@@ -32,4 +32,4 @@ output "security_group_id" { | |
|
||
output "s3_bucket_arn" { | ||
value = "${join(",", aws_s3_bucket.vault_storage.*.arn)}" | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I wonder if we could get by with just one
aws_autoscaling_group
resource.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried but terraform immediately complained if I tried to declare both.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bummer