test(instance): fix TestAccScalewayInstanceServer_AdditionalVolumes (#732)

Author: remyleone

scaleway/resource_instance_server.go

@@ -286,7 +286,8 @@ func resourceScalewayInstanceServerCreate(ctx context.Context, d *schema.Resourc
 	req.Volumes = make(map[string]*instance.VolumeTemplate)
 	if size, ok := d.GetOk("root_volume.0.size_in_gb"); ok {
 		req.Volumes["0"] = &instance.VolumeTemplate{
-			Size: scw.Size(uint64(size.(int)) * gb),
+			Size:       scw.Size(uint64(size.(int)) * gb),
+			VolumeType: instance.VolumeVolumeTypeLSSD,
 		}
 	} else {
 		// We had a local root volume if it is not already present
@@ -310,6 +311,7 @@ func resourceScalewayInstanceServerCreate(ctx context.Context, d *schema.Resourc
 				ID:         vol.Volume.ID,
 				Name:       vol.Volume.Name,
 				VolumeType: vol.Volume.VolumeType,
+				Size:       vol.Volume.Size,
 			}
 		}
 	}
@@ -468,6 +470,7 @@ func resourceScalewayInstanceServerRead(ctx context.Context, d *schema.ResourceD
 
 			rootVolume["volume_id"] = newZonedID(zone, volume.ID).String()
 			rootVolume["size_in_gb"] = int(uint64(volume.Size) / gb)
+			rootVolume["delete_on_termination"] = d.Get("root_volume.0.delete_on_termination")
 
 			_ = d.Set("root_volume", []map[string]interface{}{rootVolume})
 		} else {
@@ -551,20 +554,28 @@ func resourceScalewayInstanceServerUpdate(ctx context.Context, d *schema.Resourc
 	volumes := map[string]*instance.VolumeTemplate{}
 
 	if raw, ok := d.GetOk("additional_volume_ids"); d.HasChange("additional_volume_ids") && ok {
-		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
+		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{}) {
+			volumeHasChange := d.HasChange("additional_volume_ids." + strconv.Itoa(i))
 			// local volumes can only be added when the instance is stopped
-			if !isStopped {
+			if volumeHasChange && !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"))
+
+				// We must be able to tell whether a volume is already present in the server or not
+				if volumeResp.Volume.Server != nil {
+					if volumeResp.Volume.VolumeType == instance.VolumeVolumeTypeLSSD && volumeResp.Volume.Server.ID != "" {
+						return diag.FromErr(fmt.Errorf("instance must be stopped to change local volumes"))
+					}
 				}
 			}
 			volumes[strconv.Itoa(i+1)] = &instance.VolumeTemplate{

scaleway/resource_instance_server_test.go

@@ -2,7 +2,6 @@ package scaleway
 
 import (
 	"fmt"
-	"strings"
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
@@ -494,19 +493,67 @@ func TestAccScalewayInstanceServer_AdditionalVolumes(t *testing.T) {
 		CheckDestroy:      testAccCheckScalewayInstanceServerDestroy(tt),
 		Steps: []resource.TestStep{
 			{
-				Config: testAccCheckScalewayInstanceServerConfigVolumes(false, InstanceServerStateStarted),
+				// With additional local
+				Config: `
+					resource "scaleway_instance_volume" "local" {
+						size_in_gb = 10
+						type = "l_ssd"
+					}
+
+					resource "scaleway_instance_server" "base" {
+						image = "ubuntu_focal"
+						type = "DEV1-S"
+						
+						root_volume {
+							size_in_gb = 10
+						}
+
+						tags = [ "terraform-test", "scaleway_instance_server", "additional_volume_ids" ]
+
+						additional_volume_ids = [
+							scaleway_instance_volume.local.id
+						]
+					}
+				`,
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckScalewayInstanceServerExists(tt, "scaleway_instance_server.base"),
-					resource.TestCheckResourceAttr("scaleway_instance_server.base", "root_volume.0.size_in_gb", "20"),
+					resource.TestCheckResourceAttr("scaleway_instance_server.base", "root_volume.0.size_in_gb", "10"),
 				),
 			},
 			{
-				Config: testAccCheckScalewayInstanceServerConfigVolumes(true, InstanceServerStateStarted),
+				// With additional local and block
+				Config: `
+					resource "scaleway_instance_volume" "local" {
+						size_in_gb = 10
+						type = "l_ssd"
+					}
+
+					resource "scaleway_instance_volume" "block" {
+						size_in_gb = 10
+						type = "b_ssd"
+					}
+
+					resource "scaleway_instance_server" "base" {
+						image = "ubuntu_focal"
+						type = "DEV1-S"
+						
+						root_volume {
+							size_in_gb = 10
+						}
+
+						tags = [ "terraform-test", "scaleway_instance_server", "additional_volume_ids" ]
+
+						additional_volume_ids = [
+							scaleway_instance_volume.local.id,
+							scaleway_instance_volume.block.id
+						]
+					}
+				`,
 				Check: resource.ComposeTestCheckFunc(
-					testAccCheckScalewayInstanceVolumeExists(tt, "scaleway_instance_volume.base_block"),
+					testAccCheckScalewayInstanceVolumeExists(tt, "scaleway_instance_volume.block"),
 					testAccCheckScalewayInstanceServerExists(tt, "scaleway_instance_server.base"),
-					resource.TestCheckResourceAttr("scaleway_instance_volume.base_block", "size_in_gb", "10"),
-					resource.TestCheckResourceAttr("scaleway_instance_server.base", "root_volume.0.size_in_gb", "20"),
+					resource.TestCheckResourceAttr("scaleway_instance_volume.block", "size_in_gb", "10"),
+					resource.TestCheckResourceAttr("scaleway_instance_server.base", "root_volume.0.size_in_gb", "10"),
 				),
 			},
 		},
@@ -757,44 +804,6 @@ func testAccCheckScalewayInstanceServerDestroy(tt *TestTools) resource.TestCheck
 	}
 }
 
-func testAccCheckScalewayInstanceServerConfigVolumes(withBlock bool, state string, localVolumesInGB ...int) string {
-	additionalVolumeResources := ""
-	baseVolume := 20
-	var additionalVolumeIDs []string
-	for i, size := range localVolumesInGB {
-		additionalVolumeResources += fmt.Sprintf(`
-resource "scaleway_instance_volume" "base_volume%d" {
-  size_in_gb = %d
-  type       = "l_ssd"
-}`, i, size)
-		additionalVolumeIDs = append(additionalVolumeIDs, fmt.Sprintf(`"${scaleway_instance_volume.base_volume%d.id}"`, i))
-		baseVolume -= size
-	}
-
-	if withBlock {
-		additionalVolumeResources += `
-resource "scaleway_instance_volume" "base_block" {
-  size_in_gb = 10
-  type       = "b_ssd"
-}`
-		additionalVolumeIDs = append(additionalVolumeIDs, `"${scaleway_instance_volume.base_block.id}"`)
-	}
-	return fmt.Sprintf(`
-%s
-
-resource "scaleway_instance_server" "base" {
-  image = "ubuntu_focal"
-  type  = "DEV1-S"
-  root_volume {
-    size_in_gb = %d
-  }
-  tags = [ "terraform-test", "scaleway_instance_server", "additional_volume_ids" ]
-  state = "%s"
-
-  additional_volume_ids  = [ %s ]
-}`, additionalVolumeResources, baseVolume, state, strings.Join(additionalVolumeIDs, ","))
-}
-
 func TestAccScalewayInstanceServer_Bootscript(t *testing.T) {
 	tt := NewTestTools(t)
 	defer tt.Cleanup()