From 020dc032341807589f4ea53471d98f200740dc3b 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         | 42 +++++++++++++++++
 2 files changed, 78 insertions(+), 10 deletions(-)

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