feat(instance): server create with additional volumes sbs (scaleway#4329)

Author: Codelax

internal/namespaces/instance/v1/custom_ip.go

@@ -2,7 +2,6 @@ package instance
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"net"
 	"reflect"
@@ -191,51 +190,3 @@ func ipDetachCommand() *core.Command {
 		},
 	}
 }
-
-func cleanIPs(api *instance.API, zone scw.Zone, ipIDs []string) []error {
-	errs := []error(nil)
-	for _, ipID := range ipIDs {
-		err := api.DeleteIP(&instance.DeleteIPRequest{
-			Zone: zone,
-			IP:   ipID,
-		})
-		if err != nil {
-			errs = append(errs, err)
-		}
-	}
-
-	return errs
-}
-
-func ipIDsFromResponses(resps []*instance.CreateIPResponse) []string {
-	IDs := make([]string, 0, len(resps))
-	for _, resp := range resps {
-		IDs = append(IDs, resp.IP.ID)
-	}
-
-	return IDs
-}
-
-// createIPs will create multiple IPs, if one creation fails, all created IPs will be cleaned up.
-func createIPs(api *instance.API, reqs []*instance.CreateIPRequest, opts ...scw.RequestOption) ([]string, error) {
-	resps := make([]*instance.CreateIPResponse, 0, len(reqs))
-	for _, req := range reqs {
-		resp, err := api.CreateIP(req, opts...)
-		if err != nil {
-			if len(resps) > 0 {
-				errs := cleanIPs(api, resps[0].IP.Zone, ipIDsFromResponses(resps))
-				if len(errs) > 0 {
-					cleanErr := errors.Join(errs...)
-					cleanErr = fmt.Errorf("failed to clean IPs after creation failure: %w", cleanErr)
-					err = fmt.Errorf("%s: %w", cleanErr, err)
-				}
-			}
-
-			return nil, err
-		}
-
-		resps = append(resps, resp)
-	}
-
-	return ipIDsFromResponses(resps), nil
-}

internal/namespaces/instance/v1/custom_server_create.go

@@ -242,25 +242,32 @@ func instanceServerCreateRun(ctx context.Context, argsI interface{}) (i interfac
 		return nil, err
 	}
 
-	createReq, createIPReqs := serverBuilder.Build()
+	apiInstance := instance.NewAPI(client)
+
+	preCreationSetup := serverBuilder.BuildPreCreationSetup()
 	postCreationSetup := serverBuilder.BuildPostCreationSetup()
-	needIPCreation := len(createIPReqs) > 0
 
 	//
