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

Add computed to node_config.kubelet_config & node_pool_auto_config.node_kubelet_config #19817

Conversation

modular-magician
Copy link
Collaborator

Fixes #19792

Support for the node_config.kubelet_config.insecure_kubelet_readonly_port_enabled field was added in GoogleCloudPlatform/magic-modules#11573 and was marked Computed. node_config itself is also Computed, however node_config.kubelet_config is not.

This means that:

  • A configuration that does not specify node_config is not affected
  • A configuration that specifies node_config but not node_config.kubelet_config is affected
  • A configuration that specifies node_config.kubelet_config is not affected.

This caused numerous failing tests for us, however, not all the tests we would have expected to fail based on those criteria.I haven't determined another trigger condition so I suspect it's a partial rollout of a serialisation change. This was confirmed as a rollout in #19792 (comment).

kubelet_config appears in multiple other locations, node_pool_defaults.node_config_defaults.insecure_kubelet_readonly_port_enabled and node_pool_auto_config.node_kubelet_config.insecure_kubelet_readonly_port_enabled

  • node_pool_defaults.node_config_defaults has the same problem, as it is not Computed while node_pool_defaults and insecure_kubelet_readonly_port_enabled are. However, sending just node_pool_defaults {} already triggers the same issue with another field (logging_variant). Ideally we'd make node_config_defaults Required, but I'm not sure about the impact on autopilot. I'll file a bug.
  • I don't expect a default value to be set for node_pool_auto_config.node_kubelet_config.insecure_kubelet_readonly_port_enabled but I've set node_pool_auto_config.node_kubelet_config to Computed anyways since it could exhibit the same issue and I would not want to have to deal with the same problem again. I may cut the change if I can confirm it will never get a default (but I'm not guaranteed / unlikely to find that answer before shipping the change)

The update part of the original issue (Error: googleapi: Error 400: At least one of ['node_version', <snip>, 'max_run_duration'] must be specified.) is because we used ForceSendFields with a nil KubeletConfig value, which does nothing. We had to set KubeletConfig to &container.KubeletConfig{} in that block. NullFields would be correct for a patch in theory, but this is a PUT / customish update and the JSON null still triggers the API error.

After this change we can no longer trigger that specific update message. I'll still clean up that callsite / any similarly structured ones to ensure we send an empty struct properly if we do hit them, but in a separate change.

Release Note Template for Downstream PRs (will be copied)

container: fixed a diff triggered by a new API-side default value for `node_config.0.kubelet_config.0.insecure_kubelet_readonly_port_enabled`. Terraform will now accept server-specified values for `node_config.0.kubelet_config` when it is not defined in configuration and will not detect drift. Note that this means that removing the value from configuration will now preserve old settings instead of reverting the old settings.
container: `google_container_cluster` will now accept server-specified values for `node_pool_auto_config.0.node_kubelet_config` when it is not defined in configuration and will not detect drift. Note that this means that removing the value from configuration will now preserve old settings instead of reverting the old settings.

Derived from GoogleCloudPlatform/magic-modules#11971

…de_kubelet_config (hashicorp#11971)

[upstream:6fa720faca94d244d849be547ecffb482e6a3b75]

Signed-off-by: Modular Magician <[email protected]>
@modular-magician modular-magician merged commit 73471ac into hashicorp:main Oct 10, 2024
4 checks passed
rileykarson pushed a commit to rileykarson/terraform-provider-google that referenced this pull request Oct 11, 2024
…de_kubelet_config (hashicorp#11971) (hashicorp#19817)

[upstream:6fa720faca94d244d849be547ecffb482e6a3b75]

Signed-off-by: Modular Magician <[email protected]>
rileykarson pushed a commit that referenced this pull request Oct 11, 2024
…de_kubelet_config (#11971) (#19817)

[upstream:6fa720faca94d244d849be547ecffb482e6a3b75]

Signed-off-by: Modular Magician <[email protected]>
rileykarson added a commit that referenced this pull request Oct 11, 2024
* Add computed to node_config.kubelet_config & node_pool_auto_config.node_kubelet_config (#11971) (#19817)

[upstream:6fa720faca94d244d849be547ecffb482e6a3b75]

Signed-off-by: Modular Magician <[email protected]>

* Changelog

---------

Signed-off-by: Modular Magician <[email protected]>
Co-authored-by: The Magician <[email protected]>
Copy link

I'm going to lock this pull request 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 related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 10, 2024
@modular-magician modular-magician deleted the downstream-pr-6fa720faca94d244d849be547ecffb482e6a3b75 branch November 17, 2024 00:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

provider setting false values in google_container_node_pool kubelet_config, breaking tf apply
1 participant