-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Add random suffix to Cloud Workstations SA to prevent cross test runs from failing #7362
Add random suffix to Cloud Workstations SA to prevent cross test runs from failing #7362
Conversation
Oops! It looks like you're using an unknown release-note type in your changelog entries:
Please only use the types listed in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/.ci/RELEASE_NOTES_GUIDE.md. |
I see a lot of SAs also being created as following: resource "google_service_account" "test-account" {
account_id = "<%= ctx[:vars]['account_id'] %>"
display_name = "Test Service Account"
} What's preferred? |
@@ -42,7 +42,7 @@ resource "google_kms_crypto_key" "default" { | |||
} | |||
|
|||
resource "google_service_account" "default" { | |||
account_id = "cloud-workstations-kms" | |||
account_id = "workstations%{random_suffix}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't defined random_suffix
anywhere, but noticed it was present in many tests. Is it being injected automagically?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it'll be injected when "<%= ctx[:vars]['account_id'] %>"
is supplied (alongside a corresponding entry in terraform.yaml)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've used account_id
and set it through the terraform.yaml
like the other tests do, still confused how this will solve the issue - all the account_id
entries in other tests are set to 'my-account'... will this not cause the same issue?
That's the preferred method! Per your other comment, the random suffix is injected based on this pattern being used: https://googlecloudplatform.github.io/magic-modules/docs/how-to/add-mmv1-test/#example-configuration-file and https://googlecloudplatform.github.io/magic-modules/docs/how-to/add-mmv1-test/#results That will add a simple value to documentation, but add the suffix to the test so that we ensure the value is unique. |
Oops! It looks like you're using an unknown release-note type in your changelog entries:
Please only use the types listed in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/.ci/RELEASE_NOTES_GUIDE.md. |
@rileykarson can you check my latest commit :)? |
mmv1/templates/terraform/examples/workstation_config_encryption_key.tf.erb
Show resolved
Hide resolved
…n_key.tf.erb Co-authored-by: Riley Karson <rileykarson@google.com>
/gcbrun |
Looks good, running tests to verify. |
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. Terraform GA: Diff ( 1 file changed, 3 insertions(+), 3 deletions(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccWorkstationsWorkstationConfig_workstationConfigEncryptionKeyExample|TestAccFirebaserulesRelease_BasicRelease |
🤔 |
Re-recording is expected, since we changed the value. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rake error can be ignored (rebase would resolve)
… from failing (GoogleCloudPlatform#7362) Co-authored-by: Riley Karson <rileykarson@google.com>
… from failing (GoogleCloudPlatform#7362) Co-authored-by: Riley Karson <rileykarson@google.com>
If this PR is for Terraform, I acknowledge that I have:
make test
andmake lint
in the generated providers to ensure it passes unit and linter tests.Release Note Template for Downstream PRs (will be copied)