From 8a56dcdc1a91293175a92303c6f069c0a9990cd6 Mon Sep 17 00:00:00 2001 From: allmightyspiff Date: Mon, 23 Apr 2018 16:21:01 -0500 Subject: [PATCH 1/4] #959 added warning message when ordering legacy storage offerings from the cli, removed restriction on IOPS being multiples of 100 for performance volumes --- SoftLayer/CLI/block/order.py | 44 ++++++++++++-------------------- SoftLayer/CLI/file/order.py | 30 ++++++++-------------- tests/CLI/modules/block_tests.py | 8 ------ tests/CLI/modules/file_tests.py | 7 ----- 4 files changed, 28 insertions(+), 61 deletions(-) diff --git a/SoftLayer/CLI/block/order.py b/SoftLayer/CLI/block/order.py index abd7dd91e..f99625f27 100644 --- a/SoftLayer/CLI/block/order.py +++ b/SoftLayer/CLI/block/order.py @@ -17,17 +17,14 @@ required=True) @click.option('--size', type=int, - help='Size of block storage volume in GB. Permitted Sizes:\n' - '20, 40, 80, 100, 250, 500, 1000, 2000, 4000, 8000, 12000', + help='Size of block storage volume in GB.', required=True) @click.option('--iops', type=int, - help='Performance Storage IOPs,' - ' between 100 and 6000 in multiples of 100' - ' [required for storage-type performance]') + help="""Performance Storage IOPs. Options vary based on storage size. +[required for storage-type performance]""") @click.option('--tier', - help='Endurance Storage Tier (IOP per GB)' - ' [required for storage-type endurance]', + help='Endurance Storage Tier (IOP per GB) [required for storage-type endurance]', type=click.Choice(['0.25', '2', '4', '10'])) @click.option('--os-type', help='Operating System', @@ -49,8 +46,8 @@ 'space along with endurance block storage; specifies ' 'the size (in GB) of snapshot space to order') @click.option('--service-offering', - help='The service offering package to use for placing ' - 'the order [optional, default is \'storage_as_a_service\']', + help="""The service offering package to use for placing the order. +[optional, default is \'storage_as_a_service\']. enterprise and performance are depreciated""", default='storage_as_a_service', type=click.Choice([ 'storage_as_a_service', @@ -71,26 +68,21 @@ def cli(env, storage_type, size, iops, tier, os_type, if billing.lower() == "hourly": hourly_billing_flag = True - if hourly_billing_flag and service_offering != 'storage_as_a_service': - raise exceptions.CLIAbort( - 'Hourly billing is only available for the storage_as_a_service ' - 'service offering' - ) + if service_offering != 'storage_as_a_service': + click.secho('{} is a legacy storage offering'.format(service_offering), fg='red') + if hourly_billing_flag: + raise exceptions.CLIAbort( + 'Hourly billing is only available for the storage_as_a_service service offering' + ) if storage_type == 'performance': if iops is None: - raise exceptions.CLIAbort( - 'Option --iops required with Performance') - - if iops % 100 != 0: - raise exceptions.CLIAbort( - 'Option --iops must be a multiple of 100' - ) + raise exceptions.CLIAbort('Option --iops required with Performance') if service_offering == 'performance' and snapshot_size is not None: raise exceptions.CLIAbort( - '--snapshot-size is not available for performance volumes ' - 'ordered with the \'performance\' service offering option' + '--snapshot-size is not available for performance service offerings. ' + 'Use --service-offering storage_as_a_service' ) try: @@ -110,8 +102,7 @@ def cli(env, storage_type, size, iops, tier, os_type, if storage_type == 'endurance': if tier is None: raise exceptions.CLIAbort( - 'Option --tier required with Endurance in IOPS/GB ' - '[0.25,2,4,10]' + 'Option --tier required with Endurance in IOPS/GB [0.25,2,4,10]' ) try: @@ -134,5 +125,4 @@ def cli(env, storage_type, size, iops, tier, os_type, 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/order.py b/SoftLayer/CLI/file/order.py index ae9013392..89a6cefc3 100644 --- a/SoftLayer/CLI/file/order.py +++ b/SoftLayer/CLI/file/order.py @@ -21,12 +21,10 @@ required=True) @click.option('--iops', type=int, - help='Performance Storage IOPs,' - ' between 100 and 6000 in multiples of 100' - ' [required for storage-type performance]') + help="""Performance Storage IOPs. Options vary based on storage size. +[required for storage-type performance]""") @click.option('--tier', - help='Endurance Storage Tier (IOP per GB)' - ' [required for storage-type endurance]', + help='Endurance Storage Tier (IOP per GB) [required for storage-type endurance]', type=click.Choice(['0.25', '2', '4', '10'])) @click.option('--location', help='Datacenter short name (e.g.: dal09)', @@ -59,22 +57,18 @@ def cli(env, storage_type, size, iops, tier, if billing.lower() == "hourly": hourly_billing_flag = True - if hourly_billing_flag and service_offering != 'storage_as_a_service': - raise exceptions.CLIAbort( - 'Hourly billing is only available for the storage_as_a_service ' - 'service offering' - ) + if service_offering != 'storage_as_a_service': + click.secho('{} is a legacy storage offering'.format(service_offering), fg='red') + if hourly_billing_flag: + raise exceptions.CLIAbort( + 'Hourly billing is only available for the storage_as_a_service service offering' + ) if storage_type == 'performance': if iops is None: raise exceptions.CLIAbort( 'Option --iops required with Performance') - if iops % 100 != 0: - raise exceptions.CLIAbort( - 'Option --iops must be a multiple of 100' - ) - if service_offering == 'performance' and snapshot_size is not None: raise exceptions.CLIAbort( '--snapshot-size is not available for performance volumes ' @@ -97,8 +91,7 @@ def cli(env, storage_type, size, iops, tier, if storage_type == 'endurance': if tier is None: raise exceptions.CLIAbort( - 'Option --tier required with Endurance in IOPS/GB ' - '[0.25,2,4,10]' + 'Option --tier required with Endurance in IOPS/GB [0.25,2,4,10]' ) try: @@ -120,5 +113,4 @@ def cli(env, storage_type, size, iops, tier, 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/tests/CLI/modules/block_tests.py b/tests/CLI/modules/block_tests.py index 08914757a..50d18ad83 100644 --- a/tests/CLI/modules/block_tests.py +++ b/tests/CLI/modules/block_tests.py @@ -141,14 +141,6 @@ def test_volume_order_performance_iops_not_given(self): self.assertEqual(2, result.exit_code) - def test_volume_order_performance_iops_not_multiple_of_100(self): - result = self.run_command(['block', 'volume-order', - '--storage-type=performance', '--size=20', - '--iops=122', '--os-type=linux', - '--location=dal05']) - - self.assertEqual(2, result.exit_code) - def test_volume_order_performance_snapshot_error(self): result = self.run_command(['block', 'volume-order', '--storage-type=performance', '--size=20', diff --git a/tests/CLI/modules/file_tests.py b/tests/CLI/modules/file_tests.py index 14c522f88..f7ec48591 100644 --- a/tests/CLI/modules/file_tests.py +++ b/tests/CLI/modules/file_tests.py @@ -144,13 +144,6 @@ def test_volume_order_performance_iops_not_given(self): self.assertEqual(2, result.exit_code) - def test_volume_order_performance_iops_not_multiple_of_100(self): - result = self.run_command(['file', 'volume-order', - '--storage-type=performance', '--size=20', - '--iops=122', '--location=dal05']) - - self.assertEqual(2, result.exit_code) - def test_volume_order_performance_snapshot_error(self): result = self.run_command(['file', 'volume-order', '--storage-type=performance', '--size=20', From fe29b88b808f1be2ec427f57fbfdbe3ab846f63c Mon Sep 17 00:00:00 2001 From: allmightyspiff Date: Tue, 24 Apr 2018 15:59:28 -0500 Subject: [PATCH 2/4] added documentation link to volume-order and increased max terminal width because 80 chars is way too smal --- SoftLayer/CLI/block/order.py | 6 +++++- SoftLayer/CLI/core.py | 3 ++- SoftLayer/CLI/file/order.py | 6 +++++- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/SoftLayer/CLI/block/order.py b/SoftLayer/CLI/block/order.py index f99625f27..f3d15a196 100644 --- a/SoftLayer/CLI/block/order.py +++ b/SoftLayer/CLI/block/order.py @@ -60,7 +60,11 @@ @environment.pass_env def cli(env, storage_type, size, iops, tier, os_type, location, snapshot_size, service_offering, billing): - """Order a block storage volume.""" + """Order a block storage volume. + + Valid size and iops options can be found here: + https://console.bluemix.net/docs/infrastructure/BlockStorage/index.html#provisioning + """ block_manager = SoftLayer.BlockStorageManager(env.client) storage_type = storage_type.lower() diff --git a/SoftLayer/CLI/core.py b/SoftLayer/CLI/core.py index 82ad1f7ed..e75863806 100644 --- a/SoftLayer/CLI/core.py +++ b/SoftLayer/CLI/core.py @@ -75,7 +75,8 @@ def get_command(self, ctx, name): use: 'slcli setup'""", cls=CommandLoader, context_settings={'help_option_names': ['-h', '--help'], - 'auto_envvar_prefix': 'SLCLI'}) + 'auto_envvar_prefix': 'SLCLI', + 'max_content_width': 999}) @click.option('--format', default=DEFAULT_FORMAT, show_default=True, diff --git a/SoftLayer/CLI/file/order.py b/SoftLayer/CLI/file/order.py index 89a6cefc3..057c6f3a3 100644 --- a/SoftLayer/CLI/file/order.py +++ b/SoftLayer/CLI/file/order.py @@ -49,7 +49,11 @@ @environment.pass_env def cli(env, storage_type, size, iops, tier, location, snapshot_size, service_offering, billing): - """Order a file storage volume.""" + """Order a file storage volume. + + Valid size and iops options can be found here: + https://console.bluemix.net/docs/infrastructure/FileStorage/index.html#provisioning + """ file_manager = SoftLayer.FileStorageManager(env.client) storage_type = storage_type.lower() From e723685a0fe02e282bd9fa3a99eacbe0c1c22f86 Mon Sep 17 00:00:00 2001 From: allmightyspiff Date: Tue, 24 Apr 2018 17:47:50 -0500 Subject: [PATCH 3/4] #959 updated file order to be consistent with block order --- SoftLayer/CLI/file/order.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/SoftLayer/CLI/file/order.py b/SoftLayer/CLI/file/order.py index 057c6f3a3..3f86c45fc 100644 --- a/SoftLayer/CLI/file/order.py +++ b/SoftLayer/CLI/file/order.py @@ -35,8 +35,8 @@ 'space along with endurance file storage; specifies ' 'the size (in GB) of snapshot space to order') @click.option('--service-offering', - help='The service offering package to use for placing ' - 'the order [optional, default is \'storage_as_a_service\']', + help="""The service offering package to use for placing the order. +[optional, default is \'storage_as_a_service\']. enterprise and performance are depreciated""", default='storage_as_a_service', type=click.Choice([ 'storage_as_a_service', @@ -70,13 +70,12 @@ def cli(env, storage_type, size, iops, tier, if storage_type == 'performance': if iops is None: - raise exceptions.CLIAbort( - 'Option --iops required with Performance') + raise exceptions.CLIAbort('Option --iops required with Performance') if service_offering == 'performance' and snapshot_size is not None: raise exceptions.CLIAbort( - '--snapshot-size is not available for performance volumes ' - 'ordered with the \'performance\' service offering option' + '--snapshot-size is not available for performance service offerings. ' + 'Use --service-offering storage_as_a_service' ) try: From b153838225908d219dec42a89a3b545e50f4836d Mon Sep 17 00:00:00 2001 From: allmightyspiff Date: Tue, 24 Apr 2018 17:56:57 -0500 Subject: [PATCH 4/4] tox analyisis fixes --- SoftLayer/CLI/block/order.py | 4 ++-- SoftLayer/CLI/file/order.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/SoftLayer/CLI/block/order.py b/SoftLayer/CLI/block/order.py index f3d15a196..302b45cc8 100644 --- a/SoftLayer/CLI/block/order.py +++ b/SoftLayer/CLI/block/order.py @@ -60,9 +60,9 @@ @environment.pass_env def cli(env, storage_type, size, iops, tier, os_type, location, snapshot_size, service_offering, billing): - """Order a block storage volume. + """Order a block storage volume. - Valid size and iops options can be found here: + Valid size and iops options can be found here: https://console.bluemix.net/docs/infrastructure/BlockStorage/index.html#provisioning """ block_manager = SoftLayer.BlockStorageManager(env.client) diff --git a/SoftLayer/CLI/file/order.py b/SoftLayer/CLI/file/order.py index 3f86c45fc..9e1c9cd29 100644 --- a/SoftLayer/CLI/file/order.py +++ b/SoftLayer/CLI/file/order.py @@ -50,8 +50,8 @@ def cli(env, storage_type, size, iops, tier, location, snapshot_size, service_offering, billing): """Order a file storage volume. - - Valid size and iops options can be found here: + + Valid size and iops options can be found here: https://console.bluemix.net/docs/infrastructure/FileStorage/index.html#provisioning """ file_manager = SoftLayer.FileStorageManager(env.client)