Skip to content

Commit 7b00831

Browse files
johnsonajjohnsonaj
authored
Merge pull request #43533 from tabito-hara/f-aws_ecs_service-fix_updating_capacity_provider
aws_ecs_service: Fix behavior when updating `capacity_provider_strategy` to avoid ECS service recreation after recent AWS changes
2 parents 42d7b7d + 8284866 commit 7b00831

File tree

4 files changed

+45
-29
lines changed

4 files changed

+45
-29
lines changed

‎.changelog/43533.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
```release-note:breaking-change
2+
resource/aws_ecs_service: Fix behavior when updating `capacity_provider_strategy` to avoid ECS service recreation after recent AWS changes
3+
```

‎internal/service/ecs/service.go‎

Lines changed: 5 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2424,31 +2424,11 @@ func triggersCustomizeDiff(_ context.Context, d *schema.ResourceDiff, meta any)
24242424
}
24252425

24262426
func capacityProviderStrategyCustomizeDiff(_ context.Context, d *schema.ResourceDiff, meta any) error {
2427-
// to be backward compatible, should ForceNew almost always (previous behavior), unless:
2428-
// force_new_deployment is true and
2429-
// neither the old set nor new set is 0 length
2430-
if v := d.Get("force_new_deployment").(bool); !v {
2431-
return capacityProviderStrategyForceNew(d)
2432-
}
2433-
2434-
old, new := d.GetChange(names.AttrCapacityProviderStrategy)
2435-
2436-
ol := old.(*schema.Set).Len()
2437-
nl := new.(*schema.Set).Len()
2438-
2439-
if (ol == 0 && nl > 0) || (ol > 0 && nl == 0) {
2440-
return capacityProviderStrategyForceNew(d)
2441-
}
2442-
2443-
return nil
2444-
}
2445-
2446-
func capacityProviderStrategyForceNew(d *schema.ResourceDiff) error {
2447-
for _, key := range d.GetChangedKeysPrefix(names.AttrCapacityProviderStrategy) {
2448-
if d.HasChange(key) {
2449-
if err := d.ForceNew(key); err != nil {
2450-
return fmt.Errorf("while attempting to force a new ECS service for capacity_provider_strategy: %w", err)
2451-
}
2427+
// This if-statement is true only when the resource is being updated.
2428+
// d.Id() != "" means the resource (ecs service) already exists.
2429+
if d.Id() != "" && d.HasChange(names.AttrCapacityProviderStrategy) {
2430+
if v := d.Get("force_new_deployment").(bool); !v {
2431+
return fmt.Errorf("force_new_deployment should be true when capacity_provider_strategy is being updated")
24522432
}
24532433
}
24542434
return nil

‎internal/service/ecs/service_test.go‎

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -380,13 +380,33 @@ func TestAccECSService_CapacityProviderStrategy_basic(t *testing.T) {
380380
Config: testAccServiceConfig_capacityProviderStrategy(rName, 1, 0, false),
381381
Check: resource.ComposeTestCheckFunc(
382382
testAccCheckServiceExists(ctx, resourceName, &service),
383+
resource.TestCheckResourceAttr(resourceName, "capacity_provider_strategy.#", "1"),
384+
resource.TestCheckResourceAttr(resourceName, "capacity_provider_strategy.0.weight", "1"),
385+
resource.TestCheckResourceAttr(resourceName, "capacity_provider_strategy.0.base", "0"),
383386
),
387+
ConfigPlanChecks: resource.ConfigPlanChecks{
388+
PreApply: []plancheck.PlanCheck{
389+
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionCreate),
390+
},
391+
},
392+
},
393+
{
394+
Config: testAccServiceConfig_capacityProviderStrategy(rName, 10, 1, false),
395+
ExpectError: regexache.MustCompile(`force_new_deployment should be true when capacity_provider_strategy is being updated`),
384396
},
385397
{
386-
Config: testAccServiceConfig_capacityProviderStrategy(rName, 10, 1, false),
398+
Config: testAccServiceConfig_capacityProviderStrategy(rName, 10, 1, true),
387399
Check: resource.ComposeTestCheckFunc(
388400
testAccCheckServiceExists(ctx, resourceName, &service),
401+
resource.TestCheckResourceAttr(resourceName, "capacity_provider_strategy.#", "1"),
402+
resource.TestCheckResourceAttr(resourceName, "capacity_provider_strategy.0.weight", "10"),
403+
resource.TestCheckResourceAttr(resourceName, "capacity_provider_strategy.0.base", "1"),
389404
),
405+
ConfigPlanChecks: resource.ConfigPlanChecks{
406+
PreApply: []plancheck.PlanCheck{
407+
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate),
408+
},
409+
},
390410
},
391411
},
392412
})
@@ -408,6 +428,9 @@ func TestAccECSService_CapacityProviderStrategy_forceNewDeployment(t *testing.T)
408428
Config: testAccServiceConfig_capacityProviderStrategy(rName, 1, 0, true),
409429
Check: resource.ComposeTestCheckFunc(
410430
testAccCheckServiceExists(ctx, resourceName, &service),
431+
resource.TestCheckResourceAttr(resourceName, "capacity_provider_strategy.#", "1"),
432+
resource.TestCheckResourceAttr(resourceName, "capacity_provider_strategy.0.weight", "1"),
433+
resource.TestCheckResourceAttr(resourceName, "capacity_provider_strategy.0.base", "0"),
411434
),
412435
ConfigPlanChecks: resource.ConfigPlanChecks{
413436
PreApply: []plancheck.PlanCheck{
@@ -419,6 +442,9 @@ func TestAccECSService_CapacityProviderStrategy_forceNewDeployment(t *testing.T)
419442
Config: testAccServiceConfig_capacityProviderStrategy(rName, 10, 1, true),
420443
Check: resource.ComposeTestCheckFunc(
421444
testAccCheckServiceExists(ctx, resourceName, &service),
445+
resource.TestCheckResourceAttr(resourceName, "capacity_provider_strategy.#", "1"),
446+
resource.TestCheckResourceAttr(resourceName, "capacity_provider_strategy.0.weight", "10"),
447+
resource.TestCheckResourceAttr(resourceName, "capacity_provider_strategy.0.base", "1"),
422448
),
423449
ConfigPlanChecks: resource.ConfigPlanChecks{
424450
PreApply: []plancheck.PlanCheck{
@@ -457,17 +483,23 @@ func TestAccECSService_CapacityProviderStrategy_update(t *testing.T) {
457483
Config: testAccServiceConfig_updateCapacityProviderStrategy(rName, 1, "FARGATE"),
458484
Check: resource.ComposeTestCheckFunc(
459485
testAccCheckServiceExists(ctx, resourceName, &service),
486+
resource.TestCheckResourceAttr(resourceName, "capacity_provider_strategy.#", "1"),
487+
resource.TestCheckResourceAttr(resourceName, "capacity_provider_strategy.0.capacity_provider", "FARGATE"),
488+
resource.TestCheckResourceAttr(resourceName, "capacity_provider_strategy.0.weight", "1"),
460489
),
461490
ConfigPlanChecks: resource.ConfigPlanChecks{
462491
PreApply: []plancheck.PlanCheck{
463-
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionReplace),
492+
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate),
464493
},
465494
},
466495
},
467496
{
468497
Config: testAccServiceConfig_updateCapacityProviderStrategy(rName, 1, "FARGATE_SPOT"),
469498
Check: resource.ComposeTestCheckFunc(
470499
testAccCheckServiceExists(ctx, resourceName, &service),
500+
resource.TestCheckResourceAttr(resourceName, "capacity_provider_strategy.#", "1"),
501+
resource.TestCheckResourceAttr(resourceName, "capacity_provider_strategy.0.capacity_provider", "FARGATE_SPOT"),
502+
resource.TestCheckResourceAttr(resourceName, "capacity_provider_strategy.0.weight", "1"),
471503
),
472504
ConfigPlanChecks: resource.ConfigPlanChecks{
473505
PreApply: []plancheck.PlanCheck{
@@ -479,10 +511,11 @@ func TestAccECSService_CapacityProviderStrategy_update(t *testing.T) {
479511
Config: testAccServiceConfig_updateCapacityProviderStrategyRemove(rName),
480512
Check: resource.ComposeTestCheckFunc(
481513
testAccCheckServiceExists(ctx, resourceName, &service),
514+
resource.TestCheckResourceAttr(resourceName, "capacity_provider_strategy.#", "0"),
482515
),
483516
ConfigPlanChecks: resource.ConfigPlanChecks{
484517
PreApply: []plancheck.PlanCheck{
485-
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionReplace),
518+
plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate),
486519
},
487520
},
488521
},

‎website/docs/r/ecs_service.html.markdown‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ The following arguments are optional:
147147
* `region` - (Optional) Region where this resource will be [managed](https://docs.aws.amazon.com/general/latest/gr/rande.html#regional-endpoints). Defaults to the Region set in the [provider configuration](https://registry.terraform.io/providers/hashicorp/aws/latest/docs#aws-configuration-reference).
148148
* `alarms` - (Optional) Information about the CloudWatch alarms. [See below](#alarms).
149149
* `availability_zone_rebalancing` - (Optional) ECS automatically redistributes tasks within a service across Availability Zones (AZs) to mitigate the risk of impaired application availability due to underlying infrastructure failures and task lifecycle activities. The valid values are `ENABLED` and `DISABLED`. When creating a new service, if no value is specified, it defaults to `ENABLED` if the service is compatible with AvailabilityZoneRebalancing. When updating an existing service, if no value is specified it defaults to the existing service's AvailabilityZoneRebalancing value. If the service never had an AvailabilityZoneRebalancing value set, Amazon ECS treats this as `DISABLED`.
150-
* `capacity_provider_strategy` - (Optional) Capacity provider strategies to use for the service. Can be one or more. These can be updated without destroying and recreating the service only if `force_new_deployment = true` and not changing from 0 `capacity_provider_strategy` blocks to greater than 0, or vice versa. [See below](#capacity_provider_strategy). Conflicts with `launch_type`.
150+
* `capacity_provider_strategy` - (Optional) Capacity provider strategies to use for the service. Can be one or more. Updating this argument requires `force_new_deployment = true`. [See below](#capacity_provider_strategy). Conflicts with `launch_type`.
151151
* `cluster` - (Optional) ARN of an ECS cluster.
152152
* `deployment_circuit_breaker` - (Optional) Configuration block for deployment circuit breaker. [See below](#deployment_circuit_breaker).
153153
* `deployment_configuration` - (Optional) Configuration block for deployment settings. [See below](#deployment_configuration).

0 commit comments

Comments
 (0)