Skip to content

Commit 2840968

Browse files
allmightyspiffallmightyspiff
authored
Merge pull request softlayer#797 from camporter/fix_storage_order_price_lookup
Fix the way storage space is looked up, remove os type requirement for file storage ordering
2 parents 6dd6159 + 1ada772 commit 2840968

8 files changed

Lines changed: 312 additions & 47 deletions

File tree

‎SoftLayer/CLI/file/order.py‎

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -28,17 +28,6 @@
2828
help='Endurance Storage Tier (IOP per GB)'
2929
' [required for storage-type endurance]',
3030
type=click.Choice(['0.25', '2', '4', '10']))
31-
@click.option('--os-type',
32-
help='Operating System',
33-
type=click.Choice([
34-
'HYPER_V',
35-
'LINUX',
36-
'VMWARE',
37-
'WINDOWS_2008',
38-
'WINDOWS_GPT',
39-
'WINDOWS',
40-
'XEN']),
41-
required=True)
4231
@click.option('--location',
4332
help='Datacenter short name (e.g.: dal09)',
4433
required=True)
@@ -48,7 +37,7 @@
4837
'space along with endurance file storage; specifies '
4938
'the size (in GB) of snapshot space to order')
5039
@environment.pass_env
51-
def cli(env, storage_type, size, iops, tier, os_type,
40+
def cli(env, storage_type, size, iops, tier,
5241
location, snapshot_size):
5342
"""Order a file storage volume."""
5443
file_manager = SoftLayer.FileStorageManager(env.client)
@@ -79,8 +68,7 @@ def cli(env, storage_type, size, iops, tier, os_type,
7968
storage_type='performance_storage_nfs',
8069
location=location,
8170
size=size,
82-
iops=iops,
83-
os_type=os_type
71+
iops=iops
8472
)
8573
except ValueError as ex:
8674
raise exceptions.ArgumentError(str(ex))
@@ -98,7 +86,6 @@ def cli(env, storage_type, size, iops, tier, os_type,
9886
location=location,
9987
size=size,
10088
tier_level=float(tier),
101-
os_type=os_type,
10289
snapshot_size=snapshot_size
10390
)
10491
except ValueError as ex:

‎SoftLayer/managers/block.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ def order_block_volume(self, storage_type, location, size, os_type,
271271
package,
272272
'performance_storage_iscsi'
273273
),
274-
storage_utils.find_performance_space_price(package, iops),
274+
storage_utils.find_performance_space_price(package, size),
275275
storage_utils.find_performance_iops_price(package, size, iops),
276276
]
277277
elif storage_type == 'storage_service_enterprise':

‎SoftLayer/managers/file.py‎

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -228,20 +228,23 @@ def delete_snapshot(self, snapshot_id):
228228
return self.client.call('Network_Storage', 'deleteObject',
229229
id=snapshot_id)
230230

