Skip to content

Remove redundant validation from duplicate ordering (covered by API) #894

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Nov 29, 2017
Merged

Remove redundant validation from duplicate ordering (covered by API) #894

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9c16797
Remove redundant validation from duplicate ordering (covered by API)
Nov 21, 2017
afd54bf
Address pull request feedback
Nov 22, 2017
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
121 changes: 14 additions & 107 deletions SoftLayer/managers/storage_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -887,8 +887,8 @@ def prepare_duplicate_order_object(manager, origin_volume, iops, tier,

:param manager: The File or Block manager calling this function
:param origin_volume: The origin volume which is being duplicated
:param iops: The IOPS per GB for the duplicant volume (performance)
:param tier: The tier level for the duplicant volume (endurance)
:param iops: The IOPS for the duplicate volume (performance)
:param tier: The tier level for the duplicate volume (endurance)
:param duplicate_size: The requested size for the duplicate volume
:param duplicate_snapshot_size: The size for the duplicate snapshot space
:param volume_type: The type of the origin volume ('file' or 'block')
Expand Down Expand Up @@ -931,9 +931,9 @@ def prepare_duplicate_order_object(manager, origin_volume, iops, tier,
if duplicate_snapshot_size is None:
duplicate_snapshot_size = origin_snapshot_size

# Validate the requested duplicate size, and set the size if none was given
duplicate_size = _validate_duplicate_size(
origin_volume, duplicate_size, volume_type)
# Use the origin volume size if no size was specified for the duplicate
if duplicate_size is None:
duplicate_size = origin_volume['capacityGb']

# Get the appropriate package for the order
# ('storage_as_a_service' is currently used for duplicate volumes)
Expand All @@ -942,13 +942,13 @@ def prepare_duplicate_order_object(manager, origin_volume, iops, tier,
# Determine the IOPS or tier level for the duplicate volume, along with
# the type and prices for the order
origin_storage_type = origin_volume['storageType']['keyName']
if origin_storage_type == 'PERFORMANCE_BLOCK_STORAGE'\
or origin_storage_type == 'PERFORMANCE_BLOCK_STORAGE_REPLICANT'\
or origin_storage_type == 'PERFORMANCE_FILE_STORAGE'\
or origin_storage_type == 'PERFORMANCE_FILE_STORAGE_REPLICANT':
if 'PERFORMANCE' in origin_storage_type:
volume_is_performance = True
iops = _validate_dupl_performance_iops(
origin_volume, iops, duplicate_size)
if iops is None:
iops = int(origin_volume.get('provisionedIops', 0))
if iops <= 0:
raise exceptions.SoftLayerError(
"Cannot find origin volume's provisioned IOPS")
# Set up the price array for the order
prices = [
find_price_by_category(package, 'storage_as_a_service'),
Expand All @@ -961,12 +961,10 @@ def prepare_duplicate_order_object(manager, origin_volume, iops, tier,
prices.append(find_saas_snapshot_space_price(
package, duplicate_snapshot_size, iops=iops))

elif origin_storage_type == 'ENDURANCE_BLOCK_STORAGE'\
or origin_storage_type == 'ENDURANCE_BLOCK_STORAGE_REPLICANT'\
or origin_storage_type == 'ENDURANCE_FILE_STORAGE'\
or origin_storage_type == 'ENDURANCE_FILE_STORAGE_REPLICANT':
elif 'ENDURANCE' in origin_storage_type:
volume_is_performance = False
tier = _validate_dupl_endurance_tier(origin_volume, tier)
if tier is None:
tier = find_endurance_tier_iops_per_gb(origin_volume)
# Set up the price array for the order
prices = [
find_price_by_category(package, 'storage_as_a_service'),
Expand Down Expand Up @@ -1003,97 +1001,6 @@ def prepare_duplicate_order_object(manager, origin_volume, iops, tier,
return duplicate_order


def _validate_duplicate_size(origin_volume, duplicate_volume_size,
volume_type):
# Ensure the origin volume's size is found
if not isinstance(utils.lookup(origin_volume, 'capacityGb'), int):
raise exceptions.SoftLayerError("Cannot find origin volume's size.")

# Determine the volume size/capacity for the duplicate
if duplicate_volume_size is None:
duplicate_volume_size = origin_volume['capacityGb']
# Ensure the duplicate volume size is not below the minimum
elif duplicate_volume_size < origin_volume['capacityGb']:
raise exceptions.SoftLayerError(
"The requested duplicate volume size is too small. Duplicate "
"volumes must be at least as large as their origin volumes.")

# Ensure the duplicate volume size is not above the maximum
if volume_type == 'block':
# Determine the base size for validating the requested duplicate size
if 'originalVolumeSize' in origin_volume:
base_volume_size = int(origin_volume['originalVolumeSize'])
else:
base_volume_size = origin_volume['capacityGb']

# Current limit for block volumes: 10*[origin size]
if duplicate_volume_size > base_volume_size * 10:
raise exceptions.SoftLayerError(
"The requested duplicate volume size is too large. The "
"maximum size for duplicate block volumes is 10 times the "
"size of the origin volume or, if the origin volume was also "
"a duplicate, 10 times the size of the initial origin volume "
"(i.e. the origin volume from which the first duplicate was "
"created in the chain of duplicates). "
"Requested: %s GB. Base origin size: %s GB."
% (duplicate_volume_size, base_volume_size))

return duplicate_volume_size


def _validate_dupl_performance_iops(origin_volume, duplicate_iops,
duplicate_size):
if not isinstance(utils.lookup(origin_volume, 'provisionedIops'), str):
raise exceptions.SoftLayerError(
"Cannot find origin volume's provisioned IOPS")

if duplicate_iops is None:
duplicate_iops = int(origin_volume['provisionedIops'])
else:
origin_iops_per_gb = float(origin_volume['provisionedIops'])\
/ float(origin_volume['capacityGb'])
duplicate_iops_per_gb = float(duplicate_iops) / float(duplicate_size)
if origin_iops_per_gb < 0.3 and duplicate_iops_per_gb >= 0.3:
raise exceptions.SoftLayerError(
"Origin volume performance is < 0.3 IOPS/GB, "
"duplicate volume performance must also be < 0.3 "
"IOPS/GB. %s IOPS/GB (%s/%s) requested."
% (duplicate_iops_per_gb, duplicate_iops, duplicate_size))
elif origin_iops_per_gb >= 0.3 and duplicate_iops_per_gb < 0.3:
raise exceptions.SoftLayerError(
"Origin volume performance is >= 0.3 IOPS/GB, "
"duplicate volume performance must also be >= 0.3 "
"IOPS/GB. %s IOPS/GB (%s/%s) requested."
% (duplicate_iops_per_gb, duplicate_iops, duplicate_size))
return duplicate_iops


def _validate_dupl_endurance_tier(origin_volume, duplicate_tier):
try:
origin_tier = find_endurance_tier_iops_per_gb(origin_volume)
except ValueError:
raise exceptions.SoftLayerError(
"Cannot find origin volume's tier level")

if duplicate_tier is None:
duplicate_tier = origin_tier
else:
if duplicate_tier != 0.25:
duplicate_tier = int(duplicate_tier)

if origin_tier == 0.25 and duplicate_tier != 0.25:
raise exceptions.SoftLayerError(
"Origin volume performance tier is 0.25 IOPS/GB, "
"duplicate volume performance tier must also be 0.25 "
"IOPS/GB. %s IOPS/GB requested." % duplicate_tier)
elif origin_tier != 0.25 and duplicate_tier == 0.25:
raise exceptions.SoftLayerError(
"Origin volume performance tier is above 0.25 IOPS/GB, "
"duplicate volume performance tier must also be above 0.25 "
"IOPS/GB. %s IOPS/GB requested." % duplicate_tier)
return duplicate_tier


def _has_category(categories, category_code):
return any(
True
Expand Down
36 changes: 8 additions & 28 deletions tests/managers/block_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
:license: MIT, see LICENSE for more details.
"""

import copy
import SoftLayer
from SoftLayer import exceptions
from SoftLayer import fixtures
Expand Down Expand Up @@ -336,8 +337,7 @@ def test_order_block_volume_performance(self):
mock = self.set_mock('SoftLayer_Product_Package', 'getAllObjects')
mock.return_value = [fixtures.SoftLayer_Product_Package.SAAS_PACKAGE]

mock_volume = fixtures.SoftLayer_Network_Storage.STAAS_TEST_VOLUME
prev_storage_type_keyname = mock_volume['storageType']['keyName']
mock_volume = copy.deepcopy(fixtures.SoftLayer_Network_Storage.STAAS_TEST_VOLUME)
mock_volume['storageType']['keyName'] = 'PERFORMANCE_BLOCK_STORAGE'
mock = self.set_mock('SoftLayer_Network_Storage', 'getObject')
mock.return_value = mock_volume
Expand Down Expand Up @@ -375,8 +375,6 @@ def test_order_block_volume_performance(self):
},)
)

mock_volume['storageType']['keyName'] = prev_storage_type_keyname

def test_order_block_volume_endurance(self):
mock = self.set_mock('SoftLayer_Location_Datacenter', 'getDatacenters')
mock.return_value = [{'id': 449494, 'name': 'dal09'}]
Expand Down Expand Up @@ -498,8 +496,7 @@ def test_order_block_snapshot_space_upgrade(self):
mock = self.set_mock('SoftLayer_Product_Package', 'getAllObjects')
mock.return_value = [fixtures.SoftLayer_Product_Package.SAAS_PACKAGE]

mock_volume = fixtures.SoftLayer_Network_Storage.STAAS_TEST_VOLUME
prev_storage_type_keyname = mock_volume['storageType']['keyName']
mock_volume = copy.deepcopy(fixtures.SoftLayer_Network_Storage.STAAS_TEST_VOLUME)
mock_volume['storageType']['keyName'] = 'ENDURANCE_BLOCK_STORAGE'
mock = self.set_mock('SoftLayer_Network_Storage', 'getObject')
mock.return_value = mock_volume
Expand All @@ -525,8 +522,6 @@ def test_order_block_snapshot_space_upgrade(self):
},)
)

mock_volume['storageType']['keyName'] = prev_storage_type_keyname

def test_order_block_snapshot_space(self):
mock = self.set_mock('SoftLayer_Product_Package', 'getAllObjects')
mock.return_value = [fixtures.SoftLayer_Product_Package.SAAS_PACKAGE]
Expand Down Expand Up @@ -557,8 +552,7 @@ def test_order_block_snapshot_space(self):
)

def test_order_block_replicant_os_type_not_found(self):
mock_volume = fixtures.SoftLayer_Network_Storage.STAAS_TEST_VOLUME
prev_os_type = mock_volume['osType']
mock_volume = copy.deepcopy(fixtures.SoftLayer_Network_Storage.STAAS_TEST_VOLUME)
del mock_volume['osType']
mock = self.set_mock('SoftLayer_Network_Storage', 'getObject')
mock.return_value = mock_volume
Expand All @@ -575,17 +569,14 @@ def test_order_block_replicant_os_type_not_found(self):
str(exception)
)

mock_volume['osType'] = prev_os_type

def test_order_block_replicant_performance_os_type_given(self):
mock = self.set_mock('SoftLayer_Location_Datacenter', 'getDatacenters')
mock.return_value = [{'id': 449494, 'name': 'dal09'}]

mock = self.set_mock('SoftLayer_Product_Package', 'getAllObjects')
mock.return_value = [fixtures.SoftLayer_Product_Package.SAAS_PACKAGE]

mock_volume = fixtures.SoftLayer_Network_Storage.STAAS_TEST_VOLUME
prev_storage_type_keyname = mock_volume['storageType']['keyName']
mock_volume = copy.deepcopy(fixtures.SoftLayer_Network_Storage.STAAS_TEST_VOLUME)
mock_volume['storageType']['keyName'] = 'PERFORMANCE_BLOCK_STORAGE'
mock = self.set_mock('SoftLayer_Network_Storage', 'getObject')
mock.return_value = mock_volume
Expand Down Expand Up @@ -625,8 +616,6 @@ def test_order_block_replicant_performance_os_type_given(self):
},)
)

mock_volume['storageType']['keyName'] = prev_storage_type_keyname

def test_order_block_replicant_endurance(self):
mock = self.set_mock('SoftLayer_Location_Datacenter', 'getDatacenters')
mock.return_value = [{'id': 449494, 'name': 'dal09'}]
Expand Down Expand Up @@ -671,8 +660,7 @@ def test_order_block_duplicate_origin_os_type_not_found(self):
mock = self.set_mock('SoftLayer_Product_Package', 'getAllObjects')
mock.return_value = [fixtures.SoftLayer_Product_Package.SAAS_PACKAGE]

mock_volume = fixtures.SoftLayer_Network_Storage.STAAS_TEST_VOLUME
prev_os_type = mock_volume['osType']
mock_volume = copy.deepcopy(fixtures.SoftLayer_Network_Storage.STAAS_TEST_VOLUME)
del mock_volume['osType']
mock = self.set_mock('SoftLayer_Network_Storage', 'getObject')
mock.return_value = mock_volume
Expand All @@ -686,14 +674,11 @@ def test_order_block_duplicate_origin_os_type_not_found(self):
self.assertEqual(str(exception),
"Cannot find origin volume's os-type")

mock_volume['osType'] = prev_os_type

def test_order_block_duplicate_performance_no_duplicate_snapshot(self):
mock = self.set_mock('SoftLayer_Product_Package', 'getAllObjects')
mock.return_value = [fixtures.SoftLayer_Product_Package.SAAS_PACKAGE]

mock_volume = fixtures.SoftLayer_Network_Storage.STAAS_TEST_VOLUME
prev_storage_type_keyname = mock_volume['storageType']['keyName']
mock_volume = copy.deepcopy(fixtures.SoftLayer_Network_Storage.STAAS_TEST_VOLUME)
mock_volume['storageType']['keyName'] = 'PERFORMANCE_BLOCK_STORAGE'
mock = self.set_mock('SoftLayer_Network_Storage', 'getObject')
mock.return_value = mock_volume
Expand Down Expand Up @@ -726,14 +711,11 @@ def test_order_block_duplicate_performance_no_duplicate_snapshot(self):
'useHourlyPricing': False
},))

mock_volume['storageType']['keyName'] = prev_storage_type_keyname

def test_order_block_duplicate_performance(self):
mock = self.set_mock('SoftLayer_Product_Package', 'getAllObjects')
mock.return_value = [fixtures.SoftLayer_Product_Package.SAAS_PACKAGE]

mock_volume = fixtures.SoftLayer_Network_Storage.STAAS_TEST_VOLUME
prev_storage_type_keyname = mock_volume['storageType']['keyName']
mock_volume = copy.deepcopy(fixtures.SoftLayer_Network_Storage.STAAS_TEST_VOLUME)
mock_volume['storageType']['keyName'] = 'PERFORMANCE_BLOCK_STORAGE'
mock = self.set_mock('SoftLayer_Network_Storage', 'getObject')
mock.return_value = mock_volume
Expand Down Expand Up @@ -773,8 +755,6 @@ def test_order_block_duplicate_performance(self):
'useHourlyPricing': False
},))

mock_volume['storageType']['keyName'] = prev_storage_type_keyname

def test_order_block_duplicate_endurance_no_duplicate_snapshot(self):
mock = self.set_mock('SoftLayer_Product_Package', 'getAllObjects')
mock.return_value = [fixtures.SoftLayer_Product_Package.SAAS_PACKAGE]
Expand Down
Loading