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

Too many NAT gateways created #117

Closed
sc250024 opened this issue Apr 17, 2018 · 7 comments · Fixed by #129
Closed

Too many NAT gateways created #117

sc250024 opened this issue Apr 17, 2018 · 7 comments · Fixed by #129

Comments

@sc250024
Copy link
Contributor

sc250024 commented Apr 17, 2018

Not sure whether to file this under "feature", or "bug"; I suppose it depends on how a person views subnetting in AWS VPC.

Basically, I have 6 private subnets calculated from a base /16 prefix, and being created in the eu-west-1 region as follows:

    private_subnets = [
        "${cidrsubnet(var.cidr_block, 7, 7)}",
        "${cidrsubnet(var.cidr_block, 7, 8)}",
        "${cidrsubnet(var.cidr_block, 7, 9)}",
        "${cidrsubnet(var.cidr_block, 4, 13)}",
        "${cidrsubnet(var.cidr_block, 4, 14)}",
        "${cidrsubnet(var.cidr_block, 4, 15)}",
    ]

In the module definition, I have also specified to use all 3 availability zones eu-west-1a, eu-west-1b, and eu-west-1c. The module then generates 6 NAT gateways that look like this:

  + module.vpc.aws_route.private_nat_gateway[X]
      id:                                     <computed>
      destination_cidr_block:                 "0.0.0.0/0"
      destination_prefix_list_id:             <computed>
      egress_only_gateway_id:                 <computed>
      gateway_id:                             <computed>
      instance_id:                            <computed>
      instance_owner_id:                      <computed>
      nat_gateway_id:                         "${element(aws_nat_gateway.this.*.id, count.index)}"
      network_interface_id:                   <computed>
      origin:                                 <computed>
      route_table_id:                         "${element(aws_route_table.private.*.id, count.index)}"
      state:                                  <computed>

My question: isn't one NAT gateway per private subnet a bit excessive? If the number of availability zones is defined by ${length(var.azs)}, then shouldn't only ${length(var.azs)} number of NAT gateways be created, and added to the route tables?

This is especially true as a person's user account is charged per hour that each NAT gateway instance is running: https://aws.amazon.com/vpc/pricing/#natgatewaypricing

My info:
Terraform v0.11.7
Provider.aws v1.14.0
Module version: 1.30.0

@sc250024
Copy link
Contributor Author

sc250024 commented Apr 17, 2018

As a follow-up, I went through the instructions titled External NAT Gateway IPs on the page https://registry.terraform.io/modules/terraform-aws-modules/vpc/aws/1.30.0 as well. The module still creates 6 NAT gateways, only now 3 of them have the following error in the AWS console:

Elastic IP address [eipalloc-xxxxxxxxxxxxxxxxx] is already associated

I also see in the main.tf where the number 6 comes from:

locals {
  max_subnet_length = "${max(length(var.private_subnets), length(var.elasticache_subnets), length(var.database_subnets), length(var.redshift_subnets))}"
  nat_gateway_count = "${var.single_nat_gateway ? 1 : local.max_subnet_length}"
}

Is the only option to either have one NAT gateway, or max_subnet_length number of NAT gateways? The only reason for one per subnet is if you're really testing the bandwidth limitations of the NAT gateway itself (https://docs.aws.amazon.com/AmazonVPC/latest/UserGuide/vpc-nat-gateway.html).

@robh007
Copy link
Contributor

robh007 commented May 13, 2018

#83 Looks to have switched to use the max_subnet_length, IMHO that's incorrect. You'd either create a single NGW or if your thinking about AZ failure you would create a NGW per AZ within that region. Again there's some assumptions made with subnets and AZs. Irrespective of how many private subnets you have as this shouldn't influence the number of NGWs created.

@sc250024
Copy link
Contributor Author

@robh007 Yes I agree. Basically, the maximum number of NAT gateways created would be the number of availability zones in that region. @antonbabenko Thoughts?

@antonbabenko
Copy link
Member

I remember we've had some discussion about this in the past. It has been evolving, and I tend to agree that after #83 is merged it is a good time to reconsider a number of NAT gateways created.

I see that there are these options:

  1. Single NAT - single_nat_gateway = true
  2. One NAT per subnet - this is how it is implemented now (single_nat_gateway = false)
  3. One NAT per AZ - proper solution in principle, but harder to implement/maintain in Terraform code.

The solution with one NAT per subnet can still be helpful for situations when EIP of NAT gateway should be whitelisted on corp firewall and, at the same time, not all subnets in that AZ should have access to corp network. So, this module needs to maintain all 3 scenarios.

Can somebody come up with a PR to support these scenarios?

@sc250024
Copy link
Contributor Author

@antonbabenko Yes, I'll come up with a PR. Thanks for the input!

@sc250024
Copy link
Contributor Author

#129 created

antonbabenko pushed a commit that referenced this issue May 24, 2018
* Initial commit for issue #117

* Adding documentation for NAT Gateway creation

* Adding `enable_nat_gateway` to the documentation

* Updating README.md with `one_nat_gateway_per_az` to be 'true' by default

* Reverted changes back to `one_nat_gateway_per_az` as 'false'
@github-actions
Copy link

github-actions bot commented Nov 5, 2022

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 5, 2022
waddamski pushed a commit to hmrc/terraform-aws-vpc that referenced this issue Jul 5, 2024
…ality (terraform-aws-modules#129)

* Initial commit for issue terraform-aws-modules#117

* Adding documentation for NAT Gateway creation

* Adding `enable_nat_gateway` to the documentation

* Updating README.md with `one_nat_gateway_per_az` to be 'true' by default

* Reverted changes back to `one_nat_gateway_per_az` as 'false'
waddamski pushed a commit to hmrc/terraform-aws-vpc that referenced this issue Jul 5, 2024
…ality (terraform-aws-modules#129)

* Initial commit for issue terraform-aws-modules#117

* Adding documentation for NAT Gateway creation

* Adding `enable_nat_gateway` to the documentation

* Updating README.md with `one_nat_gateway_per_az` to be 'true' by default

* Reverted changes back to `one_nat_gateway_per_az` as 'false'
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants