From 66cc613884a9985b95de6511287511b687fd8e8d Mon Sep 17 00:00:00 2001 From: Sarah French <15078782+SarahFrench@users.noreply.github.com> Date: Fri, 5 Jan 2024 11:07:15 +0000 Subject: [PATCH 1/2] Revert "Revert "Fix permadiff that reorders `stateful_external_ip` blocks on `google_compute_instance_group_manager` and `google_compute_region_instance_group_manager` resources" (#9754)" This reverts commit 24ff1562fb29e5492f055105c3510580b7a387a1. --- ...urce_compute_instance_group_manager.go.erb | 70 ++-- ...te_instance_group_manager_internal_test.go | 78 ----- ...nstance_group_manager_internal_test.go.erb | 298 ++++++++++++++++++ ...compute_instance_group_manager_test.go.erb | 2 +- ...mpute_region_instance_group_manager.go.erb | 4 +- ..._region_instance_group_manager_test.go.erb | 110 ++++++- 6 files changed, 460 insertions(+), 102 deletions(-) delete mode 100644 mmv1/third_party/terraform/services/compute/resource_compute_instance_group_manager_internal_test.go create mode 100644 mmv1/third_party/terraform/services/compute/resource_compute_instance_group_manager_internal_test.go.erb diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_instance_group_manager.go.erb b/mmv1/third_party/terraform/services/compute/resource_compute_instance_group_manager.go.erb index 724bc0ce8fd1..a3f09e7e4af1 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_instance_group_manager.go.erb +++ b/mmv1/third_party/terraform/services/compute/resource_compute_instance_group_manager.go.erb @@ -778,10 +778,10 @@ func resourceComputeInstanceGroupManagerRead(d *schema.ResourceData, meta interf if err = d.Set("stateful_disk", flattenStatefulPolicy(manager.StatefulPolicy)); err != nil { return fmt.Errorf("Error setting stateful_disk in state: %s", err.Error()) } - if err = d.Set("stateful_internal_ip", flattenStatefulPolicyStatefulInternalIps(manager.StatefulPolicy)); err != nil { + if err = d.Set("stateful_internal_ip", flattenStatefulPolicyStatefulInternalIps(d, manager.StatefulPolicy)); err != nil { return fmt.Errorf("Error setting stateful_internal_ip in state: %s", err.Error()) } - if err = d.Set("stateful_external_ip", flattenStatefulPolicyStatefulExternalIps(manager.StatefulPolicy)); err != nil { + if err = d.Set("stateful_external_ip", flattenStatefulPolicyStatefulExternalIps(d, manager.StatefulPolicy)); err != nil { return fmt.Errorf("Error setting stateful_external_ip in state: %s", err.Error()) } if err := d.Set("fingerprint", manager.Fingerprint); err != nil { @@ -1306,36 +1306,68 @@ func flattenStatefulPolicy(statefulPolicy *compute.StatefulPolicy) []map[string] return result } -func flattenStatefulPolicyStatefulInternalIps(statefulPolicy *compute.StatefulPolicy) []map[string]interface{} { +func flattenStatefulPolicyStatefulInternalIps(d *schema.ResourceData, statefulPolicy *compute.StatefulPolicy) []map[string]interface{} { if statefulPolicy == nil || statefulPolicy.PreservedState == nil || statefulPolicy.PreservedState.InternalIPs == nil { return make([]map[string]interface{}, 0, 0) } - result := make([]map[string]interface{}, 0, len(statefulPolicy.PreservedState.InternalIPs)) - for interfaceName, internalIp := range statefulPolicy.PreservedState.InternalIPs { - data := map[string]interface{}{ - "interface_name": interfaceName, - "delete_rule": internalIp.AutoDelete, - } - result = append(result, data) - } - return result + return flattenStatefulPolicyStatefulIps(d, "stateful_internal_ip", statefulPolicy.PreservedState.InternalIPs) } -func flattenStatefulPolicyStatefulExternalIps(statefulPolicy *compute.StatefulPolicy) []map[string]interface{} { +func flattenStatefulPolicyStatefulExternalIps(d *schema.ResourceData, statefulPolicy *compute.StatefulPolicy) []map[string]interface{} { if statefulPolicy == nil || statefulPolicy.PreservedState == nil || statefulPolicy.PreservedState.ExternalIPs == nil { - return make([]map[string]interface{}, 0, 0) + return make([]map[string]interface{}, 0) } - result := make([]map[string]interface{}, 0, len(statefulPolicy.PreservedState.ExternalIPs)) - for interfaceName, externalIp := range statefulPolicy.PreservedState.ExternalIPs { + + return flattenStatefulPolicyStatefulIps(d, "stateful_external_ip", statefulPolicy.PreservedState.ExternalIPs) +} + +func flattenStatefulPolicyStatefulIps(d *schema.ResourceData, ipfieldName string, ips map[string]compute.StatefulPolicyPreservedStateNetworkIp) []map[string]interface{} { + + // statefulPolicy.PreservedState.ExternalIPs and statefulPolicy.PreservedState.InternalIPs are affected by API-side reordering + // of external/internal IPs, where ordering is done by the interface_name value. + // Below we intend to reorder the IPs to match the order in the config. + // Also, data is converted from a map (client library's statefulPolicy.PreservedState.ExternalIPs, or .InternalIPs) to a slice (stored in state). + // Any IPs found from the API response that aren't in the config are appended to the end of the slice. + + configIpOrder := d.Get(ipfieldName).([]interface{}) + order := map[string]int{} // record map of interface name to index + for i, el := range configIpOrder { + ip := el.(map[string]interface{}) + interfaceName := ip["interface_name"].(string) + order[interfaceName] = i + } + + orderedResult := make([]map[string]interface{}, len(configIpOrder)) + unexpectedIps := []map[string]interface{}{} + for interfaceName, ip := range ips { data := map[string]interface{}{ "interface_name": interfaceName, - "delete_rule": externalIp.AutoDelete, + "delete_rule": ip.AutoDelete, } - result = append(result, data) + index, found := order[interfaceName] + if !found { + unexpectedIps = append(unexpectedIps, data) + continue + } + orderedResult[index] = data // Put elements from API response in order that matches the config } - return result + + // Remove any nils from the ordered list. This can occur if the API doesn't include an interface present in the config. + finalResult := []map[string]interface{}{} + for _, item := range orderedResult { + if item != nil { + finalResult = append(finalResult, item) + } + } + + if len(unexpectedIps) > 0 { + // Additional IPs returned from API but not in the config are appended to the end of the slice + finalResult = append(finalResult, unexpectedIps...) + } + + return finalResult } func flattenUpdatePolicy(updatePolicy *compute.InstanceGroupManagerUpdatePolicy) []map[string]interface{} { diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_instance_group_manager_internal_test.go b/mmv1/third_party/terraform/services/compute/resource_compute_instance_group_manager_internal_test.go deleted file mode 100644 index 315462e277fb..000000000000 --- a/mmv1/third_party/terraform/services/compute/resource_compute_instance_group_manager_internal_test.go +++ /dev/null @@ -1,78 +0,0 @@ -package compute - -import ( - "testing" -) - -func TestInstanceGroupManager_parseUniqueId(t *testing.T) { - expectations := map[string][]string{ - "projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123": {"projects/imre-test/global/instanceTemplates/example-template-custom", "123"}, - "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123": {"https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom", "123"}, - "projects/imre-test/global/instanceTemplates/example-template-custom": {"projects/imre-test/global/instanceTemplates/example-template-custom", ""}, - "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom": {"https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom", ""}, - "example-template-custom?uniqueId=123": {"example-template-custom", "123"}, - - // this test demonstrates that uniqueIds can't override eachother - "projects/imre-test/global/instanceTemplates/example?uniqueId=123?uniqueId=456": {"projects/imre-test/global/instanceTemplates/example", "123?uniqueId=456"}, - } - - for k, v := range expectations { - aName, aUniqueId := parseUniqueId(k) - if v[0] != aName { - t.Errorf("parseUniqueId failed; name of %v should be %v, not %v", k, v[0], aName) - } - if v[1] != aUniqueId { - t.Errorf("parseUniqueId failed; uniqueId of %v should be %v, not %v", k, v[1], aUniqueId) - } - } -} - -func TestInstanceGroupManager_compareInstanceTemplate(t *testing.T) { - shouldAllMatch := []string{ - // uniqueId not present - "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom", - "projects/imre-test/global/instanceTemplates/example-template-custom", - // uniqueId present - "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123", - "projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123", - } - shouldNotMatch := map[string]string{ - // mismatching name - "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom": "projects/imre-test/global/instanceTemplates/example-template-custom2", - "projects/imre-test/global/instanceTemplates/example-template-custom": "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom2", - // matching name, but mismatching uniqueId - "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123": "projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=1234", - "projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123": "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=1234", - } - for _, v1 := range shouldAllMatch { - for _, v2 := range shouldAllMatch { - if !compareSelfLinkRelativePathsIgnoreParams("", v1, v2, nil) { - t.Fatalf("compareSelfLinkRelativePathsIgnoreParams did not match (and should have) %v and %v", v1, v2) - } - } - } - - for v1, v2 := range shouldNotMatch { - if compareSelfLinkRelativePathsIgnoreParams("", v1, v2, nil) { - t.Fatalf("compareSelfLinkRelativePathsIgnoreParams did match (and shouldn't) %v and %v", v1, v2) - } - } -} - -func TestInstanceGroupManager_convertUniqueId(t *testing.T) { - matches := map[string]string{ - // uniqueId not present (should return the same) - "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom": "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom", - "projects/imre-test/global/instanceTemplates/example-template-custom": "projects/imre-test/global/instanceTemplates/example-template-custom", - // uniqueId present (should return the last component replaced) - "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123": "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/123", - "projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123": "projects/imre-test/global/instanceTemplates/123", - "tf-test-igm-8amncgtq22?uniqueId=8361222501423044003": "8361222501423044003", - } - for input, expected := range matches { - actual := ConvertToUniqueIdWhenPresent(input) - if actual != expected { - t.Fatalf("invalid return value by ConvertToUniqueIdWhenPresent for input %v; expected: %v, actual: %v", input, expected, actual) - } - } -} diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_instance_group_manager_internal_test.go.erb b/mmv1/third_party/terraform/services/compute/resource_compute_instance_group_manager_internal_test.go.erb new file mode 100644 index 000000000000..9692c9a058f1 --- /dev/null +++ b/mmv1/third_party/terraform/services/compute/resource_compute_instance_group_manager_internal_test.go.erb @@ -0,0 +1,298 @@ +<% autogen_exception -%> +package compute + +import ( + "reflect" + "testing" + + "github.com/hashicorp/terraform-provider-google/google/tpgresource" + +<% if version == "ga" -%> + "google.golang.org/api/compute/v1" +<% else -%> + compute "google.golang.org/api/compute/v0.beta" +<% end -%> +) + +func TestInstanceGroupManager_parseUniqueId(t *testing.T) { + expectations := map[string][]string{ + "projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123": {"projects/imre-test/global/instanceTemplates/example-template-custom", "123"}, + "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123": {"https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom", "123"}, + "projects/imre-test/global/instanceTemplates/example-template-custom": {"projects/imre-test/global/instanceTemplates/example-template-custom", ""}, + "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom": {"https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom", ""}, + "example-template-custom?uniqueId=123": {"example-template-custom", "123"}, + + // this test demonstrates that uniqueIds can't override eachother + "projects/imre-test/global/instanceTemplates/example?uniqueId=123?uniqueId=456": {"projects/imre-test/global/instanceTemplates/example", "123?uniqueId=456"}, + } + + for k, v := range expectations { + aName, aUniqueId := parseUniqueId(k) + if v[0] != aName { + t.Errorf("parseUniqueId failed; name of %v should be %v, not %v", k, v[0], aName) + } + if v[1] != aUniqueId { + t.Errorf("parseUniqueId failed; uniqueId of %v should be %v, not %v", k, v[1], aUniqueId) + } + } +} + +func TestInstanceGroupManager_compareInstanceTemplate(t *testing.T) { + shouldAllMatch := []string{ + // uniqueId not present + "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom", + "projects/imre-test/global/instanceTemplates/example-template-custom", + // uniqueId present + "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123", + "projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123", + } + shouldNotMatch := map[string]string{ + // mismatching name + "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom": "projects/imre-test/global/instanceTemplates/example-template-custom2", + "projects/imre-test/global/instanceTemplates/example-template-custom": "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom2", + // matching name, but mismatching uniqueId + "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123": "projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=1234", + "projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123": "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=1234", + } + for _, v1 := range shouldAllMatch { + for _, v2 := range shouldAllMatch { + if !compareSelfLinkRelativePathsIgnoreParams("", v1, v2, nil) { + t.Fatalf("compareSelfLinkRelativePathsIgnoreParams did not match (and should have) %v and %v", v1, v2) + } + } + } + + for v1, v2 := range shouldNotMatch { + if compareSelfLinkRelativePathsIgnoreParams("", v1, v2, nil) { + t.Fatalf("compareSelfLinkRelativePathsIgnoreParams did match (and shouldn't) %v and %v", v1, v2) + } + } +} + +func TestInstanceGroupManager_convertUniqueId(t *testing.T) { + matches := map[string]string{ + // uniqueId not present (should return the same) + "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom": "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom", + "projects/imre-test/global/instanceTemplates/example-template-custom": "projects/imre-test/global/instanceTemplates/example-template-custom", + // uniqueId present (should return the last component replaced) + "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123": "https://www.googleapis.com/compute/v1/projects/imre-test/global/instanceTemplates/123", + "projects/imre-test/global/instanceTemplates/example-template-custom?uniqueId=123": "projects/imre-test/global/instanceTemplates/123", + "tf-test-igm-8amncgtq22?uniqueId=8361222501423044003": "8361222501423044003", + } + for input, expected := range matches { + actual := ConvertToUniqueIdWhenPresent(input) + if actual != expected { + t.Fatalf("invalid return value by ConvertToUniqueIdWhenPresent for input %v; expected: %v, actual: %v", input, expected, actual) + } + } +} + +func TestFlattenStatefulPolicyStatefulIps(t *testing.T) { + cases := map[string]struct { + ConfigValues []interface{} + Ips map[string]compute.StatefulPolicyPreservedStateNetworkIp + Expected []map[string]interface{} + }{ + "No IPs in config nor API data": { + ConfigValues: []interface{}{}, + Ips: map[string]compute.StatefulPolicyPreservedStateNetworkIp{}, + Expected: []map[string]interface{}{}, + }, + "Single IP (nic0) in config and API data": { + ConfigValues: []interface{}{ + map[string]interface{}{ + "interface_name": "nic0", + "delete_rule": "NEVER", + }, + }, + Ips: map[string]compute.StatefulPolicyPreservedStateNetworkIp{ + "nic0": { + AutoDelete: "NEVER", + }, + }, + Expected: []map[string]interface{}{ + { + "interface_name": "nic0", + "delete_rule": "NEVER", + }, + }, + }, + "Two IPs (nic0, nic1). Unordered in config and sorted in API data": { + ConfigValues: []interface{}{ + map[string]interface{}{ + "interface_name": "nic1", + "delete_rule": "NEVER", + }, + map[string]interface{}{ + "interface_name": "nic0", + "delete_rule": "NEVER", + }, + }, + Ips: map[string]compute.StatefulPolicyPreservedStateNetworkIp{ + "nic0": { + AutoDelete: "NEVER", + }, + "nic1": { + AutoDelete: "NEVER", + }, + }, + Expected: []map[string]interface{}{ + { + "interface_name": "nic1", + "delete_rule": "NEVER", + }, + { + "interface_name": "nic0", + "delete_rule": "NEVER", + }, + }, + }, + "Two IPs (nic0, nic1). Only nic0 in config and both stored in API data": { + ConfigValues: []interface{}{ + map[string]interface{}{ + "interface_name": "nic0", + "delete_rule": "NEVER", + }, + }, + Ips: map[string]compute.StatefulPolicyPreservedStateNetworkIp{ + "nic0": { + AutoDelete: "NEVER", + }, + "nic1": { + AutoDelete: "NEVER", + }, + }, + Expected: []map[string]interface{}{ + { + "interface_name": "nic0", + "delete_rule": "NEVER", + }, + { + "interface_name": "nic1", + "delete_rule": "NEVER", + }, + }, + }, + "Two IPs (nic0, nic1). None stored in config and both stored in API data": { + ConfigValues: []interface{}{}, + Ips: map[string]compute.StatefulPolicyPreservedStateNetworkIp{ + "nic0": { + AutoDelete: "NEVER", + }, + "nic1": { + AutoDelete: "NEVER", + }, + }, + Expected: []map[string]interface{}{ + { + "interface_name": "nic0", + "delete_rule": "NEVER", + }, + { + "interface_name": "nic1", + "delete_rule": "NEVER", + }, + }, + }, + "Three IPs (nic0, nic1, nic2). Only nic1, nic2 in config and all 3 stored in API data": { + ConfigValues: []interface{}{ + map[string]interface{}{ + "interface_name": "nic1", + "delete_rule": "NEVER", + }, + map[string]interface{}{ + "interface_name": "nic2", + "delete_rule": "NEVER", + }, + }, + Ips: map[string]compute.StatefulPolicyPreservedStateNetworkIp{ + "nic0": { + AutoDelete: "NEVER", + }, + "nic1": { + AutoDelete: "NEVER", + }, + "nic2": { + AutoDelete: "NEVER", + }, + }, + Expected: []map[string]interface{}{ + { + "interface_name": "nic1", + "delete_rule": "NEVER", + }, + { + "interface_name": "nic2", + "delete_rule": "NEVER", + }, + { + "interface_name": "nic0", + "delete_rule": "NEVER", + }, + }, + }, + "Three IPs (nic0, nic1, nic2). Only nic0, nic2 in config and only nic1, nic2 stored in API data": { + ConfigValues: []interface{}{ + map[string]interface{}{ + "interface_name": "nic2", + "delete_rule": "NEVER", + }, + map[string]interface{}{ + "interface_name": "nic0", + "delete_rule": "NEVER", + }, + }, + Ips: map[string]compute.StatefulPolicyPreservedStateNetworkIp{ + "nic1": { + AutoDelete: "NEVER", + }, + "nic2": { + AutoDelete: "NEVER", + }, + }, + Expected: []map[string]interface{}{ + { + "interface_name": "nic2", + "delete_rule": "NEVER", + }, + { + "interface_name": "nic1", + "delete_rule": "NEVER", + }, + }, + }, + } + + for tn, tc := range cases { + t.Run(tn, func(t *testing.T) { + + // Terraform config + schema := ResourceComputeRegionInstanceGroupManager().Schema + config := map[string]interface{}{ + "stateful_external_ip": tc.ConfigValues, + "stateful_internal_ip": tc.ConfigValues, + } + d := tpgresource.SetupTestResourceDataFromConfigMap(t, schema, config) + + // API response + statefulPolicyPreservedState := compute.StatefulPolicyPreservedState{ + ExternalIPs: tc.Ips, + InternalIPs: tc.Ips, + } + statefulPolicy := compute.StatefulPolicy{ + PreservedState: &statefulPolicyPreservedState, + } + + outputExternal := flattenStatefulPolicyStatefulExternalIps(d, &statefulPolicy) + if !reflect.DeepEqual(tc.Expected, outputExternal) { + t.Fatalf("expected external IPs output to be %#v, but got %#v", tc.Expected, outputExternal) + } + + outputInternal := flattenStatefulPolicyStatefulInternalIps(d, &statefulPolicy) + if !reflect.DeepEqual(tc.Expected, outputInternal) { + t.Fatalf("expected internal IPs output to be %#v, but got %#v", tc.Expected, outputInternal) + } + }) + } +} + diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_instance_group_manager_test.go.erb b/mmv1/third_party/terraform/services/compute/resource_compute_instance_group_manager_test.go.erb index 2bef96288443..7c3322419219 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_instance_group_manager_test.go.erb +++ b/mmv1/third_party/terraform/services/compute/resource_compute_instance_group_manager_test.go.erb @@ -342,7 +342,7 @@ func TestAccInstanceGroupManager_stateful(t *testing.T) { target := fmt.Sprintf("tf-test-igm-%s", acctest.RandString(t, 10)) igm := fmt.Sprintf("tf-test-igm-%s", acctest.RandString(t, 10)) hck := fmt.Sprintf("tf-test-igm-%s", acctest.RandString(t, 10)) - network := fmt.Sprintf("tf-test-igm-%s", acctest.RandString(t, 10)) + network := fmt.Sprintf("tf-test-igm-%s", acctest.RandString(t, 10)) acctest.VcrTest(t, resource.TestCase{ PreCheck: func() { acctest.AccTestPreCheck(t) }, diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_region_instance_group_manager.go.erb b/mmv1/third_party/terraform/services/compute/resource_compute_region_instance_group_manager.go.erb index 4f6360bc42e0..ff0a11876a0f 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_region_instance_group_manager.go.erb +++ b/mmv1/third_party/terraform/services/compute/resource_compute_region_instance_group_manager.go.erb @@ -768,10 +768,10 @@ func resourceComputeRegionInstanceGroupManagerRead(d *schema.ResourceData, meta if err = d.Set("status", flattenStatus(manager.Status)); err != nil { return fmt.Errorf("Error setting status in state: %s", err.Error()) } - if err = d.Set("stateful_internal_ip", flattenStatefulPolicyStatefulInternalIps(manager.StatefulPolicy)); err != nil { + if err = d.Set("stateful_internal_ip", flattenStatefulPolicyStatefulInternalIps(d, manager.StatefulPolicy)); err != nil { return fmt.Errorf("Error setting stateful_internal_ip in state: %s", err.Error()) } - if err = d.Set("stateful_external_ip", flattenStatefulPolicyStatefulExternalIps(manager.StatefulPolicy)); err != nil { + if err = d.Set("stateful_external_ip", flattenStatefulPolicyStatefulExternalIps(d, manager.StatefulPolicy)); err != nil { return fmt.Errorf("Error setting stateful_external_ip in state: %s", err.Error()) } // If unset in state set to default value diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_region_instance_group_manager_test.go.erb b/mmv1/third_party/terraform/services/compute/resource_compute_region_instance_group_manager_test.go.erb index 2394050ec57c..85416a6ebdcb 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_region_instance_group_manager_test.go.erb +++ b/mmv1/third_party/terraform/services/compute/resource_compute_region_instance_group_manager_test.go.erb @@ -309,7 +309,7 @@ func TestAccRegionInstanceGroupManager_distributionPolicy(t *testing.T) { ImportStateVerify: true, ImportStateVerifyIgnore: []string{"status"}, }, - { + { Config: testAccRegionInstanceGroupManager_distributionPolicyUpdate(template, igm, zones), }, { @@ -329,7 +329,7 @@ func TestAccRegionInstanceGroupManager_stateful(t *testing.T) { template := fmt.Sprintf("tf-test-rigm-%s", acctest.RandString(t, 10)) igm := fmt.Sprintf("tf-test-rigm-%s", acctest.RandString(t, 10)) - network := fmt.Sprintf("tf-test-igm-%s", acctest.RandString(t, 10)) + network := fmt.Sprintf("tf-test-igm-%s", acctest.RandString(t, 10)) acctest.VcrTest(t, resource.TestCase{ PreCheck: func() { acctest.AccTestPreCheck(t) }, @@ -367,6 +367,25 @@ func TestAccRegionInstanceGroupManager_stateful(t *testing.T) { }) } +func TestAccRegionInstanceGroupManager_APISideListRecordering(t *testing.T) { + t.Parallel() + + context := map[string]interface{}{ + "name": fmt.Sprintf("tf-test-%s", acctest.RandString(t, 10)), + } + + acctest.VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + CheckDestroy: testAccCheckRegionInstanceGroupManagerDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: testAccRegionInstanceGroupManager_statefulUnordered(context), + }, + }, + }) +} + func testAccCheckRegionInstanceGroupManagerDestroyProducer(t *testing.T) func(s *terraform.State) error { return func(s *terraform.State) error { config := acctest.GoogleProviderConfig(t) @@ -1572,3 +1591,90 @@ resource "google_compute_region_instance_group_manager" "igm-basic" { } `, network, template, igm) } + +func testAccRegionInstanceGroupManager_statefulUnordered(context map[string]interface{}) string { + return acctest.Nprintf(` +data "google_compute_image" "my_image" { + family = "debian-11" + project = "debian-cloud" +} + +resource "google_compute_network" "igm-basic" { + name = "%{name}" +} + +resource "google_compute_instance_template" "igm-basic" { + name = "%{name}" + 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 + device_name = "stateful-disk" + } + disk { + source_image = data.google_compute_image.my_image.self_link + auto_delete = true + device_name = "stateful-disk2" + } + network_interface { + network = "default" + } + network_interface { + network = google_compute_network.igm-basic.self_link + } +} + +resource "google_compute_region_instance_group_manager" "igm-basic" { + description = "Terraform test instance group manager" + name = "%{name}" + + 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 + update_policy { + instance_redistribution_type = "NONE" + type = "OPPORTUNISTIC" + minimal_action = "REPLACE" + max_surge_fixed = 0 + max_unavailable_fixed = 6 + } + stateful_disk { + device_name = "stateful-disk" + delete_rule = "NEVER" + } + + // stateful_internal_ip blocks are intentionally out of lexical order (for interface_name) + + stateful_internal_ip { + interface_name = "nic1" + delete_rule = "ON_PERMANENT_INSTANCE_DELETION" + } + + stateful_internal_ip { + interface_name = "nic0" + delete_rule = "ON_PERMANENT_INSTANCE_DELETION" + } + + // stateful_external_ip blocks are intentionally out of lexical order (for interface_name) + + stateful_external_ip { + interface_name = "nic1" + delete_rule = "NEVER" + } + + stateful_external_ip { + interface_name = "nic0" + delete_rule = "NEVER" + } + +} +`, context) +} \ No newline at end of file From 1e5399b3f6327c4bbacfd89f352e949c01643bd5 Mon Sep 17 00:00:00 2001 From: Sarah French Date: Fri, 5 Jan 2024 11:54:52 +0000 Subject: [PATCH 2/2] Sort unexpected IPs not present in TF config, update unit test --- ...urce_compute_instance_group_manager.go.erb | 4 ++++ ...nstance_group_manager_internal_test.go.erb | 24 ++++++++++++++++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_instance_group_manager.go.erb b/mmv1/third_party/terraform/services/compute/resource_compute_instance_group_manager.go.erb index a3f09e7e4af1..ad499ddd82b2 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_instance_group_manager.go.erb +++ b/mmv1/third_party/terraform/services/compute/resource_compute_instance_group_manager.go.erb @@ -5,6 +5,7 @@ import ( "fmt" "log" "regexp" + "sort" "strings" "time" @@ -1353,6 +1354,9 @@ func flattenStatefulPolicyStatefulIps(d *schema.ResourceData, ipfieldName string } orderedResult[index] = data // Put elements from API response in order that matches the config } + sort.Slice(unexpectedIps, func(i, j int) bool { + return unexpectedIps[i]["interface_name"].(string) < unexpectedIps[j]["interface_name"].(string) + }) // Remove any nils from the ordered list. This can occur if the API doesn't include an interface present in the config. finalResult := []map[string]interface{}{} diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_instance_group_manager_internal_test.go.erb b/mmv1/third_party/terraform/services/compute/resource_compute_instance_group_manager_internal_test.go.erb index 9692c9a058f1..3cb2a28d1386 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_instance_group_manager_internal_test.go.erb +++ b/mmv1/third_party/terraform/services/compute/resource_compute_instance_group_manager_internal_test.go.erb @@ -173,15 +173,25 @@ func TestFlattenStatefulPolicyStatefulIps(t *testing.T) { }, }, }, - "Two IPs (nic0, nic1). None stored in config and both stored in API data": { + "Five IPs (nic0 - nic4). None stored in config and all stored in API data": { ConfigValues: []interface{}{}, Ips: map[string]compute.StatefulPolicyPreservedStateNetworkIp{ + // Out of order here to encourage randomness + "nic3": { + AutoDelete: "NEVER", + }, "nic0": { AutoDelete: "NEVER", }, "nic1": { AutoDelete: "NEVER", }, + "nic4": { + AutoDelete: "NEVER", + }, + "nic2": { + AutoDelete: "NEVER", + }, }, Expected: []map[string]interface{}{ { @@ -192,6 +202,18 @@ func TestFlattenStatefulPolicyStatefulIps(t *testing.T) { "interface_name": "nic1", "delete_rule": "NEVER", }, + { + "interface_name": "nic2", + "delete_rule": "NEVER", + }, + { + "interface_name": "nic3", + "delete_rule": "NEVER", + }, + { + "interface_name": "nic4", + "delete_rule": "NEVER", + }, }, }, "Three IPs (nic0, nic1, nic2). Only nic1, nic2 in config and all 3 stored in API data": {