diff options
| author | Stephen Finucane <sfinucan@redhat.com> | 2021-03-04 18:34:00 +0000 |
|---|---|---|
| committer | Stephen Finucane <sfinucan@redhat.com> | 2021-03-05 12:44:52 +0000 |
| commit | 7c1d6f769c4f0d2afe61410fefd8bc8f26a22980 (patch) | |
| tree | a1fa4c2b0d7525266cefb906b57d14e6dd8315e8 /openstackclient/compute/v2 | |
| parent | a507fb50f8b98b026be88de41f1bac49bafe19bf (diff) | |
| download | python-openstackclient-7c1d6f769c4f0d2afe61410fefd8bc8f26a22980.tar.gz | |
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 <sfinucan@redhat.com>
Diffstat (limited to 'openstackclient/compute/v2')
| -rw-r--r-- | openstackclient/compute/v2/server.py | 30 |
1 files changed, 15 insertions, 15 deletions
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: |