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

3 files changed, 54 insertions, 33 deletions

diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py
index a1330e01..cd450036 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 b6ef8587..119ef05c 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 54f36209..53189aa2 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,