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

Add conditional validation, allow sending empty capacity scaler for regional backend service #9

Conversation

modular-magician
Copy link
Collaborator

Fixes hashicorp/terraform-provider-google#5449

compute: Added conditional requirement of `google_compute_**region**_backend_service` `backend.capacity_scaler` to no longer accept the API default if not INTERNAL. Non-INTERNAL backend services must now specify `capacity_scaler` explicitly and have a total capacity greater than 0. In addition, API default of 1.0 must now be explicitly set and will be treated as nil or zero if not set in config.
compute: Fixed `google_compute_**region**_backend_service` so it no longer has a permadiff if `backend.capacity_scaler` is unset in config by requiring capacity scaler. 
compute: Fixed `backend.capacity_scaler` to actually set zero (0.0) value.

This PR includes a breaking change/bug fix and weird validation because RegionBackendService has a conditional default, and schema.Set cannot tell the difference between nil and zero values for scalars like float/int.

Changes:

  • Adds ability to send empty/zero capacity scaler --> forces capacity scaler to be required when settable (i.e. non-INTERNAL). (added validation)
  • Adds an encoder to throw out fields that can't be sent to the API for INTERNAL requests and validation to make sure INTERNAL backends don't have these values set explicitly

Context

  1. RegionBackendService has typically only allowed for Internal Load Balancing (load_balancing_scheme defaults to INTERNAL), and the API rejects INTERNAL backend requests with specific fields in backend set like capacity_scaler, max_rate, etc

  2. A new INTERNAL_MANAGED lb_scheme allows users to specify fields that were previously managed (unsettable). If not sent in the request, the API returns 1.0 for the value of capacity_scaler. NOTE: This is technically also true for BackendService, but as BackendService cannot be INTERNAL, it will always return 1.0 in the case of nil capacity_scaler so we slapped a default value on and it works fine.

  3. Then, for INTERNAL_MANAGED region backend services: If capacity_scaler is not set in config, the API returns 1.0 --> schema.Set cannot tell capacity_scaler = 0.0 and not-set-capacity-scaler apart --> a diff is detected because of new hash-value of set object, which we can't suppress. d,

We also need to be able to send an empty value if non-INTERNAL, but we can't send the empty value if the RegionBackendService is INTERNAL (default)

FUTURE WORK

Right now, BackendService/RegionBackendService has unfixable behavior in that there is no way to send zero values for the other fields either BackendService or RegionBackendService backend. On read, schema.Set will zero any unset fields out, and they in turn will get sent to the API on next apply. We can force capacity_scaler to be set just because it's a requirement (hence default 1.0), but the other fields cannot be required because they can't all be set at once on the object (max_rate, max_rate_per_instance, etc have conflicts)

Derived from GoogleCloudPlatform/magic-modules#3033

…egional backend service (#3033)

* fix region backend service capacity scaler

* add capacity scaler to fr example

* remove recopied handwritten test

* move constant list into expanders for tpg conversion

* change licenses

Signed-off-by: Modular Magician <[email protected]>
@modular-magician modular-magician merged commit c112d4e into terraform-google-modules:master Jan 31, 2020
@modular-magician modular-magician deleted the downstream-pr-e47e9a50b087c3b4677487073e295d90c9f14a84 branch November 18, 2024 05:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix Tests using RegionBackendService
1 participant