Skip to content

Commit

Permalink
Merge pull request #17380 from matthiasr/mr/gp3-throughput
Browse files Browse the repository at this point in the history
aws_instance: honor throughput for inline volumes when iops is set too
  • Loading branch information
YakDriver authored Feb 11, 2021
2 parents 94a427b + 9b9f23a commit eeb6b37
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 3 deletions.
3 changes: 3 additions & 0 deletions .changelog/17380.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_instance: Fix use of `throughput` and `iops` for `gp3` volumes at the same time
```
6 changes: 4 additions & 2 deletions aws/resource_aws_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -1987,7 +1987,8 @@ func readBlockDeviceMappingsFromConfig(d *schema.ResourceData, conn *ec2.EC2) ([
// Reference: https://github.com/hashicorp/terraform-provider-aws/issues/12667
return nil, fmt.Errorf("error creating resource: iops attribute not supported for ebs_block_device with volume_type %s", v)
}
} else if throughput, ok := bd["throughput"].(int); ok && throughput > 0 {
}
if throughput, ok := bd["throughput"].(int); ok && throughput > 0 {
// `throughput` is only valid for gp3
if ec2.VolumeTypeGp3 == strings.ToLower(v) {
ebs.Throughput = aws.Int64(int64(throughput))
Expand Down Expand Up @@ -2061,7 +2062,8 @@ func readBlockDeviceMappingsFromConfig(d *schema.ResourceData, conn *ec2.EC2) ([
// Reference: https://github.com/hashicorp/terraform-provider-aws/issues/12667
return nil, fmt.Errorf("error creating resource: iops attribute not supported for root_block_device with volume_type %s", v)
}
} else if throughput, ok := bd["throughput"].(int); ok && throughput > 0 {
}
if throughput, ok := bd["throughput"].(int); ok && throughput > 0 {
// throughput is only valid for gp3
if ec2.VolumeTypeGp3 == strings.ToLower(v) {
ebs.Throughput = aws.Int64(int64(throughput))
Expand Down
86 changes: 85 additions & 1 deletion aws/resource_aws_instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,10 @@ func TestAccAWSInstance_blockDevices(t *testing.T) {
return fmt.Errorf("block device doesn't exist: /dev/sdf")
}

if _, ok := blockDevices["/dev/sdg"]; !ok {
return fmt.Errorf("block device doesn't exist: /dev/sdg")
}

return nil
}
}
Expand All @@ -534,7 +538,7 @@ func TestAccAWSInstance_blockDevices(t *testing.T) {
resource.TestMatchResourceAttr(resourceName, "root_block_device.0.volume_id", regexp.MustCompile("vol-[a-z0-9]+")),
resource.TestCheckResourceAttr(resourceName, "root_block_device.0.volume_size", rootVolumeSize),
resource.TestCheckResourceAttr(resourceName, "root_block_device.0.volume_type", "gp2"),
resource.TestCheckResourceAttr(resourceName, "ebs_block_device.#", "4"),
resource.TestCheckResourceAttr(resourceName, "ebs_block_device.#", "5"),
resource.TestCheckTypeSetElemNestedAttrs(resourceName, "ebs_block_device.*", map[string]string{
"device_name": "/dev/sdb",
"volume_size": "9",
Expand All @@ -555,6 +559,13 @@ func TestAccAWSInstance_blockDevices(t *testing.T) {
"volume_type": "gp3",
"throughput": "300",
}),
resource.TestCheckTypeSetElemNestedAttrs(resourceName, "ebs_block_device.*", map[string]string{
"device_name": "/dev/sdg",
"volume_size": "10",
"volume_type": "gp3",
"throughput": "300",
"iops": "4000",
}),
resource.TestMatchTypeSetElemNestedAttrs(resourceName, "ebs_block_device.*", map[string]*regexp.Regexp{
"volume_id": regexp.MustCompile("vol-[a-z0-9]+"),
}),
Expand Down Expand Up @@ -3796,6 +3807,71 @@ resource "aws_instance" "test" {
`, size, delete, volumeType, throughput))
}

func TestAccAWSInstance_GP3RootBlockDevice(t *testing.T) {
var v ec2.Instance
resourceName := "aws_instance.test"

testCheck := func() resource.TestCheckFunc {
return func(*terraform.State) error {
// Map out the block devices by name, which should be unique.
blockDevices := make(map[string]*ec2.InstanceBlockDeviceMapping)
for _, blockDevice := range v.BlockDeviceMappings {
blockDevices[*blockDevice.DeviceName] = blockDevice
}

// Check if the root block device exists.
if _, ok := blockDevices["/dev/xvda"]; !ok {
return fmt.Errorf("block device doesn't exist: /dev/xvda")
}

return nil
}
}

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
IDRefreshName: resourceName,
IDRefreshIgnore: []string{"ephemeral_block_device", "user_data"},
Providers: testAccProviders,
CheckDestroy: testAccCheckInstanceDestroy,
Steps: []resource.TestStep{
{
Config: testAccInstanceConfigGP3RootBlockDevice(),
Check: resource.ComposeTestCheckFunc(
testAccCheckInstanceExists(resourceName, &v),
resource.TestCheckResourceAttr(resourceName, "root_block_device.#", "1"),
resource.TestCheckResourceAttr(resourceName, "root_block_device.0.volume_size", "10"),
resource.TestCheckResourceAttr(resourceName, "root_block_device.0.volume_type", "gp3"),
resource.TestCheckResourceAttr(resourceName, "root_block_device.0.iops", "4000"),
resource.TestCheckResourceAttr(resourceName, "root_block_device.0.throughput", "300"),
testCheck(),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}

func testAccInstanceConfigGP3RootBlockDevice() string {
return composeConfig(testAccLatestAmazonLinuxHvmEbsAmiConfig(), `
resource "aws_instance" "test" {
ami = data.aws_ami.amzn-ami-minimal-hvm-ebs.id
instance_type = "t2.medium"
root_block_device {
volume_size = 10
volume_type = "gp3"
throughput = 300
iops = 4000
}
}
`)
}

const testAccAwsEc2InstanceAmiWithEbsRootVolume = `
data "aws_ami" "ami" {
owners = ["amazon"]
Expand Down Expand Up @@ -3871,6 +3947,14 @@ resource "aws_instance" "test" {
throughput = 300
}
ebs_block_device {
device_name = "/dev/sdg"
volume_size = 10
volume_type = "gp3"
throughput = 300
iops = 4000
}
}
`, size, delete))
}
Expand Down

0 comments on commit eeb6b37

Please sign in to comment.