Skip to content
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

google_cloud_run_v2_service should validate that scaling.min_instance_count is less than scaling.max_instance_count #18969

Comments

@minimice
Copy link

minimice commented Aug 2, 2024

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request.
  • Please do not leave +1 or me too comments, they generate extra noise for issue followers and do not help prioritize the request.
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment.
  • If an issue is assigned to a user, that user is claiming responsibility for the issue.
  • Customers working with a Google Technical Account Manager or Customer Engineer can ask them to reach out internally to expedite investigation and resolution of this issue.

Description

With regards to this page

https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/cloud_run_v2_service

For the scaling block, update the documentation to state that max_instance_count is required to be present when min_instance_count is specified and vice versa. Failure to do so would result in the following error (here I did not specify max_instance_count only min_instance_count):

Error 400: Violation in UpdateServiceRequest.service.template.scaling.max_instance_count: must be greater or equal than min_instance_count.
│ Details:
│ [
│   {
│     "@type": "type.googleapis.com/google.rpc.BadRequest",
│     "fieldViolations": [
│       {
│         "description": "must be greater or equal than min_instance_count.",
│         "field": "Violation in UpdateServiceRequest.service.template.scaling.max_instance_count"
│       }
│     ]
│   }
│ ]

New or Affected Resource(s)

  • google_cloud_run_v2_service

Potential Terraform Configuration

References

No response

b/357621141

@github-actions github-actions bot added forward/review In review; remove label to forward service/run labels Aug 5, 2024
@melinath melinath changed the title google_cloud_run_v2_service resource documentation needs to be updated google_cloud_run_v2_service should mark scaling.min_instance_count and scaling.max_instance_count as required_with each other Aug 5, 2024
@melinath
Copy link
Collaborator

melinath commented Aug 5, 2024

From triage: Updated the title to reflect that not only should the docs be updated, we should also mark the fields as required_with each other. (Note: this is not currently documented but functions similarly to exactly_one_of - that is, all fields must be listed.) This shouldn't be a breaking change as long as the API always returns errors for invalid configurations.

@melinath melinath added size/xs and removed forward/review In review; remove label to forward labels Aug 5, 2024
@melinath melinath added this to the Goals milestone Aug 5, 2024
@c2thorn
Copy link
Collaborator

c2thorn commented Aug 9, 2024

https://github.com/GoogleCloudPlatform/magic-modules/blob/main/mmv1/templates/terraform/examples/cloudrunv2_service_sql.tf.erb#L8 is a passing example of max_instance_count without min_instance_count. So maybe this is only true for min_instance_count ?

@charan-kumar-137
Copy link

max_instance_count and min_instance_count are actually NOT required with each other.
The default for max_instance_count is 100 as mentioned in https://cloud.google.com/run/docs/configuring/max-instances#limits.

The above error might be due to setting min_instance_count greater than 100 (default).

@melinath melinath changed the title google_cloud_run_v2_service should mark scaling.min_instance_count and scaling.max_instance_count as required_with each other google_cloud_run_v2_service should validate that scaling.min_instance_count is less than scaling.max_instance_count Aug 12, 2024
@melinath
Copy link
Collaborator

Indeed - the error just says that max_instance_count needs to be greater than min_instance_count. Thanks for the catch!

@charan-kumar-137
Copy link

@melinath, Not just Cloud Run, Other Services where there is Range Arguments (min & max) would need this kind of validation.

Spanner - https://cloud.google.com/spanner/docs/reference/rest/v1/projects.instances#autoscalinglimits
Cloud Functions - https://cloud.google.com/functions/docs/reference/rest/v2/projects.locations.functions#serviceconfig

Since ValidateFunc is supported Only for Primitive Types. is there any way we can add Validation for Nested Objects.

@melinath
Copy link
Collaborator

you can do cross-field validation using a CustomizeDiff function. For MMv1 resources, this can be set up using the custom_diff top-level resource property (which takes a list of function names) and the functions can be defined using custom_code.constants.

@bskaplan
Copy link

Cloud Run has a dynamic default max instances, based on the user's available quota in the region and specified instance size which makes doing accurate checks in the client rather difficult. Is there a downside to leaving this as it is and letting the server validate it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment