feat(instance): stop rebooting and stop/starting the instance (#708)

Author: Sh4d1

docs/resources/instance_server.md

@@ -143,7 +143,7 @@ to find either the right `label` or the right local image `ID` for a given `type
 - `placement_group_id` - (Optional) The [placement group](https://developers.scaleway.com/en/products/instance/api/#placement-groups-d8f653) the server is attached to.
 
 
-~> **Important:** Updates to `placement_group_id` may trigger a stop/start of the server.
+~> **Important:** When updating `placement_group_id` the `state` must be set to `stopped`, otherwise it will fail.
 
 - `root_volume` - (Optional) Root [volume](https://developers.scaleway.com/en/products/instance/api/#volumes-7e8a39) attached to the server on creation.
     - `size_in_gb` - (Required) Size of the root volume in gigabytes.
@@ -152,12 +152,14 @@ to find either the right `label` or the right local image `ID` for a given `type
     Updates to this field will recreate a new resource.
     - `delete_on_termination` - (Defaults to `true`) Forces deletion of the root volume on instance termination.
 
-~> **Important:** Updates to `root_volume.size_in_gb` will trigger a stop/start of the server.
+~> **Important:** Updates to `root_volume.size_in_gb` will be ignored after the creation of the server.
 
 - `additional_volume_ids` - (Optional) The [additional volumes](https://developers.scaleway.com/en/products/instance/api/#volumes-7e8a39)
 attached to the server. Updates to this field will trigger a stop/start of the server.
 
-~> **Important:** If this field contains local volumes, updates will trigger a stop/start of the server.
+~> **Important:** If this field contains local volumes, the `state` must be set to `stopped`, otherwise it will fail.
+
+~> **Important:** If this field contains local volumes, you have to first detach them, in one apply, and then delete the volume in another apply.
 
 - `enable_ipv6` - (Defaults to `false`) Determines if IPv6 is enabled for the server.
 

scaleway/helpers.go

@@ -1,19 +1,15 @@
 package scaleway
 
 import (
-	"bytes"
 	"encoding/json"
 	"fmt"
 	"net"
 	"net/http"
 	"regexp"
 	"strings"
-	"text/template"
 	"time"
 
-	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
-	"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
 	"github.com/scaleway/scaleway-sdk-go/namegenerator"
 	"github.com/scaleway/scaleway-sdk-go/scw"
 	"golang.org/x/xerrors"
@@ -302,37 +298,6 @@ func isUUID(s string) bool {
 	return UUIDRegex.MatchString(s)
 }
 
-// newTemplateFunc takes a go template string and returns a function that can be called to execute template.
-func newTemplateFunc(tplStr string) func(data interface{}) string {
-	t := template.Must(template.New("tpl").Parse(tplStr))
-	return func(tplParams interface{}) string {
-		buffer := bytes.Buffer{}
-		err := t.Execute(&buffer, tplParams)
-		if err != nil {
-			panic(err) // lintignore:R009
-		}
-		return buffer.String()
-	}
-}
-
-// testAccGetResourceAttr can be used in acceptance tests to extract value from state and store it in dest
-func testAccGetResourceAttr(resourceName string, attrName string, dest *string) resource.TestCheckFunc {
-	return func(state *terraform.State) error {
-		r, exist := state.RootModule().Resources[resourceName]
-		if !exist {
-			return fmt.Errorf("unknown ressource %s", resourceName)
-		}
-
-		a, exist := r.Primary.Attributes[attrName]
-		if !exist {
-			return fmt.Errorf("unknown ressource %s", resourceName)
-		}
-
-		*dest = a
-		return nil
-	}
-}
-
 func flattenTime(date *time.Time) interface{} {
 	if date != nil {
 		return date.Format(time.RFC3339)

scaleway/resource_instance_server.go

@@ -152,12 +152,6 @@ func resourceScalewayInstanceServer() *schema.Resource {
 				Computed:    true,
 				Description: "The IPv6 prefix length routed to the server.",
 			},
-			"disable_dynamic_ip": {
-				Type:        schema.TypeBool,
-				Optional:    true,
-				Default:     false,
-				Description: "Disable dynamic IP on the server",
-			},
 			"enable_dynamic_ip": {
 				Type:        schema.TypeBool,
 				Optional:    true,
@@ -494,8 +488,10 @@ func resourceScalewayInstanceServerUpdate(ctx context.Context, d *schema.Resourc
 		return diag.FromErr(err)
 	}
 
-	// This variable will be set to true if any state change requires a server reboot.
-	var forceReboot bool
+	wantedState := d.Get("state").(string)
+	isStopped := wantedState == InstanceServerStateStopped
+
+	var warnings diag.Diagnostics
 
 	////
 	// Construct UpdateServerRequest
@@ -534,24 +530,36 @@ func resourceScalewayInstanceServerUpdate(ctx context.Context, d *schema.Resourc
 		volumes["0"] = &instance.VolumeTemplate{ID: expandZonedID(d.Get("root_volume.0.volume_id")).ID, Name: newRandomName("vol")} // name is ignored by the API, any name will work here
 
 		for i, volumeID := range raw.([]interface{}) {
-			// TODO: this will be refactored soon, before next release
-			// in the meantime it will throw an error if the volume is already attached somewhere
+			// local volumes can only be added when the instance is stopped
+			if !isStopped {
+				volumeResp, err := instanceAPI.GetVolume(&instance.GetVolumeRequest{
+					Zone:     zone,
+					VolumeID: expandZonedID(volumeID).ID,
+				})
+				if err != nil {
+					return diag.FromErr(err)
+				}
+				if volumeResp.Volume.VolumeType == instance.VolumeVolumeTypeLSSD {
+					return diag.FromErr(fmt.Errorf("instance must be stopped to change local volumes"))
+				}
+			}
 			volumes[strconv.Itoa(i+1)] = &instance.VolumeTemplate{
 				ID:   expandZonedID(volumeID).ID,
 				Name: newRandomName("vol"), // name is ignored by the API, any name will work here
 			}
 		}
 
 		updateRequest.Volumes = &volumes
-		forceReboot = true
 	}
 
 	if d.HasChange("placement_group_id") {
 		placementGroupID := expandZonedID(d.Get("placement_group_id")).ID
 		if placementGroupID == "" {
 			updateRequest.PlacementGroup = &instance.NullableStringValue{Null: true}
 		} else {
-			forceReboot = true
+			if !isStopped {
+				return diag.FromErr(fmt.Errorf("instance must be stopped to change placement group"))
+			}
 			updateRequest.PlacementGroup = &instance.NullableStringValue{Value: placementGroupID}
 		}
 	}
@@ -597,12 +605,22 @@ func resourceScalewayInstanceServerUpdate(ctx context.Context, d *schema.Resourc
 	if d.HasChanges("boot_type") {
 		bootType := instance.BootType(d.Get("boot_type").(string))
 		updateRequest.BootType = &bootType
-		forceReboot = true
+		if !isStopped {
+			warnings = append(warnings, diag.Diagnostic{
+				Severity: diag.Warning,
+				Summary:  "instance may need to be rebooted to use the new boot type",
+			})
+		}
 	}
 
 	if d.HasChanges("bootscript_id") {
 		updateRequest.Bootscript = expandStringPtr(d.Get("bootscript_id").(string))
-		forceReboot = true
+		if !isStopped {
+			warnings = append(warnings, diag.Diagnostic{
+				Severity: diag.Warning,
+				Summary:  "instance may need to be rebooted to use the new bootscript",
+			})
+		}
 	}
 
 	////
@@ -626,7 +644,12 @@ func resourceScalewayInstanceServerUpdate(ctx context.Context, d *schema.Resourc
 		// cloud init script is set in user data
 		if cloudInit, ok := d.GetOk("cloud_init"); ok {
 			userDataRequests.UserData["cloud-init"] = bytes.NewBufferString(cloudInit.(string))
-			forceReboot = true // instance must reboot when cloud init script change
+			if !isStopped {
+				warnings = append(warnings, diag.Diagnostic{
+					Severity: diag.Warning,
+					Summary:  "instance may need to be rebooted to use the new cloud init config",
+				})
+			}
 		}
 
 		err := instanceAPI.SetAllServerUserData(userDataRequests)
@@ -639,29 +662,23 @@ func resourceScalewayInstanceServerUpdate(ctx context.Context, d *schema.Resourc
 	// Apply changes
 	////
 
-	if forceReboot {
-		err = reachState(ctx, instanceAPI, zone, ID, InstanceServerStateStopped)
-		if err != nil {
-			return diag.FromErr(err)
-		}
-	}
-	_, err = instanceAPI.UpdateServer(updateRequest, scw.WithContext(ctx))
+	targetState, err := serverStateExpand(d.Get("state").(string))
 	if err != nil {
 		return diag.FromErr(err)
 	}
 
-	targetState, err := serverStateExpand(d.Get("state").(string))
+	// reach expected state
+	err = reachState(ctx, instanceAPI, zone, ID, targetState)
 	if err != nil {
 		return diag.FromErr(err)
 	}
 
-	// reach expected state
-	err = reachState(ctx, instanceAPI, zone, ID, targetState)
+	_, err = instanceAPI.UpdateServer(updateRequest)
 	if err != nil {
 		return diag.FromErr(err)
 	}
 
-	return resourceScalewayInstanceServerRead(ctx, d, m)
+	return append(warnings, resourceScalewayInstanceServerRead(ctx, d, m)...)
 }
 
 func resourceScalewayInstanceServerDelete(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics {