From 0d5fb06b39e8244429be72f05e2066d24572dc2e Mon Sep 17 00:00:00 2001 From: Nikola Dipanov Date: Wed, 15 May 2013 15:47:31 +0200 Subject: DB migration to the new BDM data format This patch migrates the DB to the new data format. In addition it also utilizes routines introduced in the change I9370333059b8c9aaf92010470b8475a913d329b2 in a way that will allow us to transition into using the new data in Nova logic one step at a time. This is accomplished in a following manner in the DB/conductor layer, which is supposed to allow for subsequent changes to be as granular as possible: * Read operations - data is always read as is found in the DB - meaning in the new format, and transformed after every call. This will allow us to make granular changes in the API/Compute layers. * Data is converted inside the DB methods that do writes, and an additional 'legacy' flag is added (set to True by default). It is up to the calling method to make sure it supplies the DB layer with the format it is intending to write, to avoid guessing. An exception to the above is when using conductor due to rpcapi versioning, so this patch adds a 'legacy' flag to the block_device_mapping_get_all_by_instance conductor method and bumps the version of the API. This patch also fixes some of the block device fixtures in tests, when it was required to be aware of the new data structure (mostly when mocking DB methods that return the new data format). This patch is not supposed to provide any new functionality to Nova. blueprint: improve-block-device-handling Change-Id: If30afdb59d4c4268b97d3d10270df2cc729a0c4c --- nova/api/ec2/cloud.py | 5 +- nova/compute/api.py | 30 ++- nova/conductor/api.py | 5 +- nova/conductor/manager.py | 8 +- nova/conductor/rpcapi.py | 9 +- nova/db/api.py | 12 +- nova/db/sqlalchemy/api.py | 45 +++- .../migrate_repo/versions/186_new_bdm_format.py | 262 +++++++++++++++++++++ nova/db/sqlalchemy/models.py | 16 +- nova/tests/api/ec2/test_cloud.py | 26 +- .../api/openstack/compute/test_server_actions.py | 3 +- nova/tests/api/openstack/compute/test_servers.py | 44 +++- nova/tests/compute/test_compute.py | 28 ++- nova/tests/conductor/test_conductor.py | 2 +- nova/tests/db/test_db_api.py | 62 +++-- nova/tests/db/test_migrations.py | 118 ++++++++++ nova/tests/test_metadata.py | 12 +- 17 files changed, 586 insertions(+), 101 deletions(-) create mode 100644 nova/db/sqlalchemy/migrate_repo/versions/186_new_bdm_format.py diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index da0a52caa..8f4e20798 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -1058,8 +1058,9 @@ class CloudController(object): """Format InstanceBlockDeviceMappingResponseItemType.""" root_device_type = 'instance-store' mapping = [] - for bdm in db.block_device_mapping_get_all_by_instance(context, - instance_uuid): + for bdm in block_device.legacy_mapping( + db.block_device_mapping_get_all_by_instance(context, + instance_uuid)): volume_id = bdm['volume_id'] if (volume_id is None or bdm['no_device']): continue diff --git a/nova/compute/api.py b/nova/compute/api.py index 0abe03c2f..0674cff5f 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -860,8 +860,9 @@ class API(base.Base): values) def _validate_bdm(self, context, instance): - for bdm in self.db.block_device_mapping_get_all_by_instance( - context, instance['uuid']): + for bdm in block_device.legacy_mapping( + self.db.block_device_mapping_get_all_by_instance( + context, instance['uuid'])): # NOTE(vish): For now, just make sure the volumes are accessible. # Additionally, check that the volume can be attached to this # instance. @@ -1105,8 +1106,9 @@ class API(base.Base): return host = instance['host'] - bdms = self.db.block_device_mapping_get_all_by_instance( - context, instance['uuid']) + bdms = block_device.legacy_mapping( + self.db.block_device_mapping_get_all_by_instance( + context, instance['uuid'])) reservations = None if context.is_admin and context.project_id != instance['project_id']: @@ -1681,7 +1683,8 @@ class API(base.Base): properties['root_device_name'] = instance['root_device_name'] properties.update(extra_properties or {}) - bdms = self.get_instance_bdms(context, instance) + bdms = block_device.legacy_mapping( + self.get_instance_bdms(context, instance)) mapping = [] for bdm in bdms: @@ -1754,8 +1757,9 @@ class API(base.Base): return min_ram, min_disk def _get_block_device_info(self, context, instance_uuid): - bdms = self.db.block_device_mapping_get_all_by_instance(context, - instance_uuid) + bdms = block_device.legacy_mapping( + self.db.block_device_mapping_get_all_by_instance(context, + instance_uuid)) block_device_mapping = [] for bdm in bdms: if not bdm['volume_id']: @@ -1872,8 +1876,10 @@ class API(base.Base): # system metadata... and copy in the properties for the new image. orig_sys_metadata = _reset_image_metadata() - bdms = self.db.block_device_mapping_get_all_by_instance(context, - instance['uuid']) + bdms = block_device.legacy_mapping( + self.db.block_device_mapping_get_all_by_instance( + context, + instance['uuid'])) self._record_action_start(context, instance, instance_actions.REBUILD) @@ -2208,7 +2214,8 @@ class API(base.Base): def rescue(self, context, instance, rescue_password=None): """Rescue the given instance.""" - bdms = self.get_instance_bdms(context, instance) + bdms = block_device.legacy_mapping( + self.get_instance_bdms(context, instance)) for bdm in bdms: if bdm['volume_id']: volume = self.volume_api.get(context, bdm['volume_id']) @@ -2502,7 +2509,8 @@ class API(base.Base): return True if bdms is None: - bdms = self.get_instance_bdms(context, instance) + bdms = block_device.legacy_mapping( + self.get_instance_bdms(context, instance)) for bdm in bdms: if (block_device.strip_dev(bdm['device_name']) == diff --git a/nova/conductor/api.py b/nova/conductor/api.py index b1243b70d..74b8ce700 100644 --- a/nova/conductor/api.py +++ b/nova/conductor/api.py @@ -196,9 +196,10 @@ class LocalAPI(object): return self._manager.block_device_mapping_update_or_create(context, values) - def block_device_mapping_get_all_by_instance(self, context, instance): + def block_device_mapping_get_all_by_instance(self, context, instance, + legacy=True): return self._manager.block_device_mapping_get_all_by_instance( - context, instance) + context, instance, legacy) def block_device_mapping_destroy(self, context, bdms): return self._manager.block_device_mapping_destroy(context, bdms=bdms) diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index bd4268963..6eccaf341 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -17,6 +17,7 @@ import copy from nova.api.ec2 import ec2utils +from nova import block_device from nova.compute import api as compute_api from nova.compute import utils as compute_utils from nova import exception @@ -66,7 +67,7 @@ class ConductorManager(manager.Manager): namespace. See the ComputeTaskManager class for details. """ - RPC_API_VERSION = '1.50' + RPC_API_VERSION = '1.51' def __init__(self, *args, **kwargs): super(ConductorManager, self).__init__(service_name='conductor', @@ -264,9 +265,12 @@ class ConductorManager(manager.Manager): else: self.db.block_device_mapping_update(context, values['id'], values) - def block_device_mapping_get_all_by_instance(self, context, instance): + def block_device_mapping_get_all_by_instance(self, context, instance, + legacy=True): bdms = self.db.block_device_mapping_get_all_by_instance( context, instance['uuid']) + if legacy: + bdms = block_device.legacy_mapping(bdms) return jsonutils.to_primitive(bdms) def block_device_mapping_destroy(self, context, bdms=None, diff --git a/nova/conductor/rpcapi.py b/nova/conductor/rpcapi.py index e1f65fae2..c9b1c0af7 100644 --- a/nova/conductor/rpcapi.py +++ b/nova/conductor/rpcapi.py @@ -90,6 +90,8 @@ class ConductorAPI(nova.openstack.common.rpc.proxy.RpcProxy): 1.48 - Added compute_unrescue 1.49 - Added columns_to_join to instance_get_by_uuid 1.50 - Added object_action() and object_class_action() + 1.51 - Added the 'legacy' argument to + block_device_mapping_get_all_by_instance """ BASE_RPC_API_VERSION = '1.0' @@ -230,11 +232,12 @@ class ConductorAPI(nova.openstack.common.rpc.proxy.RpcProxy): values=values, create=create) return self.call(context, msg, version='1.12') - def block_device_mapping_get_all_by_instance(self, context, instance): + def block_device_mapping_get_all_by_instance(self, context, instance, + legacy=True): instance_p = jsonutils.to_primitive(instance) msg = self.make_msg('block_device_mapping_get_all_by_instance', - instance=instance_p) - return self.call(context, msg, version='1.13') + instance=instance_p, legacy=legacy) + return self.call(context, msg, version='1.51') def block_device_mapping_destroy(self, context, bdms=None, instance=None, volume_id=None, diff --git a/nova/db/api.py b/nova/db/api.py index 78e2eb7a4..8a7c6dc48 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -1013,20 +1013,20 @@ def ec2_snapshot_create(context, snapshot_id, forced_id=None): #################### -def block_device_mapping_create(context, values): +def block_device_mapping_create(context, values, legacy=True): """Create an entry of block device mapping.""" - return IMPL.block_device_mapping_create(context, values) + return IMPL.block_device_mapping_create(context, values, legacy) -def block_device_mapping_update(context, bdm_id, values): +def block_device_mapping_update(context, bdm_id, values, legacy=True): """Update an entry of block device mapping.""" - return IMPL.block_device_mapping_update(context, bdm_id, values) + return IMPL.block_device_mapping_update(context, bdm_id, values, legacy) -def block_device_mapping_update_or_create(context, values): +def block_device_mapping_update_or_create(context, values, legacy=True): """Update an entry of block device mapping. If not existed, create a new entry""" - return IMPL.block_device_mapping_update_or_create(context, values) + return IMPL.block_device_mapping_update_or_create(context, values, legacy) def block_device_mapping_get_all_by_instance(context, instance_uuid): diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index af9486b3e..adacc6ead 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -3122,24 +3122,35 @@ def _scrub_empty_str_values(dct, keys_to_scrub): del dct[key] +def _from_legacy_values(values, legacy, allow_updates=False): + if legacy: + if allow_updates and block_device.is_safe_for_update(values): + return values + else: + return block_device.BlockDeviceDict.from_legacy(values) + else: + return values + + @require_context -def block_device_mapping_create(context, values): +def block_device_mapping_create(context, values, legacy=True): _scrub_empty_str_values(values, ['volume_size']) + values = _from_legacy_values(values, legacy) bdm_ref = models.BlockDeviceMapping() bdm_ref.update(values) bdm_ref.save() @require_context -def block_device_mapping_update(context, bdm_id, values): +def block_device_mapping_update(context, bdm_id, values, legacy=True): _scrub_empty_str_values(values, ['volume_size']) + values = _from_legacy_values(values, legacy, allow_updates=True) _block_device_mapping_get_query(context).\ filter_by(id=bdm_id).\ update(values) -@require_context -def block_device_mapping_update_or_create(context, values): +def block_device_mapping_update_or_create(context, values, legacy=True): _scrub_empty_str_values(values, ['volume_size']) session = get_session() with session.begin(): @@ -3148,24 +3159,32 @@ def block_device_mapping_update_or_create(context, values): filter_by(device_name=values['device_name']).\ first() if not result: + values = _from_legacy_values(values, legacy) bdm_ref = models.BlockDeviceMapping() bdm_ref.update(values) bdm_ref.save(session=session) else: + values = _from_legacy_values(values, legacy, allow_updates=True) result.update(values) # NOTE(yamahata): same virtual device name can be specified multiple # times. So delete the existing ones. - virtual_name = values['virtual_name'] - if (virtual_name is not None and - block_device.is_swap_or_ephemeral(virtual_name)): - - _block_device_mapping_get_query(context, session=session).\ - filter_by(instance_uuid=values['instance_uuid']).\ - filter_by(virtual_name=virtual_name).\ + # TODO(ndipanov): Just changed to use new format for now - + # should be moved out of db layer or removed completely + if values.get('source_type') == 'blank': + is_swap = values.get('guest_format') == 'swap' + query = (_block_device_mapping_get_query(context, session=session). + filter_by(instance_uuid=values['instance_uuid']). + filter_by(source_type='blank'). filter(models.BlockDeviceMapping.device_name != - values['device_name']).\ - soft_delete() + values['device_name'])) + if is_swap: + query.filter_by(guest_format='swap').soft_delete() + else: + (query.filter(or_( + models.BlockDeviceMapping.guest_format == None, + models.BlockDeviceMapping.guest_format != 'swap')). + soft_delete()) @require_context diff --git a/nova/db/sqlalchemy/migrate_repo/versions/186_new_bdm_format.py b/nova/db/sqlalchemy/migrate_repo/versions/186_new_bdm_format.py new file mode 100644 index 000000000..bb16d7bbf --- /dev/null +++ b/nova/db/sqlalchemy/migrate_repo/versions/186_new_bdm_format.py @@ -0,0 +1,262 @@ +# vim: tabstop=4 shiftwidth=4 softtabstop=4 + +# Copyright 2013 OpenStack LLC. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import itertools +import re + +from sqlalchemy import Column, Integer, MetaData, String, Table +from sqlalchemy.sql.expression import select + +from nova.openstack.common import log as logging +from oslo.config import cfg + + +CONF = cfg.CONF +CONF.import_opt('default_ephemeral_format', 'nova.virt.driver') +LOG = logging.getLogger(__name__) + + +_ephemeral = re.compile('^ephemeral(\d|[1-9]\d+)$') + + +def _is_ephemeral(device_name): + return bool(_ephemeral.match(device_name)) + + +def _is_swap_or_ephemeral(device_name): + return (device_name and + (device_name == 'swap' or _is_ephemeral(device_name))) + + +_dev = re.compile('^/dev/') + + +def strip_dev(device_name): + """remove leading '/dev/'.""" + return _dev.sub('', device_name) if device_name else device_name + + +def upgrade(migrate_engine): + meta = MetaData(bind=migrate_engine) + + for table in ('block_device_mapping', 'shadow_block_device_mapping'): + block_device_mapping = Table(table, + meta, autoload=True) + + source_type = Column('source_type', String(255)) + destination_type = Column('destination_type', String(255)) + guest_format = Column('guest_format', String(255)) + device_type = Column('device_type', String(255)) + disk_bus = Column('disk_bus', String(255)) + boot_index = Column('boot_index', Integer) + image_id = Column('image_id', String(36)) + + source_type.create(block_device_mapping) + destination_type.create(block_device_mapping) + guest_format.create(block_device_mapping) + device_type.create(block_device_mapping) + disk_bus.create(block_device_mapping) + boot_index.create(block_device_mapping) + image_id.create(block_device_mapping) + + device_name = block_device_mapping.c.device_name + device_name.alter(nullable=True) + + _upgrade_bdm_v2(meta, block_device_mapping) + + virtual_name = block_device_mapping.c.virtual_name + virtual_name.drop() + + +def downgrade(migrate_engine): + meta = MetaData(bind=migrate_engine) + + for table in ('block_device_mapping', 'shadow_block_device_mapping'): + block_device_mapping = Table(table, meta, autoload=True) + + virtual_name = Column('virtual_name', String(255), nullable=True) + virtual_name.create(block_device_mapping) + + _downgrade_bdm_v2(meta, block_device_mapping) + + device_name = block_device_mapping.c.device_name + device_name.alter(nullable=True) + + block_device_mapping.c.source_type.drop() + block_device_mapping.c.destination_type.drop() + block_device_mapping.c.guest_format.drop() + block_device_mapping.c.device_type.drop() + block_device_mapping.c.disk_bus.drop() + block_device_mapping.c.boot_index.drop() + block_device_mapping.c.image_id.drop() + + +def _upgrade_bdm_v2(meta, bdm_table): + # Rows needed to do the upgrade + _bdm_rows_v1 = ('id', 'device_name', 'virtual_name', + 'snapshot_id', 'volume_id', 'instance_uuid') + + _bdm_rows_v2 = ('id', 'source_type', 'destination_type', 'guest_format', + 'device_type', 'disk_bus', 'boot_index', 'image_id') + + def _get_columns(table, names): + return [getattr(table.c, name) for name in names] + + def _default_bdm(): + # Set some common default values + default = {} + default['destination_type'] = 'local' + default['device_type'] = 'disk' + default['boot_index'] = -1 + return default + + instance_table = Table('instances', meta, autoload=True) + instance_shadow_table = Table('shadow_instances', meta, autoload=True) + + for instance in itertools.chain( + instance_table.select().execute().fetchall(), + instance_shadow_table.select().execute().fetchall()): + # Get all the bdms for an instance + bdm_q = select(_get_columns(bdm_table, _bdm_rows_v1)).where( + bdm_table.c.instance_uuid == instance.uuid) + + bdms_v1 = [val for val in bdm_q.execute().fetchall()] + bdms_v2 = [] + image_bdm = None + + for bdm in bdms_v1: + bdm_v2 = _default_bdm() + # Copy over some fields we'll need + bdm_v2['id'] = bdm['id'] + bdm_v2['device_name'] = bdm['device_name'] + + virt_name = bdm.virtual_name + if _is_swap_or_ephemeral(virt_name): + bdm_v2['source_type'] = 'blank' + + if virt_name == 'swap': + bdm_v2['guest_format'] = 'swap' + else: + bdm_v2['guest_format'] = CONF.default_ephemeral_format + + bdms_v2.append(bdm_v2) + + elif bdm.snapshot_id: + bdm_v2['source_type'] = 'snapshot' + bdm_v2['destination_type'] = 'volume' + + bdms_v2.append(bdm_v2) + + elif bdm.volume_id: + bdm_v2['source_type'] = 'volume' + bdm_v2['destination_type'] = 'volume' + + bdms_v2.append(bdm_v2) + else: # Log a warning that the bdm is not as expected + LOG.warn("Got an unexpected block device %s" + "that cannot be converted to v2 format" % bdm) + + if instance.image_ref: + image_bdm = _default_bdm() + image_bdm['source_type'] = 'image' + image_bdm['instance_uuid'] = instance.uuid + image_bdm['image_id'] = instance.image_ref + + # NOTE (ndipanov): Mark only the image or the bootable volume + # with boot index, as we don't support it yet. + # Also, make sure that instances started with + # the old syntax of specifying an image *and* + # a bootable volume still have consistend data. + bootable = [bdm for bdm in bdms_v2 + if strip_dev(bdm['device_name']) == + strip_dev(instance.root_device_name) + and bdm['source_type'] != 'blank'] + + if len(bootable) > 1: + LOG.warn("Found inconsistent block device data for " + "instance %s - non-unique bootable device." + % instance.uuid) + if bootable: + bootable[0]['boot_index'] = 0 + elif instance.image_ref: + image_bdm['boot_index'] = 0 + else: + LOG.warn("No bootable device found for instance %s." + % instance.uuid) + + # Update the DB + if image_bdm: + bdm_table.insert().values(**image_bdm).execute() + + for bdm in bdms_v2: + bdm_table.update().where( + bdm_table.c.id == bdm['id'] + ).values(**bdm).execute() + + +def _downgrade_bdm_v2(meta, bdm_table): + # First delete all the image bdms + + # NOTE (ndipanov): This will delete all the image bdms, even the ones + # that were potentially created as part of th normal + # operation, not only the upgrade. We have to do it, + # as we have no way of handling them in the old code. + bdm_table.delete().where(bdm_table.c.source_type == 'image').execute() + + # NOTE (ndipanov): Set all NULL device_names (if any) to '' and let the + # Nova code deal with that. This is needed so that the + # return of nullable=True does not break, and should + # happen only if there are instances that are just + # starting up when we do the downgrade + bdm_table.update().where( + bdm_table.c.device_name == None + ).values(device_name='').execute() + + instance = Table('instances', meta, autoload=True) + instance_shadow = Table('shadow_instances', meta, autoload=True) + instance_q = select([instance.c.uuid]) + instance_shadow_q = select([instance_shadow.c.uuid]) + + for instance_uuid, in itertools.chain( + instance_q.execute().fetchall(), + instance_shadow_q.execute().fetchall()): + # Get all the bdms for an instance + bdm_q = select( + [bdm_table.c.id, bdm_table.c.source_type, bdm_table.c.guest_format] + ).where( + (bdm_table.c.instance_uuid == instance_uuid) & + (bdm_table.c.source_type == 'blank') + ).order_by(bdm_table.c.id.asc()) + + blanks = [ + dict(zip(('id', 'source', 'format'), row)) + for row in bdm_q.execute().fetchall() + ] + + swap = [dev for dev in blanks if dev['format'] == 'swap'] + assert len(swap) < 2 + ephemerals = [dev for dev in blanks if dev not in swap] + + for index, eph in enumerate(ephemerals): + eph['virtual_name'] = 'ephemeral' + str(index) + + if swap: + swap[0]['virtual_name'] = 'swap' + + for bdm in swap + ephemerals: + bdm_table.update().where( + bdm_table.c.id == bdm['id'] + ).values(**bdm).execute() diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index c4660226f..f8f2c00c1 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -444,7 +444,16 @@ class BlockDeviceMapping(BASE, NovaBase): 'Instance.uuid,' 'BlockDeviceMapping.deleted==' '0)') - device_name = Column(String(255), nullable=False) + + source_type = Column(String(255)) + destination_type = Column(String(255)) + guest_format = Column(String(255)) + device_type = Column(String(255)) + disk_bus = Column(String(255)) + + boot_index = Column(Integer) + + device_name = Column(String(255)) # default=False for compatibility of the existing code. # With EC2 API, @@ -452,14 +461,13 @@ class BlockDeviceMapping(BASE, NovaBase): # default False for created with other timing. delete_on_termination = Column(Boolean, default=False) - # for ephemeral device - virtual_name = Column(String(255), nullable=True) - snapshot_id = Column(String(36)) volume_id = Column(String(36), nullable=True) volume_size = Column(Integer, nullable=True) + image_id = Column('image_id', String(36)) + # for no device to suppress devices. no_device = Column(Boolean, nullable=True) diff --git a/nova/tests/api/ec2/test_cloud.py b/nova/tests/api/ec2/test_cloud.py index 22f9c2d81..8a10712cb 100644 --- a/nova/tests/api/ec2/test_cloud.py +++ b/nova/tests/api/ec2/test_cloud.py @@ -2119,9 +2119,10 @@ class CloudTestCase(test.TestCase): def fake_block_device_mapping_get_all_by_instance(context, inst_id): return [dict(id=1, + source_type='snapshot', + destination_type='volume', snapshot_id=snapshots[0], volume_id=volumes[0], - virtual_name=None, volume_size=1, device_name='sda1', delete_on_termination=False, @@ -2210,45 +2211,54 @@ class CloudTestCase(test.TestCase): @staticmethod def _fake_bdm_get(ctxt, id): return [{'volume_id': 87654321, + 'source_type': 'volume', + 'destination_type': 'volume', 'snapshot_id': None, 'no_device': None, - 'virtual_name': None, 'delete_on_termination': True, 'device_name': '/dev/sdh'}, {'volume_id': None, 'snapshot_id': 98765432, + 'source_type': 'snapshot', + 'destination_type': 'volume', 'no_device': None, - 'virtual_name': None, 'delete_on_termination': True, 'device_name': '/dev/sdi'}, {'volume_id': None, 'snapshot_id': None, 'no_device': True, - 'virtual_name': None, 'delete_on_termination': None, 'device_name': None}, {'volume_id': None, 'snapshot_id': None, 'no_device': None, - 'virtual_name': 'ephemeral0', + 'source_type': 'blank', + 'destination_type': 'local', + 'guest_format': None, 'delete_on_termination': None, 'device_name': '/dev/sdb'}, {'volume_id': None, 'snapshot_id': None, 'no_device': None, - 'virtual_name': 'swap', + 'source_type': 'blank', + 'destination_type': 'local', + 'guest_format': 'swap', 'delete_on_termination': None, 'device_name': '/dev/sdc'}, {'volume_id': None, 'snapshot_id': None, 'no_device': None, - 'virtual_name': 'ephemeral1', + 'source_type': 'blank', + 'destination_type': 'local', + 'guest_format': None, 'delete_on_termination': None, 'device_name': '/dev/sdd'}, {'volume_id': None, 'snapshot_id': None, 'no_device': None, - 'virtual_name': 'ephemeral2', + 'source_type': 'blank', + 'destination_type': 'local', + 'guest_format': None, 'delete_on_termination': None, 'device_name': '/dev/sd3'}, ] diff --git a/nova/tests/api/openstack/compute/test_server_actions.py b/nova/tests/api/openstack/compute/test_server_actions.py index 473d3a253..f1defe039 100644 --- a/nova/tests/api/openstack/compute/test_server_actions.py +++ b/nova/tests/api/openstack/compute/test_server_actions.py @@ -843,7 +843,8 @@ class ServerActionsControllerTest(test.TestCase): def fake_block_device_mapping_get_all_by_instance(context, inst_id): return [dict(volume_id=_fake_id('a'), - virtual_name=None, + source_type='snapshot', + destination_type='volume', volume_size=1, device_name='vda', snapshot_id=1, diff --git a/nova/tests/api/openstack/compute/test_servers.py b/nova/tests/api/openstack/compute/test_servers.py index e11023308..97a1a5826 100644 --- a/nova/tests/api/openstack/compute/test_servers.py +++ b/nova/tests/api/openstack/compute/test_servers.py @@ -1807,6 +1807,7 @@ class ServersControllerCreateTest(test.TestCase): "fixed_ips": [], "task_state": "", "vm_state": "", + "root_device_name": inst.get('root_device_name', 'vda'), } self.instance_cache_by_id[instance['id']] = instance @@ -2411,7 +2412,7 @@ class ServersControllerCreateTest(test.TestCase): def test_create_instance_with_volumes_enabled(self): self.ext_mgr.extensions = {'os-volumes': 'fake'} - bdm = [{'device_name': 'foo'}] + bdm = [{'device_name': 'foo', 'volume_id': 'fake_vol'}] params = {'block_device_mapping': bdm} old_create = compute_api.API.create @@ -2419,7 +2420,11 @@ class ServersControllerCreateTest(test.TestCase): self.assertEqual(kwargs['block_device_mapping'], bdm) return old_create(*args, **kwargs) + def _validate_bdm(*args, **kwargs): + pass + self.stubs.Set(compute_api.API, 'create', create) + self.stubs.Set(compute_api.API, '_validate_bdm', _validate_bdm) self._test_create_extra(params) def test_create_instance_with_volumes_enabled_no_image(self): @@ -2471,6 +2476,9 @@ class ServersControllerCreateTest(test.TestCase): self.assertNotIn('imageRef', kwargs) return old_create(*args, **kwargs) + def _validate_bdm(*args, **kwargs): + pass + self.stubs.Set(compute_api.API, 'create', create) self.mox.ReplayAll() self._test_create_extra(params, no_image=True) @@ -2557,17 +2565,27 @@ class ServersControllerCreateTest(test.TestCase): def test_create_instance_with_bdm_delete_on_termination(self): self.ext_mgr.extensions = {'os-volumes': 'fake'} - bdm = [{'device_name': 'foo1', 'delete_on_termination': 1}, - {'device_name': 'foo2', 'delete_on_termination': True}, - {'device_name': 'foo3', 'delete_on_termination': 'invalid'}, - {'device_name': 'foo4', 'delete_on_termination': 0}, - {'device_name': 'foo5', 'delete_on_termination': False}] + bdm = [{'device_name': 'foo1', 'volume_id': 'fake_vol', + 'delete_on_termination': 1}, + {'device_name': 'foo2', 'volume_id': 'fake_vol', + 'delete_on_termination': True}, + {'device_name': 'foo3', 'volume_id': 'fake_vol', + 'delete_on_termination': 'invalid'}, + {'device_name': 'foo4', 'volume_id': 'fake_vol', + 'delete_on_termination': 0}, + {'device_name': 'foo5', 'volume_id': 'fake_vol', + 'delete_on_termination': False}] expected_bdm = [ - {'device_name': 'foo1', 'delete_on_termination': True}, - {'device_name': 'foo2', 'delete_on_termination': True}, - {'device_name': 'foo3', 'delete_on_termination': False}, - {'device_name': 'foo4', 'delete_on_termination': False}, - {'device_name': 'foo5', 'delete_on_termination': False}] + {'device_name': 'foo1', 'volume_id': 'fake_vol', + 'delete_on_termination': True}, + {'device_name': 'foo2', 'volume_id': 'fake_vol', + 'delete_on_termination': True}, + {'device_name': 'foo3', 'volume_id': 'fake_vol', + 'delete_on_termination': False}, + {'device_name': 'foo4', 'volume_id': 'fake_vol', + 'delete_on_termination': False}, + {'device_name': 'foo5', 'volume_id': 'fake_vol', + 'delete_on_termination': False}] params = {'block_device_mapping': bdm} old_create = compute_api.API.create @@ -2575,7 +2593,11 @@ class ServersControllerCreateTest(test.TestCase): self.assertEqual(expected_bdm, kwargs['block_device_mapping']) return old_create(*args, **kwargs) + def _validate_bdm(*args, **kwargs): + pass + self.stubs.Set(compute_api.API, 'create', create) + self.stubs.Set(compute_api.API, '_validate_bdm', _validate_bdm) self._test_create_extra(params) def test_create_instance_with_user_data_enabled(self): diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py index c04f5cbe7..3e0fbe63e 100644 --- a/nova/tests/compute/test_compute.py +++ b/nova/tests/compute/test_compute.py @@ -31,6 +31,7 @@ import mox from oslo.config import cfg import nova +from nova import block_device from nova import compute from nova.compute import api as compute_api from nova.compute import flavors @@ -547,7 +548,8 @@ class ComputeVolumeTestCase(BaseTestCase): block_device_mapping = [{ 'id': 1, 'no_device': None, - 'virtual_name': None, + 'source_type': 'volume', + 'destination_type': 'volume', 'snapshot_id': None, 'volume_id': self.volume_id, 'device_name': 'vda', @@ -6145,6 +6147,8 @@ class ComputeAPITestCase(BaseTestCase): def fake_get_instance_bdms(*args, **kwargs): return [{'device_name': '/dev/vda', + 'source_type': 'volume', + 'destination_type': 'volume', 'volume_id': 'bf0b6b00-a20c-11e2-9e96-0800200c9a66'}] self.stubs.Set(self.compute_api, 'get_instance_bdms', @@ -7197,12 +7201,14 @@ class ComputeAPITestCase(BaseTestCase): self.context, instance_type, instance['uuid'], mappings) bdms = [self._parse_db_block_device_mapping(bdm_ref) - for bdm_ref in db.block_device_mapping_get_all_by_instance( - self.context, instance['uuid'])] + for bdm_ref in block_device.legacy_mapping( + db.block_device_mapping_get_all_by_instance( + self.context, instance['uuid']))] expected_result = [ {'virtual_name': 'swap', 'device_name': '/dev/sdb1', - 'volume_size': swap_size}, - {'virtual_name': 'ephemeral0', 'device_name': '/dev/sdc1'}, + 'volume_size': swap_size, 'delete_on_termination': True}, + {'virtual_name': 'ephemeral0', 'device_name': '/dev/sdc1', + 'delete_on_termination': True}, # NOTE(yamahata): ATM only ephemeral0 is supported. # they're ignored for now @@ -7217,21 +7223,23 @@ class ComputeAPITestCase(BaseTestCase): self.context, flavors.get_default_instance_type(), instance['uuid'], block_device_mapping) bdms = [self._parse_db_block_device_mapping(bdm_ref) - for bdm_ref in db.block_device_mapping_get_all_by_instance( - self.context, instance['uuid'])] + for bdm_ref in block_device.legacy_mapping( + db.block_device_mapping_get_all_by_instance( + self.context, instance['uuid']))] expected_result = [ {'snapshot_id': '00000000-aaaa-bbbb-cccc-000000000000', 'device_name': '/dev/sda1'}, {'virtual_name': 'swap', 'device_name': '/dev/sdb1', - 'volume_size': swap_size}, + 'volume_size': swap_size, 'delete_on_termination': True}, {'snapshot_id': '11111111-aaaa-bbbb-cccc-111111111111', 'device_name': '/dev/sdb2'}, {'snapshot_id': '22222222-aaaa-bbbb-cccc-222222222222', 'device_name': '/dev/sdb3'}, {'no_device': True, 'device_name': '/dev/sdb4'}, - {'virtual_name': 'ephemeral0', 'device_name': '/dev/sdc1'}, + {'virtual_name': 'ephemeral0', 'device_name': '/dev/sdc1', + 'delete_on_termination': True}, {'snapshot_id': '33333333-aaaa-bbbb-cccc-333333333333', 'device_name': '/dev/sdc2'}, {'snapshot_id': '44444444-aaaa-bbbb-cccc-444444444444', @@ -7419,6 +7427,8 @@ class ComputeAPITestCase(BaseTestCase): def fake_get_instance_bdms(*args, **kwargs): return [{'device_name': '/dev/vda', + 'source_type': 'volume', + 'destination_type': 'volume', 'volume_id': 'bf0b6b00-a20c-11e2-9e96-0800200c9a66'}] self.stubs.Set(self.compute_api, 'get_instance_bdms', diff --git a/nova/tests/conductor/test_conductor.py b/nova/tests/conductor/test_conductor.py index 8b397db02..e5abd1182 100644 --- a/nova/tests/conductor/test_conductor.py +++ b/nova/tests/conductor/test_conductor.py @@ -321,7 +321,7 @@ class _BaseTestCase(object): self.context, fake_inst['uuid']).AndReturn('fake-result') self.mox.ReplayAll() result = self.conductor.block_device_mapping_get_all_by_instance( - self.context, fake_inst) + self.context, fake_inst, legacy=False) self.assertEqual(result, 'fake-result') def test_instance_get_active_by_window_joined(self): diff --git a/nova/tests/db/test_db_api.py b/nova/tests/db/test_db_api.py index d3913117b..60811e65b 100644 --- a/nova/tests/db/test_db_api.py +++ b/nova/tests/db/test_db_api.py @@ -33,6 +33,7 @@ from sqlalchemy import MetaData from sqlalchemy.orm import query from sqlalchemy.sql.expression import select +from nova import block_device from nova.compute import vm_states from nova import context from nova import db @@ -4004,8 +4005,11 @@ class BlockDeviceMappingTestCase(test.TestCase): def _create_bdm(self, values): values.setdefault('instance_uuid', self.instance['uuid']) values.setdefault('device_name', 'fake_device') - db.block_device_mapping_create(self.ctxt, values) - uuid = values['instance_uuid'] + values.setdefault('source_type', 'volume') + values.setdefault('destination_type', 'volume') + block_dev = block_device.BlockDeviceDict(values) + db.block_device_mapping_create(self.ctxt, block_dev, legacy=False) + uuid = block_dev['instance_uuid'] bdms = db.block_device_mapping_get_all_by_instance(self.ctxt, uuid) @@ -4036,81 +4040,90 @@ class BlockDeviceMappingTestCase(test.TestCase): def test_block_device_mapping_update(self): bdm = self._create_bdm({}) db.block_device_mapping_update(self.ctxt, bdm['id'], - {'virtual_name': 'some_virt_name'}) + {'destination_type': 'moon'}, + legacy=False) uuid = bdm['instance_uuid'] bdm_real = db.block_device_mapping_get_all_by_instance(self.ctxt, uuid) - self.assertEqual(bdm_real[0]['virtual_name'], 'some_virt_name') + self.assertEqual(bdm_real[0]['destination_type'], 'moon') def test_block_device_mapping_update_or_create(self): values = { 'instance_uuid': self.instance['uuid'], 'device_name': 'fake_name', - 'virtual_name': 'some_virt_name' + 'source_type': 'volume', + 'destination_type': 'volume' } # check create - db.block_device_mapping_update_or_create(self.ctxt, values) + db.block_device_mapping_update_or_create(self.ctxt, values, + legacy=False) uuid = values['instance_uuid'] bdm_real = db.block_device_mapping_get_all_by_instance(self.ctxt, uuid) self.assertEqual(len(bdm_real), 1) self.assertEqual(bdm_real[0]['device_name'], 'fake_name') # check update - values['virtual_name'] = 'virtual_name' - db.block_device_mapping_update_or_create(self.ctxt, values) + values['destination_type'] = 'camelot' + db.block_device_mapping_update_or_create(self.ctxt, values, + legacy=False) bdm_real = db.block_device_mapping_get_all_by_instance(self.ctxt, uuid) self.assertEqual(len(bdm_real), 1) bdm_real = bdm_real[0] self.assertEqual(bdm_real['device_name'], 'fake_name') - self.assertEqual(bdm_real['virtual_name'], 'virtual_name') + self.assertEqual(bdm_real['destination_type'], 'camelot') def test_block_device_mapping_update_or_create_check_remove_virt(self): uuid = self.instance['uuid'] values = { 'instance_uuid': uuid, - 'virtual_name': 'ephemeral12' + 'source_type': 'blank', + 'guest_format': 'swap', } - # check that old bdm with same virtual_names are deleted on create + # check that old swap bdms are deleted on create val1 = dict(values) val1['device_name'] = 'device1' - db.block_device_mapping_create(self.ctxt, val1) + db.block_device_mapping_create(self.ctxt, val1, legacy=False) val2 = dict(values) val2['device_name'] = 'device2' - db.block_device_mapping_update_or_create(self.ctxt, val2) + db.block_device_mapping_update_or_create(self.ctxt, val2, legacy=False) bdm_real = db.block_device_mapping_get_all_by_instance(self.ctxt, uuid) self.assertEqual(len(bdm_real), 1) bdm_real = bdm_real[0] self.assertEqual(bdm_real['device_name'], 'device2') - self.assertEqual(bdm_real['virtual_name'], 'ephemeral12') + self.assertEqual(bdm_real['source_type'], 'blank') + self.assertEqual(bdm_real['guest_format'], 'swap') + db.block_device_mapping_destroy(self.ctxt, bdm_real['id']) - # check that old bdm with same virtual_names are deleted on update + # check that old ephemerals are deleted no matter what val3 = dict(values) val3['device_name'] = 'device3' - val3['virtual_name'] = 'some_name' - db.block_device_mapping_create(self.ctxt, val3) + val3['guest_format'] = None + val4 = dict(values) + val4['device_name'] = 'device4' + val4['guest_format'] = None + db.block_device_mapping_create(self.ctxt, val3, legacy=False) + db.block_device_mapping_create(self.ctxt, val4, legacy=False) bdm_real = db.block_device_mapping_get_all_by_instance(self.ctxt, uuid) self.assertEqual(len(bdm_real), 2) - val3['virtual_name'] = 'ephemeral12' - db.block_device_mapping_update_or_create(self.ctxt, val3) + val5 = dict(values) + val5['device_name'] = 'device5' + val5['guest_format'] = None + db.block_device_mapping_update_or_create(self.ctxt, val5, legacy=False) bdm_real = db.block_device_mapping_get_all_by_instance(self.ctxt, uuid) self.assertEqual(len(bdm_real), 1) bdm_real = bdm_real[0] - self.assertEqual(bdm_real['device_name'], 'device3') - self.assertEqual(bdm_real['virtual_name'], 'ephemeral12') + self.assertEqual(bdm_real['device_name'], 'device5') def test_block_device_mapping_get_all_by_instance(self): uuid1 = self.instance['uuid'] uuid2 = db.instance_create(self.ctxt, {})['uuid'] bmds_values = [{'instance_uuid': uuid1, - 'virtual_name': 'virtual_name', 'device_name': 'first'}, {'instance_uuid': uuid2, - 'virtual_name': 'virtual_name1', 'device_name': 'second'}, {'instance_uuid': uuid2, - 'virtual_name': 'virtual_name2', 'device_name': 'third'}] for bdm in bmds_values: @@ -4118,7 +4131,6 @@ class BlockDeviceMappingTestCase(test.TestCase): bmd = db.block_device_mapping_get_all_by_instance(self.ctxt, uuid1) self.assertEqual(len(bmd), 1) - self.assertEqual(bmd[0]['virtual_name'], 'virtual_name') self.assertEqual(bmd[0]['device_name'], 'first') bmd = db.block_device_mapping_get_all_by_instance(self.ctxt, uuid2) diff --git a/nova/tests/db/test_migrations.py b/nova/tests/db/test_migrations.py index 5bbf8d0c5..0e89cd521 100644 --- a/nova/tests/db/test_migrations.py +++ b/nova/tests/db/test_migrations.py @@ -1464,6 +1464,124 @@ class TestNovaMigrations(BaseMigrationTestCase, CommonTestsMixIn): def _post_downgrade_185(self, engine): self._unique_constraint_check_migrate_185(engine) + def _pre_upgrade_186(self, engine): + fake_instances = [ + dict(uuid='mig186_uuid-1', image_ref='fake_image_1', + root_device_name='/dev/vda'), + dict(uuid='mig186_uuid-2', image_ref='', + root_device_name='vda'), + dict(uuid='mig186_uuid-3', image_ref='fake_image_2', + root_device_name='/dev/vda'), + ] + + fake_bdms = [ + # Instance 1 - image, volume and swap + dict(instance_uuid='mig186_uuid-1', device_name='/dev/vdc', + volume_id='fake_volume_1'), + dict(instance_uuid='mig186_uuid-1', device_name='/dev/vdb', + virtual_name='swap'), + # Instance 2 - no image. snapshot and volume + dict(instance_uuid='mig186_uuid-2', device_name='/dev/vda', + snapshot_id='fake_snap_1', volume_id='fake_volume_2'), + dict(instance_uuid='mig186_uuid-2', device_name='/dev/vdc', + volume_id='fake_volume_3'), + # Instance 3 - ephemerals and swap + dict(instance_uuid='mig186_uuid-3', device_name='/dev/vdc', + virtual_name='ephemeral0'), + dict(instance_uuid='mig186_uuid-3', device_name='/dev/vdd', + virtual_name='ephemeral1'), + dict(instance_uuid='mig186_uuid-3', device_name='/dev/vdb', + virtual_name='swap'), + ] + + instances = db_utils.get_table(engine, 'instances') + block_device = db_utils.get_table(engine, 'block_device_mapping') + engine.execute(instances.insert(), fake_instances) + for fake_bdm in fake_bdms: + engine.execute(block_device.insert(), fake_bdm) + + return fake_instances, fake_bdms + + def _check_186(self, engine, data): + block_device = db_utils.get_table(engine, 'block_device_mapping') + + instance_qs = [] + + for instance in ('mig186_uuid-1', 'mig186_uuid-2', 'mig186_uuid-3'): + q = block_device.select().where( + block_device.c.instance_uuid == instance).order_by( + block_device.c.id.asc() + ) + instance_qs.append(q) + + bdm_1s, bdm_2s, bdm_3s = ( + [bdm for bdm in q.execute()] + for q in instance_qs + ) + + # Instance 1 + self.assertEqual(bdm_1s[0].source_type, 'volume') + self.assertEqual(bdm_1s[0].destination_type, 'volume') + self.assertEqual(bdm_1s[0].volume_id, 'fake_volume_1') + self.assertEqual(bdm_1s[0].device_type, 'disk') + self.assertEqual(bdm_1s[0].boot_index, -1) + self.assertEqual(bdm_1s[0].device_name, '/dev/vdc') + + self.assertEqual(bdm_1s[1].source_type, 'blank') + self.assertEqual(bdm_1s[1].guest_format, 'swap') + self.assertEqual(bdm_1s[1].destination_type, 'local') + self.assertEqual(bdm_1s[1].device_type, 'disk') + self.assertEqual(bdm_1s[1].boot_index, -1) + self.assertEqual(bdm_1s[1].device_name, '/dev/vdb') + + self.assertEqual(bdm_1s[2].source_type, 'image') + self.assertEqual(bdm_1s[2].destination_type, 'local') + self.assertEqual(bdm_1s[2].device_type, 'disk') + self.assertEqual(bdm_1s[2].image_id, 'fake_image_1') + self.assertEqual(bdm_1s[2].boot_index, 0) + + # Instance 2 + self.assertEqual(bdm_2s[0].source_type, 'snapshot') + self.assertEqual(bdm_2s[0].destination_type, 'volume') + self.assertEqual(bdm_2s[0].snapshot_id, 'fake_snap_1') + self.assertEqual(bdm_2s[0].volume_id, 'fake_volume_2') + self.assertEqual(bdm_2s[0].device_type, 'disk') + self.assertEqual(bdm_2s[0].boot_index, 0) + self.assertEqual(bdm_2s[0].device_name, '/dev/vda') + + self.assertEqual(bdm_2s[1].source_type, 'volume') + self.assertEqual(bdm_2s[1].destination_type, 'volume') + self.assertEqual(bdm_2s[1].volume_id, 'fake_volume_3') + self.assertEqual(bdm_2s[1].device_type, 'disk') + self.assertEqual(bdm_2s[1].boot_index, -1) + self.assertEqual(bdm_2s[1].device_name, '/dev/vdc') + + # Instance 3 + self.assertEqual(bdm_3s[0].source_type, 'blank') + self.assertEqual(bdm_3s[0].destination_type, 'local') + self.assertEqual(bdm_3s[0].device_type, 'disk') + self.assertEqual(bdm_3s[0].boot_index, -1) + self.assertEqual(bdm_3s[0].device_name, '/dev/vdc') + + self.assertEqual(bdm_3s[1].source_type, 'blank') + self.assertEqual(bdm_3s[1].destination_type, 'local') + self.assertEqual(bdm_3s[1].device_type, 'disk') + self.assertEqual(bdm_3s[1].boot_index, -1) + self.assertEqual(bdm_3s[1].device_name, '/dev/vdd') + + self.assertEqual(bdm_3s[2].source_type, 'blank') + self.assertEqual(bdm_3s[2].guest_format, 'swap') + self.assertEqual(bdm_3s[2].destination_type, 'local') + self.assertEqual(bdm_3s[2].device_type, 'disk') + self.assertEqual(bdm_3s[2].boot_index, -1) + self.assertEqual(bdm_3s[2].device_name, '/dev/vdb') + + self.assertEqual(bdm_3s[3].source_type, 'image') + self.assertEqual(bdm_3s[3].destination_type, 'local') + self.assertEqual(bdm_3s[3].device_type, 'disk') + self.assertEqual(bdm_3s[3].image_id, 'fake_image_2') + self.assertEqual(bdm_3s[3].boot_index, 0) + class TestBaremetalMigrations(BaseMigrationTestCase, CommonTestsMixIn): """Test sqlalchemy-migrate migrations.""" diff --git a/nova/tests/test_metadata.py b/nova/tests/test_metadata.py index 86d618930..6b84121c4 100644 --- a/nova/tests/test_metadata.py +++ b/nova/tests/test_metadata.py @@ -188,19 +188,24 @@ class MetadataTestCase(test.TestCase): return [{'volume_id': 87654321, 'snapshot_id': None, 'no_device': None, - 'virtual_name': None, + 'source_type': 'volume', + 'destination_type': 'volume', 'delete_on_termination': True, 'device_name': '/dev/sdh'}, {'volume_id': None, 'snapshot_id': None, 'no_device': None, - 'virtual_name': 'swap', + 'source_type': 'blank', + 'destination_type': 'local', + 'guest_format': 'swap', 'delete_on_termination': None, 'device_name': '/dev/sdc'}, {'volume_id': None, 'snapshot_id': None, 'no_device': None, - 'virtual_name': 'ephemeral0', + 'source_type': 'blank', + 'destination_type': 'local', + 'guest_format': None, 'delete_on_termination': None, 'device_name': '/dev/sdb'}] @@ -214,6 +219,7 @@ class MetadataTestCase(test.TestCase): 'ebs0': '/dev/sdh'} capi = conductor_api.LocalAPI() + self.assertEqual(base._format_instance_mapping(capi, ctxt, instance_ref0), block_device._DEFAULT_MAPPINGS) self.assertEqual(base._format_instance_mapping(capi, ctxt, -- cgit