Skip to content

Commit 0efc359

Browse files
authored
fix(instance): handle root volume with no size as default (#681)
1 parent 0a8dc32 commit 0efc359

6 files changed

+5137
-12
lines changed

go.mod

+1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ module github.com/terraform-providers/terraform-provider-scaleway
33
require (
44
github.com/aws/aws-sdk-go v1.34.32
55
github.com/dnaeon/go-vcr v1.0.1
6+
github.com/dustin/go-humanize v1.0.0
67
github.com/google/go-cmp v0.5.2
78
github.com/hashicorp/go-retryablehttp v0.6.7
89
github.com/hashicorp/terraform-plugin-sdk/v2 v2.2.0

go.sum

+2-6
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c
7979
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
8080
github.com/dnaeon/go-vcr v1.0.1 h1:r8L/HqC0Hje5AXMu1ooW8oyQyOFv4GxqpL0nRP7SLLY=
8181
github.com/dnaeon/go-vcr v1.0.1/go.mod h1:aBB1+wY4s93YsC3HHjMBMrwTj2R9FHDzUr9KyGc8n1E=
82+
github.com/dustin/go-humanize v1.0.0 h1:VSnTsYCnlFHaM2/igO1h6X3HA71jcobQuxemgkq4zYo=
83+
github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk=
8284
github.com/emirpasic/gods v1.12.0 h1:QAUIPSaCu4G+POclxeqb3F+WPpdKqFGlw36+yOzGlrg=
8385
github.com/emirpasic/gods v1.12.0/go.mod h1:YfzfFFoVP/catgzJb4IKIqXjX78Ha8FMSDh3ymbK86o=
8486
github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4=
@@ -297,12 +299,6 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN
297299
github.com/posener/complete v1.1.1/go.mod h1:em0nMJCgc9GFtwrmVmEMR/ZL6WyhyjMBndrE9hABlRI=
298300
github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA=
299301
github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=
300-
github.com/scaleway/scaleway-sdk-go v1.0.0-beta.7.0.20201012095732-98ec365545de h1:yhT7zTv3WAMD3S3GqkpAFIL8O76emUsBKsiBcVBb4Bg=
301-
github.com/scaleway/scaleway-sdk-go v1.0.0-beta.7.0.20201012095732-98ec365545de/go.mod h1:CJJ5VAbozOl0yEw7nHB9+7BXTJbIn6h7W+f6Gau5IP8=
302-
github.com/scaleway/scaleway-sdk-go v1.0.0-beta.7.0.20201106174615-fd524d38cee8 h1:OUM8gK4Frdx3yhDJWQSToADtrvCTGaZXYrm6O3j3JaQ=
303-
github.com/scaleway/scaleway-sdk-go v1.0.0-beta.7.0.20201106174615-fd524d38cee8/go.mod h1:CJJ5VAbozOl0yEw7nHB9+7BXTJbIn6h7W+f6Gau5IP8=
304-
github.com/scaleway/scaleway-sdk-go v1.0.0-beta.7.0.20201109203223-9446a84bfb2a h1:bcVe/pXc4HWi8RwkiiYOdIjYWjQvR5eeMRaepXGW298=
305-
github.com/scaleway/scaleway-sdk-go v1.0.0-beta.7.0.20201109203223-9446a84bfb2a/go.mod h1:CJJ5VAbozOl0yEw7nHB9+7BXTJbIn6h7W+f6Gau5IP8=
306302
github.com/scaleway/scaleway-sdk-go v1.0.0-beta.7.0.20201113152841-1153aa56e20e h1:KW5n7q2CMM/MsFNAwdZWB0UioVa3E3XQSrKRZcjmaGo=
307303
github.com/scaleway/scaleway-sdk-go v1.0.0-beta.7.0.20201113152841-1153aa56e20e/go.mod h1:CJJ5VAbozOl0yEw7nHB9+7BXTJbIn6h7W+f6Gau5IP8=
308304
github.com/sergi/go-diff v1.0.0 h1:Kpca3qRNrduNnOQeazBd0ysaKrUJiIuISHxogkT9RPQ=

scaleway/helpers_instance.go

+80
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"sort"
88
"time"
99

10+
"github.com/dustin/go-humanize"
1011
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
1112
"github.com/scaleway/scaleway-sdk-go/api/instance/v1"
1213
"github.com/scaleway/scaleway-sdk-go/scw"
@@ -159,3 +160,82 @@ func reachState(ctx context.Context, instanceAPI *instance.API, zone scw.Zone, s
159160
}
160161
return nil
161162
}
163+
164+
// getServerType is a util to get a instance.ServerType by its commercialType
165+
func getServerType(apiInstance *instance.API, zone scw.Zone, commercialType string) *instance.ServerType {
166+
serverType := (*instance.ServerType)(nil)
167+
168+
serverTypesRes, err := apiInstance.ListServersTypes(&instance.ListServersTypesRequest{
169+
Zone: zone,
170+
})
171+
if err != nil {
172+
l.Warningf("cannot get server types: %s", err)
173+
} else {
174+
serverType = serverTypesRes.Servers[commercialType]
175+
if serverType == nil {
176+
l.Warningf("unrecognized server type: %s", commercialType)
177+
}
178+
}
179+
180+
return serverType
181+
}
182+
183+
// validateLocalVolumeSizes validates the total size of local volumes.
184+
func validateLocalVolumeSizes(volumes map[string]*instance.VolumeTemplate, serverType *instance.ServerType, commercialType string) error {
185+
// Calculate local volume total size.
186+
var localVolumeTotalSize scw.Size
187+
for _, volume := range volumes {
188+
if volume.VolumeType == instance.VolumeVolumeTypeLSSD {
189+
localVolumeTotalSize += volume.Size
190+
}
191+
}
192+
193+
volumeConstraint := serverType.VolumesConstraint
194+
195+
// If no root volume provided, count the default root volume size added by the API.
196+
if rootVolume := volumes["0"]; rootVolume == nil {
197+
localVolumeTotalSize += volumeConstraint.MinSize
198+
}
199+
200+
if localVolumeTotalSize < volumeConstraint.MinSize || localVolumeTotalSize > volumeConstraint.MaxSize {
201+
min := humanize.Bytes(uint64(volumeConstraint.MinSize))
202+
if volumeConstraint.MinSize == volumeConstraint.MaxSize {
203+
return fmt.Errorf("%s total local volume size must be equal to %s", commercialType, min)
204+
}
205+
206+
max := humanize.Bytes(uint64(volumeConstraint.MaxSize))
207+
return fmt.Errorf("%s total local volume size must be between %s and %s", commercialType, min, max)
208+
}
209+
210+
return nil
211+
}
212+
213+
// sanitizeVolumeMap removes extra data for API validation.
214+
//
215+
// 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
216+
// - With an image (in that case the root volume can be skipped because it is taken from the image)
217+
// - Without an image (in that case, the root volume must be defined)
218+
func sanitizeVolumeMap(serverName string, volumes map[string]*instance.VolumeTemplate) map[string]*instance.VolumeTemplate {
219+
m := make(map[string]*instance.VolumeTemplate)
220+
221+
for index, v := range volumes {
222+
v.Name = serverName + "-" + index
223+
224+
// Remove extra data for API validation.
225+
switch {
226+
// If a volume already got an ID it is passed as it to the API without specifying the volume type.
227+
// TODO: Fix once instance accept volume type in the schema validation
228+
case v.ID != "":
229+
v = &instance.VolumeTemplate{ID: v.ID, Name: v.Name}
230+
// For the root volume (index 0) if the specified size is not 0 it is considered as a new volume
231+
// It does not have yet a volume ID, it is passed to the API with only the size to be dynamically created by the API
232+
case index == "0" && v.Size != 0:
233+
v = &instance.VolumeTemplate{Size: v.Size}
234+
// If none of the above conditions are met, the volume is passed as it to the API
235+
default:
236+
}
237+
m[index] = v
238+
}
239+
240+
return m
241+
}

scaleway/resource_instance_server.go

+30-6
Original file line numberDiff line numberDiff line change
@@ -278,22 +278,50 @@ func resourceScalewayInstanceServerCreate(ctx context.Context, d *schema.Resourc
278278
req.PlacementGroup = expandStringPtr(expandZonedID(placementGroupID).ID)
279279
}
280280

281+
serverType := getServerType(instanceAPI, req.Zone, req.CommercialType)
282+
if serverType == nil {
283+
return diag.FromErr(fmt.Errorf("could not find a server type associated with %s", req.CommercialType))
284+
}
285+
281286
req.Volumes = make(map[string]*instance.VolumeTemplate)
282287
if size, ok := d.GetOk("root_volume.0.size_in_gb"); ok {
283288
req.Volumes["0"] = &instance.VolumeTemplate{
284289
Size: scw.Size(uint64(size.(int)) * gb),
285290
}
291+
} else {
292+
// We had a local root volume if it is not already present
293+
req.Volumes["0"] = &instance.VolumeTemplate{
294+
Name: newRandomName("vol"),
295+
VolumeType: instance.VolumeVolumeTypeLSSD,
296+
Size: serverType.VolumesConstraint.MinSize,
297+
}
286298
}
287299

288300
if raw, ok := d.GetOk("additional_volume_ids"); ok {
289301
for i, volumeID := range raw.([]interface{}) {
302+
// We have to get the volume to know whether it is a local or a block volume
303+
vol, err := instanceAPI.GetVolume(&instance.GetVolumeRequest{
304+
VolumeID: expandZonedID(volumeID).ID,
305+
})
306+
if err != nil {
307+
return diag.FromErr(err)
308+
}
290309
req.Volumes[strconv.Itoa(i+1)] = &instance.VolumeTemplate{
291-
ID: expandZonedID(volumeID).ID,
292-
Name: newRandomName("vol"),
310+
ID: vol.Volume.ID,
311+
Name: vol.Volume.Name,
312+
VolumeType: vol.Volume.VolumeType,
293313
}
294314
}
295315
}
296316

317+
// Validate total local volume sizes.
318+
if err := validateLocalVolumeSizes(req.Volumes, serverType, req.CommercialType); err != nil {
319+
return diag.FromErr(err)
320+
}
321+
322+
// Sanitize the volume map to respect API schemas
323+
req.Volumes = sanitizeVolumeMap(req.Name, req.Volumes)
324+
297325
res, err := instanceAPI.CreateServer(req, scw.WithContext(ctx))
298326
if err != nil {
299327
return diag.FromErr(err)
@@ -441,10 +469,6 @@ func resourceScalewayInstanceServerRead(ctx context.Context, d *schema.ResourceD
441469
rootVolume["volume_id"] = newZonedID(zone, volume.ID).String()
442470
rootVolume["size_in_gb"] = int(uint64(volume.Size) / gb)
443471

444-
if _, exist := rootVolume["delete_on_termination"]; !exist {
445-
rootVolume["delete_on_termination"] = true // default value does not work on list
446-
}
447-
448472
_ = d.Set("root_volume", []map[string]interface{}{rootVolume})
449473
} else {
450474
additionalVolumesIDs = append(additionalVolumesIDs, newZonedID(zone, volume.ID).String())

scaleway/resource_instance_server_test.go

+32
Original file line numberDiff line numberDiff line change
@@ -863,3 +863,35 @@ func TestAccScalewayInstanceServer_AlterTags(t *testing.T) {
863863
},
864864
})
865865
}
866+
867+
func TestAccScalewayInstanceServer_WithDefaultRootVolumeAndAdditionalVolume(t *testing.T) {
868+
tt := NewTestTools(t)
869+
defer tt.Cleanup()
870+
resource.ParallelTest(t, resource.TestCase{
871+
PreCheck: func() { testAccPreCheck(t) },
872+
ProviderFactories: tt.ProviderFactories,
873+
CheckDestroy: testAccCheckScalewayInstanceServerDestroy(tt),
874+
Steps: []resource.TestStep{
875+
{
876+
Config: `
877+
resource "scaleway_instance_volume" "data" {
878+
size_in_gb = 100
879+
type = "b_ssd"
880+
}
881+
882+
resource "scaleway_instance_server" "main" {
883+
type = "DEV1-S"
884+
image = "ubuntu-bionic"
885+
root_volume {
886+
delete_on_termination = false
887+
}
888+
additional_volume_ids = [ scaleway_instance_volume.data.id ]
889+
}
890+
`,
891+
Check: resource.ComposeTestCheckFunc(
892+
testAccCheckScalewayInstanceServerExists(tt, "scaleway_instance_server.main"),
893+
),
894+
},
895+
},
896+
})
897+
}

0 commit comments

Comments
 (0)