ResiLien
/
terraform
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

provider/aws: Fix AMI creation from snapshot issue 

Previously the AMI creation accepted a static value for the AMI's block device's volume size.
This change allows the user to omit the `volume_size` attribute, in order to mimic the AWS API behavior, which will use the EBS Volume's size.

Also fixes a potential panic case when setting `iops` on the AMI's block device.

The `aws_ami` resource previously didn't have any acceptance tests, adds two acceptance tests and a full testing suite for the `aws_ami` resource, so further tests can be written, as well as expansion upon the other `aws_ami_*` acceptance tests

```
$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSAMI_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/02/09 20:18:22 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSAMI_ -timeout 120m
=== RUN   TestAccAWSAMI_basic
--- PASS: TestAccAWSAMI_basic (44.21s)
=== RUN   TestAccAWSAMI_snapshotSize
--- PASS: TestAccAWSAMI_snapshotSize (45.08s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    89.320s
```
This commit is contained in:
Jake Champlin 2017-02-09 20:30:26 -05:00
parent 3b372b649b
commit fcec0a9f3d
No known key found for this signature in database
GPG Key ID: DC31F41958EF4AC2
2 changed files with 261 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

59
builtin/providers/aws/resource_aws_ami.go
View File

@ -62,12 +62,19 @@ func resourceAwsAmiCreate(d *schema.ResourceData, meta interface{}) error {
DeviceName: aws.String(ebsBlockDev["device_name"].(string)),
Ebs: &ec2.EbsBlockDevice{
DeleteOnTermination: aws.Bool(ebsBlockDev["delete_on_termination"].(bool)),
VolumeSize: aws.Int64(int64(ebsBlockDev["volume_size"].(int))),
VolumeType: aws.String(ebsBlockDev["volume_type"].(string)),
// VolumeSize: aws.Int64(int64(ebsBlockDev["volume_size"].(int))),
VolumeType: aws.String(ebsBlockDev["volume_type"].(string)),
},
}
if iops := ebsBlockDev["iops"].(int); iops != 0 {
blockDev.Ebs.Iops = aws.Int64(int64(iops))
if iops, ok := ebsBlockDev["iops"]; ok {
if iop := iops.(int); iop != 0 {
blockDev.Ebs.Iops = aws.Int64(int64(iop))
}
}
if size, ok := ebsBlockDev["volume_size"]; ok {
if s := size.(int); s != 0 {
blockDev.Ebs.VolumeSize = aws.Int64(int64(s))
}
}
encrypted := ebsBlockDev["encrypted"].(bool)
if snapshotId := ebsBlockDev["snapshot_id"].(string); snapshotId != "" {
@ -353,58 +360,58 @@ func resourceAwsAmiCommonSchema(computed bool) map[string]*schema.Schema {
}
return map[string]*schema.Schema{
"id": &schema.Schema{
"id": {
Type: schema.TypeString,
Computed: true,
},
"image_location": &schema.Schema{
"image_location": {
Type: schema.TypeString,
Optional: !computed,
Computed: true,
ForceNew: !computed,
},
"architecture": &schema.Schema{
"architecture": {
Type: schema.TypeString,
Optional: !computed,
Computed: computed,
ForceNew: !computed,
Default: architectureDefault,
},
"description": &schema.Schema{
"description": {
Type: schema.TypeString,
Optional: true,
},
"kernel_id": &schema.Schema{
"kernel_id": {
Type: schema.TypeString,
Optional: !computed,
Computed: computed,
ForceNew: !computed,
},
"name": &schema.Schema{
"name": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
},
"ramdisk_id": &schema.Schema{
"ramdisk_id": {
Type: schema.TypeString,
Optional: !computed,
Computed: computed,
ForceNew: !computed,
},
"root_device_name": &schema.Schema{
"root_device_name": {
Type: schema.TypeString,
Optional: !computed,
Computed: computed,
ForceNew: !computed,
},
"sriov_net_support": &schema.Schema{
"sriov_net_support": {
Type: schema.TypeString,
Optional: !computed,
Computed: computed,
ForceNew: !computed,
Default: sriovNetSupportDefault,
},
"virtualization_type": &schema.Schema{
"virtualization_type": {
Type: schema.TypeString,
Optional: !computed,
Computed: computed,
@ -419,13 +426,13 @@ func resourceAwsAmiCommonSchema(computed bool) map[string]*schema.Schema {
// on which root device attributes can be overridden for an instance to
// not apply when registering an AMI.
"ebs_block_device": &schema.Schema{
"ebs_block_device": {
Type: schema.TypeSet,
Optional: true,
Computed: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"delete_on_termination": &schema.Schema{
"delete_on_termination": {
Type: schema.TypeBool,
Optional: !computed,
Default: deleteEbsOnTerminationDefault,
@ -433,42 +440,42 @@ func resourceAwsAmiCommonSchema(computed bool) map[string]*schema.Schema {
Computed: computed,
},
"device_name": &schema.Schema{
"device_name": {
Type: schema.TypeString,
Required: !computed,
ForceNew: !computed,
Computed: computed,
},
"encrypted": &schema.Schema{
"encrypted": {
Type: schema.TypeBool,
Optional: !computed,
Computed: computed,
ForceNew: !computed,
},
"iops": &schema.Schema{
"iops": {
Type: schema.TypeInt,
Optional: !computed,
Computed: computed,
ForceNew: !computed,
},
"snapshot_id": &schema.Schema{
"snapshot_id": {
Type: schema.TypeString,
Optional: !computed,
Computed: computed,
ForceNew: !computed,
},
"volume_size": &schema.Schema{
"volume_size": {
Type: schema.TypeInt,
Optional: !computed,
Computed: true,
ForceNew: !computed,
},
"volume_type": &schema.Schema{
"volume_type": {
Type: schema.TypeString,
Optional: !computed,
Computed: computed,
@ -486,20 +493,20 @@ func resourceAwsAmiCommonSchema(computed bool) map[string]*schema.Schema {
},
},
"ephemeral_block_device": &schema.Schema{
"ephemeral_block_device": {
Type: schema.TypeSet,
Optional: true,
Computed: true,
ForceNew: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"device_name": &schema.Schema{
"device_name": {
Type: schema.TypeString,
Required: !computed,
Computed: computed,
},
"virtual_name": &schema.Schema{
"virtual_name": {
Type: schema.TypeString,
Required: !computed,
Computed: computed,
@ -521,7 +528,7 @@ func resourceAwsAmiCommonSchema(computed bool) map[string]*schema.Schema {
// resources record that they implicitly created new EBS snapshots that we should
// now manage. Not set by aws_ami, since the snapshots used there are presumed to
// be independently managed.
"manage_ebs_snapshots": &schema.Schema{
"manage_ebs_snapshots": {
Type: schema.TypeBool,
Computed: true,
ForceNew: true,

234
builtin/providers/aws/resource_aws_ami_test.go
View File

@ -1,8 +1,230 @@
package aws
// FIXME: The aws_ami resource doesn't currently have any acceptance tests,
// since creating an AMI requires having an EBS snapshot and we don't yet
// have a resource type for creating those.
// Once there is an aws_ebs_snapshot resource we can use it to implement
// a reasonable acceptance test for aws_ami. Until then it's necessary to
// test manually using a pre-existing EBS snapshot.
import (
"fmt"
"log"
"testing"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/hashicorp/terraform/helper/acctest"
"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/terraform"
)
func TestAccAWSAMI_basic(t *testing.T) {
var ami ec2.Image
rInt := acctest.RandInt()
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAmiDestroy,
Steps: []resource.TestStep{
{
Config: testAccAmiConfig_basic(rInt),
Check: resource.ComposeTestCheckFunc(
testAccCheckAmiExists("aws_ami.foo", &ami),
resource.TestCheckResourceAttr(
"aws_ami.foo", "name", fmt.Sprintf("tf-testing-%d", rInt)),
),
},
},
})
}
func TestAccAWSAMI_snapshotSize(t *testing.T) {
var ami ec2.Image
var bd ec2.BlockDeviceMapping
rInt := acctest.RandInt()
expectedDevice := &ec2.EbsBlockDevice{
DeleteOnTermination: aws.Bool(true),
Encrypted: aws.Bool(false),
Iops: aws.Int64(0),
VolumeSize: aws.Int64(20),
VolumeType: aws.String("standard"),
}
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAmiDestroy,
Steps: []resource.TestStep{
{
Config: testAccAmiConfig_snapshotSize(rInt),
Check: resource.ComposeTestCheckFunc(
testAccCheckAmiExists("aws_ami.foo", &ami),
testAccCheckAmiBlockDevice(&ami, &bd, "/dev/sda1"),
testAccCheckAmiEbsBlockDevice(&bd, expectedDevice),
resource.TestCheckResourceAttr(
"aws_ami.foo", "name", fmt.Sprintf("tf-testing-%d", rInt)),
resource.TestCheckResourceAttr(
"aws_ami.foo", "architecture", "x86_64"),
),
},
},
})
}
func testAccCheckAmiDestroy(s *terraform.State) error {
conn := testAccProvider.Meta().(*AWSClient).ec2conn
for _, rs := range s.RootModule().Resources {
if rs.Type != "aws_ami" {
continue
}
// Try to find the AMI
log.Printf("AMI-ID: %s", rs.Primary.ID)
DescribeAmiOpts := &ec2.DescribeImagesInput{
ImageIds: []*string{aws.String(rs.Primary.ID)},
}
resp, err := conn.DescribeImages(DescribeAmiOpts)
if err != nil {
return err
}
if len(resp.Images) > 0 {
state := resp.Images[0].State
return fmt.Errorf("AMI %s still exists in the state: %s.", *resp.Images[0].ImageId, *state)
}
}
return nil
}
func testAccCheckAmiExists(n string, ami *ec2.Image) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[n]
if !ok {
return fmt.Errorf("AMI Not found: %s", n)
}
if rs.Primary.ID == "" {
return fmt.Errorf("No AMI ID is set")
}
conn := testAccProvider.Meta().(*AWSClient).ec2conn
opts := &ec2.DescribeImagesInput{
ImageIds: []*string{aws.String(rs.Primary.ID)},
}
resp, err := conn.DescribeImages(opts)
if err != nil {
return err
}
if len(resp.Images) == 0 {
return fmt.Errorf("AMI not found")
}
*ami = *resp.Images[0]
return nil
}
}
func testAccCheckAmiBlockDevice(ami *ec2.Image, blockDevice *ec2.BlockDeviceMapping, n string) resource.TestCheckFunc {
return func(s *terraform.State) error {
devices := make(map[string]*ec2.BlockDeviceMapping)
for _, device := range ami.BlockDeviceMappings {
devices[*device.DeviceName] = device
}
// Check if the block device exists
if _, ok := devices[n]; !ok {
return fmt.Errorf("block device doesn't exist: %s", n)
}
*blockDevice = *devices[n]
return nil
}
}
func testAccCheckAmiEbsBlockDevice(bd *ec2.BlockDeviceMapping, ed *ec2.EbsBlockDevice) resource.TestCheckFunc {
return func(s *terraform.State) error {
// Test for things that ed has, don't care about unset values
cd := bd.Ebs
if ed.VolumeType != nil {
if *ed.VolumeType != *cd.VolumeType {
return fmt.Errorf("Volume type mismatch. Expected: %s Got: %s",
*ed.VolumeType, *cd.VolumeType)
}
}
if ed.DeleteOnTermination != nil {
if *ed.DeleteOnTermination != *cd.DeleteOnTermination {
return fmt.Errorf("DeleteOnTermination mismatch. Expected: %t Got: %t",
*ed.DeleteOnTermination, *cd.DeleteOnTermination)
}
}
if ed.Encrypted != nil {
if *ed.Encrypted != *cd.Encrypted {
return fmt.Errorf("Encrypted mismatch. Expected: %t Got: %t",
*ed.Encrypted, *cd.Encrypted)
}
}
// Integer defaults need to not be `0` so we don't get a panic
if ed.Iops != nil && *ed.Iops != 0 {
if *ed.Iops != *cd.Iops {
return fmt.Errorf("IOPS mismatch. Expected: %d Got: %d",
*ed.Iops, *cd.Iops)
}
}
if ed.VolumeSize != nil && *ed.VolumeSize != 0 {
if *ed.VolumeSize != *cd.VolumeSize {
return fmt.Errorf("Volume Size mismatch. Expected: %d Got: %d",
*ed.VolumeSize, *cd.VolumeSize)
}
}
return nil
}
}
func testAccAmiConfig_basic(rInt int) string {
return fmt.Sprintf(`
resource "aws_ebs_volume" "foo" {
availability_zone = "us-west-2a"
size = 8
tags {
Name = "tf-acc-test"
}
}
resource "aws_ebs_snapshot" "foo" {
volume_id = "${aws_ebs_volume.foo.id}"
}
resource "aws_ami" "foo" {
name = "tf-testing-%d"
virtualization_type = "hvm"
root_device_name = "/dev/sda1"
ebs_block_device {
device_name = "/dev/sda1"
snapshot_id = "${aws_ebs_snapshot.foo.id}"
}
}
`, rInt)
}
func testAccAmiConfig_snapshotSize(rInt int) string {
return fmt.Sprintf(`
resource "aws_ebs_volume" "foo" {
availability_zone = "us-west-2a"
size = 20
tags {
Name = "tf-acc-test"
}
}
resource "aws_ebs_snapshot" "foo" {
volume_id = "${aws_ebs_volume.foo.id}"
}
resource "aws_ami" "foo" {
name = "tf-testing-%d"
virtualization_type = "hvm"
root_device_name = "/dev/sda1"
ebs_block_device {
device_name = "/dev/sda1"
snapshot_id = "${aws_ebs_snapshot.foo.id}"
}
}
`, rInt)
}