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

refactor(instance_server): use instance_block helpers #2636

Merged
merged 5 commits into from
Jun 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 42 additions & 68 deletions internal/services/instance/helpers_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,51 +254,6 @@ func validateLocalVolumeSizes(volumes map[string]*instance.VolumeServerTemplate,
return nil
}

// sanitizeVolumeMap removes extra data for API validation.
//
// On the api side, there are two possibles validation schemas for volumes and the validator will be chosen dynamically depending on the passed JSON request
// - With an image (in that case the root volume can be skipped because it is taken from the image)
// - Without an image (in that case, the root volume must be defined)
func sanitizeVolumeMap(volumes map[string]*instance.VolumeServerTemplate) map[string]*instance.VolumeServerTemplate {
m := make(map[string]*instance.VolumeServerTemplate)

for index, v := range volumes {
// Remove extra data for API validation.
switch {
// If a volume already got an ID it is passed as it to the API without specifying the volume type.
// TODO: Fix once instance accept volume type in the schema validation
case v.ID != nil:
if strings.HasPrefix(string(v.VolumeType), "sbs") {
// If volume is from SBS api, the type must be passed
// This rules come from instance API and may not be documented
v = &instance.VolumeServerTemplate{
ID: v.ID,
Boot: v.Boot,
VolumeType: v.VolumeType,
}
} else {
v = &instance.VolumeServerTemplate{
ID: v.ID,
Name: v.Name,
Boot: v.Boot,
}
}
// For the root volume (index 0) if the size is 0, it is considered as a volume created from an image.
// The size is not passed to the API, so it's computed by the API
case index == "0" && v.Size == nil:
v = &instance.VolumeServerTemplate{
VolumeType: v.VolumeType,
Boot: v.Boot,
}
// If none of the above conditions are met, the volume is passed as it to the API
default:
}
m[index] = v
}

return m
}

