Skip to content

Commit

Permalink
Merge pull request #17646 from ewbankkit/b-aws_ebs_volume-update-volu…
Browse files Browse the repository at this point in the history 
…me-type-no-throughput

r/aws_ebs_volume: Only specify throughput on update for gp3 volumes
  • Loading branch information
@YakDriver
YakDriver authored Feb 18, 2021
2 parents 4dedb56 + e64a60b commit df3d498
Show file tree
Hide file tree
Showing 3 changed files with 109 additions and 44 deletions.
3 changes: 3 additions & 0 deletions .changelog/17646.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_ebs_volume: Only specify throughput on update for `gp3` volumes
```
3 changes: 2 additions & 1 deletion aws/resource_aws_ebs_volume.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,8 @@ func resourceAWSEbsVolumeUpdate(d *schema.ResourceData, meta interface{}) error

// "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 {
// Throughput is valid only for gp3 volumes.
if v := d.Get("throughput").(int); v > 0 && d.Get("type").(string) == ec2.VolumeTypeGp3 {
params.Throughput = aws.Int64(int64(v))
}

Expand Down
147 changes: 104 additions & 43 deletions aws/resource_aws_ebs_volume_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,21 @@ func testSweepEbsVolumes(region string) error {
return nil
}

// testAccErrorCheckSkipEBSVolume skips EBS volume tests that have error messages indicating unsupported features
func testAccErrorCheckSkipEBSVolume(t *testing.T) resource.ErrorCheckFunc {
return testAccErrorCheckSkipMessagesContaining(t,
"specified zone does not support multi-attach-enabled volumes",
"Unsupported volume type",
)
}

func TestAccAWSEBSVolume_basic(t *testing.T) {
var v ec2.Volume
resourceName := "aws_ebs_volume.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ErrorCheck: testAccErrorCheckSkipEBSVolume(t),
IDRefreshName: resourceName,
Providers: testAccProviders,
CheckDestroy: testAccCheckVolumeDestroy,
Expand Down Expand Up @@ -107,6 +116,7 @@ func TestAccAWSEBSVolume_updateAttachedEbsVolume(t *testing.T) {

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ErrorCheck: testAccErrorCheckSkipEBSVolume(t),
IDRefreshName: resourceName,
Providers: testAccProviders,
CheckDestroy: testAccCheckVolumeDestroy,
Expand Down Expand Up @@ -142,6 +152,7 @@ func TestAccAWSEBSVolume_updateSize(t *testing.T) {

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ErrorCheck: testAccErrorCheckSkipEBSVolume(t),
IDRefreshName: resourceName,
Providers: testAccProviders,
CheckDestroy: testAccCheckVolumeDestroy,
Expand Down Expand Up @@ -177,6 +188,7 @@ func TestAccAWSEBSVolume_updateType(t *testing.T) {

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ErrorCheck: testAccErrorCheckSkipEBSVolume(t),
IDRefreshName: resourceName,
Providers: testAccProviders,
CheckDestroy: testAccCheckVolumeDestroy,
Expand Down Expand Up @@ -212,6 +224,7 @@ func TestAccAWSEBSVolume_updateIops_Io1(t *testing.T) {

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ErrorCheck: testAccErrorCheckSkipEBSVolume(t),
IDRefreshName: resourceName,
Providers: testAccProviders,
CheckDestroy: testAccCheckVolumeDestroy,
Expand Down Expand Up @@ -247,6 +260,7 @@ func TestAccAWSEBSVolume_updateIops_Io2(t *testing.T) {

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ErrorCheck: testAccErrorCheckSkipEBSVolume(t),
IDRefreshName: resourceName,
Providers: testAccProviders,
CheckDestroy: testAccCheckVolumeDestroy,
Expand Down Expand Up @@ -285,6 +299,7 @@ func TestAccAWSEBSVolume_kmsKey(t *testing.T) {

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ErrorCheck: testAccErrorCheckSkipEBSVolume(t),
IDRefreshName: resourceName,
Providers: testAccProviders,
CheckDestroy: testAccCheckVolumeDestroy,
Expand Down Expand Up @@ -313,6 +328,7 @@ func TestAccAWSEBSVolume_NoIops(t *testing.T) {

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ErrorCheck: testAccErrorCheckSkipEBSVolume(t),
Providers: testAccProviders,
CheckDestroy: testAccCheckVolumeDestroy,
Steps: []resource.TestStep{
Expand All @@ -337,6 +353,7 @@ func TestAccAWSEBSVolume_InvalidIopsForType(t *testing.T) {

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ErrorCheck: testAccErrorCheckSkipEBSVolume(t),
Providers: testAccProviders,
CheckDestroy: testAccCheckVolumeDestroy,
Steps: []resource.TestStep{
Expand All @@ -352,6 +369,7 @@ func TestAccAWSEBSVolume_InvalidThroughputForType(t *testing.T) {

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ErrorCheck: testAccErrorCheckSkipEBSVolume(t),
Providers: testAccProviders,
CheckDestroy: testAccCheckVolumeDestroy,
Steps: []resource.TestStep{
Expand All @@ -369,6 +387,7 @@ func TestAccAWSEBSVolume_withTags(t *testing.T) {

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ErrorCheck: testAccErrorCheckSkipEBSVolume(t),
IDRefreshName: resourceName,
Providers: testAccProviders,
CheckDestroy: testAccCheckVolumeDestroy,
Expand Down Expand Up @@ -397,6 +416,7 @@ func TestAccAWSEBSVolume_multiAttach(t *testing.T) {

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ErrorCheck: testAccErrorCheckSkipEBSVolume(t),
IDRefreshName: resourceName,
Providers: testAccProviders,
CheckDestroy: testAccCheckVolumeDestroy,
Expand Down Expand Up @@ -425,6 +445,7 @@ func TestAccAWSEBSVolume_outpost(t *testing.T) {

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t); testAccPreCheckAWSOutpostsOutposts(t) },
ErrorCheck: testAccErrorCheckSkipEBSVolume(t),
IDRefreshName: resourceName,
Providers: testAccProviders,
CheckDestroy: testAccCheckVolumeDestroy,
Expand Down Expand Up @@ -452,12 +473,13 @@ func TestAccAWSEBSVolume_gp3_basic(t *testing.T) {

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ErrorCheck: testAccErrorCheckSkipEBSVolume(t),
IDRefreshName: resourceName,
Providers: testAccProviders,
CheckDestroy: testAccCheckVolumeDestroy,
Steps: []resource.TestStep{
{
Config: testAccAwsEbsVolumeConfigSizeType(rName, 10, "gp3"),
Config: testAccAwsEbsVolumeConfigSizeTypeIopsThroughput(rName, "10", "gp3", "", ""),
Check: resource.ComposeTestCheckFunc(
testAccCheckVolumeExists(resourceName, &v),
testAccMatchResourceAttrRegionalARN(resourceName, "arn", "ec2", regexp.MustCompile(`volume/vol-.+`)),
Expand Down Expand Up @@ -490,12 +512,13 @@ func TestAccAWSEBSVolume_gp3_iops(t *testing.T) {

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ErrorCheck: testAccErrorCheckSkipEBSVolume(t),
IDRefreshName: resourceName,
Providers: testAccProviders,
CheckDestroy: testAccCheckVolumeDestroy,
Steps: []resource.TestStep{
{
Config: testAccAwsEbsVolumeConfigGp3Iops(rName, 4000),
Config: testAccAwsEbsVolumeConfigSizeTypeIopsThroughput(rName, "10", "gp3", "4000", "200"),
Check: resource.ComposeTestCheckFunc(
testAccCheckVolumeExists(resourceName, &v),
testAccMatchResourceAttrRegionalARN(resourceName, "arn", "ec2", regexp.MustCompile(`volume/vol-.+`)),
Expand All @@ -518,7 +541,7 @@ func TestAccAWSEBSVolume_gp3_iops(t *testing.T) {
ImportStateVerify: true,
},
{
Config: testAccAwsEbsVolumeConfigGp3Iops(rName, 5000),
Config: testAccAwsEbsVolumeConfigSizeTypeIopsThroughput(rName, "10", "gp3", "5000", "200"),
Check: resource.ComposeTestCheckFunc(
testAccCheckVolumeExists(resourceName, &v),
testAccMatchResourceAttrRegionalARN(resourceName, "arn", "ec2", regexp.MustCompile(`volume/vol-.+`)),
Expand Down Expand Up @@ -546,12 +569,13 @@ func TestAccAWSEBSVolume_gp3_throughput(t *testing.T) {

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ErrorCheck: testAccErrorCheckSkipEBSVolume(t),
IDRefreshName: resourceName,
Providers: testAccProviders,
CheckDestroy: testAccCheckVolumeDestroy,
Steps: []resource.TestStep{
{
Config: testAccAwsEbsVolumeConfigGp3Throughput(rName, 400),
Config: testAccAwsEbsVolumeConfigSizeTypeIopsThroughput(rName, "10", "gp3", "", "400"),
Check: resource.ComposeTestCheckFunc(
testAccCheckVolumeExists(resourceName, &v),
testAccMatchResourceAttrRegionalARN(resourceName, "arn", "ec2", regexp.MustCompile(`volume/vol-.+`)),
Expand All @@ -574,7 +598,7 @@ func TestAccAWSEBSVolume_gp3_throughput(t *testing.T) {
ImportStateVerify: true,
},
{
Config: testAccAwsEbsVolumeConfigGp3Throughput(rName, 600),
Config: testAccAwsEbsVolumeConfigSizeTypeIopsThroughput(rName, "10", "gp3", "", "600"),
Check: resource.ComposeTestCheckFunc(
testAccCheckVolumeExists(resourceName, &v),
testAccMatchResourceAttrRegionalARN(resourceName, "arn", "ec2", regexp.MustCompile(`volume/vol-.+`)),
Expand All @@ -595,6 +619,63 @@ func TestAccAWSEBSVolume_gp3_throughput(t *testing.T) {
})
}

func TestAccAWSEBSVolume_gp3_to_gp2(t *testing.T) {
var v ec2.Volume
resourceName := "aws_ebs_volume.test"
rName := acctest.RandomWithPrefix("tf-acc-test")

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ErrorCheck: testAccErrorCheckSkipEBSVolume(t),
IDRefreshName: resourceName,
Providers: testAccProviders,
CheckDestroy: testAccCheckVolumeDestroy,
Steps: []resource.TestStep{
{
Config: testAccAwsEbsVolumeConfigSizeTypeIopsThroughput(rName, "10", "gp3", "3000", "400"),
Check: resource.ComposeTestCheckFunc(
testAccCheckVolumeExists(resourceName, &v),
testAccMatchResourceAttrRegionalARN(resourceName, "arn", "ec2", regexp.MustCompile(`volume/vol-.+`)),
resource.TestCheckResourceAttr(resourceName, "encrypted", "false"),
resource.TestCheckResourceAttr(resourceName, "iops", "3000"),
resource.TestCheckResourceAttr(resourceName, "kms_key_id", ""),
resource.TestCheckResourceAttr(resourceName, "multi_attach_enabled", "false"),
resource.TestCheckResourceAttr(resourceName, "outpost_arn", ""),
resource.TestCheckResourceAttr(resourceName, "size", "10"),
resource.TestCheckResourceAttr(resourceName, "snapshot_id", ""),
resource.TestCheckResourceAttr(resourceName, "tags.%", "1"),
resource.TestCheckResourceAttr(resourceName, "tags.Name", rName),
resource.TestCheckResourceAttr(resourceName, "throughput", "400"),
resource.TestCheckResourceAttr(resourceName, "type", "gp3"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
{
Config: testAccAwsEbsVolumeConfigSizeTypeIopsThroughput(rName, "10", "gp2", "", ""),
Check: resource.ComposeTestCheckFunc(
testAccCheckVolumeExists(resourceName, &v),
testAccMatchResourceAttrRegionalARN(resourceName, "arn", "ec2", regexp.MustCompile(`volume/vol-.+`)),
resource.TestCheckResourceAttr(resourceName, "encrypted", "false"),
resource.TestCheckResourceAttr(resourceName, "iops", "100"),
resource.TestCheckResourceAttr(resourceName, "kms_key_id", ""),
resource.TestCheckResourceAttr(resourceName, "multi_attach_enabled", "false"),
resource.TestCheckResourceAttr(resourceName, "outpost_arn", ""),
resource.TestCheckResourceAttr(resourceName, "size", "10"),
resource.TestCheckResourceAttr(resourceName, "snapshot_id", ""),
resource.TestCheckResourceAttr(resourceName, "tags.%", "1"),
resource.TestCheckResourceAttr(resourceName, "tags.Name", rName),
resource.TestCheckResourceAttr(resourceName, "throughput", "0"),
resource.TestCheckResourceAttr(resourceName, "type", "gp2"),
),
},
},
})
}

func TestAccAWSEBSVolume_snapshotID(t *testing.T) {
var v ec2.Volume
resourceName := "aws_ebs_volume.test"
Expand All @@ -603,6 +684,7 @@ func TestAccAWSEBSVolume_snapshotID(t *testing.T) {

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ErrorCheck: testAccErrorCheckSkipEBSVolume(t),
IDRefreshName: resourceName,
Providers: testAccProviders,
CheckDestroy: testAccCheckVolumeDestroy,
Expand Down Expand Up @@ -642,6 +724,7 @@ func TestAccAWSEBSVolume_snapshotIDAndSize(t *testing.T) {

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ErrorCheck: testAccErrorCheckSkipEBSVolume(t),
IDRefreshName: resourceName,
Providers: testAccProviders,
CheckDestroy: testAccCheckVolumeDestroy,
Expand Down Expand Up @@ -679,6 +762,7 @@ func TestAccAWSEBSVolume_disappears(t *testing.T) {

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ErrorCheck: testAccErrorCheckSkipEBSVolume(t),
Providers: testAccProviders,
CheckDestroy: testAccCheckVolumeDestroy,
Steps: []resource.TestStep{
Expand Down Expand Up @@ -1194,55 +1278,32 @@ resource "aws_ebs_volume" "test" {
`, rName)
}