-	// IP creation
-	//
-	apiInstance := instance.NewAPI(client)
-
-	if needIPCreation {
-		logger.Debugf("creating IP")
+	// Post server creation setup
+	/// - IPs creation
+	err = preCreationSetup.Execute(ctx)
+	if err != nil {
+		logger.Debugf("failed to create required resources, deleting created resources")
+		cleanErr := preCreationSetup.Clean(ctx)
+		if cleanErr != nil {
+			logger.Warningf("cannot clean created resources: %s.", cleanErr)
+		}
+		return nil, fmt.Errorf("cannot create resource required for server: %s", err)
+	}
 
-		ipIDs, err := createIPs(apiInstance, createIPReqs)
-		if err != nil {
-			return nil, fmt.Errorf("error while creating your public IPs: %s", err)
+	createReq, err := serverBuilder.Build()
+	if err != nil {
+		cleanErr := preCreationSetup.Clean(ctx)
+		if cleanErr != nil {
+			logger.Warningf("cannot clean created resources: %s.", cleanErr)
 		}
 
-		createReq.PublicIPs = scw.StringsPtr(ipIDs)
-		logger.Debugf("IPs created: %s", strings.Join(ipIDs, ", "))
+		return nil, fmt.Errorf("cannot create the server: %s", err)
 	}
 
 	//
@@ -269,14 +276,9 @@ func instanceServerCreateRun(ctx context.Context, argsI interface{}) (i interfac
 	logger.Debugf("creating server")
 	serverRes, err := apiInstance.CreateServer(createReq)
 	if err != nil {
-		if needIPCreation && createReq.PublicIPs != nil {
-			// Delete the created IP
-			formattedIPs := strings.Join(*createReq.PublicIPs, ", ")
-			logger.Debugf("deleting created IPs: %s", formattedIPs)
-			errs := cleanIPs(apiInstance, createReq.Zone, *createReq.PublicIPs)
-			if len(errs) > 0 {
-				logger.Warningf("cannot delete created IPs %s: %s.", formattedIPs, errors.Join(errs...))
-			}
+		cleanErr := preCreationSetup.Clean(ctx)
+		if cleanErr != nil {
+			logger.Warningf("cannot clean created resources: %s.", cleanErr)
 		}
 
 		return nil, fmt.Errorf("cannot create the server: %s", err)

internal/namespaces/instance/v1/custom_server_create_builder.go

@@ -118,22 +118,12 @@ func (sb *ServerBuilder) isWindows() bool {
 	return commercialTypeIsWindowsServer(sb.createReq.CommercialType)
 }
 
-func (sb *ServerBuilder) rootVolumeTemplate() *instance.VolumeServerTemplate {
-	rootVolume, exists := sb.createReq.Volumes["0"]
-	if !exists {
-		return nil
-	}
-
-	return rootVolume
-}
-
 func (sb *ServerBuilder) rootVolumeIsSBS() bool {
-	rootVolume := sb.rootVolumeTemplate()
-	if rootVolume == nil {
+	if sb.rootVolume == nil {
 		return false
 	}
 
-	return rootVolume.VolumeType == instance.VolumeVolumeTypeSbsVolume
+	return sb.rootVolume.VolumeType == instance.VolumeVolumeTypeSbsVolume
 }
 
 func (sb *ServerBuilder) marketplaceImageType() marketplace.LocalImageType {
@@ -237,14 +227,22 @@ func (sb *ServerBuilder) AddIP(ip string) (*ServerBuilder, error) {
 	return sb, nil
 }
 
+func (sb *ServerBuilder) addIPID(ipID string) *ServerBuilder {
+	if sb.createReq.PublicIPs == nil {
+		sb.createReq.PublicIPs = new([]string)
+	}
+
+	*sb.createReq.PublicIPs = append(*sb.createReq.PublicIPs, ipID)
+
+	return sb
+}
+
 // AddVolumes build volume templates from arguments.
 //
 // More format details in buildVolumeTemplate function.
 //
 // Also add default volumes to server, ex: scratch storage for GPU servers
 func (sb *ServerBuilder) AddVolumes(rootVolume string, additionalVolumes []string) (*ServerBuilder, error) {
-	var err error
-
 	if len(additionalVolumes) > 0 || rootVolume != "" {
 		if rootVolume != "" {
 			rootVolumeBuilder, err := NewVolumeBuilder(sb.createReq.Zone, rootVolume)
@@ -260,29 +258,6 @@ func (sb *ServerBuilder) AddVolumes(rootVolume string, additionalVolumes []strin
 			}
 			sb.volumes = append(sb.volumes, additionalVolumeBuilder)
 		}
-
-		volumes := make(map[string]*instance.VolumeServerTemplate, len(sb.volumes)+1)
-		if sb.rootVolume != nil {
-			volumes["0"], err = sb.rootVolume.BuildVolumeServerTemplate(sb.apiInstance, sb.apiBlock)
-			if err != nil {
-				return sb, fmt.Errorf("failed to build root volume: %w", err)
-			}
-		}
-		for i, volume := range sb.volumes {
-			volumeTemplate, err := volume.BuildVolumeServerTemplate(sb.apiInstance, sb.apiBlock)
-			if err != nil {
-				return sb, fmt.Errorf("failed to build volume template: %w", err)
-			}
-			index := strconv.Itoa(i + 1)
-			volumeTemplate.Name = scw.StringPtr(sb.createReq.Name + "-" + index)
-			volumes[index] = volumeTemplate
-		}
-		// Sanitize the volume map to respect API schemas
-		sb.createReq.Volumes = volumes
-	}
-
-	if sb.serverType != nil {
-		sb.createReq.Volumes = addDefaultVolumes(sb.serverType, sb.createReq.Volumes)
 	}
 
 	return sb, nil
@@ -360,11 +335,148 @@ func (sb *ServerBuilder) Validate() error {
 		logger.Warningf("skipping image server-type compatibility validation")
 	}
 
-	return sb.ValidateVolumes()
+	return nil
 }
 
-func (sb *ServerBuilder) Build() (*instance.CreateServerRequest, []*instance.CreateIPRequest) {
-	return sb.createReq, sb.createIPReqs
+func (sb *ServerBuilder) BuildVolumes() error {
+	var err error
+
+	volumes := make(map[string]*instance.VolumeServerTemplate, len(sb.volumes)+1)
+	if sb.rootVolume != nil {
+		volumes["0"], err = sb.rootVolume.BuildVolumeServerTemplate(sb.apiInstance, sb.apiBlock)
+		if err != nil {
+			return fmt.Errorf("failed to build root volume: %w", err)
+		}
+	}
+
+	for i, volume := range sb.volumes {
+		volumeTemplate, err := volume.BuildVolumeServerTemplate(sb.apiInstance, sb.apiBlock)
+		if err != nil {
+			return fmt.Errorf("failed to build volume template: %w", err)
+		}
+		index := strconv.Itoa(i + 1)
+		volumeTemplate.Name = scw.StringPtr(sb.createReq.Name + "-" + index)
+		volumes[index] = volumeTemplate
+	}
+	// Sanitize the volume map to respect API schemas
+	sb.createReq.Volumes = volumes
+
+	if sb.serverType != nil {
+		sb.createReq.Volumes = addDefaultVolumes(sb.serverType, sb.createReq.Volumes)
+	}
+
+	return nil
+}
+
+func (sb *ServerBuilder) Build() (*instance.CreateServerRequest, error) {
+	err := sb.BuildVolumes()
+	if err != nil {
+		return nil, err
+	}
+
+	return sb.createReq, sb.ValidateVolumes()
+}
+
+type PreServerCreationSetupFunc func(ctx context.Context) error
+
+type PreServerCreationSetup struct {
+	setupFunctions []PreServerCreationSetupFunc
+	cleanFunctions []PreServerCreationSetupFunc
+}
+
+func (sb *ServerBuilder) BuildPreCreationSetup() *PreServerCreationSetup {
+	setup := &PreServerCreationSetup{}
+
+	for _, ipCreationRequest := range sb.createIPReqs {
+		setup.setupFunctions = append(setup.setupFunctions, func(ctx context.Context) error {
+			resp, err := sb.apiInstance.CreateIP(ipCreationRequest, scw.WithContext(ctx))
+			if err != nil {
+				return err
+			}
+
+			sb.addIPID(resp.IP.ID)
+
+			setup.cleanFunctions = append(setup.cleanFunctions, func(ctx context.Context) error {
+				return sb.apiInstance.DeleteIP(&instance.DeleteIPRequest{
+					IP:   resp.IP.ID,
+					Zone: resp.IP.Zone,
+				}, scw.WithContext(ctx))
+			})
+
+			return nil
+		})
+	}
+
+	sb.BuildPreCreationVolumesSetup(setup)
+
+	return setup
+}
+
+// BuildPreCreationVolumesSetup configure PreServerCreationSetup to create required SBS volumes.
+// Instance API does not support SBS volumes creation alongside the server, they must be created before then imported.
+func (sb *ServerBuilder) BuildPreCreationVolumesSetup(setup *PreServerCreationSetup) {
+	for _, volume := range sb.volumes {
+		if volume.VolumeType != instance.VolumeVolumeTypeSbsVolume || volume.VolumeID != nil || volume.Size == nil {
+			continue
+		}
+
+		projectID := "" // If let empty, ProjectID will be set by scaleway client to default Project ID.
+		if sb.createReq.Project != nil {
+			projectID = *sb.createReq.Project
+		}
+
+		setup.setupFunctions = append(setup.setupFunctions, func(ctx context.Context) error {
+			vol, err := sb.apiBlock.CreateVolume(&block.CreateVolumeRequest{
+				Zone:      volume.Zone,
+				Name:      core.GetRandomName("vol"),
+				PerfIops:  volume.IOPS,
+				ProjectID: projectID,
+				FromEmpty: &block.CreateVolumeRequestFromEmpty{
+					Size: *volume.Size,
+				},
+			}, scw.WithContext(ctx))
+			if err != nil {
+				return err
+			}
+
+			volume.VolumeID = &vol.ID
+
+			setup.cleanFunctions = append(setup.cleanFunctions, func(ctx context.Context) error {
+				return sb.apiBlock.DeleteVolume(&block.DeleteVolumeRequest{
+					Zone:     vol.Zone,
+					VolumeID: vol.ID,
+				}, scw.WithContext(ctx))
+			})
+
+			return nil
+		})
+	}
+}
+
+func (s *PreServerCreationSetup) Execute(ctx context.Context) error {
+	for _, setupFunc := range s.setupFunctions {
+		if err := setupFunc(ctx); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+func (s *PreServerCreationSetup) Clean(ctx context.Context) error {
+	errs := []error(nil)
+
+	for _, cleanFunc := range s.cleanFunctions {
+		if err := cleanFunc(ctx); err != nil {
+			errs = append(errs, err)
+		}
+	}
+
+	if len(errs) > 0 {
+		return errors.Join(errs...)
+	}
+
+	return nil
 }
 
 type PostServerCreationSetupFunc func(ctx context.Context, server *instance.Server) error

internal/namespaces/instance/v1/custom_server_create_test.go

@@ -238,7 +238,7 @@ func Test_CreateServer(t *testing.T) {
 			AfterFunc: deleteServerAfterFunc(),
 		}))
 
-		t.Run("sbs additional volumes", core.Test(&core.TestConfig{
+		t.Run("sbs additional volumes from id", core.Test(&core.TestConfig{
 			Commands: core.NewCommandsMerge(
 				instance.GetCommands(),
 				block.GetCommands(),
@@ -260,6 +260,25 @@ func Test_CreateServer(t *testing.T) {
 			),
 		}))
 
+		t.Run("sbs additional volumes", core.Test(&core.TestConfig{
+			Commands: core.NewCommandsMerge(
+				instance.GetCommands(),
+				block.GetCommands(),
+			),
+			Cmd: testServerCommand("image=ubuntu_jammy additional-volumes.0=sbs:20G stopped=true"),
+			Check: core.TestCheckCombine(
+				core.TestCheckExitCode(0),
+				func(t *testing.T, ctx *core.CheckFuncCtx) {
+					t.Helper()
+					assert.NotNil(t, ctx.Result)
+					assert.Equal(t, instanceSDK.VolumeServerVolumeTypeSbsVolume, ctx.Result.(*instanceSDK.Server).Volumes["1"].VolumeType)
+				},
+			),
+			AfterFunc: core.AfterFuncCombine(
+				deleteServerAfterFunc(),
+			),
+		}))
+
 		t.Run("use sbs root volume", core.Test(&core.TestConfig{
 			Commands: core.NewCommandsMerge(
 				instance.GetCommands(),