feat(instance): support more IP options in server creation (scaleway#4219)

Author: Codelax

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

@@ -40,7 +40,8 @@ ARGS:
   [name=<generated>]                       Server name
   [root-volume]                            Local root volume of the server
   [additional-volumes.{index}]             Additional local and block volumes attached to your server
-  [ip=new]                                 Either an IP, an IP ID, 'new' to create a new IP, 'dynamic' to use a dynamic IP or 'none' for no public IP (new | dynamic | none | <id> | <address>)
+  [ip=new]                                 Either an IP, an IP ID, ('new', 'ipv4', 'ipv6' or 'both') to create new IPs, 'dynamic' to use a dynamic IP or 'none' for no public IP (new | ipv4 | ipv6 | both | dynamic | none | <id> | <address>)
+  [dynamic-ip-required]                    Define if a dynamic IPv4 is required for the Instance. If server has no IPv4, a dynamic one will be allocated.
   [tags.{index}]                           Server tags
   [ipv6]                                   Enable IPv6, to be used with routed-ip-enabled=false
   [stopped]                                Do not start server after its creation

docs/commands/instance.md

@@ -1713,7 +1713,8 @@ scw instance server create [arg=value ...]
 | name | Default: `<generated>` | Server name |
 | root-volume |  | Local root volume of the server |
 | additional-volumes.{index} |  | Additional local and block volumes attached to your server |
-| ip | Default: `new` | Either an IP, an IP ID, 'new' to create a new IP, 'dynamic' to use a dynamic IP or 'none' for no public IP (new | dynamic | none | <id> | <address>) |
+| ip | Default: `new` | Either an IP, an IP ID, ('new', 'ipv4', 'ipv6' or 'both') to create new IPs, 'dynamic' to use a dynamic IP or 'none' for no public IP (new | ipv4 | ipv6 | both | dynamic | none | <id> | <address>) |
+| dynamic-ip-required |  | Define if a dynamic IPv4 is required for the Instance. If server has no IPv4, a dynamic one will be allocated. |
 | tags.{index} |  | Server tags |
 | ipv6 |  | Enable IPv6, to be used with routed-ip-enabled=false |
 | stopped |  | Do not start server after its creation |

internal/namespaces/instance/v1/custom_ip.go

@@ -2,6 +2,7 @@ package instance
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net"
 	"reflect"
@@ -190,3 +191,51 @@ 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

@@ -26,6 +26,7 @@ type instanceCreateServerRequest struct {
 	RootVolume        string
 	AdditionalVolumes []string
 	IP                string
+	DynamicIPRequired *bool
 	Tags              []string
 	IPv6              bool
 	Stopped           bool
@@ -89,9 +90,13 @@ func serverCreateCommand() *core.Command {
 			},
 			{
 				Name:    "ip",
-				Short:   `Either an IP, an IP ID, 'new' to create a new IP, 'dynamic' to use a dynamic IP or 'none' for no public IP (new | dynamic | none | <id> | <address>)`,
+				Short:   `Either an IP, an IP ID, ('new', 'ipv4', 'ipv6' or 'both') to create new IPs, 'dynamic' to use a dynamic IP or 'none' for no public IP (new | ipv4 | ipv6 | both | dynamic | none | <id> | <address>)`,
 				Default: core.DefaultValueSetter("new"),
 			},
+			{
+				Name:  "dynamic-ip-required",
+				Short: "Define if a dynamic IPv4 is required for the Instance. If server has no IPv4, a dynamic one will be allocated.",
+			},
 			{
 				Name:  "tags.{index}",
 				Short: "Server tags",
@@ -211,6 +216,7 @@ func instanceServerCreateRun(ctx context.Context, argsI interface{}) (i interfac
 		AddEnableIPv6(scw.BoolPtr(args.IPv6)).
 		AddTags(args.Tags).
 		AddRoutedIPEnabled(args.RoutedIPEnabled).
+		AddDynamicIPRequired(args.DynamicIPRequired).
 		AddAdminPasswordEncryptionSSHKeyID(args.AdminPasswordEncryptionSSHKeyID).
 		AddBootType(args.BootType).
 		AddSecurityGroup(args.SecurityGroupID).
@@ -240,9 +246,9 @@ func instanceServerCreateRun(ctx context.Context, argsI interface{}) (i interfac
 		return nil, err
 	}
 
-	createReq, createIPReq := serverBuilder.Build()
+	createReq, createIPReqs := serverBuilder.Build()
 	postCreationSetup := serverBuilder.BuildPostCreationSetup()
-	needIPCreation := createIPReq != nil
+	needIPCreation := len(createIPReqs) > 0
 
 	//
 	// IP creation
@@ -252,12 +258,13 @@ func instanceServerCreateRun(ctx context.Context, argsI interface{}) (i interfac
 	if needIPCreation {
 		logger.Debugf("creating IP")
 
-		ipRes, err := apiInstance.CreateIP(createIPReq)
+		ipIDs, err := createIPs(apiInstance, createIPReqs)
 		if err != nil {
-			return nil, fmt.Errorf("error while creating your public IP: %s", err)
+			return nil, fmt.Errorf("error while creating your public IPs: %s", err)
 		}
-		createReq.PublicIP = scw.StringPtr(ipRes.IP.ID)
-		logger.Debugf("IP created: %s", createReq.PublicIP)
+
+		createReq.PublicIPs = scw.StringsPtr(ipIDs)
+		logger.Debugf("IPs created: %s", strings.Join(ipIDs, ", "))
 	}
 
 	//
@@ -266,15 +273,13 @@ func instanceServerCreateRun(ctx context.Context, argsI interface{}) (i interfac
 	logger.Debugf("creating server")
 	serverRes, err := apiInstance.CreateServer(createReq)
 	if err != nil {
-		if needIPCreation && createReq.PublicIP != nil {
+		if needIPCreation && createReq.PublicIPs != nil {
 			// Delete the created IP
-			logger.Debugf("deleting created IP: %s", createReq.PublicIP)
-			err := apiInstance.DeleteIP(&instance.DeleteIPRequest{
-				Zone: args.Zone,
-				IP:   *createReq.PublicIP,
-			})
-			if err != nil {
-				logger.Warningf("cannot delete the create IP %s: %s.", createReq.PublicIP, err)
+			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...))
 			}
 		}
 

internal/namespaces/instance/v1/custom_server_create_builder.go

@@ -21,8 +21,8 @@ import (
 type ServerBuilder struct {
 	// createdReq is the request being built
 	createReq *instance.CreateServerRequest
-	// createIPReq is filled with a request if an IP is needed
-	createIPReq *instance.CreateIPRequest
+	// createIPReqs is filled with requests if one or more IP are needed
+	createIPReqs []*instance.CreateIPRequest
 
 	// volumes is the list of requested volumes
 	volumes []*VolumeBuilder
@@ -98,6 +98,14 @@ func (sb *ServerBuilder) AddRoutedIPEnabled(routedIPEnabled *bool) *ServerBuilde
 	return sb
 }
 
+func (sb *ServerBuilder) AddDynamicIPRequired(dynamicIPRequired *bool) *ServerBuilder {
+	if dynamicIPRequired != nil {
+		sb.createReq.DynamicIPRequired = dynamicIPRequired
+	}
+
+	return sb
+}
+
 func (sb *ServerBuilder) AddAdminPasswordEncryptionSSHKeyID(adminPasswordEncryptionSSHKeyID *string) *ServerBuilder {
 	if adminPasswordEncryptionSSHKeyID != nil {
 		sb.createReq.AdminPasswordEncryptionSSHKeyID = adminPasswordEncryptionSSHKeyID
@@ -128,18 +136,6 @@ func (sb *ServerBuilder) rootVolumeIsSBS() bool {
 	return rootVolume.VolumeType == instance.VolumeVolumeTypeSbsVolume
 }
 
-// defaultIPType returns the default IP type when created by the CLI. Used for ServerBuilder.AddIP
-func (sb *ServerBuilder) defaultIPType() instance.IPType {
-	if sb.createReq.RoutedIPEnabled != nil { //nolint: staticcheck // Field is deprecated but still supported
-		if *sb.createReq.RoutedIPEnabled { //nolint: staticcheck // Field is deprecated but still supported
-			return instance.IPTypeRoutedIPv4
-		}
-		return instance.IPTypeNat
-	}
-
-	return ""
-}
-
 func (sb *ServerBuilder) marketplaceImageType() marketplace.LocalImageType {
 	if sb.rootVolumeIsSBS() {
 		return marketplace.LocalImageTypeInstanceSbs
@@ -195,12 +191,28 @@ func (sb *ServerBuilder) AddImage(image string) (*ServerBuilder, error) {
 //   - "none"
 func (sb *ServerBuilder) AddIP(ip string) (*ServerBuilder, error) {
 	switch {
-	case ip == "" || ip == "new":
-		sb.createIPReq = &instance.CreateIPRequest{
+	case ip == "" || ip == "new" || ip == "ipv4":
+		sb.createIPReqs = []*instance.CreateIPRequest{{
 			Zone:    sb.createReq.Zone,
 			Project: sb.createReq.Project,
-			Type:    sb.defaultIPType(),
-		}
+			Type:    instance.IPTypeRoutedIPv4,
+		}}
+	case ip == "ipv6":
+		sb.createIPReqs = []*instance.CreateIPRequest{{
+			Zone:    sb.createReq.Zone,
+			Project: sb.createReq.Project,
+			Type:    instance.IPTypeRoutedIPv6,
+		}}
+	case ip == "both":
+		sb.createIPReqs = []*instance.CreateIPRequest{{
+			Zone:    sb.createReq.Zone,
+			Project: sb.createReq.Project,
+			Type:    instance.IPTypeRoutedIPv4,
+		}, {
+			Zone:    sb.createReq.Zone,
+			Project: sb.createReq.Project,
+			Type:    instance.IPTypeRoutedIPv6,
+		}}
 	case validation.IsUUID(ip):
 		sb.createReq.PublicIP = scw.StringPtr(ip)
 	case net.ParseIP(ip) != nil:
@@ -219,7 +231,7 @@ func (sb *ServerBuilder) AddIP(ip string) (*ServerBuilder, error) {
 	case ip == "none":
 		sb.createReq.DynamicIPRequired = scw.BoolPtr(false)
 	default:
-		return sb, fmt.Errorf(`invalid IP "%s", should be either 'new', 'dynamic', 'none', an IP address ID or a reserved flexible IP address`, ip)
+		return sb, fmt.Errorf(`invalid IP "%s", should be either 'new', 'ipv4', 'ipv6', 'both', 'dynamic', 'none', an IP address ID or a reserved flexible IP address`, ip)
 	}
 
 	return sb, nil
@@ -351,8 +363,8 @@ func (sb *ServerBuilder) Validate() error {
 	return sb.ValidateVolumes()
 }
 
-func (sb *ServerBuilder) Build() (*instance.CreateServerRequest, *instance.CreateIPRequest) {
-	return sb.createReq, sb.createIPReq
+func (sb *ServerBuilder) Build() (*instance.CreateServerRequest, []*instance.CreateIPRequest) {
+	return sb.createReq, sb.createIPReqs
 }
 
 type PostServerCreationSetupFunc func(ctx context.Context, server *instance.Server) error

internal/namespaces/instance/v1/custom_server_create_test.go

@@ -402,6 +402,41 @@ func Test_CreateServer(t *testing.T) {
 			),
 			AfterFunc: deleteServerAfterFunc(),
 		}))
+
+		t.Run("with ipv6 and dynamic ip", core.Test(&core.TestConfig{
+			Commands: instance.GetCommands(),
+			Cmd:      "scw instance server create image=ubuntu_bionic dynamic-ip-required=true ip=ipv6 -w", // IPv6 is created at runtime
+			Check: core.TestCheckCombine(
+				core.TestCheckExitCode(0),
+				func(t *testing.T, ctx *core.CheckFuncCtx) {
+					t.Helper()
+					assert.NotNil(t, ctx.Result, "server is nil")
+					server := ctx.Result.(*instanceSDK.Server)
+					assert.Len(t, server.PublicIPs, 2)
+					assert.Equal(t, instanceSDK.ServerIPIPFamilyInet, server.PublicIPs[0].Family)
+					assert.True(t, server.PublicIPs[0].Dynamic)
+					assert.Equal(t, instanceSDK.ServerIPIPFamilyInet6, server.PublicIPs[1].Family)
+				},
+			),
+			AfterFunc: deleteServerAfterFunc(),
+		}))
+
+		t.Run("with ipv6 and ipv4", core.Test(&core.TestConfig{
+			Commands: instance.GetCommands(),
+			Cmd:      "scw instance server create image=ubuntu_bionic ip=both -w", // IPv6 is created at runtime
+			Check: core.TestCheckCombine(
+				core.TestCheckExitCode(0),
+				func(t *testing.T, ctx *core.CheckFuncCtx) {
+					t.Helper()
+					assert.NotNil(t, ctx.Result, "server is nil")
+					server := ctx.Result.(*instanceSDK.Server)
+					assert.Len(t, server.PublicIPs, 2)
+					assert.Equal(t, instanceSDK.ServerIPIPFamilyInet, server.PublicIPs[0].Family)
+					assert.Equal(t, instanceSDK.ServerIPIPFamilyInet6, server.PublicIPs[1].Family)
+				},
+			),
+			AfterFunc: deleteServerAfterFunc(),
+		}))
 	})
 }