Update cluster versions in TestAccContainerNodePool_concurrent

[bbasata]

At time of this writing, the cluster default version is 1.31.5. The acceptance test fails because the API will not update such a cluster to 1.29.4.

This change updates the static versions and leaves hyper-breadcrumbs for the next traveler on this path.

Fixes hashicorp/terraform-provider-google#21116

Release Note Template for Downstream PRs (will be copied)

[github-actions]

Hello! I am a robot. Tests will require approval from a repository maintainer to run.

@melinath, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look.

You can help make sure that review is quick by doing a self-review and by running impacted tests locally.

[modular-magician]

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 1 file changed, 14 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 1 file changed, 14 insertions(+), 2 deletions(-))

[modular-magician]

Tests analytics

Total tests: 225
Passed tests: 213
Skipped tests: 11
Affected tests: 1

Click here to see the affected service packages

• container

Action taken

Found 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

• TestAccContainerNodePool_concurrent

Get to know how VCR tests work

[modular-magician]

🔴 Tests failed during RECORDING mode:
TestAccContainerNodePool_concurrent [Error message] [Debug log]

🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR.

View the build log or the debug log for each test

[bbasata]

Oh yes, this needs to be recorded anew. If someone gets to this before I do, that's more than welcome.

[melinath]

@modular-magician reassign-reviewer melinath

[melinath]

@bbasata the recording is happening automatically - the failures are due to:

  
        Error: googleapi: Error 400: Node version "1.32.0-gke.1448000" must not have a greater minor version than master version "1.31.4-gke.1256000".
        Details:
        [
          {
            "@type": "type.googleapis.com/google.rpc.RequestInfo",
            "requestId": "0x19cad9635fdbce46"
          }
        ]
        , badRequest
        
          with google_container_node_pool.np1,
          on terraform_plugin_test.tf line 11, in resource "google_container_node_pool" "np1":
          11: resource "google_container_node_pool" "np1" {
        
        
        Error: googleapi: Error 400: Node version "1.32.0-gke.1448000" must not have a greater minor version than master version "1.31.4-gke.1256000".
        Details:
        [
          {
            "@type": "type.googleapis.com/google.rpc.RequestInfo",
            "requestId": "0x2a7c99b7fe91d372"
          }
        ]
        , badRequest
        
          with google_container_node_pool.np2,
          on terraform_plugin_test.tf line 23, in resource "google_container_node_pool" "np2":
          23: resource "google_container_node_pool" "np2" {

Possibly relatedly - could this test be rewritten to use the google_container_engine_versions data source instead of hardcoding versions?

[bbasata]

@bbasata the recording is happening automatically - the failures are due to:

Got it, thanks! I think I see what I missed. The node version cannot exceed the cluster (control plane) version. git push. Waiting on checks ...

could this test be rewritten to use the google_container_engine_versions data source instead of hardcoding versions?

I'd like to give it a try. If this PR goes green (maybe!), how about we merge, and then follow up with the nicer solution?

[melinath]

If this PR goes green (maybe!), how about we merge, and then follow up with the nicer solution?

SGTM

[modular-magician]

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 1 file changed, 21 insertions(+), 7 deletions(-))
google-beta provider: Diff ( 1 file changed, 21 insertions(+), 7 deletions(-))

[modular-magician]

Tests analytics

Total tests: 225
Passed tests: 213
Skipped tests: 11
Affected tests: 1

Click here to see the affected service packages

• container

Action taken

Found 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

• TestAccContainerNodePool_concurrent

Get to know how VCR tests work

[modular-magician]

🔴 Tests failed during RECORDING mode:
TestAccContainerNodePool_concurrent [Error message] [Debug log]

🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR.

View the build log or the debug log for each test

[melinath]

Error is:

        Error: Unsupported argument
        
          on terraform_plugin_test.tf line 9, in resource "google_container_cluster" "cluster":
           9:   version             = "1.32.0-gke.1448000"
        
        An argument named "version" is not expected here.

[modular-magician]

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 1 file changed, 21 insertions(+), 7 deletions(-))
google-beta provider: Diff ( 1 file changed, 21 insertions(+), 7 deletions(-))

[KatrinaHoffert]

Thanks for fixing this! I'm not sure why we ever hard coded it.

Bumping the version works for now. A perhaps cleaner long term solution would be to just choose some simpler field. The most important thing is that the operation takes a little bit of time so that there is actual concurrency (since several fields are nearly instant to update). There's many possibilities. The node_config field has a few that are probably cheap and easy. I particularly eye metadata as one that seems like it'd be a straightforward one to update. Seems like that'd improve coverage too, as we appear to not have any tests for that field already. Though if that makes it feel too risky, disk_size_gb is another option that at least has existing tests.

[modular-magician]

Tests analytics

Total tests: 225
Passed tests: 213
Skipped tests: 11
Affected tests: 1

Click here to see the affected service packages

• container

Action taken

Found 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

• TestAccContainerNodePool_concurrent

Get to know how VCR tests work

[modular-magician]

🟢 Tests passed during RECORDING mode:
TestAccContainerNodePool_concurrent [Debug log]

🟢 No issues found for passed tests after REPLAYING rerun.

---

🟢 All tests passed!

View the build log or the debug log for each test

[melinath]

approving this for now to get the test passing; we can consider a longer-term fix separately.

[melinath]

Thanks all!

[wyardley]

I updated it in #13161 with an approach that hopefully accomplishes the goal of the test in a slightly more durable way, by picking two (normally) different versions? I also sized down the node pools and deleted the default pool, which should if nothing else, make it faster and cheaper to run.

I'd come across this PR, but hadn't read through all the comments before (since I'd noticed it was already fixed). I agree with the above, though, that probably updating a different field might make more sense, along with some additional comments on the goals behind the test.