-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
fix: Correct capacity reservation target #288
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
Changes from all commits
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 |
---|---|---|
|
@@ -18,81 +18,9 @@ locals { | |
} | ||
|
||
################################################################################ | ||
# Supporting Resources | ||
# EC2 Module | ||
################################################################################ | ||
|
||
module "vpc" { | ||
source = "terraform-aws-modules/vpc/aws" | ||
version = "~> 3.0" | ||
|
||
name = local.name | ||
cidr = "10.99.0.0/18" | ||
|
||
azs = ["${local.region}a", "${local.region}b", "${local.region}c"] | ||
public_subnets = ["10.99.0.0/24", "10.99.1.0/24", "10.99.2.0/24"] | ||
private_subnets = ["10.99.3.0/24", "10.99.4.0/24", "10.99.5.0/24"] | ||
database_subnets = ["10.99.7.0/24", "10.99.8.0/24", "10.99.9.0/24"] | ||
|
||
tags = local.tags | ||
} | ||
|
||
data "aws_ami" "amazon_linux" { | ||
most_recent = true | ||
owners = ["amazon"] | ||
|
||
filter { | ||
name = "name" | ||
values = ["amzn-ami-hvm-*-x86_64-gp2"] | ||
} | ||
} | ||
|
||
module "security_group" { | ||
source = "terraform-aws-modules/security-group/aws" | ||
version = "~> 4.0" | ||
|
||
name = local.name | ||
description = "Security group for example usage with EC2 instance" | ||
vpc_id = module.vpc.vpc_id | ||
|
||
ingress_cidr_blocks = ["0.0.0.0/0"] | ||
ingress_rules = ["http-80-tcp", "all-icmp"] | ||
egress_rules = ["all-all"] | ||
|
||
tags = local.tags | ||
} | ||
|
||
resource "aws_placement_group" "web" { | ||
name = local.name | ||
strategy = "cluster" | ||
} | ||
|
||
resource "aws_kms_key" "this" { | ||
} | ||
|
||
resource "aws_network_interface" "this" { | ||
subnet_id = element(module.vpc.private_subnets, 0) | ||
} | ||
|
||
resource "aws_ec2_capacity_reservation" "open" { | ||
instance_type = "t3.micro" | ||
instance_platform = "Linux/UNIX" | ||
availability_zone = "${local.region}a" | ||
instance_count = 1 | ||
instance_match_criteria = "open" | ||
} | ||
|
||
resource "aws_ec2_capacity_reservation" "targeted" { | ||
instance_type = "t3.micro" | ||
instance_platform = "Linux/UNIX" | ||
availability_zone = "${local.region}a" | ||
instance_count = 1 | ||
instance_match_criteria = "targeted" | ||
} | ||
|
||
# # ################################################################################ | ||
# # # EC2 Module | ||
# # ################################################################################ | ||
|
||
module "ec2_disabled" { | ||
source = "../../" | ||
|
||
|
@@ -123,10 +51,6 @@ module "ec2_complete" { | |
cpu_core_count = 2 # default 4 | ||
cpu_threads_per_core = 1 # default 2 | ||
|
||
capacity_reservation_specification = { | ||
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. this is what the fix enables - users do not need to set this if they are not using capacity reservations |
||
capacity_reservation_preference = "open" | ||
} | ||
|
||
enable_volume_tags = false | ||
root_block_device = [ | ||
{ | ||
|
@@ -295,15 +219,13 @@ module "ec2_spot_instance" { | |
create_spot_instance = true | ||
|
||
ami = data.aws_ami.amazon_linux.id | ||
instance_type = "c4.4xlarge" | ||
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've got kids to feed 😅 |
||
availability_zone = element(module.vpc.azs, 0) | ||
subnet_id = element(module.vpc.private_subnets, 0) | ||
vpc_security_group_ids = [module.security_group.security_group_id] | ||
placement_group = aws_placement_group.web.id | ||
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. not required on this example, looks to be copy+paste from another example |
||
associate_public_ip_address = true | ||
|
||
# Spot request specific attributes | ||
spot_price = "0.60" | ||
spot_price = "0.1" | ||
spot_wait_for_fulfillment = true | ||
spot_type = "persistent" | ||
spot_instance_interruption_behavior = "terminate" | ||
|
@@ -314,9 +236,6 @@ module "ec2_spot_instance" { | |
cpu_core_count = 2 # default 4 | ||
cpu_threads_per_core = 1 # default 2 | ||
|
||
capacity_reservation_specification = { | ||
capacity_reservation_preference = "open" | ||
} | ||
|
||
enable_volume_tags = false | ||
root_block_device = [ | ||
|
@@ -388,3 +307,75 @@ module "ec2_targeted_capacity_reservation" { | |
|
||
tags = local.tags | ||
} | ||
|
||
################################################################################ | ||
# Supporting Resources | ||
################################################################################ | ||
|
||
module "vpc" { | ||
source = "terraform-aws-modules/vpc/aws" | ||
version = "~> 3.0" | ||
|
||
name = local.name | ||
cidr = "10.99.0.0/18" | ||
|
||
azs = ["${local.region}a", "${local.region}b", "${local.region}c"] | ||
public_subnets = ["10.99.0.0/24", "10.99.1.0/24", "10.99.2.0/24"] | ||
private_subnets = ["10.99.3.0/24", "10.99.4.0/24", "10.99.5.0/24"] | ||
database_subnets = ["10.99.7.0/24", "10.99.8.0/24", "10.99.9.0/24"] | ||
|
||
tags = local.tags | ||
} | ||
|
||
data "aws_ami" "amazon_linux" { | ||
most_recent = true | ||
owners = ["amazon"] | ||
|
||
filter { | ||
name = "name" | ||
values = ["amzn-ami-hvm-*-x86_64-gp2"] | ||
} | ||
} | ||
|
||
module "security_group" { | ||
source = "terraform-aws-modules/security-group/aws" | ||
version = "~> 4.0" | ||
|
||
name = local.name | ||
description = "Security group for example usage with EC2 instance" | ||
vpc_id = module.vpc.vpc_id | ||
|
||
ingress_cidr_blocks = ["0.0.0.0/0"] | ||
ingress_rules = ["http-80-tcp", "all-icmp"] | ||
egress_rules = ["all-all"] | ||
|
||
tags = local.tags | ||
} | ||
|
||
resource "aws_placement_group" "web" { | ||
name = local.name | ||
strategy = "cluster" | ||
} | ||
|
||
resource "aws_kms_key" "this" { | ||
} | ||
|
||
resource "aws_network_interface" "this" { | ||
subnet_id = element(module.vpc.private_subnets, 0) | ||
} | ||
|
||
resource "aws_ec2_capacity_reservation" "open" { | ||
instance_type = "t3.micro" | ||
instance_platform = "Linux/UNIX" | ||
availability_zone = "${local.region}a" | ||
instance_count = 1 | ||
instance_match_criteria = "open" | ||
} | ||
|
||
resource "aws_ec2_capacity_reservation" "targeted" { | ||
instance_type = "t3.micro" | ||
instance_platform = "Linux/UNIX" | ||
availability_zone = "${local.region}a" | ||
instance_count = 1 | ||
instance_match_criteria = "targeted" | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -35,14 +35,15 @@ resource "aws_instance" "this" { | |||||||||||||||||||||||||||||
ebs_optimized = var.ebs_optimized | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
dynamic "capacity_reservation_specification" { | ||||||||||||||||||||||||||||||
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. this was the issue and it now matches terraform-aws-ec2-instance/main.tf Lines 184 to 197 in dbf16a2
|
||||||||||||||||||||||||||||||
for_each = var.capacity_reservation_specification != null ? [var.capacity_reservation_specification] : [] | ||||||||||||||||||||||||||||||
for_each = length(var.capacity_reservation_specification) > 0 ? [var.capacity_reservation_specification] : [] | ||||||||||||||||||||||||||||||
content { | ||||||||||||||||||||||||||||||
capacity_reservation_preference = lookup(capacity_reservation_specification.value, "capacity_reservation_preference", null) | ||||||||||||||||||||||||||||||
capacity_reservation_preference = try(capacity_reservation_specification.value.capacity_reservation_preference, null) | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
dynamic "capacity_reservation_target" { | ||||||||||||||||||||||||||||||
for_each = lookup(capacity_reservation_specification.value, "capacity_reservation_target", []) | ||||||||||||||||||||||||||||||
for_each = try([capacity_reservation_specification.value.capacity_reservation_target], []) | ||||||||||||||||||||||||||||||
content { | ||||||||||||||||||||||||||||||
capacity_reservation_id = lookup(capacity_reservation_specification.value.capacity_reservation_target, "capacity_reservation_id", null) | ||||||||||||||||||||||||||||||
capacity_reservation_id = try(capacity_reservation_target.value.capacity_reservation_id, null) | ||||||||||||||||||||||||||||||
capacity_reservation_resource_group_arn = try(capacity_reservation_target.value.capacity_reservation_resource_group_arn, null) | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
|
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.
Just moved the supporting stuff to the bottom