Skip to content

Commit

Permalink
service/storagegateway: Support Cached and VTL gateway upload buffers
Browse files Browse the repository at this point in the history
Reference: #17809

The referenced issue contains the context, but the gist of this issue is that the Storage Gateway API canonicalizes the Cached and VTL disk identifiers only after first cache or upload buffer use, which means that this resource must accept the path first, then perform its own lookup after creation.

This also fixes the `aws_storagegateway_local_disk` data source, which was missing `Computed` on the `disk_node` and `disk_path` attributes, which prevented it for being used to lookup one for the value of the other.

Previously:

```
=== CONT  TestAccAWSStorageGatewayUploadBuffer_DiskPath
    resource_aws_storagegateway_upload_buffer_test.go:107: Step 1/2 error: Error running apply: exit status 1

        Error: error reading Storage Gateway Upload Buffer (arn:aws:storagegateway:us-west-2:187416307283:gateway/sgw-D0A941B9:/dev/nvme1n1): not found

          on terraform_plugin_test.tf line 129, in resource "aws_storagegateway_upload_buffer" "test":
         129: resource "aws_storagegateway_upload_buffer" "test" {

--- FAIL: TestAccAWSStorageGatewayUploadBuffer_DiskPath (418.35s)
```

Output from acceptance testing:

```
--- PASS: TestAccAWSStorageGatewayLocalDiskDataSource_DiskNode (288.52s)
--- PASS: TestAccAWSStorageGatewayLocalDiskDataSource_DiskPath (300.60s)

--- PASS: TestAccAWSStorageGatewayUploadBuffer_basic (418.26s)
--- PASS: TestAccAWSStorageGatewayUploadBuffer_DiskPath (444.33s)
```
  • Loading branch information
bflad committed Mar 19, 2021
1 parent 5ef790a commit 8e16f27
Show file tree
Hide file tree
Showing 7 changed files with 220 additions and 17 deletions.
11 changes: 11 additions & 0 deletions .changelog/pending.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
```release-note:note
resource/aws_storagegateway_upload_buffer: The Storage Gateway `ListLocalDisks` API operation has been implemented to support the `disk_path` attribute for Cached and VTL gateway types. Environments using restrictive IAM permissions may require updates.
```

```release-note:bug
data-source/aws_storagegateway_local_disk: Allow `disk_path` reference on `disk_node` lookup and vice-versa
```

```release-note:enhancement
resource/aws_storagegateway_upload_buffer: Add `disk_path` argument for Cached and VTL gateways
```
2 changes: 2 additions & 0 deletions aws/data_source_aws_storagegateway_local_disk.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,12 @@ func dataSourceAwsStorageGatewayLocalDisk() *schema.Resource {
"disk_node": {
Type: schema.TypeString,
Optional: true,
Computed: true,
},
"disk_path": {
Type: schema.TypeString,
Optional: true,
Computed: true,
},
"gateway_arn": {
Type: schema.TypeString,
Expand Down
8 changes: 6 additions & 2 deletions aws/data_source_aws_storagegateway_local_disk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ func TestAccAWSStorageGatewayLocalDiskDataSource_DiskNode(t *testing.T) {
Config: testAccAWSStorageGatewayLocalDiskDataSourceConfig_DiskNode(rName),
Check: resource.ComposeTestCheckFunc(
testAccAWSStorageGatewayLocalDiskDataSourceExists(dataSourceName),
resource.TestCheckResourceAttrSet(dataSourceName, "disk_id"),
resource.TestMatchResourceAttr(dataSourceName, "disk_id", regexp.MustCompile(`.+`)),
resource.TestMatchResourceAttr(dataSourceName, "disk_node", regexp.MustCompile(`.+`)),
resource.TestMatchResourceAttr(dataSourceName, "disk_path", regexp.MustCompile(`.+`)),
),
},
},
Expand All @@ -54,7 +56,9 @@ func TestAccAWSStorageGatewayLocalDiskDataSource_DiskPath(t *testing.T) {
Config: testAccAWSStorageGatewayLocalDiskDataSourceConfig_DiskPath(rName),
Check: resource.ComposeTestCheckFunc(
testAccAWSStorageGatewayLocalDiskDataSourceExists(dataSourceName),
resource.TestCheckResourceAttrSet(dataSourceName, "disk_id"),
resource.TestMatchResourceAttr(dataSourceName, "disk_id", regexp.MustCompile(`.+`)),
resource.TestMatchResourceAttr(dataSourceName, "disk_node", regexp.MustCompile(`.+`)),
resource.TestMatchResourceAttr(dataSourceName, "disk_path", regexp.MustCompile(`.+`)),
),
},
},
Expand Down
48 changes: 48 additions & 0 deletions aws/internal/service/storagegateway/finder/finder.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,54 @@ import (
"github.com/aws/aws-sdk-go/service/storagegateway"
)

