fix(instance_server): attach external block volume on update (scaleway#2574)

* fix(instance_server): attach external block volume on update

* update cassettes

* remove unused code

Author: Codelax

internal/services/instance/helpers_instance_block.go

@@ -1,6 +1,8 @@
 package instance
 
 import (
+	"errors"
+
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	block "github.com/scaleway/scaleway-sdk-go/api/block/v1alpha1"
 	"github.com/scaleway/scaleway-sdk-go/api/instance/v1"
@@ -14,6 +16,99 @@ type BlockAndInstanceAPI struct {
 	blockAPI *block.API
 }
 
+type GetUnknownVolumeRequest struct {
+	VolumeID string
+	Zone     scw.Zone
+}
+
+type UnknownVolume struct {
+	Zone     scw.Zone
+	ID       string
+	Name     string
+	Size     scw.Size
+	ServerID *string
+
+	// IsBlockVolume is true if volume is managed by block API
+	IsBlockVolume bool
+
+	InstanceVolumeType instance.VolumeVolumeType
+}
+
+// VolumeTemplateUpdate return a VolumeServerTemplate for an UpdateServer request
+func (volume *UnknownVolume) VolumeTemplateUpdate() *instance.VolumeServerTemplate {
+	template := &instance.VolumeServerTemplate{
+		ID:   scw.StringPtr(volume.ID),
+		Name: &volume.Name, // name is ignored by the API, any name will work here
+	}
+	if volume.IsBlockVolume {
+		template.Name = nil
+		template.VolumeType = volume.InstanceVolumeType
+	}
+
+	return template
+}
+
+// IsLocal returns true if the volume is a local volume
+func (volume *UnknownVolume) IsLocal() bool {
+	return !volume.IsBlockVolume && volume.InstanceVolumeType == instance.VolumeVolumeTypeLSSD
+}
+
+// IsAttached returns true if the volume is attached to a server
+func (volume *UnknownVolume) IsAttached() bool {
+	return volume.ServerID != nil && *volume.ServerID != ""
+}
+
+func (api *BlockAndInstanceAPI) GetUnknownVolume(req *GetUnknownVolumeRequest, opts ...scw.RequestOption) (*UnknownVolume, error) {
+	getVolumeResponse, err := api.API.GetVolume(&instance.GetVolumeRequest{
+		Zone:     req.Zone,
+		VolumeID: req.VolumeID,
+	}, opts...)
+	notFoundErr := &scw.ResourceNotFoundError{}
+	if err != nil && !errors.As(err, &notFoundErr) {
+		return nil, err
+	}
+
+	if getVolumeResponse != nil {
+		vol := &UnknownVolume{
+			Zone:               getVolumeResponse.Volume.Zone,
+			ID:                 getVolumeResponse.Volume.ID,
+			Name:               getVolumeResponse.Volume.Name,
+			Size:               getVolumeResponse.Volume.Size,
+			IsBlockVolume:      false,
+			InstanceVolumeType: getVolumeResponse.Volume.VolumeType,
+		}
+		if getVolumeResponse.Volume.Server != nil {
+			vol.ServerID = &getVolumeResponse.Volume.Server.ID
+		}
+
+		return vol, nil
+	}
+
+	blockVolume, err := api.blockAPI.GetVolume(&block.GetVolumeRequest{
+		Zone:     req.Zone,
+		VolumeID: req.VolumeID,
+	}, opts...)
+	if err != nil {
+		return nil, err
+	}
+
+	vol := &UnknownVolume{
+		Zone:               blockVolume.Zone,
+		ID:                 blockVolume.ID,
+		Name:               blockVolume.Name,
+		Size:               blockVolume.Size,
+		IsBlockVolume:      true,
+		InstanceVolumeType: instance.VolumeVolumeTypeSbsVolume,
+	}
+	for _, ref := range blockVolume.References {
+		if ref.ProductResourceType == "instance_server" {
+			vol.ServerID = &ref.ProductResourceID
+		}
+	}
+
+	return vol, nil
+}
+
 // newAPIWithZone returns a new instance API and the zone for a Create request
 func instanceAndBlockAPIWithZone(d *schema.ResourceData, m interface{}) (*BlockAndInstanceAPI, scw.Zone, error) {
 	instanceAPI := instance.NewAPI(meta.ExtractScwClient(m))

internal/services/instance/server.go

@@ -830,27 +830,19 @@ func ResourceInstanceServerUpdate(ctx context.Context, d *schema.ResourceData, m
 
 		for i, volumeID := range raw.([]interface{}) {
 			volumeHasChange := d.HasChange("additional_volume_ids." + strconv.Itoa(i))
-			// local volumes can only be added when the instanceSDK is stopped
-			if volumeHasChange && !isStopped {
-				volumeResp, err := api.API.GetVolume(&instanceSDK.GetVolumeRequest{
-					Zone:     zone,
-					VolumeID: zonal.ExpandID(volumeID).ID,
-				})
-				if err != nil {
-					return diag.FromErr(fmt.Errorf("failed to get updated volume: %w", err))
-				}
-
-				// 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 == instanceSDK.VolumeVolumeTypeLSSD && volumeResp.Volume.Server.ID != "" {
-						return diag.FromErr(errors.New("instanceSDK must be stopped to change local volumes"))
-					}
-				}
+			volume, err := api.GetUnknownVolume(&GetUnknownVolumeRequest{
+				VolumeID: zonal.ExpandID(volumeID).ID,
+				Zone:     zone,
+			}, scw.WithContext(ctx))
+			if err != nil {
+				return diag.FromErr(fmt.Errorf("failed to get updated volume: %w", err))
 			}
-			volumes[strconv.Itoa(i+1)] = &instanceSDK.VolumeServerTemplate{
-				ID:   scw.StringPtr(zonal.ExpandID(volumeID).ID),
-				Name: scw.StringPtr(types.NewRandomName("vol")), // name is ignored by the API, any name will work here
+
+			// local volumes can only be added when the server is stopped
+			if volumeHasChange && !isStopped && volume.IsLocal() && volume.IsAttached() {
+				return diag.FromErr(errors.New("instanceSDK must be stopped to change local volumes"))
 			}
+			volumes[strconv.Itoa(i+1)] = volume.VolumeTemplateUpdate()
 		}
 
 		serverShouldUpdate = true

internal/services/instance/server_test.go

@@ -1955,6 +1955,24 @@ func TestAccServer_BlockExternal(t *testing.T) {
 					resource.TestCheckResourceAttr("scaleway_instance_server.main", "additional_volume_ids.#", "0"),
 				),
 			},
+			{
+				Config: `
+					resource "scaleway_block_volume" "volume" {
+						iops = 5000
+						size_in_gb = 10
+					}
+
+					resource "scaleway_instance_server" "main" {
+						image = "ubuntu_jammy"
+						type  = "PLAY2-PICO"
+						additional_volume_ids = [scaleway_block_volume.volume.id]
+					}`,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("scaleway_instance_server.main", "type", "PLAY2-PICO"),
+					resource.TestCheckResourceAttr("scaleway_instance_server.main", "additional_volume_ids.#", "1"),
+					resource.TestCheckResourceAttrPair("scaleway_instance_server.main", "additional_volume_ids.0", "scaleway_block_volume.volume", "id"),
+				),
+			},
 		},
 	})
 }