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

feat!: use shared route table for public subnet #125

Merged
merged 2 commits into from
Mar 21, 2024
Merged

feat!: use shared route table for public subnet #125

merged 2 commits into from
Mar 21, 2024

Conversation

maxsxu
Copy link
Member

@maxsxu maxsxu commented Feb 18, 2024

Motivation

For public subnet we only need one route table, since the egress routes all go to the same internet gateway, and this is the recommended default configuration when creates VPC using AWS wizard.

Modifications

The main modification is:

  • Create only one route table for public subnet instead per-subnet

Also some related modifications along with this PR:

  • Using /20 as the recommend subnet CIDR size since /24 is not enough for current clusters requirements
  • Add subnet discovery tags for AWS Load Balancer Controller when creating subnets
  • Tidy code and docs

Verifying this change

  • Make sure that the change passes the CI checks.

Verified in test env.

Documentation

  • no-need-doc

@maxsxu maxsxu self-assigned this Feb 18, 2024
@maxsxu maxsxu requested a review from a team as a code owner February 18, 2024 06:12
@github-actions github-actions bot added the no-need-doc This pr does not need any document label Feb 18, 2024
@maxsxu maxsxu added the type/enhancement Indicates an improvement to an existing feature label Feb 18, 2024
@@ -63,12 +65,19 @@ resource "aws_internet_gateway" "gw" {

resource "aws_eip" "eip" {
count = var.num_azs
vpc = true
Copy link
Member Author

Choose a reason for hiding this comment

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

This argument is deprecated, so replace it with domain.

Refer: https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/eip#vpc

Copy link
Member

@dpappa dpappa left a comment

Choose a reason for hiding this comment

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

LGTM!

@maxsxu maxsxu merged commit 12e5ff0 into master Mar 21, 2024
6 checks passed
@maxsxu maxsxu deleted the max/vpc branch March 21, 2024 07:59
ciiiii pushed a commit that referenced this pull request May 21, 2024
🤖 I have created a release *beep* *boop*
---


##
[3.0.0](v2.8.0...v3.0.0)
(2024-05-21)


### ⚠ BREAKING CHANGES

* use shared route table for public subnet
([#125](#125))

### Features

* add new output eks which contains all outputs of module.eks
([#131](#131))
([6f7739e](6f7739e))
* add output eks for provide convenient approach to access eks module's
all outputs
([6f7739e](6f7739e))
* **cluster_autoscaler:** removed old k8s versions, added new ones
([#120](#120))
([853aba8](853aba8))
* Disable nodepool logging to cloudwatch by default
([#126](#126))
([c9be3c1](c9be3c1))
* support disable nat gateway and use public subnet
([#132](#132))
([4c1b508](4c1b508))
* Support single zone node_group
([#133](#133))
([8038bdf](8038bdf))
* use shared route table for public subnet
([#125](#125))
([12e5ff0](12e5ff0))


### Bug Fixes

* Correct default value
([#128](#128))
([25d8171](25d8171))
* Optimize external-dns args to reduce api calls
([#124](#124))
([5aa0166](5aa0166))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-need-doc This pr does not need any document type/enhancement Indicates an improvement to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants