Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 10 commits into from
Jun 3, 2022
6 changes: 5 additions & 1 deletion scaleway/resource_instance_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
67 changes: 67 additions & 0 deletions scaleway/resource_instance_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
44 changes: 20 additions & 24 deletions scaleway/resource_instance_volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Loading