From 67dc26aa91e0a3e26bdc0251f875519cc67477a9 Mon Sep 17 00:00:00 2001 From: Modular Magician Date: Mon, 21 Oct 2024 20:20:56 +0000 Subject: [PATCH] Allow proper changing of google_compute_region_instance_group_manager.distribution_policy_target_shape (#12050) [upstream:323586092752ff495a4c69b8476019bd5cd70ee0] Signed-off-by: Modular Magician --- .changelog/12050.txt | 3 + ...e_compute_region_instance_group_manager.go | 39 +++++++--- ...pute_region_instance_group_manager_test.go | 73 +++++++++++++++++-- 3 files changed, 98 insertions(+), 17 deletions(-) create mode 100644 .changelog/12050.txt diff --git a/.changelog/12050.txt b/.changelog/12050.txt new file mode 100644 index 00000000000..1a4406dfc40 --- /dev/null +++ b/.changelog/12050.txt @@ -0,0 +1,3 @@ +```release-note:bug +compute: fixed an issue where immutable `distribution_zones` was incorrectly sent to the API when updating `distribution_policy_target_shape` in `google_compute_region_instance_group_manager` resource +``` \ No newline at end of file diff --git a/google/services/compute/resource_compute_region_instance_group_manager.go b/google/services/compute/resource_compute_region_instance_group_manager.go index 1cb5b6aa48a..461a0779353 100644 --- a/google/services/compute/resource_compute_region_instance_group_manager.go +++ b/google/services/compute/resource_compute_region_instance_group_manager.go @@ -555,7 +555,7 @@ func resourceComputeRegionInstanceGroupManagerCreate(d *schema.ResourceData, met UpdatePolicy: expandRegionUpdatePolicy(d.Get("update_policy").([]interface{})), InstanceLifecyclePolicy: expandInstanceLifecyclePolicy(d.Get("instance_lifecycle_policy").([]interface{})), AllInstancesConfig: expandAllInstancesConfig(nil, d.Get("all_instances_config").([]interface{})), - DistributionPolicy: expandDistributionPolicy(d), + DistributionPolicy: expandDistributionPolicyForCreate(d), StatefulPolicy: expandStatefulPolicy(d), // Force send TargetSize to allow size of 0. ForceSendFields: []string{"TargetSize"}, @@ -815,7 +815,7 @@ func resourceComputeRegionInstanceGroupManagerUpdate(d *schema.ResourceData, met } if d.HasChange("distribution_policy_target_shape") { - updatedManager.DistributionPolicy = expandDistributionPolicy(d) + updatedManager.DistributionPolicy = expandDistributionPolicyForUpdate(d) change = true } @@ -1027,24 +1027,39 @@ func flattenRegionUpdatePolicy(updatePolicy *compute.InstanceGroupManagerUpdateP return results } -func expandDistributionPolicy(d *schema.ResourceData) *compute.DistributionPolicy { +func expandDistributionPolicyForUpdate(d *schema.ResourceData) *compute.DistributionPolicy { + dpts := d.Get("distribution_policy_target_shape").(string) + if dpts == "" { + return nil + } + // distributionPolicy.Zones is NOT updateable. + return &compute.DistributionPolicy{TargetShape: dpts} +} + +func expandDistributionPolicyForCreate(d *schema.ResourceData) *compute.DistributionPolicy { dpz := d.Get("distribution_policy_zones").(*schema.Set) dpts := d.Get("distribution_policy_target_shape").(string) if dpz.Len() == 0 && dpts == "" { return nil } + distributionPolicy := &compute.DistributionPolicy{} - distributionPolicyZoneConfigs := make([]*compute.DistributionPolicyZoneConfiguration, 0, dpz.Len()) - for _, raw := range dpz.List() { - data := raw.(string) - distributionPolicyZoneConfig := compute.DistributionPolicyZoneConfiguration{ - Zone: "zones/" + data, - } + if dpz.Len() > 0 { + distributionPolicyZoneConfigs := make([]*compute.DistributionPolicyZoneConfiguration, 0, dpz.Len()) + for _, raw := range dpz.List() { + data := raw.(string) + distributionPolicyZoneConfig := compute.DistributionPolicyZoneConfiguration{ + Zone: "zones/" + data, + } - distributionPolicyZoneConfigs = append(distributionPolicyZoneConfigs, &distributionPolicyZoneConfig) + distributionPolicyZoneConfigs = append(distributionPolicyZoneConfigs, &distributionPolicyZoneConfig) + } + distributionPolicy.Zones = distributionPolicyZoneConfigs } - - return &compute.DistributionPolicy{Zones: distributionPolicyZoneConfigs, TargetShape: dpts} + if dpts != "" { + distributionPolicy.TargetShape = dpts + } + return distributionPolicy } func flattenDistributionPolicy(distributionPolicy *compute.DistributionPolicy) []string { diff --git a/google/services/compute/resource_compute_region_instance_group_manager_test.go b/google/services/compute/resource_compute_region_instance_group_manager_test.go index 84bc425b6d4..a0a7121f6c7 100644 --- a/google/services/compute/resource_compute_region_instance_group_manager_test.go +++ b/google/services/compute/resource_compute_region_instance_group_manager_test.go @@ -311,7 +311,16 @@ func TestAccRegionInstanceGroupManager_distributionPolicy(t *testing.T) { CheckDestroy: testAccCheckRegionInstanceGroupManagerDestroyProducer(t), Steps: []resource.TestStep{ { - Config: testAccRegionInstanceGroupManager_distributionPolicy(template, igm, zones), + Config: testAccRegionInstanceGroupManager_distributionPolicyEmpty(template, igm), + }, + { + ResourceName: "google_compute_region_instance_group_manager.igm-basic", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"status"}, + }, + { + Config: testAccRegionInstanceGroupManager_distributionPolicy(template, igm), }, { ResourceName: "google_compute_region_instance_group_manager.igm-basic", @@ -328,6 +337,15 @@ func TestAccRegionInstanceGroupManager_distributionPolicy(t *testing.T) { ImportStateVerify: true, ImportStateVerifyIgnore: []string{"status"}, }, + { + Config: testAccRegionInstanceGroupManager_distributionPolicyEmpty(template, igm), + }, + { + ResourceName: "google_compute_region_instance_group_manager.igm-basic", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"status"}, + }, }, }) } @@ -1069,7 +1087,7 @@ resource "google_compute_region_instance_group_manager" "igm-basic" { `, primaryTemplate, canaryTemplate, igm) } -func testAccRegionInstanceGroupManager_distributionPolicy(template, igm string, zones []string) string { +func testAccRegionInstanceGroupManager_distributionPolicyEmpty(template, igm string) string { return fmt.Sprintf(` data "google_compute_image" "my_image" { family = "debian-11" @@ -1103,8 +1121,6 @@ resource "google_compute_region_instance_group_manager" "igm-basic" { base_instance_name = "tf-test-igm-basic" region = "us-central1" target_size = 2 - distribution_policy_zones = ["%s"] - distribution_policy_target_shape = "ANY" update_policy { instance_redistribution_type = "NONE" @@ -1114,7 +1130,54 @@ resource "google_compute_region_instance_group_manager" "igm-basic" { max_unavailable_fixed = 6 } } -`, template, igm, strings.Join(zones, "\",\"")) +`, template, igm) +} + +func testAccRegionInstanceGroupManager_distributionPolicy(template, igm string) string { + return fmt.Sprintf(` +data "google_compute_image" "my_image" { + family = "debian-11" + project = "debian-cloud" +} + +resource "google_compute_instance_template" "igm-basic" { + name = "%s" + machine_type = "e2-medium" + can_ip_forward = false + tags = ["foo", "bar"] + disk { + source_image = data.google_compute_image.my_image.self_link + auto_delete = true + boot = true + } + network_interface { + network = "default" + } +} + +resource "google_compute_region_instance_group_manager" "igm-basic" { + description = "Terraform test instance group manager" + name = "%s" + + version { + instance_template = google_compute_instance_template.igm-basic.self_link + name = "primary" + } + + base_instance_name = "tf-test-igm-basic" + region = "us-central1" + target_size = 2 + distribution_policy_target_shape = "BALANCED" + + update_policy { + instance_redistribution_type = "NONE" + type = "OPPORTUNISTIC" + minimal_action = "REPLACE" + max_surge_fixed = 0 + max_unavailable_fixed = 6 + } +} +`, template, igm) } func testAccRegionInstanceGroupManager_distributionPolicyUpdate(template, igm string, zones []string) string {