cloudrunv2: Fixed permadiff in template.scaling.min_instance_count#14971
cloudrunv2: Fixed permadiff in template.scaling.min_instance_count#14971wyardley wants to merge 1 commit into
template.scaling.min_instance_count#14971Conversation
Added `default_from_api` to `template.scaling.max_instance_count`, to resolve a permadiff if `min_instance_count` is set to the default (0) without a value set for `max_instance_count` Part of hashicorp/terraform-provider-google#20399
modular-magician
added
the
awaiting-approval
|
Hello! I am a robot. Tests will require approval from a repository maintainer to run. Googlers: For automatic test runs see go/terraform-auto-test-runs. @rileykarson, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look. You can help make sure that review is quick by doing a self-review and by running impacted tests locally. |
|
@rileykarson This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 1 week. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 2 weeks. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 3 weeks. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 4 weeks. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 5 weeks. Please take a look! Use the label |
modular-magician
removed
the
awaiting-approval
rileykarson
left a comment
rileykarson
left a comment
There was a problem hiding this comment.
Sorry, this is very stale due to my missing it.
What conditions will a value get returned by Cloud Run? I think to solve this we probably need to work through the rules for that relatively exhaustively.
The issue with default_from_api is that it makes values sticky and revert to zero / revert to null can become much more difficult. This is a problem with autoscalers/autoscaling in particular when you may want to switch between setting either end of the bounds, or scale on a different dimension.
|
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.
Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_cloud_run_v2_service" "primary" {
template {
scaling {
max_instance_count = # value needed
}
}
}
|
Tests analyticsTotal tests: 85 Click here to see the affected service packages
🟢 All tests passed! View the build log |
My brain is a little fuzzy on all this ~ 5 weeks later, but did you read this comment in the linked issue? This hopefully explains what I think is happening. For example, see comments inline here: resource "google_cloud_run_v2_service" "default" {
# [...]
template {
scaling {
min_instance_count = 0 // This is the default, but max_instance_count isn't also set
}
containers {
image = "us-docker.pkg.dev/cloudrun/container/hello"
resources {
limits = {
memory = "1Gi" // CPU not set; presumably you'd run into the same issue if CPU were set but memory wasn't
}
}
}
}
}From my limited testing, if all defaults are taken (i.e., I was going off the docs on permadiffs, and the settings of some existing parameters, and in my limited testing, the fix here seemed to work, but agree that it may be worth doing some additional testing to confirm. Baking in the default values would probably also work, so will defer to you in terms of what's the better behavior (or if there's another way to "fix" this). I wasn't able to come up with something easy for the |
|
@rileykarson This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 1 week. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 2 weeks. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 3 weeks. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 4 weeks. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 5 weeks. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 6 weeks. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 7 weeks. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 8 weeks. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 9 weeks. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 10 weeks. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 11 weeks. Please take a look! Use the label |
4 participants
Release Note Template for Downstream PRs (will be copied)
Added
default_from_apitotemplate.scaling.max_instance_count, to resolve a permadiff ifmin_instance_countis set to the default (0) without a value set formax_instance_countPart of hashicorp/terraform-provider-google#20399
See linked ticket: currently, this does not resolve the diff when one of CPU or memory is defined in limits but the other is unset.