Merge pull request #5856 from svanharmelen/b-cloudstack-instance-name

Some minor updates and added a proper test

builtin/providers/cloudstack/resource_cloudstack_instance.go

@@ -24,7 +24,6 @@ func resourceCloudStackInstance() *schema.Resource {
 				Type:     schema.TypeString,
 				Optional: true,
 				Computed: true,
-				ForceNew: true,
 			},
 
 			"display_name": &schema.Schema{
@@ -137,10 +136,8 @@ func resourceCloudStackInstanceCreate(d *schema.ResourceData, meta interface{})
 	// Set the display name
 	if displayname, ok := d.GetOk("display_name"); ok {
 		p.SetDisplayname(displayname.(string))
-	} else {
-		if hasName {
-			p.SetDisplayname(name.(string))
-		}
+	} else if hasName {
+		p.SetDisplayname(name.(string))
 	}
 
 	if zone.Networktype == "Advanced" {
@@ -249,25 +246,6 @@ func resourceCloudStackInstanceUpdate(d *schema.ResourceData, meta interface{})
 
 	name := d.Get("name").(string)
 
-	if d.HasChange("name") {
-		log.Printf("[DEBUG] Name for %s changed to %s, starting update", d.Id(), name)
-
-		// Create a new parameter struct
-		p := cs.VirtualMachine.NewUpdateVirtualMachineParams(d.Id())
-
-		// Set the new name
-		p.SetName(d.Get("name").(string))
-
-		// Update the display name
-		_, err := cs.VirtualMachine.UpdateVirtualMachine(p)
-		if err != nil {
-			return fmt.Errorf(
-				"Error updating the name for instance %s: %s", name, err)
-		}
-
-		d.SetPartial("name")
-	}
-
 	// Check if the display name is changed and if so, update the virtual machine
 	if d.HasChange("display_name") {
 		log.Printf("[DEBUG] Display name changed for %s, starting update", name)
@@ -289,7 +267,7 @@ func resourceCloudStackInstanceUpdate(d *schema.ResourceData, meta interface{})
 	}
 
 	// Attributes that require reboot to update
-	if d.HasChange("service_offering") || d.HasChange("keypair") {
+	if d.HasChange("name") || d.HasChange("service_offering") || d.HasChange("keypair") {
 		// Before we can actually make these changes, the virtual machine must be stopped
 		_, err := cs.VirtualMachine.StopVirtualMachine(
 			cs.VirtualMachine.NewStopVirtualMachineParams(d.Id()))
@@ -298,6 +276,26 @@ func resourceCloudStackInstanceUpdate(d *schema.ResourceData, meta interface{})
 				"Error stopping instance %s before making changes: %s", name, err)
 		}
 
+		// Check if the name has changed and if so, update the name
+		if d.HasChange("name") {
+			log.Printf("[DEBUG] Name for %s changed to %s, starting update", d.Id(), name)
+
+			// Create a new parameter struct
+			p := cs.VirtualMachine.NewUpdateVirtualMachineParams(d.Id())
+
+			// Set the new name
+			p.SetName(name)
+
+			// Update the display name
+			_, err := cs.VirtualMachine.UpdateVirtualMachine(p)
+			if err != nil {
+				return fmt.Errorf(
+					"Error updating the name for instance %s: %s", name, err)
+			}
+
+			d.SetPartial("name")
+		}
+
 		// Check if the service offering is changed and if so, update the offering
 		if d.HasChange("service_offering") {
 			log.Printf("[DEBUG] Service offering changed for %s, starting update", name)

builtin/providers/cloudstack/resource_cloudstack_instance_test.go

@@ -56,6 +56,8 @@ func TestAccCloudStackInstance_update(t *testing.T) {
 					testAccCheckCloudStackInstanceExists(
 						"cloudstack_instance.foobar", &instance),
 					testAccCheckCloudStackInstanceRenamedAndResized(&instance),
+					resource.TestCheckResourceAttr(
+						"cloudstack_instance.foobar", "name", "terraform-updated"),
 					resource.TestCheckResourceAttr(
 						"cloudstack_instance.foobar", "display_name", "terraform-updated"),
 					resource.TestCheckResourceAttr(
@@ -166,7 +168,7 @@ func testAccCheckCloudStackInstanceAttributes(
 			return fmt.Errorf("Bad name: %s", instance.Name)
 		}
 
-		if instance.Displayname != "terraform" {
+		if instance.Displayname != "terraform-test" {
 			return fmt.Errorf("Bad display name: %s", instance.Displayname)
 		}
 
@@ -190,6 +192,10 @@ func testAccCheckCloudStackInstanceRenamedAndResized(
 	instance *cloudstack.VirtualMachine) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 
+		if instance.Name != "terraform-updated" {
+			return fmt.Errorf("Bad name: %s", instance.Name)
+		}
+
 		if instance.Displayname != "terraform-updated" {
 			return fmt.Errorf("Bad display name: %s", instance.Displayname)
 		}
@@ -226,7 +232,7 @@ func testAccCheckCloudStackInstanceDestroy(s *terraform.State) error {
 var testAccCloudStackInstance_basic = fmt.Sprintf(`
 resource "cloudstack_instance" "foobar" {
   name = "terraform-test"
-  display_name = "terraform"
+  display_name = "terraform-test"
   service_offering= "%s"
   network = "%s"
   template = "%s"
@@ -241,7 +247,7 @@ resource "cloudstack_instance" "foobar" {
 
 var testAccCloudStackInstance_renameAndResize = fmt.Sprintf(`
 resource "cloudstack_instance" "foobar" {
-  name = "terraform-test"
+  name = "terraform-updated"
   display_name = "terraform-updated"
   service_offering= "%s"
   network = "%s"
@@ -258,7 +264,7 @@ resource "cloudstack_instance" "foobar" {
 var testAccCloudStackInstance_fixedIP = fmt.Sprintf(`
 resource "cloudstack_instance" "foobar" {
   name = "terraform-test"
-  display_name = "terraform"
+  display_name = "terraform-test"
   service_offering= "%s"
   network = "%s"
   ipaddress = "%s"
@@ -279,7 +285,7 @@ resource "cloudstack_ssh_keypair" "foo" {
 
 resource "cloudstack_instance" "foobar" {
   name = "terraform-test"
-  display_name = "terraform"
+  display_name = "terraform-test"
   service_offering= "%s"
   network = "%s"
   ipaddress = "%s"
@@ -297,7 +303,7 @@ resource "cloudstack_instance" "foobar" {
 var testAccCloudStackInstance_project = fmt.Sprintf(`
 resource "cloudstack_instance" "foobar" {
   name = "terraform-test"
-  display_name = "terraform"
+  display_name = "terraform-test"
   service_offering= "%s"
 	network = "%s"
   template = "%s"