Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aws_ebs_volume: gp3 volume scalable throughput #16517

Merged
merged 7 commits into from
Jan 19, 2021
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions aws/data_source_aws_ebs_volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ func dataSourceAwsEbsVolume() *schema.Resource {
Computed: true,
},
"tags": tagsSchemaComputed(),
"throughput": {
Type: schema.TypeInt,
Computed: true,
},
},
}
}
Expand Down Expand Up @@ -150,6 +154,7 @@ func volumeDescriptionAttributes(d *schema.ResourceData, client *AWSClient, volu
d.Set("volume_type", volume.VolumeType)
d.Set("outpost_arn", volume.OutpostArn)
d.Set("multi_attach_enabled", volume.MultiAttachEnabled)
d.Set("throughput", volume.Throughput)

if err := d.Set("tags", keyvaluetags.Ec2KeyValueTags(volume.Tags).IgnoreAws().IgnoreConfig(client.IgnoreTagsConfig).Map()); err != nil {
return fmt.Errorf("error setting tags: %s", err)
Expand Down
1 change: 1 addition & 0 deletions aws/data_source_aws_ebs_volume_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ func TestAccAWSEbsVolumeDataSource_basic(t *testing.T) {
resource.TestCheckResourceAttrPair(dataSourceName, "tags", resourceName, "tags"),
resource.TestCheckResourceAttrPair(dataSourceName, "outpost_arn", resourceName, "outpost_arn"),
resource.TestCheckResourceAttrPair(dataSourceName, "multi_attach_enabled", resourceName, "multi_attach_enabled"),
resource.TestCheckResourceAttrPair(dataSourceName, "throughput", resourceName, "throughput"),
),
},
},
Expand Down
147 changes: 91 additions & 56 deletions aws/resource_aws_ebs_volume.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package aws

