From 95c8661f86e74c9d5217869a740da11350f1f0eb Mon Sep 17 00:00:00 2001 From: Nikita Gerasimov Date: Mon, 5 Dec 2016 20:47:40 +0300 Subject: [PATCH] Switch server create to block_device_mapping_v2 Current compute_client.servers.create() relies on block_device_mapping arg which is legacy[1]. "block_device_mapping" format require device_name which is leads to hard-coded hack in --volume key handler to KVM specific. "block_device_mapping_v2" format is more friendly to hypervisiors. Support of block_device_mapping_v2 appear in python-novaclient 2.16.0, openstackclient require at least 2.29.0 Makes options --volume and --block-device-mapping work simultaneously. Appends --block-device-mapping data even if --volume used. After bug 1383338 only --volume was taken when both are used. [1]http://docs.openstack.org/developer/nova/block_device_mapping.html NOTE(dtroyer): I moved the new test_boot_from_volume() functional test to Ie51b1c375c5940856ec76a5770df3c6bd18a3eba to test our previous behaviour. The only changes required to support the new behaviour should be that the empty_volume is now attached in that test. Change-Id: I7bac3d870dd9ca404093142f8bce22a62e49180d Closes-Bug: 1647406 Closes-Bug: 1497845 --- openstackclient/compute/v2/server.py | 52 ++++++++++++------- .../functional/compute/v2/test_server.py | 12 +++-- .../tests/unit/compute/v2/test_server.py | 23 ++++---- .../notes/bug-1647406-c936581034a1b6e4.yaml | 16 ++++++ 4 files changed, 70 insertions(+), 33 deletions(-) create mode 100644 releasenotes/notes/bug-1647406-c936581034a1b6e4.yaml diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index a1330e0195..cd4500367c 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -507,28 +507,40 @@ class CreateServer(command.ShowOne): "exception": e} ) - block_device_mapping = {} + block_device_mapping_v2 = [] if volume: - # When booting from volume, for now assume no other mappings - # This device value is likely KVM-specific - block_device_mapping = {'vda': volume} - else: - for dev_map in parsed_args.block_device_mapping: - dev_key, dev_vol = dev_map.split('=', 1) - block_volume = None - if dev_vol: - vol = dev_vol.split(':', 1)[0] - if vol: - vol_id = utils.find_resource( + block_device_mapping_v2 = [{'uuid': volume, + 'boot_index': '0', + 'source_type': 'volume', + 'destination_type': 'volume' + }] + for dev_map in parsed_args.block_device_mapping: + dev_name, dev_map = dev_map.split('=', 1) + if dev_map: + dev_map = dev_map.split(':') + if len(dev_map) > 0: + mapping = { + 'device_name': dev_name, + 'uuid': utils.find_resource( volume_client.volumes, - vol, - ).id - block_volume = dev_vol.replace(vol, vol_id) + dev_map[0], + ).id} + # Block device mapping v1 compatibility + if len(dev_map) > 1 and \ + dev_map[1] in ('volume', 'snapshot'): + mapping['source_type'] = dev_map[1] else: - msg = _("Volume name or ID must be specified if " - "--block-device-mapping is specified") - raise exceptions.CommandError(msg) - block_device_mapping.update({dev_key: block_volume}) + mapping['source_type'] = 'volume' + mapping['destination_type'] = 'volume' + if len(dev_map) > 2: + mapping['volume_size'] = dev_map[2] + if len(dev_map) > 3: + mapping['delete_on_termination'] = dev_map[3] + else: + msg = _("Volume name or ID must be specified if " + "--block-device-mapping is specified") + raise exceptions.CommandError(msg) + block_device_mapping_v2.append(mapping) nics = [] if parsed_args.nic in ('auto', 'none'): @@ -598,7 +610,7 @@ class CreateServer(command.ShowOne): userdata=userdata, key_name=parsed_args.key_name, availability_zone=parsed_args.availability_zone, - block_device_mapping=block_device_mapping, + block_device_mapping_v2=block_device_mapping_v2, nics=nics, scheduler_hints=hints, config_drive=config_drive) diff --git a/openstackclient/tests/functional/compute/v2/test_server.py b/openstackclient/tests/functional/compute/v2/test_server.py index b6ef85875e..119ef05c5d 100644 --- a/openstackclient/tests/functional/compute/v2/test_server.py +++ b/openstackclient/tests/functional/compute/v2/test_server.py @@ -417,19 +417,23 @@ class ServerTests(base.TestCase): # NOTE(dtroyer): Prior to https://review.openstack.org/#/c/407111 # --block-device-mapping was ignored if --volume - # present on the command line, so this volume should - # not be attached. + # present on the command line. Now we should see the + # attachment. cmd_output = json.loads(self.openstack( 'volume show -f json ' + empty_volume_name )) attachments = cmd_output['attachments'] self.assertEqual( - 0, + 1, len(attachments), ) self.assertEqual( - "available", + server['id'], + attachments[0]['server_id'], + ) + self.assertEqual( + "in-use", cmd_output['status'], ) diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index 54f36209c0..53189aa2d3 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -308,7 +308,7 @@ class TestServerCreate(TestServer): userdata=None, key_name=None, availability_zone=None, - block_device_mapping={}, + block_device_mapping_v2=[], nics=[], scheduler_hints={}, config_drive=None, @@ -363,7 +363,7 @@ class TestServerCreate(TestServer): userdata=None, key_name='keyname', availability_zone=None, - block_device_mapping={}, + block_device_mapping_v2=[], nics=[], scheduler_hints={'a': ['b', 'c']}, config_drive=None, @@ -443,7 +443,7 @@ class TestServerCreate(TestServer): userdata=None, key_name=None, availability_zone=None, - block_device_mapping={}, + block_device_mapping_v2=[], nics=[{'net-id': 'net1_uuid', 'v4-fixed-ip': '', 'v6-fixed-ip': '', @@ -500,7 +500,7 @@ class TestServerCreate(TestServer): userdata=None, key_name=None, availability_zone=None, - block_device_mapping={}, + block_device_mapping_v2=[], nics=[], scheduler_hints={}, config_drive=None, @@ -550,7 +550,7 @@ class TestServerCreate(TestServer): userdata=None, key_name=None, availability_zone=None, - block_device_mapping={}, + block_device_mapping_v2=[], nics=[], scheduler_hints={}, config_drive=None, @@ -605,7 +605,7 @@ class TestServerCreate(TestServer): userdata=mock_file, key_name=None, availability_zone=None, - block_device_mapping={}, + block_device_mapping_v2=[], nics=[], scheduler_hints={}, config_drive=None, @@ -656,9 +656,14 @@ class TestServerCreate(TestServer): userdata=None, key_name=None, availability_zone=None, - block_device_mapping={ - 'vda': real_volume_mapping - }, + block_device_mapping_v2=[{ + 'device_name': 'vda', + 'uuid': real_volume_mapping.split(':', 1)[0], + 'destination_type': 'volume', + 'source_type': 'volume', + 'delete_on_termination': '0', + 'volume_size': '' + }], nics=[], scheduler_hints={}, config_drive=None, diff --git a/releasenotes/notes/bug-1647406-c936581034a1b6e4.yaml b/releasenotes/notes/bug-1647406-c936581034a1b6e4.yaml new file mode 100644 index 0000000000..2f327517de --- /dev/null +++ b/releasenotes/notes/bug-1647406-c936581034a1b6e4.yaml @@ -0,0 +1,16 @@ +--- +fixes: + - | + Allow ``--block-device-mapping`` option to work correctly with + ``--volume`` option in ``server create`` command. + After :lpbug:`1383338` ``--block-device-mapping`` was ignored if + ``--volume`` was present. Block device mappings are now appended + to the mapping created by the ``--volume`` option if it is present. + The device name of the boot volume specificed in the ``--volume`` option + is no longer assumed to be *'vda'* but now uses the hypervisor's boot + index to obtain the device name. This maintains the status quo for + **QEMU/KVM** hypervisors but **XEN**, **parallels** and others + *virt types* that have device naming is different from ``vd*`` + should now also work correctly. + [:lpbug:`1497845`] + [:lpbug:`1647406`]