From 10954b321d829f2fae781604a0b09701e94d9ccb Mon Sep 17 00:00:00 2001 From: Ilia Lazebnik Date: Thu, 11 Feb 2021 21:48:03 +0200 Subject: [PATCH] resource/aws_efs_access_point: Refactor waiter to separate package (#15265) Output from acceptance testing in AWS Commercial: ``` --- PASS: TestAccAWSEFSAccessPoint_basic (37.59s) --- PASS: TestAccAWSEFSAccessPoint_root_directory (47.35s) --- PASS: TestAccAWSEFSAccessPoint_posix_user_secondary_gids (51.50s) --- PASS: TestAccAWSEFSAccessPoint_tags (56.90s) --- PASS: TestAccAWSEFSAccessPoint_disappears (70.55s) --- PASS: TestAccAWSEFSAccessPoint_posix_user (73.11s) --- PASS: TestAccAWSEFSAccessPoint_root_directory_creation_info (80.01s) ``` Output from acceptance testing in AWS GovCloud (US): ``` --- PASS: TestAccAWSEFSAccessPoint_basic (41.39s) --- PASS: TestAccAWSEFSAccessPoint_posix_user (45.24s) --- PASS: TestAccAWSEFSAccessPoint_root_directory_creation_info (50.31s) --- PASS: TestAccAWSEFSAccessPoint_posix_user_secondary_gids (52.55s) --- PASS: TestAccAWSEFSAccessPoint_tags (83.77s) --- PASS: TestAccAWSEFSAccessPoint_disappears (84.26s) --- PASS: TestAccAWSEFSAccessPoint_root_directory (92.01s) ``` --- aws/internal/service/efs/waiter/status.go | 30 ++++++++ aws/internal/service/efs/waiter/waiter.go | 50 ++++++++++++ aws/resource_aws_efs_access_point.go | 92 ++++------------------- aws/resource_aws_efs_access_point_test.go | 28 +++---- 4 files changed, 108 insertions(+), 92 deletions(-) create mode 100644 aws/internal/service/efs/waiter/status.go create mode 100644 aws/internal/service/efs/waiter/waiter.go diff --git a/aws/internal/service/efs/waiter/status.go b/aws/internal/service/efs/waiter/status.go new file mode 100644 index 00000000000..d48fb58c92b --- /dev/null +++ b/aws/internal/service/efs/waiter/status.go @@ -0,0 +1,30 @@ +package waiter + +import ( + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/efs" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" +) + +// AccessPointLifeCycleState fetches the Access Point and its LifecycleState +func AccessPointLifeCycleState(conn *efs.EFS, accessPointId string) resource.StateRefreshFunc { + return func() (interface{}, string, error) { + input := &efs.DescribeAccessPointsInput{ + AccessPointId: aws.String(accessPointId), + } + + output, err := conn.DescribeAccessPoints(input) + + if err != nil { + return nil, "", err + } + + if output == nil || len(output.AccessPoints) == 0 || output.AccessPoints[0] == nil { + return nil, "", nil + } + + mt := output.AccessPoints[0] + + return mt, aws.StringValue(mt.LifeCycleState), nil + } +} diff --git a/aws/internal/service/efs/waiter/waiter.go b/aws/internal/service/efs/waiter/waiter.go new file mode 100644 index 00000000000..71db067975a --- /dev/null +++ b/aws/internal/service/efs/waiter/waiter.go @@ -0,0 +1,50 @@ +package waiter + +import ( + "time" + + "github.com/aws/aws-sdk-go/service/efs" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" +) + +const ( + // Maximum amount of time to wait for an Operation to return Success + AccessPointCreatedTimeout = 10 * time.Minute + AccessPointDeletedTimeout = 10 * time.Minute +) + +// AccessPointCreated waits for an Operation to return Success +func AccessPointCreated(conn *efs.EFS, accessPointId string) (*efs.AccessPointDescription, error) { + stateConf := &resource.StateChangeConf{ + Pending: []string{efs.LifeCycleStateCreating}, + Target: []string{efs.LifeCycleStateAvailable}, + Refresh: AccessPointLifeCycleState(conn, accessPointId), + Timeout: AccessPointCreatedTimeout, + } + + outputRaw, err := stateConf.WaitForState() + + if output, ok := outputRaw.(*efs.AccessPointDescription); ok { + return output, err + } + + return nil, err +} + +// AccessPointDelete waits for an Access Point to return Deleted +func AccessPointDeleted(conn *efs.EFS, accessPointId string) (*efs.AccessPointDescription, error) { + stateConf := &resource.StateChangeConf{ + Pending: []string{efs.LifeCycleStateAvailable, efs.LifeCycleStateDeleting, efs.LifeCycleStateDeleted}, + Target: []string{}, + Refresh: AccessPointLifeCycleState(conn, accessPointId), + Timeout: AccessPointDeletedTimeout, + } + + outputRaw, err := stateConf.WaitForState() + + if output, ok := outputRaw.(*efs.AccessPointDescription); ok { + return output, err + } + + return nil, err +} diff --git a/aws/resource_aws_efs_access_point.go b/aws/resource_aws_efs_access_point.go index b392192db4e..7776dd71e69 100644 --- a/aws/resource_aws_efs_access_point.go +++ b/aws/resource_aws_efs_access_point.go @@ -3,14 +3,13 @@ package aws import ( "fmt" "log" - "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/service/efs" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/efs/waiter" ) func resourceAwsEfsAccessPoint() *schema.Resource { @@ -143,40 +142,11 @@ func resourceAwsEfsAccessPointCreate(d *schema.ResourceData, meta interface{}) e } d.SetId(aws.StringValue(ap.AccessPointId)) - log.Printf("[INFO] EFS access point ID: %s", d.Id()) - - stateConf := &resource.StateChangeConf{ - Pending: []string{efs.LifeCycleStateCreating}, - Target: []string{efs.LifeCycleStateAvailable}, - Refresh: func() (interface{}, string, error) { - resp, err := conn.DescribeAccessPoints(&efs.DescribeAccessPointsInput{ - AccessPointId: aws.String(d.Id()), - }) - if err != nil { - return nil, "error", err - } - - if hasEmptyAccessPoints(resp) { - return nil, "error", fmt.Errorf("EFS access point %q could not be found.", d.Id()) - } - - mt := resp.AccessPoints[0] - - log.Printf("[DEBUG] Current status of %q: %q", aws.StringValue(mt.AccessPointId), aws.StringValue(mt.LifeCycleState)) - return mt, aws.StringValue(mt.LifeCycleState), nil - }, - Timeout: 10 * time.Minute, - Delay: 2 * time.Second, - MinTimeout: 3 * time.Second, - } - _, err = stateConf.WaitForState() - if err != nil { - return fmt.Errorf("Error waiting for EFS access point (%s) to create: %s", d.Id(), err) + if _, err := waiter.AccessPointCreated(conn, d.Id()); err != nil { + return fmt.Errorf("error waiting for EFS access point (%s) to be available: %w", d.Id(), err) } - log.Printf("[DEBUG] EFS access point created: %s", *ap.AccessPointId) - return resourceAwsEfsAccessPointRead(d, meta) } @@ -187,7 +157,7 @@ func resourceAwsEfsAccessPointUpdate(d *schema.ResourceData, meta interface{}) e o, n := d.GetChange("tags") if err := keyvaluetags.EfsUpdateTags(conn, d.Id(), o, n); err != nil { - return fmt.Errorf("error updating EFS file system (%s) tags: %s", d.Id(), err) + return fmt.Errorf("error updating EFS file system (%s) tags: %w", d.Id(), err) } } @@ -207,7 +177,7 @@ func resourceAwsEfsAccessPointRead(d *schema.ResourceData, meta interface{}) err d.SetId("") return nil } - return fmt.Errorf("Error reading EFS access point %s: %s", d.Id(), err) + return fmt.Errorf("Error reading EFS access point %s: %w", d.Id(), err) } if hasEmptyAccessPoints(resp) { @@ -218,8 +188,6 @@ func resourceAwsEfsAccessPointRead(d *schema.ResourceData, meta interface{}) err log.Printf("[DEBUG] Found EFS access point: %#v", ap) - d.SetId(aws.StringValue(ap.AccessPointId)) - fsARN := arn.ARN{ AccountID: meta.(*AWSClient).accountid, Partition: meta.(*AWSClient).partition, @@ -234,15 +202,15 @@ func resourceAwsEfsAccessPointRead(d *schema.ResourceData, meta interface{}) err d.Set("owner_id", ap.OwnerId) if err := d.Set("posix_user", flattenEfsAccessPointPosixUser(ap.PosixUser)); err != nil { - return fmt.Errorf("error setting posix user: %s", err) + return fmt.Errorf("error setting posix user: %w", err) } if err := d.Set("root_directory", flattenEfsAccessPointRootDirectory(ap.RootDirectory)); err != nil { - return fmt.Errorf("error setting root directory: %s", err) + return fmt.Errorf("error setting root directory: %w", err) } if err := d.Set("tags", keyvaluetags.EfsKeyValueTags(ap.Tags).IgnoreAws().IgnoreConfig(ignoreTagsConfig).Map()); err != nil { - return fmt.Errorf("error setting tags: %s", err) + return fmt.Errorf("error setting tags: %w", err) } return nil @@ -256,12 +224,17 @@ func resourceAwsEfsAccessPointDelete(d *schema.ResourceData, meta interface{}) e AccessPointId: aws.String(d.Id()), }) if err != nil { + if isAWSErr(err, efs.ErrCodeAccessPointNotFound, "") { + return nil + } return fmt.Errorf("error deleting EFS Access Point (%s): %w", d.Id(), err) } - err = waitForDeleteEfsAccessPoint(conn, d.Id(), 10*time.Minute) - if err != nil { - return fmt.Errorf("Error waiting for EFS access point (%q) to delete: %s", d.Id(), err.Error()) + if _, err := waiter.AccessPointDeleted(conn, d.Id()); err != nil { + if isAWSErr(err, efs.ErrCodeAccessPointNotFound, "") { + return nil + } + return fmt.Errorf("error waiting for EFS access point (%s) deletion: %w", d.Id(), err) } log.Printf("[DEBUG] EFS access point %q deleted.", d.Id()) @@ -269,39 +242,6 @@ func resourceAwsEfsAccessPointDelete(d *schema.ResourceData, meta interface{}) e return nil } -func waitForDeleteEfsAccessPoint(conn *efs.EFS, id string, timeout time.Duration) error { - stateConf := &resource.StateChangeConf{ - Pending: []string{efs.LifeCycleStateAvailable, efs.LifeCycleStateDeleting, efs.LifeCycleStateDeleted}, - Target: []string{}, - Refresh: func() (interface{}, string, error) { - resp, err := conn.DescribeAccessPoints(&efs.DescribeAccessPointsInput{ - AccessPointId: aws.String(id), - }) - if err != nil { - if isAWSErr(err, efs.ErrCodeAccessPointNotFound, "") { - return nil, "", nil - } - - return nil, "error", err - } - - if hasEmptyAccessPoints(resp) { - return nil, "", nil - } - - mt := resp.AccessPoints[0] - - log.Printf("[DEBUG] Current status of %q: %q", aws.StringValue(mt.AccessPointId), aws.StringValue(mt.LifeCycleState)) - return mt, aws.StringValue(mt.LifeCycleState), nil - }, - Timeout: timeout, - Delay: 2 * time.Second, - MinTimeout: 3 * time.Second, - } - _, err := stateConf.WaitForState() - return err -} - func hasEmptyAccessPoints(aps *efs.DescribeAccessPointsOutput) bool { if aps != nil && len(aps.AccessPoints) > 0 { return false diff --git a/aws/resource_aws_efs_access_point_test.go b/aws/resource_aws_efs_access_point_test.go index 8588a5ff93c..878b36a1154 100644 --- a/aws/resource_aws_efs_access_point_test.go +++ b/aws/resource_aws_efs_access_point_test.go @@ -5,7 +5,6 @@ import ( "log" "regexp" "testing" - "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/efs" @@ -25,9 +24,10 @@ func init() { func testSweepEfsAccessPoints(region string) error { client, err := sharedClientForRegion(region) if err != nil { - return fmt.Errorf("error getting client: %s", err) + return fmt.Errorf("error getting client: %w", err) } conn := client.(*AWSClient).efsconn + var sweeperErrs *multierror.Error var errors error input := &efs.DescribeFileSystemsInput{} @@ -36,7 +36,6 @@ func testSweepEfsAccessPoints(region string) error { id := aws.StringValue(filesystem.FileSystemId) log.Printf("[INFO] Deleting access points for EFS File System: %s", id) - var errors error input := &efs.DescribeAccessPointsInput{ FileSystemId: filesystem.FileSystemId, } @@ -56,17 +55,14 @@ func testSweepEfsAccessPoints(region string) error { id := aws.StringValue(AccessPoint.AccessPointId) log.Printf("[INFO] Deleting EFS access point: %s", id) - _, err := conn.DeleteAccessPoint(&efs.DeleteAccessPointInput{ - AccessPointId: AccessPoint.AccessPointId, - }) - if err != nil { - errors = multierror.Append(errors, fmt.Errorf("error deleting EFS access point %q: %w", id, err)) - continue - } + r := resourceAwsEfsAccessPoint() + d := r.Data(nil) + d.SetId(id) + err := r.Delete(d, client) - err = waitForDeleteEfsAccessPoint(conn, id, 10*time.Minute) if err != nil { - errors = multierror.Append(errors, fmt.Errorf("error waiting for EFS access point %q to delete: %w", id, err)) + log.Printf("[ERROR] %s", err) + sweeperErrs = multierror.Append(sweeperErrs, err) continue } } @@ -83,7 +79,7 @@ func testSweepEfsAccessPoints(region string) error { errors = multierror.Append(errors, fmt.Errorf("error retrieving EFS File Systems: %w", err)) } - return errors + return sweeperErrs.ErrorOrNil() } func TestAccAWSEFSAccessPoint_basic(t *testing.T) { @@ -350,9 +346,9 @@ func testAccCheckEfsAccessPointExists(resourceID string, mount *efs.AccessPointD return err } - if *mt.AccessPoints[0].AccessPointId != fs.Primary.ID { - return fmt.Errorf("access point ID mismatch: %q != %q", - *mt.AccessPoints[0].AccessPointId, fs.Primary.ID) + apId := aws.StringValue(mt.AccessPoints[0].AccessPointId) + if apId != fs.Primary.ID { + return fmt.Errorf("access point ID mismatch: %q != %q", apId, fs.Primary.ID) } *mount = *mt.AccessPoints[0]