Fix Spanner update when adding/removing autoscaling config (#9814) (#17033)

* Add reproducer test for error.

Creating a Spanner instance without auto-scaling and then updating it to
add auto-scaling should work, but currently doesn't.

* Fix `fieldMask` for spanner updates when enabling/disable autoscaling.

When adding/removing auto-scaling config wholesale, the API expects only
the top-level `autoscalingConfig` key to be listed in the `fieldMask`.
Listing sub-fields of the autoscaling config causes an error.

* Add tests for dropping auto-scaling.

Not sure if this was broken before, but makes sense to me that we should
test in both directions.

* Apply review feedback

* Fix indentation in template
* Rename test instances to match expected pattern
* Consolidate tests

* Fix name-too-long errors
[upstream:8f488115df8493669aae88a65b80a2fcb31d7557]

Signed-off-by: Modular Magician <[email protected]>

Author: modular-magician

.changelog/9814.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+spanner: fixed error when adding `autoscaling_config` to an existing `google_spanner_instance`
+```

google/services/spanner/resource_spanner_instance.go

@@ -1153,23 +1153,32 @@ func resourceSpannerInstanceUpdateEncoder(d *schema.ResourceData, meta interface
 	if d.HasChange("processing_units") {
 		updateMask = append(updateMask, "processingUnits")
 	}
-	if d.HasChange("autoscaling_config.0.autoscaling_limits.0.max_processing_units") {
-		updateMask = append(updateMask, "autoscalingConfig.autoscalingLimits.maxProcessingUnits")
-	}
-	if d.HasChange("autoscaling_config.0.autoscaling_limits.0.min_processing_units") {
-		updateMask = append(updateMask, "autoscalingConfig.autoscalingLimits.minProcessingUnits")
-	}
-	if d.HasChange("autoscaling_config.0.autoscaling_limits.0.max_nodes") {
-		updateMask = append(updateMask, "autoscalingConfig.autoscalingLimits.maxNodes")
-	}
-	if d.HasChange("autoscaling_config.0.autoscaling_limits.0.min_nodes") {
-		updateMask = append(updateMask, "autoscalingConfig.autoscalingLimits.minNodes")
-	}
-	if d.HasChange("autoscaling_config.0.autoscaling_targets.0.high_priority_cpu_utilization_percent") {
-		updateMask = append(updateMask, "autoscalingConfig.autoscalingTargets.highPriorityCpuUtilizationPercent")
-	}
-	if d.HasChange("autoscaling_config.0.autoscaling_targets.0.storage_utilization_percent") {
-		updateMask = append(updateMask, "autoscalingConfig.autoscalingTargets.storageUtilizationPercent")
+	if d.HasChange("autoscaling_config") {
+		old, new := d.GetChange("autoscaling_config")
+		oldSlice := old.([]interface{})
+		newSlice := new.([]interface{})
+		if len(oldSlice) == 0 || len(newSlice) == 0 {
+			updateMask = append(updateMask, "autoscalingConfig")
+		} else {
+			if d.HasChange("autoscaling_config.0.autoscaling_limits.0.max_processing_units") {
+				updateMask = append(updateMask, "autoscalingConfig.autoscalingLimits.maxProcessingUnits")
+			}
+			if d.HasChange("autoscaling_config.0.autoscaling_limits.0.min_processing_units") {
+				updateMask = append(updateMask, "autoscalingConfig.autoscalingLimits.minProcessingUnits")
+			}
+			if d.HasChange("autoscaling_config.0.autoscaling_limits.0.max_nodes") {
+				updateMask = append(updateMask, "autoscalingConfig.autoscalingLimits.maxNodes")
+			}
+			if d.HasChange("autoscaling_config.0.autoscaling_limits.0.min_nodes") {
+				updateMask = append(updateMask, "autoscalingConfig.autoscalingLimits.minNodes")
+			}
+			if d.HasChange("autoscaling_config.0.autoscaling_targets.0.high_priority_cpu_utilization_percent") {
+				updateMask = append(updateMask, "autoscalingConfig.autoscalingTargets.highPriorityCpuUtilizationPercent")
+			}
+			if d.HasChange("autoscaling_config.0.autoscaling_targets.0.storage_utilization_percent") {
+				updateMask = append(updateMask, "autoscalingConfig.autoscalingTargets.storageUtilizationPercent")
+			}
+		}
 	}
 	newObj["fieldMask"] = strings.Join(updateMask, ",")
 	return newObj, nil

google/services/spanner/resource_spanner_instance_test.go

@@ -171,12 +171,24 @@ func TestAccSpannerInstance_basicWithAutoscalingUsingProcessingUnitConfig(t *tes
 func TestAccSpannerInstance_basicWithAutoscalingUsingProcessingUnitConfigUpdate(t *testing.T) {
 	t.Parallel()
 
-	displayName := fmt.Sprintf("spanner-test-%s-dname", acctest.RandString(t, 10))
+	displayName := fmt.Sprintf("tf-test-%s", acctest.RandString(t, 10))
 	acctest.VcrTest(t, resource.TestCase{
 		PreCheck:                 func() { acctest.AccTestPreCheck(t) },
 		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
 		CheckDestroy:             testAccCheckSpannerInstanceDestroyProducer(t),
 		Steps: []resource.TestStep{
+			{
+				Config: testAccSpannerInstance_basic(displayName),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttrSet("google_spanner_instance.basic", "state"),
+				),
+			},
+			{
+				ResourceName:            "google_spanner_instance.basic",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"labels", "terraform_labels"},
+			},
 			{
 				Config: testAccSpannerInstance_basicWithAutoscalerConfigUsingProcessingUnitsAsConfigsUpdate(displayName, 1000, 2000, 65, 95),
 				Check: resource.ComposeTestCheckFunc(
@@ -201,6 +213,18 @@ func TestAccSpannerInstance_basicWithAutoscalingUsingProcessingUnitConfigUpdate(
 				ImportStateVerify:       true,
 				ImportStateVerifyIgnore: []string{"labels", "terraform_labels"},
 			},
+			{
+				Config: testAccSpannerInstance_basic(displayName),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttrSet("google_spanner_instance.basic", "state"),
+				),
+			},
+			{
+				ResourceName:            "google_spanner_instance.basic",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"labels", "terraform_labels"},
+			},
 		},
 	})
 }
@@ -232,12 +256,24 @@ func TestAccSpannerInstance_basicWithAutoscalingUsingNodeConfig(t *testing.T) {
 func TestAccSpannerInstance_basicWithAutoscalingUsingNodeConfigUpdate(t *testing.T) {
 	t.Parallel()
 
-	displayName := fmt.Sprintf("spanner-test-%s-dname", acctest.RandString(t, 10))
+	displayName := fmt.Sprintf("tf-test-%s", acctest.RandString(t, 10))
 	acctest.VcrTest(t, resource.TestCase{
 		PreCheck:                 func() { acctest.AccTestPreCheck(t) },
 		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
 		CheckDestroy:             testAccCheckSpannerInstanceDestroyProducer(t),
 		Steps: []resource.TestStep{
+			{
+				Config: testAccSpannerInstance_basic(displayName),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttrSet("google_spanner_instance.basic", "state"),
+				),
+			},
+			{
+				ResourceName:            "google_spanner_instance.basic",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"labels", "terraform_labels"},
+			},
 			{
 				Config: testAccSpannerInstance_basicWithAutoscalerConfigUsingNodesAsConfigsUpdate(displayName, 1, 2, 65, 95),
 				Check: resource.ComposeTestCheckFunc(
@@ -262,6 +298,18 @@ func TestAccSpannerInstance_basicWithAutoscalingUsingNodeConfigUpdate(t *testing
 				ImportStateVerify:       true,
 				ImportStateVerifyIgnore: []string{"labels", "terraform_labels"},
 			},
+			{
+				Config: testAccSpannerInstance_basic(displayName),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttrSet("google_spanner_instance.basic", "state"),
+				),
+			},
+			{
+				ResourceName:            "google_spanner_instance.basic",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"labels", "terraform_labels"},
+			},
 		},
 	})
 }