diff --git a/cloudstack/resource_cloudstack_instance.go b/cloudstack/resource_cloudstack_instance.go index afa50931..9ed78135 100644 --- a/cloudstack/resource_cloudstack_instance.go +++ b/cloudstack/resource_cloudstack_instance.go @@ -692,8 +692,9 @@ func resourceCloudStackInstanceUpdate(d *schema.ResourceData, meta interface{}) } // Attributes that require reboot to update - if d.HasChange("name") || d.HasChange("service_offering") || d.HasChange("affinity_group_ids") || - d.HasChange("affinity_group_names") || d.HasChange("keypair") || d.HasChange("keypairs") || + if d.HasChange("name") || d.HasChange("service_offering") || d.HasChange("details") || + d.HasChange("affinity_group_ids") || d.HasChange("affinity_group_names") || + d.HasChange("keypair") || d.HasChange("keypairs") || d.HasChange("user_data") || d.HasChange("userdata_id") || d.HasChange("userdata_details") { // Before we can actually make these changes, the virtual machine must be stopped @@ -704,234 +705,363 @@ 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) + // Apply each attribute change that requires the VM to be stopped. + // Each helper is a no-op unless its own attribute changed. + if err := updateInstanceName(cs, d, name); err != nil { + return err + } + if err := updateServiceOffering(cs, d, name); err != nil { + return err + } + if err := updateComputeDetails(cs, d, name); err != nil { + return err + } + if err := updateAffinityGroupIds(cs, d, name); err != nil { + return err + } + if err := updateAffinityGroupNames(cs, d, name); err != nil { + return err + } + if err := updateKeypair(cs, d, name); err != nil { + return err + } + if err := updateUserData(cs, d, name); err != nil { + return err + } + if err := updateUserdataId(cs, d, name); err != nil { + return err + } + if err := updateUserdataDetails(cs, d, name); err != nil { + return err + } - // Create a new parameter struct - p := cs.VirtualMachine.NewUpdateVirtualMachineParams(d.Id()) + // Start the virtual machine again + _, err = cs.VirtualMachine.StartVirtualMachine( + cs.VirtualMachine.NewStartVirtualMachineParams(d.Id())) + if err != nil { + return fmt.Errorf( + "Error starting instance %s after making changes", name) + } + } - // Set the new name - p.SetName(name) + if err := updateInstanceTags(cs, d, name); err != nil { + return err + } + if err := updateDeleteProtection(cs, d, name); err != nil { + return err + } - // 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) - } + return resourceCloudStackInstanceRead(d, meta) +} - } +// updateInstanceTags applies changed resource tags to the VM. +func updateInstanceTags(cs *cloudstack.CloudStackClient, d *schema.ResourceData, name string) error { + if !d.HasChange("tags") { + return nil + } - // 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) + if err := updateTags(cs, d, "UserVm"); err != nil { + return fmt.Errorf("Error updating tags on instance %s: %s", name, err) + } - // Retrieve the zone ID first (needed for service_offering lookup) - zoneid, e := retrieveID(cs, "zone", d.Get("zone").(string)) - if e != nil { - return e.Error() - } + return nil +} - // Retrieve the service_offering ID (filtered by zone) - serviceofferingid, e := retrieveServiceOfferingID(cs, zoneid, d.Get("service_offering").(string)) - if e != nil { - return e.Error() - } +// updateDeleteProtection toggles delete protection on the VM. +func updateDeleteProtection(cs *cloudstack.CloudStackClient, d *schema.ResourceData, name string) error { + if !d.HasChange("delete_protection") { + return nil + } - // Create a new parameter struct - p := cs.VirtualMachine.NewChangeServiceForVirtualMachineParams(d.Id(), serviceofferingid) + p := cs.VirtualMachine.NewUpdateVirtualMachineParams(d.Id()) + p.SetDeleteprotection(d.Get("delete_protection").(bool)) - // Change the service offering - _, err = cs.VirtualMachine.ChangeServiceForVirtualMachine(p) - if err != nil { - return fmt.Errorf( - "Error changing the service offering for instance %s: %s", name, err) - } - } + if _, err := cs.VirtualMachine.UpdateVirtualMachine(p); err != nil { + return fmt.Errorf( + "Error updating the delete protection for instance %s: %s", name, err) + } - // Check if the affinity group IDs have changed and if so, update the IDs - if d.HasChange("affinity_group_ids") { - p := cs.AffinityGroup.NewUpdateVMAffinityGroupParams(d.Id()) - groups := []string{} + return nil +} - if agIDs := d.Get("affinity_group_ids").(*schema.Set); agIDs.Len() > 0 { - for _, group := range agIDs.List() { - groups = append(groups, group.(string)) - } - } +// updateInstanceName renames the (stopped) VM when the name attribute changed. +func updateInstanceName(cs *cloudstack.CloudStackClient, d *schema.ResourceData, name string) error { + if !d.HasChange("name") { + return nil + } - // Set the new groups - p.SetAffinitygroupids(groups) + log.Printf("[DEBUG] Name for %s changed to %s, starting update", d.Id(), name) - // Update the affinity groups - _, err = cs.AffinityGroup.UpdateVMAffinityGroup(p) - if err != nil { - return fmt.Errorf( - "Error updating the affinity groups for instance %s: %s", name, err) - } - } + p := cs.VirtualMachine.NewUpdateVirtualMachineParams(d.Id()) + p.SetName(name) - // Check if the affinity group names have changed and if so, update the names - if d.HasChange("affinity_group_names") { - p := cs.AffinityGroup.NewUpdateVMAffinityGroupParams(d.Id()) - groups := []string{} + if _, err := cs.VirtualMachine.UpdateVirtualMachine(p); err != nil { + return fmt.Errorf( + "Error updating the name for instance %s: %s", name, err) + } - if agNames := d.Get("affinity_group_names").(*schema.Set); agNames.Len() > 0 { - for _, group := range agNames.List() { - groups = append(groups, group.(string)) - } - } + return nil +} - // Set the new groups - p.SetAffinitygroupnames(groups) +// updateServiceOffering scales the (stopped) VM to a new service offering. +func updateServiceOffering(cs *cloudstack.CloudStackClient, d *schema.ResourceData, name string) error { + if !d.HasChange("service_offering") { + return nil + } - // Update the affinity groups - _, err = cs.AffinityGroup.UpdateVMAffinityGroup(p) - if err != nil { - return fmt.Errorf( - "Error updating the affinity groups for instance %s: %s", name, err) - } - } + oldOffering, newOffering := d.GetChange("service_offering") + log.Printf("[DEBUG] Service offering changed for %s from %s to %s, starting scale", name, oldOffering, newOffering) - // Check if the keypair has changed and if so, update the keypair - if d.HasChange("keypair") || d.HasChange("keypairs") { - log.Printf("[DEBUG] SSH keypair(s) changed for %s, starting update", name) + // Retrieve the zone ID first (needed for service_offering lookup) + zoneid, e := retrieveID(cs, "zone", d.Get("zone").(string)) + if e != nil { + return e.Error() + } - p := cs.SSH.NewResetSSHKeyForVirtualMachineParams(d.Id()) + // Retrieve the service_offering ID (filtered by zone) + serviceofferingid, e := retrieveServiceOfferingID(cs, zoneid, d.Get("service_offering").(string)) + if e != nil { + return e.Error() + } - if keypair, ok := d.GetOk("keypair"); ok { - p.SetKeypair(keypair.(string)) - } + p := cs.VirtualMachine.NewScaleVirtualMachineParams(d.Id(), serviceofferingid) + if _, err := cs.VirtualMachine.ScaleVirtualMachine(p); err != nil { + return fmt.Errorf( + "Error scaling VM %s from %s to %s: %s", name, oldOffering, newOffering, err) + } - if keypairs, ok := d.GetOk("keypairs"); ok { + return nil +} - // Convert keypairsInterface to []interface{} - keypairsInterfaces := keypairs.([]interface{}) +// updateComputeDetails scales the (stopped) VM when the compute-related details +// (cpuNumber, cpuSpeed, memory) changed, keeping the current service offering. +func updateComputeDetails(cs *cloudstack.CloudStackClient, d *schema.ResourceData, name string) error { + if !d.HasChange("details") { + return nil + } - // Now, safely convert []interface{} to []string with error handling - strKeyPairs := make([]string, len(keypairsInterfaces)) + oldDetails, newDetails := d.GetChange("details") - for i, v := range keypairsInterfaces { - switch v := v.(type) { - case string: - strKeyPairs[i] = v - default: - log.Printf("Value at index %d is not a string: %v", i, v) - continue - } - } - p.SetKeypairs(strKeyPairs) - } + // Safely coerce details, treating nil as empty map + var oldDetailsMap, newDetailsMap map[string]interface{} + if oldDetails != nil { + oldDetailsMap = oldDetails.(map[string]interface{}) + } else { + oldDetailsMap = make(map[string]interface{}) + } + if newDetails != nil { + newDetailsMap = newDetails.(map[string]interface{}) + } else { + newDetailsMap = make(map[string]interface{}) + } - // If there is a project supplied, we retrieve and set the project id - if err := setProjectid(p, cs, d); err != nil { - return err - } - // Change the ssh keypair - _, err = cs.SSH.ResetSSHKeyForVirtualMachine(p) - if err != nil { - return fmt.Errorf( - "Error changing the SSH keypair(s) for instance %s: %s", name, err) - } - } + // Convert details map for API call, safely stringifying values + detailsForAPI := make(map[string]string) + for k, v := range newDetailsMap { + detailsForAPI[k] = fmt.Sprintf("%v", v) + } - // Check if the user data has changed and if so, update the user data - if d.HasChange("user_data") { - log.Printf("[DEBUG] user_data changed for %s, starting update", name) + // Check if any compute-related details changed (cpuNumber, cpuSpeed, memory) + computeDetailsChanged := false + for _, key := range []string{"cpuNumber", "cpuSpeed", "memory"} { + if oldDetailsMap[key] != newDetailsMap[key] { + computeDetailsChanged = true + break + } + } - ud, err := getUserData(d.Get("user_data").(string)) - if err != nil { - return err - } + // Compute-related detail changes must go through ScaleVirtualMachine so + // CPU/memory are actually resized on the (stopped) VM. + if computeDetailsChanged { + log.Printf("[DEBUG] Compute details changed for %s, scaling VM", name) - p := cs.VirtualMachine.NewUpdateVirtualMachineParams(d.Id()) - p.SetUserdata(ud) - _, err = cs.VirtualMachine.UpdateVirtualMachine(p) - if err != nil { - return fmt.Errorf( - "Error updating user_data for instance %s: %s", name, err) - } + // Get current service offering ID for details-only scaling + currentServiceOfferingID, e := retrieveID(cs, "service_offering", d.Get("service_offering").(string)) + if e != nil { + return e.Error() } - if d.HasChange("userdata_id") { - log.Printf("[DEBUG] userdata_id changed for %s, starting update", name) + p := cs.VirtualMachine.NewScaleVirtualMachineParams(d.Id(), currentServiceOfferingID) + p.SetDetails(detailsForAPI) - p := cs.VirtualMachine.NewUpdateVirtualMachineParams(d.Id()) - if userdataID, ok := d.GetOk("userdata_id"); ok { - p.SetUserdataid(userdataID.(string)) - } - _, err := cs.VirtualMachine.UpdateVirtualMachine(p) - if err != nil { - return fmt.Errorf( - "Error updating userdata_id for instance %s: %s", name, err) - } + if _, err := cs.VirtualMachine.ScaleVirtualMachine(p); err != nil { + return fmt.Errorf( + "Error scaling compute resources for instance %s: %s", name, err) } + } - if d.HasChange("userdata_details") { - log.Printf("[DEBUG] userdata_details changed for %s, starting update", name) + // Persist the full details map via UpdateVirtualMachine so non-compute + // detail changes (custom keys) are applied even when no compute key changed. + p := cs.VirtualMachine.NewUpdateVirtualMachineParams(d.Id()) + p.SetDetails(detailsForAPI) - p := cs.VirtualMachine.NewUpdateVirtualMachineParams(d.Id()) - if userdataDetails, ok := d.GetOk("userdata_details"); ok { - ud := make(map[string]string) - for k, v := range userdataDetails.(map[string]interface{}) { - ud[k] = v.(string) - } - p.SetUserdatadetails(ud) - } - _, err := cs.VirtualMachine.UpdateVirtualMachine(p) - if err != nil { - return fmt.Errorf( - "Error updating userdata_details for instance %s: %s", name, err) - } - } + if _, err := cs.VirtualMachine.UpdateVirtualMachine(p); err != nil { + return fmt.Errorf( + "Error updating the details for instance %s: %s", name, err) + } - // Start the virtual machine again - _, err = cs.VirtualMachine.StartVirtualMachine( - cs.VirtualMachine.NewStartVirtualMachineParams(d.Id())) - if err != nil { - return fmt.Errorf( - "Error starting instance %s after making changes", name) + return nil +} + +// updateAffinityGroupIds re-applies the affinity groups by ID for the (stopped) VM. +func updateAffinityGroupIds(cs *cloudstack.CloudStackClient, d *schema.ResourceData, name string) error { + if !d.HasChange("affinity_group_ids") { + return nil + } + + p := cs.AffinityGroup.NewUpdateVMAffinityGroupParams(d.Id()) + groups := []string{} + + if agIDs := d.Get("affinity_group_ids").(*schema.Set); agIDs.Len() > 0 { + for _, group := range agIDs.List() { + groups = append(groups, group.(string)) } } - // Check if the tags have changed and if so, update the tags - if d.HasChange("tags") { - if err := updateTags(cs, d, "UserVm"); err != nil { - return fmt.Errorf("Error updating tags on instance %s: %s", name, err) + p.SetAffinitygroupids(groups) + + if _, err := cs.AffinityGroup.UpdateVMAffinityGroup(p); err != nil { + return fmt.Errorf( + "Error updating the affinity groups for instance %s: %s", name, err) + } + + return nil +} + +// updateAffinityGroupNames re-applies the affinity groups by name for the (stopped) VM. +func updateAffinityGroupNames(cs *cloudstack.CloudStackClient, d *schema.ResourceData, name string) error { + if !d.HasChange("affinity_group_names") { + return nil + } + + p := cs.AffinityGroup.NewUpdateVMAffinityGroupParams(d.Id()) + groups := []string{} + + if agNames := d.Get("affinity_group_names").(*schema.Set); agNames.Len() > 0 { + for _, group := range agNames.List() { + groups = append(groups, group.(string)) } } - // Check if the details have changed and if so, update the details - if d.HasChange("details") { - p := cs.VirtualMachine.NewUpdateVirtualMachineParams(d.Id()) - vmDetails := make(map[string]string) - if details := d.Get("details"); details != nil { - for k, v := range details.(map[string]interface{}) { - vmDetails[k] = v.(string) + p.SetAffinitygroupnames(groups) + + if _, err := cs.AffinityGroup.UpdateVMAffinityGroup(p); err != nil { + return fmt.Errorf( + "Error updating the affinity groups for instance %s: %s", name, err) + } + + return nil +} + +// updateKeypair resets the SSH keypair(s) for the (stopped) VM. +func updateKeypair(cs *cloudstack.CloudStackClient, d *schema.ResourceData, name string) error { + if !d.HasChange("keypair") && !d.HasChange("keypairs") { + return nil + } + + log.Printf("[DEBUG] SSH keypair(s) changed for %s, starting update", name) + + p := cs.SSH.NewResetSSHKeyForVirtualMachineParams(d.Id()) + + if keypair, ok := d.GetOk("keypair"); ok { + p.SetKeypair(keypair.(string)) + } + + if keypairs, ok := d.GetOk("keypairs"); ok { + keypairsInterfaces := keypairs.([]interface{}) + + // Safely convert []interface{} to []string, skipping non-string values + strKeyPairs := make([]string, len(keypairsInterfaces)) + for i, v := range keypairsInterfaces { + switch v := v.(type) { + case string: + strKeyPairs[i] = v + default: + log.Printf("Value at index %d is not a string: %v", i, v) + continue } } - p.SetDetails(vmDetails) - _, err := cs.VirtualMachine.UpdateVirtualMachine(p) - if err != nil { - return fmt.Errorf( - "Error updating the details for instance %s: %s", vmDetails, err) - } + p.SetKeypairs(strKeyPairs) } - // Check if the delete protection has changed and if so, update the deleteprotection - if d.HasChange("delete_protection") { - p := cs.VirtualMachine.NewUpdateVirtualMachineParams(d.Id()) - p.SetDeleteprotection(d.Get("delete_protection").(bool)) + // If there is a project supplied, we retrieve and set the project id + if err := setProjectid(p, cs, d); err != nil { + return err + } - _, err := cs.VirtualMachine.UpdateVirtualMachine(p) - if err != nil { - return fmt.Errorf( - "Error updating the delete protection for instance %s: %s", name, err) + if _, err := cs.SSH.ResetSSHKeyForVirtualMachine(p); err != nil { + return fmt.Errorf( + "Error changing the SSH keypair(s) for instance %s: %s", name, err) + } + + return nil +} + +// updateUserData applies a changed inline user_data to the (stopped) VM. +func updateUserData(cs *cloudstack.CloudStackClient, d *schema.ResourceData, name string) error { + if !d.HasChange("user_data") { + return nil + } + + log.Printf("[DEBUG] user_data changed for %s, starting update", name) + + ud, err := getUserData(d.Get("user_data").(string)) + if err != nil { + return err + } + + p := cs.VirtualMachine.NewUpdateVirtualMachineParams(d.Id()) + p.SetUserdata(ud) + if _, err := cs.VirtualMachine.UpdateVirtualMachine(p); err != nil { + return fmt.Errorf( + "Error updating user_data for instance %s: %s", name, err) + } + + return nil +} + +// updateUserdataId applies a changed userdata_id to the (stopped) VM. +func updateUserdataId(cs *cloudstack.CloudStackClient, d *schema.ResourceData, name string) error { + if !d.HasChange("userdata_id") { + return nil + } + + log.Printf("[DEBUG] userdata_id changed for %s, starting update", name) + + p := cs.VirtualMachine.NewUpdateVirtualMachineParams(d.Id()) + if userdataID, ok := d.GetOk("userdata_id"); ok { + p.SetUserdataid(userdataID.(string)) + } + if _, err := cs.VirtualMachine.UpdateVirtualMachine(p); err != nil { + return fmt.Errorf( + "Error updating userdata_id for instance %s: %s", name, err) + } + + return nil +} + +// updateUserdataDetails applies changed userdata_details to the (stopped) VM. +func updateUserdataDetails(cs *cloudstack.CloudStackClient, d *schema.ResourceData, name string) error { + if !d.HasChange("userdata_details") { + return nil + } + + log.Printf("[DEBUG] userdata_details changed for %s, starting update", name) + + p := cs.VirtualMachine.NewUpdateVirtualMachineParams(d.Id()) + if userdataDetails, ok := d.GetOk("userdata_details"); ok { + ud := make(map[string]string) + for k, v := range userdataDetails.(map[string]interface{}) { + ud[k] = v.(string) } + p.SetUserdatadetails(ud) + } + if _, err := cs.VirtualMachine.UpdateVirtualMachine(p); err != nil { + return fmt.Errorf( + "Error updating userdata_details for instance %s: %s", name, err) } - return resourceCloudStackInstanceRead(d, meta) + return nil } func resourceCloudStackInstanceDelete(d *schema.ResourceData, meta interface{}) error { diff --git a/cloudstack/resource_cloudstack_instance_test.go b/cloudstack/resource_cloudstack_instance_test.go index e298ee90..e4e33817 100644 --- a/cloudstack/resource_cloudstack_instance_test.go +++ b/cloudstack/resource_cloudstack_instance_test.go @@ -357,6 +357,37 @@ func TestAccCloudStackInstance_deleteProtection(t *testing.T) { }) } +func TestAccCloudStackInstance_scale(t *testing.T) { + var instance cloudstack.VirtualMachine + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudStackInstanceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccCloudStackInstance_scale, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackInstanceExists( + "cloudstack_instance.foobar", &instance), + resource.TestCheckResourceAttr( + "cloudstack_instance.foobar", "service_offering", "Small Instance"), + ), + }, + { + Config: testAccCloudStackInstance_scaleUp, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackInstanceExists( + "cloudstack_instance.foobar", &instance), + testAccCheckCloudStackInstanceScaled(&instance), + resource.TestCheckResourceAttr( + "cloudstack_instance.foobar", "service_offering", "Medium Instance"), + ), + }, + }, + }) +} + func testAccCheckCloudStackInstanceExists( n string, instance *cloudstack.VirtualMachine) resource.TestCheckFunc { return func(s *terraform.State) error { @@ -429,6 +460,41 @@ func testAccCheckCloudStackInstanceRenamedAndResized( return fmt.Errorf("Bad service offering: %s", instance.Serviceofferingname) } + // Verify that ScaleVirtualMachine was actually invoked by checking that + // the VM's CPU and memory are set. This ensures the scaling operation + // completed successfully, not just the metadata update. + if instance.Cpunumber <= 0 { + return fmt.Errorf("CPU number not set - VM scaling may not have completed, got: %d", instance.Cpunumber) + } + + if instance.Memory <= 0 { + return fmt.Errorf("Memory not set - VM scaling may not have completed, got: %d", instance.Memory) + } + + return nil + } +} + +func testAccCheckCloudStackInstanceScaled( + instance *cloudstack.VirtualMachine) resource.TestCheckFunc { + return func(s *terraform.State) error { + // Verify that the VM has actually been scaled by checking CPU and memory + // are set to the Medium Instance values. This ensures ScaleVirtualMachine + // was invoked and completed successfully. + if instance.Cpunumber <= 0 { + return fmt.Errorf("CPU number not set after scaling, got: %d", instance.Cpunumber) + } + + if instance.Memory <= 0 { + return fmt.Errorf("Memory not set after scaling, got: %d", instance.Memory) + } + + // Medium Instance should have more resources than Small Instance + // (this is environment-dependent, but at minimum both should be > 0) + if instance.Serviceofferingname != "Medium Instance" { + return fmt.Errorf("Bad service offering after scaling: %s", instance.Serviceofferingname) + } + return nil } } @@ -695,3 +761,41 @@ resource "cloudstack_instance" "foobar" { expunge = true # Note: project is NOT specified here - it should be inherited from the network }` + +const testAccCloudStackInstance_scale = ` +resource "cloudstack_network" "foo" { + name = "terraform-network" + display_text = "terraform-network" + cidr = "10.1.1.0/24" + network_offering = "DefaultIsolatedNetworkOfferingWithSourceNatService" + zone = "Sandbox-simulator" +} + +resource "cloudstack_instance" "foobar" { + name = "terraform-test" + display_name = "terraform-test" + service_offering = "Small Instance" + network_id = cloudstack_network.foo.id + template = "CentOS 5.6 (64-bit) no GUI (Simulator)" + zone = "Sandbox-simulator" + expunge = true +}` + +const testAccCloudStackInstance_scaleUp = ` +resource "cloudstack_network" "foo" { + name = "terraform-network" + display_text = "terraform-network" + cidr = "10.1.1.0/24" + network_offering = "DefaultIsolatedNetworkOfferingWithSourceNatService" + zone = "Sandbox-simulator" +} + +resource "cloudstack_instance" "foobar" { + name = "terraform-test" + display_name = "terraform-test" + service_offering = "Medium Instance" + network_id = cloudstack_network.foo.id + template = "CentOS 5.6 (64-bit) no GUI (Simulator)" + zone = "Sandbox-simulator" + expunge = true +}`