From 7c1d6f769c4f0d2afe61410fefd8bc8f26a22980 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Thu, 4 Mar 2021 18:34:00 +0000 Subject: compute: Add functional tests for --block-device This mostly reuses the existing tests for '--block-device-mapping', which can hopefully be removed at some point in the future. This highlights two issues with the implementation of this option. Firstly, the 'boot_index' parameter is not required so don't mandate it. Secondly, and more significantly, we were defaulting the destination type for the 'image' source type to 'local'. Nova only allows you to attach a single image to local mapping [1], which means this default would only make sense if you were expecting users to use the '--block-device' option exclusively and omit the '--image' option. This is the *less common* case so this is a bad default. Default instead to a destination type of 'volume' like everything else, and require users specifying '--block-device' alone to pass 'destination_type=local' explicitly. [1] https://github.com/openstack/nova/blob/c8a6f8d2e/nova/block_device.py#L193-L206 Change-Id: I1718be965f57c3bbdb8a14f3cfac967dd4c55b4d Signed-off-by: Stephen Finucane --- openstackclient/compute/v2/server.py | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) (limited to 'openstackclient/compute') diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index 9838ed54..b2ae93c8 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -832,13 +832,12 @@ class CreateServer(command.ShowOne): action=parseractions.MultiKeyValueAction, dest='block_devices', default=[], - required_keys=[ - 'boot_index', - ], + required_keys=[], optional_keys=[ 'uuid', 'source_type', 'destination_type', - 'disk_bus', 'device_type', 'device_name', 'guest_format', - 'volume_size', 'volume_type', 'delete_on_termination', 'tag', + 'disk_bus', 'device_type', 'device_name', 'volume_size', + 'guest_format', 'boot_index', 'delete_on_termination', 'tag', + 'volume_type', ], help=_( 'Create a block device on the server.\n' @@ -1336,14 +1335,15 @@ class CreateServer(command.ShowOne): block_device_mapping_v2.append(mapping) for mapping in parsed_args.block_devices: - try: - mapping['boot_index'] = int(mapping['boot_index']) - except ValueError: - msg = _( - 'The boot_index key of --block-device should be an ' - 'integer' - ) - raise exceptions.CommandError(msg) + if 'boot_index' in mapping: + try: + mapping['boot_index'] = int(mapping['boot_index']) + except ValueError: + msg = _( + 'The boot_index key of --block-device should be an ' + 'integer' + ) + raise exceptions.CommandError(msg) if 'tag' in mapping and ( compute_client.api_version < api_versions.APIVersion('2.42') @@ -1383,9 +1383,9 @@ class CreateServer(command.ShowOne): ) raise exceptions.CommandError(msg) else: - if mapping['source_type'] in ('image', 'blank'): + if mapping['source_type'] in ('blank',): mapping['destination_type'] = 'local' - else: # volume, snapshot + else: # volume, image, snapshot mapping['destination_type'] = 'volume' if 'delete_on_termination' in mapping: -- cgit v1.2.1