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

resource/aws_instance: Add check for empty credit_specification block #9003

Merged
merged 1 commit into from
Jun 20, 2019
+78 −3
Merged

resource/aws_instance: Add check for empty credit_specification block #9003

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 6 additions & 3 deletions aws/resource_aws_instance.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1764,9 +1764,12 @@ func buildAwsInstanceOpts(
if v, ok := d.GetOk("credit_specification"); ok {
// Only T2 and T3 are burstable performance instance types and supports Unlimited
if strings.HasPrefix(instanceType, "t2") || strings.HasPrefix(instanceType, "t3") {
cs := v.([]interface{})[0].(map[string]interface{})
opts.CreditSpecification = &ec2.CreditSpecificationRequest{
CpuCredits: aws.String(cs["cpu_credits"].(string)),
if cs, ok := v.([]interface{})[0].(map[string]interface{}); ok {
opts.CreditSpecification = &ec2.CreditSpecificationRequest{
CpuCredits: aws.String(cs["cpu_credits"].(string)),
}
} else {
log.Print("[WARN] credit_specification is defined but the value of cpu_credits is missing, default value will be used.")
}
} else {
log.Print("[WARN] credit_specification is defined but instance type is not T2/T3. Ignoring...")
Expand Down
72 changes: 72 additions & 0 deletions aws/resource_aws_instance_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1693,6 +1693,50 @@ func TestAccAWSInstance_creditSpecification_unlimitedCpuCredits(t *testing.T) {
})
}

func TestAccAWSInstance_creditSpecification_unknownCpuCredits_t2(t *testing.T) {
var instance ec2.Instance
rInt := acctest.RandInt()
resName := "aws_instance.foo"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckInstanceDestroy,
Steps: []resource.TestStep{
{
Config: testAccInstanceConfig_creditSpecification_unknownCpuCredits(rInt, "t2.micro"),
Check: resource.ComposeTestCheckFunc(
testAccCheckInstanceExists(resName, &instance),
resource.TestCheckResourceAttr(resName, "credit_specification.#", "1"),
resource.TestCheckResourceAttr(resName, "credit_specification.0.cpu_credits", "standard"),
),
},
},
})
}

func TestAccAWSInstance_creditSpecification_unknownCpuCredits_t3(t *testing.T) {
var instance ec2.Instance
rInt := acctest.RandInt()
resName := "aws_instance.foo"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckInstanceDestroy,
Steps: []resource.TestStep{
{
Config: testAccInstanceConfig_creditSpecification_unknownCpuCredits(rInt, "t3.micro"),
Check: resource.ComposeTestCheckFunc(
testAccCheckInstanceExists(resName, &instance),
resource.TestCheckResourceAttr(resName, "credit_specification.#", "1"),
resource.TestCheckResourceAttr(resName, "credit_specification.0.cpu_credits", "unlimited"),
),
},
},
})
}

func TestAccAWSInstance_creditSpecification_updateCpuCredits(t *testing.T) {
var first, second, third ec2.Instance
resName := "aws_instance.foo"
Expand Down Expand Up @@ -3827,6 +3871,7 @@ resource "aws_instance" "foo" {
ami = "ami-51537029" # us-west-2
instance_type = "t3.micro"
subnet_id = "${aws_subnet.my_subnet.id}"

}
`, rInt)
}
Expand Down Expand Up @@ -3971,6 +4016,33 @@ resource "aws_instance" "foo" {
`, rInt)
}

func testAccInstanceConfig_creditSpecification_unknownCpuCredits(rInt int, instanceType string) string {
return fmt.Sprintf(`
resource "aws_vpc" "my_vpc" {
cidr_block = "172.16.0.0/16"

tags = {
Name = "tf-acctest-%d"
}
}

resource "aws_subnet" "my_subnet" {
vpc_id = "${aws_vpc.my_vpc.id}"
cidr_block = "172.16.20.0/24"
availability_zone = "us-west-2a"
Copy link
Contributor

Choose a reason for hiding this comment

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

Our acceptance testing should prefer to remain AWS region and availability zone agnostic so it can be used against multiple AWS partitions or otherwise be redirected for region/AZ outages. See also: #4987 and this section under https://github.com/terraform-providers/terraform-provider-aws/blob/master/.github/CONTRIBUTING.md#acceptance-testing-guidelines

The below are location-based items that may be noted during review and are recommended for consistency with testing flexibility. Resource testing is expected to pass across multiple AWS environments supported by the Terraform AWS Provider (e.g. AWS Standard and AWS GovCloud (US)).

In this case, we can either allow EC2 to automatically select the subnet availability zone by omitting the argument or use the aws_availability_zones data source to select one in the available state, e.g.

data "aws_availability_zones" "current" {
  state = "available"
}

resource "aws_subnet" "my_subnet" {
  # ... other configuration ...
  availability_zone = "${data.aws_availability_zones.current.names[0]}"

}

resource "aws_instance" "foo" {
ami = "ami-51537029" # us-west-2
Copy link
Contributor

Choose a reason for hiding this comment

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

Same note and references here about being AWS region agnostic. We can handle AMI lookup via the aws_ami data source against Amazon Linux AMIs that are generally available in all AWS partitions and regions. e.g. from the contributing guide

data "aws_ami" "amzn-ami-minimal-hvm-ebs" {
  most_recent = true
  owners      = ["amazon"]

  filter {
    name = "name"
    values = ["amzn-ami-minimal-hvm-*"]
  }
  filter {
    name = "root-device-type"
    values = ["ebs"]
  }
}

resource "aws_instance" "foo" {
  ami           = "${data.aws_ami.amzn-ami-minimal-hvm-ebs.id}"

instance_type = %q
subnet_id = "${aws_subnet.my_subnet.id}"

credit_specification {
}
}
`, rInt, instanceType)
}

func testAccInstanceConfig_UserData_Base(rInt int) string {
return fmt.Sprintf(`
data "aws_ami" "amzn-ami-minimal-hvm-ebs" {
Expand Down