Move to using for_each for node pools #257
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Dev25
commented
Sep 5, 2019
Dev25
requested review from
aaron-lane,
adrienthebo,
Jberlinsky and
morgante
as code owners
morgante
suggested changes
Nov 20, 2019
autogen/cluster.tf
Outdated
| @@ -225,62 +225,63 @@ resource "google_container_node_pool" "pools" { | |||
| {% else %} | |||
| provider = google | |||
| {% endif %} | |||
| count = length(var.node_pools) | |||
| name = var.node_pools[count.index]["name"] | |||
| for_each = var.node_pools | |||
There was a problem hiding this comment.
Let's keep the external interface as list but build the map internally within the module.
There was a problem hiding this comment.
Will keep the existing interface via having this in locals:
node_pool_names = [for np in toset(var.node_pools) : np.name]
node_pools = zipmap(local.node_pool_names, tolist(toset(var.node_pools)))
|
@Dev25 Just checking if you think you'll be able to make the requested changes (and rebase)? |
|
Yep, hopefully within the next week or 2 @morgante |
|
Rebased and kept the same external interface @morgante Will be porting over my existing clusters to this rebased version this week which should further test it, PTAL |
|
@morgante Can you take another look now? CI is passing after the recent master fixes. |
morgante
approved these changes
Jan 22, 2020
kri5
pushed a commit
to kri5/terraform-google-kubernetes-engine
that referenced
this pull request
Jan 27, 2020
…#257) BREAKING CHANGE: moves node pool state location to allow using for_each on them
viglesiasce
pushed a commit
to GoogleCloudPlatform/solutions-modern-cicd-anthos
that referenced
this pull request
May 5, 2020
Also: * Pinning the terraform-google-kubernetes-engine module to 6.2.0 due to breaking change: terraform-google-modules/terraform-google-kubernetes-engine#257 * Cleaning buckets before removing them (TF fails otherwise) Fixes: http://b/148036307 Change-Id: Icde3335ee8690190ac87c46a9cbde265f263e84c
CPL-markus
pushed a commit
to WALTER-GROUP/terraform-google-kubernetes-engine
that referenced
this pull request
Jul 15, 2024
…#257) BREAKING CHANGE: moves node pool state location to allow using for_each on them
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Use for_each instead of count/index for managing node pools. This is going to be a breaking change for users and they will need to manually edit the state to migrate node pools over.
State Migration:
terraform state rm module.<name>.google_container_node_pool.poolsterraform import 'module.<name>.google_container_node_pool.pools["name"]' {region}/{cluster}{name}terraform import 'module.gke.google_container_node_pool.pools["spot-pool"]' europe-west1/dev-cluster/spot-poolFixes #128