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 extra eip creation logic #118

Merged
merged 1 commit into from
May 22, 2024
Merged

fix extra eip creation logic #118

merged 1 commit into from
May 22, 2024

Conversation

ravisinghkr
Copy link
Contributor

πŸ“² What

Fix to avoid creation of 3 EIPs (per AZ for NAT) if we are creating just 1 NAT Gateway.

πŸ€” Why

We dont want to create unnecessary resources in our environments.

πŸ›  How

Adding a check while creating the aws_eip resource.

πŸ‘€ Evidence

Screenshot 2024-05-21 at 17 15 23

πŸ•΅οΈ How to test

Notes on how a reviewer can test the changes, e.g. how to run the tests.

βœ… Acceptance criteria Checklist

  • Code peer reviewed?
  • Documentation has been updated to reflect the changes?
  • Passing all automated tests, including a successful deployment?
  • Passing any exploratory testing?
  • Rebased/merged with latest changes from development and re-tested?
  • Meeting the Coding Standards?

@ElvenSpellmaker
Copy link
Contributor

Hey, aren't all the IPs used even in the single mode?

@ravisinghkr
Copy link
Contributor Author

ravisinghkr commented May 22, 2024

Hey, aren't all the IPs used even in the single mode?

not really, 2 are just created and never used. Only one of them is used by the NAT gateway

resource "aws_nat_gateway" "public" {
  count = var.vpc_nat_gateway_per_az ? length(data.aws_availability_zones.available.names) : 1

  allocation_id = aws_eip.nat[count.index].id
  subnet_id     = aws_subnet.public[count.index].id

  tags = merge(var.tags, {
    "Name" = "${var.vpc_name}-public-nat-${data.aws_availability_zones.available.names[count.index]}"
  })

@ElvenSpellmaker
Copy link
Contributor

Hey, aren't all the IPs used even in the single mode?

not really, 2 are just created and never used. Only one of them is used by the NAT gateway

resource "aws_nat_gateway" "public" {
  count = var.vpc_nat_gateway_per_az ? length(data.aws_availability_zones.available.names) : 1

  allocation_id = aws_eip.nat[count.index].id
  subnet_id     = aws_subnet.public[count.index].id

  tags = merge(var.tags, {
    "Name" = "${var.vpc_name}-public-nat-${data.aws_availability_zones.available.names[count.index]}"
  })

Fair enough, in that case LGTM.

@ravisinghkr ravisinghkr merged commit 768b12b into master May 22, 2024
2 checks passed
@ravisinghkr ravisinghkr deleted the fix_extra_eip_creation branch May 22, 2024 11:02
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

3 participants