Merge pull request #2271 from hashicorp/b-root-block-device-ami-mismatch

provider/aws: fix root_block_device for odd AMIs

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)}}
+	res, err := conn.DescribeImages(&ec2.DescribeImagesInput{
-	if res, err := conn.DescribeImages(req); err == nil {
+		ImageIDs: []*string{aws.String(ami)},
-		if len(res.Images) == 1 {
+	})
-			return res.Images[0].RootDeviceName, nil
+	if err != 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 {
 		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(

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
+	}
+}
+`