Check if volume is bootable when creating an instance

Cinder provides a bootable flags for volumes that's not being
checked when creating an instance.

If a user select a non-bootable volume, then the VM became unusable
since nothing will be booted at all.

This check is done only in case that a volume is selected as root
device for a VM.  Boot from snapshot will still work (even that the
volume will be marked as non-bootable by cinder, but will boot the
attached snapshot instead)
If the attribute bootable is not present, we keep the old behaviour
and allow the instance creation.

Change-Id: I8e028d40d6a11beeac4f043afccb17eb61f6ce62
Closes-Bug: #1321334

nova/api/openstack/compute/plugins/v3/block_device_mapping.py

@@ -47,7 +47,8 @@ def server_create(self, server_dict, create_kwargs):
             block_device_mapping = [
                 block_device.BlockDeviceDict.from_api(bdm_dict)
                 for bdm_dict in block_device_mapping]
-        except exception.InvalidBDMFormat as e:
+        except (exception.InvalidBDMFormat,
+                exception.InvalidBDMVolumeNotBootable) as e:
             raise exc.HTTPBadRequest(explanation=e.format_message())
 
         create_kwargs['block_device_mapping'] = block_device_mapping

nova/compute/api.py

@@ -877,12 +877,14 @@ def _get_bdm_image_metadata(self, context, block_device_mapping,
                 try:
                     volume_id = bdm['volume_id']
                     volume = self.volume_api.get(context, volume_id)
-                    return volume.get('volume_image_metadata', {})
                 except exception.CinderConnectionFailed:
                     raise
                 except Exception:
                     raise exception.InvalidBDMVolume(id=volume_id)
 
+                if not volume.get('bootable', True):
+                    raise exception.InvalidBDMVolumeNotBootable(id=volume_id)
+                return volume.get('volume_image_metadata', {})
         return {}
 
     @staticmethod

nova/exception.py

@@ -246,6 +246,10 @@ class InvalidBDMForLegacy(InvalidBDM):
                 "be converted to legacy format. ")
 
 
+class InvalidBDMVolumeNotBootable(InvalidBDM):
+    msg_fmt = _("Block Device %(id)s is not bootable.")
+
+
 class InvalidAttribute(Invalid):
     msg_fmt = _("Attribute not supported: %(attr)s")
 

nova/tests/api/openstack/compute/plugins/v3/test_block_device_mapping.py

@@ -13,6 +13,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import mock
 import mox
 from oslo.config import cfg
 from webob import exc
@@ -271,3 +272,17 @@ def _validate(*args, **kwargs):
 
         params = {block_device_mapping.ATTRIBUTE_NAME: self.bdm}
         self.assertRaises(exc.HTTPBadRequest, self._test_create, params)
+
+    @mock.patch('nova.compute.api.API._get_bdm_image_metadata')
+    def test_create_instance_non_bootable_volume_fails(self, fake_bdm_meta):
+        bdm = [{
+            'id': 1,
+            'bootable': False,
+            'volume_id': '1',
+            'status': 'active',
+            'device_name': 'vda',
+        }]
+        params = {'block_device_mapping': bdm}
+        fake_bdm_meta.side_effect = exception.InvalidBDMVolumeNotBootable(id=1)
+        self.assertRaises(exc.HTTPBadRequest, self._test_create, params,
+                          no_image=True)

nova/tests/api/openstack/compute/test_servers.py

@@ -21,6 +21,7 @@
 
 import iso8601
 from lxml import etree
+import mock
 import mox
 from oslo.config import cfg
 import six.moves.urllib.parse as urlparse
@@ -2655,6 +2656,21 @@ def create(*args, **kwargs):
         self.stubs.Set(compute_api.API, 'create', create)
         self._test_create_extra(params)
 
+    @mock.patch('nova.compute.api.API._get_bdm_image_metadata')
+    def test_create_instance_non_bootable_volume_fails(self, fake_bdm_meta):
+        self.ext_mgr.extensions = {'os-volumes': 'fake'}
+        bdm = [{
+            'id': 1,
+            'bootable': False,
+            'volume_id': self.volume_id,
+            'status': 'active',
+            'device_name': 'vda',
+        }]
+        params = {'block_device_mapping': bdm}
+        fake_bdm_meta.side_effect = exception.InvalidBDMVolumeNotBootable(id=1)
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          self._test_create_extra, params, no_image=True)
+
     def test_create_instance_with_device_name_not_string(self):
         self.ext_mgr.extensions = {'os-volumes': 'fake'}
         old_create = compute_api.API.create

nova/tests/compute/test_compute_api.py

@@ -1693,6 +1693,36 @@ def test_volume_snapshot_delete(self):
         self.compute_api.volume_snapshot_delete(self.context, volume_id,
                 snapshot_id, {})
 
+    def _test_boot_volume_bootable(self, is_bootable=False):
+        def get_vol_data(*args, **kwargs):
+            return {'bootable': is_bootable}
+        block_device_mapping = [{
+            'id': 1,
+            'device_name': 'vda',
+            'no_device': None,
+            'virtual_name': None,
+            'snapshot_id': None,
+            'volume_id': '1',
+            'delete_on_termination': False,
+        }]
+
+        with mock.patch.object(self.compute_api.volume_api, 'get',
+                               side_effect=get_vol_data):
+            if not is_bootable:
+                self.assertRaises(exception.InvalidBDMVolumeNotBootable,
+                                  self.compute_api._get_bdm_image_metadata,
+                                  self.context, block_device_mapping)
+            else:
+                meta = self.compute_api._get_bdm_image_metadata(self.context,
+                                    block_device_mapping)
+                self.assertEqual({}, meta)
+
+    def test_boot_volume_non_bootable(self):
+        self._test_boot_volume_bootable(False)
+
+    def test_boot_volume_bootable(self):
+        self._test_boot_volume_bootable(True)
+
     def _create_instance_with_disabled_disk_config(self, object=False):
         sys_meta = {"image_auto_disk_config": "Disabled"}
         params = {"system_metadata": sys_meta}

nova/tests/test_cinder.py

@@ -35,6 +35,7 @@ def _stub_volume(**kwargs):
         "snapshot_id": None,
         "status": "available",
         "volume_type": "None",
+        "bootable": "true"
     }
     volume.update(kwargs)
     return volume

nova/volume/cinder.py

@@ -30,6 +30,7 @@
 from nova import exception
 from nova.openstack.common.gettextutils import _
 from nova.openstack.common import log as logging
+from nova.openstack.common import strutils
 
 cinder_opts = [
     cfg.StrOpt('cinder_catalog_info',
@@ -137,7 +138,7 @@ def _untranslate_volume_summary_view(context, vol):
     # TODO(jdg): Information may be lost in this translation
     d['volume_type_id'] = vol.volume_type
     d['snapshot_id'] = vol.snapshot_id
-
+    d['bootable'] = strutils.bool_from_string(vol.bootable)
     d['volume_metadata'] = {}
     for key, value in vol.metadata.items():
         d['volume_metadata'][key] = value