From 44bf0bec96f8c60fff156401cf0463b4b2f7feb4 Mon Sep 17 00:00:00 2001
From: Paddy <paddy@paddy.io>
Date: Mon, 9 Jan 2017 15:15:50 -0800
Subject: [PATCH] Read update_strategy before overwriting it. (#11013)

As brought up in #10174, our update_strategy property for instance group
managers in GCP would always be set to "RESTART" on read, even if the
user asked for them to be "NONE" in the config.

This adds a test to ensure that the user wishes were respected, which
fails until we check for update_strategy in the ResourceData before we
update it within the Read function. Because the update_strategy property
doesn't map to anything in the API, we never need to read it from
anywhere but the config, which means the ResourceData should be
considered authoritative by the time we get to the Read function.

The fix for this was provided by @JDiPierro in #10198 originally, but
was missing tests, so it got squashed into this.
---
 resource_compute_instance_group_manager.go    |  6 +-
 ...rce_compute_instance_group_manager_test.go | 83 +++++++++++++++++++
 2 files changed, 88 insertions(+), 1 deletion(-)

diff --git a/resource_compute_instance_group_manager.go b/resource_compute_instance_group_manager.go
index ff39f023..89bff60d 100644
--- a/resource_compute_instance_group_manager.go
+++ b/resource_compute_instance_group_manager.go
@@ -242,7 +242,11 @@ func resourceComputeInstanceGroupManagerRead(d *schema.ResourceData, meta interf
 	d.Set("instance_group", manager.InstanceGroup)
 	d.Set("target_size", manager.TargetSize)
 	d.Set("self_link", manager.SelfLink)
-	d.Set("update_strategy", "RESTART") //this field doesn't match the manager api, set to default value
+	update_strategy, ok := d.GetOk("update_strategy")
+	if !ok {
+		update_strategy = "RESTART"
+	}
+	d.Set("update_strategy", update_strategy.(string))
 
 	return nil
 }
diff --git a/resource_compute_instance_group_manager_test.go b/resource_compute_instance_group_manager_test.go
index 16e370b0..a16646db 100644
--- a/resource_compute_instance_group_manager_test.go
+++ b/resource_compute_instance_group_manager_test.go
@@ -112,6 +112,29 @@ func TestAccInstanceGroupManager_updateLifecycle(t *testing.T) {
 		},
 	})
 }
+
+func TestAccInstanceGroupManager_updateStrategy(t *testing.T) {
+	var manager compute.InstanceGroupManager
+	igm := fmt.Sprintf("igm-test-%s", acctest.RandString(10))
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckInstanceGroupManagerDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccInstanceGroupManager_updateStrategy(igm),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckInstanceGroupManagerExists(
+						"google_compute_instance_group_manager.igm-update-strategy", &manager),
+					testAccCheckInstanceGroupManagerUpdateStrategy(
+						"google_compute_instance_group_manager.igm-update-strategy", "NONE"),
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckInstanceGroupManagerDestroy(s *terraform.State) error {
 	config := testAccProvider.Meta().(*Config)
 
@@ -268,6 +291,25 @@ func testAccCheckInstanceGroupManagerTemplateTags(n string, tags []string) resou
 	}
 }
 
+func testAccCheckInstanceGroupManagerUpdateStrategy(n, strategy string) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		rs, ok := s.RootModule().Resources[n]
+		if !ok {
+			return fmt.Errorf("Not found: %s", n)
+		}
+
+		if rs.Primary.ID == "" {
+			return fmt.Errorf("No ID is set")
+		}
+
+		if rs.Primary.Attributes["update_strategy"] != strategy {
+			return fmt.Errorf("Expected strategy to be %s, got %s",
+				strategy, rs.Primary.Attributes["update_strategy"])
+		}
+		return nil
+	}
+}
+
 func testAccInstanceGroupManager_basic(template, target, igm1, igm2 string) string {
 	return fmt.Sprintf(`
 	resource "google_compute_instance_template" "igm-basic" {
@@ -488,6 +530,47 @@ func testAccInstanceGroupManager_updateLifecycle(tag, igm string) string {
 	}`, tag, igm)
 }
 
+func testAccInstanceGroupManager_updateStrategy(igm string) string {
+	return fmt.Sprintf(`
+	resource "google_compute_instance_template" "igm-update-strategy" {
+		machine_type = "n1-standard-1"
+		can_ip_forward = false
+		tags = ["terraform-testing"]
+
+		disk {
+			source_image = "debian-cloud/debian-8-jessie-v20160803"
+			auto_delete = true
+			boot = true
+		}
+
+		network_interface {
+			network = "default"
+		}
+
+		service_account {
+			scopes = ["userinfo-email", "compute-ro", "storage-ro"]
+		}
+
+		lifecycle {
+			create_before_destroy = true
+		}
+	}
+
+	resource "google_compute_instance_group_manager" "igm-update-strategy" {
+		description = "Terraform test instance group manager"
+		name = "%s"
+		instance_template = "${google_compute_instance_template.igm-update-strategy.self_link}"
+		base_instance_name = "igm-update-strategy"
+		zone = "us-central1-c"
+		target_size = 2
+		update_strategy = "NONE"
+		named_port {
+			name = "customhttp"
+			port = 8080
+		}
+	}`, igm)
+}
+
 func resourceSplitter(resource string) string {
 	splits := strings.Split(resource, "/")