Adds `grpc` field to `liveness_probe` and `startup_probe` to `google_cloud_run_v2_service` resource #6987

mmizutani · 2022-12-18T06:18:46Z

This PR adds gRPC probes options for Cloud Run v2 Service resources.

The grpc fields in startup_probe and liveness_probe in the google_cloud_run_service (v1) resource were added by #6781.
The support for Cloud Run v2 Services was introduced by #6850 and released in terraform-google-provider 4.46.0, but the gRPC options for liveness probe and startup probe for the v2 services are missing.

The Go SDK v0.102.0, which this project currently uses, does not yet support the gRPC probe options for Cloud Run v2 Services. The SDK needs to be upgraded to at least v0.104.0.
So this PR waits for #6962 to be merged.

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)

added `template.0.containers0.liveness_probe.grpc`, `template.0.containers0.startup_probe.grpc` fields to `google_cloud_run_v2_service` resource

modular-magician · 2022-12-18T06:19:45Z

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

I've detected that you're a community contributor. @ScottSuarez, a repository maintainer, has been assigned to assist you and help review your changes.

❓ First time contributing? Click here for more details

Your assigned reviewer will help review your code by:

Ensuring it's backwards compatible, covers common error cases, etc.
Summarizing the change into a user-facing changelog note.
Passes tests, either our "VCR" suite, a set of presubmit tests, or with manual test runs.

You can help make sure that review is quick by running local tests and ensuring they're passing in between each push you make to your PR's branch. Also, try to leave a comment with each push you make, as pushes generally don't generate emails.

If your reviewer doesn't get back to you within a week after your most recent change, please feel free to leave a comment on the issue asking them to take a look! In the absence of a dedicated review dashboard most maintainers manage their pending reviews through email, and those will sometimes get lost in their inbox.

modular-magician · 2022-12-18T06:45:55Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 3 files changed, 384 insertions(+), 7 deletions(-))
Terraform Beta: Diff ( 3 files changed, 384 insertions(+), 7 deletions(-))
TF Validator: Diff ( 3 files changed, 95 insertions(+), 3 deletions(-))

modular-magician · 2022-12-18T06:47:20Z

Tests analytics

Total tests: 0
Passed tests 0
Skipped tests: 0
Failed tests: 0

Errors occurred during REPLAYING mode. Please fix them to complete your PR
View the build log

ScottSuarez · 2022-12-19T23:43:26Z

Hello I notice this PR is in draft. Please let me know if anything is needed!

mmizutani · 2022-12-20T00:25:55Z

Thanks. I am waiting for #6962 to be merged.

modular-magician · 2023-01-03T22:48:28Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 3 files changed, 384 insertions(+), 7 deletions(-))
Terraform Beta: Diff ( 3 files changed, 384 insertions(+), 7 deletions(-))
TF Validator: Diff ( 3 files changed, 95 insertions(+), 3 deletions(-))

modular-magician · 2023-01-03T22:49:42Z

Tests analytics

Total tests: 0
Passed tests 0
Skipped tests: 0
Failed tests: 0

Errors occurred during REPLAYING mode. Please fix them to complete your PR
View the build log

modular-magician · 2023-01-04T10:40:14Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 3 files changed, 381 insertions(+), 7 deletions(-))
Terraform Beta: Diff ( 3 files changed, 381 insertions(+), 7 deletions(-))
TF Validator: Diff ( 3 files changed, 95 insertions(+), 3 deletions(-))

modular-magician · 2023-01-04T10:41:53Z

Tests analytics

Total tests: 0
Passed tests 0
Skipped tests: 0
Failed tests: 0

Errors occurred during REPLAYING mode. Please fix them to complete your PR
View the build log

modular-magician · 2023-01-04T11:01:40Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 3 files changed, 379 insertions(+), 7 deletions(-))
Terraform Beta: Diff ( 3 files changed, 379 insertions(+), 7 deletions(-))
TF Validator: Diff ( 3 files changed, 95 insertions(+), 3 deletions(-))

modular-magician · 2023-01-04T11:03:01Z

Tests analytics

Total tests: 0
Passed tests 0
Skipped tests: 0
Failed tests: 0

Errors occurred during REPLAYING mode. Please fix them to complete your PR
View the build log

modular-magician · 2023-01-04T14:49:46Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 3 files changed, 379 insertions(+), 7 deletions(-))
Terraform Beta: Diff ( 3 files changed, 379 insertions(+), 7 deletions(-))
TF Validator: Diff ( 3 files changed, 95 insertions(+), 3 deletions(-))

