From eb78d460e2072b0e13cd121f92f936aadd401406 Mon Sep 17 00:00:00 2001 From: Clifford Richardson Date: Wed, 15 Jan 2025 17:08:13 +0100 Subject: [PATCH 1/3] add logic for handling pricePerformanceTarget data on serverless resources --- .../service/redshiftserverless/workgroup.go | 109 ++++++++++++++++++ .../redshiftserverless/workgroup_test.go | 69 +++++++++++ 2 files changed, 178 insertions(+) diff --git a/internal/service/redshiftserverless/workgroup.go b/internal/service/redshiftserverless/workgroup.go index 9ad73c680f1..ce651c66e74 100644 --- a/internal/service/redshiftserverless/workgroup.go +++ b/internal/service/redshiftserverless/workgroup.go @@ -56,6 +56,32 @@ func resourceWorkgroup() *schema.Resource { Optional: true, Computed: true, }, + "price_performance_target": { + Type: schema.TypeList, + MaxItems: 1, + Optional: true, + Computed: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "enabled": { + Type: schema.TypeBool, + Required: true, + }, + "level": { + Type: schema.TypeString, + Default: "BALANCED", + Optional: true, + ValidateFunc: validation.StringInSlice([]string{ + "LOW_COST", + "ECONOMICAL", + "BALANCED", + "RESOURCEFUL", + "HIGH_PERFORMANCE", + }, false), + }, + }, + }, + }, "config_parameter": { Type: schema.TypeSet, Optional: true, @@ -222,6 +248,14 @@ func resourceWorkgroupCreate(ctx context.Context, d *schema.ResourceData, meta i input.BaseCapacity = aws.Int32(int32(v.(int))) } + if v, ok := d.GetOk("price_performance_target"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { + input.PricePerformanceTarget = expandPricePerformanceTarget(v.([]interface{})) + } + + if input.BaseCapacity != nil && input.PricePerformanceTarget != nil && input.PricePerformanceTarget.Status == awstypes.PerformanceTargetStatusEnabled { + return sdkdiag.AppendErrorf(diags, "base_capacity cannot be set when price_performance_target.enabled is true") + } + if v, ok := d.GetOk("config_parameter"); ok && v.(*schema.Set).Len() > 0 { input.ConfigParameters = expandConfigParameters(v.(*schema.Set).List()) } @@ -283,6 +317,9 @@ func resourceWorkgroupRead(ctx context.Context, d *schema.ResourceData, meta int d.Set(names.AttrARN, out.WorkgroupArn) d.Set("base_capacity", out.BaseCapacity) + if err := d.Set("price_performance_target", flattenPricePerformanceTarget(out.PricePerformanceTarget)); err != nil { + return sdkdiag.AppendErrorf(diags, "setting price_performance_target: %s", err) + } if err := d.Set("config_parameter", flattenConfigParameters(out.ConfigParameters)); err != nil { return sdkdiag.AppendErrorf(diags, "setting config_parameter: %s", err) } @@ -376,6 +413,17 @@ func resourceWorkgroupUpdate(ctx context.Context, d *schema.ResourceData, meta i } } + if d.HasChange("price_performance_target") { + input := &redshiftserverless.UpdateWorkgroupInput{ + PricePerformanceTarget: expandPricePerformanceTarget(d.Get("price_performance_target").([]interface{})), + WorkgroupName: aws.String(d.Id()), + } + + if err := updateWorkgroup(ctx, conn, input, d.Timeout(schema.TimeoutUpdate)); err != nil { + return sdkdiag.AppendFromErr(diags, err) + } + } + if d.HasChange("config_parameter") { input := &redshiftserverless.UpdateWorkgroupInput{ ConfigParameters: expandConfigParameters(d.Get("config_parameter").(*schema.Set).List()), @@ -582,6 +630,67 @@ func waitWorkgroupDeleted(ctx context.Context, conn *redshiftserverless.Client, return nil, err } +func expandPricePerformanceTarget(list []interface{}) *awstypes.PerformanceTarget { + if len(list) == 0 { + return nil + } + + tfMap := list[0].(map[string]interface{}) + apiObject := &awstypes.PerformanceTarget{} + // bool is a nicer way to represent the enabled/disabled state but the API expects + // a string enumeration + if enabled, ok := tfMap["enabled"].(bool); ok { + if enabled { + apiObject.Status = awstypes.PerformanceTargetStatus(awstypes.PerformanceTargetStatusEnabled) + } else { + apiObject.Status = awstypes.PerformanceTargetStatus(awstypes.PerformanceTargetStatusDisabled) + } + } + // The target price performance level for the workgroup. Valid values include 1, + // 25, 50, 75, and 100. These correspond to the price performance levels LOW_COST, + // ECONOMICAL, BALANCED, RESOURCEFUL, and HIGH_PERFORMANCE. + if level, ok := tfMap["level"].(string); ok { + switch level { + case "LOW_COST": + apiObject.Level = aws.Int32(int32(1)) + case "ECONOMICAL": + apiObject.Level = aws.Int32(int32(25)) + case "BALANCED": + apiObject.Level = aws.Int32(int32(50)) + case "RESOURCEFUL": + apiObject.Level = aws.Int32(int32(75)) + case "HIGH_PERFORMANCE": + apiObject.Level = aws.Int32(int32(100)) + } + } + + return apiObject +} + +func flattenPricePerformanceTarget(apiObject *awstypes.PerformanceTarget) []interface{} { + if apiObject == nil { + return []interface{}{} + } + + tfMap := map[string]interface{}{} + + tfMap["enabled"] = apiObject.Status == awstypes.PerformanceTargetStatusEnabled + switch aws.ToInt32(apiObject.Level) { + case 1: + tfMap["level"] = "LOW_COST" + case 25: + tfMap["level"] = "ECONOMICAL" + case 50: + tfMap["level"] = "BALANCED" + case 75: + tfMap["level"] = "RESOURCEFUL" + case 100: + tfMap["level"] = "HIGH_PERFORMANCE" + } + + return []interface{}{tfMap} +} + func expandConfigParameter(tfMap map[string]interface{}) awstypes.ConfigParameter { apiObject := awstypes.ConfigParameter{} diff --git a/internal/service/redshiftserverless/workgroup_test.go b/internal/service/redshiftserverless/workgroup_test.go index 2989a3f0b12..1951ca7471d 100644 --- a/internal/service/redshiftserverless/workgroup_test.go +++ b/internal/service/redshiftserverless/workgroup_test.go @@ -6,6 +6,7 @@ package redshiftserverless_test import ( "context" "fmt" + "regexp" "testing" "github.com/YakDriver/regexache" @@ -119,6 +120,39 @@ func TestAccRedshiftServerlessWorkgroup_baseAndMaxCapacityAndPubliclyAccessible( }) } +// Tests the logic involved in validating/updating 'base_capacity' and 'price_performance_target'. +func TestAccRedshiftServerlessWorkgroup_pricePerformanceTarget(t *testing.T) { + ctx := acctest.Context(t) + resourceName := "aws_redshiftserverless_workgroup.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.RedshiftServerlessServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckWorkgroupDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccWorkgroupConfig_pricePerformanceTargetAndBaseCapacity(rName, true), + ExpectError: regexp.MustCompile("base_capacity cannot be set when price_performance_target.enabled is true"), + }, + { + Config: testAccWorkgroupConfig_pricePerformanceTargetAndBaseCapacity(rName, false), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(resourceName, "base_capacity", "128"), + resource.TestCheckResourceAttr(resourceName, names.AttrMaxCapacity, "0"), + ), + }, + { + Config: testAccWorkgroupConfig_pricePerformanceTarget(rName, "LOW_COST"), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(resourceName, "price_performance_target.0.enabled", acctest.CtTrue), + resource.TestCheckResourceAttr(resourceName, "price_performance_target.0.level", "LOW_COST"), + ), + }, + }, + }) +} + func TestAccRedshiftServerlessWorkgroup_configParameters(t *testing.T) { ctx := acctest.Context(t) resourceName := "aws_redshiftserverless_workgroup.test" @@ -407,6 +441,41 @@ resource "aws_redshiftserverless_workgroup" "test" { `, rName, baseCapacity) } +func testAccWorkgroupConfig_pricePerformanceTarget(rName string, targetLevel string) string { + return fmt.Sprintf(` +resource "aws_redshiftserverless_namespace" "test" { + namespace_name = %[1]q +} + +resource "aws_redshiftserverless_workgroup" "test" { + namespace_name = aws_redshiftserverless_namespace.test.namespace_name + workgroup_name = %[1]q + price_performance_target { + enabled = true + level = %[2]q + } +} + +`, rName, targetLevel) +} + +func testAccWorkgroupConfig_pricePerformanceTargetAndBaseCapacity(rName string, pricePerformanceEnabled bool) string { + return fmt.Sprintf(` +resource "aws_redshiftserverless_namespace" "test" { + namespace_name = %[1]q +} + +resource "aws_redshiftserverless_workgroup" "test" { + namespace_name = aws_redshiftserverless_namespace.test.namespace_name + workgroup_name = %[1]q + base_capacity = 128 + price_performance_target { + enabled = %[2]t + } +} +`, rName, pricePerformanceEnabled) +} + func testAccWorkgroupConfig_configParameters(rName, maxQueryExecutionTime string) string { return fmt.Sprintf(` resource "aws_redshiftserverless_namespace" "test" { From 2e585a252d1c68565a3a6140cf21162ed6f7e331 Mon Sep 17 00:00:00 2001 From: Clifford Richardson Date: Thu, 16 Jan 2025 10:05:47 +0100 Subject: [PATCH 2/3] update documentation and add changelog entry for PR --- .changelog/40915.txt | 3 +++ website/docs/r/redshiftserverless_workgroup.html.markdown | 6 ++++++ 2 files changed, 9 insertions(+) create mode 100644 .changelog/40915.txt diff --git a/.changelog/40915.txt b/.changelog/40915.txt new file mode 100644 index 00000000000..a37e87c153a --- /dev/null +++ b/.changelog/40915.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +resource/aws_redshiftserverless_workgroup: Add price_performance_target argument +``` diff --git a/website/docs/r/redshiftserverless_workgroup.html.markdown b/website/docs/r/redshiftserverless_workgroup.html.markdown index 124d8592b5c..2dd89334031 100644 --- a/website/docs/r/redshiftserverless_workgroup.html.markdown +++ b/website/docs/r/redshiftserverless_workgroup.html.markdown @@ -29,6 +29,7 @@ The following arguments are required: The following arguments are optional: * `base_capacity` - (Optional) The base data warehouse capacity of the workgroup in Redshift Processing Units (RPUs). +* `price_performance_target` - (Optional) Price-performance scaling for the workgroup. See `Price Performance Target` below. * `config_parameter` - (Optional) An array of parameters to set for more control over a serverless database. See `Config Parameter` below. * `enhanced_vpc_routing` - (Optional) The value that specifies whether to turn on enhanced virtual private cloud (VPC) routing, which forces Amazon Redshift Serverless to route traffic through your VPC instead of over the internet. * `max_capacity` - (Optional) The maximum data-warehouse capacity Amazon Redshift Serverless uses to serve queries, specified in Redshift Processing Units (RPUs). @@ -38,6 +39,11 @@ The following arguments are optional: * `subnet_ids` - (Optional) An array of VPC subnet IDs to associate with the workgroup. When set, must contain at least three subnets spanning three Availability Zones. A minimum number of IP addresses is required and scales with the Base Capacity. For more information, see the following [AWS document](https://docs.aws.amazon.com/redshift/latest/mgmt/serverless-known-issues.html). * `tags` - (Optional) A map of tags to assign to the resource. If configured with a provider [`default_tags` configuration block](https://registry.terraform.io/providers/hashicorp/aws/latest/docs#default_tags-configuration-block) present, tags with matching keys will overwrite those defined at the provider-level. +### Price Performance Target + +* `enabled` - (Required) Whether to enable price-performance scaling. +* `level` - (Required) The price-performance scaling level. Valid values are `LOW_COST`, `ECONOMICAL`, `BALANCED`, `RESOURCEFUL`, and `HIGH_PERFORMANCE`. + ### Config Parameter * `parameter_key` - (Required) The key of the parameter. The options are `auto_mv`, `datestyle`, `enable_case_sensitive_identifier`, `enable_user_activity_logging`, `query_group`, `search_path`, `require_ssl`, `use_fips_ssl`, and [query monitoring metrics](https://docs.aws.amazon.com/redshift/latest/dg/cm-c-wlm-query-monitoring-rules.html#cm-c-wlm-query-monitoring-metrics-serverless) that let you define performance boundaries: `max_query_cpu_time`, `max_query_blocks_read`, `max_scan_row_count`, `max_query_execution_time`, `max_query_queue_time`, `max_query_cpu_usage_percent`, `max_query_temp_blocks_to_disk`, `max_join_row_count` and `max_nested_loop_join_row_count`. From 2d5a1b0b0d54d8ecdf62756a0731bd41ab400bfd Mon Sep 17 00:00:00 2001 From: Clifford Richardson Date: Thu, 16 Jan 2025 23:15:58 +0100 Subject: [PATCH 3/3] fix minor linting issues --- internal/service/redshiftserverless/consts.go | 27 ++++++++ .../service/redshiftserverless/workgroup.go | 64 +++++++++---------- .../redshiftserverless/workgroup_test.go | 3 +- 3 files changed, 57 insertions(+), 37 deletions(-) create mode 100644 internal/service/redshiftserverless/consts.go diff --git a/internal/service/redshiftserverless/consts.go b/internal/service/redshiftserverless/consts.go new file mode 100644 index 00000000000..36fef7f02f6 --- /dev/null +++ b/internal/service/redshiftserverless/consts.go @@ -0,0 +1,27 @@ +package redshiftserverless + +const ( + performanceTargetLevelLowCostValue = 1 + performanceTargetLevelEconomicalValue = 25 + performanceTargetLevelBalancedValue = 50 + performanceTargetLevelResourcefulValue = 75 + performanceTargetLevelHighPerformanceValue = 100 +) + +const ( + performanceTargetLevelLowCost = "LOW_COST" + performanceTargetLevelEconomical = "ECONOMICAL" + performanceTargetLevelBalanced = "BALANCED" + performanceTargetLevelResourceful = "RESOURCEFUL" + performanceTargetLevelHighPerformance = "HIGH_PERFORMANCE" +) + +func performanceTargetLevel_Values() []string { + return []string{ + performanceTargetLevelLowCost, + performanceTargetLevelEconomical, + performanceTargetLevelBalanced, + performanceTargetLevelResourceful, + performanceTargetLevelHighPerformance, + } +} diff --git a/internal/service/redshiftserverless/workgroup.go b/internal/service/redshiftserverless/workgroup.go index ce651c66e74..51062d8f024 100644 --- a/internal/service/redshiftserverless/workgroup.go +++ b/internal/service/redshiftserverless/workgroup.go @@ -63,21 +63,15 @@ func resourceWorkgroup() *schema.Resource { Computed: true, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ - "enabled": { + names.AttrEnabled: { Type: schema.TypeBool, Required: true, }, "level": { - Type: schema.TypeString, - Default: "BALANCED", - Optional: true, - ValidateFunc: validation.StringInSlice([]string{ - "LOW_COST", - "ECONOMICAL", - "BALANCED", - "RESOURCEFUL", - "HIGH_PERFORMANCE", - }, false), + Type: schema.TypeString, + Default: performanceTargetLevelBalanced, + Optional: true, + ValidateFunc: validation.StringInSlice(performanceTargetLevel_Values(), false), }, }, }, @@ -639,11 +633,11 @@ func expandPricePerformanceTarget(list []interface{}) *awstypes.PerformanceTarge apiObject := &awstypes.PerformanceTarget{} // bool is a nicer way to represent the enabled/disabled state but the API expects // a string enumeration - if enabled, ok := tfMap["enabled"].(bool); ok { + if enabled, ok := tfMap[names.AttrEnabled].(bool); ok { if enabled { - apiObject.Status = awstypes.PerformanceTargetStatus(awstypes.PerformanceTargetStatusEnabled) + apiObject.Status = awstypes.PerformanceTargetStatusEnabled } else { - apiObject.Status = awstypes.PerformanceTargetStatus(awstypes.PerformanceTargetStatusDisabled) + apiObject.Status = awstypes.PerformanceTargetStatusDisabled } } // The target price performance level for the workgroup. Valid values include 1, @@ -651,16 +645,16 @@ func expandPricePerformanceTarget(list []interface{}) *awstypes.PerformanceTarge // ECONOMICAL, BALANCED, RESOURCEFUL, and HIGH_PERFORMANCE. if level, ok := tfMap["level"].(string); ok { switch level { - case "LOW_COST": - apiObject.Level = aws.Int32(int32(1)) - case "ECONOMICAL": - apiObject.Level = aws.Int32(int32(25)) - case "BALANCED": - apiObject.Level = aws.Int32(int32(50)) - case "RESOURCEFUL": - apiObject.Level = aws.Int32(int32(75)) - case "HIGH_PERFORMANCE": - apiObject.Level = aws.Int32(int32(100)) + case performanceTargetLevelLowCost: + apiObject.Level = aws.Int32(int32(performanceTargetLevelLowCostValue)) + case performanceTargetLevelEconomical: + apiObject.Level = aws.Int32(int32(performanceTargetLevelEconomicalValue)) + case performanceTargetLevelBalanced: + apiObject.Level = aws.Int32(int32(performanceTargetLevelBalancedValue)) + case performanceTargetLevelResourceful: + apiObject.Level = aws.Int32(int32(performanceTargetLevelResourcefulValue)) + case performanceTargetLevelHighPerformance: + apiObject.Level = aws.Int32(int32(performanceTargetLevelHighPerformanceValue)) } } @@ -674,18 +668,18 @@ func flattenPricePerformanceTarget(apiObject *awstypes.PerformanceTarget) []inte tfMap := map[string]interface{}{} - tfMap["enabled"] = apiObject.Status == awstypes.PerformanceTargetStatusEnabled + tfMap[names.AttrEnabled] = apiObject.Status == awstypes.PerformanceTargetStatusEnabled switch aws.ToInt32(apiObject.Level) { - case 1: - tfMap["level"] = "LOW_COST" - case 25: - tfMap["level"] = "ECONOMICAL" - case 50: - tfMap["level"] = "BALANCED" - case 75: - tfMap["level"] = "RESOURCEFUL" - case 100: - tfMap["level"] = "HIGH_PERFORMANCE" + case performanceTargetLevelLowCostValue: + tfMap["level"] = performanceTargetLevelLowCost + case performanceTargetLevelEconomicalValue: + tfMap["level"] = performanceTargetLevelEconomical + case performanceTargetLevelBalancedValue: + tfMap["level"] = performanceTargetLevelBalanced + case performanceTargetLevelResourcefulValue: + tfMap["level"] = performanceTargetLevelResourceful + case performanceTargetLevelHighPerformanceValue: + tfMap["level"] = performanceTargetLevelHighPerformance } return []interface{}{tfMap} diff --git a/internal/service/redshiftserverless/workgroup_test.go b/internal/service/redshiftserverless/workgroup_test.go index 1951ca7471d..99d91f3da19 100644 --- a/internal/service/redshiftserverless/workgroup_test.go +++ b/internal/service/redshiftserverless/workgroup_test.go @@ -6,7 +6,6 @@ package redshiftserverless_test import ( "context" "fmt" - "regexp" "testing" "github.com/YakDriver/regexache" @@ -133,7 +132,7 @@ func TestAccRedshiftServerlessWorkgroup_pricePerformanceTarget(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccWorkgroupConfig_pricePerformanceTargetAndBaseCapacity(rName, true), - ExpectError: regexp.MustCompile("base_capacity cannot be set when price_performance_target.enabled is true"), + ExpectError: regexache.MustCompile("base_capacity cannot be set when price_performance_target.enabled is true"), }, { Config: testAccWorkgroupConfig_pricePerformanceTargetAndBaseCapacity(rName, false),