data/aws: create fewer NAT gateways #1450
Conversation
openshift-ci-robot
added
the
size/S
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: eparis If they are not already assigned, you can assign the PR to them by writing The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| resource "aws_nat_gateway" "nat_gw" { | ||
| count = "min(${local.new_az_count}, 3)" | ||
| allocation_id = "${aws_eip.nat_eip.*.id[count.index]}" | ||
| subnet_id = "${aws_subnet.public_subnet.*.id[count.index]}" |
There was a problem hiding this comment.
Why move these public-subnet resources into vpc-private.tf?
There was a problem hiding this comment.
because they are private subnet resources, not public. the move should happen irrespective of this PR
There was a problem hiding this comment.
because they are private subnet resources, not public...
Ah, because while this resource lives in the public subnet, its purpose is to service instances in the private subnets.
the move should happen irrespective of this PR
Personally I'd rather collapse vpc-private.tf and vpc-public.tf together. The unified file would only be ~100 lines. I'd work up a PR, but there are other PRs in flight touching this code and I don't think we could get this file-rename landed before the coming freeze.
|
Is this a trade-off between cost/limits (favoring NAT/EIP sharing) and external-net latency (favoring per-zone-with-instances NATs)? |
Instead of making 1 nat gateway for every subnet, make a nat gateway in 3 subnets and use those in all subnets
|
@eparis: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
it gets the eip limit to 3, which co-incide with the 3 subnets we use out of the box. It would create some cross az faults for potential workers. If us-east-1a goes out you'll lose internet access from nodes in 1a and 1d |
|
I've pushed #1481 with an alternative approach that only creates subnet infrastructure in a given zone when we have instances configured for that zone. So if for some reason you wanted a six-zone cluster in us-east-1, you'd still need six EIPs (vs. this PR's three). But all nodes are getting out through a NAT in their own subnet (vs. this PR's cross-zone traffic as mentioned in your last comment). |
Instead of making 1 nat gateway for every subnet, make a nat gateway in 3
subnets and use those in all subnets