GNUmakefile

modular-magician · 2023-01-04T15:33:33Z

Tests analytics

Total tests: 2386
Passed tests 2135
Skipped tests: 248
Failed tests: 3

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccContainerCluster_withInvalidGatewayApiConfigChannel|TestAccCloudRunV2Service_cloudrunv2ServiceProbesExample|TestAccCloudRunV2Service_cloudrunv2ServiceProbesUpdate

modular-magician · 2023-01-04T15:35:50Z

Tests passed during RECORDING mode:
TestAccContainerCluster_withInvalidGatewayApiConfigChannel[Debug log]
TestAccCloudRunV2Service_cloudrunv2ServiceProbesExample[Debug log]

Tests failed during RECORDING mode:
TestAccCloudRunV2Service_cloudrunv2ServiceProbesUpdate[Error message] [Debug log]

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

modular-magician · 2023-01-07T03:37:42Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 3 files changed, 379 insertions(+), 7 deletions(-))
Terraform Beta: Diff ( 3 files changed, 379 insertions(+), 7 deletions(-))
TF Validator: Diff ( 3 files changed, 95 insertions(+), 3 deletions(-))

ScottSuarez · 2023-02-16T21:49:07Z

/gcbrun :)

modular-magician · 2023-02-16T22:05:57Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 5 files changed, 534 insertions(+), 5 deletions(-))
Terraform Beta: Diff ( 5 files changed, 534 insertions(+), 5 deletions(-))
TF Validator: Diff ( 4 files changed, 100 insertions(+), 3 deletions(-))

modular-magician · 2023-02-16T22:47:36Z

Tests analytics

Total tests: 2468
Passed tests 2209
Skipped tests: 256
Failed tests: 3

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccFirebaserulesRelease_BasicRelease|TestAccCloudRunV2Service_cloudrunv2ServiceGRPCProbesUpdate|TestAccApigeeAddonsConfig_apigeeAddonsTestExample

modular-magician · 2023-02-16T22:59:42Z

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[Debug log]
TestAccApigeeAddonsConfig_apigeeAddonsTestExample[Debug log]

Tests failed during RECORDING mode:
TestAccCloudRunV2Service_cloudrunv2ServiceGRPCProbesUpdate[Error message] [Debug log]

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

ScottSuarez · 2023-02-17T00:35:51Z

Looks like still an outstanding test failure

