Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix error with missing zonal value for compute attached disk #9371

Merged
merged 3 commits into from
Nov 9, 2023

Conversation

NickElliot
Copy link
Contributor

fixes hashicorp/terraform-provider-google#16265

Release Note Template for Downstream PRs (will be copied)

compute: fixed issue where `google_compute_attached_disk` would produce an error for certain zone configs

@NickElliot NickElliot marked this pull request as ready for review October 26, 2023 21:58
@modular-magician
Copy link
Collaborator

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.

Terraform GA: Diff ( 2 files changed, 75 insertions(+), 1 deletion(-))
Terraform Beta: Diff ( 2 files changed, 75 insertions(+), 1 deletion(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 3174
Passed tests 2852
Skipped tests: 319
Affected tests: 3

Action taken

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

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccContainerAwsCluster_BasicEnumHandWritten[Debug log]

Rerun these tests in REPLAYING mode to catch issues

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$


$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccContainerCluster_withGatewayApiConfig[Error message] [Debug log]
TestAccContainerAwsCluster_BasicHandWritten[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

@NickElliot NickElliot requested a review from trodge October 27, 2023 01:48
Copy link
Contributor

@trodge trodge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to add a test that fails without this change and passes with it?

@modular-magician
Copy link
Collaborator

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.

Terraform GA: Diff ( 3 files changed, 107 insertions(+), 1 deletion(-))
Terraform Beta: Diff ( 3 files changed, 107 insertions(+), 1 deletion(-))

@NickElliot NickElliot requested a review from trodge November 7, 2023 22:11
@NickElliot
Copy link
Contributor Author

The test will be skipped in CI because the test precheck wants GOOGLE_ZONE and GOOGLE_REGION set -- but it fails while on current provider, succeeds with this PR's built one.

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 3220
Passed tests 2889
Skipped tests: 329
Affected tests: 2

Action taken

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

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccContainerAwsCluster_BasicEnumHandWritten[Debug log]
TestAccDataprocClusterIamPolicy[Debug log]

Rerun these tests in REPLAYING mode to catch issues

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$


$\textcolor{green}{\textsf{All tests passed!}}$
View the build log or the debug log for each test

@trodge
Copy link
Contributor

trodge commented Nov 8, 2023

The test will be skipped in CI because the test precheck wants GOOGLE_ZONE and GOOGLE_REGION set -- but it fails while on current provider, succeeds with this PR's built one.

What's the error you see when you don't skip the test in VCR?

@modular-magician
Copy link
Collaborator

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.

Terraform GA: Diff ( 3 files changed, 106 insertions(+), 1 deletion(-))
Terraform Beta: Diff ( 3 files changed, 106 insertions(+), 1 deletion(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 3226
Passed tests 2896
Skipped tests: 328
Affected tests: 2

Action taken

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

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccComputeAttachedDisk_zoneless[Debug log]
TestAccDataprocClusterIamPolicy[Debug log]

Rerun these tests in REPLAYING mode to catch issues

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$


$\textcolor{green}{\textsf{All tests passed!}}$
View the build log or the debug log for each test

Copy link
Contributor

@trodge trodge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

google_compute_attached_disk failing for zone
3 participants