From d98baba852640cf354f1b73d91cce1d8b2b8f708 Mon Sep 17 00:00:00 2001 From: zachfeld Date: Wed, 9 Mar 2022 16:39:30 -0500 Subject: [PATCH 1/6] r/aws_ecs_service: remove ForceNew from resource arguments --- internal/service/ecs/service.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/internal/service/ecs/service.go b/internal/service/ecs/service.go index 674fd98771f..6bb0d437189 100644 --- a/internal/service/ecs/service.go +++ b/internal/service/ecs/service.go @@ -153,7 +153,6 @@ func ResourceService() *schema.Resource { Type: schema.TypeBool, Optional: true, Default: false, - ForceNew: true, }, "enable_execute_command": { Type: schema.TypeBool, @@ -185,7 +184,6 @@ func ResourceService() *schema.Resource { "load_balancer": { Type: schema.TypeSet, Optional: true, - ForceNew: true, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "elb_name": { @@ -309,7 +307,6 @@ func ResourceService() *schema.Resource { "propagate_tags": { Type: schema.TypeString, Optional: true, - ForceNew: true, DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { if old == "NONE" && new == "" { return true @@ -335,7 +332,6 @@ func ResourceService() *schema.Resource { "service_registries": { Type: schema.TypeList, Optional: true, - ForceNew: true, MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ From 19e79d49bfbebe2891af301d61b2a3a7d32d1631 Mon Sep 17 00:00:00 2001 From: zachfeld Date: Wed, 9 Mar 2022 18:42:26 -0500 Subject: [PATCH 2/6] r/aws_ecs_service: rebase onto main --- internal/service/ecs/service.go | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/internal/service/ecs/service.go b/internal/service/ecs/service.go index 6bb0d437189..7b83463c0d4 100644 --- a/internal/service/ecs/service.go +++ b/internal/service/ecs/service.go @@ -316,7 +316,7 @@ func ResourceService() *schema.Resource { ValidateFunc: validation.StringInSlice([]string{ ecs.PropagateTagsService, ecs.PropagateTagsTaskDefinition, - "", + ecs.PropagateTagsNone, }, false), }, "scheduling_strategy": { @@ -1107,6 +1107,26 @@ func resourceServiceUpdate(d *schema.ResourceData, meta interface{}) error { input.EnableExecuteCommand = aws.Bool(d.Get("enable_execute_command").(bool)) } + if d.HasChange("enable_ecs_managed_tags") { + updateService = true + input.EnableECSManagedTags = aws.Bool(d.Get("enable_ecs_managed_tags").(bool)) + } + + if d.HasChange("load_balancer") { + updateService = true + input.LoadBalancers = expandLoadBalancers(d.Get("load_balancer").([]interface{})) + } + + if d.HasChange("propagate_tags") { + updateService = true + input.PropagateTags = aws.String(d.Get("propagate_tags").(string)) + } + + if d.HasChange("service_registries") { + updateService = true + input.ServiceRegistries = expandServiceRegistries(d.Get("service_registries").([]interface{})) + } + if updateService { log.Printf("[DEBUG] Updating ECS Service (%s): %s", d.Id(), input) // Retry due to IAM eventual consistency From f565613c18c94aeab7930708e42e3ae9beefa68a Mon Sep 17 00:00:00 2001 From: zachfeld Date: Wed, 9 Mar 2022 22:39:58 -0500 Subject: [PATCH 3/6] r/aws_ecs_service: update TestAccECSService_Tags_propagate to test ecs.PropagateTagsNone --- internal/service/ecs/service_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/service/ecs/service_test.go b/internal/service/ecs/service_test.go index 76ebdadc59d..8e7846a073a 100644 --- a/internal/service/ecs/service_test.go +++ b/internal/service/ecs/service_test.go @@ -1233,10 +1233,10 @@ func TestAccECSService_Tags_propagate(t *testing.T) { ), }, { - Config: testAccServiceManagedTagsConfig(rName), + Config: testAccServicePropagateTagsConfig(rName, "NONE"), Check: resource.ComposeTestCheckFunc( testAccCheckServiceExists(resourceName, &third), - resource.TestCheckResourceAttr(resourceName, "propagate_tags", "NONE"), + resource.TestCheckResourceAttr(resourceName, "propagate_tags", ecs.PropagateTagsNone), ), }, }, From de8c5b78aae0311b457b75104611d9d73f0fe230 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 14 Mar 2022 09:26:06 -0400 Subject: [PATCH 4/6] Add CHANGELOG entry. --- .changelog/23600.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/23600.txt diff --git a/.changelog/23600.txt b/.changelog/23600.txt new file mode 100644 index 00000000000..1fe84f981b3 --- /dev/null +++ b/.changelog/23600.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +resource/aws_ecs_service: `enable_ecs_managed_tags`, `load_balancer`, `propagate_tags` and `service_registries` can now be updated in-place +``` \ No newline at end of file From 937eca2c3c2a2c65b2f7dba5d6456a10e821222b Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 14 Mar 2022 09:43:47 -0400 Subject: [PATCH 5/6] Use '_Values' for validations (#14601). --- internal/service/ecs/service.go | 52 +++++++++++---------------------- 1 file changed, 17 insertions(+), 35 deletions(-) diff --git a/internal/service/ecs/service.go b/internal/service/ecs/service.go index 7b83463c0d4..35bd263ce0a 100644 --- a/internal/service/ecs/service.go +++ b/internal/service/ecs/service.go @@ -107,15 +107,11 @@ func ResourceService() *schema.Resource { Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "type": { - Type: schema.TypeString, - ForceNew: true, - Optional: true, - Default: ecs.DeploymentControllerTypeEcs, - ValidateFunc: validation.StringInSlice([]string{ - ecs.DeploymentControllerTypeCodeDeploy, - ecs.DeploymentControllerTypeEcs, - ecs.DeploymentControllerTypeExternal, - }, false), + Type: schema.TypeString, + ForceNew: true, + Optional: true, + Default: ecs.DeploymentControllerTypeEcs, + ValidateFunc: validation.StringInSlice(ecs.DeploymentControllerType_Values(), false), }, }, }, @@ -253,13 +249,9 @@ func ResourceService() *schema.Resource { Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "type": { - Type: schema.TypeString, - Required: true, - ValidateFunc: validation.StringInSlice([]string{ - ecs.PlacementStrategyTypeBinpack, - ecs.PlacementStrategyTypeRandom, - ecs.PlacementStrategyTypeSpread, - }, false), + Type: schema.TypeString, + Required: true, + ValidateFunc: validation.StringInSlice(ecs.PlacementStrategyType_Values(), false), }, "field": { Type: schema.TypeString, @@ -289,12 +281,9 @@ func ResourceService() *schema.Resource { Optional: true, }, "type": { - Type: schema.TypeString, - Required: true, - ValidateFunc: validation.StringInSlice([]string{ - ecs.PlacementConstraintTypeDistinctInstance, - ecs.PlacementConstraintTypeMemberOf, - }, false), + Type: schema.TypeString, + Required: true, + ValidateFunc: validation.StringInSlice(ecs.PlacementConstraintType_Values(), false), }, }, }, @@ -313,21 +302,14 @@ func ResourceService() *schema.Resource { } return false }, - ValidateFunc: validation.StringInSlice([]string{ - ecs.PropagateTagsService, - ecs.PropagateTagsTaskDefinition, - ecs.PropagateTagsNone, - }, false), + ValidateFunc: validation.StringInSlice(ecs.PropagateTags_Values(), false), }, "scheduling_strategy": { - Type: schema.TypeString, - Optional: true, - ForceNew: true, - Default: ecs.SchedulingStrategyReplica, - ValidateFunc: validation.StringInSlice([]string{ - ecs.SchedulingStrategyDaemon, - ecs.SchedulingStrategyReplica, - }, false), + Type: schema.TypeString, + Optional: true, + ForceNew: true, + Default: ecs.SchedulingStrategyReplica, + ValidateFunc: validation.StringInSlice(ecs.SchedulingStrategy_Values(), false), }, "service_registries": { Type: schema.TypeList, From 4b21dcdee2cdf1c16883bd6308302ec888110c49 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Mon, 14 Mar 2022 09:47:50 -0400 Subject: [PATCH 6/6] r/aws_ec2_service: Simplify update logic by using 'if d.HasChangesExcept("tags", "tags_all")'. --- internal/service/ecs/service.go | 202 +++++++++++++++----------------- 1 file changed, 92 insertions(+), 110 deletions(-) diff --git a/internal/service/ecs/service.go b/internal/service/ecs/service.go index 35bd263ce0a..a1abb886e6c 100644 --- a/internal/service/ecs/service.go +++ b/internal/service/ecs/service.go @@ -978,142 +978,124 @@ func flattenServiceRegistries(srs []*ecs.ServiceRegistry) []map[string]interface func resourceServiceUpdate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*conns.AWSClient).ECSConn - updateService := false - input := ecs.UpdateServiceInput{ - Cluster: aws.String(d.Get("cluster").(string)), - ForceNewDeployment: aws.Bool(d.Get("force_new_deployment").(bool)), - Service: aws.String(d.Id()), - } + if d.HasChangesExcept("tags", "tags_all") { + input := &ecs.UpdateServiceInput{ + Cluster: aws.String(d.Get("cluster").(string)), + ForceNewDeployment: aws.Bool(d.Get("force_new_deployment").(bool)), + Service: aws.String(d.Id()), + } - schedulingStrategy := d.Get("scheduling_strategy").(string) + schedulingStrategy := d.Get("scheduling_strategy").(string) - if schedulingStrategy == ecs.SchedulingStrategyDaemon { - if d.HasChange("deployment_minimum_healthy_percent") { - updateService = true - input.DeploymentConfiguration = &ecs.DeploymentConfiguration{ - MinimumHealthyPercent: aws.Int64(int64(d.Get("deployment_minimum_healthy_percent").(int))), + if schedulingStrategy == ecs.SchedulingStrategyDaemon { + if d.HasChange("deployment_minimum_healthy_percent") { + input.DeploymentConfiguration = &ecs.DeploymentConfiguration{ + MinimumHealthyPercent: aws.Int64(int64(d.Get("deployment_minimum_healthy_percent").(int))), + } + } + } else if schedulingStrategy == ecs.SchedulingStrategyReplica { + if d.HasChange("desired_count") { + input.DesiredCount = aws.Int64(int64(d.Get("desired_count").(int))) } - } - } else if schedulingStrategy == ecs.SchedulingStrategyReplica { - if d.HasChange("desired_count") { - updateService = true - input.DesiredCount = aws.Int64(int64(d.Get("desired_count").(int))) - } - if d.HasChanges("deployment_maximum_percent", "deployment_minimum_healthy_percent") { - updateService = true - input.DeploymentConfiguration = &ecs.DeploymentConfiguration{ - MaximumPercent: aws.Int64(int64(d.Get("deployment_maximum_percent").(int))), - MinimumHealthyPercent: aws.Int64(int64(d.Get("deployment_minimum_healthy_percent").(int))), + if d.HasChanges("deployment_maximum_percent", "deployment_minimum_healthy_percent") { + input.DeploymentConfiguration = &ecs.DeploymentConfiguration{ + MaximumPercent: aws.Int64(int64(d.Get("deployment_maximum_percent").(int))), + MinimumHealthyPercent: aws.Int64(int64(d.Get("deployment_minimum_healthy_percent").(int))), + } } } - } - - if d.HasChange("deployment_circuit_breaker") { - updateService = true - if input.DeploymentConfiguration == nil { - input.DeploymentConfiguration = &ecs.DeploymentConfiguration{} - } + if d.HasChange("deployment_circuit_breaker") { + if input.DeploymentConfiguration == nil { + input.DeploymentConfiguration = &ecs.DeploymentConfiguration{} + } - // To remove an existing deployment circuit breaker, specify an empty object. - input.DeploymentConfiguration.DeploymentCircuitBreaker = &ecs.DeploymentCircuitBreaker{} + // To remove an existing deployment circuit breaker, specify an empty object. + input.DeploymentConfiguration.DeploymentCircuitBreaker = &ecs.DeploymentCircuitBreaker{} - if v, ok := d.GetOk("deployment_circuit_breaker"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { - input.DeploymentConfiguration.DeploymentCircuitBreaker = expandDeploymentCircuitBreaker(v.([]interface{})[0].(map[string]interface{})) + if v, ok := d.GetOk("deployment_circuit_breaker"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { + input.DeploymentConfiguration.DeploymentCircuitBreaker = expandDeploymentCircuitBreaker(v.([]interface{})[0].(map[string]interface{})) + } } - } - if d.HasChange("ordered_placement_strategy") { - updateService = true - // Reference: https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_UpdateService.html#ECS-UpdateService-request-placementStrategy - // To remove an existing placement strategy, specify an empty object. - input.PlacementStrategy = []*ecs.PlacementStrategy{} + if d.HasChange("ordered_placement_strategy") { + // Reference: https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_UpdateService.html#ECS-UpdateService-request-placementStrategy + // To remove an existing placement strategy, specify an empty object. + input.PlacementStrategy = []*ecs.PlacementStrategy{} - if v, ok := d.GetOk("ordered_placement_strategy"); ok && len(v.([]interface{})) > 0 { - ps, err := expandPlacementStrategy(v.([]interface{})) + if v, ok := d.GetOk("ordered_placement_strategy"); ok && len(v.([]interface{})) > 0 { + ps, err := expandPlacementStrategy(v.([]interface{})) - if err != nil { - return err - } + if err != nil { + return err + } - input.PlacementStrategy = ps + input.PlacementStrategy = ps + } } - } - if d.HasChange("placement_constraints") { - updateService = true - // Reference: https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_UpdateService.html#ECS-UpdateService-request-placementConstraints - // To remove all existing placement constraints, specify an empty array. - input.PlacementConstraints = []*ecs.PlacementConstraint{} + if d.HasChange("placement_constraints") { + // Reference: https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_UpdateService.html#ECS-UpdateService-request-placementConstraints + // To remove all existing placement constraints, specify an empty array. + input.PlacementConstraints = []*ecs.PlacementConstraint{} - if v, ok := d.Get("placement_constraints").(*schema.Set); ok && v.Len() > 0 { - pc, err := expandPlacementConstraints(v.List()) + if v, ok := d.Get("placement_constraints").(*schema.Set); ok && v.Len() > 0 { + pc, err := expandPlacementConstraints(v.List()) - if err != nil { - return err - } + if err != nil { + return err + } - input.PlacementConstraints = pc + input.PlacementConstraints = pc + } } - } - if d.HasChange("platform_version") { - updateService = true - input.PlatformVersion = aws.String(d.Get("platform_version").(string)) - } + if d.HasChange("platform_version") { + input.PlatformVersion = aws.String(d.Get("platform_version").(string)) + } - if d.HasChange("health_check_grace_period_seconds") { - updateService = true - input.HealthCheckGracePeriodSeconds = aws.Int64(int64(d.Get("health_check_grace_period_seconds").(int))) - } + if d.HasChange("health_check_grace_period_seconds") { + input.HealthCheckGracePeriodSeconds = aws.Int64(int64(d.Get("health_check_grace_period_seconds").(int))) + } - if d.HasChange("task_definition") { - updateService = true - input.TaskDefinition = aws.String(d.Get("task_definition").(string)) - } + if d.HasChange("task_definition") { + input.TaskDefinition = aws.String(d.Get("task_definition").(string)) + } - if d.HasChange("network_configuration") { - updateService = true - input.NetworkConfiguration = expandNetworkConfiguration(d.Get("network_configuration").([]interface{})) - } + if d.HasChange("network_configuration") { + input.NetworkConfiguration = expandNetworkConfiguration(d.Get("network_configuration").([]interface{})) + } - if d.HasChange("capacity_provider_strategy") { - updateService = true - input.CapacityProviderStrategy = expandCapacityProviderStrategy(d.Get("capacity_provider_strategy").(*schema.Set)) - } + if d.HasChange("capacity_provider_strategy") { + input.CapacityProviderStrategy = expandCapacityProviderStrategy(d.Get("capacity_provider_strategy").(*schema.Set)) + } - if d.HasChange("enable_execute_command") { - updateService = true - input.EnableExecuteCommand = aws.Bool(d.Get("enable_execute_command").(bool)) - } + if d.HasChange("enable_execute_command") { + input.EnableExecuteCommand = aws.Bool(d.Get("enable_execute_command").(bool)) + } - if d.HasChange("enable_ecs_managed_tags") { - updateService = true - input.EnableECSManagedTags = aws.Bool(d.Get("enable_ecs_managed_tags").(bool)) - } + if d.HasChange("enable_ecs_managed_tags") { + input.EnableECSManagedTags = aws.Bool(d.Get("enable_ecs_managed_tags").(bool)) + } - if d.HasChange("load_balancer") { - updateService = true - input.LoadBalancers = expandLoadBalancers(d.Get("load_balancer").([]interface{})) - } + if d.HasChange("load_balancer") { + input.LoadBalancers = expandLoadBalancers(d.Get("load_balancer").([]interface{})) + } - if d.HasChange("propagate_tags") { - updateService = true - input.PropagateTags = aws.String(d.Get("propagate_tags").(string)) - } + if d.HasChange("propagate_tags") { + input.PropagateTags = aws.String(d.Get("propagate_tags").(string)) + } - if d.HasChange("service_registries") { - updateService = true - input.ServiceRegistries = expandServiceRegistries(d.Get("service_registries").([]interface{})) - } + if d.HasChange("service_registries") { + input.ServiceRegistries = expandServiceRegistries(d.Get("service_registries").([]interface{})) + } - if updateService { log.Printf("[DEBUG] Updating ECS Service (%s): %s", d.Id(), input) // Retry due to IAM eventual consistency err := resource.Retry(tfiam.PropagationTimeout+serviceUpdateTimeout, func() *resource.RetryError { - _, err := conn.UpdateService(&input) + _, err := conn.UpdateService(input) if err != nil { if tfawserr.ErrMessageContains(err, ecs.ErrCodeInvalidParameterException, "verify that the ECS service role being passed has the proper permissions") { @@ -1130,22 +1112,22 @@ func resourceServiceUpdate(d *schema.ResourceData, meta interface{}) error { }) if tfresource.TimedOut(err) { - _, err = conn.UpdateService(&input) + _, err = conn.UpdateService(input) } if err != nil { return fmt.Errorf("error updating ECS Service (%s): %w", d.Id(), err) } - } - if d.Get("wait_for_steady_state").(bool) { - cluster := "" - if v, ok := d.GetOk("cluster"); ok { - cluster = v.(string) - } + if d.Get("wait_for_steady_state").(bool) { + cluster := "" + if v, ok := d.GetOk("cluster"); ok { + cluster = v.(string) + } - if err := waitServiceStable(conn, d.Id(), cluster); err != nil { - return fmt.Errorf("error waiting for ECS service (%s) to become ready: %w", d.Id(), err) + if err := waitServiceStable(conn, d.Id(), cluster); err != nil { + return fmt.Errorf("error waiting for ECS service (%s) to become ready: %w", d.Id(), err) + } } }