provider/aws: Fix bug w/ changing ECS svc/ELB association

[radeksimko]

This is fixing #3444 and #4227

Copy-pasting the RCA:

Changing ELB association is apparently a destructive action since there's no way to update it - we'll need to mark all aws_ecs_service.load_balancer.* fields as ForceNew: true.

You could probably make it less destructive by using lifecycle { create_before_destroy = true } on the aws_ecs_service, so that the new association takes place, traffic is served from the new service and old association is removed. For that however we will need to make name optional & generated, otherwise this would fail due to name conflict.

The other commit is also addressing the problem mentioned in #4326

It is not addressing the optional&generated name yet. Having not very positive experience with ELBs and LCs & generated names there, I'd like to make it generated with a prefix, so a human is able to identify each service outside of terraform. The lack of tags in ECS is making prefixes even more important.

[radeksimko]

@hngkr this is what I meant in my comment #4326 (comment)

[radeksimko]

I've been trying to make the LB change non-destructive (as described in #3444) and it seems it's not as easy as I thought due to the lifecycle of ECS task definitions.

In order to change the port number, we have to change both the aws_ecs_task_definition and aws_ecs_service which in combination w/ lifecycle { create_before_destroy = true } (rightly) ends up in a cycle. We don't permit two versions (two task definition ARNs) to be present & referenced at the same time.

It would be nice to make changing LB ports smooth, but for now I will just polish what I have to fix the original bug.

[radeksimko]

$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=AWSEcsService'

==> Checking that code complies with gofmt requirements...
go generate ./...
TF_ACC=1 go test ./builtin/providers/aws -v -run=AWSEcsService -timeout 90m
=== RUN   TestAccAWSEcsServiceWithARN
--- PASS: TestAccAWSEcsServiceWithARN (59.60s)
=== RUN   TestAccAWSEcsServiceWithFamilyAndRevision
--- PASS: TestAccAWSEcsServiceWithFamilyAndRevision (108.20s)
=== RUN   TestAccAWSEcsServiceWithRenamedCluster
--- PASS: TestAccAWSEcsServiceWithRenamedCluster (157.00s)
=== RUN   TestAccAWSEcsService_withIamRole
--- PASS: TestAccAWSEcsService_withIamRole (122.41s)
=== RUN   TestAccAWSEcsService_withLbChanges
--- PASS: TestAccAWSEcsService_withLbChanges (69.52s)
=== RUN   TestAccAWSEcsService_withEcsClusterName
--- PASS: TestAccAWSEcsService_withEcsClusterName (19.72s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    536.469s

Theoretically the tests may fail intermittently due to #4375 but that's out of scope of this PR, so it's ready for review & merge.

[catsby]

This looks good to me @radeksimko , thanks!
Sorry for the delay in review :)

[catsby]

Feel free to merge at your convenience :)

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.