From 63008b857fa059aa3a34411fcb159bc86a5c8284 Mon Sep 17 00:00:00 2001 From: James Nugent Date: Fri, 11 Mar 2016 15:00:10 +0000 Subject: [PATCH 1/2] Make name optional on cloudstack_instance resource This commit matches the behaviour of the cloudstack_instance resource to the documentation at [1], which indicates that the virtual machine name is optional. This also changes the update logic to allow it to be updated as described in [2]. [1] http://cloudstack.apache.org/api/apidocs-4.8/domain_admin/deployVirtualMachine.html [2] http://cloudstack.apache.org/api/apidocs-4.8/domain_admin/updateVirtualMachine.html --- .../resource_cloudstack_instance.go | 32 ++++++++++++++++--- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/builtin/providers/cloudstack/resource_cloudstack_instance.go b/builtin/providers/cloudstack/resource_cloudstack_instance.go index 504a2dbbf..556824799 100644 --- a/builtin/providers/cloudstack/resource_cloudstack_instance.go +++ b/builtin/providers/cloudstack/resource_cloudstack_instance.go @@ -22,7 +22,8 @@ func resourceCloudStackInstance() *schema.Resource { Schema: map[string]*schema.Schema{ "name": &schema.Schema{ Type: schema.TypeString, - Required: true, + Optional: true, + Computed: true, ForceNew: true, }, @@ -128,14 +129,18 @@ func resourceCloudStackInstanceCreate(d *schema.ResourceData, meta interface{}) p := cs.VirtualMachine.NewDeployVirtualMachineParams(serviceofferingid, templateid, zone.Id) // Set the name - name := d.Get("name").(string) - p.SetName(name) + name, hasName := d.GetOk("name") + if hasName { + p.SetName(name.(string)) + } // Set the display name if displayname, ok := d.GetOk("display_name"); ok { p.SetDisplayname(displayname.(string)) } else { - p.SetDisplayname(name) + if hasName { + p.SetDisplayname(name.(string)) + } } if zone.Networktype == "Advanced" { @@ -244,6 +249,25 @@ 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) From 5c1f410ebee1c98440e6b4c0a369d251c94e6383 Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Fri, 25 Mar 2016 16:59:03 +0100 Subject: [PATCH 2/2] Some minor updates and added a proper test --- .../resource_cloudstack_instance.go | 48 +++++++++---------- .../resource_cloudstack_instance_test.go | 18 ++++--- 2 files changed, 35 insertions(+), 31 deletions(-) diff --git a/builtin/providers/cloudstack/resource_cloudstack_instance.go b/builtin/providers/cloudstack/resource_cloudstack_instance.go index 556824799..05898dc23 100644 --- a/builtin/providers/cloudstack/resource_cloudstack_instance.go +++ b/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) diff --git a/builtin/providers/cloudstack/resource_cloudstack_instance_test.go b/builtin/providers/cloudstack/resource_cloudstack_instance_test.go index ced4514be..4d95068c4 100644 --- a/builtin/providers/cloudstack/resource_cloudstack_instance_test.go +++ b/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"