Add maxUtilization field to hash function of BackendService (#3938) (#2620)

Co-authored-by: Dana Hoffman <[email protected]>
Signed-off-by: Modular Magician <[email protected]>

Co-authored-by: Dana Hoffman <[email protected]>

Author: modular-magician

.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
+```

google-beta/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.
@@ -3312,17 +3337,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
 		}

google-beta/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))
@@ -929,6 +967,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(`