Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

b/aws_ecs_service: fix VPC Lattice configuration removal #41594

Merged

Conversation

marcinbelczewski
Copy link

@marcinbelczewski marcinbelczewski commented Feb 27, 2025

Description

This PR:

  • fixes the problem with VPC Lattice configuration removal from aws_ecs_service. ECS API requires empty array to be passed in UpdateService request for that and currently null is being passed.
  • fixes TestAccECSService_LatticeConfigurations test - currently two nginx containers conflict on port 80 and in reality the service would never start properly, IPv6 configuration amended.
  • amends aws_vpclattice_target_group configuration so that delete request can be sent while the targets are still draining (that will happen for example during acceptance tests of aws_ecs_service)

Relations

Closes #41600.
Closes #41601.

References

Output from Acceptance Testing

%make testacc TESTS=TestAccECSService_LatticeConfigurations PKG=ecs
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.23.5 test ./internal/service/ecs/... -v -count 1 -parallel 20 -run='TestAccECSService_LatticeConfigurations'  -timeout 360m -vet=off
2025/02/27 21:26:53 Initializing Terraform AWS Provider...
=== RUN   TestAccECSService_LatticeConfigurations
=== PAUSE TestAccECSService_LatticeConfigurations
=== CONT  TestAccECSService_LatticeConfigurations
--- PASS: TestAccECSService_LatticeConfigurations (384.75s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/ecs	391.282s

Copy link

Community Note

Voting for Prioritization

  • Please vote on this pull request by adding a 👍 reaction to the original post to help the community and maintainers prioritize this pull request.
  • Please see our prioritization guide for information on how we prioritize.
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

For Submitters

  • Review the contribution guide relating to the type of change you are making to ensure all of the necessary steps have been taken.
  • For new resources and data sources, use skaff to generate scaffolding with comments detailing common expectations.
  • Whether or not the branch has been rebased will not impact prioritization, but doing so is always a welcome surprise.

@github-actions github-actions bot added tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. service/ecs Issues and PRs that pertain to the ecs service. service/vpclattice Issues and PRs that pertain to the vpclattice service. needs-triage Waiting for first response or review from a maintainer. labels Feb 27, 2025
@marcinbelczewski marcinbelczewski force-pushed the b/ecs-service-lattice-config-removal branch from 2ddf119 to 1d52d18 Compare February 27, 2025 20:35
r/aws_vpclattice_target_group: allow for submitting delete request while targets are still draining
@marcinbelczewski marcinbelczewski force-pushed the b/ecs-service-lattice-config-removal branch from 1d52d18 to ae3e1ea Compare February 27, 2025 20:47
@marcinbelczewski marcinbelczewski marked this pull request as ready for review February 28, 2025 07:55
@marcinbelczewski marcinbelczewski requested a review from a team as a code owner February 28, 2025 07:55
@ewbankkit ewbankkit added bug Addresses a defect in current functionality. and removed needs-triage Waiting for first response or review from a maintainer. labels Feb 28, 2025
@ewbankkit ewbankkit self-assigned this Feb 28, 2025
@github-actions github-actions bot added the prioritized Part of the maintainer teams immediate focus. To be addressed within the current quarter. label Feb 28, 2025
Copy link
Contributor

@ewbankkit ewbankkit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀.

% make testacc TESTARGS='-run=TestAccECSService_LatticeConfigurations\|TestAccECSService_basic' PKG=ecs
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.23.5 test ./internal/service/ecs/... -v -count 1 -parallel 20  -run=TestAccECSService_LatticeConfigurations\|TestAccECSService_basic -timeout 360m -vet=off
2025/02/28 10:51:28 Initializing Terraform AWS Provider...
=== RUN   TestAccECSService_basic
=== PAUSE TestAccECSService_basic
=== RUN   TestAccECSService_basicImport
=== PAUSE TestAccECSService_basicImport
=== RUN   TestAccECSService_LatticeConfigurations
=== PAUSE TestAccECSService_LatticeConfigurations
=== CONT  TestAccECSService_basic
=== CONT  TestAccECSService_LatticeConfigurations
=== CONT  TestAccECSService_basicImport
--- PASS: TestAccECSService_basic (69.48s)
--- PASS: TestAccECSService_basicImport (195.62s)
--- PASS: TestAccECSService_LatticeConfigurations (565.70s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/ecs	571.236s
% make testacc TESTARGS='-run=TestAccVPCLatticeTargetGroup_' PKG=vpclattice ACCTEST_PARALLELISM=3
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.23.5 test ./internal/service/vpclattice/... -v -count 1 -parallel 3  -run=TestAccVPCLatticeTargetGroup_ -timeout 360m -vet=off
2025/02/28 11:32:31 Initializing Terraform AWS Provider...
=== RUN   TestAccVPCLatticeTargetGroup_basic
=== PAUSE TestAccVPCLatticeTargetGroup_basic
=== RUN   TestAccVPCLatticeTargetGroup_disappears
=== PAUSE TestAccVPCLatticeTargetGroup_disappears
=== RUN   TestAccVPCLatticeTargetGroup_tags
=== PAUSE TestAccVPCLatticeTargetGroup_tags
=== RUN   TestAccVPCLatticeTargetGroup_lambda
=== PAUSE TestAccVPCLatticeTargetGroup_lambda
=== RUN   TestAccVPCLatticeTargetGroup_lambdaEventStructureVersion
=== PAUSE TestAccVPCLatticeTargetGroup_lambdaEventStructureVersion
=== RUN   TestAccVPCLatticeTargetGroup_ip
=== PAUSE TestAccVPCLatticeTargetGroup_ip
=== RUN   TestAccVPCLatticeTargetGroup_alb
=== PAUSE TestAccVPCLatticeTargetGroup_alb
=== CONT  TestAccVPCLatticeTargetGroup_basic
=== CONT  TestAccVPCLatticeTargetGroup_lambdaEventStructureVersion
=== CONT  TestAccVPCLatticeTargetGroup_alb
--- PASS: TestAccVPCLatticeTargetGroup_lambdaEventStructureVersion (17.21s)
=== CONT  TestAccVPCLatticeTargetGroup_tags
--- PASS: TestAccVPCLatticeTargetGroup_tags (32.40s)
=== CONT  TestAccVPCLatticeTargetGroup_ip
--- PASS: TestAccVPCLatticeTargetGroup_basic (71.47s)
=== CONT  TestAccVPCLatticeTargetGroup_lambda
--- PASS: TestAccVPCLatticeTargetGroup_lambda (17.60s)
=== CONT  TestAccVPCLatticeTargetGroup_disappears
--- PASS: TestAccVPCLatticeTargetGroup_alb (87.80s)
--- PASS: TestAccVPCLatticeTargetGroup_ip (80.83s)
--- PASS: TestAccVPCLatticeTargetGroup_disappears (63.23s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/vpclattice	153.457s

@ewbankkit
Copy link
Contributor

@marcinbelczewski Thanks for the contribution 🎉 👏.

@ewbankkit ewbankkit merged commit 077d986 into hashicorp:main Feb 28, 2025
37 checks passed
@github-actions github-actions bot added this to the v5.90.0 milestone Feb 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Addresses a defect in current functionality. prioritized Part of the maintainer teams immediate focus. To be addressed within the current quarter. service/ecs Issues and PRs that pertain to the ecs service. service/vpclattice Issues and PRs that pertain to the vpclattice service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
3 participants