-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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 job #6852
Cloudrun v2 job #6852
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, 4321 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 testsTestAccComputeGlobalForwardingRule_externalHttpLbMigBackendCustomHeaderExample|TestAccComputeGlobalForwardingRule_externalTcpProxyLbMigBackendExample|TestAccComputeGlobalForwardingRule_internalLoadBalancing|TestAccComputeForwardingRule_forwardingRuleRegionalHttpXlbExample|TestAccComputeForwardingRule_forwardingRuleHttpLbExample|TestAccComputeGlobalForwardingRule_globalForwardingRuleInternalExample|TestAccComputeForwardingRule_internalTcpUdpLbWithMigBackendExample|TestAccComputeForwardingRule_internalHttpLbWithMigBackendExample|TestAccCloudRunV2Job_cloudrunv2JobSqlExample|TestAccCloudRunV2Job_cloudrunv2JobBasicExample|TestAccLoggingBucketConfigProject_cmekSettings |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
c5ae89b
to
cab0a18
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, 4822 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|TestAccContainerNodePool_withNodeConfig|TestAccCloudRunV2Job_cloudrunv2JobFullUpdate|TestAccCloudRunV2Job_cloudrunv2JobProbesExample|TestAccCloudRunV2Job_cloudrunv2JobProbesUpdate|TestAccCloudRunV2Job_cloudrunv2JobVpcaccessExample |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
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, 4820 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 testsTestAccCloudRunV2Job_cloudrunv2JobFullUpdate|TestAccLoggingBucketConfigProject_cmekSettings|TestAccFirebaserulesRelease_BasicRelease |
up ? |
a9012ab
to
77f8870
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, 4962 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|TestAccDataFusionInstance_dataFusionInstanceZoneExample|TestAccDataFusionInstance_dataFusionInstanceEventExample|TestAccDataFusionInstance_dataFusionInstanceFullExample|TestAccCloudRunV2Job_cloudrunv2JobSecretExample|TestAccDataFusionInstanceIamPolicyGenerated|TestAccDataFusionInstanceIamMemberGenerated|TestAccDataFusionInstanceIamBindingGenerated|TestAccVertexAIIndex_updated|TestAccVertexAIIndex_vertexAiIndexStreamingExample|TestAccVertexAIIndex_vertexAiIndexExample|TestAccLoggingBucketConfigProject_cmekSettings |
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.
Looks great! A vast majority of this matches the cloudrun service, and from what I saw, everything looked consistent.
Only nit is that I didn't see some fields in the update test (for example, container.command
and livenessProbe.tcpSocket
), but I don't know if we generally aim for full coverage there.
Adding Stephen for follow-up
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 - one question below.
Adding tests for the fields Ryan mentioned above does seem like it would be nice to do.
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.
Copying from the cloud run service PR:
ConflictsWith
,ExactlyOneOf
andAtLeastOneOf
within lists are currently not supported at the SDK level.
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, 5025 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|TestAccCloudFunctions2Function_fullUpdate|TestAccCloudfunctions2function_cloudfunctions2BasicAuditlogsExample|TestAccCloudfunctions2function_cloudfunctions2BasicGcsExample|TestAccCloudRunV2Job_cloudrunv2JobFullUpdate|TestAccCloudRunV2Job_cloudrunv2JobProbesUpdate |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
cl/492014758 which disallows job.name in v2 CreateJobRequest is submitted and will be rolled out next week. Therefore, I marked the |
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, 5001 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|TestAccCloudRunV2Job_cloudrunv2JobBasicExample|TestAccCloudRunV2Job_cloudrunv2JobVpcaccessExample|TestAccCloudRunV2Job_cloudrunv2JobSqlExample|TestAccCloudRunV2Job_cloudrunv2JobProbesUpdate|TestAccCloudRunV2Job_cloudrunv2JobFullUpdate|TestAccCloudRunV2Job_cloudrunv2JobSecretExample|TestAccCloudRunV2Job_cloudrunv2JobProbesExample|TestAccLoggingBucketConfigProject_cmekSettings |
Tests passed during RECORDING mode: All tests passed |
This should resolve hashicorp/terraform-provider-google#11743
Internal ticket: b/240175294
API doc can be found in the internal ticket
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)