From 3a8e1464590c3a89a915734b1c5ac2aa54f66c2f Mon Sep 17 00:00:00 2001 From: The Magician Date: Mon, 19 Oct 2020 14:13:12 -0700 Subject: [PATCH] Add maxUtilization field to hash function of BackendService (#3938) (#7575) Co-authored-by: Dana Hoffman Signed-off-by: Modular Magician Co-authored-by: Dana Hoffman --- .changelog/3938.txt | 3 + google/resource_compute_backend_service.go | 37 +++++-- .../resource_compute_backend_service_test.go | 96 +++++++++++++++++++ 3 files changed, 126 insertions(+), 10 deletions(-) create mode 100644 .changelog/3938.txt diff --git a/.changelog/3938.txt b/.changelog/3938.txt new file mode 100644 index 00000000000..5f34a774ef0 --- /dev/null +++ b/.changelog/3938.txt @@ -0,0 +1,3 @@ +```release-note:bug +compute: fixed issue where `google_compute_[region_]backend_service.backend.max_utilization` could not be updated +``` diff --git a/google/resource_compute_backend_service.go b/google/resource_compute_backend_service.go index eef67f00f3f..50c49320f7e 100644 --- a/google/resource_compute_backend_service.go +++ b/google/resource_compute_backend_service.go @@ -30,6 +30,21 @@ import ( "google.golang.org/api/googleapi" ) +// Whether the backend is a global or regional NEG +func isNegBackend(backend map[string]interface{}) bool { + backendGroup, ok := backend["group"] + if !ok { + return false + } + + match, err := regexp.MatchString("(?:global|regions/[^/]+)/networkEndpointGroups", backendGroup.(string)) + if err != nil { + // should not happen as long as the regexp pattern compiled correctly + return false + } + return match +} + func resourceGoogleComputeBackendServiceBackendHash(v interface{}) int { if v == nil { return 0 @@ -129,6 +144,16 @@ func resourceGoogleComputeBackendServiceBackendHash(v interface{}) int { // the hash function doesn't return something else. buf.WriteString(fmt.Sprintf("%f-", v.(float64))) } + if v, ok := m["max_utilization"]; ok && !isNegBackend(m) { + if v == nil { + v = 0.0 + } + + // floats can't be added to the hash with %v as the other values are because + // %v and %f are not equivalent strings so this must remain as a float so that + // the hash function doesn't return something else. + buf.WriteString(fmt.Sprintf("%f-", v.(float64))) + } // This is in region backend service, but not in backend service. Should be a no-op // if it's not present. @@ -3195,17 +3220,9 @@ func resourceComputeBackendServiceEncoder(d *schema.ResourceData, meta interface backends := backendsRaw.([]interface{}) for _, backendRaw := range backends { backend := backendRaw.(map[string]interface{}) - backendGroup, ok := backend["group"] - if !ok { - continue - } - match, err := regexp.MatchString("(?:global|regions/[^/]+)/networkEndpointGroups", backendGroup.(string)) - if err != nil { - return nil, err - } - if match { - // Remove `max_utilization` from any backend that belongs to a serverless NEG. This field + if isNegBackend(backend) { + // Remove `max_utilization` from any backend that belongs to an NEG. This field // has a default value and causes API validation errors backend["maxUtilization"] = nil } diff --git a/google/resource_compute_backend_service_test.go b/google/resource_compute_backend_service_test.go index 22cee136618..2716c660184 100644 --- a/google/resource_compute_backend_service_test.go +++ b/google/resource_compute_backend_service_test.go @@ -74,6 +74,44 @@ func TestAccComputeBackendService_withBackend(t *testing.T) { }) } +func TestAccComputeBackendService_withBackendAndMaxUtilization(t *testing.T) { + serviceName := fmt.Sprintf("tf-test-%s", randString(t, 10)) + igName := fmt.Sprintf("tf-test-%s", randString(t, 10)) + itName := fmt.Sprintf("tf-test-%s", randString(t, 10)) + checkName := fmt.Sprintf("tf-test-%s", randString(t, 10)) + vcrTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckComputeBackendServiceDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: testAccComputeBackendService_withBackend( + serviceName, igName, itName, checkName, 10), + }, + { + ResourceName: "google_compute_backend_service.lipsum", + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccComputeBackendService_withBackendAndMaxUtilization( + serviceName, igName, itName, checkName, 10), + PlanOnly: true, + ExpectNonEmptyPlan: true, + }, + { + Config: testAccComputeBackendService_withBackendAndMaxUtilization( + serviceName, igName, itName, checkName, 10), + }, + { + ResourceName: "google_compute_backend_service.lipsum", + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + func TestAccComputeBackendService_withBackendAndIAP(t *testing.T) { serviceName := fmt.Sprintf("tf-test-%s", randString(t, 10)) igName := fmt.Sprintf("tf-test-%s", randString(t, 10)) @@ -786,6 +824,64 @@ resource "google_compute_http_health_check" "default" { `, serviceName, timeout, igName, itName, checkName) } +func testAccComputeBackendService_withBackendAndMaxUtilization( + serviceName, igName, itName, checkName string, timeout int64) string { + return fmt.Sprintf(` +data "google_compute_image" "my_image" { + family = "debian-9" + project = "debian-cloud" +} + +resource "google_compute_backend_service" "lipsum" { + name = "%s" + description = "Hello World 1234" + port_name = "http" + protocol = "HTTP" + timeout_sec = %v + + backend { + group = google_compute_instance_group_manager.foobar.instance_group + max_utilization = 1.0 + } + + health_checks = [google_compute_http_health_check.default.self_link] +} + +resource "google_compute_instance_group_manager" "foobar" { + name = "%s" + version { + instance_template = google_compute_instance_template.foobar.self_link + name = "primary" + } + base_instance_name = "foobar" + zone = "us-central1-f" + target_size = 1 +} + +resource "google_compute_instance_template" "foobar" { + name = "%s" + machine_type = "e2-medium" + + network_interface { + network = "default" + } + + disk { + source_image = data.google_compute_image.my_image.self_link + auto_delete = true + boot = true + } +} + +resource "google_compute_http_health_check" "default" { + name = "%s" + request_path = "/" + check_interval_sec = 1 + timeout_sec = 1 +} +`, serviceName, timeout, igName, itName, checkName) +} + func testAccComputeBackendService_withBackendAndIAP( serviceName, igName, itName, checkName string, timeout int64) string { return fmt.Sprintf(`