From 5263d7879671dbe9516c70becb6833f2b0a54b36 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 28 Mar 2022 11:02:37 -0400 Subject: [PATCH] r/aws_redshift_cluster: Use ClusterAvailabilityStatus when waiting for cluster state changes (#23638). Acceptance test output: % make testacc TESTS=TestAccRedshiftCluster_ PKG=redshift ACCTEST_PARALLELISM=3 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/redshift/... -v -count 1 -parallel 3 -run='TestAccRedshiftCluster_' -timeout 180m === RUN TestAccRedshiftCluster_basic === PAUSE TestAccRedshiftCluster_basic === RUN TestAccRedshiftCluster_disappears === PAUSE TestAccRedshiftCluster_disappears === RUN TestAccRedshiftCluster_withFinalSnapshot === PAUSE TestAccRedshiftCluster_withFinalSnapshot === RUN TestAccRedshiftCluster_kmsKey === PAUSE TestAccRedshiftCluster_kmsKey === RUN TestAccRedshiftCluster_enhancedVPCRoutingEnabled === PAUSE TestAccRedshiftCluster_enhancedVPCRoutingEnabled === RUN TestAccRedshiftCluster_loggingEnabled === PAUSE TestAccRedshiftCluster_loggingEnabled === RUN TestAccRedshiftCluster_snapshotCopy === PAUSE TestAccRedshiftCluster_snapshotCopy === RUN TestAccRedshiftCluster_iamRoles === PAUSE TestAccRedshiftCluster_iamRoles === RUN TestAccRedshiftCluster_publiclyAccessible === PAUSE TestAccRedshiftCluster_publiclyAccessible === RUN TestAccRedshiftCluster_updateNodeCount === PAUSE TestAccRedshiftCluster_updateNodeCount === RUN TestAccRedshiftCluster_updateNodeType === PAUSE TestAccRedshiftCluster_updateNodeType === RUN TestAccRedshiftCluster_tags === PAUSE TestAccRedshiftCluster_tags === RUN TestAccRedshiftCluster_forceNewUsername === PAUSE TestAccRedshiftCluster_forceNewUsername === RUN TestAccRedshiftCluster_changeAvailabilityZone === PAUSE TestAccRedshiftCluster_changeAvailabilityZone === RUN TestAccRedshiftCluster_changeAvailabilityZoneAndSetAvailabilityZoneRelocation === PAUSE TestAccRedshiftCluster_changeAvailabilityZoneAndSetAvailabilityZoneRelocation === RUN TestAccRedshiftCluster_changeAvailabilityZone_availabilityZoneRelocationNotSet === PAUSE TestAccRedshiftCluster_changeAvailabilityZone_availabilityZoneRelocationNotSet === RUN TestAccRedshiftCluster_changeEncryption1 === PAUSE TestAccRedshiftCluster_changeEncryption1 === RUN TestAccRedshiftCluster_changeEncryption2 === PAUSE TestAccRedshiftCluster_changeEncryption2 === RUN TestAccRedshiftCluster_availabilityZoneRelocation === PAUSE TestAccRedshiftCluster_availabilityZoneRelocation === RUN TestAccRedshiftCluster_availabilityZoneRelocation_publiclyAccessible === PAUSE TestAccRedshiftCluster_availabilityZoneRelocation_publiclyAccessible === CONT TestAccRedshiftCluster_basic === CONT TestAccRedshiftCluster_updateNodeType === CONT TestAccRedshiftCluster_changeAvailabilityZone_availabilityZoneRelocationNotSet --- PASS: TestAccRedshiftCluster_basic (222.68s) === CONT TestAccRedshiftCluster_availabilityZoneRelocation --- PASS: TestAccRedshiftCluster_changeAvailabilityZone_availabilityZoneRelocationNotSet (390.44s) === CONT TestAccRedshiftCluster_changeEncryption2 --- PASS: TestAccRedshiftCluster_availabilityZoneRelocation (456.19s) === CONT TestAccRedshiftCluster_changeEncryption1 --- PASS: TestAccRedshiftCluster_updateNodeType (1428.13s) === CONT TestAccRedshiftCluster_availabilityZoneRelocation_publiclyAccessible === CONT TestAccRedshiftCluster_changeAvailabilityZone --- PASS: TestAccRedshiftCluster_availabilityZoneRelocation_publiclyAccessible (2.04s) --- PASS: TestAccRedshiftCluster_changeAvailabilityZone (518.74s) === CONT TestAccRedshiftCluster_changeAvailabilityZoneAndSetAvailabilityZoneRelocation --- PASS: TestAccRedshiftCluster_changeEncryption2 (1651.28s) === CONT TestAccRedshiftCluster_loggingEnabled --- PASS: TestAccRedshiftCluster_changeEncryption1 (1640.72s) === CONT TestAccRedshiftCluster_iamRoles --- PASS: TestAccRedshiftCluster_loggingEnabled (336.16s) === CONT TestAccRedshiftCluster_snapshotCopy --- PASS: TestAccRedshiftCluster_changeAvailabilityZoneAndSetAvailabilityZoneRelocation (682.67s) === CONT TestAccRedshiftCluster_forceNewUsername --- PASS: TestAccRedshiftCluster_snapshotCopy (288.47s) === CONT TestAccRedshiftCluster_publiclyAccessible --- PASS: TestAccRedshiftCluster_iamRoles (402.34s) === CONT TestAccRedshiftCluster_updateNodeCount --- PASS: TestAccRedshiftCluster_publiclyAccessible (304.97s) === CONT TestAccRedshiftCluster_tags --- PASS: TestAccRedshiftCluster_forceNewUsername (503.93s) === CONT TestAccRedshiftCluster_kmsKey --- PASS: TestAccRedshiftCluster_tags (255.92s) === CONT TestAccRedshiftCluster_enhancedVPCRoutingEnabled --- PASS: TestAccRedshiftCluster_kmsKey (420.64s) === CONT TestAccRedshiftCluster_withFinalSnapshot --- PASS: TestAccRedshiftCluster_enhancedVPCRoutingEnabled (649.53s) === CONT TestAccRedshiftCluster_disappears --- PASS: TestAccRedshiftCluster_withFinalSnapshot (496.08s) --- PASS: TestAccRedshiftCluster_disappears (247.65s) --- PASS: TestAccRedshiftCluster_updateNodeCount (1669.39s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/redshift 4394.922s --- internal/service/redshift/cluster_test.go | 4 +- internal/service/redshift/enum.go | 8 ++++ internal/service/redshift/status.go | 16 ++++++++ internal/service/redshift/wait.go | 48 ++++++++--------------- 4 files changed, 43 insertions(+), 33 deletions(-) diff --git a/internal/service/redshift/cluster_test.go b/internal/service/redshift/cluster_test.go index 1586e975246..4f061156829 100644 --- a/internal/service/redshift/cluster_test.go +++ b/internal/service/redshift/cluster_test.go @@ -547,7 +547,7 @@ func TestAccRedshiftCluster_changeAvailabilityZone_availabilityZoneRelocationNot }, { Config: testAccClusterConfig_updateAvailabilityZone_availabilityZoneRelocationNotSet(rName, 1), - ExpectError: regexp.MustCompile(`cannot change availability_zone if availability_zone_relocation_enabled is not true`), + ExpectError: regexp.MustCompile("cannot change `availability_zone` if `availability_zone_relocation_enabled` is not true"), }, }, }) @@ -663,7 +663,7 @@ func TestAccRedshiftCluster_availabilityZoneRelocation_publiclyAccessible(t *tes Steps: []resource.TestStep{ { Config: testAccClusterConfig_availabilityZoneRelocation_publiclyAccessible(rName), - ExpectError: regexp.MustCompile(`availability_zone_relocation_enabled can not be true when publicly_accessible is true`), + ExpectError: regexp.MustCompile("`availability_zone_relocation_enabled` cannot be true when `publicly_accessible` is true"), }, }, }) diff --git a/internal/service/redshift/enum.go b/internal/service/redshift/enum.go index 1dc10a665b1..2acaa786891 100644 --- a/internal/service/redshift/enum.go +++ b/internal/service/redshift/enum.go @@ -1,5 +1,13 @@ package redshift +const ( + clusterAvailabilityStatusAvailable = "Available" + clusterAvailabilityStatusFailed = "Failed" + clusterAvailabilityStatusMaintenance = "Maintenance" + clusterAvailabilityStatusModifying = "Modifying" + clusterAvailabilityStatusUnavailable = "Unavailable" +) + // https://docs.aws.amazon.com/redshift/latest/mgmt/working-with-clusters.html#rs-mgmt-cluster-status. //nolint:deadcode,varcheck // These constants are missing from the AWS SDK const ( diff --git a/internal/service/redshift/status.go b/internal/service/redshift/status.go index 63898f2afa6..6678b020fb5 100644 --- a/internal/service/redshift/status.go +++ b/internal/service/redshift/status.go @@ -23,6 +23,22 @@ func statusCluster(conn *redshift.Redshift, id string) resource.StateRefreshFunc } } +func statusClusterAvailability(conn *redshift.Redshift, id string) resource.StateRefreshFunc { + return func() (interface{}, string, error) { + output, err := FindClusterByID(conn, id) + + if tfresource.NotFound(err) { + return nil, "", nil + } + + if err != nil { + return nil, "", err + } + + return output, aws.StringValue(output.ClusterAvailabilityStatus), nil + } +} + func statusClusterAvailabilityZoneRelocation(conn *redshift.Redshift, id string) resource.StateRefreshFunc { return func() (interface{}, string, error) { output, err := FindClusterByID(conn, id) diff --git a/internal/service/redshift/wait.go b/internal/service/redshift/wait.go index b9fc5c96a5d..7132ef732c5 100644 --- a/internal/service/redshift/wait.go +++ b/internal/service/redshift/wait.go @@ -1,10 +1,13 @@ package redshift import ( + "errors" "time" + "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/redshift" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) const ( @@ -15,15 +18,9 @@ const ( func waitClusterCreated(conn *redshift.Redshift, id string, timeout time.Duration) (*redshift.Cluster, error) { stateConf := &resource.StateChangeConf{ - Pending: []string{ - clusterStatusAvailablePrepForResize, - clusterStatusBackingUp, - clusterStatusCreating, - clusterStatusModifying, - clusterStatusRestoring, - }, - Target: []string{clusterStatusAvailable}, - Refresh: statusCluster(conn, id), + Pending: []string{clusterAvailabilityStatusModifying}, + Target: []string{clusterAvailabilityStatusAvailable}, + Refresh: statusClusterAvailability(conn, id), Timeout: timeout, MinTimeout: 10 * time.Second, } @@ -31,6 +28,8 @@ func waitClusterCreated(conn *redshift.Redshift, id string, timeout time.Duratio outputRaw, err := stateConf.WaitForState() if output, ok := outputRaw.(*redshift.Cluster); ok { + tfresource.SetLastError(err, errors.New(aws.StringValue(output.ClusterStatus))) + return output, err } @@ -39,23 +38,17 @@ func waitClusterCreated(conn *redshift.Redshift, id string, timeout time.Duratio func waitClusterDeleted(conn *redshift.Redshift, id string, timeout time.Duration) (*redshift.Cluster, error) { stateConf := &resource.StateChangeConf{ - Pending: []string{ - clusterStatusAvailable, - clusterStatusCreating, - clusterStatusDeleting, - clusterStatusFinalSnapshot, - clusterStatusRebooting, - clusterStatusRenaming, - clusterStatusResizing, - }, + Pending: []string{clusterAvailabilityStatusModifying}, Target: []string{}, - Refresh: statusCluster(conn, id), + Refresh: statusClusterAvailability(conn, id), Timeout: timeout, } outputRaw, err := stateConf.WaitForState() if output, ok := outputRaw.(*redshift.Cluster); ok { + tfresource.SetLastError(err, errors.New(aws.StringValue(output.ClusterStatus))) + return output, err } @@ -64,24 +57,17 @@ func waitClusterDeleted(conn *redshift.Redshift, id string, timeout time.Duratio func waitClusterUpdated(conn *redshift.Redshift, id string, timeout time.Duration) (*redshift.Cluster, error) { stateConf := &resource.StateChangeConf{ - Pending: []string{ - clusterStatusAvailablePrepForResize, - clusterStatusCreating, - clusterStatusDeleting, - clusterStatusModifying, - clusterStatusRebooting, - clusterStatusRecovering, - clusterStatusRenaming, - clusterStatusResizing, - }, - Target: []string{clusterStatusAvailable}, - Refresh: statusCluster(conn, id), + Pending: []string{clusterAvailabilityStatusMaintenance, clusterAvailabilityStatusModifying, clusterAvailabilityStatusUnavailable}, + Target: []string{clusterAvailabilityStatusAvailable}, + Refresh: statusClusterAvailability(conn, id), Timeout: timeout, } outputRaw, err := stateConf.WaitForState() if output, ok := outputRaw.(*redshift.Cluster); ok { + tfresource.SetLastError(err, errors.New(aws.StringValue(output.ClusterStatus))) + return output, err }