Add maxUtilization field to hash function of BackendService

[hyww]

fixes hashicorp/terraform-provider-google#7169

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)

compute: fixed issue where `google_compute_[region_]backend_service.backend.max_utilization` could not be updated

[hyww]

I am not sure if removing default value is going to be a breaking change. It seems that GCP behaves like maxUtilization 0.8 when no maxUtilization is specified in UTILIZATION balancing mode.

[modular-magician]

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

I have detected that you are a community contributor, so your PR will be assigned to someone with a commit-bit on this repo for initial review.

Thanks for your contribution! A human will be with you soon.

@danawillow, please review this PR or find an appropriate assignee.

[modular-magician]

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 2 files changed, 13 insertions(+), 6 deletions(-))
Terraform Beta: Diff ( 2 files changed, 13 insertions(+), 6 deletions(-))
Ansible: Diff ( 2 files changed, 4 insertions(+), 5 deletions(-))
TF Conversion: Diff ( 1 file changed, 11 insertions(+), 1 deletion(-))
Inspec: Diff ( 1 file changed, 1 insertion(+), 1 deletion(-))

[modular-magician]

Oops! It looks like you're using an unknown release-note type in your changelog entries:

• REPLACEME

Please only use the types listed in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/.ci/RELEASE_NOTES_GUIDE.md.

[danawillow]

I'm not totally sure I understand the purpose of this change. It looks like you don't believe the default for this field should be 0.8, but should instead be 0. Why is that? My impression is that 0.8 is the server-side default, so we set it as the default in the provider so that it shows up in terraform plan output. At the moment, setting the field to 0 will allow you to set the value to 0 if that's what you're looking for.

[hyww]

When I create a backend service with balancing mode RATE from web console, I get a backend service with no max_utilization field instead of one with value 0.8. I have no idea if this affects actual behavior, but maybe we should make it behaves like official client?

This is an optional parameter if the backend's balancingMode is UTILIZATION.
https://cloud.google.com/compute/docs/reference/rest/v1/backendServices

p.s. the provider doesn't set default max_utilization value for region backend service currently.

p.p.s. We ran into some backend utilization web UI bug recently and I doubt that it might affect backend service in RATE mode if max_utilization is set.

[hyww]

Another part of this PR is fixing the hash function. I can separate it into another PR if you think that would be better.

[hyww]

Hi @danawillow , I removed the commit that removed default value of maxUtilization. Could you review this PR again? Thanks!

[modular-magician]

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 1 file changed, 10 insertions(+))
Terraform Beta: Diff ( 1 file changed, 10 insertions(+))
TF Conversion: Diff ( 1 file changed, 10 insertions(+))

[modular-magician]

Oops! It looks like you're using an unknown release-note type in your changelog entries:

• REPLACEME

Please only use the types listed in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/.ci/RELEASE_NOTES_GUIDE.md.

[danawillow]

/gcbrun

[modular-magician]

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=152331"

[danawillow]

This looks good so far! I just triggered our test pipeline to make sure it passes our existing tests. Can you also modify an existing test (or add a new one) that tests updating this field, since that's what this PR fixes?

[modular-magician]

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 1 file changed, 10 insertions(+))
Terraform Beta: Diff ( 1 file changed, 10 insertions(+))
TF Conversion: Diff ( 1 file changed, 10 insertions(+))

[modular-magician]

I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccProviderMeta_setModuleName|TestAccComputeBackendService_backendServiceNetworkEndpointExample|TestAccComputeBackendService_regionNegBackend You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=152336"

[danawillow]

Looks like this failed an existing test:

