From a58f292d88c2474d2ac1913e479a643c769f48ff Mon Sep 17 00:00:00 2001 From: Jake Champlin Date: Thu, 2 Feb 2017 18:57:50 -0500 Subject: [PATCH] provider/aws: Fix spot_instance_request bug Discovered after #11619 was fixed, and while fixing acceptance tests for the `aws_spot_instance_request` resource. Previously the `aws_spot_instance_request` resource wouldn't populate any of the block device attributes from the resulting instance's metadata. This fixes that issue, and also fixes the `aws_spot_instance_request` acceptance tests to be more equipped for running in parallel. ``` make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSSpotInstanceRequest_' ==> Checking that code complies with gofmt requirements... go generate $(go list ./... | grep -v /terraform/vendor/) 2017/02/02 18:02:37 Generated command/internal_plugin_list.go TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSSpotInstanceRequest_ -timeout 120m === RUN TestAccAWSSpotInstanceRequest_basic --- PASS: TestAccAWSSpotInstanceRequest_basic (111.96s) === RUN TestAccAWSSpotInstanceRequest_withBlockDuration --- PASS: TestAccAWSSpotInstanceRequest_withBlockDuration (105.12s) === RUN TestAccAWSSpotInstanceRequest_vpc --- PASS: TestAccAWSSpotInstanceRequest_vpc (115.81s) === RUN TestAccAWSSpotInstanceRequest_SubnetAndSG --- PASS: TestAccAWSSpotInstanceRequest_SubnetAndSG (130.46s) PASS ok github.com/hashicorp/terraform/builtin/providers/aws 463.377s ``` --- .../aws/resource_aws_spot_instance_request.go | 3 + ...resource_aws_spot_instance_request_test.go | 269 +++++++++--------- 2 files changed, 142 insertions(+), 130 deletions(-) diff --git a/builtin/providers/aws/resource_aws_spot_instance_request.go b/builtin/providers/aws/resource_aws_spot_instance_request.go index 73563f62f..6b37d52cc 100644 --- a/builtin/providers/aws/resource_aws_spot_instance_request.go +++ b/builtin/providers/aws/resource_aws_spot_instance_request.go @@ -263,6 +263,9 @@ func readInstance(d *schema.ResourceData, meta interface{}) error { "host": *instance.PrivateIpAddress, }) } + if err := readBlockDevices(d, instance, conn); err != nil { + return err + } } return nil diff --git a/builtin/providers/aws/resource_aws_spot_instance_request_test.go b/builtin/providers/aws/resource_aws_spot_instance_request_test.go index fe0299537..268bf7ee3 100644 --- a/builtin/providers/aws/resource_aws_spot_instance_request_test.go +++ b/builtin/providers/aws/resource_aws_spot_instance_request_test.go @@ -7,25 +7,27 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" ) func TestAccAWSSpotInstanceRequest_basic(t *testing.T) { var sir ec2.SpotInstanceRequest + rInt := acctest.RandInt() resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckAWSSpotInstanceRequestDestroy, Steps: []resource.TestStep{ - resource.TestStep{ - Config: testAccAWSSpotInstanceRequestConfig, + { + Config: testAccAWSSpotInstanceRequestConfig(rInt), Check: resource.ComposeTestCheckFunc( testAccCheckAWSSpotInstanceRequestExists( "aws_spot_instance_request.foo", &sir), testAccCheckAWSSpotInstanceRequestAttributes(&sir), - testCheckKeyPair("tmp-key", &sir), + testCheckKeyPair(fmt.Sprintf("tmp-key-%d", rInt), &sir), resource.TestCheckResourceAttr( "aws_spot_instance_request.foo", "spot_bid_status", "fulfilled"), resource.TestCheckResourceAttr( @@ -38,19 +40,20 @@ func TestAccAWSSpotInstanceRequest_basic(t *testing.T) { func TestAccAWSSpotInstanceRequest_withBlockDuration(t *testing.T) { var sir ec2.SpotInstanceRequest + rInt := acctest.RandInt() resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckAWSSpotInstanceRequestDestroy, Steps: []resource.TestStep{ - resource.TestStep{ - Config: testAccAWSSpotInstanceRequestConfig_withBlockDuration, + { + Config: testAccAWSSpotInstanceRequestConfig_withBlockDuration(rInt), Check: resource.ComposeTestCheckFunc( testAccCheckAWSSpotInstanceRequestExists( "aws_spot_instance_request.foo", &sir), testAccCheckAWSSpotInstanceRequestAttributes(&sir), - testCheckKeyPair("tmp-key", &sir), + testCheckKeyPair(fmt.Sprintf("tmp-key-%d", rInt), &sir), resource.TestCheckResourceAttr( "aws_spot_instance_request.foo", "spot_bid_status", "fulfilled"), resource.TestCheckResourceAttr( @@ -65,19 +68,20 @@ func TestAccAWSSpotInstanceRequest_withBlockDuration(t *testing.T) { func TestAccAWSSpotInstanceRequest_vpc(t *testing.T) { var sir ec2.SpotInstanceRequest + rInt := acctest.RandInt() resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckAWSSpotInstanceRequestDestroy, Steps: []resource.TestStep{ - resource.TestStep{ - Config: testAccAWSSpotInstanceRequestConfigVPC, + { + Config: testAccAWSSpotInstanceRequestConfigVPC(rInt), Check: resource.ComposeTestCheckFunc( testAccCheckAWSSpotInstanceRequestExists( "aws_spot_instance_request.foo_VPC", &sir), testAccCheckAWSSpotInstanceRequestAttributes(&sir), - testCheckKeyPair("tmp-key", &sir), + testCheckKeyPair(fmt.Sprintf("tmp-key-%d", rInt), &sir), testAccCheckAWSSpotInstanceRequestAttributesVPC(&sir), resource.TestCheckResourceAttr( "aws_spot_instance_request.foo_VPC", "spot_bid_status", "fulfilled"), @@ -91,18 +95,19 @@ func TestAccAWSSpotInstanceRequest_vpc(t *testing.T) { func TestAccAWSSpotInstanceRequest_SubnetAndSG(t *testing.T) { var sir ec2.SpotInstanceRequest + rInt := acctest.RandInt() resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckAWSSpotInstanceRequestDestroy, Steps: []resource.TestStep{ - resource.TestStep{ - Config: testAccAWSSpotInstanceRequestConfig_SubnetAndSG, + { + Config: testAccAWSSpotInstanceRequestConfig_SubnetAndSG(rInt), Check: resource.ComposeTestCheckFunc( testAccCheckAWSSpotInstanceRequestExists( "aws_spot_instance_request.foo", &sir), - testAccCheckAWSSpotInstanceRequest_InstanceAttributes(&sir), + testAccCheckAWSSpotInstanceRequest_InstanceAttributes(&sir, rInt), ), }, }, @@ -240,7 +245,7 @@ func testAccCheckAWSSpotInstanceRequestAttributes( } func testAccCheckAWSSpotInstanceRequest_InstanceAttributes( - sir *ec2.SpotInstanceRequest) resource.TestCheckFunc { + sir *ec2.SpotInstanceRequest, rInt int) resource.TestCheckFunc { return func(s *terraform.State) error { conn := testAccProvider.Meta().(*AWSClient).ec2conn resp, err := conn.DescribeInstances(&ec2.DescribeInstancesInput{ @@ -264,13 +269,13 @@ func testAccCheckAWSSpotInstanceRequest_InstanceAttributes( for _, s := range instance.SecurityGroups { // Hardcoded name for the security group that should be added inside the // VPC - if *s.GroupName == "tf_test_sg_ssh" { + if *s.GroupName == fmt.Sprintf("tf_test_sg_ssh-%d", rInt) { sgMatch = true } } if !sgMatch { - return fmt.Errorf("Error in matching Spot Instance Security Group, expected 'tf_test_sg_ssh', got %s", instance.SecurityGroups) + return fmt.Errorf("Error in matching Spot Instance Security Group, expected 'tf_test_sg_ssh-%d', got %s", rInt, instance.SecurityGroups) } return nil @@ -287,132 +292,136 @@ func testAccCheckAWSSpotInstanceRequestAttributesVPC( } } -const testAccAWSSpotInstanceRequestConfig = ` -resource "aws_key_pair" "debugging" { - key_name = "tmp-key" - public_key = "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQD3F6tyPEFEzV0LX3X8BsXdMsQz1x2cEikKDEY0aIj41qgxMCP/iteneqXSIFZBp5vizPvaoIR3Um9xK7PGoW8giupGn+EPuxIA4cDM4vzOqOkiMPhz5XK0whEjkVzTo4+S0puvDZuwIsdiW9mxhJc7tgBNL0cYlWSYVkz4G/fslNfRPW5mYAM49f4fhtxPb5ok4Q2Lg9dPKVHO/Bgeu5woMc7RY0p1ej6D4CKFE6lymSDJpW0YHX/wqE9+cfEauh7xZcG0q9t2ta6F6fmX0agvpFyZo8aFbXeUBr7osSCJNgvavWbM/06niWrOvYX2xwWdhXmXSrbX8ZbabVohBK41 phodgson@thoughtworks.com" -} - -resource "aws_spot_instance_request" "foo" { - ami = "ami-4fccb37f" - instance_type = "m1.small" - key_name = "${aws_key_pair.debugging.key_name}" - - // base price is $0.044 hourly, so bidding above that should theoretically - // always fulfill - spot_price = "0.05" - - // we wait for fulfillment because we want to inspect the launched instance - // and verify termination behavior - wait_for_fulfillment = true - - tags { - Name = "terraform-test" +func testAccAWSSpotInstanceRequestConfig(rInt int) string { + return fmt.Sprintf(` + resource "aws_key_pair" "debugging" { + key_name = "tmp-key-%d" + public_key = "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQD3F6tyPEFEzV0LX3X8BsXdMsQz1x2cEikKDEY0aIj41qgxMCP/iteneqXSIFZBp5vizPvaoIR3Um9xK7PGoW8giupGn+EPuxIA4cDM4vzOqOkiMPhz5XK0whEjkVzTo4+S0puvDZuwIsdiW9mxhJc7tgBNL0cYlWSYVkz4G/fslNfRPW5mYAM49f4fhtxPb5ok4Q2Lg9dPKVHO/Bgeu5woMc7RY0p1ej6D4CKFE6lymSDJpW0YHX/wqE9+cfEauh7xZcG0q9t2ta6F6fmX0agvpFyZo8aFbXeUBr7osSCJNgvavWbM/06niWrOvYX2xwWdhXmXSrbX8ZbabVohBK41 phodgson@thoughtworks.com" } -} -` -const testAccAWSSpotInstanceRequestConfig_withBlockDuration = ` -resource "aws_key_pair" "debugging" { - key_name = "tmp-key" - public_key = "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQD3F6tyPEFEzV0LX3X8BsXdMsQz1x2cEikKDEY0aIj41qgxMCP/iteneqXSIFZBp5vizPvaoIR3Um9xK7PGoW8giupGn+EPuxIA4cDM4vzOqOkiMPhz5XK0whEjkVzTo4+S0puvDZuwIsdiW9mxhJc7tgBNL0cYlWSYVkz4G/fslNfRPW5mYAM49f4fhtxPb5ok4Q2Lg9dPKVHO/Bgeu5woMc7RY0p1ej6D4CKFE6lymSDJpW0YHX/wqE9+cfEauh7xZcG0q9t2ta6F6fmX0agvpFyZo8aFbXeUBr7osSCJNgvavWbM/06niWrOvYX2xwWdhXmXSrbX8ZbabVohBK41 phodgson@thoughtworks.com" + resource "aws_spot_instance_request" "foo" { + ami = "ami-4fccb37f" + instance_type = "m1.small" + key_name = "${aws_key_pair.debugging.key_name}" + + // base price is $0.044 hourly, so bidding above that should theoretically + // always fulfill + spot_price = "0.05" + + // we wait for fulfillment because we want to inspect the launched instance + // and verify termination behavior + wait_for_fulfillment = true + + tags { + Name = "terraform-test" + } + }`, rInt) } -resource "aws_spot_instance_request" "foo" { - ami = "ami-4fccb37f" - instance_type = "m1.small" - key_name = "${aws_key_pair.debugging.key_name}" - - // base price is $0.044 hourly, so bidding above that should theoretically - // always fulfill - spot_price = "0.05" - - // we wait for fulfillment because we want to inspect the launched instance - // and verify termination behavior - wait_for_fulfillment = true - - block_duration_minutes = 60 - - tags { - Name = "terraform-test" +func testAccAWSSpotInstanceRequestConfig_withBlockDuration(rInt int) string { + return fmt.Sprintf(` + resource "aws_key_pair" "debugging" { + key_name = "tmp-key-%d" + public_key = "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQD3F6tyPEFEzV0LX3X8BsXdMsQz1x2cEikKDEY0aIj41qgxMCP/iteneqXSIFZBp5vizPvaoIR3Um9xK7PGoW8giupGn+EPuxIA4cDM4vzOqOkiMPhz5XK0whEjkVzTo4+S0puvDZuwIsdiW9mxhJc7tgBNL0cYlWSYVkz4G/fslNfRPW5mYAM49f4fhtxPb5ok4Q2Lg9dPKVHO/Bgeu5woMc7RY0p1ej6D4CKFE6lymSDJpW0YHX/wqE9+cfEauh7xZcG0q9t2ta6F6fmX0agvpFyZo8aFbXeUBr7osSCJNgvavWbM/06niWrOvYX2xwWdhXmXSrbX8ZbabVohBK41 phodgson@thoughtworks.com" } -} -` -const testAccAWSSpotInstanceRequestConfigVPC = ` -resource "aws_vpc" "foo_VPC" { - cidr_block = "10.1.0.0/16" + resource "aws_spot_instance_request" "foo" { + ami = "ami-4fccb37f" + instance_type = "m1.small" + key_name = "${aws_key_pair.debugging.key_name}" + + // base price is $0.044 hourly, so bidding above that should theoretically + // always fulfill + spot_price = "0.05" + + // we wait for fulfillment because we want to inspect the launched instance + // and verify termination behavior + wait_for_fulfillment = true + + block_duration_minutes = 60 + + tags { + Name = "terraform-test" + } + }`, rInt) } -resource "aws_subnet" "foo_VPC" { - cidr_block = "10.1.1.0/24" - vpc_id = "${aws_vpc.foo_VPC.id}" -} - -resource "aws_key_pair" "debugging" { - key_name = "tmp-key" - public_key = "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQD3F6tyPEFEzV0LX3X8BsXdMsQz1x2cEikKDEY0aIj41qgxMCP/iteneqXSIFZBp5vizPvaoIR3Um9xK7PGoW8giupGn+EPuxIA4cDM4vzOqOkiMPhz5XK0whEjkVzTo4+S0puvDZuwIsdiW9mxhJc7tgBNL0cYlWSYVkz4G/fslNfRPW5mYAM49f4fhtxPb5ok4Q2Lg9dPKVHO/Bgeu5woMc7RY0p1ej6D4CKFE6lymSDJpW0YHX/wqE9+cfEauh7xZcG0q9t2ta6F6fmX0agvpFyZo8aFbXeUBr7osSCJNgvavWbM/06niWrOvYX2xwWdhXmXSrbX8ZbabVohBK41 phodgson@thoughtworks.com" -} - -resource "aws_spot_instance_request" "foo_VPC" { - ami = "ami-4fccb37f" - instance_type = "m1.small" - key_name = "${aws_key_pair.debugging.key_name}" - - // base price is $0.044 hourly, so bidding above that should theoretically - // always fulfill - spot_price = "0.05" - - // VPC settings - subnet_id = "${aws_subnet.foo_VPC.id}" - - // we wait for fulfillment because we want to inspect the launched instance - // and verify termination behavior - wait_for_fulfillment = true - - tags { - Name = "terraform-test-VPC" +func testAccAWSSpotInstanceRequestConfigVPC(rInt int) string { + return fmt.Sprintf(` + resource "aws_vpc" "foo_VPC" { + cidr_block = "10.1.0.0/16" } -} -` -const testAccAWSSpotInstanceRequestConfig_SubnetAndSG = ` -resource "aws_spot_instance_request" "foo" { - ami = "ami-4fccb37f" - instance_type = "m1.small" - spot_price = "0.05" - wait_for_fulfillment = true - subnet_id = "${aws_subnet.tf_test_subnet.id}" - vpc_security_group_ids = ["${aws_security_group.tf_test_sg_ssh.id}"] - associate_public_ip_address = true + resource "aws_subnet" "foo_VPC" { + cidr_block = "10.1.1.0/24" + vpc_id = "${aws_vpc.foo_VPC.id}" + } + + resource "aws_key_pair" "debugging" { + key_name = "tmp-key-%d" + public_key = "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQD3F6tyPEFEzV0LX3X8BsXdMsQz1x2cEikKDEY0aIj41qgxMCP/iteneqXSIFZBp5vizPvaoIR3Um9xK7PGoW8giupGn+EPuxIA4cDM4vzOqOkiMPhz5XK0whEjkVzTo4+S0puvDZuwIsdiW9mxhJc7tgBNL0cYlWSYVkz4G/fslNfRPW5mYAM49f4fhtxPb5ok4Q2Lg9dPKVHO/Bgeu5woMc7RY0p1ej6D4CKFE6lymSDJpW0YHX/wqE9+cfEauh7xZcG0q9t2ta6F6fmX0agvpFyZo8aFbXeUBr7osSCJNgvavWbM/06niWrOvYX2xwWdhXmXSrbX8ZbabVohBK41 phodgson@thoughtworks.com" + } + + resource "aws_spot_instance_request" "foo_VPC" { + ami = "ami-4fccb37f" + instance_type = "m1.small" + key_name = "${aws_key_pair.debugging.key_name}" + + // base price is $0.044 hourly, so bidding above that should theoretically + // always fulfill + spot_price = "0.05" + + // VPC settings + subnet_id = "${aws_subnet.foo_VPC.id}" + + // we wait for fulfillment because we want to inspect the launched instance + // and verify termination behavior + wait_for_fulfillment = true + + tags { + Name = "terraform-test-VPC" + } + }`, rInt) } -resource "aws_vpc" "default" { - cidr_block = "10.0.0.0/16" - enable_dns_hostnames = true +func testAccAWSSpotInstanceRequestConfig_SubnetAndSG(rInt int) string { + return fmt.Sprintf(` + resource "aws_spot_instance_request" "foo" { + ami = "ami-4fccb37f" + instance_type = "m1.small" + spot_price = "0.05" + wait_for_fulfillment = true + subnet_id = "${aws_subnet.tf_test_subnet.id}" + vpc_security_group_ids = ["${aws_security_group.tf_test_sg_ssh.id}"] + associate_public_ip_address = true + } - tags { - Name = "tf_test_vpc" - } + resource "aws_vpc" "default" { + cidr_block = "10.0.0.0/16" + enable_dns_hostnames = true + + tags { + Name = "tf_test_vpc" + } + } + + resource "aws_subnet" "tf_test_subnet" { + vpc_id = "${aws_vpc.default.id}" + cidr_block = "10.0.0.0/24" + map_public_ip_on_launch = true + + tags { + Name = "tf_test_subnet-%d" + } + } + + resource "aws_security_group" "tf_test_sg_ssh" { + name = "tf_test_sg_ssh-%d" + description = "tf_test_sg_ssh" + vpc_id = "${aws_vpc.default.id}" + + tags { + Name = "tf_test_sg_ssh-%d" + } + }`, rInt, rInt, rInt) } - -resource "aws_subnet" "tf_test_subnet" { - vpc_id = "${aws_vpc.default.id}" - cidr_block = "10.0.0.0/24" - map_public_ip_on_launch = true - - tags { - Name = "tf_test_subnet" - } -} - -resource "aws_security_group" "tf_test_sg_ssh" { - name = "tf_test_sg_ssh" - description = "tf_test_sg_ssh" - vpc_id = "${aws_vpc.default.id}" - - tags { - Name = "tf_test_sg_ssh" - } -} -`