magic-modules icon indicating copy to clipboard operation
magic-modules copied to clipboard

Support V5P for type of accelerator_config in google_tpu_v2_vm

Open marblejenka opened this issue 11 months ago • 11 comments

This is PR for https://github.com/hashicorp/terraform-provider-google/issues/17734 .

tpu: added `type` of `accelerator_config` to `google_tpu_v2_vm` resource

marblejenka avatar Mar 31 '24 12:03 marblejenka

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

@BBBmau, 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.

github-actions[bot] avatar Mar 31 '24 12:03 github-actions[bot]

/gcbrun

BBBmau avatar Apr 01 '24 16:04 BBBmau

@marblejenka we ran into some issues with CI, could you rebase to cover these issues? We can then run the tests before being able to approve.

BBBmau avatar Apr 03 '24 22:04 BBBmau

I found that the tests in the local environment were not running properly, so I will make some corrections. Please wait for a while.

marblejenka avatar Apr 05 '24 08:04 marblejenka

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, 32 insertions(+), 1 deletion(-)) google-beta provider: Diff ( 3 files changed, 85 insertions(+), 3 deletions(-))

modular-magician avatar Apr 26 '24 07:04 modular-magician

Tests analytics

Total tests: 6 Passed tests: 5 Skipped tests: 0 Affected tests: 1

Click here to see the affected service packages
  • tpuv2

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
TestAccTpuV2Vm_tpuV2VmV5pExample

Get to know how VCR tests work

modular-magician avatar Apr 26 '24 07:04 modular-magician

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$ TestAccTpuV2Vm_tpuV2VmV5pExample[Error message] [Debug log]

$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$ View the build log or the debug log for each test

modular-magician avatar Apr 26 '24 07:04 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, 32 insertions(+), 1 deletion(-)) google-beta provider: Diff ( 3 files changed, 85 insertions(+), 3 deletions(-))

modular-magician avatar Apr 26 '24 07:04 modular-magician

Tests analytics

Total tests: 6 Passed tests: 5 Skipped tests: 0 Affected tests: 1

Click here to see the affected service packages
  • tpuv2

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
TestAccTpuV2Vm_tpuV2VmV5pExample

Get to know how VCR tests work

modular-magician avatar Apr 26 '24 07:04 modular-magician

$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$ TestAccTpuV2Vm_tpuV2VmV5pExample[Error message] [Debug log]

$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$ View the build log or the debug log for each test

modular-magician avatar Apr 26 '24 07:04 modular-magician

@BBBmau The test is failing with Error 403: Location us-east5-a is not found or access is unauthorized.. From what I've tried, us-east5-a seems to be the only region where we can use TPU v5p with QIR. Can you change settings for the testing Google Cloud project?

The requirement is to increase TPUV5PPreemptiblePerProjectPerZoneForTPUAPI quota to 8 or higher (default value is 0) mabye in us-east5-a.

marblejenka avatar Apr 26 '24 08:04 marblejenka

@BBBmau it looks like in general we've not added accelerator quota for each generation to this test project. The PR overall looks good and we can actually do without the additional test example for the time being, as we don't have access to long term Terraform testing quota.

I'd recommend removing the new example and approving the PR otherwise.

bollenberger avatar May 09 '24 21:05 bollenberger

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, 1 insertion(+), 1 deletion(-)) google-beta provider: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician avatar May 13 '24 14:05 modular-magician

Tests analytics

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

Click here to see the affected service packages
  • tpuv2
$\textcolor{green}{\textsf{All tests passed!}}$ View the [build log](https://storage.cloud.google.com/ci-vcr-logs/beta/refs/heads/auto-pr-10333/artifacts/964c3a65-6804-4cb2-9ea4-cec47b02a92f/build-log/replaying_test.log)

modular-magician avatar May 13 '24 14:05 modular-magician

@BBBmau @bollenberger Finally, I deleted all the codes except "V5P". This document suggests that this PR works well.

marblejenka avatar May 13 '24 15:05 marblejenka

@BBBmau Can you merge this one? I don't have the permission to merge.

marblejenka avatar May 15 '24 05:05 marblejenka

thank you for the patience, I wasn't able to merge this right away due to some work being done on branches.

BBBmau avatar May 15 '24 15:05 BBBmau