Skip to content

Commit

Permalink
provider/aws: fix root_block_device for odd AMIs
Browse files Browse the repository at this point in the history
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
  • Loading branch information
phinze committed Jun 23, 2015
1 parent ce8baea commit 020dc03
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 10 deletions.
46 changes: 36 additions & 10 deletions builtin/providers/aws/resource_aws_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
42 changes: 42 additions & 0 deletions builtin/providers/aws/resource_aws_instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -924,6 +944,7 @@ resource "aws_eip" "foo_eip" {
depends_on = ["aws_internet_gateway.gw"]
}
`

const testAccInstanceConfigKeyPair = `
provider "aws" {
region = "us-east-1"
Expand All @@ -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
}
}
`

0 comments on commit 020dc03

Please sign in to comment.