From 4bd8a4bd8e7651aae115a50d237f909fdc0f3389 Mon Sep 17 00:00:00 2001 From: Nikola Dipanov Date: Wed, 10 Jun 2015 17:26:24 +0100 Subject: [PATCH] Fix overloading of block device on boot by device name Currently it is not possible to override an existing image block device by supplying the device with the same name at boot (see also Ib1ba130042aabbbe7bb8d60fc212c66e446c1d73). Even though we want to discourage usage of device names as much as possible in the Nova API (as not all hypervisors can honour them), EC2 API requires that this is possible. While we want to make sure we document that supplying device names at boot is only really desirable if you want to override some of the ones contained in the image, introducing a different labeling system just so that we don't use the device names seems like an overkill for a feature that does not seem to be very used. This patch adds a method that will do this deterministically when compiling all the block device information for the request. It is also worth noting that The EC2 API allows only subset of block device attributes to be overridden in this way (see [1]). This limitation did not exist previously in Nova, and there seems to be no reason why we would need that complexity, so it would be up to the EC2 compatibility code to deal with this. [1] https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/block-device-mapping-concepts.html#Using_OverridingAMIBDM Doc-Impact Closes-bug: #1370250 Change-Id: I60ecdcae81ff5dec34f0fa0a39e0739759a6fa59 --- nova/compute/api.py | 11 +++++- nova/tests/unit/compute/test_compute.py | 48 +++++++++++++++++++++++-- 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index a963181adcc2..4414cec49041 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -714,6 +714,14 @@ class API(base.Base): return flavor_defined_bdms + def _merge_with_image_bdms(self, block_device_mapping, image_mappings): + """Override any block devices from the image by device name""" + device_names = set(bdm['device_name'] for bdm in block_device_mapping + if bdm['device_name']) + return (block_device_mapping + + [bdm for bdm in image_mappings + if bdm['device_name'] not in device_names]) + def _check_and_transform_bdm(self, context, base_options, instance_type, image_meta, min_count, max_count, block_device_mapping, legacy_bdm): @@ -758,7 +766,8 @@ class API(base.Base): block_device_mapping = ( filter(not_image_and_root_bdm, block_device_mapping)) - block_device_mapping += image_defined_bdms + block_device_mapping = self._merge_with_image_bdms( + block_device_mapping, image_defined_bdms) if min_count > 1 or max_count > 1: if any(map(lambda bdm: bdm['source_type'] == 'volume', diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 9fa87b262987..c8aff82c2063 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -8637,8 +8637,7 @@ class ComputeAPITestCase(BaseTestCase): self.context, base_options, {}, image_meta, 1, 1, bdms, legacy_bdms) for expected, got in zip(expected_bdms, transformed_bdm): - self.assertThat(dict(expected.items()), - matchers.DictMatches(dict(got.items()))) + self.assertEqual(dict(expected.items()), dict(got.items())) def test_check_and_transform_legacy_bdm_no_image_bdms(self): legacy_bdms = [ @@ -8716,6 +8715,51 @@ class ComputeAPITestCase(BaseTestCase): self._test_check_and_transform_bdm(bdms, expected_bdms, image_bdms=image_bdms) + def test_check_and_transform_bdm_image_bdms_w_overrides(self): + bdms = [block_device.BlockDeviceDict({'source_type': 'image', + 'destination_type': 'local', + 'image_id': FAKE_IMAGE_REF, + 'boot_index': 0}), + block_device.BlockDeviceDict({'device_name': 'vdb', + 'no_device': True})] + image_bdms = [block_device.BlockDeviceDict( + {'source_type': 'volume', 'destination_type': 'volume', + 'volume_id': '33333333-aaaa-bbbb-cccc-444444444444', + 'device_name': '/dev/vdb'})] + expected_bdms = block_device_obj.block_device_make_list_from_dicts( + self.context, bdms) + self._test_check_and_transform_bdm(bdms, expected_bdms, + image_bdms=image_bdms) + + def test_check_and_transform_bdm_image_bdms_w_overrides_complex(self): + bdms = [block_device.BlockDeviceDict({'source_type': 'image', + 'destination_type': 'local', + 'image_id': FAKE_IMAGE_REF, + 'boot_index': 0}), + block_device.BlockDeviceDict({'device_name': 'vdb', + 'no_device': True}), + block_device.BlockDeviceDict( + {'source_type': 'volume', 'destination_type': 'volume', + 'volume_id': '11111111-aaaa-bbbb-cccc-222222222222', + 'device_name': 'vdc'})] + image_bdms = [ + block_device.BlockDeviceDict( + {'source_type': 'volume', 'destination_type': 'volume', + 'volume_id': '33333333-aaaa-bbbb-cccc-444444444444', + 'device_name': '/dev/vdb'}), + block_device.BlockDeviceDict( + {'source_type': 'volume', 'destination_type': 'volume', + 'volume_id': '55555555-aaaa-bbbb-cccc-666666666666', + 'device_name': '/dev/vdc'}), + block_device.BlockDeviceDict( + {'source_type': 'volume', 'destination_type': 'volume', + 'volume_id': '77777777-aaaa-bbbb-cccc-8888888888888', + 'device_name': '/dev/vdd'})] + expected_bdms = block_device_obj.block_device_make_list_from_dicts( + self.context, bdms + [image_bdms[2]]) + self._test_check_and_transform_bdm(bdms, expected_bdms, + image_bdms=image_bdms) + def test_check_and_transform_bdm_legacy_image_bdms(self): bdms = [block_device.BlockDeviceDict({'source_type': 'image', 'destination_type': 'local',