------- Stdout: -------
=== RUN   TestAccComputeBackendService_regionNegBackend
=== PAUSE TestAccComputeBackendService_regionNegBackend
=== CONT  TestAccComputeBackendService_regionNegBackend
TestAccComputeBackendService_regionNegBackend: provider_test.go:266: Step 1/2 error: After applying this test step and performing a `terraform refresh`, the plan was not empty.
stdout
An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
~ update in-place
Terraform will perform the following actions:
# google_compute_backend_service.backend will be updated in-place
~ resource "google_compute_backend_service" "backend" {
affinity_cookie_ttl_sec         = 0
connection_draining_timeout_sec = 10
creation_timestamp              = "2020-10-15T11:35:06.268-07:00"
custom_request_headers          = []
enable_cdn                      = true
fingerprint                     = "rhPzSjFnj9A="
health_checks                   = []
id                              = "projects/ci-test-project-188019/global/backendServices/tf-test-backendj3jhdqnlxp"
load_balancing_scheme           = "EXTERNAL"
name                            = "tf-test-backendj3jhdqnlxp"
port_name                       = "http"
project                         = "ci-test-project-188019"
protocol                        = "HTTP"
self_link                       = "https://www.googleapis.com/compute/v1/projects/ci-test-project-188019/global/backendServices/tf-test-backendj3jhdqnlxp
session_affinity                = "NONE"
timeout_sec                     = 30
- backend {
- balancing_mode               = "UTILIZATION" -> null
- capacity_scaler              = 1 -> null
- group                        = "https://www.googleapis.com/compute/v1/projects/ci-test-project-188019/regions/us-central1/networkEndpointGroups/tf-test-negj3jhdqnlxp" -> null
- max_connections              = 0 -> null
- max_connections_per_endpoint = 0 -> null
- max_connections_per_instance = 0 -> null
- max_rate                     = 0 -> null
- max_rate_per_endpoint        = 0 -> null
- max_rate_per_instance        = 0 -> null
- max_utilization              = 0 -> null
}
+ backend {
+ balancing_mode  = "UTILIZATION"
+ capacity_scaler = 1
+ group           = "projects/ci-test-project-188019/regions/us-central1/networkEndpointGroups/tf-test-negj3jhdqnlxp"
+ max_utilization = 0.8
}
cdn_policy {
signed_url_cache_max_age_sec = 0
}
}
Plan: 0 to add, 1 to change, 0 to destroy.
--- FAIL: TestAccComputeBackendService_regionNegBackend (60.73s)
FAIL

Let me know if you want some help debugging the failure!

[hyww]

I have added a new test that tests updating max_utilization field.

The failed test seems to have something to do with this change.

According to @slevenick 's comment here, max_utilzation of NEGs seems to be kept in plan intentionally. I am not sure if I should fix the test (by adding ExpectNonEmptyPlan to the test step?) in this PR or maybe we need another solution of the max_utilzation of NEGs problem.

[modular-magician]

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 2 files changed, 106 insertions(+))
Terraform Beta: Diff ( 2 files changed, 106 insertions(+))
TF Conversion: Diff ( 1 file changed, 10 insertions(+))

[danawillow]

I see. So it sounds like the value is kept in the plan, but removed from the request if it's a NEG. We definitely don't want ExpectNonEmptyPlan in the test, since that means that any user that tries to run a config like the one in the test will experience a permadiff. Instead, I think we need to try to have the hash function set the value only if it isn't a NEG.

[hyww]

Fixed. Add max_utilization to hash only if isn't a NEG.

[modular-magician]

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 2 files changed, 126 insertions(+), 10 deletions(-))
Terraform Beta: Diff ( 2 files changed, 126 insertions(+), 10 deletions(-))
TF Conversion: Diff ( 1 file changed, 30 insertions(+), 10 deletions(-))

[danawillow]

/gcbrun

[modular-magician]

I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=152971"

[modular-magician]

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 2 files changed, 123 insertions(+), 10 deletions(-))
Terraform Beta: Diff ( 2 files changed, 123 insertions(+), 10 deletions(-))
TF Conversion: Diff ( 1 file changed, 27 insertions(+), 10 deletions(-))

[modular-magician]

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 2 files changed, 123 insertions(+), 10 deletions(-))
Terraform Beta: Diff ( 2 files changed, 123 insertions(+), 10 deletions(-))
TF Conversion: Diff ( 1 file changed, 27 insertions(+), 10 deletions(-))

[modular-magician]

I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccDataSourceSpannerInstance_basic|TestAccComputeBackendService_withBackendAndMaxUtilization You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=152977"