From 6037568c94e795b599e850714545336bd1cc938a Mon Sep 17 00:00:00 2001 From: Stijn Simons Date: Thu, 17 Sep 2026 08:39:02 +0200 Subject: [PATCH] Restart instance to update secgroup --- cloudstack/resource_cloudstack_instance.go | 122 ++++++++++++++++++--- website/docs/r/instance.html.markdown | 8 +- 2 files changed, 113 insertions(+), 17 deletions(-) diff --git a/cloudstack/resource_cloudstack_instance.go b/cloudstack/resource_cloudstack_instance.go index afa50931..87946cba 100644 --- a/cloudstack/resource_cloudstack_instance.go +++ b/cloudstack/resource_cloudstack_instance.go @@ -126,7 +126,6 @@ func resourceCloudStackInstance() *schema.Resource { "security_group_ids": { Type: schema.TypeSet, Optional: true, - ForceNew: true, Elem: &schema.Schema{Type: schema.TypeString}, Set: schema.HashString, ConflictsWith: []string{"security_group_names"}, @@ -135,7 +134,6 @@ func resourceCloudStackInstance() *schema.Resource { "security_group_names": { Type: schema.TypeSet, Optional: true, - ForceNew: true, Elem: &schema.Schema{Type: schema.TypeString}, Set: schema.HashString, ConflictsWith: []string{"security_group_ids"}, @@ -647,7 +645,66 @@ func resourceCloudStackInstanceRead(d *schema.ResourceData, meta interface{}) er return nil } -func resourceCloudStackInstanceUpdate(d *schema.ResourceData, meta interface{}) error { +func stopInstanceForUpdate( + cs *cloudstack.CloudStackClient, + id, project, name string, +) (bool, error) { + vm, count, err := cs.VirtualMachine.GetVirtualMachineByID( + id, + cloudstack.WithProject(project), + ) + if count == 0 && project == "" { + vm, count, err = cs.VirtualMachine.GetVirtualMachineByID( + id, + cloudstack.WithProject("-1"), + ) + } + if err != nil { + return false, fmt.Errorf("Error reading instance %s before making changes: %w", name, err) + } + if count == 0 { + return false, fmt.Errorf("Error reading instance %s before making changes: instance not found", name) + } + + if strings.EqualFold(vm.State, "Stopped") { + return false, nil + } + if !strings.EqualFold(vm.State, "Running") { + return false, fmt.Errorf( + "Error updating instance %s: instance must be Running or Stopped, currently %s", + name, vm.State) + } + + _, err = cs.VirtualMachine.StopVirtualMachine( + cs.VirtualMachine.NewStopVirtualMachineParams(id)) + if err != nil { + return false, fmt.Errorf( + "Error stopping instance %s before making changes: %w", name, err) + } + + return true, nil +} + +func restartInstanceAfterUpdate( + cs *cloudstack.CloudStackClient, + id, name string, + updateErr error, +) error { + _, restartErr := cs.VirtualMachine.StartVirtualMachine( + cs.VirtualMachine.NewStartVirtualMachineParams(id)) + if restartErr == nil { + return updateErr + } + if updateErr != nil { + return fmt.Errorf( + "%w; additionally failed to restart instance %s: %v", + updateErr, name, restartErr) + } + return fmt.Errorf( + "Error starting instance %s after making changes: %w", name, restartErr) +} + +func resourceCloudStackInstanceUpdate(d *schema.ResourceData, meta interface{}) (retErr error) { cs := meta.(*cloudstack.CloudStackClient) @@ -693,15 +750,20 @@ 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") || + d.HasChange("affinity_group_names") || d.HasChange("security_group_ids") || + d.HasChange("security_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 - _, err := cs.VirtualMachine.StopVirtualMachine( - cs.VirtualMachine.NewStopVirtualMachineParams(d.Id())) + restartNeeded, err := stopInstanceForUpdate(cs, d.Id(), d.Get("project").(string), name) if err != nil { - return fmt.Errorf( - "Error stopping instance %s before making changes: %s", name, err) + return err + } + if restartNeeded { + defer func() { + if restartNeeded { + retErr = restartInstanceAfterUpdate(cs, d.Id(), name, retErr) + } + }() } // Check if the name has changed and if so, update the name @@ -794,6 +856,35 @@ func resourceCloudStackInstanceUpdate(d *schema.ResourceData, meta interface{}) } } + // Update security groups once, even when switching between names and IDs. + if d.HasChange("security_group_ids") || d.HasChange("security_group_names") { + p := cs.VirtualMachine.NewUpdateVirtualMachineParams(d.Id()) + ids := d.Get("security_group_ids").(*schema.Set) + names := d.Get("security_group_names").(*schema.Set) + + groups := names + useIDs := ids.Len() > 0 || (names.Len() == 0 && d.HasChange("security_group_ids")) + if useIDs { + groups = ids + } + + values := make([]string, 0, groups.Len()) + for _, group := range groups.List() { + values = append(values, group.(string)) + } + + if useIDs { + p.SetSecuritygroupids(values) + } else { + p.SetSecuritygroupnames(values) + } + + _, err = cs.VirtualMachine.UpdateVirtualMachine(p) + if err != nil { + return fmt.Errorf("Error updating the security groups for instance %s: %w", name, err) + } + } + // 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) @@ -886,13 +977,14 @@ func resourceCloudStackInstanceUpdate(d *schema.ResourceData, meta interface{}) } } - // 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) + if restartNeeded { + restartErr := restartInstanceAfterUpdate(cs, d.Id(), name, nil) + restartNeeded = false + if restartErr != nil { + return restartErr + } } + } // Check if the tags have changed and if so, update the tags diff --git a/website/docs/r/instance.html.markdown b/website/docs/r/instance.html.markdown index bae07f85..d856136a 100644 --- a/website/docs/r/instance.html.markdown +++ b/website/docs/r/instance.html.markdown @@ -181,10 +181,14 @@ The following arguments are supported: this instance. * `security_group_ids` - (Optional) List of security group IDs to apply to this - instance. Changing this forces a new resource to be created. + instance. Changing this temporarily stops a running instance, updates its + security groups, and starts it again. An instance that is already stopped + remains stopped. * `security_group_names` - (Optional) List of security group names to apply to - this instance. Changing this forces a new resource to be created. + this instance. Changing this temporarily stops a running instance, updates + its security groups, and starts it again. An instance that is already + stopped remains stopped. * `project` - (Optional) The name or ID of the project to deploy this instance to. Changing this forces a new resource to be created. If not