Update mutex on google_apigee_instance_attachment to lock globally

[xuchenma]

This PR updates the mutex on google_apigee_instance_attachment to create a global lock.
It solves hashicorp/terraform-provider-google#11445.

If this PR is for Terraform, I acknowledge that I have:

• Searched through the issue tracker for an open issue that this either resolves or contributes to, commented on it to claim it, and written "fixes {url}" or "part of {url}" in this PR description. If there were no relevant open issues, I opened one and commented that I would like to work on it (not necessary for very small changes).
• Generated Terraform, and ran make test and make lint to ensure it passes unit and linter tests.
• Ensured that all new fields I added that can be set by a user appear in at least one example (for generated resources) or third_party test (for handwritten resources or update tests).
• Ran relevant acceptance tests (If the acceptance tests do not yet pass or you are unable to run them, please let your reviewer know).
• Read the Release Notes Guide before writing my release note below.

Release Note Template for Downstream PRs (will be copied)

Update mutex on google_apigee_instance_attachment to lock on org_id.

[modular-magician]

Hello! I am a robot who works on Magic Modules PRs.

I have detected that you are a community contributor, so your PR will be assigned to someone with a commit-bit on this repo for initial review.

Thanks for your contribution! A human will be with you soon.

@melinath, please review this PR or find an appropriate assignee.

[xuchenma]

I tested locally and it has worked. I cannot add a test case as in hashicorp/terraform-provider-google#11445 because only paid ApigeeOrgs can create multiple instances, and paid ApigeeOrgs cannot be automatically created/deleted in CI/CD tests.

[modular-magician]

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 1 file changed, 2 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 1 file changed, 2 insertions(+), 2 deletions(-))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

[modular-magician]

Tests analytics

Total tests: 1971
Passed tests 1731
Skipped tests: 237
Failed tests: 3

Action taken

Triggering VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccAccessContextManager|TestAccContainerCluster_withAuthenticatorGroupsConfig|TestAccComputeGlobalForwardingRule_externalCdnLbWithBackendBucketExample

[modular-magician]

Tests failed during RECORDING mode:
TestAccAccessContextManager[view]
TestAccComputeGlobalForwardingRule_externalCdnLbWithBackendBucketExample[view]
TestAccContainerCluster_withAuthenticatorGroupsConfig[view]

Please fix these to complete your PR
View the build log or the debug log for each test

[melinath]

@xuchenma I don't think this is going to do exactly what you want it to. The mutex will be filled in by getting the {{org_id}} field from the terraform resource - but google_apigee_instance_attachment doesn't have that field. That means the mutex will end up being on an empty string. This does resolve the issue that you identified, but it does it by creating a global lock on instance attachments - not a lock per organization.

Unfortunately there may not be a better solution than a global lock, since the terraform resource only has access to organization and environment. (But it would be better to make that explicit by giving it a hardcoded string as the mutex.)

[xuchenma]

I see, thanks for the explanation! I updated it to lock on a hardcoded string. Tested locally, it worked for me.

[modular-magician]

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 1 file changed, 2 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 1 file changed, 2 insertions(+), 2 deletions(-))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

[modular-magician]

Tests analytics

Total tests: 1971
Passed tests 1730
Skipped tests: 238
Failed tests: 3

Action taken

Triggering VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccFirebaserulesRelease_BasicRelease|TestAccComputeForwardingRule_update|TestAccDatasourceGoogleServiceNetworkingPeeredDnsDomain_basic

[modular-magician]

Tests passed during RECORDING mode:
TestAccDatasourceGoogleServiceNetworkingPeeredDnsDomain_basic[view]
TestAccFirebaserulesRelease_BasicRelease[view]
TestAccComputeForwardingRule_update[view]

All tests passed
View the build log or the debug log for each test

[melinath]

it would be good to add the resource name to the global mutex name; for now I think this is the only one but we should still namespace it to be future-proof.

[xuchenma]

Got it, updated!

[modular-magician]

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 1 file changed, 2 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 1 file changed, 2 insertions(+), 2 deletions(-))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

[modular-magician]

Tests analytics

Total tests: 1971
Passed tests 1731
Skipped tests: 238
Failed tests: 2

Action taken

Triggering VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccFirebaserulesRelease_BasicRelease|TestAccServiceNetworkingPeeredDNSDomain_basic

[modular-magician]

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[view]
TestAccServiceNetworkingPeeredDNSDomain_basic[view]

All tests passed
View the build log or the debug log for each test