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

insecureKubeletReadonlyPortEnabled support #2013

Closed
wyardley opened this issue Jul 26, 2024 · 7 comments · Fixed by #2082 or #2145
Closed

insecureKubeletReadonlyPortEnabled support #2013

wyardley opened this issue Jul 26, 2024 · 7 comments · Fixed by #2082 or #2145
Labels
enhancement New feature or request

Comments

@wyardley
Copy link
Contributor

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 hashicorp/terraform-provider-google#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 #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

@wyardley wyardley added the enhancement New feature or request label Jul 26, 2024
@DrFaust92
Copy link
Contributor

seems this depends on open PR GoogleCloudPlatform/magic-modules#11272 so its not an issue on the module

@wyardley
Copy link
Contributor Author

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
Copy link
Contributor Author

@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
Copy link
Contributor Author

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

@wyardley
Copy link
Contributor Author

wyardley commented Sep 11, 2024

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
hashicorp/terraform-provider-google#15767
hashicorp/terraform-provider-google#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).

wyardley added a commit to wyardley/terraform-google-kubernetes-engine that referenced this issue Sep 11, 2024
wyardley added a commit to wyardley/terraform-google-kubernetes-engine that referenced this issue Sep 13, 2024
wyardley added a commit to wyardley/terraform-google-kubernetes-engine that referenced this issue Sep 17, 2024
@UDtorrey
Copy link

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
Copy link
Contributor Author

wyardley commented Sep 18, 2024

@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.

wyardley added a commit to wyardley/terraform-google-kubernetes-engine that referenced this issue Sep 20, 2024
wyardley added a commit to wyardley/terraform-google-kubernetes-engine that referenced this issue Sep 25, 2024
wyardley added a commit to wyardley/terraform-google-kubernetes-engine that referenced this issue Oct 10, 2024
wyardley added a commit to wyardley/terraform-google-kubernetes-engine that referenced this issue Oct 14, 2024
wyardley added a commit to wyardley/terraform-google-kubernetes-engine that referenced this issue Oct 17, 2024
The upstream provider (intentionally) uses an enum of `"TRUE"` /
`"FALSE"` vs. a boolean. Update the code to follow this, and add a test
case that covers the cluster level setting vs node pool one.

Fixes terraform-google-modules#2013
wyardley added a commit to wyardley/terraform-google-kubernetes-engine that referenced this issue Oct 17, 2024
The upstream provider (intentionally) uses an enum of `"TRUE"` /
`"FALSE"` vs. a boolean. Update the code to follow this, and add a test
case that covers the cluster level setting vs node pool one.

Fixes terraform-google-modules#2013

Co-authored-by: Andrew Peabody <[email protected]>
wyardley added a commit to wyardley/terraform-google-kubernetes-engine that referenced this issue Oct 17, 2024
Add `insecureKubeletReadonlyPortEnabled` to `node_config.kubelet_config`
for the default node-pool and for additional pools. It may also be
necessary to define the top level `node_config` more broadly for the
case where `remove_default_node_pool` is set to false, which should
probably be handled separately.

Also, the upstream provider (intentionally) uses an enum of `"TRUE"` /
`"FALSE"` vs. a boolean. Update the code to follow this, and add a test
case that covers the cluster level setting vs node pool one.

Fixes terraform-google-modules#2013

Co-authored-by: Andrew Peabody <[email protected]>
wyardley added a commit to wyardley/terraform-google-kubernetes-engine that referenced this issue Oct 21, 2024
Add `insecureKubeletReadonlyPortEnabled` to `node_config.kubelet_config`
for the default node-pool and for additional pools. It may also be
necessary to define the top level `node_config` more broadly for the
case where `remove_default_node_pool` is set to false, which should
probably be handled separately.

Also, the upstream provider (intentionally) uses an enum of `"TRUE"` /
`"FALSE"` vs. a boolean. Update the code to follow this, and add a test
case that covers the cluster level setting vs node pool one.

Fixes terraform-google-modules#2013

Co-authored-by: Andrew Peabody <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment