fix: allow null `enable_gcfs` setting in defined nodepools

[wyardley]

Even with 6.4.0 or 5.44.1 (with my upstream fix), I am still seeing the issue described in #2100, since the earlier fixes were related to the default node-pool vs explicitly defined ones.

Fixes #2100 This basically replicates the fixes from #2093, #2095, but at the scope of implicitly defined nodepools.

[apeabody]

/gcbrun

[wyardley]

Do you know if we test this in any of the examples?

https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/blob/073390897e38310124646e56a39c305809fbded4/examples/node_pool/main.tf#L54

We test the case where it's false here; since the other node pools in this example don't have it defined, I assume that implicitly tests the null case, though it's interesting - if the test is already checking for idempotence (is it?), you'd think it would have already caught the diff in plan already? Maybe because in the case where we're creating it and the provider creates it as false, it stays that way, but just speculating.

Is the failure in CI from one of these changes, or a spurious / transient one?

[apeabody]

I believe this change is intentional, so just need to update the test data to match.

https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/blob/master/test/integration/private_zonal_with_networking/testdata/TestPrivateZonalWithNetworking.json#L213

Step #98 - "verify private-zonal-with-networking":          Error Trace:    /workspace/test/integration/private_zonal_with_networking/private_zonal_with_networking_test.go:81
Step #98 - "verify private-zonal-with-networking":                                      /builder/home/go/pkg/mod/github.com/!google!cloud!platform/cloud-foundation-toolkit/infra/blueprint-test@v0.16.2/pkg/tft/terraform.go:638
Step #98 - "verify private-zonal-with-networking":                                      /builder/home/go/pkg/mod/github.com/!google!cloud!platform/cloud-foundation-toolkit/infra/blueprint-test@v0.16.2/pkg/tft/terraform.go:670
Step #98 - "verify private-zonal-with-networking":                                      /builder/home/go/pkg/mod/github.com/!google!cloud!platform/cloud-foundation-toolkit/infra/blueprint-test@v0.16.2/pkg/utils/stages.go:31
Step #98 - "verify private-zonal-with-networking":                                      /builder/home/go/pkg/mod/github.com/!google!cloud!platform/cloud-foundation-toolkit/infra/blueprint-test@v0.16.2/pkg/tft/terraform.go:670
Step #98 - "verify private-zonal-with-networking":          Error:          Not equal: 
Step #98 - "verify private-zonal-with-networking":                          expected: map[string]interface {}{"diskSizeGb":100, "diskType":"pd-standard", "gcfsConfig":map[string]interface {}{}, "imageType":"COS_CONTAINERD", "labels":map[string]interface {}{"cluster_name":"CLUSTER_NAME", "node_pool":"default-node-pool"}, "loggingConfig":map[string]interface {}{"variantConfig":map[string]interface {}{"variant":"DEFAULT"}}, "machineType":"e2-medium", "metadata":map[string]interface {}{"cluster_name":"CLUSTER_NAME", "disable-legacy-endpoints":"true", "node_pool":"default-node-pool"}, "oauthScopes":[]interface {}{"https://www.googleapis.com/auth/cloud-platform"}, "serviceAccount":"SERVICE_ACCOUNT", "shieldedInstanceConfig":map[string]interface {}{"enableIntegrityMonitoring":true}, "tags":[]interface {}{"gke-CLUSTER_NAME", "gke-CLUSTER_NAME-default-node-pool"}, "windowsNodeConfig":map[string]interface {}{}, "workloadMetadataConfig":map[string]interface {}{"mode":"GKE_METADATA"}}
Step #98 - "verify private-zonal-with-networking":                          actual  : map[string]interface {}{"diskSizeGb":100, "diskType":"pd-standard", "imageType":"COS_CONTAINERD", "labels":map[string]interface {}{"cluster_name":"CLUSTER_NAME", "node_pool":"default-node-pool"}, "loggingConfig":map[string]interface {}{"variantConfig":map[string]interface {}{"variant":"DEFAULT"}}, "machineType":"e2-medium", "metadata":map[string]interface {}{"cluster_name":"CLUSTER_NAME", "disable-legacy-endpoints":"true", "node_pool":"default-node-pool"}, "oauthScopes":[]interface {}{"https://www.googleapis.com/auth/cloud-platform"}, "serviceAccount":"SERVICE_ACCOUNT", "shieldedInstanceConfig":map[string]interface {}{"enableIntegrityMonitoring":true}, "tags":[]interface {}{"gke-CLUSTER_NAME", "gke-CLUSTER_NAME-default-node-pool"}, "windowsNodeConfig":map[string]interface {}{}, "workloadMetadataConfig":map[string]interface {}{"mode":"GKE_METADATA"}}
Step #98 - "verify private-zonal-with-networking":                          
Step #98 - "verify private-zonal-with-networking":                          Diff:
Step #98 - "verify private-zonal-with-networking":                          --- Expected
Step #98 - "verify private-zonal-with-networking":                          +++ Actual
Step #98 - "verify private-zonal-with-networking":                          @@ -1,6 +1,4 @@
Step #98 - "verify private-zonal-with-networking":                          -(map[string]interface {}) (len=14) {
Step #98 - "verify private-zonal-with-networking":                          +(map[string]interface {}) (len=13) {
Step #98 - "verify private-zonal-with-networking":                            (string) (len=10) "diskSizeGb": (float64) 100,
Step #98 - "verify private-zonal-with-networking":                            (string) (len=8) "diskType": (string) (len=11) "pd-standard",
Step #98 - "verify private-zonal-with-networking":                          - (string) (len=10) "gcfsConfig": (map[string]interface {}) {
Step #98 - "verify private-zonal-with-networking":                          - },
Step #98 - "verify private-zonal-with-networking":                            (string) (len=9) "imageType": (string) (len=14) "COS_CONTAINERD",
Step #98 - "verify private-zonal-with-networking":          Test:           TestPrivateZonalWithNetworking

[apeabody]

Do you know if we test this in any of the examples?

https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/blob/073390897e38310124646e56a39c305809fbded4/examples/node_pool/main.tf#L54

We test the case where it's false here; since the other node pools in this example don't have it defined, I assume that implicitly tests the null case, though it's interesting - if the test is already checking for idempotence (is it?), you'd think it would have already caught the diff in plan already? Maybe because in the case where we're creating it and the provider creates it as false, it stays that way, but just speculating.

Is the failure in CI from one of these changes, or a spurious / transient one?

It looks like most of the idempotence tests are disabled(e.g. https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/blob/master/test/integration/private_zonal_with_networking/private_zonal_with_networking_test.go#L36), so I actually started work to bring back in: https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/pull/2060

[wyardley]

so just need to update the test data to match.

Just to confirm, you're suggesting removing that line, right? and that's the only one where it's failing? I did a quick search for gcfsConfig in other testdata directories, and mostly turned up autopilot stuff.

[apeabody]

so just need to update the test data to match.

Just to confirm, you're suggesting removing that line, right? and that's the only one where it's failing? I did a quick search for gcfsConfig in other testdata directories, and mostly turned up autopilot stuff.

That's as far as the tests got in that run, but yes, that particular test doesn't define, so with this change the gcfsConfig block should not be created.

[wyardley]

Cool. I didn’t see another from a quick look, and removed it in that last commit.

[apeabody]

/gcbrun