diff --git a/builtin/providers/aws/resource_aws_instance.go b/builtin/providers/aws/resource_aws_instance.go index 5ac89b4a5..7d996e7f3 100644 --- a/builtin/providers/aws/resource_aws_instance.go +++ b/builtin/providers/aws/resource_aws_instance.go @@ -709,18 +709,44 @@ func fetchRootDeviceName(ami string, conn *ec2.EC2) (*string, error) { } log.Printf("[DEBUG] Describing AMI %q to get root block device name", ami) - req := &ec2.DescribeImagesInput{ImageIDs: []*string{aws.String(ami)}} - if res, err := conn.DescribeImages(req); err == nil { - if len(res.Images) == 1 { - return res.Images[0].RootDeviceName, nil - } else if len(res.Images) == 0 { - return nil, nil - } else { - return nil, fmt.Errorf("Expected 1 AMI for ID: %s, got: %#v", ami, res.Images) - } - } else { + res, err := conn.DescribeImages(&ec2.DescribeImagesInput{ + ImageIDs: []*string{aws.String(ami)}, + }) + if err != nil { return nil, err } + + // For a bad image, we just return nil so we don't block a refresh + if len(res.Images) == 0 { + return nil, nil + } + + image := res.Images[0] + rootDeviceName := image.RootDeviceName + + // Some AMIs have a RootDeviceName like "/dev/sda1" that does not appear as a + // DeviceName in the BlockDeviceMapping list (which will instead have + // something like "/dev/sda") + // + // While this seems like it breaks an invariant of AMIs, it ends up working + // on the AWS side, and AMIs like this are common enough that we need to + // special case it so Terraform does the right thing. + // + // Our heuristic is: if the RootDeviceName does not appear in the + // BlockDeviceMapping, assume that the DeviceName of the first + // BlockDeviceMapping entry serves as the root device. + rootDeviceNameInMapping := false + for _, bdm := range image.BlockDeviceMappings { + if bdm.DeviceName == image.RootDeviceName { + rootDeviceNameInMapping = true + } + } + + if !rootDeviceNameInMapping && len(image.BlockDeviceMappings) > 0 { + rootDeviceName = image.BlockDeviceMappings[0].DeviceName + } + + return rootDeviceName, nil } func readBlockDeviceMappingsFromConfig( diff --git a/builtin/providers/aws/resource_aws_instance_test.go b/builtin/providers/aws/resource_aws_instance_test.go index ef7edcc20..d1bdd00ed 100644 --- a/builtin/providers/aws/resource_aws_instance_test.go +++ b/builtin/providers/aws/resource_aws_instance_test.go @@ -491,6 +491,26 @@ func TestAccAWSInstance_keyPairCheck(t *testing.T) { }) } +func TestAccAWSInstance_rootBlockDeviceMismatch(t *testing.T) { + var v ec2.Instance + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckInstanceDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccInstanceConfigRootBlockDeviceMismatch, + Check: resource.ComposeTestCheckFunc( + testAccCheckInstanceExists("aws_instance.foo", &v), + resource.TestCheckResourceAttr( + "aws_instance.foo", "root_block_device.0.volume_size", "13"), + ), + }, + }, + }) +} + func testAccCheckInstanceDestroy(s *terraform.State) error { return testAccCheckInstanceDestroyWithProvider(s, testAccProvider) } @@ -924,6 +944,7 @@ resource "aws_eip" "foo_eip" { depends_on = ["aws_internet_gateway.gw"] } ` + const testAccInstanceConfigKeyPair = ` provider "aws" { region = "us-east-1" @@ -940,3 +961,24 @@ resource "aws_instance" "foo" { key_name = "${aws_key_pair.debugging.key_name}" } ` + +const testAccInstanceConfigRootBlockDeviceMismatch = ` +resource "aws_vpc" "foo" { + cidr_block = "10.1.0.0/16" +} + +resource "aws_subnet" "foo" { + cidr_block = "10.1.1.0/24" + vpc_id = "${aws_vpc.foo.id}" +} + +resource "aws_instance" "foo" { + // This is an AMI with RootDeviceName: "/dev/sda1"; actual root: "/dev/sda" + ami = "ami-ef5b69df" + instance_type = "t1.micro" + subnet_id = "${aws_subnet.foo.id}" + root_block_device { + volume_size = 13 + } +} +`