import (
"context"
"fmt"
"log"
"time"
Expand All @@ -11,6 +12,7 @@ import (
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags"
)

Expand All @@ -24,6 +26,8 @@ func resourceAwsEbsVolume() *schema.Resource {
State: schema.ImportStatePassthrough,
},

CustomizeDiff: resourceAwsEbsVolumeCustomizeDiff,

Schema: map[string]*schema.Schema{
"arn": {
Type: schema.TypeString,
Expand All @@ -44,9 +48,6 @@ func resourceAwsEbsVolume() *schema.Resource {
Type: schema.TypeInt,
Optional: true,
Computed: true,
DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool {
return (d.Get("type").(string) != ec2.VolumeTypeIo1 && new == "0") || (d.Get("type").(string) != ec2.VolumeTypeIo2 && new == "0")
},
},
"kms_key_id": {
Type: schema.TypeString,
Expand All @@ -61,15 +62,17 @@ func resourceAwsEbsVolume() *schema.Resource {
ForceNew: true,
},
"size": {
Type: schema.TypeInt,
Optional: true,
Computed: true,
Type: schema.TypeInt,
Optional: true,
Computed: true,
ExactlyOneOf: []string{"size", "snapshot_id"},
},
"snapshot_id": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
ExactlyOneOf: []string{"size", "snapshot_id"},
},
"outpost_arn": {
Type: schema.TypeString,
Expand All @@ -83,6 +86,12 @@ func resourceAwsEbsVolume() *schema.Resource {
Computed: true,
},
"tags": tagsSchema(),
"throughput": {
Type: schema.TypeInt,
Optional: true,
Computed: true,
ValidateFunc: validation.IntBetween(125, 1000),
},
},
}
}
Expand All @@ -97,6 +106,9 @@ 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 @@ -112,28 +124,11 @@ func resourceAwsEbsVolumeCreate(d *schema.ResourceData, meta interface{}) error
if value, ok := d.GetOk("outpost_arn"); ok {
request.OutpostArn = aws.String(value.(string))
}

// IOPs are only valid, and required for, storage type io1 and io2. The current minimum
// is 100. Hard validation in place to return an error if IOPs are provided
// for an unsupported storage type.
// Reference: https://github.com/hashicorp/terraform-provider-aws/issues/12667
var t string
if value, ok := d.GetOk("type"); ok {
t = value.(string)
request.VolumeType = aws.String(t)
if value, ok := d.GetOk("throughput"); ok {
request.Throughput = aws.Int64(int64(value.(int)))
}

if iops := d.Get("iops").(int); iops > 0 {
if t != ec2.VolumeTypeIo1 && t != ec2.VolumeTypeIo2 {
if t == "" {
// Volume creation would default to gp2
t = ec2.VolumeTypeGp2
}
return fmt.Errorf("error creating ebs_volume: iops attribute not supported for type %s", t)
}
// 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))
if value, ok := d.GetOk("type"); ok {
request.VolumeType = aws.String(value.(string))
}

log.Printf("[DEBUG] EBS Volume create opts: %s", request)
Expand Down Expand Up @@ -168,27 +163,29 @@ func resourceAwsEbsVolumeCreate(d *schema.ResourceData, meta interface{}) error
func resourceAWSEbsVolumeUpdate(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).ec2conn

requestUpdate := false
params := &ec2.ModifyVolumeInput{
VolumeId: aws.String(d.Id()),
}
if d.HasChangesExcept("tags") {
params := &ec2.ModifyVolumeInput{
VolumeId: aws.String(d.Id()),
}

if d.HasChange("size") {
requestUpdate = true
params.Size = aws.Int64(int64(d.Get("size").(int)))
}
if d.HasChange("size") {
params.Size = aws.Int64(int64(d.Get("size").(int)))
}

if d.HasChange("type") {
requestUpdate = true
params.VolumeType = aws.String(d.Get("type").(string))
}
if d.HasChange("type") {
params.VolumeType = aws.String(d.Get("type").(string))
}

if d.HasChange("iops") {
requestUpdate = true
params.Iops = aws.Int64(int64(d.Get("iops").(int)))
}
if d.HasChange("iops") {
params.Iops = aws.Int64(int64(d.Get("iops").(int)))
}

// "If no throughput value is specified, the existing value is retained."
// Not currently correct, so always specify any non-zero throughput value.
if v := d.Get("throughput").(int); v > 0 {
params.Throughput = aws.Int64(int64(v))
}

if requestUpdate {
result, err := conn.ModifyVolume(params)
if err != nil {
return err
Expand Down Expand Up @@ -278,20 +275,21 @@ func resourceAwsEbsVolumeRead(d *schema.ResourceData, meta interface{}) error {
Service: "ec2",
}
d.Set("arn", arn.String())
d.Set("availability_zone", aws.StringValue(volume.AvailabilityZone))
d.Set("encrypted", aws.BoolValue(volume.Encrypted))
d.Set("iops", aws.Int64Value(volume.Iops))
d.Set("kms_key_id", aws.StringValue(volume.KmsKeyId))
d.Set("size", aws.Int64Value(volume.Size))
d.Set("snapshot_id", aws.StringValue(volume.SnapshotId))
d.Set("outpost_arn", aws.StringValue(volume.OutpostArn))
d.Set("availability_zone", volume.AvailabilityZone)
d.Set("encrypted", volume.Encrypted)
d.Set("iops", volume.Iops)
d.Set("kms_key_id", volume.KmsKeyId)
d.Set("size", volume.Size)
d.Set("snapshot_id", volume.SnapshotId)
d.Set("outpost_arn", volume.OutpostArn)
d.Set("multi_attach_enabled", volume.MultiAttachEnabled)
d.Set("throughput", volume.Throughput)

if err := d.Set("tags", keyvaluetags.Ec2KeyValueTags(volume.Tags).IgnoreAws().IgnoreConfig(ignoreTagsConfig).Map()); err != nil {
return fmt.Errorf("error setting tags: %s", err)
}

d.Set("type", aws.StringValue(volume.VolumeType))
d.Set("type", volume.VolumeType)

return nil
}
Expand Down Expand Up @@ -373,3 +371,40 @@ func resourceAwsEbsVolumeDelete(d *schema.ResourceData, meta interface{}) error

return nil
}

func resourceAwsEbsVolumeCustomizeDiff(_ context.Context, diff *schema.ResourceDiff, meta interface{}) error {
iops := diff.Get("iops").(int)
ewbankkit marked this conversation as resolved.
Show resolved Hide resolved
volumeType := diff.Get("type").(string)

if diff.Id() == "" {
// Create.

// Iops is required for io1 and io2 volumes.
// The default for gp3 volumes is 3,000 IOPS.
// This parameter is not supported for gp2, st1, sc1, or standard volumes.
// Hard validation in place to return an error if IOPs are provided
// for an unsupported storage type.
// Reference: https://github.com/hashicorp/terraform-provider-aws/issues/12667
switch volumeType {
case ec2.VolumeTypeIo1, ec2.VolumeTypeIo2:
if iops == 0 {
return fmt.Errorf("'iops' must be set when 'type' is '%s'", volumeType)
}

case ec2.VolumeTypeGp3:
ewbankkit marked this conversation as resolved.
Show resolved Hide resolved

default:
if iops != 0 {
return fmt.Errorf("'iops' must not be set when 'type' is '%s'", volumeType)
}
}
} else {
// Update.

if diff.HasChange("iops") && volumeType != ec2.VolumeTypeIo1 && volumeType != ec2.VolumeTypeIo2 && iops == 0 {
return diff.Clear("iops")
}
}

return nil
}
Loading