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
Cloudrun v2 service #6850
Cloudrun v2 service #6850
Conversation
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 ( 8 files changed, 4611 insertions(+), 2 deletions(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccLoggingBucketConfigProject_cmekSettings|TestAccComputeGlobalForwardingRule_globalForwardingRuleInternalExample|TestAccComputeGlobalForwardingRule_externalHttpLbMigBackendCustomHeaderExample|TestAccComputeGlobalForwardingRule_externalTcpProxyLbMigBackendExample|TestAccComputeForwardingRule_internalTcpUdpLbWithMigBackendExample|TestAccComputeForwardingRule_forwardingRuleRegionalHttpXlbExample|TestAccComputeForwardingRule_forwardingRuleHttpLbExample|TestAccComputeForwardingRule_update|TestAccComputeForwardingRule_internalHttpLbWithMigBackendExample|TestAccComputeGlobalForwardingRule_internalLoadBalancing|TestAccCloudRunV2Service_cloudrunv2ServiceSqlExample|TestAccCloudRunV2Service_cloudrunv2ServiceBasicExample |
Tests passed during RECORDING mode: All tests passed |
554536c
to
033bb05
Compare
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 ( 9 files changed, 5155 insertions(+), 2 deletions(-)) |
description: |- | ||
Map of string keys and values that can be used to organize and categorize objects. User-provided labels are shared with Google's billing system, so they can be used to filter, or break down billing charges by team, component, environment, state, etc. For more information, visit https://cloud.google.com/resource-manager/docs/creating-managing-labels or https://cloud.google.com/run/docs/configuring/labels Cloud Run will populate some labels with 'run.googleapis.com' or 'serving.knative.dev' namespaces. Those labels are read-only, and user changes will not be preserved. | ||
# Block on b/244872932 for implementation | ||
# - !ruby/object:Api::Type::KeyValuePairs |
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.
annotation
is currently blocked on b/244872932. Discussed with the service team and decide to omit the field from implementation until the API validation is launched.
name: "value" | ||
description: |- | ||
Variable references $(VAR_NAME) are expanded using the previous defined environment variables in the container and any route environment variables. If a variable cannot be resolved, the reference in the input string will be unchanged. The $(VAR_NAME) syntax can be escaped with a double $$, ie: $$(VAR_NAME). Escaped references will never be expanded, regardless of whether the variable exists or not. Defaults to "", and the maximum length is 32768 bytes | ||
# exactly_one_of: |
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.
ConflictsWith
, ExactlyOneOf
and AtLeastOneOf
within lists are currently not supported at the SDK level.
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccLoggingBucketConfigProject_cmekSettings|TestAccCloudRunV2Service_cloudrunv2ServiceProbesUpdate|TestAccCloudRunV2Service_cloudrunv2ServiceFullUpdate|TestAccCloudRunV2Service_cloudrunv2ServiceProbesExample|TestAccCloudRunV2Service_cloudrunv2ServiceVpcaccessExample|TestAccCloudRunV2Service_cloudrunv2ServiceSqlExample |
Tests passed during RECORDING mode: All tests passed |
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.
Overall this looks great to me. I noticed some small things that might need changes, but the only thing that seems like a potential blocker is a field called status
which I think needs to be state
.
mmv1/templates/terraform/examples/cloudrunv2_service_probes.tf.erb
Outdated
Show resolved
Hide resolved
033bb05
to
10c7524
Compare
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 ( 9 files changed, 5155 insertions(+), 2 deletions(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccContainerNodePool_withNodeConfig|TestAccLoggingBucketConfigProject_cmekSettings |
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.
Adding Stephen for follow-up review
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.
a couple small changes, lgtm otherwise.
10c7524
to
7a6edb9
Compare
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 ( 9 files changed, 5142 insertions(+), 2 deletions(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccCloudRunV2Service_cloudrunv2ServiceFullUpdate|TestAccLoggingBucketConfigProject_cmekSettings |
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.
LGTM - it could be nice to have tests for container.command
and liveness_probe.tcp_socket
for this resource as well.
7a6edb9
to
7b932d1
Compare
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 ( 6 files changed, 5256 insertions(+), 2 deletions(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccLoggingBucketConfigProject_cmekSettings|TestAccCloudRunV2Service_cloudrunv2ServiceFullUpdate|TestAccCloudRunV2Service_cloudrunv2ServiceSecretExample|TestAccCloudRunV2Service_cloudrunv2ServiceProbesUpdate|TestAccCloudRunV2Service_cloudrunv2ServiceSqlExample |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
7b932d1
to
587fba5
Compare
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccFirebaserulesRelease_BasicRelease|TestAccLoggingBucketConfigProject_cmekSettings |
/gcbrun |
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 ( 6 files changed, 5260 insertions(+), 2 deletions(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccFirebaserulesRelease_BasicRelease|TestAccLoggingBucketConfigProject_cmekSettings |
@melinath I suspect the TFV tests failed here maybe due to the TFV build failure for the last two commits. |
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.
+1 TFV failure is unrelated - opened #6904 to remove that test case for now.
587fba5
to
5ebdbce
Compare
Only rebase to get TFV fix in. No new code changes and will merge it after tests pass. |
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 ( 6 files changed, 5260 insertions(+), 2 deletions(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccLoggingBucketConfigProject_cmekSettings|TestAccFirebaserulesRelease_BasicRelease |
Internal ticket: b/254103525
API: https://cloud.google.com/run/docs/reference/rest/v2/projects.locations.services
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)