Skip to content

Commit

Permalink
Merge pull request #4146 from hashicorp/b-aws-ebs-validate
Browse files Browse the repository at this point in the history
providers/aws: Validate IOPs for EBS Volumes
  • Loading branch information
catsby committed Dec 15, 2015
2 parents 43fb403 + 7bf4046 commit 6448242
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 11 deletions.
42 changes: 32 additions & 10 deletions builtin/providers/aws/resource_aws_ebs_volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,6 @@ func resourceAwsEbsVolumeCreate(d *schema.ResourceData, meta interface{}) error
if value, ok := d.GetOk("encrypted"); ok {
request.Encrypted = aws.Bool(value.(bool))
}
if value, ok := d.GetOk("iops"); ok {
request.Iops = aws.Int64(int64(value.(int)))
}
if value, ok := d.GetOk("kms_key_id"); ok {
request.KmsKeyId = aws.String(value.(string))
}
Expand All @@ -88,18 +85,35 @@ func resourceAwsEbsVolumeCreate(d *schema.ResourceData, meta interface{}) error
if value, ok := d.GetOk("snapshot_id"); ok {
request.SnapshotId = aws.String(value.(string))
}

// IOPs are only valid, and required for, storage type io1. The current minimu
// is 100. Instead of a hard validation we we only apply the IOPs to the
// request if the type is io1, and log a warning otherwise. This allows users
// to "disable" iops. See https://github.com/hashicorp/terraform/pull/4146
var t string
if value, ok := d.GetOk("type"); ok {
request.VolumeType = aws.String(value.(string))
t = value.(string)
request.VolumeType = aws.String(t)
}

iops := d.Get("iops").(int)
if t != "io1" && iops > 0 {
log.Printf("[WARN] IOPs is only valid for storate type io1 for EBS Volumes")
} else if t == "io1" {
// We add the iops value without validating it's size, to allow AWS to
// enforce a size requirement (currently 100)
request.Iops = aws.Int64(int64(iops))
}

log.Printf(
"[DEBUG] EBS Volume create opts: %s", request)
result, err := conn.CreateVolume(request)
if err != nil {
return fmt.Errorf("Error creating EC2 volume: %s", err)
}

log.Printf(
"[DEBUG] Waiting for Volume (%s) to become available",
d.Id())
log.Println(
"[DEBUG] Waiting for Volume to become available")

stateConf := &resource.StateChangeConf{
Pending: []string{"creating"},
Expand Down Expand Up @@ -199,9 +213,6 @@ func readVolume(d *schema.ResourceData, volume *ec2.Volume) error {
if volume.Encrypted != nil {
d.Set("encrypted", *volume.Encrypted)
}
if volume.Iops != nil {
d.Set("iops", *volume.Iops)
}
if volume.KmsKeyId != nil {
d.Set("kms_key_id", *volume.KmsKeyId)
}
Expand All @@ -214,6 +225,17 @@ func readVolume(d *schema.ResourceData, volume *ec2.Volume) error {
if volume.VolumeType != nil {
d.Set("type", *volume.VolumeType)
}

if volume.VolumeType != nil && *volume.VolumeType == "io1" {
// Only set the iops attribute if the volume type is io1. Setting otherwise
// can trigger a refresh/plan loop based on the computed value that is given
// from AWS, and prevent us from specifying 0 as a valid iops.
// See https://github.com/hashicorp/terraform/pull/4146
if volume.Iops != nil {
d.Set("iops", *volume.Iops)
}
}

if volume.Tags != nil {
d.Set("tags", tagsToMap(volume.Tags))
}
Expand Down
28 changes: 28 additions & 0 deletions builtin/providers/aws/resource_aws_ebs_volume_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,22 @@ func TestAccAWSEBSVolume_basic(t *testing.T) {
})
}

func TestAccAWSEBSVolume_NoIops(t *testing.T) {
var v ec2.Volume
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
resource.TestStep{
Config: testAccAwsEbsVolumeConfigWithNoIops,
Check: resource.ComposeTestCheckFunc(
testAccCheckVolumeExists("aws_ebs_volume.iops_test", &v),
),
},
},
})
}

func TestAccAWSEBSVolume_withTags(t *testing.T) {
var v ec2.Volume
resource.Test(t, resource.TestCase{
Expand Down Expand Up @@ -86,3 +102,15 @@ resource "aws_ebs_volume" "tags_test" {
}
}
`

const testAccAwsEbsVolumeConfigWithNoIops = `
resource "aws_ebs_volume" "iops_test" {
availability_zone = "us-west-2a"
size = 10
type = "gp2"
iops = 0
tags {
Name = "TerraformTest"
}
}
`
2 changes: 1 addition & 1 deletion website/source/docs/providers/aws/r/ebs_volume.html.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ Manages a single EBS volume.

```
resource "aws_ebs_volume" "example" {
availability_zone = "us-west-1a"
availability_zone = "us-west-2a"
size = 40
tags {
Name = "HelloWorld"
Expand Down

0 comments on commit 6448242

Please sign in to comment.