fix(instance): Fix block volume detach when no additional volumes remaining (#789)

Co-authored-by: Jules Casteran <[email protected]>

Author: remyleone

scaleway/resource_instance_server.go

@@ -677,12 +677,16 @@ func resourceScalewayInstanceServerUpdate(ctx context.Context, d *schema.Resourc
 
 	volumes := map[string]*instance.VolumeServerTemplate{}
 
-	if raw, ok := d.GetOk("additional_volume_ids"); d.HasChange("additional_volume_ids") && ok {
+	if raw, hasAdditionalVolumes := d.GetOk("additional_volume_ids"); d.HasChange("additional_volume_ids") {
 		volumes["0"] = &instance.VolumeServerTemplate{
 			ID:   expandZonedID(d.Get("root_volume.0.volume_id")).ID,
 			Name: newRandomName("vol"), // name is ignored by the API, any name will work here
 		}
 
+		if !hasAdditionalVolumes {
+			raw = []interface{}{} // Set an empty list if not volumes exist
+		}
+
 		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

scaleway/resource_instance_server_test.go

@@ -543,6 +543,73 @@ func TestAccScalewayInstanceServer_AdditionalVolumes(t *testing.T) {
 	})
 }
 
+func TestAccScalewayInstanceServer_AdditionalVolumesDetach(t *testing.T) {
+	tt := NewTestTools(t)
+	defer tt.Cleanup()
+	resource.Test(t, resource.TestCase{
+		PreCheck:          func() { testAccPreCheck(t) },
+		ProviderFactories: tt.ProviderFactories,
+		CheckDestroy: resource.ComposeTestCheckFunc(
+			testAccCheckScalewayInstanceVolumeDestroy(tt),
+			testAccCheckScalewayInstanceServerDestroy(tt),
+		),
+		Steps: []resource.TestStep{
+			{
+				Config: `
+					variable "zone" {
+						type    = string
+						default = "fr-par-1"
+					}
+
+					resource "scaleway_instance_volume" "main" {
+  						type       = "b_ssd"
+  						name       = "foobar"
+  						size_in_gb = 1
+					}
+
+					resource "scaleway_instance_server" "main" {
+						type  = "DEV1-S"
+  						image = "ubuntu_focal"
+  						name  = "foobar"
+
+						enable_dynamic_ip = true
+
+						additional_volume_ids = [scaleway_instance_volume.main.id]
+					}
+				`,
+			},
+			{
+				Config: `
+					variable "zone" {
+						type    = string
+						default = "fr-par-1"
+					}
+
+					resource "scaleway_instance_volume" "main" {
+  						type       = "b_ssd"
+  						name       = "foobar"
+  						size_in_gb = 1
+					}
+
+					resource "scaleway_instance_server" "main" {
+						type  = "DEV1-S"
+  						image = "ubuntu_focal"
+  						name  = "foobar"
+
+						enable_dynamic_ip = true
+
+						additional_volume_ids = []
+					}
+				`,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckScalewayInstanceVolumeExists(tt, "scaleway_instance_volume.main"),
+					resource.TestCheckResourceAttr("scaleway_instance_server.main", "additional_volume_ids.#", "0"),
+				),
+			},
+		},
+	})
+}
+
 func TestAccScalewayInstanceServer_WithPlacementGroup(t *testing.T) {
 	tt := NewTestTools(t)
 	defer tt.Cleanup()

scaleway/resource_instance_volume.go

@@ -5,7 +5,6 @@ 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"
@@ -237,35 +236,32 @@ func resourceScalewayInstanceVolumeDelete(ctx context.Context, d *schema.Resourc
 		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)
+	volume, err := instanceAPI.WaitForVolume(&instance.WaitForVolumeRequest{
+		Zone:          zone,
+		VolumeID:      id,
+		RetryInterval: DefaultWaitRetryInterval,
+		Timeout:       scw.TimeDurationPtr(d.Timeout(schema.TimeoutDelete)),
+	}, scw.WithContext(ctx))
+	if err != nil {
+		if is404Error(err) {
+			return nil
 		}
+		return diag.FromErr(err)
+	}
 
-		if volumeResp.Volume.Server != nil {
-			return resource.RetryableError(fmt.Errorf("volume is still attached to a server"))
-		}
+	if volume.Server != nil {
+		return diag.FromErr(fmt.Errorf("volume is still attached to a server"))
+	}
 
-		deleteRequest := &instance.DeleteVolumeRequest{
-			Zone:     zone,
-			VolumeID: id,
-		}
+	deleteRequest := &instance.DeleteVolumeRequest{
+		Zone:     zone,
+		VolumeID: id,
+	}
 
-		err = instanceAPI.DeleteVolume(deleteRequest, scw.WithContext(ctx))
-		if err != nil {
-			return resource.NonRetryableError(err)
-		}
-		return nil
-	})
+	err = instanceAPI.DeleteVolume(deleteRequest, scw.WithContext(ctx))
 	if err != nil {
 		return diag.FromErr(err)
 	}
+
 	return nil
 }