Copilot commented on code in PR #323:
URL:
https://github.com/apache/cloudstack-terraform-provider/pull/323#discussion_r3796347629
##########
cloudstack/resource_cloudstack_instance.go:
##########
@@ -681,24 +681,60 @@ func resourceCloudStackInstanceUpdate(d
*schema.ResourceData, meta interface{})
}
- // Check if the service offering is changed and if so, update
the offering
+ // Check if the service offering is changed and if so, scale
the VM
if d.HasChange("service_offering") {
- log.Printf("[DEBUG] Service offering changed for %s,
starting update", name)
+ oldOffering, newOffering :=
d.GetChange("service_offering")
+ log.Printf("[DEBUG] Service offering changed for %s
from %s to %s, starting scale", name, oldOffering, newOffering)
// Retrieve the service_offering ID
serviceofferingid, e := retrieveID(cs,
"service_offering", d.Get("service_offering").(string))
if e != nil {
return e.Error()
}
- // Create a new parameter struct
- p :=
cs.VirtualMachine.NewChangeServiceForVirtualMachineParams(d.Id(),
serviceofferingid)
+ // Create a new parameter struct for scaling
+ p :=
cs.VirtualMachine.NewScaleVirtualMachineParams(d.Id(), serviceofferingid)
- // Change the service offering
- _, err =
cs.VirtualMachine.ChangeServiceForVirtualMachine(p)
+ // Scale the VM to the new service offering
+ _, err = cs.VirtualMachine.ScaleVirtualMachine(p)
if err != nil {
return fmt.Errorf(
- "Error changing the service offering
for instance %s: %s", name, err)
+ "Error scaling instance %s to service
offering %s: %s", name, newOffering, err)
+ }
+ }
+
+ // Check if compute-related details have changed and scale the
VM
+ if d.HasChange("details") {
+ oldDetails, newDetails := d.GetChange("details")
+ oldDetailsMap := oldDetails.(map[string]interface{})
+ newDetailsMap := newDetails.(map[string]interface{})
Review Comment:
`d.GetChange("details")` can return `nil` for the old/new value (e.g., when
the map is added/removed). The direct type assertions to
`map[string]interface{}` will panic in that case. Coerce the values safely and
treat `nil` as an empty map.
This issue also appears on line 725 of the same file.
##########
cloudstack/resource_cloudstack_instance.go:
##########
@@ -681,24 +681,60 @@ func resourceCloudStackInstanceUpdate(d
*schema.ResourceData, meta interface{})
}
- // Check if the service offering is changed and if so, update
the offering
+ // Check if the service offering is changed and if so, scale
the VM
if d.HasChange("service_offering") {
- log.Printf("[DEBUG] Service offering changed for %s,
starting update", name)
+ oldOffering, newOffering :=
d.GetChange("service_offering")
+ log.Printf("[DEBUG] Service offering changed for %s
from %s to %s, starting scale", name, oldOffering, newOffering)
// Retrieve the service_offering ID
serviceofferingid, e := retrieveID(cs,
"service_offering", d.Get("service_offering").(string))
if e != nil {
return e.Error()
}
- // Create a new parameter struct
- p :=
cs.VirtualMachine.NewChangeServiceForVirtualMachineParams(d.Id(),
serviceofferingid)
+ // Create a new parameter struct for scaling
+ p :=
cs.VirtualMachine.NewScaleVirtualMachineParams(d.Id(), serviceofferingid)
- // Change the service offering
- _, err =
cs.VirtualMachine.ChangeServiceForVirtualMachine(p)
+ // Scale the VM to the new service offering
+ _, err = cs.VirtualMachine.ScaleVirtualMachine(p)
if err != nil {
return fmt.Errorf(
- "Error changing the service offering
for instance %s: %s", name, err)
+ "Error scaling instance %s to service
offering %s: %s", name, newOffering, err)
+ }
+ }
+
+ // Check if compute-related details have changed and scale the
VM
+ if d.HasChange("details") {
+ oldDetails, newDetails := d.GetChange("details")
+ oldDetailsMap := oldDetails.(map[string]interface{})
+ newDetailsMap := newDetails.(map[string]interface{})
+
+ // 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
+ }
+ }
+
+ if computeDetailsChanged {
+ log.Printf("[DEBUG] Compute details changed for
%s, scaling VM", name)
+
+ // Convert details map for API call
+ detailsForAPI := make(map[string]string)
+ for k, v := range newDetailsMap {
+ detailsForAPI[k] = v.(string)
+ }
+
+ p :=
cs.VirtualMachine.NewScaleVirtualMachineParams(d.Id(), "")
+ p.SetDetails(detailsForAPI)
Review Comment:
This scaling call constructs params with an empty service offering ID
(`NewScaleVirtualMachineParams(d.Id(), "")`). In the CloudStack API/client this
parameter is required, so this will likely fail validation or behave
unexpectedly. Resolve the current service offering ID (or reuse the one from
the service_offering change) and pass it into the ScaleVirtualMachine params.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
