improve replace image function

CHANGELOG.md

@@ -1,4 +1,11 @@
-## 1.0.6 (unreleased)
+## 1.0.7 (unreleased)
+
+## 1.0.6 (May 2, 2017)
+
+IMPROVEMENTS:
+
+  * resource/alicloud_instance: add replaceing system disk function ([#119](https://github.com/alibaba/terraform-provider/pull/119))
+
 
 ## 1.0.5 (April 18, 2017)
 

alicloud/resource_alicloud_instance.go

@@ -365,87 +365,83 @@ func resourceAliyunInstanceUpdate(d *schema.ResourceData, meta interface{}) erro
 	}
 
 	imageUpdate := false
-	if d.HasChange("image_id") {
-		oImage, nImage := d.GetChange("image_id")
-		if oImage != nil && oImage != "" {
-			replaceSystemArgs := &ecs.ReplaceSystemDiskArgs{
-				InstanceId: d.Id(),
-				ImageId:    nImage.(string),
-				SystemDisk: ecs.SystemDiskType{
-					Size: d.Get("system_disk_size").(int),
-				},
-			}
+	if d.HasChange("image_id") && !d.IsNewResource() {
+		log.Printf("[DEBUG] Replace instance system disk via changing image_id")
+		replaceSystemArgs := &ecs.ReplaceSystemDiskArgs{
+			InstanceId: d.Id(),
+			ImageId:    d.Get("image_id").(string),
+			SystemDisk: ecs.SystemDiskType{
+				Size: d.Get("system_disk_size").(int),
+			},
+		}
 
-			// Ensure instance's status must be stopped before replace system disk.
-			if v, ok := d.GetOk("status"); ok && v.(string) != "" {
-				if ecs.InstanceStatus(d.Get("status").(string)) == ecs.Running {
-					log.Printf("[DEBUG] StopInstance before change system disk")
-					if err := conn.StopInstance(d.Id(), true); err != nil {
-						return fmt.Errorf("Force Stop Instance got an error: %#v", err)
-					}
-					if err := conn.WaitForInstance(d.Id(), ecs.Stopped, 60); err != nil {
-						return fmt.Errorf("WaitForInstance got error: %#v", err)
-					}
+		if v, ok := d.GetOk("status"); ok && v.(string) != "" {
+			if ecs.InstanceStatus(d.Get("status").(string)) == ecs.Running {
+				log.Printf("[DEBUG] StopInstance before change system disk")
+				if err := conn.StopInstance(d.Id(), true); err != nil {
+					return fmt.Errorf("Force Stop Instance got an error: %#v", err)
+				}
+				if err := conn.WaitForInstance(d.Id(), ecs.Stopped, 60); err != nil {
+					return fmt.Errorf("WaitForInstance got error: %#v", err)
 				}
 			}
+		}
 
-			_, err := conn.ReplaceSystemDisk(replaceSystemArgs)
-			if err != nil {
-				return fmt.Errorf("Replace system disk got an error: %#v", err)
-			}
-
-			// Ensure instance's image has been replaced successfully.
-			timeout := ecs.InstanceDefaultTimeout
-			for {
-				instance, errDesc := conn.DescribeInstanceAttribute(d.Id())
-				if errDesc != nil {
-					return fmt.Errorf("Describe instance got an error: %#v", errDesc)
-				}
+		_, err := conn.ReplaceSystemDisk(replaceSystemArgs)
+		if err != nil {
+			return fmt.Errorf("Replace system disk got an error: %#v", err)
+		}
 
-				if instance.ImageId == nImage {
-					break
-				}
-				time.Sleep(ecs.DefaultWaitForInterval * time.Second)
+		// Ensure instance's image has been replaced successfully.
+		timeout := ecs.InstanceDefaultTimeout
+		for {
+			instance, errDesc := conn.DescribeInstanceAttribute(d.Id())
+			if errDesc != nil {
+				return fmt.Errorf("Describe instance got an error: %#v", errDesc)
+			}
 
-				timeout = timeout - ecs.DefaultWaitForInterval
-				if timeout <= 0 {
-					return common.GetClientErrorFromString("Timeout")
-				}
+			if instance.ImageId == d.Get("image_id") {
+				break
 			}
+			time.Sleep(ecs.DefaultWaitForInterval * time.Second)
 
-			imageUpdate = true
-			d.SetPartial("system_disk_size")
-			d.SetPartial("image_id")
-		}
-	} else {
-		// Provider doesn't support change 'system_disk_size'separately.
-		if d.HasChange("system_disk_size") {
-			return fmt.Errorf("Update resource failed. 'system_disk_size' isn't allowed to change separately. You can update them via renewing instance or changing 'image_id' to replace system disk.")
+			timeout = timeout - ecs.DefaultWaitForInterval
+			if timeout <= 0 {
+				return common.GetClientErrorFromString("Timeout")
+			}
 		}
+
+		imageUpdate = true
+		d.SetPartial("system_disk_size")
+		d.SetPartial("image_id")
+	}
+	// Provider doesn't support change 'system_disk_size'separately.
+	if d.HasChange("system_disk_size") && !d.HasChange("image_id") {
+		return fmt.Errorf("Update resource failed. 'system_disk_size' isn't allowed to change separately. You can update it via renewing instance or replacing system disk.")
 	}
 
 	attributeUpdate := false
 	args := &ecs.ModifyInstanceAttributeArgs{
 		InstanceId: d.Id(),
 	}
 
-	if d.HasChange("instance_name") {
+	if d.HasChange("instance_name") && !d.IsNewResource() {
 		log.Printf("[DEBUG] ModifyInstanceAttribute instance_name")
 		d.SetPartial("instance_name")
 		args.InstanceName = d.Get("instance_name").(string)
 
 		attributeUpdate = true
 	}
 
-	if d.HasChange("description") {
+	if d.HasChange("description") && !d.IsNewResource() {
 		log.Printf("[DEBUG] ModifyInstanceAttribute description")
 		d.SetPartial("description")
 		args.Description = d.Get("description").(string)
 
 		attributeUpdate = true
 	}
 
-	if d.HasChange("host_name") {
+	if d.HasChange("host_name") && !d.IsNewResource() {
 		log.Printf("[DEBUG] ModifyInstanceAttribute host_name")
 		d.SetPartial("host_name")
 		args.HostName = d.Get("host_name").(string)
@@ -454,7 +450,7 @@ func resourceAliyunInstanceUpdate(d *schema.ResourceData, meta interface{}) erro
 	}
 
 	passwordUpdate := false
-	if d.HasChange("password") {
+	if d.HasChange("password") && !d.IsNewResource() {
 		log.Printf("[DEBUG] ModifyInstanceAttribute password")
 		d.SetPartial("password")
 		args.Password = d.Get("password").(string)
@@ -474,18 +470,20 @@ func resourceAliyunInstanceUpdate(d *schema.ResourceData, meta interface{}) erro
 		if errDesc != nil {
 			return fmt.Errorf("Describe instance got an error: %#v", errDesc)
 		}
-
-		if instance.Status == ecs.Running {
+		if instance.Status != ecs.Running && instance.Status != ecs.Stopped {
+			return fmt.Errorf("ECS instance's status doesn't support to start or reboot operation after replace image_id or update password. The current instance's status is %#v", instance.Status)
+		} else if instance.Status == ecs.Running {
 			log.Printf("[DEBUG] Reboot instance after change image or password")
 			if err := conn.RebootInstance(d.Id(), false); err != nil {
 				return fmt.Errorf("RebootInstance got error: %#v", err)
 			}
-		} else if instance.Status == ecs.Stopped {
+		} else {
 			log.Printf("[DEBUG] Start instance after change image or password")
 			if err := conn.StartInstance(d.Id()); err != nil {
 				return fmt.Errorf("StartInstance got error: %#v", err)
 			}
 		}
+
 		// Start instance sometimes costs more than 6 minutes when os type is centos.
 		if err := conn.WaitForInstance(d.Id(), ecs.Running, 400); err != nil {
 			return fmt.Errorf("WaitForInstance got error: %#v", err)

alicloud/resource_alicloud_instance_test.go

@@ -410,6 +410,41 @@ func TestAccAlicloudInstance_update(t *testing.T) {
 	})
 }
 
+func TestAccAlicloudInstanceImage_update(t *testing.T) {
+	var instance ecs.InstanceAttributesType
+
+	resource.Test(t, resource.TestCase{
+		PreCheck: func() {
+			testAccPreCheck(t)
+		},
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckInstanceDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccCheckInstanceImageOrigin,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckInstanceExists("alicloud_instance.update_image", &instance),
+					resource.TestCheckResourceAttr(
+						"alicloud_instance.update_image",
+						"system_disk_size",
+						"50"),
+				),
+			},
+
+			resource.TestStep{
+				Config: testAccCheckInstanceImageUpdate,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckInstanceExists("alicloud_instance.update_image", &instance),
+					resource.TestCheckResourceAttr(
+						"alicloud_instance.update_image",
+						"system_disk_size",
+						"60"),
+				),
+			},
+		},
+	})
+}
+
 func TestAccAlicloudInstance_privateIP(t *testing.T) {
 	var instance ecs.InstanceAttributesType
 
@@ -1223,5 +1258,78 @@ resource "alicloud_instance" "foo" {
     	instance_name = "test_foo"
     	io_optimized = "optimized"
 }
+`
+const testAccCheckInstanceImageOrigin = `
+data "alicloud_images" "centos" {
+	most_recent = true
+	owners = "system"
+	name_regex = "^centos_6\\w{1,5}[64]{1}.*"
+}
+
+resource "alicloud_vpc" "foo" {
+	name = "tf_test_image"
+	cidr_block = "10.1.0.0/21"
+}
+
+resource "alicloud_vswitch" "foo" {
+	vpc_id = "${alicloud_vpc.foo.id}"
+	cidr_block = "10.1.1.0/24"
+	availability_zone = "cn-beijing-a"
+}
 
+resource "alicloud_security_group" "tf_test_foo" {
+	name = "tf_test_foo"
+	description = "foo"
+	vpc_id = "${alicloud_vpc.foo.id}"
+}
+
+resource "alicloud_instance" "update_image" {
+	image_id = "${data.alicloud_images.centos.images.0.id}"
+	availability_zone = "cn-beijing-a"
+  	system_disk_category = "cloud_efficiency"
+  	system_disk_size = 50
+
+  	instance_type = "ecs.n1.small"
+  	internet_charge_type = "PayByBandwidth"
+  	instance_name = "update_image"
+  	io_optimized = "optimized"
+  	password = "Test12345"
+}
+`
+const testAccCheckInstanceImageUpdate = `
+data "alicloud_images" "ubuntu" {
+	most_recent = true
+	owners = "system"
+	name_regex = "^ubuntu_14\\w{1,5}[64]{1}.*"
+}
+
+resource "alicloud_vpc" "foo" {
+	name = "tf_test_image"
+	cidr_block = "10.1.0.0/21"
+}
+
+resource "alicloud_vswitch" "foo" {
+	vpc_id = "${alicloud_vpc.foo.id}"
+	cidr_block = "10.1.1.0/24"
+	availability_zone = "cn-beijing-a"
+}
+
+resource "alicloud_security_group" "tf_test_foo" {
+	name = "tf_test_foo"
+	description = "foo"
+	vpc_id = "${alicloud_vpc.foo.id}"
+}
+
+resource "alicloud_instance" "update_image" {
+	image_id = "${data.alicloud_images.ubuntu.images.0.id}"
+	availability_zone = "cn-beijing-a"
+  	system_disk_category = "cloud_efficiency"
+  	system_disk_size = 60
+
+  	instance_type = "ecs.n1.small"
+  	internet_charge_type = "PayByBandwidth"
+  	instance_name = "update_image"
+  	io_optimized = "optimized"
+  	password = "Test12345"
+}
 `