func testAccAwsEbsVolumeConfigSizeType(rName string, size int, volumeType string) string {
return composeConfig(
testAccAvailableAZsNoOptInConfig(),
fmt.Sprintf(`
resource "aws_ebs_volume" "test" {
availability_zone = data.aws_availability_zones.available.names[0]
type = %[3]q
size = %[2]d
tags = {
Name = %[1]q
}
}
`, rName, size, volumeType))
}

func testAccAwsEbsVolumeConfigGp3Throughput(rName string, throughput int) string {
return composeConfig(
testAccAvailableAZsNoOptInConfig(),
fmt.Sprintf(`
resource "aws_ebs_volume" "test" {
availability_zone = data.aws_availability_zones.available.names[0]
type = "gp3"
size = 10
throughput = %[2]d
tags = {
Name = %[1]q
}
}
`, rName, throughput))
}
func testAccAwsEbsVolumeConfigSizeTypeIopsThroughput(rName, size, volumeType, iops, throughput string) string {
if volumeType == "" {
volumeType = "null"
}
if iops == "" {
iops = "null"
}
if throughput == "" {
throughput = "null"
}

func testAccAwsEbsVolumeConfigGp3Iops(rName string, iops int) string {
return composeConfig(
testAccAvailableAZsNoOptInConfig(),
fmt.Sprintf(`
resource "aws_ebs_volume" "test" {
availability_zone = data.aws_availability_zones.available.names[0]
type = "gp3"
iops = %[2]d
size = 10
throughput = 200
size = %[2]s
type = %[3]q
iops = %[4]s
throughput = %[5]s
tags = {
Name = %[1]q
}
}
`, rName, iops))
`, rName, size, volumeType, iops, throughput))
}

func testAccAwsEbsVolumeConfigSnapshotId(rName string) string {
Expand Down

0 comments on commit df3d498

Please sign in to comment.