From 3987f9e70dcfe60c91951cf990102f796f0b86f9 Mon Sep 17 00:00:00 2001
From: Paul Hinze <phinze@phinze.com>
Date: Mon, 8 Jun 2015 11:00:05 -0500
Subject: [PATCH] provider/aws: fix root_block_device for odd AMIs

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.

fixes #2224
---
 .../providers/aws/resource_aws_instance.go    | 46 +++++++++++++++----
 .../aws/resource_aws_instance_test.go         | 41 +++++++++++++++++
 2 files changed, 77 insertions(+), 10 deletions(-)

diff --git a/builtin/providers/aws/resource_aws_instance.go b/builtin/providers/aws/resource_aws_instance.go
index 945d72b8a971..5379e2a09e94 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 eb52289bd93d..f81fc4bfbd2a 100644
--- a/builtin/providers/aws/resource_aws_instance_test.go
+++ b/builtin/providers/aws/resource_aws_instance_test.go
@@ -456,6 +456,26 @@ func TestAccAWSInstance_associatePublicIPAndPrivateIP(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)
 }
@@ -889,3 +909,24 @@ resource "aws_eip" "foo_eip" {
 	depends_on = ["aws_internet_gateway.gw"]
 }
 `
+
+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
+	}
+}
+`