compute: Allow users to manually specify bootable volumes

When creating a server with an attached volume, you can specify a block
device with a 'boot_index' of '0' and this will become the bootable
device. OSC allows users to do this by using either the '--volume'
option or a combination of the '--image' and '--boot-from-volume'
options, but we should also allow them to do it the "hard way" via the
'--block-device' option. For example:

  openstack server create \
    --block-device uuid=0a89ecd8-1fe2-45f0-94da-7789067911c9,boot_index=0 \
    --block-device uuid=589266ef-fd88-46e9-b7b2-94503ce8f88f,boot_index=1 \
    ... \
    my-server

Make this possible.

Change-Id: Ia48449fecbc590346630807b1c7da40102d53b33
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
Story: 2010376
Task: 46617

2 files changed, 51 insertions, 12 deletions

diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py
index 25065e22..1e685f89 100644
--- a/openstackclient/compute/v2/server.py
+++ b/openstackclient/compute/v2/server.py
@@ -891,9 +891,7 @@ class CreateServer(command.ShowOne):
             required=True,
             help=_('Create server with this flavor (name or ID)'),
         )
-        disk_group = parser.add_mutually_exclusive_group(
-            required=True,
-        )
+        disk_group = parser.add_mutually_exclusive_group()
         disk_group.add_argument(
             '--image',
             metavar='<image>',
@@ -1451,14 +1449,14 @@ class CreateServer(command.ShowOne):
         if volume:
             block_device_mapping_v2 = [{
                 'uuid': volume,
-                'boot_index': '0',
+                'boot_index': 0,
                 'source_type': 'volume',
                 'destination_type': 'volume'
             }]
         elif snapshot:
             block_device_mapping_v2 = [{
                 'uuid': snapshot,
-                'boot_index': '0',
+                'boot_index': 0,
                 'source_type': 'snapshot',
                 'destination_type': 'volume',
                 'delete_on_termination': False
@@ -1467,7 +1465,7 @@ class CreateServer(command.ShowOne):
             # Tell nova to create a root volume from the image provided.
             block_device_mapping_v2 = [{
                 'uuid': image.id,
-                'boot_index': '0',
+                'boot_index': 0,
                 'source_type': 'image',
                 'destination_type': 'volume',
                 'volume_size': parsed_args.boot_from_volume
@@ -1604,6 +1602,15 @@ class CreateServer(command.ShowOne):
 
             block_device_mapping_v2.append(mapping)
 
+        if not image and not any(
+            [bdm.get('boot_index') == 0 for bdm in block_device_mapping_v2]
+        ):
+            msg = _(
+                'An image (--image, --image-property) or bootable volume '
+                '(--volume, --snapshot, --block-device) is required'
+            )
+            raise exceptions.CommandError(msg)
+
         nics = parsed_args.nics
 
         if 'auto' in nics or 'none' in nics:
diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py
index 27ae3e9b..1fd92e93 100644
--- a/openstackclient/tests/unit/compute/v2/test_server.py
+++ b/openstackclient/tests/unit/compute/v2/test_server.py
@@ -2455,7 +2455,7 @@ class TestServerCreate(TestServer):
             'admin_pass': None,
             'block_device_mapping_v2': [{
                 'uuid': self.volume.id,
-                'boot_index': '0',
+                'boot_index': 0,
                 'source_type': 'volume',
                 'destination_type': 'volume',
             }],
@@ -2506,7 +2506,7 @@ class TestServerCreate(TestServer):
             'admin_pass': None,
             'block_device_mapping_v2': [{
                 'uuid': self.snapshot.id,
-                'boot_index': '0',
+                'boot_index': 0,
                 'source_type': 'snapshot',
                 'destination_type': 'volume',
                 'delete_on_termination': False,
@@ -2529,20 +2529,20 @@ class TestServerCreate(TestServer):
         self.assertEqual(self.datalist(), data)
 
     def test_server_create_with_block_device(self):
-        block_device = f'uuid={self.volume.id},source_type=volume'
+        block_device = f'uuid={self.volume.id},source_type=volume,boot_index=0'
         arglist = [
-            '--image', 'image1',
             '--flavor', self.flavor.id,
             '--block-device', block_device,
             self.new_server.name,
         ]
         verifylist = [
-            ('image', 'image1'),
+            ('image', None),
             ('flavor', self.flavor.id),
             ('block_devices', [
                 {
                     'uuid': self.volume.id,
                     'source_type': 'volume',
+                    'boot_index': '0',
                 },
             ]),
             ('server_name', self.new_server.name),
@@ -2569,6 +2569,7 @@ class TestServerCreate(TestServer):
                     'uuid': self.volume.id,
                     'source_type': 'volume',
                     'destination_type': 'volume',
+                    'boot_index': 0,
                 },
             ],
             'nics': [],
@@ -2578,7 +2579,7 @@ class TestServerCreate(TestServer):
         # ServerManager.create(name, image, flavor, **kwargs)
         self.servers_mock.create.assert_called_with(
             self.new_server.name,
-            self.image,
+            None,
             self.flavor,
             **kwargs
         )
@@ -3506,6 +3507,37 @@ class TestServerCreate(TestServer):
         self.assertEqual(self.columns, columns)
         self.assertEqual(self.datalist(), data)
 
+    def test_server_create_no_boot_device(self):
+        block_device = f'uuid={self.volume.id},source_type=volume,boot_index=1'
+        arglist = [
+            '--block-device', block_device,
+            '--flavor', self.flavor.id,
+            self.new_server.name,
+        ]
+        verifylist = [
+            ('image', None),
+            ('flavor', self.flavor.id),
+            ('block_devices', [
+                {
+                    'uuid': self.volume.id,
+                    'source_type': 'volume',
+                    'boot_index': '1',
+                },
+            ]),
+            ('server_name', self.new_server.name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        exc = self.assertRaises(
+            exceptions.CommandError,
+            self.cmd.take_action,
+            parsed_args,
+        )
+        self.assertIn(
+            'An image (--image, --image-property) or bootable volume '
+            '(--volume, --snapshot, --block-device) is required',
+            str(exc),
+        )
+
     def test_server_create_with_swap(self):
         arglist = [
             '--image', 'image1',