From 9b3c4736a35b0db6ceff38786fb706a6a312a7ab Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Tue, 7 Feb 2017 20:28:13 -0500 Subject: [PATCH] Allow None for block_device_mapping_v2.boot_index The legacy v2 API allowed None for the boot_index [1]. It allowed this implicitly because the API code would convert the block_device_mapping_v2 dict from the request into a BlockDeviceMapping object, which has a boot_index field that is nullable (allows None). The API reference documentation [2] also says: "To disable a device from booting, set the boot index to a negative value or use the default boot index value, which is None." It appears that with the move to v2.1 and request schema validation, the boot_index schema was erroneously set to not allow None for a value, which is not backward compatible with the v2 API behavior. This change fixes the schema to allow boot_index=None again and adds a test to show it working. This should not require a microversion bump since it's fixing a regression in the v2.1 API which worked in the v2 API and is already handled throughout Nova's block device code. Closes-Bug: #1662699 [1] https://github.com/openstack/nova/blob/13.0.0/nova/compute/api.py#L1268 [2] http://developer.openstack.org/api-ref/compute/#create-server Change-Id: Ice78a0982bcce491f0c9690903ed2c6b6aaab1be (cherry picked from commit e34f05edb2efc79bfdd8e73cca8fa02ea6ef2d60) (cherry picked from commit ff1925ae47245f056690889a6e063ebce1bb7a80) (cherry picked from commit dce8618166d80dc6cf2854920f6275eee73b8d84) --- .../compute/schemas/block_device_mapping.py | 4 +- nova/block_device.py | 8 +++- .../compute/test_block_device_mapping.py | 45 +++++++++++++++++++ .../notes/bug-1662699-06203e7262e02aa6.yaml | 10 +++++ 4 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/bug-1662699-06203e7262e02aa6.yaml diff --git a/nova/api/openstack/compute/schemas/block_device_mapping.py b/nova/api/openstack/compute/schemas/block_device_mapping.py index b991142bb323..1fe13aaace11 100644 --- a/nova/api/openstack/compute/schemas/block_device_mapping.py +++ b/nova/api/openstack/compute/schemas/block_device_mapping.py @@ -55,8 +55,10 @@ block_device_mapping_new_item = { 'type': 'string', 'maxLength': 255, }, # Defined as integer in nova/block_device.py:from_api() + # NOTE(mriedem): boot_index=None is also accepted for backward + # compatibility with the legacy v2 API. 'boot_index': { - 'type': ['integer', 'string'], + 'type': ['integer', 'string', 'null'], 'pattern': '^-?[0-9]+$', }, } diff --git a/nova/block_device.py b/nova/block_device.py index b40aa5c7a0dc..8e6d034b65b7 100644 --- a/nova/block_device.py +++ b/nova/block_device.py @@ -197,7 +197,13 @@ class BlockDeviceDict(dict): api_dict[source_type + '_id'] = device_uuid if source_type == 'image' and destination_type == 'local': try: - boot_index = int(api_dict.get('boot_index', -1)) + # NOTE(mriedem): boot_index can be None so we need to + # account for that to avoid a TypeError. + boot_index = api_dict.get('boot_index', -1) + if boot_index is None: + # boot_index=None is equivalent to -1. + boot_index = -1 + boot_index = int(boot_index) except ValueError: raise exception.InvalidBDMFormat( details=_("Boot index is invalid.")) diff --git a/nova/tests/unit/api/openstack/compute/test_block_device_mapping.py b/nova/tests/unit/api/openstack/compute/test_block_device_mapping.py index 8471a534794f..40ac9a985cb6 100644 --- a/nova/tests/unit/api/openstack/compute/test_block_device_mapping.py +++ b/nova/tests/unit/api/openstack/compute/test_block_device_mapping.py @@ -165,6 +165,51 @@ class BlockDeviceMappingTestV21(test.TestCase): self.assertRaises(exception.ValidationError, self._test_create, params) + def test_create_instance_with_boot_index_none_ok(self): + """Tests creating a server with two block devices. One is the boot + device and the other is a non-bootable device. + """ + # From the docs: + # To disable a device from booting, set the boot index to a negative + # value or use the default boot index value, which is None. The + # simplest usage is, set the boot index of the boot device to 0 and use + # the default boot index value, None, for any other devices. + bdms = [ + # This is the bootable device that would create a 20GB cinder + # volume from the given image. + { + 'source_type': 'image', + 'destination_type': 'volume', + 'boot_index': 0, + 'uuid': '155d900f-4e14-4e4c-a73d-069cbf4541e6', + 'volume_size': 20 + }, + # This is the non-bootable 10GB ext4 ephemeral block device. + { + 'source_type': 'blank', + 'destination_type': 'local', + 'boot_index': None, + # If 'guest_format' is 'swap' then a swap device is created. + 'guest_format': 'ext4' + } + ] + params = {block_device_mapping.ATTRIBUTE_NAME: bdms} + self._test_create(params, no_image=True) + + def test_create_instance_with_boot_index_none_image_local_fails(self): + """Tests creating a server with a local image-based block device which + has a boot_index of None which is invalid. + """ + bdms = [{ + 'source_type': 'image', + 'destination_type': 'local', + 'boot_index': None, + 'uuid': '155d900f-4e14-4e4c-a73d-069cbf4541e6' + }] + params = {block_device_mapping.ATTRIBUTE_NAME: bdms} + self.assertRaises(exc.HTTPBadRequest, self._test_create, + params, no_image=True) + def test_create_instance_with_device_name_not_string(self): self.bdm[0]['device_name'] = 123 old_create = compute_api.API.create diff --git a/releasenotes/notes/bug-1662699-06203e7262e02aa6.yaml b/releasenotes/notes/bug-1662699-06203e7262e02aa6.yaml new file mode 100644 index 000000000000..94a5ab87014f --- /dev/null +++ b/releasenotes/notes/bug-1662699-06203e7262e02aa6.yaml @@ -0,0 +1,10 @@ +--- +fixes: + - | + Fixes `bug 1662699`_ which was a regression in the v2.1 API from the + ``block_device_mapping_v2.boot_index`` validation that was performed in the + legacy v2 API. With this fix, requests to create a server with + ``boot_index=None`` will be treated as if ``boot_index`` was not specified, + which defaults to meaning a non-bootable block device. + + .. _bug 1662699: https://bugs.launchpad.net/nova/+bug/1662699