Added windows_node_config support for GKE Clusters #8265
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
Hello! I am a robot who works on Magic Modules PRs. I've detected that you're a community contributor. @roaks3, a repository maintainer, has been assigned to assist you and help review your changes. ❓ First time contributing? Click here for more detailsYour assigned reviewer will help review your code by:
You can help make sure that review is quick by running local tests and ensuring they're passing in between each push you make to your PR's branch. Also, try to leave a comment with each push you make, as pushes generally don't generate emails. If your reviewer doesn't get back to you within a week after your most recent change, please feel free to leave a comment on the issue asking them to take a look! In the absence of a dedicated review dashboard most maintainers manage their pending reviews through email, and those will sometimes get lost in their inbox. |
modular-magician
added
awaiting-approval
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 4 files changed, 160 insertions(+)) Missing test reportYour PR includes resource fields which are not covered by any test. Resource: Please add acceptance tests which include these fields. |
Tests analyticsTotal tests: Action takenFound 41 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccContainerNodePool_ephemeralStorageLocalSsdConfig|TestAccContainerNodePool_ephemeralStorageConfig|TestAccContainerNodePool_withSoleTenantConfig|TestAccContainerNodePool_shieldedInstanceConfig|TestAccContainerNodePool_EmptyGuestAccelerator|TestAccContainerCluster_withNodePoolNodeConfig|TestAccContainerCluster_withSoleTenantGroup|TestAccContainerNodePool_012_ConfigModeAttr|TestAccContainerCluster_withILBSubsetting|TestAccContainerCluster_withConfidentialNodes|TestAccContainerNodePool_withNodeConfigScopeAlias|TestAccContainerNodePool_withManagement|TestAccContainerNodePool_withGPU|TestAccContainerNodePool_withWindowsNodeConfig|TestAccContainerNodePool_withEnablePrivateNodesToggle|TestAccContainerNodePool_withLinuxNodeConfig|TestAccContainerNodePool_withSandboxConfig|TestAccContainerNodePool_withKubeletConfig|TestAccContainerNodePool_withReservationAffinitySpecific|TestAccContainerNodePool_withNodeConfig|TestAccContainerNodePool_withWorkloadIdentityConfig|TestAccContainerNodePool_withReservationAffinity|TestAccContainerCluster_backend|TestAccContainerNodePool_withLoggingVariantUpdates|TestAccContainerCluster_withBootDiskKmsKey|TestAccContainerCluster_withSandboxConfig|TestAccContainerCluster_withNodeConfigReservationAffinitySpecific|TestAccContainerCluster_withNodeConfigReservationAffinity|TestAccContainerCluster_withNodeConfigShieldedInstanceConfig|TestAccContainerCluster_withNodeConfigScopeAlias|TestAccContainerCluster_withLoggingVariantInNodePool|TestAccContainerCluster_withLoggingVariantInNodeConfig|TestAccContainerCluster_withWorkloadMetadataConfig|TestAccContainerNodePool_localNvmeSsdBlockConfig|TestAccContainerNodePool_withNetworkConfig|TestAccContainerNodePool_threadsPerCore|TestAccContainerNodePool_gvnic|TestAccContainerCluster_withNodeConfig|TestAccContainerNodePool_compactPlacement|TestAccContainerNodePool_gcfsConfig|TestAccComputeFirewallPolicyRule_multipleRules |
|
Based on changes in this commit that adds support for windows node pool provisioning, I feel the proposed changes isn't sufficient. It seems to me that the correct approach would be introducing a local map such as "node_pools_windows_node_configs_osVersion", and define the "windows_node_config" in "windows_pools" similar to the "linux_node_config" in "pools". @g-awmalik, what do you think? |
|
Rerolling reviewer as the original was OOO |
|
/gcbrun |
modular-magician
added
awaiting-approval
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 4 files changed, 160 insertions(+)) Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_container_cluster" "primary" {
node_config {
windows_node_config {
osversion = # value needed
}
}
node_pool {
node_config {
windows_node_config {
osversion = # value needed
}
}
}
}
|
Tests analyticsTotal tests: Action takenFound 44 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccContainerNodePool_gvnic|TestAccContainerNodePool_gcfsConfig|TestAccContainerNodePool_localNvmeSsdBlockConfig|TestAccContainerNodePool_ephemeralStorageLocalSsdConfig|TestAccContainerNodePool_ephemeralStorageConfig|TestAccContainerNodePool_withSoleTenantConfig|TestAccContainerNodePool_shieldedInstanceConfig|TestAccContainerNodePool_EmptyGuestAccelerator|TestAccContainerNodePool_012_ConfigModeAttr|TestAccContainerCluster_withNodePoolNodeConfig|TestAccContainerCluster_withILBSubsetting|TestAccContainerCluster_backend|TestAccContainerNodePool_withManagement|TestAccContainerCluster_withConfidentialNodes|TestAccContainerCluster_withBootDiskKmsKey|TestAccContainerNodePool_withGPU|TestAccContainerCluster_withWorkloadMetadataConfig|TestAccContainerCluster_withSandboxConfig|TestAccContainerCluster_withNodeConfigReservationAffinity|TestAccContainerCluster_withNodeConfigReservationAffinitySpecific|TestAccContainerCluster_withNodeConfigShieldedInstanceConfig|TestAccContainerCluster_withNodeConfigScopeAlias|TestAccContainerCluster_withLoggingVariantInNodePool|TestAccContainerCluster_withLoggingVariantInNodeConfig|TestAccContainerCluster_withNodeConfig|TestAccContainerNodePool_withNodeConfigScopeAlias|TestAccContainerNodePool_withEnablePrivateNodesToggle|TestAccContainerNodePool_withMultiNicNetworkConfig|TestAccContainerNodePool_withWindowsNodeConfig|TestAccContainerNodePool_withLinuxNodeConfig|TestAccContainerNodePool_withKubeletConfig|TestAccContainerNodePool_withSandboxConfig|TestAccContainerNodePool_withWorkloadIdentityConfig|TestAccContainerNodePool_withReservationAffinitySpecific|TestAccContainerNodePool_withReservationAffinity|TestAccContainerNodePool_withNodeConfig|TestAccContainerNodePool_withLoggingVariantUpdates|TestAccContainerCluster_customPlacementPolicy|TestAccContainerNodePool_withHostMaintenancePolicy|TestAccContainerNodePool_threadsPerCore|TestAccContainerNodePool_compactPlacement|TestAccContainerCluster_withSoleTenantGroup|TestAccContainerNodePool_withNetworkConfig|TestAccContainerNodePool_customPlacementPolicy |
|
Rerun these tests in REPLAYING mode to catch issues
|
|
@pranavparuchuri I am looking for something similar to add support for |
Adding support for windows_node_config in gke cluster node_pools. This is required to specify windows 2022 as os version for the nodes. The corresponding issue is hashicorp/terraform-provider-google#14979
If this PR is for Terraform, I acknowledge that I have:
make testandmake lintin the generated providers to ensure it passes unit and linter tests.Release Note Template for Downstream PRs (will be copied)