cloudposse / terraform-aws-dynamic-subnets

Terraform module for public and private subnets provisioning in existing VPC
https://cloudposse.com/accelerate
Apache License 2.0
197 stars 167 forks source link

PROPOSAL: Single NGW option #154

Closed nnsense closed 2 years ago

nnsense commented 2 years ago

what

why

jamengual commented 2 years ago

/test all

osterman commented 2 years ago

Why even bother running in multiple AZs if you only deploy a single NGW? If you lose the AZ with the NGW, all other AZs lose egress. If the concern is cost, then reduce the number of AZs, not the number of NGWs. It's a false sense of redundancy.

What's the counter argument?

jamengual commented 2 years ago

Why even bother running in multiple AZs if you only deploy a single NGW? If you lose the AZ with the NGW, all other AZs lose egress. If the concern is cost, then reduce the number of AZs, not the number of NGWs. It's a false sense of redundancy.

What's the counter argument?

In accounts like audit, security, etc you might do more management than deploying services and on those, you might not even have critical services in them but you might need to have a way to connect to the internet but not highly available (like for sending reports, analytics etc)

In an account that you use for back end services connected through a TGW and you have multiple RDS clusters that are highly available but the egress is through the TGW and not the NGW you might not need that many NGWs

in companies with low budgets.

nnsense commented 2 years ago

Hi @osterman , that is of course a good point, but in our case we didn't want to change the infrastructure, just pay for a single NGW while developing with a single server which we had the chance to run in HA for testing (still using a single NGW) and the reduce back to single. At the time we needed to save money, and I've been using this branch to deploy a single NGW

jamengual commented 2 years ago

/test all

jamengual commented 2 years ago

@nnsense can you address the comment?

nnsense commented 2 years ago

Hi @jamengual - Sorry I'm a bit lost, what comment I need to address?

jamengual commented 2 years ago

Andry comment about the name of the locals

On Sat., Apr. 16, 2022, 2:53 a.m. nnsense, @.***> wrote:

Hi @jamengual https://github.com/jamengual - Sorry I'm a bit lost, what comment I need to address?

— Reply to this email directly, view it on GitHub https://github.com/cloudposse/terraform-aws-dynamic-subnets/pull/154#issuecomment-1100624623, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQ3ERHJUNUZHIM6ZNQVTTTVFKEZ7ANCNFSM5TNK7TZA . You are receiving this because you were mentioned.Message ID: @.***>

nnsense commented 2 years ago

Done, thanks I've missed that comment :)

jamengual commented 2 years ago

/test all

osterman commented 2 years ago

just pay for a single NGW while developing with a single server which we had the chance to run in HA for testing (still using a single NGW) and the reduce back to single. At the time we needed to save money, and I've been using this branch to deploy a single NGW

In that case, why run more than one subnet if running a single server?

osterman commented 2 years ago

you might need to have a way to connect to the internet but not highly available (like for sending reports, analytics etc)

Yep, I don't disagree wit the use-case. But then why have multiple AZs instead of single AZ? A single AZ communicates that it's not HA.

jamengual commented 2 years ago

you might need to have a way to connect to the internet but not highly available (like for sending reports, analytics etc)

Yep, I don't disagree wit the use-case. But then why have multiple AZs instead of single AZ? A single AZ communicates that it's not HA.

Erik, if this was an account that has only Aurora RDS clusters and the internet is not the primary ingress then you might not want to run multiple nat gateways, I know this is not what ClousPosse recommends but it is a very common practice and that is why this option is disabled by default.

osterman commented 2 years ago

Erik, if this was an account that has only Aurora RDS clusters and the internet is not the primary ingress then you might not want to run multiple nat gateways,

Okay, I can accept that.

jamengual commented 2 years ago

@nnsense please look at the new comments

jamengual commented 2 years ago

@nnsense can you address the rename change from Erik ( I know we asked for this before, sorry) so we can get this merged? thanks.

nnsense commented 2 years ago

Hi @jamengual @osterman Sorry for the delay, busy weeks :) It's actually a good idea, basically we can set the number of NGWs or leave it to default which is == length(var.availability_zones) , I like it! I will change it over the weekend, have a nice one :) Matt

nnsense commented 2 years ago

@Nuru @jamengual I've implemented the change, apparently too late. What does "Supersedes" means? Is the mentioned PR covering this change too? If it isnt should I open a new PR? https://github.com/nnsense/terraform-aws-dynamic-subnets

nnsense commented 2 years ago

@Nuru @jamengual @osterman I've checked version 2, and I see that the max number of NGW has been implemented. One comment here to notify me would have been great, no one is here to waste time. Thanks.