Make GKE module cluster_name computed attribute

[Monkeyanator]

Fixes #1181.

Change to the simple_zonal_with_asm module is just to get a passing run in CI with a static cluster name.

[morgante]

Approach looks good, note changes need to be made in autogen/ then run make build.

[comment-bot-dev]

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

[Monkeyanator]

Hm, looks like even after the changes in this PR, CI fails on the ASM module when using a static cluster name:

Step #60 - "converge simple-zonal-with-asm-local":        Error: Invalid template interpolation value
Step #60 - "converge simple-zonal-with-asm-local":        
Step #60 - "converge simple-zonal-with-asm-local":          on ../../../modules/asm/hub.tf line 21, in resource "google_gke_hub_membership" "membership":
Step #60 - "converge simple-zonal-with-asm-local":          21:   membership_id = "${data.google_container_cluster.asm.name}-membership"
Step #60 - "converge simple-zonal-with-asm-local":            ├────────────────
Step #60 - "converge simple-zonal-with-asm-local":            │ data.google_container_cluster.asm.name is null
Step #60 - "converge simple-zonal-with-asm-local":        
Step #60 - "converge simple-zonal-with-asm-local":        The expression result is null. Cannot include a null value in a string
Step #60 - "converge simple-zonal-with-asm-local":        template.
Step #60 - "converge simple-zonal-with-asm-local":        
Step #60 - "converge simple-zonal-with-asm-local":        Error: Invalid template interpolation value
Step #60 - "converge simple-zonal-with-asm-local":        
Step #60 - "converge simple-zonal-with-asm-local":          on ../../../modules/asm/hub.tf line 24, in resource "google_gke_hub_membership" "membership":
Step #60 - "converge simple-zonal-with-asm-local":          24:       resource_link = "//container.googleapis.com/${data.google_container_cluster.asm.id}"
Step #60 - "converge simple-zonal-with-asm-local":            ├────────────────
Step #60 - "converge simple-zonal-with-asm-local":            │ data.google_container_cluster.asm.id is null
Step #60 - "converge simple-zonal-with-asm-local":        
Step #60 - "converge simple-zonal-with-asm-local":        The expression result is null. Cannot include a null value in a string
Step #60 - "converge simple-zonal-with-asm-local":        template.

[morgante]

@Monkeyanator Instead of using depends_on what if you make it the output an implicit reference to the cluster resource?

[Monkeyanator]

Sam Naser Instead of using depends_on what if you make it the output an implicit reference to the cluster resource?

I thought it already was since the value is local.cluster_name = local.cluster_output_name = google_container_cluster.primary.name?

[morgante]

Sam Naser Instead of using depends_on what if you make it the output an implicit reference to the cluster resource?

I thought it already was since the value is local.cluster_name = local.cluster_output_name = google_container_cluster.primary.name?

You're right, it's kind of confusing why this isn't being deferred. What if we try inlining the .id workaround from the other PR? There's no way it could know that before applying.

[bharathkkb]

Adding the workaround to the gke module output directly should work. However if we are not depending on nodepools, a zonal cluster can go into an upgrade after google_container_cluster is resolved and ASM apply could fail. I am thinking we should derive the cluster name from google_container_node_pool[*].id for regular clusters and google_container_cluster.id for autopilot clusters.

[bharathkkb]

Minor comments, LGTM otherwise

[bharathkkb]

Test failing due to #1197

[pramodsetlur]

Hey all, thanks for the fix and comments. I am pointing to the module's master branch and still see the same issue during the apply. Once it occurs on the apply, the issue is seen on the subsequent plans as well. I am creating the GKE cluster alongside with ASM (per the original issue). Not sure what I am missing.

Error: Invalid template interpolation value

  on .terraform/modules/gke_cluster.asm_managed_control_plane/modules/asm/hub.tf line 21, in resource "google_gke_hub_membership" "membership":
  21:   membership_id = "${data.google_container_cluster.asm.name}-membership"
    |----------------
    | data.google_container_cluster.asm.name is null

The expression result is null. Cannot include a null value in a string
template.


Error: Invalid template interpolation value

  on .terraform/modules/gke_cluster.asm_managed_control_plane/modules/asm/hub.tf line 24, in resource "google_gke_hub_membership" "membership":
  24:       resource_link = "//container.googleapis.com/${data.google_container_cluster.asm.id}"
    |----------------
    | data.google_container_cluster.asm.id is null

The expression result is null. Cannot include a null value in a string
template.

Here is the reference to the module:

module "asm_managed_control_plane" {
  count                     = var.asm_managed_control_plane_enabled == true ? 1 : 0
  source                    = "github.com/terraform-google-modules/terraform-google-kubernetes-engine//modules/asm"
  project_id                = var.project_id
  cluster_name              = "${google_container_cluster.gke_cluster.id}"
  cluster_location          = var.region
  enable_cni                = false
  enable_fleet_registration = true
  enable_mesh_feature       = false
}

A few points:

• The cluster_name = "${google_container_cluster.gke_cluster.id}" is not being picked up by data resource even though the cluster seems to have been created. Not sure why it is null.
• I am using a variable asm_managed_control_plane_enabled and install Managed ASM Control plane only if necessary.
• I had to update the source to github.com/terraform-google-modules/terraform-google-kubernetes-engine//modules/asm instead of terraform-google-modules/kubernetes-engine/google//modules/asm (terraform-google-kubernetes-engine vs kubernetes-engine) .
• enable_mesh_feature = false - Setting this to true will fail on apply if the mesh feature is already enabled (from a previous installation of service mesh). Wondering if it can be made idempotent, so that the code and resource can match.

Any help is appreciated. Thanks in advance!

[morgante]

@pramodsetlur Please open a new issue.

[pramodsetlur]

Hmm, as I was writing a new issue, i tried explicitly depending on the gke cluster resource - which seem to have worked.

module "asm_managed_control_plane" {
  depends_on = [
    google_container_cluster.gke_cluster
  ]
  count = var.asm_managed_control_plane_enabled == true ? 1 : 0
  source                    = "github.com/terraform-google-modules/terraform-google-kubernetes-engine//modules/asm"
  project_id                = var.project_id
  cluster_name              = var.cluster_name
  cluster_location          = var.region
  enable_cni                = false
  enable_fleet_registration = true
  # TODO: Mesh feature already enabled in infrak8s-dev project. To be variablized. 
  enable_mesh_feature       = false
}

Thanks for the response.

[morgante]

Glad to hear it!

enable_mesh_feature = false - Setting this to true will fail on apply if the mesh feature is already enabled (from a previous installation of service mesh). Wondering if it can be made idempotent, so that the code and resource can match.

Unfortunately no, this is not possible. The mesh API fails if you try to activate an existing feature that is already enabled.