fix: additional_ip_ranges_config - again #2458

DrFaust92 · 2025-09-26T02:46:37Z

apologies, missed this again

Fixes: #2437

apeabody · 2025-09-26T03:16:12Z

/gcbrun

autogen/main/cluster.tf.tmpl

apeabody · 2025-10-03T04:45:15Z

/gcbrun

apeabody · 2025-10-03T20:08:23Z

/gcbrun

apeabody · 2025-10-03T20:08:49Z

/gcbrun

apeabody · 2025-10-03T22:10:05Z

Hi @DrFaust92 - I'm going to run the test again, but this is the only PR I'm currently seeing this error:

Error: Cannot determine region: set in this resource, or set provider-level 'region' or 'zone'.

  with module.example.module.gke.google_container_cluster.primary,
  on ../../../modules/beta-public-cluster/cluster.tf line 22, in resource "google_container_cluster" "primary":
  22: resource "google_container_cluster" "primary" {
}

apeabody · 2025-10-03T22:10:17Z

/gcbrun

apeabody · 2025-10-07T21:29:04Z

Thanks @DrFaust92, still seeing:

TestNodePool 2025-10-07T21:08:49Z command.go:212: module.example.module.gke.google_container_cluster.primary: Creating...
TestNodePool 2025-10-07T21:08:49Z command.go:212: 
TestNodePool 2025-10-07T21:08:49Z command.go:212: Error: Cannot determine region: set in this resource, or set provider-level 'region' or 'zone'.
TestNodePool 2025-10-07T21:08:49Z command.go:212: 
TestNodePool 2025-10-07T21:08:49Z command.go:212:   with module.example.module.gke.google_container_cluster.primary,
TestNodePool 2025-10-07T21:08:49Z command.go:212:   on ../../../modules/beta-public-cluster/cluster.tf line 22, in resource "google_container_cluster" "primary":
TestNodePool 2025-10-07T21:08:49Z command.go:212:   22: resource "google_container_cluster" "primary" {
TestNodePool 2025-10-07T21:08:49Z command.go:212:

cwh-hcl · 2025-10-08T16:41:41Z

I don't think the additional_ip_ranges_config variable needs to be defined as a list... see my comment here -> #2437 (comment)

examples/node_pool/main.tf

apeabody · 2025-10-08T18:19:17Z

/gcbrun

apeabody · 2025-10-08T23:58:16Z

/gcbrun

modules/beta-private-cluster-update-variant/README.md

apeabody · 2025-10-09T17:13:39Z

/gcbrun

DrFaust92 · 2025-10-09T22:00:28Z

apeabody, can you share the current failure? 🙏

apeabody · 2025-10-09T22:11:07Z

apeabody, can you share the current failure? 🙏

Hi @DrFaust92 - Running again as I'm not convinced this is due to the change:

Step #60 - "apply node-pool-local": Error: Error updating cluster for []: googleapi: Error 400: Cluster is running incompatible operation operation-1760041804071-8378a7c3-539b-4e68-a2d3-8f58bb864553.
Step #60 - "apply node-pool-local": Details:
Step #60 - "apply node-pool-local": [
Step #60 - "apply node-pool-local":   {
Step #60 - "apply node-pool-local":     "@type": "type.googleapis.com/google.rpc.RequestInfo",
Step #60 - "apply node-pool-local":     "requestId": "0xf2eadd1a8ccb3137"
Step #60 - "apply node-pool-local":   },
Step #60 - "apply node-pool-local":   {
Step #60 - "apply node-pool-local":     "@type": "type.googleapis.com/google.rpc.ErrorInfo",
Step #60 - "apply node-pool-local":     "domain": "container.googleapis.com",
Step #60 - "apply node-pool-local":     "reason": "CLUSTER_ALREADY_HAS_OPERATION"
Step #60 - "apply node-pool-local":   }
Step #60 - "apply node-pool-local": ]
Step #60 - "apply node-pool-local": , failedPrecondition
Step #60 - "apply node-pool-local": 
Step #60 - "apply node-pool-local":   with module.example.module.gke.google_container_cluster.primary,
Step #60 - "apply node-pool-local":   on ../../../modules/beta-public-cluster/cluster.tf line 22, in resource "google_container_cluster" "primary":
Step #60 - "apply node-pool-local":   22: resource "google_container_cluster" "primary" {
Step #60 - "apply node-pool-local": }

apeabody · 2025-10-10T16:30:48Z

apeabody, can you share the current failure? 🙏

Hi @DrFaust92 - Running again as I'm not convinced this is due to the change:

Step #60 - "apply node-pool-local": Error: Error updating cluster for []: googleapi: Error 400: Cluster is running incompatible operation operation-1760041804071-8378a7c3-539b-4e68-a2d3-8f58bb864553.
Step #60 - "apply node-pool-local": Details:
Step #60 - "apply node-pool-local": [
Step #60 - "apply node-pool-local":   {
Step #60 - "apply node-pool-local":     "@type": "type.googleapis.com/google.rpc.RequestInfo",
Step #60 - "apply node-pool-local":     "requestId": "0xf2eadd1a8ccb3137"
Step #60 - "apply node-pool-local":   },
Step #60 - "apply node-pool-local":   {
Step #60 - "apply node-pool-local":     "@type": "type.googleapis.com/google.rpc.ErrorInfo",
Step #60 - "apply node-pool-local":     "domain": "container.googleapis.com",
Step #60 - "apply node-pool-local":     "reason": "CLUSTER_ALREADY_HAS_OPERATION"
Step #60 - "apply node-pool-local":   }
Step #60 - "apply node-pool-local": ]
Step #60 - "apply node-pool-local": , failedPrecondition
Step #60 - "apply node-pool-local": 
Step #60 - "apply node-pool-local":   with module.example.module.gke.google_container_cluster.primary,
Step #60 - "apply node-pool-local":   on ../../../modules/beta-public-cluster/cluster.tf line 22, in resource "google_container_cluster" "primary":
Step #60 - "apply node-pool-local":   22: resource "google_container_cluster" "primary" {
Step #60 - "apply node-pool-local": }

I'm seeing the same error in #2466

Signed-off-by: drfaust92 <[email protected]>

DrFaust92 · 2025-10-26T19:34:20Z

on the side new smaller test works well, there is some race condition between changes that happen during create. with the "node_pool" config. so i copied the "simple_regional" one and added the additional_ip_ranges config for a dedicate test

Signed-off-by: drfaust92 <[email protected]>

DrFaust92 · 2025-10-26T20:00:08Z

@apeabody if you can kickstart tests, local apply is passing

apeabody · 2025-10-27T21:25:39Z

/gemini review

gemini-code-assist

Code Review

This pull request correctly fixes an issue with iterating over additional_ip_ranges_config in dynamic blocks across multiple module variants. The change to use additional_ip_ranges_config.value is the correct approach for accessing elements within a for_each loop. The addition of a new example and integration tests for this feature is a good improvement. However, I've identified an issue in the new integration test that will cause it to fail.

gemini-code-assist · 2025-10-27T21:27:19Z

...ntegration/simple_regional_additional_ip_ranges/simple_regional_additional_ip_ranges_test.go

+		clusterName := bpt.GetStringOutput("cluster_name")
+		serviceAccount := bpt.GetStringOutput("service_account")
+
+		op := gcloud.Runf(t, "container clusters describe %s --zone %s --project %s", clusterName, location, projectId)


The gcloud container clusters describe command is using the --zone flag for a regional cluster. Since this test is for a regional cluster, the --region flag should be used instead. The location output for a regional cluster is the region, so using it with --zone will cause an error.

Suggested change

op := gcloud.Runf(t, "container clusters describe %s --zone %s --project %s", clusterName, location, projectId)

op := gcloud.Runf(t, "container clusters describe %s --region %s --project %s", clusterName, location, projectId)

test/fixtures/node_pool/network.tf

apeabody · 2025-10-27T21:44:41Z

/gcbrun

examples/simple_regional_additional_ip_ranges/variables.tf

test/fixtures/simple_regional_additional_ip_ranges/network.tf

test/fixtures/simple_regional_additional_ip_ranges/outputs.tf

test/fixtures/node_pool/network.tf

apeabody · 2025-10-28T15:50:58Z

/gcbrun

apeabody · 2025-10-28T17:34:30Z

Thanks @DrFaust92!

This looks great, we just need to add the new integration test to the build: https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/blob/main/build/int.cloudbuild.yaml#L514

Something like this:

- id: apply test-simple-regional
  waitFor:
    - init-all
  name: 'gcr.io/cloud-foundation-cicd/$_DOCKER_IMAGE_DEVELOPER_TOOLS:$_DOCKER_TAG_VERSION_DEVELOPER_TOOLS'
  args: ['/bin/bash', '-c', 'cft test run TestSimpleRegional --stage apply --verbose']
- id: verify test-simple-regional
  waitFor:
    - apply test-simple-regional
  name: 'gcr.io/cloud-foundation-cicd/$_DOCKER_IMAGE_DEVELOPER_TOOLS:$_DOCKER_TAG_VERSION_DEVELOPER_TOOLS'
  args: ['/bin/bash', '-c', 'cft test run TestSimpleRegional --stage verify --verbose']
- id: teardown test-simple-regional
  waitFor:
    - verify test-simple-regional
  name: 'gcr.io/cloud-foundation-cicd/$_DOCKER_IMAGE_DEVELOPER_TOOLS:$_DOCKER_TAG_VERSION_DEVELOPER_TOOLS'
  args: ['/bin/bash', '-c', 'cft test run TestSimpleRegional --stage teardown --verbose']

Signed-off-by: drfaust92 <[email protected]>

DrFaust92 · 2025-10-31T17:55:29Z

Thanks @DrFaust92!

This looks great, we just need to add the new integration test to the build: https://github.com/terraform-google-modules/terraform-google-kubernetes-engine/blob/main/build/int.cloudbuild.yaml#L514

Something like this:

- id: apply test-simple-regional
  waitFor:
    - init-all
  name: 'gcr.io/cloud-foundation-cicd/$_DOCKER_IMAGE_DEVELOPER_TOOLS:$_DOCKER_TAG_VERSION_DEVELOPER_TOOLS'
  args: ['/bin/bash', '-c', 'cft test run TestSimpleRegional --stage apply --verbose']
- id: verify test-simple-regional
  waitFor:
    - apply test-simple-regional
  name: 'gcr.io/cloud-foundation-cicd/$_DOCKER_IMAGE_DEVELOPER_TOOLS:$_DOCKER_TAG_VERSION_DEVELOPER_TOOLS'
  args: ['/bin/bash', '-c', 'cft test run TestSimpleRegional --stage verify --verbose']
- id: teardown test-simple-regional
  waitFor:
    - verify test-simple-regional
  name: 'gcr.io/cloud-foundation-cicd/$_DOCKER_IMAGE_DEVELOPER_TOOLS:$_DOCKER_TAG_VERSION_DEVELOPER_TOOLS'
  args: ['/bin/bash', '-c', 'cft test run TestSimpleRegional --stage teardown --verbose']

added

apeabody · 2025-10-31T18:01:44Z

/gcbrun

apeabody · 2025-10-31T18:02:33Z

/gcbrun

...ntegration/simple_regional_additional_ip_ranges/simple_regional_additional_ip_ranges_test.go

…egional_additional_ip_ranges_test.go

apeabody · 2025-10-31T18:46:02Z

/gcbrun

DrFaust92 marked this pull request as ready for review September 26, 2025 02:46

DrFaust92 requested review from a team, apeabody and ericyz as code owners September 26, 2025 02:46

pib reviewed Sep 26, 2025

View reviewed changes

autogen/main/cluster.tf.tmpl Outdated Show resolved Hide resolved

DrFaust92 force-pushed the fix-pods-ip-again branch from 1c84b6c to 8960c33 Compare September 28, 2025 01:59

DrFaust92 marked this pull request as draft September 28, 2025 01:59

DrFaust92 force-pushed the fix-pods-ip-again branch from 8960c33 to a82e717 Compare October 2, 2025 23:58

DrFaust92 marked this pull request as ready for review October 3, 2025 01:07

apeabody self-assigned this Oct 3, 2025

apeabody mentioned this pull request Oct 8, 2025

Unable to specify pod_ipv4_range_names? #2437

Open

DrFaust92 force-pushed the fix-pods-ip-again branch from db2480a to faed61b Compare October 8, 2025 17:54

DrFaust92 marked this pull request as draft October 8, 2025 17:54

DrFaust92 commented Oct 8, 2025

View reviewed changes

examples/node_pool/main.tf Outdated Show resolved Hide resolved

DrFaust92 marked this pull request as ready for review October 8, 2025 18:10

apeabody reviewed Oct 9, 2025

View reviewed changes

modules/beta-private-cluster-update-variant/README.md Show resolved Hide resolved

DrFaust92 marked this pull request as draft October 26, 2025 19:15

DrFaust92 added 2 commits October 26, 2025 15:18

change test

26e4b05

Signed-off-by: drfaust92 <[email protected]>

change test

564e104

Signed-off-by: drfaust92 <[email protected]>

change test

6903e63

Signed-off-by: drfaust92 <[email protected]>

DrFaust92 marked this pull request as ready for review October 26, 2025 19:57

gemini-code-assist bot reviewed Oct 27, 2025

View reviewed changes