Merge pull request #36032 from marcinbelczewski/b-aws_redshiftserverl…

…ess_workgroup-capacity-update

b/aws_redshiftserverless_workgroup: Fix base_capacity, max_capacity ordered updates and max_capacity limit removal

.changelog/36032.txt

@@ -0,0 +1,7 @@
+```release-note:bug
+resource/aws_redshiftserverless_workgroup: Fix updating both `base_capacity` and `max_capacity`
+```
+
+```release-note:bug
+resource/aws_redshiftserverless_workgroup: Fix `max_capacity` removal
+```

internal/service/redshiftserverless/workgroup.go

@@ -157,7 +157,6 @@ func ResourceWorkgroup() *schema.Resource {
  "max_capacity": {
  Type: schema.TypeInt,
  Optional: true,
- Computed: true,
  },
  "namespace_name": {
  Type: schema.TypeString,
@@ -306,15 +305,72 @@ func resourceWorkgroupUpdate(ctx context.Context, d *schema.ResourceData, meta i
  var diags diag.Diagnostics
  conn := meta.(*conns.AWSClient).RedshiftServerlessConn(ctx)
 
- // You can't update multiple parameters in one request.
-
- if d.HasChange("base_capacity") {
- input := &redshiftserverless.UpdateWorkgroupInput{
- BaseCapacity: aws.Int64(int64(d.Get("base_capacity").(int))),
- WorkgroupName: aws.String(d.Id()),
+ checkCapacityChange := func(key string) (bool, int, int) {
+ o, n := d.GetChange(key)
+ oldCapacity, newCapacity := o.(int), n.(int)
+ hasCapacityChange := newCapacity != oldCapacity
+ return hasCapacityChange, oldCapacity, newCapacity
+ }
+
+ // You can't update multiple workgroup parameters in one request.
+ // This is particularly important when adjusting base_capacity and max_capacity due to their interdependencies:
+ // - base_capacity cannot be increased to a value greater than the current max_capacity.
+ // - max_capacity cannot be decreased to a value smaller than the current base_capacity.
+ // The value 0 of max_capacity in the state signifies "not set".
+ // Sending max_capacity value of -1 to AWS API removes max_capacity limit, but -1 cannot be used as max_capacity in the state,
+ // because AWS API never returns -1 as the value of unset max_capacity. There would be a diff on subsequent apply,
+ // resulting in errors due to the lack of AWS API idempotency.
+ // Some validations, such as increasing base_capacity beyond an unchanged max_capacity, are deferred to the AWS API.
+
+ hasBaseCapacityChange, _, newBaseCapacity := checkCapacityChange("base_capacity")
+ hasMaxCapacityChange, oldMaxCapacity, newMaxCapacity := checkCapacityChange("max_capacity")
+
+ switch {
+ case hasMaxCapacityChange && newMaxCapacity == 0:
+ if err :=
+ updateWorkgroup(ctx, conn,
+ &redshiftserverless.UpdateWorkgroupInput{MaxCapacity: aws.Int64(-1), WorkgroupName: aws.String(d.Id())},
+ d.Timeout(schema.TimeoutUpdate)); err != nil {
+ return sdkdiag.AppendFromErr(diags, err)
  }
-
- if err := updateWorkgroup(ctx, conn, input, d.Timeout(schema.TimeoutUpdate)); err != nil {
+ case hasBaseCapacityChange && hasMaxCapacityChange && (oldMaxCapacity == 0 || newBaseCapacity <= oldMaxCapacity):
+ if err :=
+ updateWorkgroup(ctx, conn,
+ &redshiftserverless.UpdateWorkgroupInput{BaseCapacity: aws.Int64(int64(newBaseCapacity)), WorkgroupName: aws.String(d.Id())},
+ d.Timeout(schema.TimeoutUpdate)); err != nil {
+ return sdkdiag.AppendFromErr(diags, err)
+ }
+ if err :=
+ updateWorkgroup(ctx, conn,
+ &redshiftserverless.UpdateWorkgroupInput{MaxCapacity: aws.Int64(int64(newMaxCapacity)), WorkgroupName: aws.String(d.Id())},
+ d.Timeout(schema.TimeoutUpdate)); err != nil {
+ return sdkdiag.AppendFromErr(diags, err)
+ }
+ case hasBaseCapacityChange && hasMaxCapacityChange && newBaseCapacity > oldMaxCapacity:
+ if err :=
+ updateWorkgroup(ctx, conn,
+ &redshiftserverless.UpdateWorkgroupInput{MaxCapacity: aws.Int64(int64(newMaxCapacity)), WorkgroupName: aws.String(d.Id())},
+ d.Timeout(schema.TimeoutUpdate)); err != nil {
+ return sdkdiag.AppendFromErr(diags, err)
+ }
+ if err :=
+ updateWorkgroup(ctx, conn,
+ &redshiftserverless.UpdateWorkgroupInput{BaseCapacity: aws.Int64(int64(newBaseCapacity)), WorkgroupName: aws.String(d.Id())},
+ d.Timeout(schema.TimeoutUpdate)); err != nil {
+ return sdkdiag.AppendFromErr(diags, err)
+ }
+ case hasBaseCapacityChange:
+ if err :=
+ updateWorkgroup(ctx, conn,
+ &redshiftserverless.UpdateWorkgroupInput{BaseCapacity: aws.Int64(int64(newBaseCapacity)), WorkgroupName: aws.String(d.Id())},
+ d.Timeout(schema.TimeoutUpdate)); err != nil {
+ return sdkdiag.AppendFromErr(diags, err)
+ }
+ case hasMaxCapacityChange:
+ if err :=
+ updateWorkgroup(ctx, conn,
+ &redshiftserverless.UpdateWorkgroupInput{MaxCapacity: aws.Int64(int64(newMaxCapacity)), WorkgroupName: aws.String(d.Id())},
+ d.Timeout(schema.TimeoutUpdate)); err != nil {
  return sdkdiag.AppendFromErr(diags, err)
  }
  }
@@ -341,17 +397,6 @@ func resourceWorkgroupUpdate(ctx context.Context, d *schema.ResourceData, meta i
  }
  }
 
- if d.HasChange("max_capacity") {
- input := &redshiftserverless.UpdateWorkgroupInput{
- MaxCapacity: aws.Int64(int64(d.Get("max_capacity").(int))),
- 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("port") {
  input := &redshiftserverless.UpdateWorkgroupInput{
  Port: aws.Int64(int64(d.Get("port").(int))),

internal/service/redshiftserverless/workgroup_test.go

@@ -50,6 +50,8 @@ func TestAccRedshiftServerlessWorkgroup_basic(t *testing.T) {
  })
 }
 
+// Tests the complex logic involved in updating 'base_capacity' and 'max_capacity'.
+// The order of updates is crucial and is determined by their current state values.
 func TestAccRedshiftServerlessWorkgroup_baseAndMaxCapacityAndPubliclyAccessible(t *testing.T) {
  ctx := acctest.Context(t)
  resourceName := "aws_redshiftserverless_workgroup.test"
@@ -62,28 +64,57 @@ func TestAccRedshiftServerlessWorkgroup_baseAndMaxCapacityAndPubliclyAccessible(
  CheckDestroy: testAccCheckWorkgroupDestroy(ctx),
  Steps: []resource.TestStep{
  {
- Config: testAccWorkgroupConfig_baseAndMaxCapacityAndPubliclyAccessible(rName, 64, 128, true),
+ Config: testAccWorkgroupConfig_baseCapacity(rName, 128),
  Check: resource.ComposeTestCheckFunc(
  testAccCheckWorkgroupExists(ctx, resourceName),
+ resource.TestCheckResourceAttr(resourceName, "base_capacity", "128"),
+ resource.TestCheckResourceAttr(resourceName, "max_capacity", "0"),
+ ),
+ },
+ {
+ Config: testAccWorkgroupConfig_baseCapacity(rName, 256),
+ Check: resource.ComposeTestCheckFunc(
+ resource.TestCheckResourceAttr(resourceName, "base_capacity", "256"),
+ resource.TestCheckResourceAttr(resourceName, "max_capacity", "0"),
+ ),
+ },
+ {
+ Config: testAccWorkgroupConfig_baseAndMaxCapacityAndPubliclyAccessible(rName, 64, 128, false),
+ Check: resource.ComposeTestCheckFunc(
  resource.TestCheckResourceAttr(resourceName, "base_capacity", "64"),
  resource.TestCheckResourceAttr(resourceName, "max_capacity", "128"),
- resource.TestCheckResourceAttr(resourceName, "publicly_accessible", "true"),
  ),
  },
  {
- ResourceName: resourceName,
- ImportState: true,
- ImportStateVerify: true,
+ Config: testAccWorkgroupConfig_baseAndMaxCapacityAndPubliclyAccessible(rName, 128, 256, false),
+ Check: resource.ComposeTestCheckFunc(
+ resource.TestCheckResourceAttr(resourceName, "base_capacity", "128"),
+ resource.TestCheckResourceAttr(resourceName, "max_capacity", "256"),
+ ),
  },
  {
- Config: testAccWorkgroupConfig_baseAndMaxCapacityAndPubliclyAccessible(rName, 128, 5632, false),
+ Config: testAccWorkgroupConfig_baseAndMaxCapacityAndPubliclyAccessible(rName, 512, 5632, false),
  Check: resource.ComposeTestCheckFunc(
- testAccCheckWorkgroupExists(ctx, resourceName),
- resource.TestCheckResourceAttr(resourceName, "base_capacity", "128"),
+ resource.TestCheckResourceAttr(resourceName, "base_capacity", "512"),
  resource.TestCheckResourceAttr(resourceName, "max_capacity", "5632"),
  resource.TestCheckResourceAttr(resourceName, "publicly_accessible", "false"),
  ),
  },
+ {
+ Config: testAccWorkgroupConfig_baseAndMaxCapacityAndPubliclyAccessible(rName, 128, 256, true),
+ Check: resource.ComposeTestCheckFunc(
+ resource.TestCheckResourceAttr(resourceName, "base_capacity", "128"),
+ resource.TestCheckResourceAttr(resourceName, "max_capacity", "256"),
+ resource.TestCheckResourceAttr(resourceName, "publicly_accessible", "true"),
+ ),
+ },
+ {
+ Config: testAccWorkgroupConfig_baseCapacity(rName, 128),
+ Check: resource.ComposeTestCheckFunc(
+ resource.TestCheckResourceAttr(resourceName, "base_capacity", "128"),
+ resource.TestCheckResourceAttr(resourceName, "max_capacity", "0"),
+ ),
+ },
  },
  })
 }
@@ -361,6 +392,20 @@ resource "aws_redshiftserverless_workgroup" "test" {
 `, rName, baseCapacity, maxCapacity, publiclyAccessible)
 }
 
+func testAccWorkgroupConfig_baseCapacity(rName string, baseCapacity int) 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 = %[2]d
+}
+`, rName, baseCapacity)
+}
+
 func testAccWorkgroupConfig_configParameters(rName, maxQueryExecutionTime string) string {
  return fmt.Sprintf(`
 resource "aws_redshiftserverless_namespace" "test" {