fix: IAM Policy 409 concurrent changes error - take 2#15995
fix: IAM Policy 409 concurrent changes error - take 2#15995slevenick merged 5 commits intoGoogleCloudPlatform:mainfrom
Conversation
|
Hello! I am a robot. Tests will require approval from a repository maintainer to run. Googlers: For automatic test runs see go/terraform-auto-test-runs. @slevenick, 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. |
|
@slevenick This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @slevenick This PR has been waiting for review for 1 week. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @slevenick This PR has been waiting for review for 2 weeks. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @slevenick This PR has been waiting for review for 3 weeks. Please take a look! Use the label |
|
@slevenick Can you take a look please? |
|
@slevenick This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
slevenick
left a comment
There was a problem hiding this comment.
I think this is a better approach and shouldn't have the impact of the previous change. I'm going to get another pair of eyes on this as it's such a core part of shared provider functionality
|
Sure thing |
|
@slevenick This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @slevenick This PR has been waiting for review for 1 week. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @slevenick This PR has been waiting for review for 2 weeks. Please take a look! Use the label |
|
@slevenick got a chance to have someone else look at it? Seems like the bot assigned you again |
…-modules into fix/25305-take2
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Tests analyticsTotal tests: 6011 Click here to see the affected service packages
Action takenFound 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
|
🟢 Tests passed during RECORDING mode: 🔴 Tests failed when rerunning REPLAYING mode: Tests failed due to non-determinism or randomness when the VCR replayed the response after the HTTP request was made. Please fix these to complete your PR. If you believe these test failures to be incorrect or unrelated to your change, or if you have any questions, please raise the concern with your reviewer. 🟢 All tests passed! |
36e9056
Fixes hashicorp/terraform-provider-google#25305
There is already a backoff retry mechanism but it's broken due to wrapped error. The current usage for checking if it's googleapi error doesn't work well.
#15972 reverted #15825 due to regressions. However, the original IAM 409 concurrent error is now back due to that.
I debugged that specific error and found that the
errvar returned is by the typefmt.Errorfon level 0 andgoogleapi.Erroron level 1.errwrap simply doesn't detect it.
I wanted to avoid walking or unwrapping and make minimal changes as possible.
Given the fact we have to keep using errwrap for now, I added the same changes I did originally below the current errwrap code and also I added tests.
I tested the generated provider and it solves the 409 issue.
Tagging @slevenick @ScottSuarez @BBBmau for extra verification and input.
Release Note Template for Downstream PRs (will be copied)
See Write release notes for guidance.