Add NetworkConfig and NetworkConfigs to TPU v2 QueuedResource

[zeleena]

Add NetworkConfig and NetworkConfigs fields. NetworkConfigs (array) and NetworkConfig (singular) conflict. NetworkConfigs should be the field to specify multiple NetworkConfig, so I copied over the NetworkConfig NestedObject properties to its array form, NetworkConfigs.

tpuv2: added `network_config` and `network_configs` fields to `google_tpu_v2_queued_resource` resource

[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, 112 insertions(+))
google-beta provider: Diff ( 3 files changed, 550 insertions(+))
terraform-google-conversion: Diff ( 1 file changed, 151 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_tpu_v2_queued_resource (2 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_tpu_v2_queued_resource" "primary" {
  tpu {
    node_spec {
      node {
        network_configs {
          can_ip_forward      = # value needed
          enable_external_ips = # value needed
          network             = # value needed
          queue_count         = # value needed
          subnetwork          = # value needed
        }
      }
    }
  }
}

[modular-magician]

Tests analytics

Total tests: 7
Passed tests: 3
Skipped tests: 4
Affected tests: 0

Click here to see the affected service packages

• tpuv2

#### Non-exercised tests

🔴 Tests were added that are skipped in VCR:

• TestAccTpuV2QueuedResource_tpuV2QueuedResourceFullExample
  🟢 All tests passed!

View the build log

[zeleena]

Hi @melinath, it's been about two business days and wanted to see if you could please take a look? I was aiming to merge this change prior to the cut for the next release.

[melinath]

This seems straightforward; just have one question below. I'll also need to manually run a test since it's skipped in VCR.

[melinath]

default_from_api has a couple drawbacks:

• Users won't be able to know accurate information about the end state of the resource (even if it could be known)
• Users can't set the field to its empty/zero value.

I just want to double-check that this field (and the rest marked with default_from_api) don't have other options?

[zeleena]

Thanks for highlighting. The network and subnetwork fields do get a default value from the API if not set. Consequently, those are subfields of networkConfig, which has subfields where some have API defaults and some do not, so I set networkConfig to get its default from the API so that its subfields like network and subnetwork can get these default values. This is also the case for a similar resource TPUv2 VM, where networkConfig has default_from_api: true and has been working as expected.

I double checked queueCount and that does not get a default value from the TPU API, so I removed default_from_api from the queueCount subfield here and below.

[melinath]

Manual beta test run: https://hashicorp.teamcity.com/buildConfiguration/TerraformProviders_GoogleCloud_GOOGLE_BETA_MMUPSTREAMTESTS_GOOGLEBETA_PACKAGE_TPUV2/277507?hideTestsFromDependencies=false&hideProblemsFromDependencies=false&expandBuildChangesSection=true

[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, 112 insertions(+))
google-beta provider: Diff ( 3 files changed, 548 insertions(+))
terraform-google-conversion: Diff ( 1 file changed, 151 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_tpu_v2_queued_resource (2 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_tpu_v2_queued_resource" "primary" {
  tpu {
    node_spec {
      node {
        network_configs {
          can_ip_forward      = # value needed
          enable_external_ips = # value needed
          network             = # value needed
          queue_count         = # value needed
          subnetwork          = # value needed
        }
      }
    }
  }
}

[zeleena]

Manual beta test run: https://hashicorp.teamcity.com/buildConfiguration/TerraformProviders_GoogleCloud_GOOGLE_BETA_MMUPSTREAMTESTS_GOOGLEBETA_PACKAGE_TPUV2/277507?hideTestsFromDependencies=false&hideProblemsFromDependencies=false&expandBuildChangesSection=true

Thank you for the manual test run. I'm not sure I have proper permissions to view the results, can you let me know if there's any issues or actions I need to take, or can you let me know the best way to setup my account and get permissions to view the test at this link? When I try to login via GitHub, it says Authenticated successfully, but failed to create a new user since creating new users by "GitHub.com" login is not allowed

[modular-magician]

Tests analytics

Total tests: 7
Passed tests: 3
Skipped tests: 4
Affected tests: 0

Click here to see the affected service packages

• tpuv2

#### Non-exercised tests

🔴 Tests were added that are skipped in VCR:

• TestAccTpuV2QueuedResource_tpuV2QueuedResourceFullExample
  🟢 All tests passed!

View the build log

[melinath]

New manual run: https://hashicorp.teamcity.com/buildConfiguration/TerraformProviders_GoogleCloud_GOOGLE_BETA_MMUPSTREAMTESTS_GOOGLEBETA_PACKAGE_TPUV2/277975?hideTestsFromDependencies=false&hideProblemsFromDependencies=false&expandBuildChangesSection=true

[melinath]

Thank you for the manual test run. I'm not sure I have proper permissions to view the results, can you let me know if there's any issues or actions I need to take, or can you let me know the best way to setup my account and get permissions to view the test at this link? When I try to login via GitHub, it says Authenticated successfully, but failed to create a new user since creating new users by "GitHub.com" login is not allowed

Sorry, we can't grant access to the manual runs due to the platform they run on.

[melinath]

Sorry for not catching this sooner - it looks like we're missing tests for the network_configs field: #12482 (comment) Could you add them?

[melinath]

TestAccTpuV2QueuedResource_tpuV2QueuedResourceFullExample passed though and that all looks good!

[zeleena]

Sorry, we can't grant access to the manual runs due to the platform they run on.

No worries and sounds good, just wanted to make sure there was no action needed from me at that point!

Sorry for not catching this sooner - it looks like we're missing tests for the network_configs field: #12482 (comment) Could you add them?

It's a copy of the existing network_config field which is tested and conflicts with network_configs. Since it's mutually exclusive, I can't add it to the full example test since network_config is already defined. I suppose I could create a separate and dedicated test for the array field to work around the mutual exclusivity if that's preferred?

TestAccTpuV2QueuedResource_tpuV2QueuedResourceFullExample passed though and that all looks good!

Great, thanks for running this!

[melinath]

yes, a separate test for the networkConfigs field would be great. Even though it's a duplicate, we should have tests to cover it.

[melinath]

/gcbrun

[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, 184 insertions(+))
google-beta provider: Diff ( 3 files changed, 719 insertions(+))
terraform-google-conversion: Diff ( 1 file changed, 151 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_tpu_v2_queued_resource (3 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_tpu_v2_queued_resource" "primary" {
  tpu {
    node_spec {
      node {
        network_configs {
          can_ip_forward      = # value needed
          enable_external_ips = # value needed
          network             = # value needed
          queue_count         = # value needed
          subnetwork          = # value needed
        }
      }
    }
  }
}

[modular-magician]

Tests analytics

Total tests: 8
Passed tests: 3
Skipped tests: 5
Affected tests: 0

Click here to see the affected service packages

• tpuv2

#### Non-exercised tests

🔴 Tests were added that are skipped in VCR:

• TestAccTpuV2QueuedResource_tpuV2QueuedResourceFullExample
• TestAccTpuV2QueuedResource_tpuV2QueuedResourceNetworkConfigsExample
  🟢 All tests passed!

View the build log

[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, 182 insertions(+))
google-beta provider: Diff ( 3 files changed, 715 insertions(+))
terraform-google-conversion: Diff ( 1 file changed, 151 insertions(+))

[modular-magician]

Tests analytics

Total tests: 8
Passed tests: 3
Skipped tests: 5
Affected tests: 0

Click here to see the affected service packages

• tpuv2

#### Non-exercised tests

🔴 Tests were added that are skipped in VCR:

• TestAccTpuV2QueuedResource_tpuV2QueuedResourceFullExample
• TestAccTpuV2QueuedResource_tpuV2QueuedResourceNetworkConfigsExample
  🟢 All tests passed!

View the build log

[melinath]

Manual test run for the new test: https://hashicorp.teamcity.com/buildConfiguration/TerraformProviders_GoogleCloud_GOOGLE_BETA_MMUPSTREAMTESTS_GOOGLEBETA_PACKAGE_TPUV2/279679?hideTestsFromDependencies=false&hideProblemsFromDependencies=false&expandBuildChangesSection=true

[melinath]

Manual test run failed with:

    resource_tpu_v2_queued_resource_generated_test.go:169: Step 1/2 error: Error running apply: exit status 1
        Error: Error creating QueuedResource: googleapi: Error 400: Invalid JSON payload received. Unknown name "networkConfigs" at 'queued_resource.tpu.node_spec[0].node': Cannot find field.
        Details:
        [
          {
            "@type": "type.googleapis.com/google.rpc.BadRequest",
            "fieldViolations": [
              {
                "description": "Invalid JSON payload received. Unknown name \"networkConfigs\" at 'queued_resource.tpu.node_spec[0].node': Cannot find field.",
                "field": "queued_resource.tpu.node_spec[0].node"
              }
            ]
          }
        ]
          with google_tpu_v2_queued_resource.qr,
          on terraform_plugin_test.tf line 2, in resource "google_tpu_v2_queued_resource" "qr":
           2: resource "google_tpu_v2_queued_resource" "qr" {

[melinath]

we need to remove the unreleased field so that it's safe to add the field that's already supported in the API

[melinath]

I'm not able to modify the PR myself, so I don't think it's going possible to get this into a mergeable state today. We may as well wait for the networkConfigs field to be present in the API.