From d51cbe494d179a63af065e9b4f17090186e05399 Mon Sep 17 00:00:00 2001 From: Karol Gorczyk Date: Thu, 8 Aug 2024 12:40:55 +0200 Subject: [PATCH 1/5] Fix condition in forceNewIfNetworkIPNotUpdatableFunc --- .../services/compute/resource_compute_instance.go.erb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_instance.go.erb b/mmv1/third_party/terraform/services/compute/resource_compute_instance.go.erb index 0cafa76a2e16..63c0969197fb 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_instance.go.erb +++ b/mmv1/third_party/terraform/services/compute/resource_compute_instance.go.erb @@ -115,12 +115,12 @@ func forceNewIfNetworkIPNotUpdatableFunc(d tpgresource.TerraformResourceDiff) er for i := 0; i < newCount.(int); i++ { prefix := fmt.Sprintf("network_interface.%d", i) - networkKey := prefix + ".network" + oldN, newN := d.GetChange(prefix + ".network") subnetworkKey := prefix + ".subnetwork" subnetworkProjectKey := prefix + ".subnetwork_project" networkIPKey := prefix + ".network_ip" if d.HasChange(networkIPKey) { - if !d.HasChange(networkKey) && !d.HasChange(subnetworkKey) && !d.HasChange(subnetworkProjectKey) { + if tpgresource.CompareSelfLinkOrResourceName("", oldN.(string), newN.(string), nil) && !d.HasChange(subnetworkKey) && !d.HasChange(subnetworkProjectKey) { if err := d.ForceNew(networkIPKey); err != nil { return err } From 53902925b9b11e838160cc3a833ce830ecc62b7b Mon Sep 17 00:00:00 2001 From: Karol Gorczyk Date: Thu, 8 Aug 2024 12:49:50 +0200 Subject: [PATCH 2/5] broaden the if statement --- .../services/compute/resource_compute_instance.go.erb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_instance.go.erb b/mmv1/third_party/terraform/services/compute/resource_compute_instance.go.erb index 63c0969197fb..db871b32efd5 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_instance.go.erb +++ b/mmv1/third_party/terraform/services/compute/resource_compute_instance.go.erb @@ -115,12 +115,13 @@ func forceNewIfNetworkIPNotUpdatableFunc(d tpgresource.TerraformResourceDiff) er for i := 0; i < newCount.(int); i++ { prefix := fmt.Sprintf("network_interface.%d", i) - oldN, newN := d.GetChange(prefix + ".network") + networkKey := prefix + ".network" + oldN, newN := d.GetChange(networkKey) subnetworkKey := prefix + ".subnetwork" subnetworkProjectKey := prefix + ".subnetwork_project" networkIPKey := prefix + ".network_ip" if d.HasChange(networkIPKey) { - if tpgresource.CompareSelfLinkOrResourceName("", oldN.(string), newN.(string), nil) && !d.HasChange(subnetworkKey) && !d.HasChange(subnetworkProjectKey) { + if !d.HasChange(subnetworkKey) && !d.HasChange(subnetworkProjectKey) && tpgresource.CompareSelfLinkOrResourceName("", oldN.(string), newN.(string), nil) || !d.HasChange(networkKey) { if err := d.ForceNew(networkIPKey); err != nil { return err } From 05b175bb1a95d796e893f71d9172c433a0b00660 Mon Sep 17 00:00:00 2001 From: Karol Gorczyk Date: Thu, 8 Aug 2024 12:51:27 +0200 Subject: [PATCH 3/5] fix white space --- .../terraform/services/compute/resource_compute_instance.go.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_instance.go.erb b/mmv1/third_party/terraform/services/compute/resource_compute_instance.go.erb index db871b32efd5..79e3f236dfe9 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_instance.go.erb +++ b/mmv1/third_party/terraform/services/compute/resource_compute_instance.go.erb @@ -121,7 +121,7 @@ func forceNewIfNetworkIPNotUpdatableFunc(d tpgresource.TerraformResourceDiff) er subnetworkProjectKey := prefix + ".subnetwork_project" networkIPKey := prefix + ".network_ip" if d.HasChange(networkIPKey) { - if !d.HasChange(subnetworkKey) && !d.HasChange(subnetworkProjectKey) && tpgresource.CompareSelfLinkOrResourceName("", oldN.(string), newN.(string), nil) || !d.HasChange(networkKey) { + if !d.HasChange(subnetworkKey) && !d.HasChange(subnetworkProjectKey) && tpgresource.CompareSelfLinkOrResourceName("", oldN.(string), newN.(string), nil) || !d.HasChange(networkKey) { if err := d.ForceNew(networkIPKey); err != nil { return err } From 6c742f6374ab76d8cf1ddf82db9845f0031ef1b7 Mon Sep 17 00:00:00 2001 From: Karol Gorczyk Date: Tue, 13 Aug 2024 12:41:34 +0200 Subject: [PATCH 4/5] add testing for network_ip updating --- .../resource_compute_instance_test.go.erb | 178 ++++++++++++++++++ 1 file changed, 178 insertions(+) diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_instance_test.go.erb b/mmv1/third_party/terraform/services/compute/resource_compute_instance_test.go.erb index 4f9605294de4..4514b9c383fd 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_instance_test.go.erb +++ b/mmv1/third_party/terraform/services/compute/resource_compute_instance_test.go.erb @@ -2577,6 +2577,46 @@ func TestAccComputeInstance_subnetworkUpdate(t *testing.T) { }) } +func TestAccComputeInstance_networkIpUpdate(t *testing.T) { + t.Parallel() + + var instance compute.Instance + instanceName := fmt.Sprintf("tf-test-%s", acctest.RandString(t, 10)) + suffix := fmt.Sprintf("%s", acctest.RandString(t, 10)) + + acctest.VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + CheckDestroy: testAccCheckComputeInstanceDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: testAccComputeInstance_networkIpUpdate(suffix, instanceName), + Check: resource.ComposeTestCheckFunc( + testAccCheckComputeInstanceExists(t, "google_compute_instance.foobar", &instance), + testAccCheckComputeInstanceHasNetworkIP(&instance, "10.3.0.3"), + ), + }, + computeInstanceImportStep("us-east1-d", instanceName, []string{}), + { + Config: testAccComputeInstance_networkIpUpdateByHand(suffix, instanceName), + Check: resource.ComposeTestCheckFunc( + testAccCheckComputeInstanceExists(t, "google_compute_instance.foobar", &instance), + testAccCheckComputeInstanceHasNetworkIP(&instance, "10.3.0.4"), + ), + }, + computeInstanceImportStep("us-east1-d", instanceName, []string{}), + { + Config: testAccComputeInstance_networkIpUpdateWithComputeAddress(suffix, instanceName), + Check: resource.ComposeTestCheckFunc( + testAccCheckComputeInstanceExists(t, "google_compute_instance.foobar", &instance), + testAccCheckComputeInstanceHasNetworkIP(&instance, "10.3.0.5"), + ), + }, + computeInstanceImportStep("us-east1-d", instanceName, []string{}), + }, + }) +} + func TestAccComputeInstance_queueCount(t *testing.T) { t.Parallel() instanceName := fmt.Sprintf("tf-test-%s", acctest.RandString(t, 10)) @@ -8351,6 +8391,144 @@ func testAccComputeInstance_subnetworkUpdateTwo(suffix, instance string) string `, suffix, suffix, suffix, suffix, instance) } +func testAccComputeInstance_networkIpUpdate(suffix, instance string) string { + return fmt.Sprintf(` + data "google_compute_image" "my_image" { + family = "debian-11" + project = "debian-cloud" + } + + resource "google_compute_network" "inst-test-network" { + name = "tf-test-network-%s" + auto_create_subnetworks = false + } + + resource "google_compute_subnetwork" "inst-test-subnetwork" { + name = "tf-test-compute-subnet-%s" + ip_cidr_range = "10.3.0.0/16" + region = "us-east1" + network = google_compute_network.inst-test-network.id + } + + resource "google_compute_address" "inst-test-address" { + name = "tf-test-compute-address-%s" + region = "us-east1" + subnetwork = google_compute_subnetwork.inst-test-subnetwork.id + address_type = "INTERNAL" + address = "10.3.0.5" + } + + resource "google_compute_instance" "foobar" { + name = "%s" + machine_type = "e2-medium" + zone = "us-east1-d" + + boot_disk { + initialize_params { + image = data.google_compute_image.my_image.id + } + } + + network_interface { + subnetwork = google_compute_subnetwork.inst-test-subnetwork.id + network_ip = "10.3.0.3" + } + } +`, suffix, suffix, suffix, instance) +} + +func testAccComputeInstance_networkIpUpdateByHand(suffix, instance string) string { + return fmt.Sprintf(` + data "google_compute_image" "my_image" { + family = "debian-11" + project = "debian-cloud" + } + + resource "google_compute_network" "inst-test-network" { + name = "tf-test-network-%s" + auto_create_subnetworks = false + } + + resource "google_compute_subnetwork" "inst-test-subnetwork" { + name = "tf-test-compute-subnet-%s" + ip_cidr_range = "10.3.0.0/16" + region = "us-east1" + network = google_compute_network.inst-test-network.id + } + + resource "google_compute_address" "inst-test-address" { + name = "tf-test-compute-address-%s" + region = "us-east1" + subnetwork = google_compute_subnetwork.inst-test-subnetwork.id + address_type = "INTERNAL" + address = "10.3.0.5" + } + + resource "google_compute_instance" "foobar" { + name = "%s" + machine_type = "e2-medium" + zone = "us-east1-d" + + boot_disk { + initialize_params { + image = data.google_compute_image.my_image.id + } + } + + network_interface { + subnetwork = google_compute_subnetwork.inst-test-subnetwork.id + network_ip = "10.3.0.4" + } + } +`, suffix, suffix, suffix, instance) +} + +func testAccComputeInstance_networkIpUpdateWithComputeAddress(suffix, instance string) string { + return fmt.Sprintf(` + data "google_compute_image" "my_image" { + family = "debian-11" + project = "debian-cloud" + } + + resource "google_compute_network" "inst-test-network" { + name = "tf-test-network-%s" + auto_create_subnetworks = false + } + + resource "google_compute_subnetwork" "inst-test-subnetwork" { + name = "tf-test-compute-subnet-%s" + ip_cidr_range = "10.3.0.0/16" + region = "us-east1" + network = google_compute_network.inst-test-network.id + } + + resource "google_compute_address" "inst-test-address" { + name = "tf-test-compute-address-%s" + region = "us-east1" + subnetwork = google_compute_subnetwork.inst-test-subnetwork.id + address_type = "INTERNAL" + address = "10.3.0.5" + } + + resource "google_compute_instance" "foobar" { + name = "%s" + machine_type = "e2-medium" + zone = "us-east1-d" + + boot_disk { + initialize_params { + image = data.google_compute_image.my_image.id + } + } + + network_interface { + subnetwork = google_compute_subnetwork.inst-test-subnetwork.id + network_ip = google_compute_address.inst-test-address.address + } + } +`, suffix, suffix, suffix, instance) +} + func testAccComputeInstance_queueCountSet(instance string) string { return fmt.Sprintf(` data "google_compute_image" "my_image" { From c8a5e93dcae12176e3eef0e24bfcbd5eec611952 Mon Sep 17 00:00:00 2001 From: Karol Gorczyk Date: Wed, 14 Aug 2024 12:47:07 +0200 Subject: [PATCH 5/5] change the if statement to resolve subnetwork selflinks, as well. Make more restrictive for unit test --- .../services/compute/resource_compute_instance.go.erb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_instance.go.erb b/mmv1/third_party/terraform/services/compute/resource_compute_instance.go.erb index 79e3f236dfe9..116ffc2c2378 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_instance.go.erb +++ b/mmv1/third_party/terraform/services/compute/resource_compute_instance.go.erb @@ -118,10 +118,11 @@ func forceNewIfNetworkIPNotUpdatableFunc(d tpgresource.TerraformResourceDiff) er networkKey := prefix + ".network" oldN, newN := d.GetChange(networkKey) subnetworkKey := prefix + ".subnetwork" + oldS, newS := d.GetChange(subnetworkKey) subnetworkProjectKey := prefix + ".subnetwork_project" networkIPKey := prefix + ".network_ip" if d.HasChange(networkIPKey) { - if !d.HasChange(subnetworkKey) && !d.HasChange(subnetworkProjectKey) && tpgresource.CompareSelfLinkOrResourceName("", oldN.(string), newN.(string), nil) || !d.HasChange(networkKey) { + if tpgresource.CompareSelfLinkOrResourceName("", oldS.(string), newS.(string), nil) && !d.HasChange(subnetworkProjectKey) && tpgresource.CompareSelfLinkOrResourceName("", oldN.(string), newN.(string), nil) { if err := d.ForceNew(networkIPKey); err != nil { return err }