feat(instance): remove lockLocalized and use an retryable error (#688)

Signed-off-by: Patrik Cyvoct <[email protected]>

Author: Sh4d1

scaleway/helpers_instance.go

@@ -18,6 +18,7 @@ const (
 	InstanceServerStateStandby = "standby"
 
 	InstanceServerWaitForTimeout = 10 * time.Minute
+	InstanceVolumeDeleteTimeout  = 10 * time.Minute
 )
 
 // instanceAPIWithZone returns a new instance API and the zone for a Create request
@@ -145,45 +146,3 @@ func reachState(ctx context.Context, instanceAPI *instance.API, zone scw.Zone, s
 	}
 	return nil
 }
-
-// detachVolume will make sure a volume is not attached to any server. If volume is attached to a server, it will be stopped
-// to allow volume detachment.
-func detachVolume(ctx context.Context, instanceAPI *instance.API, zone scw.Zone, volumeID string) error {
-	res, err := instanceAPI.GetVolume(&instance.GetVolumeRequest{
-		Zone:     zone,
-		VolumeID: volumeID,
-	}, scw.WithContext(ctx))
-	if err != nil {
-		return err
-	}
-
-	if res.Volume.Server == nil {
-		return nil
-	}
-
-	defer lockLocalizedID(newZonedIDString(zone, res.Volume.Server.ID))()
-
-	// We need to stop server only for VolumeTypeLSSD volume type
-	if res.Volume.VolumeType == instance.VolumeVolumeTypeLSSD {
-		err = reachState(ctx, instanceAPI, zone, res.Volume.Server.ID, instance.ServerStateStopped)
-
-		// If 404 this mean server is deleted and volume is already detached
-		if is404Error(err) {
-			return nil
-		}
-		if err != nil {
-			return err
-		}
-	}
-	_, err = instanceAPI.DetachVolume(&instance.DetachVolumeRequest{
-		Zone:     zone,
-		VolumeID: res.Volume.ID,
-	}, scw.WithContext(ctx))
-
-	// TODO find a better way to test this error
-	if err != nil && err.Error() != "scaleway-sdk-go: volume should be attached to a server" {
-		return err
-	}
-
-	return nil
-}

scaleway/resource_instance_server.go

@@ -532,11 +532,8 @@ 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{}) {
-			// We make sure volume is detached so we can attach it to the server.
-			err = detachVolume(nil, instanceAPI, zone, expandZonedID(volumeID).ID)
-			if err != nil {
-				return diag.FromErr(err)
-			}
+			// TODO: this will be refactored soon, before next release
+			// in the meantime it will throw an error if the volume is already attached somewhere
 			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
@@ -640,8 +637,6 @@ func resourceScalewayInstanceServerUpdate(ctx context.Context, d *schema.Resourc
 	// Apply changes
 	////
 
-	defer lockLocalizedID(d.Id())()
-
 	if forceReboot {
 		err = reachState(ctx, instanceAPI, zone, ID, InstanceServerStateStopped)
 		if err != nil {
@@ -672,7 +667,6 @@ func resourceScalewayInstanceServerDelete(ctx context.Context, d *schema.Resourc
 	if err != nil {
 		return diag.FromErr(err)
 	}
-	defer lockLocalizedID(d.Id())()
 
 	// reach stopped state
 	err = reachState(ctx, instanceAPI, zone, ID, instance.ServerStateStopped)

scaleway/resource_instance_volume.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
+	"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/helper/validation"
 	"github.com/scaleway/scaleway-sdk-go/api/instance/v1"
@@ -20,6 +21,9 @@ func resourceScalewayInstanceVolume() *schema.Resource {
 		Importer: &schema.ResourceImporter{
 			State: schema.ImportStatePassthrough,
 		},
+		Timeouts: &schema.ResourceTimeout{
+			Delete: scw.TimeDurationPtr(InstanceVolumeDeleteTimeout),
+		},
 
 		Schema: map[string]*schema.Schema{
 			"name": {
@@ -172,17 +176,33 @@ func resourceScalewayInstanceVolumeDelete(ctx context.Context, d *schema.Resourc
 		return diag.FromErr(err)
 	}
 
-	err = detachVolume(ctx, instanceAPI, zone, id)
-	if err != nil {
-		return diag.FromErr(err)
-	}
+	err = resource.Retry(d.Timeout(schema.TimeoutDelete), func() *resource.RetryError {
+		volumeResp, err := instanceAPI.GetVolume(&instance.GetVolumeRequest{
+			Zone:     zone,
+			VolumeID: id,
+		})
+		if err != nil {
+			if is404Error(err) {
+				return nil
+			}
+			return resource.NonRetryableError(err)
+		}
 
-	deleteRequest := &instance.DeleteVolumeRequest{
-		Zone:     zone,
-		VolumeID: id,
-	}
+		if volumeResp.Volume.Server != nil {
+			return resource.RetryableError(fmt.Errorf("volume is still attached to a server"))
+		}
 
-	err = instanceAPI.DeleteVolume(deleteRequest, scw.WithContext(ctx))
+		deleteRequest := &instance.DeleteVolumeRequest{
+			Zone:     zone,
+			VolumeID: id,
+		}
+
+		err = instanceAPI.DeleteVolume(deleteRequest, scw.WithContext(ctx))
+		if err != nil {
+			return resource.NonRetryableError(err)
+		}
+		return nil
+	})
 	if err != nil {
 		return diag.FromErr(err)
 	}