func LocalDiskByDiskId(conn *storagegateway.StorageGateway, gatewayARN string, diskID string) (*storagegateway.Disk, error) {
input := &storagegateway.ListLocalDisksInput{
GatewayARN: aws.String(gatewayARN),
}

output, err := conn.ListLocalDisks(input)

if err != nil {
return nil, err
}

if output == nil {
return nil, nil
}

for _, disk := range output.Disks {
if aws.StringValue(disk.DiskId) == diskID {
return disk, nil
}
}

return nil, nil
}

func LocalDiskByDiskPath(conn *storagegateway.StorageGateway, gatewayARN string, diskPath string) (*storagegateway.Disk, error) {
input := &storagegateway.ListLocalDisksInput{
GatewayARN: aws.String(gatewayARN),
}

output, err := conn.ListLocalDisks(input)

if err != nil {
return nil, err
}

if output == nil {
return nil, nil
}

for _, disk := range output.Disks {
if aws.StringValue(disk.DiskPath) == diskPath {
return disk, nil
}
}

return nil, nil
}

func UploadBufferDisk(conn *storagegateway.StorageGateway, gatewayARN string, diskID string) (*string, error) {
input := &storagegateway.DescribeUploadBufferInput{
GatewayARN: aws.String(gatewayARN),
Expand Down
74 changes: 62 additions & 12 deletions aws/resource_aws_storagegateway_upload_buffer.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,18 @@ func resourceAwsStorageGatewayUploadBuffer() *schema.Resource {

Schema: map[string]*schema.Schema{
"disk_id": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
ExactlyOneOf: []string{"disk_id", "disk_path"},
},
"disk_path": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
ExactlyOneOf: []string{"disk_id", "disk_path"},
},
"gateway_arn": {
Type: schema.TypeString,
Expand All @@ -40,21 +49,48 @@ func resourceAwsStorageGatewayUploadBuffer() *schema.Resource {
func resourceAwsStorageGatewayUploadBufferCreate(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).storagegatewayconn

diskID := d.Get("disk_id").(string)
gatewayARN := d.Get("gateway_arn").(string)
input := &storagegateway.AddUploadBufferInput{}

if v, ok := d.GetOk("disk_id"); ok {
input.DiskIds = aws.StringSlice([]string{v.(string)})
}

// Reference: https://github.com/hashicorp/terraform-provider-aws/issues/17809
if v, ok := d.GetOk("disk_path"); ok {
input.DiskIds = aws.StringSlice([]string{v.(string)})
}

if v, ok := d.GetOk("gateway_arn"); ok {
input.GatewayARN = aws.String(v.(string))
}

output, err := conn.AddUploadBuffer(input)

if err != nil {
return fmt.Errorf("error adding Storage Gateway upload buffer: %w", err)
}

if output == nil {
return fmt.Errorf("error adding Storage Gateway upload buffer: empty response")
}

if v, ok := d.GetOk("disk_id"); ok {
d.SetId(fmt.Sprintf("%s:%s", aws.StringValue(output.GatewayARN), v.(string)))

input := &storagegateway.AddUploadBufferInput{
DiskIds: []*string{aws.String(diskID)},
GatewayARN: aws.String(gatewayARN),
return resourceAwsStorageGatewayUploadBufferRead(d, meta)
}

log.Printf("[DEBUG] Adding Storage Gateway upload buffer: %s", input)
_, err := conn.AddUploadBuffer(input)
disk, err := finder.LocalDiskByDiskPath(conn, aws.StringValue(output.GatewayARN), aws.StringValue(input.DiskIds[0]))

if err != nil {
return fmt.Errorf("error adding Storage Gateway upload buffer: %s", err)
return fmt.Errorf("error listing Storage Gateway Local Disks after creating Upload Buffer: %w", err)
}

if disk == nil {
return fmt.Errorf("error listing Storage Gateway Local Disks after creating Upload Buffer: disk not found")
}

d.SetId(fmt.Sprintf("%s:%s", gatewayARN, diskID))
d.SetId(fmt.Sprintf("%s:%s", aws.StringValue(output.GatewayARN), aws.StringValue(disk.DiskId)))

return resourceAwsStorageGatewayUploadBufferRead(d, meta)
}
Expand Down Expand Up @@ -92,6 +128,20 @@ func resourceAwsStorageGatewayUploadBufferRead(d *schema.ResourceData, meta inte
d.Set("disk_id", foundDiskID)
d.Set("gateway_arn", gatewayARN)

if _, ok := d.GetOk("disk_path"); !ok {
disk, err := finder.LocalDiskByDiskId(conn, gatewayARN, aws.StringValue(foundDiskID))

if err != nil {
return fmt.Errorf("error listing Storage Gateway Local Disks: %w", err)
}

if disk == nil {
return fmt.Errorf("error listing Storage Gateway Local Disks: disk not found")
}

d.Set("disk_path", disk.DiskPath)
}

return nil
}

Expand Down
70 changes: 68 additions & 2 deletions aws/resource_aws_storagegateway_upload_buffer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package aws

import (
"fmt"
"regexp"
"testing"

"github.com/aws/aws-sdk-go/service/storagegateway"
Expand Down Expand Up @@ -82,10 +83,44 @@ func TestAccAWSStorageGatewayUploadBuffer_basic(t *testing.T) {
CheckDestroy: testAccCheckAWSStorageGatewayGatewayDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSStorageGatewayUploadBufferConfig_Basic(rName),
Config: testAccAWSStorageGatewayUploadBufferConfigDiskId(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSStorageGatewayUploadBufferExists(resourceName),
resource.TestCheckResourceAttrPair(resourceName, "disk_id", localDiskDataSourceName, "id"),
resource.TestCheckResourceAttrPair(resourceName, "disk_path", localDiskDataSourceName, "disk_path"),
resource.TestCheckResourceAttrPair(resourceName, "gateway_arn", gatewayResourceName, "arn"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}

// Reference: https://github.com/hashicorp/terraform-provider-aws/issues/17809
func TestAccAWSStorageGatewayUploadBuffer_DiskPath(t *testing.T) {
rName := acctest.RandomWithPrefix("tf-acc-test")
resourceName := "aws_storagegateway_upload_buffer.test"
localDiskDataSourceName := "data.aws_storagegateway_local_disk.test"
gatewayResourceName := "aws_storagegateway_gateway.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ErrorCheck: testAccErrorCheck(t, storagegateway.EndpointsID),
Providers: testAccProviders,
// Storage Gateway API does not support removing upload buffers,
// but we want to ensure other resources are removed.
CheckDestroy: testAccCheckAWSStorageGatewayGatewayDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSStorageGatewayUploadBufferConfigDiskPath(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSStorageGatewayUploadBufferExists(resourceName),
resource.TestMatchResourceAttr(resourceName, "disk_id", regexp.MustCompile(`.+`)),
resource.TestCheckResourceAttrPair(resourceName, "disk_path", localDiskDataSourceName, "disk_path"),
resource.TestCheckResourceAttrPair(resourceName, "gateway_arn", gatewayResourceName, "arn"),
),
},
Expand Down Expand Up @@ -126,7 +161,7 @@ func testAccCheckAWSStorageGatewayUploadBufferExists(resourceName string) resour
}
}

func testAccAWSStorageGatewayUploadBufferConfig_Basic(rName string) string {
func testAccAWSStorageGatewayUploadBufferConfigDiskId(rName string) string {
return testAccAWSStorageGatewayGatewayConfig_GatewayType_Stored(rName) + fmt.Sprintf(`
resource "aws_ebs_volume" "test" {
availability_zone = aws_instance.test.availability_zone
Expand Down Expand Up @@ -156,3 +191,34 @@ resource "aws_storagegateway_upload_buffer" "test" {
}
`, rName)
}

func testAccAWSStorageGatewayUploadBufferConfigDiskPath(rName string) string {
return testAccAWSStorageGatewayGatewayConfig_GatewayType_Cached(rName) + fmt.Sprintf(`
resource "aws_ebs_volume" "test" {
availability_zone = aws_instance.test.availability_zone
size = "10"
type = "gp2"
tags = {
Name = %[1]q
}
}
resource "aws_volume_attachment" "test" {
device_name = "/dev/xvdc"
force_detach = true
instance_id = aws_instance.test.id
volume_id = aws_ebs_volume.test.id
}
data "aws_storagegateway_local_disk" "test" {
disk_node = aws_volume_attachment.test.device_name
gateway_arn = aws_storagegateway_gateway.test.arn
}
resource "aws_storagegateway_upload_buffer" "test" {
disk_path = data.aws_storagegateway_local_disk.test.disk_path
gateway_arn = aws_storagegateway_gateway.test.arn
}
`, rName)
}
24 changes: 23 additions & 1 deletion website/docs/r/storagegateway_upload_buffer.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,28 @@ Manages an AWS Storage Gateway upload buffer.

## Example Usage

### Cached and VTL Gateway Type

```terraform
data "aws_storagegateway_local_disk" "test" {
disk_node = aws_volume_attachment.test.device_name
gateway_arn = aws_storagegateway_gateway.test.arn
}
resource "aws_storagegateway_upload_buffer" "test" {
disk_path = data.aws_storagegateway_local_disk.test.disk_path
gateway_arn = aws_storagegateway_gateway.test.arn
}
```

### Stored Gateway Type

```terraform
data "aws_storagegateway_local_disk" "test" {
disk_node = aws_volume_attachment.test.device_name
gateway_arn = aws_storagegateway_gateway.test.arn
}
resource "aws_storagegateway_upload_buffer" "example" {
disk_id = data.aws_storagegateway_local_disk.example.id
gateway_arn = aws_storagegateway_gateway.example.arn
Expand All @@ -25,7 +46,8 @@ resource "aws_storagegateway_upload_buffer" "example" {

The following arguments are supported:

* `disk_id` - (Required) Local disk identifier. For example, `pci-0000:03:00.0-scsi-0:0:0:0`.
* `disk_id` - (Optional) Local disk identifier. For example, `pci-0000:03:00.0-scsi-0:0:0:0`.
* `disk_path` - (Optional) Local disk path. For example, `/dev/nvme1n1`.
* `gateway_arn` - (Required) The Amazon Resource Name (ARN) of the gateway.

## Attributes Reference
Expand Down

0 comments on commit 8e16f27

Please sign in to comment.