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

fix: Do not replace NAT gateways when additional subnets are added #1055

Conversation

manuelmazzuola
Copy link
Contributor

Description

Adding new private subnets to an already deployed vpc module leads to the replacement of the nat gateways for all the subnets that already existing.

There is no reason to use the try function in that case.

Motivation and Context

Breaking Changes

none

How Has This Been Tested?

  • I have updated at least one of the examples/* to demonstrate and validate my change(s)
  • I have tested and validated these changes using one or more of the provided examples/* projects
  • I have executed pre-commit run -a on my pull request

Before the change

1 to add, 1 to destroy.

  # module.network.module.network.module.vpc.aws_nat_gateway.this[0] must be replaced
-/+ resource "aws_nat_gateway" "this" {
      ~ allocation_id        = "redacted-***" -> (known after apply) # forces replacement
      ~ association_id       = "redacted-***" -> (known after apply)
      ~ id                   = "redacted-***" -> (known after apply)
      ~ network_interface_id = "redacted-***" -> (known after apply)
      ~ private_ip           = "redacted-***" -> (known after apply)
      ~ public_ip            = "redacted-***" -> (known after apply)
        # (3 unchanged attributes hidden)
    }

After the change

1 to add, 0 to destroy.

@manuelmazzuola manuelmazzuola changed the title Do not replace nat gateways when new private subnets are added fix - Do not replace nat gateways when new private subnets are added Mar 21, 2024
@manuelmazzuola manuelmazzuola force-pushed the fix-nat-gateway-replacement branch 2 times, most recently from e8bfc7c to 4dff716 Compare March 21, 2024 14:42
@manuelmazzuola manuelmazzuola changed the title fix - Do not replace nat gateways when new private subnets are added fix: Do not replace nat gateways when new private subnets are added Mar 21, 2024
@Smana
Copy link

Smana commented Apr 2, 2024

tf-controller plan output:

No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration
and found no differences, so no changes are needed.

To apply this plan, please merge this pull request.

@@ -1034,7 +1034,7 @@ resource "aws_route" "private_ipv6_egress" {

locals {
nat_gateway_count = var.single_nat_gateway ? 1 : var.one_nat_gateway_per_az ? length(var.azs) : local.max_subnet_length
nat_gateway_ips = var.reuse_nat_ips ? var.external_nat_ip_ids : try(aws_eip.nat[*].id, [])
nat_gateway_ips = var.reuse_nat_ips ? var.external_nat_ip_ids : aws_eip.nat[*].id
Copy link
Member

Choose a reason for hiding this comment

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

why would this change make a difference?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because the try statement can't be evaluated while planning, so the nat gateways are all replaced, despite the fact that they are not changed

@bryantbiggs bryantbiggs changed the title fix: Do not replace nat gateways when new private subnets are added fix: Do not replace NAT gateways when additional subnets are added Apr 26, 2024
@bryantbiggs bryantbiggs merged commit cf18c37 into terraform-aws-modules:master Apr 26, 2024
20 checks passed
antonbabenko pushed a commit that referenced this pull request Apr 26, 2024
## [5.8.1](v5.8.0...v5.8.1) (2024-04-26)

### Bug Fixes

* Do not replace NAT gateways when additional subnets are added ([#1055](#1055)) ([cf18c37](cf18c37))
@antonbabenko
Copy link
Member

This PR is included in version 5.8.1 🎉

@manuelmazzuola manuelmazzuola deleted the fix-nat-gateway-replacement branch May 2, 2024 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants