terraform-google-modules / terraform-google-kubernetes-engine

Configures opinionated GKE clusters
https://registry.terraform.io/modules/terraform-google-modules/kubernetes-engine/google
Apache License 2.0
1.13k stars 1.16k forks source link

insecureKubeletReadonlyPortEnabled support #2013

Open wyardley opened 1 month ago

wyardley commented 1 month ago

TL;DR

Is the insecureKubeletReadonlyPortEnabled flag in the kubelet config supported? With the recent announcements https://cloud.google.com/kubernetes-engine/docs/how-to/disable-kubelet-readonly-port#migrate-apps, I updated the cluster configs.

Probably depends on https://github.com/hashicorp/terraform-provider-google/issues/15208

After running:

% gcloud container clusters update xxx \
    --location=us-central1 --project xxx \
    --no-enable-insecure-kubelet-readonly-port
% gcloud container node-pools update primary \     
    --cluster=cluster --project xxx \
    --location=us-central1 \                  
    --no-enable-insecure-kubelet-readonly-port

I seem to be getting some unresolvable diffs with the kubelet config. I might be just running into something related to https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/pull/1922 but I believe I did check the configs after updating to the latest module version, and I didn't have any luck just by setting pod_pids_limit and cpu_cfs_quota in the pod config.

diffs:

          - kubelet_config {
              - cpu_cfs_quota  = false -> null
              - pod_pids_limit = 0 -> null
            }

I can see this in the debug logs

2024-07-26T11:40:22.622-0700 [DEBUG] provider.terraform-provider-google:   "kubeletConfig": {
2024-07-26T11:40:22.622-0700 [DEBUG] provider.terraform-provider-google:    "insecureKubeletReadonlyPortEnabled": false
2024-07-26T11:40:22.622-0700 [DEBUG] provider.terraform-provider-google:   },

Terraform Resources

No response

Detailed design

No response

Additional information

No response

DrFaust92 commented 1 month ago

seems this depends on open PR https://github.com/GoogleCloudPlatform/magic-modules/pull/11272 so its not an issue on the module

wyardley commented 3 weeks ago

Side note: with latest provider and module, I was getting a permadiff closer to this (when that setting was changed); presumably because another field under nodeConfig.kubeletConfig is returning something)

      ~ node_config {
         [...]
          ~ kubelet_config {
              + cpu_manager_policy = "static"
                # (2 unchanged attributes hidden)
            }

            # (2 unchanged blocks hidden)
        }

For me, setting that to the undocumented value of "" in the first node pool solved the permadiff (and doesn't seem to cause issues with other settings, including the insecure kubelet one) in the short term.

Also filed a separate issue for that here

wyardley commented 3 weeks ago

@DrFaust92 Should be supported in upcoming 6.x releases and is also supposed to get cherry-picked into 5.x

(also, sorry for causing some conflicts for your GCFS PR)

wyardley commented 2 weeks ago

It will be worth keeping an eye out for issues (in the module) related to https://github.com/hashicorp/terraform-provider-google/issues/15767 once this is supported.

wyardley commented 1 week ago

FWIW, this is now out in v5.44.0 and v6.2.0.

https://github.com/hashicorp/terraform-provider-google/releases/tag/v5.44.0 https://github.com/hashicorp/terraform-provider-google/releases/tag/v6.2.0

I'm throwing up a pass at adding the support in #2082

That said, there may be some quirks when people start adding the setting, especially before https://github.com/hashicorp/terraform-provider-google/issues/15767 https://github.com/hashicorp/terraform-provider-google/issues/19225 are resolved -- setting cpu_manager_policy explicitly to "" when it's not set in the module might help as a short term workaround, though (see comment again).

UDtorrey commented 9 hours ago

similar to OP, trying to disable the insecure kubelet readonly port.

looking at feat(TPG>=5.44)!: add support for insecureKubeletReadonlyPortEnabled, I see this was updated in numerous modules, beta-private-cluster, beta-public-cluster, private-cluster-update-variant, etc.

We use beta-autopilot-private-cluster, but i dont see that updated? Perhaps I'm missing it, or it's downstream of beta-autopilot-private-cluster

Is there guidance on how to proceed with modules that were not updated? Will insecure_kubelet_readonly_port_enabled be supported in all modules, or just a select few?

Note: I have not executed the gcloud container clusters update xxx --no-enable-insecure-kubelet-readonly-port to identify any resulting drift from the TF. I'd prefer not to create drift (due to internal politics).

wyardley commented 9 hours ago

@UDtorrey with my limited testing so far, if you've set the value out of band, since it's optional / computed at the provider level, you shouldn't need to change any settings or see drift (other than getting a "state changed" notification), even if you have a provider version that supports the attribute.

That said, it's unclear whether the fixes for some of the bugs / issues causing problems with 5.xx will be cherry-picked into 5.x or be 6.x only... but since you've made the changes directly, once some of the other issues with related / similar fields are resolved, you may be able to have the state apply cleanly even without direct support for this parameter in the module.

Right now, I'm using the latest of this module (with a non autopilot cluster) and 6.2 TPG, and I see just the gcfs_config perrmadrift that will soon be fixed.

I ran into some other issues with the draft PR I currently have w/r/t autopilot specifically. If you look at the comments there, though, I think the intent is to try and support it (using the same parameter name), either in that PR or in a followup, but there are some complexities with bugs that affect different provider versions for the autopilot and non-autopilot variants.

Note: I don't work for Google, and this is not an "official" answer to your questions about what the module will / won't support, but hope it helps.