-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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 field traffic.tag
and traffic.url
to Cloud Run
#5772
Add field traffic.tag
and traffic.url
to Cloud Run
#5772
Conversation
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. @rileykarson, please review this PR or find an appropriate assignee. |
Can you use the |
I added the |
This one looks good! |
I was on my way to making this exact change, but then I discovered this PR 🎉. Making the @rileykarson, insofar that I can see, the pipeline looks green. Is there anything missing in this PR so that it can be merged? Edit: @ybourgery, this might be a silly suggestion, but maybe it this helps give this PR more attention and fast-track it to |
Sorry! Lost email on my part, I'll fix the Magician message to provide a call-to-action. @ybourgery: Do you mind rebasing this? We've changed part of our CI, and it partially draws from the config at the merge base. |
@rileykarson I did the rebase |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccOrganizationPolicy|TestAccServiceNetworkingPeeredDNSDomain_basic|TestAccFirebaserulesRelease_BasicRelease|TestAccCloudRunService_foregroundDeletion|TestAccCloudRunService_cloudRunServiceUpdate|TestAccDatasourceGoogleServiceNetworkingPeeredDnsDomain_basic |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
This seems to cause an issue when health checking for both tests affected by the changed config, here's the error:
I'm not super familiar with Cloud Run configuration in-depth, but a read of https://cloud.google.com/run/docs/rollouts-rollbacks-traffic-migration#tags suggests that we probably need a second untagged traffic block, in order to receive traffic and pass the health check. |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 3 files changed, 54 insertions(+), 1 deletion(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccCloudRunService_foregroundDeletion|TestAccCloudRunService_cloudRunServiceUpdate|TestAccServiceNetworkingPeeredDNSDomain_basic|TestAccDatasourceGoogleServiceNetworkingPeeredDnsDomain_basic |
@rileykarson I think the issue was coming from the fact that the test was using the |
Tests passed during RECORDING mode: All tests passed |
Cool! Thanks for figuring that out. |
Fixes hashicorp/terraform-provider-google#8654
If this PR is for Terraform, I acknowledge that I have:
make test
andmake lint
to ensure it passes unit and linter tests.Release Note Template for Downstream PRs (will be copied)