Fix server create for boot-from-volume

* server create required --image even when booting the server from a
  volume.  Change options to require either --image or --volume to
  specify the server boot disk.  Using --volume currently uses device
  'vda' for the block mapping and ignores any other block mappings
  given in --block-device-mapping.
* server create and server show are both affected by bug 1378842 where
  an excepion was thrown when no image ID was present in the returned
  server object, which is the case for a server booted from a volume.
* Fix the remaining assertEqual() order problems in test_server.py

Closes-Bug: 1378842
Closes-Bug: 1383338
Change-Id: I5daebf4e50a765d4920088dfead95b6295af6a4d

2 files changed, 52 insertions, 21 deletions

diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py
index a6d645b9..d58df86c 100644
--- a/openstackclient/compute/v2/server.py
+++ b/openstackclient/compute/v2/server.py
@@ -67,9 +67,10 @@ def _prep_server_detail(compute_client, server):
 
     # Convert the image blob to a name
     image_info = info.get('image', {})
-    image_id = image_info.get('id', '')
-    image = utils.find_resource(compute_client.images, image_id)
-    info['image'] = "%s (%s)" % (image.name, image_id)
+    if image_info:
+        image_id = image_info.get('id', '')
+        image = utils.find_resource(compute_client.images, image_id)
+        info['image'] = "%s (%s)" % (image.name, image_id)
 
     # Convert the flavor blob to a name
     flavor_info = info.get('flavor', {})
@@ -192,11 +193,17 @@ class CreateServer(show.ShowOne):
             'server_name',
             metavar='<server-name>',
             help=_('New server name'))
-        parser.add_argument(
+        disk_group = parser.add_mutually_exclusive_group(
+            required=True,
+        )
+        disk_group.add_argument(
             '--image',
             metavar='<image>',
-            required=True,
             help=_('Create server from this image'))
+        disk_group.add_argument(
+            '--volume',
+            metavar='<volume>',
+            help=_('Create server from this volume'))
         parser.add_argument(
             '--flavor',
             metavar='<flavor>',
@@ -282,10 +289,23 @@ class CreateServer(show.ShowOne):
     def take_action(self, parsed_args):
         self.log.debug('take_action(%s)', parsed_args)
         compute_client = self.app.client_manager.compute
+        volume_client = self.app.client_manager.volume
 
         # Lookup parsed_args.image
-        image = utils.find_resource(compute_client.images,
-                                    parsed_args.image)
+        image = None
+        if parsed_args.image:
+            image = utils.find_resource(
+                compute_client.images,
+                parsed_args.image,
+            )
+
+        # Lookup parsed_args.volume
+        volume = None
+        if parsed_args.volume:
+            volume = utils.find_resource(
+                volume_client.volumes,
+                parsed_args.volume,
+            ).id
 
         # Lookup parsed_args.flavor
         flavor = utils.find_resource(compute_client.flavors,
@@ -319,8 +339,21 @@ class CreateServer(show.ShowOne):
                 msg = "Can't open '%s': %s"
                 raise exceptions.CommandError(msg % (parsed_args.user_data, e))
 
-        block_device_mapping = dict(v.split('=', 1)
-                                    for v in parsed_args.block_device_mapping)
+        block_device_mapping = {}
+        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:
+                    block_volume = utils.find_resource(
+                        volume_client.volumes,
+                        dev_vol,
+                    ).id
+                block_device_mapping.update({dev_key: block_volume})
 
         nics = []
         for nic_str in parsed_args.nic:
diff --git a/openstackclient/tests/compute/v2/test_server.py b/openstackclient/tests/compute/v2/test_server.py
index 50de5c6a..43aa7a70 100644
--- a/openstackclient/tests/compute/v2/test_server.py
+++ b/openstackclient/tests/compute/v2/test_server.py
@@ -134,17 +134,16 @@ class TestServerCreate(TestServer):
             **kwargs
         )
 
-        collist = ('addresses', 'flavor', 'id', 'image', 'name', 'properties')
-        self.assertEqual(columns, collist)
+        collist = ('addresses', 'flavor', 'id', 'name', 'properties')
+        self.assertEqual(collist, columns)
         datalist = (
             '',
             'Large ()',
             compute_fakes.server_id,
-            'graven ()',
             compute_fakes.server_name,
             '',
         )
-        self.assertEqual(data, datalist)
+        self.assertEqual(datalist, data)
 
     @mock.patch('openstackclient.compute.v2.server.io.open')
     def test_server_create_userdata(self, mock_open):
@@ -200,17 +199,16 @@ class TestServerCreate(TestServer):
             **kwargs
         )
 
-        collist = ('addresses', 'flavor', 'id', 'image', 'name', 'properties')
-        self.assertEqual(columns, collist)
+        collist = ('addresses', 'flavor', 'id', 'name', 'properties')
+        self.assertEqual(collist, columns)
         datalist = (
             '',
             'Large ()',
             compute_fakes.server_id,
-            'graven ()',
             compute_fakes.server_name,
             '',
         )
-        self.assertEqual(data, datalist)
+        self.assertEqual(datalist, data)
 
 
 class TestServerDelete(TestServer):
@@ -288,14 +286,14 @@ class TestServerImageCreate(TestServer):
         )
 
         collist = ('id', 'is_public', 'name', 'owner')
-        self.assertEqual(columns, collist)
+        self.assertEqual(collist, columns)
         datalist = (
             image_fakes.image_id,
             False,
             image_fakes.image_name,
             image_fakes.image_owner,
         )
-        self.assertEqual(data, datalist)
+        self.assertEqual(datalist, data)
 
     def test_server_image_create_name(self):
         arglist = [
@@ -318,14 +316,14 @@ class TestServerImageCreate(TestServer):
         )
 
         collist = ('id', 'is_public', 'name', 'owner')
-        self.assertEqual(columns, collist)
+        self.assertEqual(collist, columns)
         datalist = (
             image_fakes.image_id,
             False,
             image_fakes.image_name,
             image_fakes.image_owner,
         )
-        self.assertEqual(data, datalist)
+        self.assertEqual(datalist, data)
 
 
 class TestServerResize(TestServer):