=== CONT  TestAccCloudRunV2Service_cloudrunv2ServiceGRPCProbesUpdate
    provider_test.go:315: Step 1/8 error: Error running apply: exit status 1
        
        Error: Error creating Service: googleapi: Error 400: Violation in CreateServiceRequest.service.template.containers[0].liveness_probe.probe_type: required oneof field 'probe_type' must have one initialized field
        Details:
        [
          {
            "@type": "type.googleapis.com/google.rpc.BadRequest",
            "fieldViolations": [
              {
                "description": "required oneof field 'probe_type' must have one initialized field",
                "field": "Violation in CreateServiceRequest.service.template.containers[0].liveness_probe.probe_type"
              }
            ]
          }
        ]
        
          with google_cloud_run_v2_service.default,
          on terraform_plugin_test.tf line 3, in resource "google_cloud_run_v2_service" "default":
           3: resource "google_cloud_run_v2_service" "default" {
        
--- FAIL: TestAccCloudRunV2Service_cloudrunv2ServiceGRPCProbesUpdate (2.04s)

…empty true

mmizutani · 2023-02-18T04:55:00Z

Let me summarize the current situation of this pull request review.

This pull request has been in an intricate deadlock due to the following conflicting suggestions based on different perspectives from different reviewers regarding the optionality of the fields of the service.template.containers[0].liveness_probe.grpc and service.template.containers[0].startup_probe.grpc blocks.

@ScottSuarez advised that at least one of the fields of the grpc blocks should be marked as required since enabling features with empty blocks is unintuitive and confusing from the perspective of the general Terraform convention.
@yanweiguo suggested that the grpc blocks should be allowed to be empty to be in line with the v1 run resources as well as the http_get and tcp_socket blocks of the v1/v2 run resources.
@shuyama1 implied that the v1 run resource and v2 run resource http_get and tcp_socket blocks intentionally allowed to be empty in Terraform syntax in order to send empty fields to the Google Run Admin API since requests missing the fields and requests with empty fields are interpreted differently by the API.

I have confirmed that @shuyama1's point is likely true. When I allow empty grpc blocks in Terraform and the following v2 run resource with an empty grpc block, I observed the following POST request with an JSON body containing an empty grpc block field rather than a null grpc field, and the API indeed interpreted the empty block fields and the null fields differently.

resource "google_cloud_run_v2_service" "default" {
  name     ="%{service_name}"
  location = "us-central1"

  template {
    containers {
      image = "us-docker.pkg.dev/cloudrun/container/hello"
      ports {
        container_port = 8080
      }
      liveness_probe {
        grpc {}
      }
    }
  }
}

---[ REQUEST ]---------------------------------------
POST /v2/projects/PROJECT_ID/locations/us-central1/services?alt=json&serviceId=tf-test-cloudrun-serviceuut6x9qm0w HTTP/1.1
Host: run.googleapis.com
User-Agent: Terraform/1.3.6 (+https://www.terraform.io) Terraform-Plugin-SDK/2.10.1 terraform-provider-google/acc
Content-Length: 237
Content-Type: application/json
Accept-Encoding: gzip

{
 "template": {
  "containers": [
   {
    "image": "us-docker.pkg.dev/cloudrun/container/hello",
    "livenessProbe": {
     "failureThreshold": 3,
     "grpc": {}, /// {} not null
     "httpGet": null,
     "periodSeconds": 10,
     "tcpSocket": null,
     "timeoutSeconds": 1
    },
    "ports": [
     {
      "containerPort": 8080
     }
    ]
   }
  ]
 }
}

As long as sending a JSON request body with empty, non-null fields requires writing empty blocks in Terraform code in google-terraform-provider, it seems that we need to allow template.containers[0].liveness_probe.grpc and service.template.containers[0].startup_probe.grpc blocks to be empty just like http_get and tcp_socket as they are today.
Given such tight coupling of the Terraform block syntax and JSON API request body, to avoid confusing situations where users have to enable features using empty Terraform blocks, we would need to either refactor the syntax of google_cloud_run_v2_service to allow enabling features without writing empty blocks or simply mark at least one of the fields of the blocks as required. The former of course introduces breaking changes and would be unacceptable. The latter is much easier to implement but causes inconsistency with the existing siblings that allow setting empty blocks (v1 run resource and v2 run resource's http_get and tcp_socket blocks), and changing the existing http_get and tcp_socket blocks from allowing empty blocks to disallowing empty blocks also breaks backward compatibility.
Introducing a new field enabled = true|false within the grpc block is awkward here since grpc is one of the exactly_one_of blocks.

To move things forward, I would suggest that we allow service.template.containers[0].liveness_probe.grpc and service.template.containers[0].startup_probe.grpc to be set empty just like similar, other parts of this repository and move the continuation of the discussion on the appropriateness and alternatives of the currently allowed empty blocks syntax to a standalone repository issue ticket as this issue pertains to lots of other parts of this repository as well (my original suggestion).

Could you discuss among you three reviewers and reconcile opinions as to how to proceed?
It would be totally fine to agree to disagree, but differentiating pull request specific concerns and repository wide concerns and addressing them separately would be appreciated.

ScottSuarez · 2023-02-22T19:55:37Z

Hi @mmizutani, let go with your suggestion. Unfortunately there are scenarios, as you mentioned, where we need to utilize empty blocks. It's not an ideal design pattern from my point of view but it's not a blocker. You can go ahead and implement it as you think best and we can work to get that in. I'll follow up with any clarifying questions.

mmizutani · 2023-02-25T08:26:22Z

@ScottSuarez
Acceptance tests for the google_cloud_run_v2_service resource are passing in my local environment. Could you trigger the generate-diffs and run-rake-tests CI workflows again?

trodge · 2023-02-25T16:55:12Z

/gcbrun

modular-magician · 2023-02-25T17:12:36Z

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 5 files changed, 538 insertions(+), 5 deletions(-))
Terraform Beta: Diff ( 5 files changed, 538 insertions(+), 5 deletions(-))
TF Validator: Diff ( 4 files changed, 110 insertions(+), 3 deletions(-))

modular-magician · 2023-02-25T17:58:38Z

Tests analytics

Total tests: 2475
Passed tests 2214
Skipped tests: 255
Failed tests: 6

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccCloudRunV2Service_cloudrunv2ServiceGRPCProbesUpdate|TestAccCloudRunV2Service_cloudrunv2ServiceHTTPProbesUpdate|TestAccCloudRunV2Service_cloudrunv2ServiceTCPProbesUpdate|TestAccCloudRunV2Service_cloudrunv2ServiceProbesExample|TestAccComputeForwardingRule_update|TestAccWorkstationsWorkstationConfig_workstationConfigEncryptionKeyExample

modular-magician · 2023-02-25T18:50:43Z

Tests passed during RECORDING mode:
TestAccCloudRunV2Service_cloudrunv2ServiceGRPCProbesUpdate[Debug log]
TestAccCloudRunV2Service_cloudrunv2ServiceHTTPProbesUpdate[Debug log]
TestAccCloudRunV2Service_cloudrunv2ServiceTCPProbesUpdate[Debug log]
TestAccCloudRunV2Service_cloudrunv2ServiceProbesExample[Debug log]
TestAccComputeForwardingRule_update[Debug log]

Tests failed during RECORDING mode:
TestAccWorkstationsWorkstationConfig_workstationConfigEncryptionKeyExample[Error message] [Debug log]

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

ScottSuarez

Thanks for your work and patience @mmizutani :)

…cloud_run_v2_service` resource (GoogleCloudPlatform#6987)

* main: (41 commits) update the test cases to resolve resourcename not found error Adds `grpc` field to `liveness_probe` and `startup_probe` to `google_cloud_run_v2_service` resource (GoogleCloudPlatform#6987) Upgrade DCL to v1.34 (GoogleCloudPlatform#7276) Add max_distance field to group placement policy (GoogleCloudPlatform#7354) Add stateful_ips to region_per_instance_config and per_instance_config (GoogleCloudPlatform#7316) Added support for workload-vulnerability-scanning and workload-config-audit (GoogleCloudPlatform#7310) datacatalog - bump Taxonomy and PolicyTag to ga (GoogleCloudPlatform#6989) Added best practices documentation for ForceNew fields (GoogleCloudPlatform#7127) Split resources in "B" products (GoogleCloudPlatform#7350) force recreate on master_config.num_instances (GoogleCloudPlatform#7349) Fix DataFusion instance versions used in tests (GoogleCloudPlatform#7343) remove duplicate word in Cluster.yaml (GoogleCloudPlatform#7347) Move more billing tests that require permissions beyond Billing User to master billing account (GoogleCloudPlatform#7344) Remove artifact repository beta URL, fixup handwritten tests (GoogleCloudPlatform#7345) Cloud Workstations - Workstation Config (GoogleCloudPlatform#7017) Add missing `type` argument to data source docs (GoogleCloudPlatform#7341) Fix caps in spanner resource schema accesses (GoogleCloudPlatform#7346) Downgrade Go to 1.18, modify comments (GoogleCloudPlatform#7339) feat: Add support for deletion_policy on shared vpc service project (GoogleCloudPlatform#7283) fixed virtual field update issues (GoogleCloudPlatform#7318) ...

…cloud_run_v2_service` resource (GoogleCloudPlatform#6987)

Add grpc liveness/startup probes to Cloud Run v2 Service

a690964

modular-magician requested a review from ScottSuarez December 18, 2022 06:19

mmizutani force-pushed the cloud-run-v2-grpc-probe branch from 919999b to d75f700 Compare December 18, 2022 06:28

Add tests

2286421

mmizutani force-pushed the cloud-run-v2-grpc-probe branch from d75f700 to 2286421 Compare December 18, 2022 06:30

Merge branch 'GoogleCloudPlatform:main' into cloud-run-v2-grpc-probe

60972f5

mmizutani added 3 commits January 4, 2023 19:00

Fix typo

aa46350

Use fully qualified gRPC service names

87ed0f0

Remove BETA annotations since healthcheck probes got GA on Dec 9, 2022

c3e709a

mmizutani and others added 2 commits January 4, 2023 19:45

Fix build errors

93cbc33

Merge branch 'GoogleCloudPlatform:main' into cloud-run-v2-grpc-probe

aa465c6

mmizutani added 2 commits January 4, 2023 23:32

Correct Sprintf to Nprintf

badf2ae

Add test make command

e99eaed

mmizutani commented Jan 4, 2023

View reviewed changes

GNUmakefile Outdated Show resolved Hide resolved

Merge branch 'GoogleCloudPlatform:main' into cloud-run-v2-grpc-probe

23ffce2

mmizutani requested a review from a team as a code owner February 16, 2023 20:48

mmizutani and others added 2 commits February 17, 2023 06:00

Merge branch 'GoogleCloudPlatform:main' into cloud-run-v2-grpc-probe

24a13cb

Fix yaml properties order

6e50c09

modular-magician requested a review from shuyama1 February 16, 2023 21:50

mmizutani and others added 2 commits February 18, 2023 11:41

Restore the original implementation with exactly_one_of comments and …

a213d1f

…empty true

Merge branch 'GoogleCloudPlatform:main' into cloud-run-v2-grpc-probe

352092f

Merge branch 'main' into cloud-run-v2-grpc-probe

68ddd69

ScottSuarez approved these changes Feb 27, 2023

View reviewed changes

ScottSuarez merged commit e2918f1 into GoogleCloudPlatform:main Feb 27, 2023

mmizutani deleted the cloud-run-v2-grpc-probe branch February 28, 2023 00:30

mdtro pushed a commit to mdtro/magic-modules that referenced this pull request Mar 2, 2023

Adds grpc field to liveness_probe and startup_probe to `google_…

1eff04b

…cloud_run_v2_service` resource (GoogleCloudPlatform#6987)

ericayyliu pushed a commit to ericayyliu/magic-modules that referenced this pull request Jul 26, 2023

Adds grpc field to liveness_probe and startup_probe to `google_…

b4dcd00

…cloud_run_v2_service` resource (GoogleCloudPlatform#6987)

rileykarson mentioned this pull request Mar 4, 2024

removing liveness_probe declaration from google_cloud_run_v2_service does not remove the probe from the service hashicorp/terraform-provider-google#17468

Open

yanweiguo mentioned this pull request May 30, 2024

Support gRPC probes for google_cloud_run_service hashicorp/terraform-provider-google#12954

Open

Adds grpc field to liveness_probe and startup_probe to google_cloud_run_v2_service resource #6987

Adds grpc field to liveness_probe and startup_probe to google_cloud_run_v2_service resource #6987

Uh oh!

Conversation

mmizutani commented Dec 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

modular-magician commented Dec 18, 2022

Uh oh!

modular-magician commented Dec 18, 2022

Diff report

Uh oh!

modular-magician commented Dec 18, 2022

Tests analytics

Uh oh!

ScottSuarez commented Dec 19, 2022

Uh oh!

mmizutani commented Dec 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

modular-magician commented Jan 3, 2023

Diff report

Uh oh!

modular-magician commented Jan 3, 2023

Tests analytics

Uh oh!

modular-magician commented Jan 4, 2023

Diff report

Uh oh!

modular-magician commented Jan 4, 2023

Tests analytics

Uh oh!

modular-magician commented Jan 4, 2023

Diff report

Uh oh!

modular-magician commented Jan 4, 2023

Tests analytics

Uh oh!

modular-magician commented Jan 4, 2023

Diff report

Uh oh!

Uh oh!

modular-magician commented Jan 4, 2023

Tests analytics

Action taken

Uh oh!

modular-magician commented Jan 4, 2023

Uh oh!

modular-magician commented Jan 7, 2023

Diff report

Uh oh!

ScottSuarez commented Feb 16, 2023

Uh oh!

modular-magician commented Feb 16, 2023

Diff report

Uh oh!

modular-magician commented Feb 16, 2023

Tests analytics

Action taken

Uh oh!

modular-magician commented Feb 16, 2023

Uh oh!

ScottSuarez commented Feb 17, 2023

Uh oh!

mmizutani commented Feb 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ScottSuarez commented Feb 22, 2023

Uh oh!

mmizutani commented Feb 25, 2023

Uh oh!

trodge commented Feb 25, 2023

Uh oh!

modular-magician commented Feb 25, 2023

Diff report

Uh oh!

modular-magician commented Feb 25, 2023

Tests analytics

Action taken

Uh oh!

Adds `grpc` field to `liveness_probe` and `startup_probe` to `google_cloud_run_v2_service` resource #6987

Adds `grpc` field to `liveness_probe` and `startup_probe` to `google_cloud_run_v2_service` resource #6987

mmizutani commented Dec 18, 2022 •

edited

Loading

mmizutani commented Dec 20, 2022 •

edited

Loading

mmizutani commented Feb 18, 2023 •

edited

Loading