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 e34f05edb2)
(cherry picked from commit ff1925ae47)
This commit is contained in:
Matt Riedemann 2017-02-07 20:28:13 -05:00
parent 99f8a3c4e9
commit dce8618166
4 changed files with 65 additions and 2 deletions

View File

@ -56,8 +56,10 @@ block_device_mapping_new_item = {
'type': 'string', 'maxLength': 255, 'type': 'string', 'maxLength': 255,
}, },
# Defined as integer in nova/block_device.py:from_api() # 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': { 'boot_index': {
'type': ['integer', 'string'], 'type': ['integer', 'string', 'null'],
'pattern': '^-?[0-9]+$', 'pattern': '^-?[0-9]+$',
}, },
} }

View File

@ -197,7 +197,13 @@ class BlockDeviceDict(dict):
api_dict[source_type + '_id'] = device_uuid api_dict[source_type + '_id'] = device_uuid
if source_type == 'image' and destination_type == 'local': if source_type == 'image' and destination_type == 'local':
try: 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: except ValueError:
raise exception.InvalidBDMFormat( raise exception.InvalidBDMFormat(
details=_("Boot index is invalid.")) details=_("Boot index is invalid."))

View File

@ -201,6 +201,51 @@ class BlockDeviceMappingTestV21(test.TestCase):
self.assertRaises(exception.ValidationError, self.assertRaises(exception.ValidationError,
self._test_create, params) 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): def test_create_instance_with_device_name_not_string(self):
self.bdm[0]['device_name'] = 123 self.bdm[0]['device_name'] = 123
old_create = compute_api.API.create old_create = compute_api.API.create

View File

@ -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