feat(instance): improve volume deletion on server delete

cmd/scw/testdata/test-all-usage-instance-server-delete-usage.stderr.golden

@@ -11,11 +11,11 @@ EXAMPLES:
     scw instance server delete zone=fr-par-1 server-id=11111111-1111-1111-1111-111111111111
 
 ARGS:
-  [zone]             Zone to target. If none is passed will use default zone from the config
-  server-id          
-  [delete-ip]        Delete the IP attached to the server as well
-  [delete-volumes]   Delete the volumes attached to the server as well
-  [force-shutdown]   Force shutdown of the instance server before deleting it
+  [zone]                Zone to target. If none is passed will use default zone from the config
+  server-id             
+  [with-volumes=none]   Delete the volumes attached to the server (none | local | block | root | all)
+  [with-ip]             Delete the IP attached to the server
+  [force-shutdown]      Force shutdown of the instance server before deleting it
 
 FLAGS:
   -h, --help   help for delete

internal/namespaces/instance/v1/custom_server.go

@@ -13,8 +13,10 @@ import (
 	"github.com/hashicorp/go-multierror"
 	"github.com/scaleway/scaleway-cli/internal/core"
 	"github.com/scaleway/scaleway-cli/internal/human"
+	"github.com/scaleway/scaleway-cli/internal/interactive"
 	"github.com/scaleway/scaleway-cli/internal/terminal"
 	"github.com/scaleway/scaleway-sdk-go/api/instance/v1"
+	"github.com/scaleway/scaleway-sdk-go/logger"
 	"github.com/scaleway/scaleway-sdk-go/scw"
 )
 
