From d2eb29a6f8c09312463988f991332ef870854b22 Mon Sep 17 00:00:00 2001 From: Surya Ghatty Date: Tue, 14 Nov 2017 13:10:04 -0600 Subject: [PATCH 1/7] Added volume-modify feature to file/block. --- SoftLayer/CLI/block/modify.py | 70 +++++++++++ SoftLayer/CLI/file/modify.py | 70 +++++++++++ SoftLayer/CLI/routes.py | 2 + SoftLayer/managers/block.py | 26 ++++ SoftLayer/managers/file.py | 26 ++++ SoftLayer/managers/storage_utils.py | 176 ++++++++++++++++++++++++++++ 6 files changed, 370 insertions(+) create mode 100644 SoftLayer/CLI/block/modify.py create mode 100644 SoftLayer/CLI/file/modify.py diff --git a/SoftLayer/CLI/block/modify.py b/SoftLayer/CLI/block/modify.py new file mode 100644 index 000000000..e00bae543 --- /dev/null +++ b/SoftLayer/CLI/block/modify.py @@ -0,0 +1,70 @@ +"""Modify an existing block storage volume.""" +# :license: MIT, see LICENSE for more details. + +import click +import SoftLayer +from SoftLayer.CLI import environment +from SoftLayer.CLI import exceptions + + +CONTEXT_SETTINGS = {'token_normalize_func': lambda x: x.upper()} + + +@click.command(context_settings=CONTEXT_SETTINGS) +@click.argument('volume-id') +@click.option('--new-size', '-c', + type=int, + help='New Size of block volume in GB. ' + '***If no size is specified, the original size of ' + 'volume will be used.***\n' + 'Potential Sizes: [20, 40, 80, 100, 250, ' + '500, 1000, 2000, 4000, 8000, 12000] ' + 'Minimum: [the size of the origin volume] ' + 'Maximum: [the minimum of 12000 GB or ' + '10*(origin volume size)]') +@click.option('--new-iops', '-i', + type=int, + help='Performance Storage IOPS, between 100 and 6000 in ' + 'multiples of 100 [only used for performance volumes] ' + '***If no IOPS value is specified, the original IOPS value of the ' + 'volume will be used.***\n' + 'Requirements: [If original IOPS/GB for the volume is less ' + 'than 0.3, new IOPS/GB must also be less ' + 'than 0.3. If original IOPS/GB for the volume is greater ' + 'than or equal to 0.3, new IOPS/GB for the volume must ' + 'also be greater than or equal to 0.3.]') +@click.option('--new-tier', '-t', + help='Endurance Storage Tier (IOPS per GB) [only used for ' + 'endurance volumes] ***If no tier is specified, the original tier ' + 'of the volume will be used.***\n' + 'Requirements: [If original IOPS/GB for the volume is 0.25, ' + 'new IOPS/GB for the volume must also be 0.25. If original IOPS/GB ' + 'for the volume is greater than 0.25, new IOPS/GB ' + 'for the volume must also be greater than 0.25.]', + type=click.Choice(['0.25', '2', '4', '10'])) +@environment.pass_env +def cli(env, volume_id, new_size, new_iops, new_tier): + """Modify an existing block storage volume.""" + block_manager = SoftLayer.BlockStorageManager(env.client) + + if new_tier is not None: + new_tier = float(new_tier) + + try: + order = block_manager.order_modified_volume( + volume_id, + new_size=new_size, + new_iops=new_iops, + new_tier_level=new_tier, + ) + except ValueError as ex: + raise exceptions.ArgumentError(str(ex)) + + if 'placedOrder' in order.keys(): + click.echo("Order #{0} placed successfully!".format( + order['placedOrder']['id'])) + for item in order['placedOrder']['items']: + click.echo(" > %s" % item['description']) + else: + click.echo("Order could not be placed! Please verify your options " + + "and try again.") diff --git a/SoftLayer/CLI/file/modify.py b/SoftLayer/CLI/file/modify.py new file mode 100644 index 000000000..7346c8dc1 --- /dev/null +++ b/SoftLayer/CLI/file/modify.py @@ -0,0 +1,70 @@ +"""Modify an existing file storage volume.""" +# :license: MIT, see LICENSE for more details. + +import click +import SoftLayer +from SoftLayer.CLI import environment +from SoftLayer.CLI import exceptions + + +CONTEXT_SETTINGS = {'token_normalize_func': lambda x: x.upper()} + + +@click.command(context_settings=CONTEXT_SETTINGS) +@click.argument('volume-id') +@click.option('--new-size', '-c', + type=int, + help='New Size of file volume in GB. ' + '***If no size is specified, the original size of ' + 'volume will be used.***\n' + 'Potential Sizes: [20, 40, 80, 100, 250, ' + '500, 1000, 2000, 4000, 8000, 12000] ' + 'Minimum: [the size of the origin volume] ' + 'Maximum: [the minimum of 12000 GB or ' + '10*(origin volume size)]') +@click.option('--new-iops', '-i', + type=int, + help='Performance Storage IOPS, between 100 and 6000 in ' + 'multiples of 100 [only used for performance volumes] ' + '***If no IOPS value is specified, the original IOPS value of the ' + 'volume will be used.***\n' + 'Requirements: [If original IOPS/GB for the volume is less ' + 'than 0.3, new IOPS/GB must also be less ' + 'than 0.3. If original IOPS/GB for the volume is greater ' + 'than or equal to 0.3, new IOPS/GB for the volume must ' + 'also be greater than or equal to 0.3.]') +@click.option('--new-tier', '-t', + help='Endurance Storage Tier (IOPS per GB) [only used for ' + 'endurance volumes] ***If no tier is specified, the original tier ' + 'of the volume will be used.***\n' + 'Requirements: [IOPS/GB for the volume is 0.25, ' + 'new IOPS/GB for the volume must also be 0.25. If original IOPS/GB ' + 'for the volume is greater than 0.25, new IOPS/GB ' + 'for the volume must also be greater than 0.25.]', + type=click.Choice(['0.25', '2', '4', '10'])) +@environment.pass_env +def cli(env, volume_id, new_size, new_iops, new_tier): + """Modify an existing file storage volume.""" + file_manager = SoftLayer.FileStorageManager(env.client) + + if new_tier is not None: + new_tier = float(new_tier) + + try: + order = file_manager.order_modified_volume( + volume_id, + new_size=new_size, + new_iops=new_iops, + new_tier_level=new_tier, + ) + except ValueError as ex: + raise exceptions.ArgumentError(str(ex)) + + if 'placedOrder' in order.keys(): + click.echo("Order #{0} placed successfully!".format( + order['placedOrder']['id'])) + for item in order['placedOrder']['items']: + click.echo(" > %s" % item['description']) + else: + click.echo("Order could not be placed! Please verify your options " + + "and try again.") diff --git a/SoftLayer/CLI/routes.py b/SoftLayer/CLI/routes.py index 2ff3379ac..52e338b50 100644 --- a/SoftLayer/CLI/routes.py +++ b/SoftLayer/CLI/routes.py @@ -79,6 +79,7 @@ ('block:volume-count', 'SoftLayer.CLI.block.count:cli'), ('block:volume-detail', 'SoftLayer.CLI.block.detail:cli'), ('block:volume-duplicate', 'SoftLayer.CLI.block.duplicate:cli'), + ('block:volume-modify', 'SoftLayer.CLI.block.modify:cli'), ('block:volume-list', 'SoftLayer.CLI.block.list:cli'), ('block:volume-order', 'SoftLayer.CLI.block.order:cli'), ('block:volume-set-lun-id', 'SoftLayer.CLI.block.lun:cli'), @@ -104,6 +105,7 @@ ('file:volume-count', 'SoftLayer.CLI.file.count:cli'), ('file:volume-detail', 'SoftLayer.CLI.file.detail:cli'), ('file:volume-duplicate', 'SoftLayer.CLI.file.duplicate:cli'), + ('file:volume-modify', 'SoftLayer.CLI.file.modify:cli'), ('file:volume-list', 'SoftLayer.CLI.file.list:cli'), ('file:volume-order', 'SoftLayer.CLI.file.order:cli'), diff --git a/SoftLayer/managers/block.py b/SoftLayer/managers/block.py index 766312012..06394830c 100644 --- a/SoftLayer/managers/block.py +++ b/SoftLayer/managers/block.py @@ -303,6 +303,32 @@ def order_duplicate_volume(self, origin_volume_id, origin_snapshot_id=None, return self.client.call('Product_Order', 'placeOrder', order) + def order_modified_volume(self, volume_id, + new_size=None, new_iops=None, + new_tier_level=None): + """Places an order for modifying an existing block volume. + + :param volume_id: The ID of the volume to be modified + :param new_size: New Size/capacity for the volume + :param new_iops: The new IOPS per GB for the volume + :param new_tier_level: New Tier level for the volume + :return: Returns a SoftLayer_Container_Product_Order_Receipt + """ + + block_mask = 'id,billingItem[location,hourlyFlag],snapshotCapacityGb,'\ + 'storageType[keyName],capacityGb,originalVolumeSize,'\ + 'provisionedIops,storageTierLevel,osType[keyName],'\ + 'staasVersion,hasEncryptionAtRest' + origin_volume = self.get_block_volume_details(volume_id, + mask=block_mask) + + order = storage_utils.prepare_modify_order_object( + self, origin_volume, new_iops, new_tier_level, + new_size, 'block' + ) + + return self.client.call('Product_Order', 'placeOrder', order) + def delete_snapshot(self, snapshot_id): """Deletes the specified snapshot object. diff --git a/SoftLayer/managers/file.py b/SoftLayer/managers/file.py index 2ba39cca4..419f7c22f 100644 --- a/SoftLayer/managers/file.py +++ b/SoftLayer/managers/file.py @@ -283,6 +283,32 @@ def order_duplicate_volume(self, origin_volume_id, origin_snapshot_id=None, return self.client.call('Product_Order', 'placeOrder', order) + def order_modified_volume(self, volume_id, + new_size=None, new_iops=None, + new_tier_level=None): + """Places an order for modifying an existing file volume. + + :param volume_id: The ID of the volume to be modified + :param new_size: New Size/capacity for the volume + :param new_iops: The new IOPS per GB for the volume + :param new_tier_level: New Tier level for the volume + :return: Returns a SoftLayer_Container_Product_Order_Receipt + """ + + file_mask = 'id,billingItem[location,hourlyFlag],snapshotCapacityGb,'\ + 'storageType[keyName],capacityGb,originalVolumeSize,'\ + 'provisionedIops,storageTierLevel,osType[keyName],'\ + 'staasVersion,hasEncryptionAtRest' + origin_volume = self.get_file_volume_details(volume_id, + mask=file_mask) + + order = storage_utils.prepare_modify_order_object( + self, origin_volume, new_iops, new_tier_level, + new_size, 'file' + ) + + return self.client.call('Product_Order', 'placeOrder', order) + def delete_snapshot(self, snapshot_id): """Deletes the specified snapshot object. diff --git a/SoftLayer/managers/storage_utils.py b/SoftLayer/managers/storage_utils.py index 5040078e6..5a68312dc 100644 --- a/SoftLayer/managers/storage_utils.py +++ b/SoftLayer/managers/storage_utils.py @@ -1001,6 +1001,182 @@ def prepare_duplicate_order_object(manager, origin_volume, iops, tier, return duplicate_order +def prepare_modify_order_object(manager, origin_volume, new_iops, new_tier, + new_size, volume_type): + """Prepare the duplicate order to submit to SoftLayer_Product::placeOrder() + + :param manager: The File or Block manager calling this function + :param origin_volume: The origin volume which is being modified + :param new_iops: The new IOPS per GB for the volume (performance) + :param new_tier: The new tier level for the volume (endurance) + :param new_size: The requested new size for the volume + :param volume_type: The type of the origin volume ('file' or 'block') + :return: Returns the order object to be passed to the + placeOrder() method of the Product_Order service + """ + + # Verify that the origin volume has not been cancelled + if 'billingItem' not in origin_volume: + raise exceptions.SoftLayerError( + "The origin volume has been cancelled; " + "unable to order modify volume") + + # Ensure the origin volume is STaaS v2 or higher + # and supports Encryption at Rest + if not _staas_version_is_v2_or_above(origin_volume): + raise exceptions.SoftLayerError( + "This volume cannot be modified since it " + "does not support Encryption at Rest.") + + # Validate the requested new size, and set the size if none was given + new_size = _validate_new_size( + origin_volume, new_size, volume_type) + + # Get the appropriate package for the order + # ('storage_as_a_service' is currently used for modifying volumes) + package = get_package(manager, 'storage_as_a_service') + + # Determine the new IOPS or new tier level for the 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': + volume_is_performance = True + new_iops = _validate_new_performance_iops( + origin_volume, new_iops, new_size) + # Set up the price array for the order + prices = [ + find_price_by_category(package, 'storage_as_a_service'), + find_saas_perform_space_price(package, new_size), + find_saas_perform_iops_price(package, new_size, new_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': + volume_is_performance = False + new_tier = _validate_new_endurance_tier(origin_volume, new_tier) + # Set up the price array for the order + prices = [ + find_price_by_category(package, 'storage_as_a_service'), + find_saas_endurance_space_price(package, new_size, new_tier), + find_saas_endurance_tier_price(package, new_tier), + ] + + else: + raise exceptions.SoftLayerError( + "Origin volume does not have a valid storage type " + "(with an appropriate keyName to indicate the " + "volume is a PERFORMANCE or an ENDURANCE volume)") + + modify_order = { + 'complexType': 'SoftLayer_Container_Product_Order_' + 'Network_Storage_AsAService_Upgrade', + 'packageId': package['id'], + 'prices': prices, + 'volume': origin_volume, + #'useHourlyPricing' : 'true', + 'volumeSize': new_size + } + + if volume_is_performance: + modify_order['iops'] = new_iops + + return modify_order + + +def _validate_new_size(origin_volume, new_volume_size, + volume_type): + # Ensure the volume's current size is found + if not isinstance(utils.lookup(origin_volume, 'capacityGb'), int): + raise exceptions.SoftLayerError("Cannot find the Volume's current size.") + + # Determine the new volume size/capacity + if new_volume_size is None: + new_volume_size = origin_volume['capacityGb'] + # Ensure the new volume size is not below the minimum + elif new_volume_size < origin_volume['capacityGb']: + raise exceptions.SoftLayerError( + "The requested new size is too small. Specify a new size " + "that is at least as large as the current size.") + + # Ensure the new volume size is not above the maximum + if volume_type == 'block': + # Determine the base size for validating the requested new 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 new_volume_size > base_volume_size * 10: + raise exceptions.SoftLayerError( + "The requested new volume size is too large. The " + "maximum size for block volumes is 10 times the " + "size of the origin volume or, if the origin volume was " + "a duplicate or was modified, 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." + % (new_volume_size, base_volume_size)) + + return new_volume_size + + +def _validate_new_performance_iops(origin_volume, new_iops, + new_size): + if not isinstance(utils.lookup(origin_volume, 'provisionedIops'), str): + raise exceptions.SoftLayerError( + "Cannot find the volume's provisioned IOPS") + + if new_iops is None: + new_iops = int(origin_volume['provisionedIops']) + else: + origin_iops_per_gb = float(origin_volume['provisionedIops'])\ + / float(origin_volume['capacityGb']) + new_iops_per_gb = float(new_iops) / float(new_size) + if origin_iops_per_gb < 0.3 and new_iops_per_gb >= 0.3: + raise exceptions.SoftLayerError( + "Current volume performance is < 0.3 IOPS/GB, " + "new volume performance must also be < 0.3 " + "IOPS/GB. %s IOPS/GB (%s/%s) requested." + % (new_iops_per_gb, new_iops, new_size)) + elif origin_iops_per_gb >= 0.3 and new_iops_per_gb < 0.3: + raise exceptions.SoftLayerError( + "Current volume performance is >= 0.3 IOPS/GB, " + "new volume performance must also be >= 0.3 " + "IOPS/GB. %s IOPS/GB (%s/%s) requested." + % (new_iops_per_gb, new_iops, new_size)) + return new_iops + + +def _validate_new_endurance_tier(origin_volume, new_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 new_tier is None: + new_tier = origin_tier + else: + if new_tier != 0.25: + new_tier = int(new_tier) + + if origin_tier == 0.25: + raise exceptions.SoftLayerError( + "IOPS change is not available for Endurance volumes with 0.25 IOPS tier ") + elif origin_tier != 0.25 and new_tier == 0.25: + raise exceptions.SoftLayerError( + "Current volume performance tier is above 0.25 IOPS/GB, " + "new volume performance tier must also be above 0.25 " + "IOPS/GB. %s IOPS/GB requested." % new_tier) + return new_tier + + def _has_category(categories, category_code): return any( True From 26977f2f0b731901588068de9bdf276870df4ba0 Mon Sep 17 00:00:00 2001 From: Surya Ghatty Date: Thu, 16 Nov 2017 16:07:46 -0600 Subject: [PATCH 2/7] Updated volume details remote for modification --- SoftLayer/CLI/block/detail.py | 20 +++++++++++++------- SoftLayer/CLI/file/detail.py | 20 +++++++++++++------- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/SoftLayer/CLI/block/detail.py b/SoftLayer/CLI/block/detail.py index 70a41c0c0..6dcfcf9df 100644 --- a/SoftLayer/CLI/block/detail.py +++ b/SoftLayer/CLI/block/detail.py @@ -102,12 +102,18 @@ def cli(env, volume_id): table.add_row(['Replicant Volumes', replicant_list]) if block_volume.get('originalVolumeSize'): - duplicate_info = formatting.Table(['Original Volume Name', - block_volume['originalVolumeName']]) - duplicate_info.add_row(['Original Volume Size', - block_volume['originalVolumeSize']]) - duplicate_info.add_row(['Original Snapshot Name', - block_volume['originalSnapshotName']]) - table.add_row(['Duplicate Volume Properties', duplicate_info]) + if block_volume.get('originalVolumeSize'): + + origin_volume_info = formatting.Table(['Property', + 'Value']) + origin_volume_info.add_row(['Original Volume Size', + block_volume['originalVolumeSize']]) + if block_volume.get('originalVolumeName'): + origin_volume_info.add_row(['Original Volume Name', + block_volume['originalVolumeName']]) + if block_volume.get('originalSnapshotName'): + origin_volume_info.add_row(['Original Snapshot Name', + block_volume['originalSnapshotName']]) + table.add_row(['Original Volume Properties', origin_volume_info]) env.fout(table) diff --git a/SoftLayer/CLI/file/detail.py b/SoftLayer/CLI/file/detail.py index 96437dc36..944c021e3 100644 --- a/SoftLayer/CLI/file/detail.py +++ b/SoftLayer/CLI/file/detail.py @@ -118,12 +118,18 @@ def cli(env, volume_id): table.add_row(['Replicant Volumes', replicant_list]) if file_volume.get('originalVolumeSize'): - duplicate_info = formatting.Table(['Original Volume Name', - file_volume['originalVolumeName']]) - duplicate_info.add_row(['Original Volume Size', - file_volume['originalVolumeSize']]) - duplicate_info.add_row(['Original Snapshot Name', - file_volume['originalSnapshotName']]) - table.add_row(['Duplicate Volume Properties', duplicate_info]) + if file_volume.get('originalVolumeSize'): + + origin_volume_info = formatting.Table(['Property', + 'Value']) + origin_volume_info.add_row(['Original Volume Size', + file_volume['originalVolumeSize']]) + if file_volume.get('originalVolumeName'): + origin_volume_info.add_row(['Original Volume Name', + file_volume['originalVolumeName']]) + if file_volume.get('originalSnapshotName'): + origin_volume_info.add_row(['Original Snapshot Name', + file_volume['originalSnapshotName']]) + table.add_row(['Original Volume Properties', origin_volume_info]) env.fout(table) From ae30c6647eb6b7d427150f320830e8630b6517bb Mon Sep 17 00:00:00 2001 From: Surya Ghatty Date: Fri, 17 Nov 2017 08:22:39 -0600 Subject: [PATCH 3/7] Address pull request feedback; improve code style/formatting --- SoftLayer/CLI/block/detail.py | 12 +++++----- SoftLayer/CLI/block/modify.py | 29 ++++++++++++++++++++++++ SoftLayer/CLI/file/detail.py | 14 +++++++----- SoftLayer/CLI/file/modify.py | 34 ++++++++++++++++++++++++++--- SoftLayer/CLI/routes.py | 4 ++-- SoftLayer/managers/file.py | 2 +- SoftLayer/managers/storage_utils.py | 3 +-- tests/CLI/modules/block_tests.py | 12 +++++----- tests/CLI/modules/file_tests.py | 12 +++++----- 9 files changed, 93 insertions(+), 29 deletions(-) diff --git a/SoftLayer/CLI/block/detail.py b/SoftLayer/CLI/block/detail.py index 6dcfcf9df..7fd5a68fd 100644 --- a/SoftLayer/CLI/block/detail.py +++ b/SoftLayer/CLI/block/detail.py @@ -62,11 +62,13 @@ def cli(env, volume_id): if block_volume['activeTransactions']: for trans in block_volume['activeTransactions']: - table.add_row([ - 'Ongoing Transactions', - trans['transactionStatus']['friendlyName']]) + if trans['transactionStatus']: + table.add_row([ + 'Ongoing Transactions', + trans['transactionStatus']['friendlyName']]) - table.add_row(['Replicant Count', "%u" + if block_volume['replicationPartnerCount']: + table.add_row(['Replicant Count', "%u" % block_volume['replicationPartnerCount']]) if block_volume['replicationPartnerCount'] > 0: @@ -105,7 +107,7 @@ def cli(env, volume_id): if block_volume.get('originalVolumeSize'): origin_volume_info = formatting.Table(['Property', - 'Value']) + 'Value']) origin_volume_info.add_row(['Original Volume Size', block_volume['originalVolumeSize']]) if block_volume.get('originalVolumeName'): diff --git a/SoftLayer/CLI/block/modify.py b/SoftLayer/CLI/block/modify.py index e00bae543..ff5bcd240 100644 --- a/SoftLayer/CLI/block/modify.py +++ b/SoftLayer/CLI/block/modify.py @@ -5,6 +5,7 @@ import SoftLayer from SoftLayer.CLI import environment from SoftLayer.CLI import exceptions +from SoftLayer import utils CONTEXT_SETTINGS = {'token_normalize_func': lambda x: x.upper()} @@ -47,6 +48,34 @@ def cli(env, volume_id, new_size, new_iops, new_tier): """Modify an existing block storage volume.""" block_manager = SoftLayer.BlockStorageManager(env.client) + block_volume = block_manager.get_block_volume_details(volume_id) + block_volume = utils.NestedDict(block_volume) + + storage_type = block_volume['storageType']['keyName'].split('_').pop(0) + help_message = "For help, try \"slcli block volume-modify --help\"." + + if storage_type == 'ENDURANCE': + if new_iops is not None: + raise exceptions.CLIAbort( + 'Invalid option --new-iops for Endurance volume. Please use --new-tier instead.') + + if new_size is None and new_tier is None: + raise exceptions.CLIAbort( + 'Option --new-size or --new-tier must be specified for modifying an Endurance volume. \n'+ + help_message + ) + + if storage_type == 'PERFORMANCE': + if new_tier is not None: + raise exceptions.CLIAbort( + 'Invalid option --new-tier for Performance volume. Please use --new-iops instead.') + + if new_size is None and new_iops is None: + raise exceptions.CLIAbort( + 'Option --new-size or --new-iops must be specified for modifying a Performance volume. \n' + + help_message + ) + if new_tier is not None: new_tier = float(new_tier) diff --git a/SoftLayer/CLI/file/detail.py b/SoftLayer/CLI/file/detail.py index 944c021e3..251192785 100644 --- a/SoftLayer/CLI/file/detail.py +++ b/SoftLayer/CLI/file/detail.py @@ -78,12 +78,14 @@ def cli(env, volume_id): if file_volume['activeTransactions']: for trans in file_volume['activeTransactions']: - table.add_row([ - 'Ongoing Transactions', - trans['transactionStatus']['friendlyName']]) + if trans['transactionStatus'] and trans['transactionStatus']['friendlyName']: + table.add_row([ + 'Ongoing Transactions', + trans['transactionStatus']['friendlyName']]) - table.add_row(['Replicant Count', "%u" - % file_volume['replicationPartnerCount']]) + if file_volume['replicationPartnerCount']: + table.add_row(['Replicant Count', "%u" + % file_volume['replicationPartnerCount']]) if file_volume['replicationPartnerCount'] > 0: # This if/else temporarily handles a bug in which the SL API @@ -121,7 +123,7 @@ def cli(env, volume_id): if file_volume.get('originalVolumeSize'): origin_volume_info = formatting.Table(['Property', - 'Value']) + 'Value']) origin_volume_info.add_row(['Original Volume Size', file_volume['originalVolumeSize']]) if file_volume.get('originalVolumeName'): diff --git a/SoftLayer/CLI/file/modify.py b/SoftLayer/CLI/file/modify.py index 7346c8dc1..8a485c1f8 100644 --- a/SoftLayer/CLI/file/modify.py +++ b/SoftLayer/CLI/file/modify.py @@ -5,6 +5,7 @@ import SoftLayer from SoftLayer.CLI import environment from SoftLayer.CLI import exceptions +from SoftLayer import utils CONTEXT_SETTINGS = {'token_normalize_func': lambda x: x.upper()} @@ -20,8 +21,7 @@ 'Potential Sizes: [20, 40, 80, 100, 250, ' '500, 1000, 2000, 4000, 8000, 12000] ' 'Minimum: [the size of the origin volume] ' - 'Maximum: [the minimum of 12000 GB or ' - '10*(origin volume size)]') + 'Maximum: 12000 GB.') @click.option('--new-iops', '-i', type=int, help='Performance Storage IOPS, between 100 and 6000 in ' @@ -37,7 +37,7 @@ help='Endurance Storage Tier (IOPS per GB) [only used for ' 'endurance volumes] ***If no tier is specified, the original tier ' 'of the volume will be used.***\n' - 'Requirements: [IOPS/GB for the volume is 0.25, ' + 'Requirements: [If original IOPS/GB for the volume is 0.25, ' 'new IOPS/GB for the volume must also be 0.25. If original IOPS/GB ' 'for the volume is greater than 0.25, new IOPS/GB ' 'for the volume must also be greater than 0.25.]', @@ -46,6 +46,34 @@ def cli(env, volume_id, new_size, new_iops, new_tier): """Modify an existing file storage volume.""" file_manager = SoftLayer.FileStorageManager(env.client) + + file_volume = file_manager.get_file_volume_details(volume_id) + file_volume = utils.NestedDict(file_volume) + + storage_type = file_volume['storageType']['keyName'].split('_').pop(0) + help_message = "For help, try \"slcli file volume-modify --help\"" + + if storage_type == 'ENDURANCE': + if new_iops is not None: + raise exceptions.CLIAbort( + 'Invalid option --new-iops for Endurance volume. Please use --new-tier instead.') + + if new_size is None and new_tier is None: + raise exceptions.CLIAbort( + 'Option --new-size or --new-tier must be specified for modifying an Endurance volume. \n'+ + help_message + ) + + if storage_type == 'PERFORMANCE': + if new_tier is not None: + raise exceptions.CLIAbort( + 'Invalid option --new-tier for Performance volume. Please use --new-iops instead.') + + if new_size is None and new_iops is None: + raise exceptions.CLIAbort( + 'Option --new-size or --new-iops must be specified for modifying a Performance volume. \n' + + help_message + ) if new_tier is not None: new_tier = float(new_tier) diff --git a/SoftLayer/CLI/routes.py b/SoftLayer/CLI/routes.py index 52e338b50..3b406ec25 100644 --- a/SoftLayer/CLI/routes.py +++ b/SoftLayer/CLI/routes.py @@ -79,8 +79,8 @@ ('block:volume-count', 'SoftLayer.CLI.block.count:cli'), ('block:volume-detail', 'SoftLayer.CLI.block.detail:cli'), ('block:volume-duplicate', 'SoftLayer.CLI.block.duplicate:cli'), - ('block:volume-modify', 'SoftLayer.CLI.block.modify:cli'), ('block:volume-list', 'SoftLayer.CLI.block.list:cli'), + ('block:volume-modify', 'SoftLayer.CLI.block.modify:cli'), ('block:volume-order', 'SoftLayer.CLI.block.order:cli'), ('block:volume-set-lun-id', 'SoftLayer.CLI.block.lun:cli'), @@ -105,8 +105,8 @@ ('file:volume-count', 'SoftLayer.CLI.file.count:cli'), ('file:volume-detail', 'SoftLayer.CLI.file.detail:cli'), ('file:volume-duplicate', 'SoftLayer.CLI.file.duplicate:cli'), - ('file:volume-modify', 'SoftLayer.CLI.file.modify:cli'), ('file:volume-list', 'SoftLayer.CLI.file.list:cli'), + ('file:volume-modify', 'SoftLayer.CLI.file.modify:cli'), ('file:volume-order', 'SoftLayer.CLI.file.order:cli'), ('firewall', 'SoftLayer.CLI.firewall'), diff --git a/SoftLayer/managers/file.py b/SoftLayer/managers/file.py index 419f7c22f..ab322fd0b 100644 --- a/SoftLayer/managers/file.py +++ b/SoftLayer/managers/file.py @@ -297,7 +297,7 @@ def order_modified_volume(self, volume_id, file_mask = 'id,billingItem[location,hourlyFlag],snapshotCapacityGb,'\ 'storageType[keyName],capacityGb,originalVolumeSize,'\ - 'provisionedIops,storageTierLevel,osType[keyName],'\ + 'provisionedIops,storageTierLevel,'\ 'staasVersion,hasEncryptionAtRest' origin_volume = self.get_file_volume_details(volume_id, mask=file_mask) diff --git a/SoftLayer/managers/storage_utils.py b/SoftLayer/managers/storage_utils.py index 5a68312dc..9a57ff626 100644 --- a/SoftLayer/managers/storage_utils.py +++ b/SoftLayer/managers/storage_utils.py @@ -1003,7 +1003,7 @@ def prepare_duplicate_order_object(manager, origin_volume, iops, tier, def prepare_modify_order_object(manager, origin_volume, new_iops, new_tier, new_size, volume_type): - """Prepare the duplicate order to submit to SoftLayer_Product::placeOrder() + """Prepare the modification order to submit to SoftLayer_Product::placeOrder() :param manager: The File or Block manager calling this function :param origin_volume: The origin volume which is being modified @@ -1078,7 +1078,6 @@ def prepare_modify_order_object(manager, origin_volume, new_iops, new_tier, 'packageId': package['id'], 'prices': prices, 'volume': origin_volume, - #'useHourlyPricing' : 'true', 'volumeSize': new_size } diff --git a/tests/CLI/modules/block_tests.py b/tests/CLI/modules/block_tests.py index d7ad2c3c1..f78f8b4a6 100644 --- a/tests/CLI/modules/block_tests.py +++ b/tests/CLI/modules/block_tests.py @@ -86,11 +86,13 @@ def test_volume_detail(self): {'Replicant ID': 'Data Center', '1785': 'dal01'}, {'Replicant ID': 'Schedule', '1785': 'REPLICATION_DAILY'}, ]], - 'Duplicate Volume Properties': [ - {'Original Volume Name': 'Original Volume Size', - 'test-origin-volume-name': '20'}, - {'Original Volume Name': 'Original Snapshot Name', - 'test-origin-volume-name': 'test-origin-snapshot-name'} + 'Original Volume Properties': [ + {'Property': 'Original Volume Size', + 'Value': '20'}, + {'Property': 'Original Volume Name', + 'Value': 'test-origin-volume-name'}, + {'Property': 'Original Snapshot Name', + 'Value': 'test-origin-snapshot-name'} ] }, json.loads(result.output)) diff --git a/tests/CLI/modules/file_tests.py b/tests/CLI/modules/file_tests.py index 6aad4564d..ff2653dc7 100644 --- a/tests/CLI/modules/file_tests.py +++ b/tests/CLI/modules/file_tests.py @@ -126,11 +126,13 @@ def test_volume_detail(self): {'Replicant ID': 'Data Center', '1785': 'dal01'}, {'Replicant ID': 'Schedule', '1785': 'REPLICATION_DAILY'}, ]], - 'Duplicate Volume Properties': [ - {'Original Volume Name': 'Original Volume Size', - 'test-origin-volume-name': '20'}, - {'Original Volume Name': 'Original Snapshot Name', - 'test-origin-volume-name': 'test-origin-snapshot-name'} + 'Original Volume Properties': [ + {'Property': 'Original Volume Size', + 'Value': '20'}, + {'Property': 'Original Volume Name', + 'Value': 'test-origin-volume-name'}, + {'Property': 'Original Snapshot Name', + 'Value': 'test-origin-snapshot-name'} ] }, json.loads(result.output)) From 745867a9b067142c78f0cb76e9bd9122f795bd22 Mon Sep 17 00:00:00 2001 From: David Pickle Date: Wed, 29 Nov 2017 15:29:32 -0600 Subject: [PATCH 4/7] Address PR feedback; remove validation which duplicates API --- SoftLayer/CLI/block/detail.py | 26 +-- SoftLayer/CLI/block/duplicate.py | 4 +- SoftLayer/CLI/block/modify.py | 72 ++----- SoftLayer/CLI/file/detail.py | 26 +-- SoftLayer/CLI/file/modify.py | 71 ++----- .../fixtures/SoftLayer_Network_Storage.py | 4 +- SoftLayer/managers/block.py | 22 +-- SoftLayer/managers/file.py | 22 +-- SoftLayer/managers/storage_utils.py | 177 ++++-------------- tests/CLI/modules/block_tests.py | 4 +- tests/CLI/modules/file_tests.py | 4 +- 11 files changed, 109 insertions(+), 323 deletions(-) diff --git a/SoftLayer/CLI/block/detail.py b/SoftLayer/CLI/block/detail.py index 7fd5a68fd..050121d5a 100644 --- a/SoftLayer/CLI/block/detail.py +++ b/SoftLayer/CLI/block/detail.py @@ -62,14 +62,10 @@ def cli(env, volume_id): if block_volume['activeTransactions']: for trans in block_volume['activeTransactions']: - if trans['transactionStatus']: - table.add_row([ - 'Ongoing Transactions', - trans['transactionStatus']['friendlyName']]) + if isinstance(utils.lookup(trans, 'transactionStatus', 'friendlyName'), str): + table.add_row(['Ongoing Transaction', trans['transactionStatus']['friendlyName']]) - if block_volume['replicationPartnerCount']: - table.add_row(['Replicant Count', "%u" - % block_volume['replicationPartnerCount']]) + table.add_row(['Replicant Count', "%u" % block_volume.get('replicationPartnerCount', 0)]) if block_volume['replicationPartnerCount'] > 0: # This if/else temporarily handles a bug in which the SL API @@ -104,18 +100,12 @@ def cli(env, volume_id): table.add_row(['Replicant Volumes', replicant_list]) if block_volume.get('originalVolumeSize'): - if block_volume.get('originalVolumeSize'): - - origin_volume_info = formatting.Table(['Property', - 'Value']) - origin_volume_info.add_row(['Original Volume Size', - block_volume['originalVolumeSize']]) + original_volume_info = formatting.Table(['Property', 'Value']) + original_volume_info.add_row(['Original Volume Size', block_volume['originalVolumeSize']]) if block_volume.get('originalVolumeName'): - origin_volume_info.add_row(['Original Volume Name', - block_volume['originalVolumeName']]) + original_volume_info.add_row(['Original Volume Name', block_volume['originalVolumeName']]) if block_volume.get('originalSnapshotName'): - origin_volume_info.add_row(['Original Snapshot Name', - block_volume['originalSnapshotName']]) - table.add_row(['Original Volume Properties', origin_volume_info]) + original_volume_info.add_row(['Original Snapshot Name', block_volume['originalSnapshotName']]) + table.add_row(['Original Volume Properties', original_volume_info]) env.fout(table) diff --git a/SoftLayer/CLI/block/duplicate.py b/SoftLayer/CLI/block/duplicate.py index 0ecf59110..ec728f87c 100644 --- a/SoftLayer/CLI/block/duplicate.py +++ b/SoftLayer/CLI/block/duplicate.py @@ -22,9 +22,7 @@ 'the origin volume will be used.***\n' 'Potential Sizes: [20, 40, 80, 100, 250, ' '500, 1000, 2000, 4000, 8000, 12000] ' - 'Minimum: [the size of the origin volume] ' - 'Maximum: [the minimum of 12000 GB or ' - '10*(origin volume size)]') + 'Minimum: [the size of the origin volume]') @click.option('--duplicate-iops', '-i', type=int, help='Performance Storage IOPS, between 100 and 6000 in ' diff --git a/SoftLayer/CLI/block/modify.py b/SoftLayer/CLI/block/modify.py index ff5bcd240..3697ddd79 100644 --- a/SoftLayer/CLI/block/modify.py +++ b/SoftLayer/CLI/block/modify.py @@ -5,7 +5,6 @@ import SoftLayer from SoftLayer.CLI import environment from SoftLayer.CLI import exceptions -from SoftLayer import utils CONTEXT_SETTINGS = {'token_normalize_func': lambda x: x.upper()} @@ -15,67 +14,28 @@ @click.argument('volume-id') @click.option('--new-size', '-c', type=int, - help='New Size of block volume in GB. ' - '***If no size is specified, the original size of ' - 'volume will be used.***\n' - 'Potential Sizes: [20, 40, 80, 100, 250, ' - '500, 1000, 2000, 4000, 8000, 12000] ' - 'Minimum: [the size of the origin volume] ' - 'Maximum: [the minimum of 12000 GB or ' - '10*(origin volume size)]') + help='New Size of block volume in GB. ***If no size is given, the original size of volume is used.***\n' + 'Potential Sizes: [20, 40, 80, 100, 250, 500, 1000, 2000, 4000, 8000, 12000]\n' + 'Minimum: [the original size of the volume]') @click.option('--new-iops', '-i', type=int, - help='Performance Storage IOPS, between 100 and 6000 in ' - 'multiples of 100 [only used for performance volumes] ' - '***If no IOPS value is specified, the original IOPS value of the ' - 'volume will be used.***\n' - 'Requirements: [If original IOPS/GB for the volume is less ' - 'than 0.3, new IOPS/GB must also be less ' - 'than 0.3. If original IOPS/GB for the volume is greater ' - 'than or equal to 0.3, new IOPS/GB for the volume must ' - 'also be greater than or equal to 0.3.]') + help='Performance Storage IOPS, between 100 and 6000 in multiples of 100 [only for performance volumes] ' + '***If no IOPS value is specified, the original IOPS value of the volume will be used.***\n' + 'Requirements: [If original IOPS/GB for the volume is less than 0.3, new IOPS/GB must also be ' + 'less than 0.3. If original IOPS/GB for the volume is greater than or equal to 0.3, new IOPS/GB ' + 'for the volume must also be greater than or equal to 0.3.]') @click.option('--new-tier', '-t', - help='Endurance Storage Tier (IOPS per GB) [only used for ' - 'endurance volumes] ***If no tier is specified, the original tier ' - 'of the volume will be used.***\n' - 'Requirements: [If original IOPS/GB for the volume is 0.25, ' - 'new IOPS/GB for the volume must also be 0.25. If original IOPS/GB ' - 'for the volume is greater than 0.25, new IOPS/GB ' - 'for the volume must also be greater than 0.25.]', + help='Endurance Storage Tier (IOPS per GB) [only for endurance volumes] ' + '***If no tier is specified, the original tier of the volume will be used.***\n' + 'Requirements: [If original IOPS/GB for the volume is 0.25, new IOPS/GB for the volume must also ' + 'be 0.25. If original IOPS/GB for the volume is greater than 0.25, new IOPS/GB for the volume ' + 'must also be greater than 0.25.]', type=click.Choice(['0.25', '2', '4', '10'])) @environment.pass_env def cli(env, volume_id, new_size, new_iops, new_tier): """Modify an existing block storage volume.""" block_manager = SoftLayer.BlockStorageManager(env.client) - block_volume = block_manager.get_block_volume_details(volume_id) - block_volume = utils.NestedDict(block_volume) - - storage_type = block_volume['storageType']['keyName'].split('_').pop(0) - help_message = "For help, try \"slcli block volume-modify --help\"." - - if storage_type == 'ENDURANCE': - if new_iops is not None: - raise exceptions.CLIAbort( - 'Invalid option --new-iops for Endurance volume. Please use --new-tier instead.') - - if new_size is None and new_tier is None: - raise exceptions.CLIAbort( - 'Option --new-size or --new-tier must be specified for modifying an Endurance volume. \n'+ - help_message - ) - - if storage_type == 'PERFORMANCE': - if new_tier is not None: - raise exceptions.CLIAbort( - 'Invalid option --new-tier for Performance volume. Please use --new-iops instead.') - - if new_size is None and new_iops is None: - raise exceptions.CLIAbort( - 'Option --new-size or --new-iops must be specified for modifying a Performance volume. \n' + - help_message - ) - if new_tier is not None: new_tier = float(new_tier) @@ -90,10 +50,8 @@ def cli(env, volume_id, new_size, new_iops, new_tier): raise exceptions.ArgumentError(str(ex)) if 'placedOrder' in order.keys(): - click.echo("Order #{0} placed successfully!".format( - order['placedOrder']['id'])) + click.echo("Order #{0} placed successfully!".format(order['placedOrder']['id'])) for item in order['placedOrder']['items']: click.echo(" > %s" % item['description']) else: - click.echo("Order could not be placed! Please verify your options " + - "and try again.") + click.echo("Order could not be placed! Please verify your options and try again.") diff --git a/SoftLayer/CLI/file/detail.py b/SoftLayer/CLI/file/detail.py index 251192785..9adcacb2b 100644 --- a/SoftLayer/CLI/file/detail.py +++ b/SoftLayer/CLI/file/detail.py @@ -78,14 +78,10 @@ def cli(env, volume_id): if file_volume['activeTransactions']: for trans in file_volume['activeTransactions']: - if trans['transactionStatus'] and trans['transactionStatus']['friendlyName']: - table.add_row([ - 'Ongoing Transactions', - trans['transactionStatus']['friendlyName']]) + if isinstance(utils.lookup(trans, 'transactionStatus', 'friendlyName'), str): + table.add_row(['Ongoing Transaction', trans['transactionStatus']['friendlyName']]) - if file_volume['replicationPartnerCount']: - table.add_row(['Replicant Count', "%u" - % file_volume['replicationPartnerCount']]) + table.add_row(['Replicant Count', "%u" % file_volume.get('replicationPartnerCount', 0)]) if file_volume['replicationPartnerCount'] > 0: # This if/else temporarily handles a bug in which the SL API @@ -120,18 +116,12 @@ def cli(env, volume_id): table.add_row(['Replicant Volumes', replicant_list]) if file_volume.get('originalVolumeSize'): - if file_volume.get('originalVolumeSize'): - - origin_volume_info = formatting.Table(['Property', - 'Value']) - origin_volume_info.add_row(['Original Volume Size', - file_volume['originalVolumeSize']]) + original_volume_info = formatting.Table(['Property', 'Value']) + original_volume_info.add_row(['Original Volume Size', file_volume['originalVolumeSize']]) if file_volume.get('originalVolumeName'): - origin_volume_info.add_row(['Original Volume Name', - file_volume['originalVolumeName']]) + original_volume_info.add_row(['Original Volume Name', file_volume['originalVolumeName']]) if file_volume.get('originalSnapshotName'): - origin_volume_info.add_row(['Original Snapshot Name', - file_volume['originalSnapshotName']]) - table.add_row(['Original Volume Properties', origin_volume_info]) + original_volume_info.add_row(['Original Snapshot Name', file_volume['originalSnapshotName']]) + table.add_row(['Original Volume Properties', original_volume_info]) env.fout(table) diff --git a/SoftLayer/CLI/file/modify.py b/SoftLayer/CLI/file/modify.py index 8a485c1f8..5e0c097b3 100644 --- a/SoftLayer/CLI/file/modify.py +++ b/SoftLayer/CLI/file/modify.py @@ -5,7 +5,6 @@ import SoftLayer from SoftLayer.CLI import environment from SoftLayer.CLI import exceptions -from SoftLayer import utils CONTEXT_SETTINGS = {'token_normalize_func': lambda x: x.upper()} @@ -15,65 +14,27 @@ @click.argument('volume-id') @click.option('--new-size', '-c', type=int, - help='New Size of file volume in GB. ' - '***If no size is specified, the original size of ' - 'volume will be used.***\n' - 'Potential Sizes: [20, 40, 80, 100, 250, ' - '500, 1000, 2000, 4000, 8000, 12000] ' - 'Minimum: [the size of the origin volume] ' - 'Maximum: 12000 GB.') + help='New Size of file volume in GB. ***If no size is given, the original size of volume is used.***\n' + 'Potential Sizes: [20, 40, 80, 100, 250, 500, 1000, 2000, 4000, 8000, 12000]\n' + 'Minimum: [the original size of the volume]') @click.option('--new-iops', '-i', type=int, - help='Performance Storage IOPS, between 100 and 6000 in ' - 'multiples of 100 [only used for performance volumes] ' - '***If no IOPS value is specified, the original IOPS value of the ' - 'volume will be used.***\n' - 'Requirements: [If original IOPS/GB for the volume is less ' - 'than 0.3, new IOPS/GB must also be less ' - 'than 0.3. If original IOPS/GB for the volume is greater ' - 'than or equal to 0.3, new IOPS/GB for the volume must ' - 'also be greater than or equal to 0.3.]') + help='Performance Storage IOPS, between 100 and 6000 in multiples of 100 [only for performance volumes] ' + '***If no IOPS value is specified, the original IOPS value of the volume will be used.***\n' + 'Requirements: [If original IOPS/GB for the volume is less than 0.3, new IOPS/GB must also be ' + 'less than 0.3. If original IOPS/GB for the volume is greater than or equal to 0.3, new IOPS/GB ' + 'for the volume must also be greater than or equal to 0.3.]') @click.option('--new-tier', '-t', - help='Endurance Storage Tier (IOPS per GB) [only used for ' - 'endurance volumes] ***If no tier is specified, the original tier ' - 'of the volume will be used.***\n' - 'Requirements: [If original IOPS/GB for the volume is 0.25, ' - 'new IOPS/GB for the volume must also be 0.25. If original IOPS/GB ' - 'for the volume is greater than 0.25, new IOPS/GB ' - 'for the volume must also be greater than 0.25.]', + help='Endurance Storage Tier (IOPS per GB) [only for endurance volumes] ' + '***If no tier is specified, the original tier of the volume will be used.***\n' + 'Requirements: [If original IOPS/GB for the volume is 0.25, new IOPS/GB for the volume must also ' + 'be 0.25. If original IOPS/GB for the volume is greater than 0.25, new IOPS/GB for the volume ' + 'must also be greater than 0.25.]', type=click.Choice(['0.25', '2', '4', '10'])) @environment.pass_env def cli(env, volume_id, new_size, new_iops, new_tier): """Modify an existing file storage volume.""" file_manager = SoftLayer.FileStorageManager(env.client) - - file_volume = file_manager.get_file_volume_details(volume_id) - file_volume = utils.NestedDict(file_volume) - - storage_type = file_volume['storageType']['keyName'].split('_').pop(0) - help_message = "For help, try \"slcli file volume-modify --help\"" - - if storage_type == 'ENDURANCE': - if new_iops is not None: - raise exceptions.CLIAbort( - 'Invalid option --new-iops for Endurance volume. Please use --new-tier instead.') - - if new_size is None and new_tier is None: - raise exceptions.CLIAbort( - 'Option --new-size or --new-tier must be specified for modifying an Endurance volume. \n'+ - help_message - ) - - if storage_type == 'PERFORMANCE': - if new_tier is not None: - raise exceptions.CLIAbort( - 'Invalid option --new-tier for Performance volume. Please use --new-iops instead.') - - if new_size is None and new_iops is None: - raise exceptions.CLIAbort( - 'Option --new-size or --new-iops must be specified for modifying a Performance volume. \n' + - help_message - ) if new_tier is not None: new_tier = float(new_tier) @@ -89,10 +50,8 @@ def cli(env, volume_id, new_size, new_iops, new_tier): raise exceptions.ArgumentError(str(ex)) if 'placedOrder' in order.keys(): - click.echo("Order #{0} placed successfully!".format( - order['placedOrder']['id'])) + click.echo("Order #{0} placed successfully!".format(order['placedOrder']['id'])) for item in order['placedOrder']['items']: click.echo(" > %s" % item['description']) else: - click.echo("Order could not be placed! Please verify your options " + - "and try again.") + click.echo("Order could not be placed! Please verify your options and try again.") diff --git a/SoftLayer/fixtures/SoftLayer_Network_Storage.py b/SoftLayer/fixtures/SoftLayer_Network_Storage.py index b4dd0b751..81d377ddd 100644 --- a/SoftLayer/fixtures/SoftLayer_Network_Storage.py +++ b/SoftLayer/fixtures/SoftLayer_Network_Storage.py @@ -104,8 +104,8 @@ 'lunId': 2, 'nasType': 'ISCSI', 'notes': """{'status': 'available'}""", - 'originalSnapshotName': 'test-origin-snapshot-name', - 'originalVolumeName': 'test-origin-volume-name', + 'originalSnapshotName': 'test-original-snapshot-name', + 'originalVolumeName': 'test-original-volume-name', 'originalVolumeSize': '20', 'osType': {'keyName': 'LINUX'}, 'parentVolume': {'snapshotSizeBytes': 1024}, diff --git a/SoftLayer/managers/block.py b/SoftLayer/managers/block.py index 06394830c..bc5cb8798 100644 --- a/SoftLayer/managers/block.py +++ b/SoftLayer/managers/block.py @@ -303,28 +303,22 @@ def order_duplicate_volume(self, origin_volume_id, origin_snapshot_id=None, return self.client.call('Product_Order', 'placeOrder', order) - def order_modified_volume(self, volume_id, - new_size=None, new_iops=None, - new_tier_level=None): + def order_modified_volume(self, volume_id, new_size=None, new_iops=None, new_tier_level=None): """Places an order for modifying an existing block volume. :param volume_id: The ID of the volume to be modified - :param new_size: New Size/capacity for the volume - :param new_iops: The new IOPS per GB for the volume - :param new_tier_level: New Tier level for the volume + :param new_size: The new size/capacity for the volume + :param new_iops: The new IOPS for the volume + :param new_tier_level: The new tier level for the volume :return: Returns a SoftLayer_Container_Product_Order_Receipt """ - block_mask = 'id,billingItem[location,hourlyFlag],snapshotCapacityGb,'\ - 'storageType[keyName],capacityGb,originalVolumeSize,'\ - 'provisionedIops,storageTierLevel,osType[keyName],'\ - 'staasVersion,hasEncryptionAtRest' - origin_volume = self.get_block_volume_details(volume_id, - mask=block_mask) + block_mask = 'id,billingItem,storageType[keyName],capacityGb,provisionedIops,'\ + 'storageTierLevel,staasVersion,hasEncryptionAtRest' + volume = self.get_block_volume_details(volume_id, mask=block_mask) order = storage_utils.prepare_modify_order_object( - self, origin_volume, new_iops, new_tier_level, - new_size, 'block' + self, volume, new_iops, new_tier_level, new_size ) return self.client.call('Product_Order', 'placeOrder', order) diff --git a/SoftLayer/managers/file.py b/SoftLayer/managers/file.py index ab322fd0b..5646b7399 100644 --- a/SoftLayer/managers/file.py +++ b/SoftLayer/managers/file.py @@ -283,28 +283,22 @@ def order_duplicate_volume(self, origin_volume_id, origin_snapshot_id=None, return self.client.call('Product_Order', 'placeOrder', order) - def order_modified_volume(self, volume_id, - new_size=None, new_iops=None, - new_tier_level=None): + def order_modified_volume(self, volume_id, new_size=None, new_iops=None, new_tier_level=None): """Places an order for modifying an existing file volume. :param volume_id: The ID of the volume to be modified - :param new_size: New Size/capacity for the volume - :param new_iops: The new IOPS per GB for the volume - :param new_tier_level: New Tier level for the volume + :param new_size: The new size/capacity for the volume + :param new_iops: The new IOPS for the volume + :param new_tier_level: The new tier level for the volume :return: Returns a SoftLayer_Container_Product_Order_Receipt """ - file_mask = 'id,billingItem[location,hourlyFlag],snapshotCapacityGb,'\ - 'storageType[keyName],capacityGb,originalVolumeSize,'\ - 'provisionedIops,storageTierLevel,'\ - 'staasVersion,hasEncryptionAtRest' - origin_volume = self.get_file_volume_details(volume_id, - mask=file_mask) + file_mask = 'id,billingItem,storageType[keyName],capacityGb,provisionedIops,'\ + 'storageTierLevel,staasVersion,hasEncryptionAtRest' + volume = self.get_file_volume_details(volume_id, mask=file_mask) order = storage_utils.prepare_modify_order_object( - self, origin_volume, new_iops, new_tier_level, - new_size, 'file' + self, volume, new_iops, new_tier_level, new_size ) return self.client.call('Product_Order', 'placeOrder', order) diff --git a/SoftLayer/managers/storage_utils.py b/SoftLayer/managers/storage_utils.py index 9a57ff626..84e4d7c1a 100644 --- a/SoftLayer/managers/storage_utils.py +++ b/SoftLayer/managers/storage_utils.py @@ -947,8 +947,7 @@ def prepare_duplicate_order_object(manager, origin_volume, iops, tier, if iops is None: iops = int(origin_volume.get('provisionedIops', 0)) if iops <= 0: - raise exceptions.SoftLayerError( - "Cannot find origin volume's provisioned IOPS") + 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'), @@ -1001,65 +1000,61 @@ def prepare_duplicate_order_object(manager, origin_volume, iops, tier, return duplicate_order -def prepare_modify_order_object(manager, origin_volume, new_iops, new_tier, - new_size, volume_type): +def prepare_modify_order_object(manager, volume, new_iops, new_tier, new_size): """Prepare the modification order to submit to SoftLayer_Product::placeOrder() :param manager: The File or Block manager calling this function - :param origin_volume: The origin volume which is being modified - :param new_iops: The new IOPS per GB for the volume (performance) + :param volume: The volume which is being modified + :param new_iops: The new IOPS for the volume (performance) :param new_tier: The new tier level for the volume (endurance) :param new_size: The requested new size for the volume - :param volume_type: The type of the origin volume ('file' or 'block') - :return: Returns the order object to be passed to the - placeOrder() method of the Product_Order service + :return: Returns the order object to be passed to the placeOrder() method of the Product_Order service """ # Verify that the origin volume has not been cancelled - if 'billingItem' not in origin_volume: - raise exceptions.SoftLayerError( - "The origin volume has been cancelled; " - "unable to order modify volume") - - # Ensure the origin volume is STaaS v2 or higher - # and supports Encryption at Rest - if not _staas_version_is_v2_or_above(origin_volume): - raise exceptions.SoftLayerError( - "This volume cannot be modified since it " - "does not support Encryption at Rest.") + if 'billingItem' not in volume: + raise exceptions.SoftLayerError("The volume has been cancelled; unable to modify volume") - # Validate the requested new size, and set the size if none was given - new_size = _validate_new_size( - origin_volume, new_size, volume_type) + # Ensure the origin volume is STaaS v2 or higher and supports Encryption at Rest + if not _staas_version_is_v2_or_above(volume): + raise exceptions.SoftLayerError("This volume cannot be modified since it does not support Encryption at Rest.") - # Get the appropriate package for the order - # ('storage_as_a_service' is currently used for modifying volumes) + # Get the appropriate package for the order ('storage_as_a_service' is currently used for modifying volumes) package = get_package(manager, 'storage_as_a_service') - # Determine the new IOPS or new tier level for the 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': + # Based on volume storage type, ensure at least one volume property is being modified, + # use current values if some are not specified, and lookup price codes for the order + volume_storage_type = volume['storageType']['keyName'] + if 'PERFORMANCE' in volume_storage_type: volume_is_performance = True - new_iops = _validate_new_performance_iops( - origin_volume, new_iops, new_size) - # Set up the price array for the order + if new_size is None and new_iops is None: + raise exceptions.SoftLayerError("A size or IOPS value must be given to modify this performance volume.") + + if new_size is None: + new_size = volume['capacityGb'] + elif new_iops is None: + new_iops = int(volume.get('provisionedIops', 0)) + if new_iops <= 0: + raise exceptions.SoftLayerError("Cannot find volume's provisioned IOPS") + + # Set up the prices array for the order prices = [ find_price_by_category(package, 'storage_as_a_service'), find_saas_perform_space_price(package, new_size), find_saas_perform_iops_price(package, new_size, new_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 volume_storage_type: volume_is_performance = False - new_tier = _validate_new_endurance_tier(origin_volume, new_tier) - # Set up the price array for the order + if new_size is None and new_tier is None: + raise exceptions.SoftLayerError("A size or tier value must be given to modify this endurance volume.") + + if new_size is None: + new_size = volume['capacityGb'] + elif new_tier is None: + new_tier = find_endurance_tier_iops_per_gb(volume) + + # Set up the prices array for the order prices = [ find_price_by_category(package, 'storage_as_a_service'), find_saas_endurance_space_price(package, new_size, new_tier), @@ -1067,17 +1062,14 @@ def prepare_modify_order_object(manager, origin_volume, new_iops, new_tier, ] else: - raise exceptions.SoftLayerError( - "Origin volume does not have a valid storage type " - "(with an appropriate keyName to indicate the " - "volume is a PERFORMANCE or an ENDURANCE volume)") + raise exceptions.SoftLayerError("Origin volume does not have a valid storage type (with an appropriate " + "keyName to indicate the volume is a PERFORMANCE or an ENDURANCE volume)") modify_order = { - 'complexType': 'SoftLayer_Container_Product_Order_' - 'Network_Storage_AsAService_Upgrade', + 'complexType': 'SoftLayer_Container_Product_Order_Network_Storage_AsAService_Upgrade', 'packageId': package['id'], 'prices': prices, - 'volume': origin_volume, + 'volume': {'id': volume['id']}, 'volumeSize': new_size } @@ -1087,95 +1079,6 @@ def prepare_modify_order_object(manager, origin_volume, new_iops, new_tier, return modify_order -def _validate_new_size(origin_volume, new_volume_size, - volume_type): - # Ensure the volume's current size is found - if not isinstance(utils.lookup(origin_volume, 'capacityGb'), int): - raise exceptions.SoftLayerError("Cannot find the Volume's current size.") - - # Determine the new volume size/capacity - if new_volume_size is None: - new_volume_size = origin_volume['capacityGb'] - # Ensure the new volume size is not below the minimum - elif new_volume_size < origin_volume['capacityGb']: - raise exceptions.SoftLayerError( - "The requested new size is too small. Specify a new size " - "that is at least as large as the current size.") - - # Ensure the new volume size is not above the maximum - if volume_type == 'block': - # Determine the base size for validating the requested new 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 new_volume_size > base_volume_size * 10: - raise exceptions.SoftLayerError( - "The requested new volume size is too large. The " - "maximum size for block volumes is 10 times the " - "size of the origin volume or, if the origin volume was " - "a duplicate or was modified, 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." - % (new_volume_size, base_volume_size)) - - return new_volume_size - - -def _validate_new_performance_iops(origin_volume, new_iops, - new_size): - if not isinstance(utils.lookup(origin_volume, 'provisionedIops'), str): - raise exceptions.SoftLayerError( - "Cannot find the volume's provisioned IOPS") - - if new_iops is None: - new_iops = int(origin_volume['provisionedIops']) - else: - origin_iops_per_gb = float(origin_volume['provisionedIops'])\ - / float(origin_volume['capacityGb']) - new_iops_per_gb = float(new_iops) / float(new_size) - if origin_iops_per_gb < 0.3 and new_iops_per_gb >= 0.3: - raise exceptions.SoftLayerError( - "Current volume performance is < 0.3 IOPS/GB, " - "new volume performance must also be < 0.3 " - "IOPS/GB. %s IOPS/GB (%s/%s) requested." - % (new_iops_per_gb, new_iops, new_size)) - elif origin_iops_per_gb >= 0.3 and new_iops_per_gb < 0.3: - raise exceptions.SoftLayerError( - "Current volume performance is >= 0.3 IOPS/GB, " - "new volume performance must also be >= 0.3 " - "IOPS/GB. %s IOPS/GB (%s/%s) requested." - % (new_iops_per_gb, new_iops, new_size)) - return new_iops - - -def _validate_new_endurance_tier(origin_volume, new_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 new_tier is None: - new_tier = origin_tier - else: - if new_tier != 0.25: - new_tier = int(new_tier) - - if origin_tier == 0.25: - raise exceptions.SoftLayerError( - "IOPS change is not available for Endurance volumes with 0.25 IOPS tier ") - elif origin_tier != 0.25 and new_tier == 0.25: - raise exceptions.SoftLayerError( - "Current volume performance tier is above 0.25 IOPS/GB, " - "new volume performance tier must also be above 0.25 " - "IOPS/GB. %s IOPS/GB requested." % new_tier) - return new_tier - - def _has_category(categories, category_code): return any( True diff --git a/tests/CLI/modules/block_tests.py b/tests/CLI/modules/block_tests.py index f78f8b4a6..7e2a5f170 100644 --- a/tests/CLI/modules/block_tests.py +++ b/tests/CLI/modules/block_tests.py @@ -90,9 +90,9 @@ def test_volume_detail(self): {'Property': 'Original Volume Size', 'Value': '20'}, {'Property': 'Original Volume Name', - 'Value': 'test-origin-volume-name'}, + 'Value': 'test-original-volume-name'}, {'Property': 'Original Snapshot Name', - 'Value': 'test-origin-snapshot-name'} + 'Value': 'test-original-snapshot-name'} ] }, json.loads(result.output)) diff --git a/tests/CLI/modules/file_tests.py b/tests/CLI/modules/file_tests.py index ff2653dc7..41de1d00d 100644 --- a/tests/CLI/modules/file_tests.py +++ b/tests/CLI/modules/file_tests.py @@ -130,9 +130,9 @@ def test_volume_detail(self): {'Property': 'Original Volume Size', 'Value': '20'}, {'Property': 'Original Volume Name', - 'Value': 'test-origin-volume-name'}, + 'Value': 'test-original-volume-name'}, {'Property': 'Original Snapshot Name', - 'Value': 'test-origin-snapshot-name'} + 'Value': 'test-original-snapshot-name'} ] }, json.loads(result.output)) From 20f2644e6c07285845d641b587d3a93e6cffae99 Mon Sep 17 00:00:00 2001 From: David Pickle Date: Mon, 4 Dec 2017 16:03:45 -0600 Subject: [PATCH 5/7] Add unit tests for volume modification commands/functions --- .../fixtures/SoftLayer_Network_Storage.py | 6 +- SoftLayer/managers/storage_utils.py | 8 +- tests/CLI/modules/block_tests.py | 34 +++- tests/CLI/modules/file_tests.py | 34 +++- tests/managers/block_tests.py | 44 +++++ tests/managers/file_tests.py | 45 +++++ tests/managers/storage_utils_tests.py | 184 ++++++++++++++++++ 7 files changed, 347 insertions(+), 8 deletions(-) diff --git a/SoftLayer/fixtures/SoftLayer_Network_Storage.py b/SoftLayer/fixtures/SoftLayer_Network_Storage.py index 81d377ddd..80996cd73 100644 --- a/SoftLayer/fixtures/SoftLayer_Network_Storage.py +++ b/SoftLayer/fixtures/SoftLayer_Network_Storage.py @@ -39,8 +39,10 @@ getObject = { 'accountId': 1234, - 'activeTransactionCount': 0, - 'activeTransactions': None, + 'activeTransactionCount': 1, + 'activeTransactions': [{ + 'transactionStatus': {'friendlyName': 'This is a buffer time in which the customer may cancel the server'} + }], 'allowedHardware': [{ 'allowedHost': { 'credential': {'username': 'joe', 'password': '12345'}, diff --git a/SoftLayer/managers/storage_utils.py b/SoftLayer/managers/storage_utils.py index 84e4d7c1a..b6e1a3d46 100644 --- a/SoftLayer/managers/storage_utils.py +++ b/SoftLayer/managers/storage_utils.py @@ -1013,7 +1013,7 @@ def prepare_modify_order_object(manager, volume, new_iops, new_tier, new_size): # Verify that the origin volume has not been cancelled if 'billingItem' not in volume: - raise exceptions.SoftLayerError("The volume has been cancelled; unable to modify volume") + raise exceptions.SoftLayerError("The volume has been cancelled; unable to modify volume.") # Ensure the origin volume is STaaS v2 or higher and supports Encryption at Rest if not _staas_version_is_v2_or_above(volume): @@ -1035,7 +1035,7 @@ def prepare_modify_order_object(manager, volume, new_iops, new_tier, new_size): elif new_iops is None: new_iops = int(volume.get('provisionedIops', 0)) if new_iops <= 0: - raise exceptions.SoftLayerError("Cannot find volume's provisioned IOPS") + raise exceptions.SoftLayerError("Cannot find volume's provisioned IOPS.") # Set up the prices array for the order prices = [ @@ -1062,8 +1062,8 @@ def prepare_modify_order_object(manager, volume, new_iops, new_tier, new_size): ] else: - raise exceptions.SoftLayerError("Origin volume does not have a valid storage type (with an appropriate " - "keyName to indicate the volume is a PERFORMANCE or an ENDURANCE volume)") + raise exceptions.SoftLayerError("Volume does not have a valid storage type (with an appropriate " + "keyName to indicate the volume is a PERFORMANCE or an ENDURANCE volume).") modify_order = { 'complexType': 'SoftLayer_Container_Product_Order_Network_Storage_AsAService_Upgrade', diff --git a/tests/CLI/modules/block_tests.py b/tests/CLI/modules/block_tests.py index 7e2a5f170..352871b15 100644 --- a/tests/CLI/modules/block_tests.py +++ b/tests/CLI/modules/block_tests.py @@ -71,7 +71,8 @@ def test_volume_detail(self): 'Data Center': 'dal05', 'Type': 'ENDURANCE', 'ID': 100, - '# of Active Transactions': '0', + '# of Active Transactions': '1', + 'Ongoing Transaction': 'This is a buffer time in which the customer may cancel the server', 'Replicant Count': '1', 'Replication Status': 'Replicant Volume Provisioning ' 'has completed.', @@ -601,6 +602,37 @@ def test_duplicate_order_hourly_billing(self, order_mock): 'Order #24602 placed successfully!\n' ' > Storage as a Service\n') + @mock.patch('SoftLayer.BlockStorageManager.order_modified_volume') + def test_modify_order_exception_caught(self, order_mock): + order_mock.side_effect = ValueError('order attempt failed, noooo!') + + result = self.run_command(['block', 'volume-modify', '102', '--new-size=1000']) + + self.assertEqual(2, result.exit_code) + self.assertEqual('Argument Error: order attempt failed, noooo!', result.exception.message) + + @mock.patch('SoftLayer.BlockStorageManager.order_modified_volume') + def test_modify_order_order_not_placed(self, order_mock): + order_mock.return_value = {} + + result = self.run_command(['block', 'volume-modify', '102', '--new-iops=1400']) + + self.assert_no_fail(result) + self.assertEqual('Order could not be placed! Please verify your options and try again.\n', result.output) + + @mock.patch('SoftLayer.BlockStorageManager.order_modified_volume') + def test_modify_order(self, order_mock): + order_mock.return_value = {'placedOrder': {'id': 24602, 'items': [{'description': 'Storage as a Service'}, + {'description': '1000 GBs'}, + {'description': '4 IOPS per GB'}]}} + + result = self.run_command(['block', 'volume-modify', '102', '--new-size=1000', '--new-tier=4']) + + order_mock.assert_called_with('102', new_size=1000, new_iops=None, new_tier_level=4) + self.assert_no_fail(result) + self.assertEqual('Order #24602 placed successfully!\n > Storage as a Service\n > 1000 GBs\n > 4 IOPS per GB\n', + result.output) + def test_set_password(self): result = self.run_command(['block', 'access-password', '1234', '--password=AAAAA']) self.assert_no_fail(result) diff --git a/tests/CLI/modules/file_tests.py b/tests/CLI/modules/file_tests.py index 41de1d00d..6614e115f 100644 --- a/tests/CLI/modules/file_tests.py +++ b/tests/CLI/modules/file_tests.py @@ -111,7 +111,8 @@ def test_volume_detail(self): 'Data Center': 'dal05', 'Type': 'ENDURANCE', 'ID': 100, - '# of Active Transactions': '0', + '# of Active Transactions': '1', + 'Ongoing Transaction': 'This is a buffer time in which the customer may cancel the server', 'Replicant Count': '1', 'Replication Status': 'Replicant Volume Provisioning ' 'has completed.', @@ -579,3 +580,34 @@ def test_duplicate_order_hourly_billing(self, order_mock): self.assertEqual(result.output, 'Order #24602 placed successfully!\n' ' > Storage as a Service\n') + + @mock.patch('SoftLayer.FileStorageManager.order_modified_volume') + def test_modify_order_exception_caught(self, order_mock): + order_mock.side_effect = ValueError('order attempt failed, noooo!') + + result = self.run_command(['file', 'volume-modify', '102', '--new-size=1000']) + + self.assertEqual(2, result.exit_code) + self.assertEqual('Argument Error: order attempt failed, noooo!', result.exception.message) + + @mock.patch('SoftLayer.FileStorageManager.order_modified_volume') + def test_modify_order_order_not_placed(self, order_mock): + order_mock.return_value = {} + + result = self.run_command(['file', 'volume-modify', '102', '--new-iops=1400']) + + self.assert_no_fail(result) + self.assertEqual('Order could not be placed! Please verify your options and try again.\n', result.output) + + @mock.patch('SoftLayer.FileStorageManager.order_modified_volume') + def test_modify_order(self, order_mock): + order_mock.return_value = {'placedOrder': {'id': 24602, 'items': [{'description': 'Storage as a Service'}, + {'description': '1000 GBs'}, + {'description': '4 IOPS per GB'}]}} + + result = self.run_command(['file', 'volume-modify', '102', '--new-size=1000', '--new-tier=4']) + + order_mock.assert_called_with('102', new_size=1000, new_iops=None, new_tier_level=4) + self.assert_no_fail(result) + self.assertEqual('Order #24602 placed successfully!\n > Storage as a Service\n > 1000 GBs\n > 4 IOPS per GB\n', + result.output) diff --git a/tests/managers/block_tests.py b/tests/managers/block_tests.py index d3ebe922a..2f6cf2abc 100644 --- a/tests/managers/block_tests.py +++ b/tests/managers/block_tests.py @@ -832,6 +832,50 @@ def test_order_block_duplicate_endurance(self): 'useHourlyPricing': False },)) + def test_order_block_modified_performance(self): + mock = self.set_mock('SoftLayer_Product_Package', 'getAllObjects') + mock.return_value = [fixtures.SoftLayer_Product_Package.SAAS_PACKAGE] + + 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 + + result = self.block.order_modified_volume(102, new_size=1000, new_iops=2000, new_tier_level=None) + + self.assertEqual(fixtures.SoftLayer_Product_Order.placeOrder, result) + self.assert_called_with( + 'SoftLayer_Product_Order', + 'placeOrder', + args=({'complexType': 'SoftLayer_Container_Product_Order_Network_Storage_AsAService_Upgrade', + 'packageId': 759, + 'prices': [{'id': 189433}, {'id': 190113}, {'id': 190173}], + 'volume': {'id': 102}, + 'volumeSize': 1000, + 'iops': 2000},) + ) + + def test_order_block_modified_endurance(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 + mock = self.set_mock('SoftLayer_Network_Storage', 'getObject') + mock.return_value = mock_volume + + result = self.block.order_modified_volume(102, new_size=1000, new_iops=None, new_tier_level=4) + + self.assertEqual(fixtures.SoftLayer_Product_Order.placeOrder, result) + self.assert_called_with( + 'SoftLayer_Product_Order', + 'placeOrder', + args=({'complexType': 'SoftLayer_Container_Product_Order_Network_Storage_AsAService_Upgrade', + 'packageId': 759, + 'prices': [{'id': 189433}, {'id': 194763}, {'id': 194703}], + 'volume': {'id': 102}, + 'volumeSize': 1000},) + ) + def test_setCredentialPassword(self): mock = self.set_mock('SoftLayer_Network_Storage_Allowed_Host', 'setCredentialPassword') mock.return_value = True diff --git a/tests/managers/file_tests.py b/tests/managers/file_tests.py index 6a6d14d82..389682cdc 100644 --- a/tests/managers/file_tests.py +++ b/tests/managers/file_tests.py @@ -785,3 +785,48 @@ def test_order_file_duplicate_endurance(self): 'duplicateOriginSnapshotId': 470, 'useHourlyPricing': False },)) + + def test_order_file_modified_performance(self): + mock = self.set_mock('SoftLayer_Product_Package', 'getAllObjects') + mock.return_value = [fixtures.SoftLayer_Product_Package.SAAS_PACKAGE] + + mock_volume = copy.deepcopy(fixtures.SoftLayer_Network_Storage.STAAS_TEST_VOLUME) + mock_volume['storageType']['keyName'] = 'PERFORMANCE_FILE_STORAGE' + mock = self.set_mock('SoftLayer_Network_Storage', 'getObject') + mock.return_value = mock_volume + + result = self.file.order_modified_volume(102, new_size=1000, new_iops=2000, new_tier_level=None) + + self.assertEqual(fixtures.SoftLayer_Product_Order.placeOrder, result) + self.assert_called_with( + 'SoftLayer_Product_Order', + 'placeOrder', + args=({'complexType': 'SoftLayer_Container_Product_Order_Network_Storage_AsAService_Upgrade', + 'packageId': 759, + 'prices': [{'id': 189433}, {'id': 190113}, {'id': 190173}], + 'volume': {'id': 102}, + 'volumeSize': 1000, + 'iops': 2000},) + ) + + def test_order_file_modified_endurance(self): + mock = self.set_mock('SoftLayer_Product_Package', 'getAllObjects') + mock.return_value = [fixtures.SoftLayer_Product_Package.SAAS_PACKAGE] + + mock_volume = copy.deepcopy(fixtures.SoftLayer_Network_Storage.STAAS_TEST_VOLUME) + mock_volume['storageType']['keyName'] = 'ENDURANCE_FILE_STORAGE' + mock = self.set_mock('SoftLayer_Network_Storage', 'getObject') + mock.return_value = mock_volume + + result = self.file.order_modified_volume(102, new_size=1000, new_iops=None, new_tier_level=4) + + self.assertEqual(fixtures.SoftLayer_Product_Order.placeOrder, result) + self.assert_called_with( + 'SoftLayer_Product_Order', + 'placeOrder', + args=({'complexType': 'SoftLayer_Container_Product_Order_Network_Storage_AsAService_Upgrade', + 'packageId': 759, + 'prices': [{'id': 189433}, {'id': 194763}, {'id': 194703}], + 'volume': {'id': 102}, + 'volumeSize': 1000},) + ) diff --git a/tests/managers/storage_utils_tests.py b/tests/managers/storage_utils_tests.py index c3676d9cb..7c32e0832 100644 --- a/tests/managers/storage_utils_tests.py +++ b/tests/managers/storage_utils_tests.py @@ -3851,3 +3851,187 @@ def test_prep_duplicate_order_invalid_origin_storage_type(self): "Origin volume does not have a valid storage type " "(with an appropriate keyName to indicate the " "volume is a PERFORMANCE or an ENDURANCE volume)") + + # --------------------------------------------------------------------- + # Tests for prepare_modify_order_object() + # --------------------------------------------------------------------- + def test_prep_modify_order_origin_volume_cancelled(self): + mock_volume = copy.deepcopy(fixtures.SoftLayer_Network_Storage.STAAS_TEST_VOLUME) + del mock_volume['billingItem'] + + exception = self.assertRaises(exceptions.SoftLayerError, storage_utils.prepare_modify_order_object, + self.block, mock_volume, None, None, None) + + self.assertEqual("The volume has been cancelled; unable to modify volume.", str(exception)) + + def test_prep_modify_order_origin_volume_staas_version_below_v2(self): + mock_volume = copy.deepcopy(fixtures.SoftLayer_Network_Storage.STAAS_TEST_VOLUME) + mock_volume['staasVersion'] = 1 + + exception = self.assertRaises(exceptions.SoftLayerError, storage_utils.prepare_modify_order_object, + self.block, mock_volume, None, None, None) + + self.assertEqual("This volume cannot be modified since it does not support Encryption at Rest.", + str(exception)) + + def test_prep_modify_order_performance_values_not_given(self): + mock = self.set_mock('SoftLayer_Product_Package', 'getAllObjects') + mock.return_value = [fixtures.SoftLayer_Product_Package.SAAS_PACKAGE] + + mock_volume = copy.deepcopy(fixtures.SoftLayer_Network_Storage.STAAS_TEST_VOLUME) + mock_volume['storageType']['keyName'] = 'PERFORMANCE_BLOCK_STORAGE' + + exception = self.assertRaises(exceptions.SoftLayerError, storage_utils.prepare_modify_order_object, + self.block, mock_volume, None, None, None) + + self.assertEqual("A size or IOPS value must be given to modify this performance volume.", str(exception)) + + def test_prep_modify_order_performance_iops_not_found(self): + mock = self.set_mock('SoftLayer_Product_Package', 'getAllObjects') + mock.return_value = [fixtures.SoftLayer_Product_Package.SAAS_PACKAGE] + + mock_volume = copy.deepcopy(fixtures.SoftLayer_Network_Storage.STAAS_TEST_VOLUME) + mock_volume['storageType']['keyName'] = 'PERFORMANCE_BLOCK_STORAGE' + del mock_volume['provisionedIops'] + + exception = self.assertRaises(exceptions.SoftLayerError, storage_utils.prepare_modify_order_object, + self.block, mock_volume, None, None, 40) + + self.assertEqual("Cannot find volume's provisioned IOPS.", str(exception)) + + def test_prep_modify_order_performance_use_existing_iops(self): + mock = self.set_mock('SoftLayer_Product_Package', 'getAllObjects') + mock.return_value = [fixtures.SoftLayer_Product_Package.SAAS_PACKAGE] + + mock_volume = copy.deepcopy(fixtures.SoftLayer_Network_Storage.STAAS_TEST_VOLUME) + mock_volume['storageType']['keyName'] = 'PERFORMANCE_FILE_STORAGE' + + expected_object = { + 'complexType': 'SoftLayer_Container_Product_Order_Network_Storage_AsAService_Upgrade', + 'packageId': 759, + 'prices': [{'id': 189433}, {'id': 190113}, {'id': 190173}], + 'volume': {'id': 102}, + 'volumeSize': 1000, + 'iops': 1000 + } + + result = storage_utils.prepare_modify_order_object(self.file, mock_volume, None, None, 1000) + self.assertEqual(expected_object, result) + + def test_prep_modify_order_performance_use_existing_size(self): + mock = self.set_mock('SoftLayer_Product_Package', 'getAllObjects') + mock.return_value = [fixtures.SoftLayer_Product_Package.SAAS_PACKAGE] + + mock_volume = copy.deepcopy(fixtures.SoftLayer_Network_Storage.STAAS_TEST_VOLUME) + mock_volume['storageType']['keyName'] = 'PERFORMANCE_BLOCK_STORAGE' + + expected_object = { + 'complexType': 'SoftLayer_Container_Product_Order_Network_Storage_AsAService_Upgrade', + 'packageId': 759, + 'prices': [{'id': 189433}, {'id': 189993}, {'id': 190053}], + 'volume': {'id': 102}, + 'volumeSize': 500, + 'iops': 2000 + } + + result = storage_utils.prepare_modify_order_object(self.block, mock_volume, 2000, None, None) + self.assertEqual(expected_object, result) + + def test_prep_modify_order_performance(self): + mock = self.set_mock('SoftLayer_Product_Package', 'getAllObjects') + mock.return_value = [fixtures.SoftLayer_Product_Package.SAAS_PACKAGE] + + mock_volume = copy.deepcopy(fixtures.SoftLayer_Network_Storage.STAAS_TEST_VOLUME) + mock_volume['storageType']['keyName'] = 'PERFORMANCE_FILE_STORAGE' + + expected_object = { + 'complexType': 'SoftLayer_Container_Product_Order_Network_Storage_AsAService_Upgrade', + 'packageId': 759, + 'prices': [{'id': 189433}, {'id': 190113}, {'id': 190173}], + 'volume': {'id': 102}, + 'volumeSize': 1000, + 'iops': 2000 + } + + result = storage_utils.prepare_modify_order_object(self.file, mock_volume, 2000, None, 1000) + self.assertEqual(expected_object, result) + + def test_prep_modify_order_endurance_values_not_given(self): + mock = self.set_mock('SoftLayer_Product_Package', 'getAllObjects') + mock.return_value = [fixtures.SoftLayer_Product_Package.SAAS_PACKAGE] + + mock_volume = copy.deepcopy(fixtures.SoftLayer_Network_Storage.STAAS_TEST_VOLUME) + mock_volume['storageType']['keyName'] = 'ENDURANCE_BLOCK_STORAGE' + + exception = self.assertRaises(exceptions.SoftLayerError, storage_utils.prepare_modify_order_object, + self.block, mock_volume, None, None, None) + + self.assertEqual("A size or tier value must be given to modify this endurance volume.", str(exception)) + + def test_prep_modify_order_endurance_use_existing_tier(self): + mock = self.set_mock('SoftLayer_Product_Package', 'getAllObjects') + mock.return_value = [fixtures.SoftLayer_Product_Package.SAAS_PACKAGE] + + mock_volume = copy.deepcopy(fixtures.SoftLayer_Network_Storage.STAAS_TEST_VOLUME) + mock_volume['storageType']['keyName'] = 'ENDURANCE_FILE_STORAGE' + + expected_object = { + 'complexType': 'SoftLayer_Container_Product_Order_Network_Storage_AsAService_Upgrade', + 'packageId': 759, + 'prices': [{'id': 189433}, {'id': 193433}, {'id': 193373}], + 'volume': {'id': 102}, + 'volumeSize': 1000 + } + + result = storage_utils.prepare_modify_order_object(self.file, mock_volume, None, None, 1000) + self.assertEqual(expected_object, result) + + def test_prep_modify_order_endurance_use_existing_size(self): + mock = self.set_mock('SoftLayer_Product_Package', 'getAllObjects') + mock.return_value = [fixtures.SoftLayer_Product_Package.SAAS_PACKAGE] + + mock_volume = copy.deepcopy(fixtures.SoftLayer_Network_Storage.STAAS_TEST_VOLUME) + mock_volume['storageType']['keyName'] = 'ENDURANCE_BLOCK_STORAGE' + + expected_object = { + 'complexType': 'SoftLayer_Container_Product_Order_Network_Storage_AsAService_Upgrade', + 'packageId': 759, + 'prices': [{'id': 189433}, {'id': 194763}, {'id': 194703}], + 'volume': {'id': 102}, + 'volumeSize': 500 + } + + result = storage_utils.prepare_modify_order_object(self.block, mock_volume, None, 4, None) + self.assertEqual(expected_object, result) + + def test_prep_modify_order_endurance(self): + mock = self.set_mock('SoftLayer_Product_Package', 'getAllObjects') + mock.return_value = [fixtures.SoftLayer_Product_Package.SAAS_PACKAGE] + + mock_volume = copy.deepcopy(fixtures.SoftLayer_Network_Storage.STAAS_TEST_VOLUME) + mock_volume['storageType']['keyName'] = 'ENDURANCE_FILE_STORAGE' + + expected_object = { + 'complexType': 'SoftLayer_Container_Product_Order_Network_Storage_AsAService_Upgrade', + 'packageId': 759, + 'prices': [{'id': 189433}, {'id': 194763}, {'id': 194703}], + 'volume': {'id': 102}, + 'volumeSize': 1000 + } + + result = storage_utils.prepare_modify_order_object(self.file, mock_volume, None, 4, 1000) + self.assertEqual(expected_object, result) + + def test_prep_modify_order_invalid_volume_storage_type(self): + mock = self.set_mock('SoftLayer_Product_Package', 'getAllObjects') + mock.return_value = [fixtures.SoftLayer_Product_Package.SAAS_PACKAGE] + + mock_volume = copy.deepcopy(fixtures.SoftLayer_Network_Storage.STAAS_TEST_VOLUME) + mock_volume['storageType']['keyName'] = 'NINJA_PENGUINS' + + exception = self.assertRaises(exceptions.SoftLayerError, storage_utils.prepare_modify_order_object, + self.block, mock_volume, None, None, None) + + self.assertEqual("Volume does not have a valid storage type (with an appropriate " + "keyName to indicate the volume is a PERFORMANCE or an ENDURANCE volume).", + str(exception)) From a10f5a8e76516c550d72eac4135358384b7812ef Mon Sep 17 00:00:00 2001 From: David Pickle Date: Tue, 5 Dec 2017 13:42:19 -0600 Subject: [PATCH 6/7] Update formatting of object masks in volume modification functions --- SoftLayer/managers/block.py | 13 +++++++++++-- SoftLayer/managers/file.py | 13 +++++++++++-- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/SoftLayer/managers/block.py b/SoftLayer/managers/block.py index bc5cb8798..1e5931fce 100644 --- a/SoftLayer/managers/block.py +++ b/SoftLayer/managers/block.py @@ -313,8 +313,17 @@ def order_modified_volume(self, volume_id, new_size=None, new_iops=None, new_tie :return: Returns a SoftLayer_Container_Product_Order_Receipt """ - block_mask = 'id,billingItem,storageType[keyName],capacityGb,provisionedIops,'\ - 'storageTierLevel,staasVersion,hasEncryptionAtRest' + mask_items = [ + 'id', + 'billingItem', + 'storageType[keyName]', + 'capacityGb', + 'provisionedIops', + 'storageTierLevel', + 'staasVersion', + 'hasEncryptionAtRest', + ] + block_mask = ','.join(mask_items) volume = self.get_block_volume_details(volume_id, mask=block_mask) order = storage_utils.prepare_modify_order_object( diff --git a/SoftLayer/managers/file.py b/SoftLayer/managers/file.py index 5646b7399..e4a5ac238 100644 --- a/SoftLayer/managers/file.py +++ b/SoftLayer/managers/file.py @@ -293,8 +293,17 @@ def order_modified_volume(self, volume_id, new_size=None, new_iops=None, new_tie :return: Returns a SoftLayer_Container_Product_Order_Receipt """ - file_mask = 'id,billingItem,storageType[keyName],capacityGb,provisionedIops,'\ - 'storageTierLevel,staasVersion,hasEncryptionAtRest' + mask_items = [ + 'id', + 'billingItem', + 'storageType[keyName]', + 'capacityGb', + 'provisionedIops', + 'storageTierLevel', + 'staasVersion', + 'hasEncryptionAtRest', + ] + file_mask = ','.join(mask_items) volume = self.get_file_volume_details(volume_id, mask=file_mask) order = storage_utils.prepare_modify_order_object( From 74830b54aa19eb1f83d311fb0e63b07c9953e090 Mon Sep 17 00:00:00 2001 From: David Pickle Date: Tue, 5 Dec 2017 16:56:57 -0600 Subject: [PATCH 7/7] Fix parsing logic in volume-detail commands to handle invalid types Some certain types of transactions which are currently returned in a volume's activeTransactions contain transactionStatus sub-properties of inconsistent types. There is a plan to fix the core issue in the backend, but until that fix is added, the changes in this commit should prevent the volume-detail commands from resulting in an error if any invalid transactionStatus objects are returned. --- SoftLayer/CLI/block/detail.py | 2 +- SoftLayer/CLI/file/detail.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/SoftLayer/CLI/block/detail.py b/SoftLayer/CLI/block/detail.py index 050121d5a..ecfd5c4d5 100644 --- a/SoftLayer/CLI/block/detail.py +++ b/SoftLayer/CLI/block/detail.py @@ -62,7 +62,7 @@ def cli(env, volume_id): if block_volume['activeTransactions']: for trans in block_volume['activeTransactions']: - if isinstance(utils.lookup(trans, 'transactionStatus', 'friendlyName'), str): + if 'transactionStatus' in trans and 'friendlyName' in trans['transactionStatus']: table.add_row(['Ongoing Transaction', trans['transactionStatus']['friendlyName']]) table.add_row(['Replicant Count', "%u" % block_volume.get('replicationPartnerCount', 0)]) diff --git a/SoftLayer/CLI/file/detail.py b/SoftLayer/CLI/file/detail.py index 9adcacb2b..cb712dc97 100644 --- a/SoftLayer/CLI/file/detail.py +++ b/SoftLayer/CLI/file/detail.py @@ -78,7 +78,7 @@ def cli(env, volume_id): if file_volume['activeTransactions']: for trans in file_volume['activeTransactions']: - if isinstance(utils.lookup(trans, 'transactionStatus', 'friendlyName'), str): + if 'transactionStatus' in trans and 'friendlyName' in trans['transactionStatus']: table.add_row(['Ongoing Transaction', trans['transactionStatus']['friendlyName']]) table.add_row(['Replicant Count', "%u" % file_volume.get('replicationPartnerCount', 0)])