Skip to content

Commit

Permalink
Merge pull request #20812 from nickyamanaka/f-redshift-cluster-az-rel…
Browse files Browse the repository at this point in the history
…ocation-issue-#19098

Redshift cluster AZ relocation support
  • Loading branch information
gdavison authored Mar 12, 2022
2 parents 97dd8fe + eaeeec7 commit 29acd34
Show file tree
Hide file tree
Showing 10 changed files with 466 additions and 60 deletions.
7 changes: 7 additions & 0 deletions .changelog/20812.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
```release-note:enhancement
resource/aws_redshift_cluster: Add `availability_zone_relocation_enabled` attribute and allow `availability_zone` to be changed in-place.
```

```release-note:enhancement
data_source/aws_redshift_cluster: Add `availability_zone_relocation_enabled` attribute.
```
127 changes: 105 additions & 22 deletions internal/service/redshift/cluster.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package redshift

import (
"context"
"errors"
"fmt"
"log"
"regexp"
Expand All @@ -11,6 +13,7 @@ import (
"github.com/aws/aws-sdk-go/aws/arn"
"github.com/aws/aws-sdk-go/service/redshift"
"github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
Expand Down Expand Up @@ -56,9 +59,12 @@ func ResourceCluster() *schema.Resource {
"availability_zone": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
Computed: true,
},
"availability_zone_relocation_enabled": {
Type: schema.TypeBool,
Optional: true,
},
"cluster_identifier": {
Type: schema.TypeString,
Required: true,
Expand Down Expand Up @@ -313,7 +319,28 @@ func ResourceCluster() *schema.Resource {
"tags_all": tftags.TagsSchemaComputed(),
},

CustomizeDiff: verify.SetTagsDiff,
CustomizeDiff: customdiff.All(
verify.SetTagsDiff,
func(_ context.Context, diff *schema.ResourceDiff, v interface{}) error {
if diff.Get("availability_zone_relocation_enabled").(bool) && diff.Get("publicly_accessible").(bool) {
return errors.New("availability_zone_relocation_enabled can not be true when publicly_accessible is true")
}
return nil
},
func(_ context.Context, diff *schema.ResourceDiff, v interface{}) error {
if diff.Id() == "" {
return nil
}
if diff.Get("availability_zone_relocation_enabled").(bool) {
return nil
}
o, n := diff.GetChange("availability_zone")
if o.(string) != n.(string) {
return fmt.Errorf("cannot change availability_zone if availability_zone_relocation_enabled is not true")
}
return nil
},
),
}
}

Expand Down Expand Up @@ -354,6 +381,10 @@ func resourceClusterCreate(d *schema.ResourceData, meta interface{}) error {
restoreOpts.AvailabilityZone = aws.String(v.(string))
}

if v, ok := d.GetOk("availability_zone_relocation_enabled"); ok {
restoreOpts.AvailabilityZoneRelocation = aws.Bool(v.(bool))
}

if v, ok := d.GetOk("cluster_subnet_group_name"); ok {
restoreOpts.ClusterSubnetGroupName = aws.String(v.(string))
}
Expand Down Expand Up @@ -394,8 +425,7 @@ func resourceClusterCreate(d *schema.ResourceData, meta interface{}) error {

resp, err := conn.RestoreFromClusterSnapshot(restoreOpts)
if err != nil {
log.Printf("[ERROR] Error Restoring Redshift Cluster from Snapshot: %s", err)
return err
return fmt.Errorf("error restoring Redshift Cluster from snapshot: %w", err)
}

d.SetId(aws.StringValue(resp.Cluster.ClusterIdentifier))
Expand Down Expand Up @@ -446,6 +476,10 @@ func resourceClusterCreate(d *schema.ResourceData, meta interface{}) error {
createOpts.AvailabilityZone = aws.String(v.(string))
}

if v, ok := d.GetOk("availability_zone_relocation_enabled"); ok {
createOpts.AvailabilityZoneRelocation = aws.Bool(v.(bool))
}

if v, ok := d.GetOk("preferred_maintenance_window"); ok {
createOpts.PreferredMaintenanceWindow = aws.String(v.(string))
}
Expand Down Expand Up @@ -477,8 +511,7 @@ func resourceClusterCreate(d *schema.ResourceData, meta interface{}) error {
log.Printf("[DEBUG] Redshift Cluster create options: %s", createOpts)
resp, err := conn.CreateCluster(createOpts)
if err != nil {
log.Printf("[ERROR] Error creating Redshift Cluster: %s", err)
return err
return fmt.Errorf("error creating Redshift Cluster: %w", err)
}

log.Printf("[DEBUG]: Cluster create response: %s", resp)
Expand All @@ -492,10 +525,14 @@ func resourceClusterCreate(d *schema.ResourceData, meta interface{}) error {
Timeout: d.Timeout(schema.TimeoutCreate),
MinTimeout: 10 * time.Second,
}

_, err := stateConf.WaitForState()
if err != nil {
return fmt.Errorf("Error waiting for Redshift Cluster state to be \"available\": %s", err)
return fmt.Errorf("Error waiting for Redshift Cluster state to be \"available\": %w", err)
}

_, err = waitClusterRelocationStatusResolved(conn, d.Id())
if err != nil {
return fmt.Errorf("error waiting for Redshift Cluster Availability Zone Relocation Status to resolve: %w", err)
}

if v, ok := d.GetOk("snapshot_copy"); ok {
Expand All @@ -507,7 +544,7 @@ func resourceClusterCreate(d *schema.ResourceData, meta interface{}) error {

if _, ok := d.GetOk("logging.0.enable"); ok {
if err := enableRedshiftClusterLogging(d, conn); err != nil {
return fmt.Errorf("error enabling Redshift Cluster (%s) logging: %s", d.Id(), err)
return fmt.Errorf("error enabling Redshift Cluster (%s) logging: %w", d.Id(), err)
}
}

Expand Down Expand Up @@ -550,6 +587,11 @@ func resourceClusterRead(d *schema.ResourceData, meta interface{}) error {
d.Set("arn", arn)
d.Set("automated_snapshot_retention_period", rsc.AutomatedSnapshotRetentionPeriod)
d.Set("availability_zone", rsc.AvailabilityZone)
azr, err := clusterAvailabilityZoneRelocationStatus(rsc)
if err != nil {
return fmt.Errorf("error reading Redshift Cluster (%s): %w", d.Id(), err)
}
d.Set("availability_zone_relocation_enabled", azr)
d.Set("cluster_identifier", rsc.ClusterIdentifier)
if err := d.Set("cluster_nodes", flattenRedshiftClusterNodes(rsc.ClusterNodes)); err != nil {
return fmt.Errorf("error setting cluster_nodes: %w", err)
Expand Down Expand Up @@ -661,6 +703,11 @@ func resourceClusterUpdate(d *schema.ResourceData, meta interface{}) error {
requestUpdate = true
}

if d.HasChange("availability_zone_relocation_enabled") {
req.AvailabilityZoneRelocation = aws.Bool(d.Get("availability_zone_relocation_enabled").(bool))
requestUpdate = true
}

if d.HasChange("cluster_security_groups") {
req.ClusterSecurityGroups = flex.ExpandStringSet(d.Get("cluster_security_groups").(*schema.Set))
requestUpdate = true
Expand Down Expand Up @@ -722,11 +769,10 @@ func resourceClusterUpdate(d *schema.ResourceData, meta interface{}) error {
}

if requestUpdate {
log.Printf("[INFO] Modifying Redshift Cluster: %s", d.Id())
log.Printf("[DEBUG] Redshift Cluster Modify options: %s", req)
log.Printf("[DEBUG] Modifying Redshift Cluster: %s", d.Id())
_, err := conn.ModifyCluster(req)
if err != nil {
return fmt.Errorf("Error modifying Redshift Cluster (%s): %s", d.Id(), err)
return fmt.Errorf("Error modifying Redshift Cluster (%s): %w", d.Id(), err)
}
}

Expand All @@ -745,35 +791,60 @@ func resourceClusterUpdate(d *schema.ResourceData, meta interface{}) error {
removeIams := os.Difference(ns)
addIams := ns.Difference(os)

log.Printf("[INFO] Building Redshift Modify Cluster IAM Role Options")
req := &redshift.ModifyClusterIamRolesInput{
ClusterIdentifier: aws.String(d.Id()),
AddIamRoles: flex.ExpandStringSet(addIams),
RemoveIamRoles: flex.ExpandStringSet(removeIams),
}

log.Printf("[INFO] Modifying Redshift Cluster IAM Roles: %s", d.Id())
log.Printf("[DEBUG] Redshift Cluster Modify IAM Role options: %s", req)
log.Printf("[DEBUG] Modifying Redshift Cluster IAM Roles: %s", d.Id())
_, err := conn.ModifyClusterIamRoles(req)
if err != nil {
return fmt.Errorf("Error modifying Redshift Cluster IAM Roles (%s): %s", d.Id(), err)
return fmt.Errorf("Error modifying Redshift Cluster IAM Roles (%s): %w", d.Id(), err)
}
}

if requestUpdate || d.HasChange("iam_roles") {

stateConf := &resource.StateChangeConf{
Pending: []string{"creating", "deleting", "rebooting", "resizing", "renaming", "modifying", "available, prep-for-resize"},
Target: []string{"available"},
Refresh: resourceClusterStateRefreshFunc(d.Id(), conn),
Timeout: d.Timeout(schema.TimeoutUpdate),
MinTimeout: 10 * time.Second,
}

// Wait, catching any errors
_, err := stateConf.WaitForState()
if err != nil {
return fmt.Errorf("Error Modifying Redshift Cluster (%s): %s", d.Id(), err)
return fmt.Errorf("Error waiting for Redshift Cluster modification (%s): %w", d.Id(), err)
}

_, err = waitClusterRelocationStatusResolved(conn, d.Id())
if err != nil {
return fmt.Errorf("error waiting for Redshift Cluster Availability Zone Relocation Status to resolve: %w", err)
}
}

// Availability Zone cannot be changed at the same time as other settings
if d.HasChange("availability_zone") {
req := &redshift.ModifyClusterInput{
ClusterIdentifier: aws.String(d.Id()),
AvailabilityZone: aws.String(d.Get("availability_zone").(string)),
}
log.Printf("[DEBUG] Relocating Redshift Cluster: %s", d.Id())
_, err := conn.ModifyCluster(req)
if err != nil {
return fmt.Errorf("Error relocating Redshift Cluster (%s): %w", d.Id(), err)
}

stateConf := &resource.StateChangeConf{
Pending: []string{"creating", "deleting", "rebooting", "resizing", "renaming", "modifying", "available, prep-for-resize", "recovering"},
Target: []string{"available"},
Refresh: resourceClusterStateRefreshFunc(d.Id(), conn),
Timeout: d.Timeout(schema.TimeoutUpdate),
MinTimeout: 10 * time.Second,
}
_, err = stateConf.WaitForState()
if err != nil {
return fmt.Errorf("Error waiting for Redshift Cluster relocation (%s): %w", d.Id(), err)
}
}

Expand All @@ -788,7 +859,7 @@ func resourceClusterUpdate(d *schema.ResourceData, meta interface{}) error {
ClusterIdentifier: aws.String(d.Id()),
})
if err != nil {
return fmt.Errorf("Failed to disable snapshot copy: %s", err)
return fmt.Errorf("Failed to disable snapshot copy: %w", err)
}
}
}
Expand Down Expand Up @@ -868,7 +939,7 @@ func enableRedshiftSnapshotCopy(id string, scList []interface{}, conn *redshift.

_, err := conn.EnableSnapshotCopy(&input)
if err != nil {
return fmt.Errorf("Failed to enable snapshot copy: %s", err)
return fmt.Errorf("Failed to enable snapshot copy: %w", err)
}
return nil
}
Expand Down Expand Up @@ -990,3 +1061,15 @@ func flattenRedshiftClusterNodes(apiObjects []*redshift.ClusterNode) []interface

return tfList
}

func clusterAvailabilityZoneRelocationStatus(cluster *redshift.Cluster) (bool, error) {
// AvailabilityZoneRelocation is not returned by the API, and AvailabilityZoneRelocationStatus is not implemented as Const at this time.
switch availabilityZoneRelocationStatus := aws.StringValue(cluster.AvailabilityZoneRelocationStatus); availabilityZoneRelocationStatus {
case "enabled":
return true, nil
case "disabled":
return false, nil
default:
return false, fmt.Errorf("unexpected AvailabilityZoneRelocationStatus value %q returned by API", availabilityZoneRelocationStatus)
}
}
10 changes: 10 additions & 0 deletions internal/service/redshift/cluster_data_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ func DataSourceCluster() *schema.Resource {
Computed: true,
},

"availability_zone_relocation_enabled": {
Type: schema.TypeBool,
Computed: true,
},

"bucket_name": {
Type: schema.TypeString,
Computed: true,
Expand Down Expand Up @@ -196,6 +201,11 @@ func dataSourceClusterRead(d *schema.ResourceData, meta interface{}) error {
d.Set("allow_version_upgrade", rsc.AllowVersionUpgrade)
d.Set("automated_snapshot_retention_period", rsc.AutomatedSnapshotRetentionPeriod)
d.Set("availability_zone", rsc.AvailabilityZone)
azr, err := clusterAvailabilityZoneRelocationStatus(rsc)
if err != nil {
return fmt.Errorf("error reading Redshift Cluster (%s): %w", d.Id(), err)
}
d.Set("availability_zone_relocation_enabled", azr)
d.Set("cluster_identifier", rsc.ClusterIdentifier)

if len(rsc.ClusterParameterGroups) > 0 {
Expand Down
44 changes: 44 additions & 0 deletions internal/service/redshift/cluster_data_source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

func TestAccRedshiftClusterDataSource_basic(t *testing.T) {
dataSourceName := "data.aws_redshift_cluster.test"
resourceName := "aws_redshift_cluster.test"
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)

resource.ParallelTest(t, resource.TestCase{
Expand Down Expand Up @@ -40,6 +41,7 @@ func TestAccRedshiftClusterDataSource_basic(t *testing.T) {
resource.TestCheckResourceAttrSet(dataSourceName, "port"),
resource.TestCheckResourceAttrSet(dataSourceName, "preferred_maintenance_window"),
resource.TestCheckResourceAttrSet(dataSourceName, "publicly_accessible"),
resource.TestCheckResourceAttrPair(dataSourceName, "availability_zone_relocation_enabled", resourceName, "availability_zone_relocation_enabled"),
),
},
},
Expand Down Expand Up @@ -91,6 +93,26 @@ func TestAccRedshiftClusterDataSource_logging(t *testing.T) {
})
}

func TestAccRedshiftClusterDataSource_availabilityZoneRelocationEnabled(t *testing.T) {
dataSourceName := "data.aws_redshift_cluster.test"
resourceName := "aws_redshift_cluster.test"
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t) },
ErrorCheck: acctest.ErrorCheck(t, redshift.EndpointsID),
Providers: acctest.Providers,
Steps: []resource.TestStep{
{
Config: testAccClusterDataSourceConfig_availabilityZoneRelocationEnabled(rName),
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttrPair(dataSourceName, "availability_zone_relocation_enabled", resourceName, "availability_zone_relocation_enabled"),
),
},
},
})
}

func testAccClusterDataSourceConfig(rName string) string {
return fmt.Sprintf(`
resource "aws_redshift_cluster" "test" {
Expand Down Expand Up @@ -234,3 +256,25 @@ data "aws_redshift_cluster" "test" {
}
`, rName)
}

func testAccClusterDataSourceConfig_availabilityZoneRelocationEnabled(rName string) string {
return fmt.Sprintf(`
resource "aws_redshift_cluster" "test" {
cluster_identifier = %[1]q
database_name = "testdb"
master_username = "foo"
master_password = "Password1"
node_type = "ra3.xlplus"
cluster_type = "single-node"
skip_final_snapshot = true
publicly_accessible = false
availability_zone_relocation_enabled = true
}
data "aws_redshift_cluster" "test" {
cluster_identifier = aws_redshift_cluster.test.cluster_identifier
}
`, rName)
}
Loading

0 comments on commit 29acd34

Please sign in to comment.