Support node_kublet_config in node_pool_auto_config so users can disable the kubelet read-only port

[wyardley]

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request.
• Please do not leave +1 or me too comments, they generate extra noise for issue followers and do not help prioritize the request.
• If you are interested in working on this issue or have submitted a pull request, please leave a comment.
• If an issue is assigned to a user, that user is claiming responsibility for the issue.
• Customers working with a Google Technical Account Manager or Customer Engineer can ask them to reach out internally to expedite investigation and resolution of this issue.

Description

[comes from a comment from @hoskeri in #15208]

It appears that node_pool_auto_config doesn't currently support node_kublet_config.
I'm not sure how necessary it is for autopilot clusters (hopefully Google provides sensible defaults there), but there was discussion about adding support for some or all of the parameters supported there for node_pool_auto_config as well, similar to some of the other blocks / resources it's supported for.

New or Affected Resource(s)

• google_container_cluster

Potential Terraform Configuration

resource "google_container_cluster" "autopilot" {
  name               = "autopilot"
  location           = "us-central1-f"
  enable_autopilot = true

  node_pool_auto_config {
    kubelet_config {
       # supported params set here
    }
  }

  deletion_protection = false
  network             = "default"
  subnetwork          = "default"
}

References

#15208
GoogleCloudPlatform/magic-modules#11272

b/362277444

[wyardley]

The good news is that I made pretty good progress on this. I'll put up a draft or a PR shortly.

However, even though it uses NodeKubeletConfig, https://pkg.go.dev/google.golang.org/api/container/v1#NodePoolAutoConfig mentions:

Currently only insecure_kubelet_readonly_port_enabled can be set here.

So I'm not sure if it makes sense to implement it using the same code / schema as other KubeletConfig stuff or not. Another problem with it being this way is that cpu_manager_policy must be set, but then will have permadrift unless set to something other than an empty string. I'm starting to think that maybe it should be required (tangentially related to #19225)

Also, naming-wise, I think it makes sense to match the upstream as originally planned:

  node_pool_auto_config {
    node_kubelet_config {}
  }

though maybe an argument could be made for matching kubelet_config as we have elsewhere?

[melinath]

the naming should match the API - and FWIW even if the API supported other child fields I'd be inclined to only add the read-only port for now so that we can backport more easily.

[melinath]

(This is probably the only remaining ticket in this cluster that we'd consider for backporting, barring further evidence of impact.)

[wyardley]

barring further evidence of impact.

GoogleCloudPlatform/magic-modules#11572 possibly might be worth considering too, if we can get it narrow enough - otherwise, I'm guessing users of the GKE cluster module might see some issues if they don't set it to ""

[melinath]

Although that is definitely a confusing bug, it probably doesn't rise to the level of backport, because it has a clear workaround and doesn't cause the cluster to get recreated. (Even if it did, that might not be sufficient for a backport on its own.) The read-only port fields are an exceptional case because they involve giving users a workaround for a known breaking change that will be made on the API side in the near future.

[github-actions]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.