func preparePrivateNIC(
ctx context.Context, data interface{},
server *instance.Server, vpcAPI *vpc.API,
Expand Down Expand Up @@ -535,34 +490,53 @@ func instanceIPHasMigrated(d *schema.ResourceData) bool {
}

func instanceServerAdditionalVolumeTemplate(api *BlockAndInstanceAPI, zone scw.Zone, volumeID string) (*instance.VolumeServerTemplate, error) {
vol, err := api.GetVolume(&instance.GetVolumeRequest{
Zone: zone,
vol, err := api.GetUnknownVolume(&GetUnknownVolumeRequest{
VolumeID: locality.ExpandID(volumeID),
Zone: zone,
})
if err == nil {
return &instance.VolumeServerTemplate{
ID: &vol.Volume.ID,
Name: &vol.Volume.Name,
VolumeType: vol.Volume.VolumeType,
Size: &vol.Volume.Size,
}, nil
}
if !httperrors.Is404(err) {
if err != nil {
return nil, err
}
return vol.VolumeTemplate(), nil
}

func prepareRootVolume(rootVolumeI map[string]any, serverType *instance.ServerType, image string) *UnknownVolume {
serverTypeCanBootOnBlock := serverType.VolumesConstraint.MaxSize == 0

rootVolumeIsBootVolume := types.ExpandBoolPtr(types.GetMapValue[bool](rootVolumeI, "boot"))
rootVolumeType := types.GetMapValue[string](rootVolumeI, "volume_type")
sizeInput := types.GetMapValue[int](rootVolumeI, "size_in_gb")
rootVolumeID := zonal.ExpandID(types.GetMapValue[string](rootVolumeI, "volume_id")).ID

// If the rootVolumeType is not defined, define it depending on the offer
if rootVolumeType == "" {
if serverTypeCanBootOnBlock {
rootVolumeType = instance.VolumeVolumeTypeBSSD.String()
} else {
rootVolumeType = instance.VolumeVolumeTypeLSSD.String()
}
}

blockVol, err := api.blockAPI.GetVolume(&blockSDK.GetVolumeRequest{
Zone: zone,
VolumeID: locality.ExpandID(volumeID),
})
if err == nil {
return &instance.VolumeServerTemplate{
ID: &blockVol.ID,
Name: &blockVol.Name,
VolumeType: "sbs_volume",
Size: &blockVol.Size,
}, nil
rootVolumeName := ""
if image == "" { // When creating an instance from an image, volume should not have a name
rootVolumeName = types.NewRandomName("vol")
}

return nil, err
var rootVolumeSize *scw.Size
if sizeInput == 0 && rootVolumeType == instance.VolumeVolumeTypeLSSD.String() {
// Compute the rootVolumeSize so it will be valid against the local volume constraints
// It wouldn't be valid if another local volume is added, but in this case
// the user would be informed that it does not fulfill the local volume constraints
rootVolumeSize = scw.SizePtr(serverType.VolumesConstraint.MaxSize)
} else if sizeInput > 0 {
rootVolumeSize = scw.SizePtr(scw.Size(uint64(sizeInput) * gb))
}

return &UnknownVolume{
Name: rootVolumeName,
ID: rootVolumeID,
InstanceVolumeType: instance.VolumeVolumeType(rootVolumeType),
Size: rootVolumeSize,
Boot: rootVolumeIsBootVolume,
}
}
27 changes: 18 additions & 9 deletions internal/services/instance/helpers_instance_block.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,24 +25,33 @@ type UnknownVolume struct {
Zone scw.Zone
ID string
Name string
Size scw.Size
Size *scw.Size
ServerID *string
Boot *bool

// IsBlockVolume is true if volume is managed by block API
IsBlockVolume bool

InstanceVolumeType instance.VolumeVolumeType
}

// VolumeTemplateUpdate return a VolumeServerTemplate for an UpdateServer request
func (volume *UnknownVolume) VolumeTemplateUpdate() *instance.VolumeServerTemplate {
template := &instance.VolumeServerTemplate{
ID: scw.StringPtr(volume.ID),
Name: &volume.Name, // name is ignored by the API, any name will work here
// VolumeTemplate returns a template to be used for servers requests.
func (volume *UnknownVolume) VolumeTemplate() *instance.VolumeServerTemplate {
template := &instance.VolumeServerTemplate{}
if volume.ID != "" {
template.ID = &volume.ID
} else {
template.Size = volume.Size
}

if volume.Boot != nil {
template.Boot = volume.Boot
}

if volume.IsBlockVolume {
template.Name = nil
template.VolumeType = volume.InstanceVolumeType
} else {
template.Name = &volume.Name
}

return template
Expand Down Expand Up @@ -73,7 +82,7 @@ func (api *BlockAndInstanceAPI) GetUnknownVolume(req *GetUnknownVolumeRequest, o
Zone: getVolumeResponse.Volume.Zone,
ID: getVolumeResponse.Volume.ID,
Name: getVolumeResponse.Volume.Name,
Size: getVolumeResponse.Volume.Size,
Size: &getVolumeResponse.Volume.Size,
IsBlockVolume: false,
InstanceVolumeType: getVolumeResponse.Volume.VolumeType,
}
Expand All @@ -96,7 +105,7 @@ func (api *BlockAndInstanceAPI) GetUnknownVolume(req *GetUnknownVolumeRequest, o
Zone: blockVolume.Zone,
ID: blockVolume.ID,
Name: blockVolume.Name,
Size: blockVolume.Size,
Size: &blockVolume.Size,
IsBlockVolume: true,
InstanceVolumeType: instance.VolumeVolumeTypeSbsVolume,
}
Expand Down
43 changes: 3 additions & 40 deletions internal/services/instance/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -430,43 +430,9 @@ func ResourceInstanceServerCreate(ctx context.Context, d *schema.ResourceData, m
}

req.Volumes = make(map[string]*instanceSDK.VolumeServerTemplate)
serverTypeCanBootOnBlock := serverType.VolumesConstraint.MaxSize == 0
rootVolumeIsBootVolume := types.ExpandBoolPtr(d.Get("root_volume.0.boot"))
rootVolumeType := d.Get("root_volume.0.volume_type").(string)
sizeInput := d.Get("root_volume.0.size_in_gb").(int)
rootVolumeID := zonal.ExpandID(d.Get("root_volume.0.volume_id").(string)).ID

// If the rootVolumeType is not defined, define it depending on the offer
if rootVolumeType == "" {
if serverTypeCanBootOnBlock {
rootVolumeType = instanceSDK.VolumeVolumeTypeBSSD.String()
} else {
rootVolumeType = instanceSDK.VolumeVolumeTypeLSSD.String()
}
}

rootVolumeName := ""
if req.Image == "" { // When creating an instanceSDK from an image, volume should not have a name
rootVolumeName = types.NewRandomName("vol")
}
rootVolume := d.Get("root_volume.0").(map[string]any)

var rootVolumeSize *scw.Size
if sizeInput == 0 && rootVolumeType == instanceSDK.VolumeVolumeTypeLSSD.String() {
// Compute the rootVolumeSize so it will be valid against the local volume constraints
// It wouldn't be valid if another local volume is added, but in this case
// the user would be informed that it does not fulfill the local volume constraints
rootVolumeSize = scw.SizePtr(serverType.VolumesConstraint.MaxSize)
} else if sizeInput > 0 {
rootVolumeSize = scw.SizePtr(scw.Size(uint64(sizeInput) * gb))
}

req.Volumes["0"] = &instanceSDK.VolumeServerTemplate{
Name: types.ExpandStringPtr(rootVolumeName),
ID: types.ExpandStringPtr(rootVolumeID),
VolumeType: instanceSDK.VolumeVolumeType(rootVolumeType),
Size: rootVolumeSize,
Boot: rootVolumeIsBootVolume,
}
req.Volumes["0"] = prepareRootVolume(rootVolume, serverType, req.Image).VolumeTemplate()
if raw, ok := d.GetOk("additional_volume_ids"); ok {
for i, volumeID := range raw.([]interface{}) {
// We have to get the volume to know whether it is a local or a block volume
Expand All @@ -483,9 +449,6 @@ func ResourceInstanceServerCreate(ctx context.Context, d *schema.ResourceData, m
return diag.FromErr(err)
}

// Sanitize the volume map to respect API schemas
req.Volumes = sanitizeVolumeMap(req.Volumes)

res, err := api.CreateServer(req, scw.WithContext(ctx))
if err != nil {
return diag.FromErr(err)
Expand Down Expand Up @@ -837,7 +800,7 @@ func ResourceInstanceServerUpdate(ctx context.Context, d *schema.ResourceData, m
if volumeHasChange && !isStopped && volume.IsLocal() && volume.IsAttached() {
return diag.FromErr(errors.New("instanceSDK must be stopped to change local volumes"))
}
volumes[strconv.Itoa(i+1)] = volume.VolumeTemplateUpdate()
volumes[strconv.Itoa(i+1)] = volume.VolumeTemplate()
}

serverShouldUpdate = true
Expand Down
11 changes: 11 additions & 0 deletions internal/types/map.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,14 @@ func ExpandMapStringString(data any) map[string]string {
}
return m
}

// GetMapValue returns the value for a key from a map.
// returns zero value if key does not exist in map.
func GetMapValue[T any](m map[string]any, key string) T { //nolint: ireturn
var val T
valI, exists := m[key]
if exists {
val = valI.(T)
}
return val
}
Loading