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

[Bug] Fix issues related to secondary EIPs and Private IPs being updated #41403

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

drewtul
Copy link
Contributor

@drewtul drewtul commented Feb 14, 2025

Description

Currently updating the EIP associations for a NAT Gateway causes a replacement, which is not required.

This removes ForceNew from secondary_private_ip_address_count and fixes issues allowing EIPs to be updated without recreating the resource which affects network connectivity for the duration.

Also makes it possible to move from secondary_private_ip_address_count being specified to specifying exact IPs in secondary_private_ip_addresses allowing users to increase from their initial number of private ips for a private NAT gateway.

Relations

Closes #33964.
Closes #34268.
Closes #36360.

References

Output from Acceptance Testing

%  make testacc ACCTEST_PARALLELISM=3 TESTS=TestAccVPCNATGateway_ PKG=ec2
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.23.5 test ./internal/service/ec2/... -v -count 1 -parallel 3 -run='TestAccVPCNATGateway_'  -timeout 360m -vet=off
2025/02/17 12:57:40 Initializing Terraform AWS Provider...
=== RUN   TestAccVPCNATGateway_basic
=== PAUSE TestAccVPCNATGateway_basic
=== RUN   TestAccVPCNATGateway_disappears
=== PAUSE TestAccVPCNATGateway_disappears
=== RUN   TestAccVPCNATGateway_ConnectivityType_private
=== PAUSE TestAccVPCNATGateway_ConnectivityType_private
=== RUN   TestAccVPCNATGateway_privateIP
=== PAUSE TestAccVPCNATGateway_privateIP
=== RUN   TestAccVPCNATGateway_tags
=== PAUSE TestAccVPCNATGateway_tags
=== RUN   TestAccVPCNATGateway_secondaryAllocationIDs
=== PAUSE TestAccVPCNATGateway_secondaryAllocationIDs
=== RUN   TestAccVPCNATGateway_secondaryPrivateIPAddressCount
=== PAUSE TestAccVPCNATGateway_secondaryPrivateIPAddressCount
=== RUN   TestAccVPCNATGateway_secondaryPrivateIPAddressCountToSpecific
=== PAUSE TestAccVPCNATGateway_secondaryPrivateIPAddressCountToSpecific
=== RUN   TestAccVPCNATGateway_secondaryPrivateIPAddresses
=== PAUSE TestAccVPCNATGateway_secondaryPrivateIPAddresses
=== RUN   TestAccVPCNATGateway_SecondaryPrivateIPAddresses_private
=== PAUSE TestAccVPCNATGateway_SecondaryPrivateIPAddresses_private
=== CONT  TestAccVPCNATGateway_basic
=== CONT  TestAccVPCNATGateway_secondaryAllocationIDs
=== CONT  TestAccVPCNATGateway_SecondaryPrivateIPAddresses_private
--- PASS: TestAccVPCNATGateway_basic (174.04s)
=== CONT  TestAccVPCNATGateway_secondaryPrivateIPAddresses
--- PASS: TestAccVPCNATGateway_SecondaryPrivateIPAddresses_private (568.75s)
=== CONT  TestAccVPCNATGateway_secondaryPrivateIPAddressCountToSpecific
--- PASS: TestAccVPCNATGateway_secondaryAllocationIDs (591.50s)
=== CONT  TestAccVPCNATGateway_secondaryPrivateIPAddressCount
--- PASS: TestAccVPCNATGateway_secondaryPrivateIPAddresses (572.73s)
=== CONT  TestAccVPCNATGateway_privateIP
--- PASS: TestAccVPCNATGateway_secondaryPrivateIPAddressCount (171.92s)
=== CONT  TestAccVPCNATGateway_tags
--- PASS: TestAccVPCNATGateway_privateIP (203.95s)
=== CONT  TestAccVPCNATGateway_ConnectivityType_private
--- PASS: TestAccVPCNATGateway_tags (202.74s)
=== CONT  TestAccVPCNATGateway_disappears
--- PASS: TestAccVPCNATGateway_secondaryPrivateIPAddressCountToSpecific (564.72s)
--- PASS: TestAccVPCNATGateway_ConnectivityType_private (193.75s)
--- PASS: TestAccVPCNATGateway_disappears (220.74s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/ec2        1191.618s

…c to resourceNATGatewayCustomizeDiff..

Update tests to ensure NatGateways it not recreated, fix issus rising from this with secondary_private_ip_addresses not being NewComputed when secondary_allocation_ids is changed.
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/vpc Issues and PRs that pertain to the vpc service. needs-triage Waiting for first response or review from a maintainer. partner Contribution from a partner. labels Feb 14, 2025
@drewtul drewtul marked this pull request as ready for review February 17, 2025 15:05
@drewtul drewtul requested a review from a team as a code owner February 17, 2025 15:05
@justinretzolk justinretzolk added bug Addresses a defect in current functionality. and removed needs-triage Waiting for first response or review from a maintainer. labels Feb 19, 2025
@ewbankkit ewbankkit self-assigned this Feb 25, 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 25, 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=TestAccVPCNATGateway_' PKG=ec2 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/ec2/... -v -count 1 -parallel 3  -run=TestAccVPCNATGateway_ -timeout 360m -vet=off
2025/02/25 15:58:21 Initializing Terraform AWS Provider...
=== RUN   TestAccVPCNATGateway_basic
=== PAUSE TestAccVPCNATGateway_basic
=== RUN   TestAccVPCNATGateway_disappears
=== PAUSE TestAccVPCNATGateway_disappears
=== RUN   TestAccVPCNATGateway_ConnectivityType_private
=== PAUSE TestAccVPCNATGateway_ConnectivityType_private
=== RUN   TestAccVPCNATGateway_privateIP
=== PAUSE TestAccVPCNATGateway_privateIP
=== RUN   TestAccVPCNATGateway_tags
=== PAUSE TestAccVPCNATGateway_tags
=== RUN   TestAccVPCNATGateway_secondaryAllocationIDs
=== PAUSE TestAccVPCNATGateway_secondaryAllocationIDs
=== RUN   TestAccVPCNATGateway_secondaryPrivateIPAddressCount
=== PAUSE TestAccVPCNATGateway_secondaryPrivateIPAddressCount
=== RUN   TestAccVPCNATGateway_secondaryPrivateIPAddressCountToSpecific
=== PAUSE TestAccVPCNATGateway_secondaryPrivateIPAddressCountToSpecific
=== RUN   TestAccVPCNATGateway_secondaryPrivateIPAddresses
=== PAUSE TestAccVPCNATGateway_secondaryPrivateIPAddresses
=== RUN   TestAccVPCNATGateway_SecondaryPrivateIPAddresses_private
=== PAUSE TestAccVPCNATGateway_SecondaryPrivateIPAddresses_private
=== CONT  TestAccVPCNATGateway_basic
=== CONT  TestAccVPCNATGateway_secondaryAllocationIDs
=== CONT  TestAccVPCNATGateway_privateIP
--- PASS: TestAccVPCNATGateway_privateIP (156.82s)
=== CONT  TestAccVPCNATGateway_secondaryPrivateIPAddresses
--- PASS: TestAccVPCNATGateway_basic (186.53s)
=== CONT  TestAccVPCNATGateway_SecondaryPrivateIPAddresses_private
--- PASS: TestAccVPCNATGateway_secondaryAllocationIDs (602.29s)
=== CONT  TestAccVPCNATGateway_ConnectivityType_private
--- PASS: TestAccVPCNATGateway_secondaryPrivateIPAddresses (565.47s)
=== CONT  TestAccVPCNATGateway_tags
--- PASS: TestAccVPCNATGateway_SecondaryPrivateIPAddresses_private (581.84s)
=== CONT  TestAccVPCNATGateway_disappears
--- PASS: TestAccVPCNATGateway_ConnectivityType_private (176.49s)
=== CONT  TestAccVPCNATGateway_secondaryPrivateIPAddressCountToSpecific
--- PASS: TestAccVPCNATGateway_tags (190.78s)
=== CONT  TestAccVPCNATGateway_secondaryPrivateIPAddressCount
--- PASS: TestAccVPCNATGateway_disappears (155.46s)
--- PASS: TestAccVPCNATGateway_secondaryPrivateIPAddressCount (195.30s)
--- PASS: TestAccVPCNATGateway_secondaryPrivateIPAddressCountToSpecific (418.86s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/ec2	1203.534s

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. partner Contribution from a partner. prioritized Part of the maintainer teams immediate focus. To be addressed within the current quarter. service/vpc Issues and PRs that pertain to the vpc service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
3 participants