-
Notifications
You must be signed in to change notification settings - Fork 9.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Redshift cluster AZ relocation support #20812
Redshift cluster AZ relocation support #20812
Conversation
I've reimplemented/simplified the read functionality based on how other resources already implemented are dealing with the same scenario.
|
Pull request #21306 has significantly refactored the AWS Provider codebase. As a result, most PRs opened prior to the refactor now have merge conflicts that must be resolved before proceeding. Specifically, PR #21306 relocated the code for all AWS resources and data sources from a single We recognize that many pull requests have been open for some time without yet being addressed by our maintainers. Therefore, we want to make it clear that resolving these conflicts in no way affects the prioritization of a particular pull request. Once a pull request has been prioritized for review, the necessary changes will be made by a maintainer -- either directly or in collaboration with the pull request author. For a more complete description of this refactor, including examples of how old filepaths and function names correspond to their new counterparts: please refer to issue #20000. For a quick guide on how to amend your pull request to resolve the merge conflicts resulting from this refactor and bring it in line with our new code patterns: please refer to our Service Package Refactor Pull Request Guide. |
…corp#19098 # Conflicts: # aws/resource_aws_redshift_cluster.go # aws/resource_aws_redshift_cluster_test.go
…_relocation attribute
…ew(fmt.Sprintf(...))
…on_enabled` for clarity
…ith `availability_zone_relocation_enabled`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for submitting this, @nickyamanaka. 🚀
I've made a couple changes to the PR:
availability_zone_relocation
andavailability_zone_relocation_status
are essentially the same field, one for input and one for output because it can be in a pending state. I've combined them and renamedavailability_zone_relocation
toavailability_zone_relocation_enabled
for clarity- Since
availability_zone_relocation
enables relocation, but doesn't cause a relocation, I've added tests for relocating the cluster instead of recreating the cluster as it did previously. I also discovered that no other modifications can be made during relocation, so I updated that as well.
--- PASS: TestAccRedshiftCluster_availabilityZoneRelocation_publiclyAccessible (78.25s)
--- PASS: TestAccRedshiftCluster_disappears (408.94s)
--- PASS: TestAccRedshiftClusterDataSource_basic (422.33s)
--- PASS: TestAccRedshiftCluster_basic (456.92s)
--- PASS: TestAccRedshiftClusterDataSource_logging (464.35s)
--- PASS: TestAccRedshiftCluster_kmsKey (500.20s)
--- PASS: TestAccRedshiftCluster_snapshotCopy (510.77s)
--- PASS: TestAccRedshiftCluster_withFinalSnapshot (524.53s)
--- PASS: TestAccRedshiftCluster_loggingEnabled (531.72s)
--- PASS: TestAccRedshiftCluster_forceNewUsername (551.24s)
--- PASS: TestAccRedshiftCluster_iamRoles (556.87s)
--- PASS: TestAccRedshiftCluster_tags (605.85s)
--- PASS: TestAccRedshiftCluster_enhancedVPCRoutingEnabled (618.59s)
--- PASS: TestAccRedshiftCluster_publiclyAccessible (622.10s)
--- PASS: TestAccRedshiftClusterDataSource_availabilityZoneRelocationEnabled (572.44s)
--- PASS: TestAccRedshiftClusterDataSource_vpc (277.32s)
--- PASS: TestAccRedshiftCluster_availabilityZoneRelocation (780.50s)
--- PASS: TestAccRedshiftCluster_changeAvailabilityZone_availabilityZoneRelocationNotSet (420.35s)
--- PASS: TestAccRedshiftCluster_changeAvailabilityZone (833.09s)
--- PASS: TestAccRedshiftCluster_changeAvailabilityZoneAndSetAvailabilityZoneRelocation (676.53s)
--- PASS: TestAccRedshiftCluster_changeEncryption2 (1303.43s)
--- PASS: TestAccRedshiftCluster_updateNodeType (1340.54s)
--- PASS: TestAccRedshiftCluster_updateNodeCount (1506.16s)
--- PASS: TestAccRedshiftCluster_changeEncryption1 (1855.43s)
This functionality has been released in v4.6.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
Updated for new structure
Adds support for Cluster Relocation.
https://aws.amazon.com/about-aws/whats-new/2020/12/amazon-redshift-launches-ability-easily-move-clusters-between-aws-availability-zones/
https://github.com/aws/aws-sdk-go/blob/main/models/apis/redshift/2012-12-01/api-2.json#L2432
https://github.com/aws/aws-sdk-go/blob/main/models/apis/redshift/2012-12-01/api-2.json#L2952
https://github.com/aws/aws-sdk-go/blob/main/models/apis/redshift/2012-12-01/api-2.json#L4904
https://github.com/aws/aws-sdk-go/blob/main/models/apis/redshift/2012-12-01/api-2.json#L5614
Notes for the tests:
Notes for the API:
AvailabilityZoneRelocation
is not available when describing clusters, I've usedAvailabilityZoneRelocationStatus
and inferring the status of the former for plans and imports. The API is official but not documented properly, but it seems to have 4 statuses from my own research, I've added a fallback to the state and default if the API returns something else. I would love to have feedback regarding that piece of the implementation, and advice for how to structure properly (if not) when the reads from the API are not 1:1 translatable to create/update.Community Note
Closes #19098
Output from acceptance testing: