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!: Add support for enable_l4_ilb_subsetting flag #896

Conversation

Jberlinsky
Copy link
Contributor

@Jberlinsky Jberlinsky commented May 16, 2021

Adds support for the google_container_cluster enable_l4_ilb_subsetting flag, which is particularly useful for clusters that require more than 250 total Nodes and build ILBs.

Note that enable_l4_ilb_subsetting is only supported in google-beta >= 3.63.0. Minimum module versions are bumped in accordance with this version change.

@Jberlinsky Jberlinsky requested review from bharathkkb and a team as code owners May 16, 2021 03:42
@comment-bot-dev
Copy link

comment-bot-dev commented May 16, 2021

Thanks for the PR! 🚀
✅ Lint checks have passed.

@Jberlinsky Jberlinsky changed the title Add support for enable_l4_ilb_subsetting flag [WIP] Add support for enable_l4_ilb_subsetting flag May 16, 2021
@Jberlinsky Jberlinsky force-pushed the feature/add-support-for-l4-ilb-subsetting branch from cc6732a to 7bcf966 Compare May 16, 2021 21:31
@Jberlinsky Jberlinsky changed the title [WIP] Add support for enable_l4_ilb_subsetting flag Add support for enable_l4_ilb_subsetting flag May 16, 2021
@Jberlinsky
Copy link
Contributor Author

@bharathkkb @morgante @terraform-google-modules/cft-admins this is ready for review :)

@Jberlinsky Jberlinsky changed the title Add support for enable_l4_ilb_subsetting flag Add support for enable_l4_ilb_subsetting flag May 16, 2021
@Jberlinsky Jberlinsky changed the title Add support for enable_l4_ilb_subsetting flag Add support for enable_l4_ilb_subsetting flag May 16, 2021
Copy link
Member

@bharathkkb bharathkkb left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @Jberlinsky
Since we are bumping min provider versions this will be a breaking change.

autogen/main/variables.tf.tmpl Outdated Show resolved Hide resolved
@bharathkkb bharathkkb changed the title Add support for enable_l4_ilb_subsetting flag feat!: Add support for enable_l4_ilb_subsetting flag May 17, 2021
Jberlinsky and others added 2 commits May 16, 2021 22:56
Remove unnecessary mention of `enabled` RE: `enable_l4_ilb_subsetting` variable

Co-authored-by: Bharath KKB <bharathkrishnakb@gmail.com>
@bharathkkb bharathkkb merged commit 7531f90 into terraform-google-modules:master May 17, 2021
@release-please release-please bot mentioned this pull request May 17, 2021
@gauravkr19
Copy link

Hi @Jberlinsky @bharathkkb

After this commit - 7531f90,
I am getting the following error:

Error: Error waiting for creating GKE cluster: Failed to create cluster

on .terraform/modules/jenkins-gke/modules/beta-public-cluster/cluster.tf line 22, in resource "google_container_cluster" "primary":
22: resource "google_container_cluster" "primary" {

Is the issue because of this Change or global issue with GKE
https://status.cloud.google.com/incidents/sqeWSRmcrJZyE2zSrJ74

Appreciate any help on this.

@bharathkkb
Copy link
Member

Hi @gauravkr19
Our integration tests which spins up the clusters passed and this flag is false by default. I am inclined to say it is not related to this change. If you are able to reproduce, please open an issue with that and we can debug further.

@gauravkr19
Copy link

Thanks @bharathkkb for the quick response.

Can get the cluster up now, must be due to the outage at that point of time(tried 3 times).

By the way, I am implementing the jenkins-pipeline from below article and its working with terraform 0.12.24.
Will have to update the terraform version soon.
https://cloud.google.com/architecture/managing-infrastructure-as-code-with-terraform-jenkins-and-gitops

CPL-markus pushed a commit to WALTER-GROUP/terraform-google-kubernetes-engine that referenced this pull request Jul 15, 2024
…gle-modules#896)

* Add support for enable_l4_ilb_subsetting flag

* Fix typo

* Bump minimum google-beta provider version

* Bump google-provider-beta version in test definitions

* Update autogen/main/variables.tf.tmpl

Remove unnecessary mention of `enabled` RE: `enable_l4_ilb_subsetting` variable

Co-authored-by: Bharath KKB <bharathkrishnakb@gmail.com>

* Remove unnecessary `enabled` identifier in description

Co-authored-by: Bharath KKB <bharathkrishnakb@gmail.com>
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

4 participants