@@ -530,8 +532,8 @@ func getRunServerAction(action instance.ServerAction) core.CommandRunner {
 type customDeleteServerRequest struct {
 	Zone          scw.Zone
 	ServerID      string
-	DeleteIP      bool
-	DeleteVolumes bool
+	WithVolumes   string
+	WithIP        bool
 	ForceShutdown bool
 }
 
@@ -550,12 +552,20 @@ func serverDeleteCommand() *core.Command {
 				Required: true,
 			},
 			{
-				Name:  "delete-ip",
-				Short: "Delete the IP attached to the server as well",
+				Name:    "with-volumes",
+				Short:   "Delete the volumes attached to the server",
+				Default: core.DefaultValueSetter("none"),
+				EnumValues: []string{
+					"none",
+					"local",
+					"block",
+					"root",
+					"all",
+				},
 			},
 			{
-				Name:  "delete-volumes",
-				Short: "Delete the volumes attached to the server as well",
+				Name:  "with-ip",
+				Short: "Delete the IP attached to the server",
 			},
 			{
 				Name:  "force-shutdown",
@@ -579,32 +589,32 @@ func serverDeleteCommand() *core.Command {
 			},
 		},
 		Run: func(ctx context.Context, argsI interface{}) (interface{}, error) {
-			args := argsI.(*customDeleteServerRequest)
+			deleteServerRequest := argsI.(*customDeleteServerRequest)
 
 			client := core.ExtractClient(ctx)
 			api := instance.NewAPI(client)
 
 			server, err := api.GetServer(&instance.GetServerRequest{
-				Zone:     args.Zone,
-				ServerID: args.ServerID,
+				Zone:     deleteServerRequest.Zone,
+				ServerID: deleteServerRequest.ServerID,
 			})
 			if err != nil {
 				return nil, err
 			}
 
-			if args.ForceShutdown {
+			if deleteServerRequest.ForceShutdown {
 				finalStateServer, err := api.WaitForServer(&instance.WaitForServerRequest{
-					Zone:     args.Zone,
-					ServerID: args.ServerID,
+					Zone:     deleteServerRequest.Zone,
+					ServerID: deleteServerRequest.ServerID,
 				})
 				if err != nil {
 					return nil, err
 				}
 
 				if finalStateServer.State != instance.ServerStateStopped {
 					err = api.ServerActionAndWait(&instance.ServerActionAndWaitRequest{
-						Zone:     args.Zone,
-						ServerID: args.ServerID,
+						Zone:     deleteServerRequest.Zone,
+						ServerID: deleteServerRequest.ServerID,
 						Action:   instance.ServerActionPoweroff,
 					})
 					if err != nil {
@@ -614,33 +624,53 @@ func serverDeleteCommand() *core.Command {
 			}
 
 			err = api.DeleteServer(&instance.DeleteServerRequest{
-				Zone:     args.Zone,
-				ServerID: args.ServerID,
+				Zone:     deleteServerRequest.Zone,
+				ServerID: deleteServerRequest.ServerID,
 			})
 			if err != nil {
 				return nil, err
 			}
 
 			var multiErr error
-			if args.DeleteIP && server.Server.PublicIP != nil && !server.Server.PublicIP.Dynamic {
+			if deleteServerRequest.WithIP && server.Server.PublicIP != nil && !server.Server.PublicIP.Dynamic {
 				err = api.DeleteIP(&instance.DeleteIPRequest{
-					Zone: args.Zone,
+					Zone: deleteServerRequest.Zone,
 					IP:   server.Server.PublicIP.ID,
 				})
 				if err != nil {
 					multiErr = multierror.Append(multiErr, err)
+				} else {
+					_, _ = interactive.Printf("successfully deleted ip %s\n", server.Server.PublicIP.Address.String())
 				}
 			}
 
-			if args.DeleteVolumes {
-				for _, volume := range server.Server.Volumes {
-					err = api.DeleteVolume(&instance.DeleteVolumeRequest{
-						Zone:     args.Zone,
-						VolumeID: volume.ID,
-					})
+			deletedVolumeMessages := [][2]string(nil)
+			for index, volume := range server.Server.Volumes {
+				switch {
+				case deleteServerRequest.WithVolumes == "none":
+					break
+				case deleteServerRequest.WithVolumes == "root" && index != "0":
+					continue
+				case deleteServerRequest.WithVolumes == "local" && volume.VolumeType != instance.VolumeTypeLSSD:
+					continue
+				case deleteServerRequest.WithVolumes == "block" && volume.VolumeType != instance.VolumeTypeBSSD:
+					continue
+				}
+				err = api.DeleteVolume(&instance.DeleteVolumeRequest{
+					Zone:     deleteServerRequest.Zone,
+					VolumeID: volume.ID,
+				})
+				if err != nil {
+					multiErr = multierror.Append(multiErr, err)
+				} else {
+					humanSize, err := human.Marshal(volume.Size, nil)
 					if err != nil {
-						multiErr = multierror.Append(multiErr, err)
+						logger.Debugf("cannot marshal human size %v", volume.Size)
 					}
+					deletedVolumeMessages = append(deletedVolumeMessages, [2]string{
+						index,
+						fmt.Sprintf("successfully deleted volume %s (%s %s)", volume.Name, humanSize, volume.VolumeType),
+					})
 				}
 			}
 			if multiErr != nil {
@@ -650,6 +680,14 @@ func serverDeleteCommand() *core.Command {
 				}
 			}
 
+			// Sort and print deleted volume messages
+			sort.Slice(deletedVolumeMessages, func(i, j int) bool {
+				return deletedVolumeMessages[i][0] < deletedVolumeMessages[j][0]
+			})
+			for _, message := range deletedVolumeMessages {
+				_, _ = interactive.Println(message[1])
+			}
+
 			return &core.SuccessResult{}, nil
 		},
 	}

internal/namespaces/instance/v1/custom_server_create_test.go

@@ -11,7 +11,7 @@ import (
 
 // deleteServerAfterFunc deletes the created server and its attached volumes and IPs.
 func deleteServerAfterFunc(ctx *core.AfterFuncCtx) error {
-	ctx.ExecuteCmd("scw instance server delete delete-volumes delete-ip force-shutdown server-id=" + ctx.CmdResult.(*instance.Server).ID)
+	ctx.ExecuteCmd("scw instance server delete with-volumes=all with-ip force-shutdown server-id=" + ctx.CmdResult.(*instance.Server).ID)
 	return nil
 }
 

internal/namespaces/instance/v1/custom_server_test.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/alecthomas/assert"
 	"github.com/scaleway/scaleway-cli/internal/core"
+	"github.com/scaleway/scaleway-cli/internal/interactive"
 	"github.com/scaleway/scaleway-sdk-go/api/instance/v1"
 	"github.com/scaleway/scaleway-sdk-go/scw"
 	"github.com/stretchr/testify/require"
@@ -229,3 +230,41 @@ func Test_ServerUpdateCustom(t *testing.T) {
 		}))
 	})
 }
+
+func Test_ServerDelete(t *testing.T) {
+	interactive.IsInteractive = true
+
+	t.Run("with all volumes", core.Test(&core.TestConfig{
+		Commands:   GetCommands(),
+		BeforeFunc: core.ExecStoreBeforeCmd("Server", "scw instance server create stopped=true image=ubuntu-bionic additional-volumes.0=block:10G"),
+		Cmd:        `scw instance server delete server-id={{ .Server.ID }} with-ip=true with-volumes=all`,
+		Check: core.TestCheckCombine(
+			core.TestCheckGolden(),
+			core.TestCheckExitCode(0),
+		),
+	}))
+
+	t.Run("only block volumes", core.Test(&core.TestConfig{
+		Commands:   GetCommands(),
+		BeforeFunc: core.ExecStoreBeforeCmd("Server", "scw instance server create stopped=true image=ubuntu-bionic additional-volumes.0=block:10G"),
+		Cmd:        `scw instance server delete server-id={{ .Server.ID }} with-ip=true with-volumes=block`,
+		Check: core.TestCheckCombine(
+			core.TestCheckGolden(),
+			core.TestCheckExitCode(0),
+		),
+		AfterFunc: core.ExecAfterCmd(`scw instance delete volume volume-id={{ (index .Server.Volumes "0").ID }}`),
+	}))
+
+	t.Run("only local volumes", core.Test(&core.TestConfig{
+		Commands:   GetCommands(),
+		BeforeFunc: core.ExecStoreBeforeCmd("Server", "scw instance server create stopped=true image=ubuntu-bionic additional-volumes.0=block:10G"),
+		Cmd:        `scw instance server delete server-id={{ .Server.ID }} with-ip=true with-volumes=local`,
+		Check: core.TestCheckCombine(
+			core.TestCheckGolden(),
+			core.TestCheckExitCode(0),
+		),
+		AfterFunc: core.ExecAfterCmd(`scw instance delete volume volume-id={{ (index .Server.Volumes "1").ID }}`),
+	}))
+
+	interactive.IsInteractive = false
+}

internal/namespaces/instance/v1/helpers_test.go

@@ -20,7 +20,7 @@ func createServer(metaKey string) core.BeforeFunc {
 // deleteServer deletes a server and its attached IP and volumes
 // previously registered in the context Meta at metaKey.
 func deleteServer(metaKey string) core.AfterFunc {
-	return core.ExecAfterCmd("scw instance server delete server-id={{ ." + metaKey + ".ID }} delete-ip=true delete-volumes=true")
+	return core.ExecAfterCmd("scw instance server delete server-id={{ ." + metaKey + ".ID }} with-ip=true with-volumes=all")
 }
 
 //