advanced_machine_features and queued_provisioning forces replacement of node pool created through UI

[AlDemion]

TL;DR

Seems like values that are set during the creation of node pool through UI are incompatible with existing module defaults which leads to node pool recreation, and there is no way to specify desired values in the module inputs.

Expected behavior

Successful import of the resource without recreation

Observed behavior

- advanced_machine_features { # forces replacement
    - enable_nested_virtualization = false -> null
    - threads_per_core             = 0 -> null
  }

- queued_provisioning { # forces replacement
    - enabled = false -> null
  }

Terraform Configuration

node_pools = [
    {
      name               = "nodepool"
      auto_repair        = true
      auto_upgrade       = false
      initial_node_count = 1
      min_count          = 1
      max_count          = 44
      max_surge          = 6
      machine_type       = "e2-custom-24-98304"
      version            = "1.30.5-gke.1713000"
      preemptible        = false
      enable_nested_virtualization = false
      threads_per_core = 0
    },
]

Terraform Version

1.11.1

Terraform Provider Versions

google = {
      source  = "hashicorp/google"
      version = ">= 6.14.0, < 7"
    }
    google-beta = {
      source  = "hashicorp/google-beta"
      version = ">= 6.14.0, < 7"
    }
    kubernetes = {
      source  = "hashicorp/kubernetes"
      version = "~> 2.10"
    }
    random = {
      source  = "hashicorp/random"
      version = ">= 2.1"
    }

Additional information

No response

[cwh-hcl]

I encountered this issue as well. If a user explicitly specifies enable_nested_virtualization or threads_per_core regardless of the value given it should create the advanced_machine_features block to avoid having to replace entire nodepool. Same with the queued_provisioning block if queued_provisioning is explicitly specified.

Can confirm this issue is also in the private cluster.

[cwh-hcl]

I was able to work around this by updating our local copy of the module to fix this by using the below code update check the condition of not null instead for the dynamic "advanced_machine_features" block. Similarly done for the dynamic "queued_provisioning" block as well to just check condition of not null.

Is anyone able to review my changes to see if it makes sense to make these changes in the upcoming release of this module where these blocks are being called?

Old:

for_each = lookup(each.value, "threads_per_core", 0) > 0 || lookup(each.value, "enable_nested_virtualization", false) ? [1] : []

New:

for_each = lookup(each.value, "threads_per_core", null) != null || lookup(each.value, "enable_nested_virtualization", null) != null ? [1] : []

[DrFaust92]

Ill try to take a look

[cwh-hcl]

@DrFaust92 appreciate that! Curious if any updates on getting this in?

Each time our org updates to the latest module I am having to manually make these customizations to the module to avoid re-creating the production node pools and causing down time.

[cwh-hcl]

Confirming this is still an issue in version 38.1.0 of the terraform GKE module.

[cwh-hcl]

Here is the code I changed in my local copy of the module to fix. Feel free to use as reference:

dynamic "queued_provisioning" {
    #CHARLIE CHANGED the for_each to check for null instead of false due to
    #bug github bug https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/issues/2304
    # for_each = lookup(each.value, "queued_provisioning", false) ? [true] : []
    for_each = lookup(each.value, "queued_provisioning", null) != null ? [true] : []
    content {
      enabled = lookup(each.value, "queued_provisioning", null)
    }
  }

dynamic "advanced_machine_features" {
      #CHARLIE CHANGED the for_each to check for null instead of false due to
      #bug github bug https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/issues/2304
      #for_each = lookup(each.value, "threads_per_core", 0) > 0 || lookup(each.value, "enable_nested_virtualization", false) || lookup(each.value, "performance_monitoring_unit", null) != null ? [1] : []
      for_each = lookup(each.value, "threads_per_core", null) != null || lookup(each.value, "enable_nested_virtualization", null) != null ? [1] : []
      content {
        threads_per_core             = lookup(each.value, "threads_per_core", 0)
        enable_nested_virtualization = lookup(each.value, "enable_nested_virtualization", null)
        performance_monitoring_unit  = lookup(each.value, "performance_monitoring_unit", null)
      }
    }

[github-actions]

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days

[cwh-hcl]

Re-commenting as this issue still affects us, but the "stale" label got applied to it due to no activity. It is still an issue in the latest modules. I proposed a fix in the comments above.

[ItsMattL]

@cwh-hcl have you created a pull request for your fix?

[cwh-hcl]

@cwh-hcl have you created a pull request for your fix?

I did not. I am not one of the developers of the module. If I get some time I can try to figure out how to put one together, but the general idea of the code changes to fix this are shown above

[ItsMattL]

I'm also affected and willing to submit a PR for it, but didn't know if you wanted to be the one to take credit for the fix by doing so.