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

Make node_pool_labels, node_pool_tags, and node_pool_taints all optional #3

Closed
morgante opened this issue Aug 30, 2018 · 8 comments · Fixed by #318
Closed

Make node_pool_labels, node_pool_tags, and node_pool_taints all optional #3

morgante opened this issue Aug 30, 2018 · 8 comments · Fixed by #318
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@morgante
Copy link
Contributor

morgante commented Aug 30, 2018

Desired flow:

  1. Defaults are available for the entire variable.
  2. I can override the variable for one node pool without having to set a value for the whole variable.
@aaron-lane aaron-lane added enhancement New feature or request good first issue Good for newcomers labels May 3, 2019
@kiran002
Copy link

Could you elaborate a bit more? Because from the definition all of these variable seem to have default values (https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/blob/master/variables.tf#L167-L170) and if i am not wrong, that should enough correct?

@morgante
Copy link
Contributor Author

This might be out of date, but the specific desire is:

  1. Defaults are available for the entire variable.
  2. I can override the variable for one node pool without having to set a value for the whole variable.

@evkuzin evkuzin self-assigned this Sep 11, 2019
@evkuzin
Copy link

evkuzin commented Sep 19, 2019

@morgante @aaron-lane It could be move to done actually.
Now you could just add block of tags/labels/taints with name of pool and it will not affect other pools

variable "node_pools_labels" {
  type        = map(map(string))
  description = "Map of maps containing node labels by node-pool name"

  default = {
    all               = {}
    default-node-pool = {}
  }
}

@evkuzin evkuzin removed their assignment Sep 19, 2019
@morgante
Copy link
Contributor Author

@evkuzin What if I have a custom node pool? Do I have to set the node_pools_labels to set an empty map for that custom pool.

@evkuzin
Copy link

evkuzin commented Sep 24, 2019

@morgante exactly you need to create object inside node_pools_labels with name equal to node pool name and set it to {}

labels = merge(
      {
        "cluster_name" = var.name
      },
      {
        "node_pool" = var.node_pools[count.index]["name"]
      },
      var.node_pools_labels["all"],
      var.node_pools_labels[var.node_pools[count.index]["name"]],
    )

@morgante
Copy link
Contributor Author

So this has not been fixed.

As stated in the original issue, the ask is that I don't have to specify node_pools_labels at all if I don't want to set any custom labels.

@evkuzin
Copy link

evkuzin commented Sep 25, 2019

Okay, I understand now.

mmontan added a commit to mmontan/terraform-google-kubernetes-engine that referenced this issue Sep 30, 2019
# This is the 1st commit message:

Initial definition of a Safer Cluster module.

# This is the commit message terraform-google-modules#2:

Add a sample for using the safer-cluster module.

# This is the commit message terraform-google-modules#3:

Add a test kitchen instance

# This is the commit message terraform-google-modules#4:

Formatting TF files.

# This is the commit message terraform-google-modules#5:

Add a test for the safer-cluster module

# This is the commit message terraform-google-modules#6:

Additional fixes
@naseemkullah
Copy link
Contributor

I find myself having to do this with clusters whose default-pool is deleted and custom pools default and preemptible are added.


  node_pools_metadata = {
    all         = {}
    default     = {}
    preemptible = {}
  }

  node_pools_labels = {
    all = {
      env = var.env
    }
    default     = {}
    preemptible = {}
  }

  node_pools_taints = {
    all         = []
    default     = []
    preemptible = []
  }

  node_pools_tags = {
    all         = []
    default     = []
    preemptible = []
  }

My understanding is that #269 will allow me to not specify the empty values, is that correct?

@cray0000 cray0000 self-assigned this Nov 13, 2019
CPL-markus pushed a commit to WALTER-GROUP/terraform-google-kubernetes-engine that referenced this issue Jul 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
6 participants