231-
def order_file_volume(self, storage_type, location, size, os_type,
231+
def order_file_volume(self, storage_type, location, size, os_type=None,
232232
iops=None, tier_level=None, snapshot_size=None):
233233
"""Places an order for a file volume.
234234
235235
:param storage_type: "performance_storage_iscsi" (performance)
236236
or "storage_service_enterprise" (endurance)
237237
:param location: Datacenter in which to order iSCSI volume
238238
:param size: Size of the desired volume, in GB
239-
:param os_type: OS Type to use for volume alignment, see help for list
239+
:param os_type: Not used for file storage orders, leave None
240240
:param iops: Number of IOPs for a "Performance" order
241241
:param tier_level: Tier level to use for an "Endurance" order
242242
:param snapshot_size: The size of optional snapshot space,
243243
if snapshot space should also be ordered (None if not ordered)
244244
"""
245+
if os_type:
246+
raise exceptions.SoftLayerError(
247+
'OS type is not used on file storage orders')
245248

246249
try:
247250
location_id = storage_utils.get_location_id(self, location)
@@ -259,7 +262,7 @@ def order_file_volume(self, storage_type, location, size, os_type,
259262
package,
260263
'performance_storage_nfs'
261264
),
262-
storage_utils.find_performance_space_price(package, iops),
265+
storage_utils.find_performance_space_price(package, size),
263266
storage_utils.find_performance_iops_price(package, size, iops),
264267
]
265268
elif storage_type == 'storage_service_enterprise':
@@ -288,7 +291,6 @@ def order_file_volume(self, storage_type, location, size, os_type,
288291
order = {
289292
'complexType': complex_type,
290293
'packageId': package['id'],
291-
'osFormatType': {'keyName': os_type},
292294
'prices': prices,
293295
'quantity': 1,
294296
'location': location_id,
@@ -313,6 +315,15 @@ def enable_snapshots(self, volume_id, schedule_type, retention_count,
313315
314316
:param integer volume_id: The id of the volume
315317
:param string schedule_type: 'HOURLY'|'DAILY'|'WEEKLY'
318+
:param integer retention_count: The number of snapshots to attempt to
319+
retain in this schedule
320+
:param integer minute: The minute of the hour at which HOURLY, DAILY,
321+
and WEEKLY snapshots should be taken
322+
:param integer hour: The hour of the day at which DAILY and WEEKLY
323+
snapshots should be taken
324+
:param string|integer day_of_week: The day of the week on which WEEKLY
325+
snapshots should be taken, either as a string ('SUNDAY') or integer
326+
('0' is Sunday)
316327
:return: Returns whether successfully scheduled or not
317328
"""
318329

@@ -340,7 +351,7 @@ def order_snapshot_space(self, volume_id, capacity, tier,
340351
upgrade, **kwargs):
341352
"""Orders snapshot space for the given file volume.
342353
343-
:param integer volume_id: The id of the volume
354+
:param integer volume_id: The ID of the volume
344355
:param integer capacity: The capacity to order, in GB
345356
:param float tier: The tier level of the file volume, in IOPS per GB
346357
:param boolean upgrade: Flag to indicate if this order is an upgrade
@@ -390,7 +401,7 @@ def cancel_snapshot_space(self, volume_id,
390401
391402
:param integer volume_id: The volume ID
392403
:param string reason: The reason for cancellation
393-
:param boolean immediate_flag: Cancel immediately or
404+
:param boolean immediate: Cancel immediately or
394405
on anniversary date
395406
"""
396407

@@ -422,9 +433,9 @@ def cancel_snapshot_space(self, volume_id,
422433
def restore_from_snapshot(self, volume_id, snapshot_id):
423434
"""Restores a specific volume from a snapshot
424435
425-
:param integer volume_id: The id of the volume
436+
:param integer volume_id: The ID of the volume
426437
:param integer snapshot_id: The id of the restore point
427-
:return: Returns whether succesfully restored or not
438+
:return: Returns whether successfully restored or not
428439
"""
429440

430441
return self.client.call('Network_Storage', 'restoreFromSnapshot',
@@ -437,7 +448,7 @@ def cancel_file_volume(self, volume_id,
437448
438449
:param integer volume_id: The volume ID
439450
:param string reason: The reason for cancellation
440-
:param boolean immediate_flag: Cancel immediately or
451+
:param boolean immediate: Cancel immediately or
441452
on anniversary date
442453
"""
443454
file_volume = self.get_file_volume_details(
@@ -454,7 +465,7 @@ def cancel_file_volume(self, volume_id,
454465
def failover_to_replicant(self, volume_id, replicant_id, immediate=False):
455466
"""Failover to a volume replicant.
456467
457-
:param integer volume_id: The id of the volume
468+
:param integer volume_id: The ID of the volume
458469
:param integer replicant_id: ID of replicant to failover to
459470
:param boolean immediate: Flag indicating if failover is immediate
460471
:return: Returns whether failover was successful or not
@@ -466,8 +477,8 @@ def failover_to_replicant(self, volume_id, replicant_id, immediate=False):
466477
def failback_from_replicant(self, volume_id, replicant_id):
467478
"""Failback from a volume replicant.
468479
469-
:param integer volume_id: The id of the volume
470-
:param integer: ID of replicant to failback from
480+
:param integer volume_id: The ID of the volume
481+
:param integer replicant_id: ID of replicant to failback from
471482
:return: Returns whether failback was successful or not
472483
"""
473484

‎SoftLayer/managers/storage_utils.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ def populate_host_templates(host_templates,
6060

6161

6262
def get_package(manager, category_code):
63-
"""Returns a product packaged based on type of storage.
63+
"""Returns a product package based on type of storage.
6464
6565
:param manager: The storage manager which calls this function.
6666
:param category_code: Category code of product package.

‎tests/CLI/modules/block_tests.py‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,30 @@ def test_volume_order_order_not_placed(self, order_mock):
217217
'Order could not be placed! Please verify '
218218
'your options and try again.\n')
219219

220+
@mock.patch('SoftLayer.BlockStorageManager.order_block_volume')
221+
def test_volume_order_performance_manager_error(self, order_mock):
222+
order_mock.side_effect = ValueError('failure!')
223+
224+
result = self.run_command(['block', 'volume-order',
225+
'--storage-type=performance', '--size=20',
226+
'--iops=100', '--os-type=linux',
227+
'--location=dal05'])
228+
229+
self.assertEqual(2, result.exit_code)
230+
self.assertEqual('Argument Error: failure!', result.exception.message)
231+
232+
@mock.patch('SoftLayer.BlockStorageManager.order_block_volume')
233+
def test_volume_order_endurance_manager_error(self, order_mock):
234+
order_mock.side_effect = ValueError('failure!')
235+
236+
result = self.run_command(['block', 'volume-order',
237+
'--storage-type=endurance', '--size=20',
238+
'--tier=0.25', '--os-type=linux',
239+
'--location=dal05'])
240+
241+
self.assertEqual(2, result.exit_code)
242+
self.assertEqual('Argument Error: failure!', result.exception.message)
243+
220244
def test_enable_snapshots(self):
221245
result = self.run_command(['block', 'snapshot-enable', '12345678',
222246
'--schedule-type=HOURLY', '--minute=10',

‎tests/CLI/modules/file_tests.py‎

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -151,31 +151,29 @@ def test_volume_detail(self):
151151
def test_volume_order_performance_iops_not_given(self):
152152
result = self.run_command(['file', 'volume-order',
153153
'--storage-type=performance', '--size=20',
154-
'--os-type=linux', '--location=dal05'])
154+
'--location=dal05'])
155155

156156
self.assertEqual(2, result.exit_code)
157157

158158
def test_volume_order_performance_iops_out_of_range(self):
159159
result = self.run_command(['file', 'volume-order',
160160
'--storage-type=performance', '--size=20',
161-
'--iops=80000', '--os-type=linux',
162-
'--location=dal05'])
161+
'--iops=80000', '--location=dal05'])
163162

164163
self.assertEqual(2, result.exit_code)
165164

166165
def test_volume_order_performance_iops_not_multiple_of_100(self):
167166
result = self.run_command(['file', 'volume-order',
168167
'--storage-type=performance', '--size=20',
169-
'--iops=122', '--os-type=linux',
170-
'--location=dal05'])
168+
'--iops=122', '--location=dal05'])
171169

172170
self.assertEqual(2, result.exit_code)
173171

174172
def test_volume_order_performance_snapshot_error(self):
175173
result = self.run_command(['file', 'volume-order',
176174
'--storage-type=performance', '--size=20',
177-
'--iops=100', '--os-type=linux',
178-
'--location=dal05', '--snapshot-size=10'])
175+
'--iops=100', '--location=dal05',
176+
'--snapshot-size=10'])
179177

180178
self.assertEqual(2, result.exit_code)
181179

@@ -194,8 +192,7 @@ def test_volume_order_performance(self, order_mock):
194192

195193
result = self.run_command(['file', 'volume-order',
196194
'--storage-type=performance', '--size=20',
197-
'--iops=100', '--os-type=linux',
198-
'--location=dal05'])
195+
'--iops=100', '--location=dal05'])
199196

200197
self.assert_no_fail(result)
201198
self.assertEqual(result.output,
@@ -206,7 +203,7 @@ def test_volume_order_performance(self, order_mock):
206203
def test_volume_order_endurance_tier_not_given(self):
207204
result = self.run_command(['file', 'volume-order',
208205
'--storage-type=endurance', '--size=20',
209-
'--os-type=linux', '--location=dal05'])
206+
'--location=dal05'])
210207

211208
self.assertEqual(2, result.exit_code)
212209

@@ -226,8 +223,8 @@ def test_volume_order_endurance(self, order_mock):
226223

227224
result = self.run_command(['file', 'volume-order',
228225
'--storage-type=endurance', '--size=20',
229-
'--tier=0.25', '--os-type=linux',
230-
'--location=dal05', '--snapshot-size=10'])
226+
'--tier=0.25', '--location=dal05',
227+
'--snapshot-size=10'])
231228

232229
self.assert_no_fail(result)
233230
self.assertEqual(result.output,
@@ -242,14 +239,35 @@ def test_volume_order_order_not_placed(self, order_mock):
242239

243240
result = self.run_command(['file', 'volume-order',
244241
'--storage-type=endurance', '--size=20',
245-
'--tier=0.25', '--os-type=linux',
246-
'--location=dal05'])
242+
'--tier=0.25', '--location=dal05'])
247243

248244
self.assert_no_fail(result)
249245
self.assertEqual(result.output,
250246
'Order could not be placed! Please verify '
251247
'your options and try again.\n')
252248

249+
@mock.patch('SoftLayer.FileStorageManager.order_file_volume')
250+
def test_volume_order_performance_manager_error(self, order_mock):
251+
order_mock.side_effect = ValueError('failure!')
252+
253+
result = self.run_command(['file', 'volume-order',
254+
'--storage-type=performance', '--size=20',
255+
'--iops=100', '--location=dal05'])
256+
257+
self.assertEqual(2, result.exit_code)
258+
self.assertEqual('Argument Error: failure!', result.exception.message)
259+
260+
@mock.patch('SoftLayer.FileStorageManager.order_file_volume')
261+
def test_volume_order_endurance_manager_error(self, order_mock):
262+
order_mock.side_effect = ValueError('failure!')
263+
264+
result = self.run_command(['file', 'volume-order',
265+
'--storage-type=endurance', '--size=20',
266+
'--tier=0.25', '--location=dal05'])
267+
268+
self.assertEqual(2, result.exit_code)
269+
self.assertEqual('Argument Error: failure!', result.exception.message)
270+
253271
def test_enable_snapshots(self):
254272
result = self.run_command(['file', 'snapshot-enable', '12345678',
255273
'--schedule-type=HOURLY', '--minute=10',

0 commit comments

Comments
 (0)