openstack/nova
Code Issues Proposed changes

Add uuid column to BlockDeviceMapping

Browse Source 
This adds a uuid column to the block_device_mapping table,
and makes it semi-optional for graceful upgrade of existing rows
without a uuid.

Part of bp local-disk-serial-numbers

Co-Authored-By: Lee Yarwood <lyarwood@redhat.com>
Co-Authored-By: Matthew Booth <mbooth@redhat.com>
Partial-Bug: #1489581
Change-Id: Ibf0db6ad5b8367fc3267ac309516c08547d47e8c
This commit is contained in:
Dan Smith 2015-11-06 11:01:19 -08:00 committed by Matthew Booth
parent 0fe71a9a7d
commit cd3901c067
6 changed files with 147 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
nova/block_device.py
View File

@ -48,7 +48,7 @@ bdm_new_fields = set(['source_type', 'destination_type',
'connection_info', 'tag'])
bdm_db_only_fields = set(['id', 'instance_uuid', 'attachment_id'])
bdm_db_only_fields = set(['id', 'instance_uuid', 'attachment_id', 'uuid'])
bdm_db_inherited_fields = set(['created_at', 'updated_at',

37
nova/db/sqlalchemy/api.py
View File

@ -3708,6 +3708,19 @@ def _from_legacy_values(values, legacy, allow_updates=False):
return values
def _set_or_validate_uuid(values):
uuid = values.get('uuid')
# values doesn't contain uuid, or it's blank
if not uuid:
values['uuid'] = uuidutils.generate_uuid()
# values contains a uuid
else:
if not uuidutils.is_uuid_like(uuid):
raise exception.InvalidUUID(uuid=uuid)
@require_context
@pick_context_manager_writer
def block_device_mapping_create(context, values, legacy=True):
@ -3715,6 +3728,8 @@ def block_device_mapping_create(context, values, legacy=True):
values = _from_legacy_values(values, legacy)
convert_objects_related_datetimes(values)
_set_or_validate_uuid(values)
bdm_ref = models.BlockDeviceMapping()
bdm_ref.update(values)
bdm_ref.save(context.session)
@ -3735,14 +3750,25 @@ def block_device_mapping_update(context, bdm_id, values, legacy=True):
@pick_context_manager_writer
def block_device_mapping_update_or_create(context, values, legacy=True):
# TODO(mdbooth): Remove this method entirely. Callers should know whether
# they require update or create, and call the appropriate method.
_scrub_empty_str_values(values, ['volume_size'])
values = _from_legacy_values(values, legacy, allow_updates=True)
convert_objects_related_datetimes(values)
result = None
# NOTE(xqueralt): Only update a BDM when device_name was provided. We
# allow empty device names so they will be set later by the manager.
if values['device_name']:
# NOTE(xqueralt,danms): Only update a BDM when device_name or
# uuid was provided. Prefer the uuid, if available, but fall
# back to device_name if no uuid is provided, which can happen
# for BDMs created before we had a uuid. We allow empty device
# names so they will be set later by the manager.
if 'uuid' in values:
query = _block_device_mapping_get_query(context)
result = query.filter_by(instance_uuid=values['instance_uuid'],
uuid=values['uuid']).one_or_none()
if not result and values['device_name']:
query = _block_device_mapping_get_query(context)
result = query.filter_by(instance_uuid=values['instance_uuid'],
device_name=values['device_name']).first()
@ -3750,8 +3776,9 @@ def block_device_mapping_update_or_create(context, values, legacy=True):
if result:
result.update(values)
else:
# Either the device_name doesn't exist in the database yet, or no
# device_name was provided. Both cases mean creating a new BDM.
# Either the device_name or uuid doesn't exist in the database yet, or
# neither was provided. Both cases mean creating a new BDM.
_set_or_validate_uuid(values)
result = models.BlockDeviceMapping(**values)
result.save(context.session)

35
nova/db/sqlalchemy/migrate_repo/versions/374_bdm_uuid.py Normal file
View File

@ -0,0 +1,35 @@
# 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.
from migrate import UniqueConstraint
from sqlalchemy import Column
from sqlalchemy import MetaData
from sqlalchemy import String
from sqlalchemy import Table
def upgrade(migrate_engine):
meta = MetaData()
meta.bind = migrate_engine
for prefix in ('', 'shadow_'):
table = Table(prefix + 'block_device_mapping', meta, autoload=True)
if not hasattr(table.c, 'uuid'):
new_column = Column('uuid', String(36), nullable=True)
table.create_column(new_column)
if prefix == '':
# Only add the constraint on the non-shadow table...
con = UniqueConstraint('uuid', table=table,
name="uniq_block_device_mapping0uuid")
con.create(migrate_engine)

8
nova/db/sqlalchemy/models.py
View File

@ -583,10 +583,18 @@ class BlockDeviceMapping(BASE, NovaBase, models.SoftDeleteMixin):
Index('block_device_mapping_instance_uuid_volume_id_idx',
'instance_uuid', 'volume_id'),
Index('block_device_mapping_instance_uuid_idx', 'instance_uuid'),
schema.UniqueConstraint('uuid', name='uniq_block_device_mapping0uuid'),
)
id = Column(Integer, primary_key=True, autoincrement=True)
instance_uuid = Column(String(36), ForeignKey('instances.uuid'))
# NOTE(mdbooth): The REST API for BDMs includes a UUID field. That uuid
# refers to an image, volume, or snapshot which will be used in the
# initialisation of the BDM. It is only relevant during the API call, and
# is not persisted directly. This is the UUID of the BDM itself.
# FIXME(danms): This should eventually be non-nullable, but we need a
# transition period first.
uuid = Column(String(36))
instance = orm.relationship(Instance,
backref=orm.backref('block_device_mapping'),
foreign_keys=instance_uuid,

67
nova/tests/unit/db/test_db_api.py
View File

@ -6427,6 +6427,16 @@ class BlockDeviceMappingTestCase(test.TestCase):
def test_block_device_mapping_create(self):
bdm = self._create_bdm({})
self.assertIsNotNone(bdm)
self.assertTrue(uuidutils.is_uuid_like(bdm['uuid']))
def test_block_device_mapping_create_with_blank_uuid(self):
bdm = self._create_bdm({'uuid': ''})
self.assertIsNotNone(bdm)
self.assertTrue(uuidutils.is_uuid_like(bdm['uuid']))
def test_block_device_mapping_create_with_invalid_uuid(self):
self.assertRaises(exception.InvalidUUID,
self._create_bdm, {'uuid': 'invalid-uuid'})
def test_block_device_mapping_create_with_attachment_id(self):
bdm = self._create_bdm({'attachment_id': uuidsentinel.attachment_id})
@ -6456,16 +6466,19 @@ class BlockDeviceMappingTestCase(test.TestCase):
'destination_type': 'volume'
}
# check create
db.block_device_mapping_update_or_create(self.ctxt, values,
legacy=False)
bdm = db.block_device_mapping_update_or_create(self.ctxt,
copy.deepcopy(values),
legacy=False)
self.assertTrue(uuidutils.is_uuid_like(bdm['uuid']))
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['destination_type'] = 'camelot'
db.block_device_mapping_update_or_create(self.ctxt, values,
bdm0 = copy.deepcopy(values)
bdm0['destination_type'] = 'camelot'
db.block_device_mapping_update_or_create(self.ctxt, bdm0,
legacy=False)
bdm_real = db.block_device_mapping_get_all_by_instance(self.ctxt, uuid)
self.assertEqual(len(bdm_real), 1)
@ -6474,12 +6487,13 @@ class BlockDeviceMappingTestCase(test.TestCase):
self.assertEqual(bdm_real['destination_type'], 'camelot')
# check create without device_name
bdm1 = dict(values)
bdm1 = copy.deepcopy(values)
bdm1['device_name'] = None
db.block_device_mapping_update_or_create(self.ctxt, bdm1, legacy=False)
bdms = db.block_device_mapping_get_all_by_instance(self.ctxt, uuid)
with_device_name = [b for b in bdms if b['device_name'] is not None]
without_device_name = [b for b in bdms if b['device_name'] is None]
self.assertEqual(2, len(bdms))
self.assertEqual(len(with_device_name), 1,
'expected 1 bdm with device_name, found %d' %
len(with_device_name))
@ -6501,6 +6515,49 @@ class BlockDeviceMappingTestCase(test.TestCase):
'expected 2 bdms without device_name, found %d' %
len(without_device_name))
def test_block_device_mapping_update_or_create_with_uuid(self):
# Test that we are able to change device_name when calling
# block_device_mapping_update_or_create with a uuid.
bdm = self._create_bdm({})
values = {
'uuid': bdm['uuid'],
'instance_uuid': bdm['instance_uuid'],
'device_name': 'foobar',
}
db.block_device_mapping_update_or_create(self.ctxt, values,
legacy=False)
real_bdms = db.block_device_mapping_get_all_by_instance(
self.ctxt, bdm['instance_uuid'])
self.assertEqual(1, len(real_bdms))
self.assertEqual('foobar', real_bdms[0]['device_name'])
def test_block_device_mapping_update_or_create_with_blank_uuid(self):
# Test that create with block_device_mapping_update_or_create raises an
# exception if given an invalid uuid
values = {
'uuid': '',
'instance_uuid': uuidsentinel.instance,
'device_name': 'foobar',
}
db.block_device_mapping_update_or_create(self.ctxt, values)
real_bdms = db.block_device_mapping_get_all_by_instance(
self.ctxt, uuidsentinel.instance)
self.assertEqual(1, len(real_bdms))
self.assertTrue(uuidutils.is_uuid_like(real_bdms[0]['uuid']))
def test_block_device_mapping_update_or_create_with_invalid_uuid(self):
# Test that create with block_device_mapping_update_or_create raises an
# exception if given an invalid uuid
values = {
'uuid': 'invalid-uuid',
'instance_uuid': uuidsentinel.instance,
'device_name': 'foobar',
}
self.assertRaises(exception.InvalidUUID,
db.block_device_mapping_update_or_create,
self.ctxt, values)
def test_block_device_mapping_update_or_create_multiple_ephemeral(self):
uuid = self.instance['uuid']
values = {

9
nova/tests/unit/db/test_migrations.py
View File

@ -968,6 +968,15 @@ class NovaMigrationsCheckers(test_migrations.ModelsMigrationsSync,
def _check_373(self, engine, data):
self.assertColumnExists(engine, 'migrations', 'uuid')
def _check_374(self, engine, data):
self.assertColumnExists(engine, 'block_device_mapping', 'uuid')
self.assertColumnExists(engine, 'shadow_block_device_mapping', 'uuid')
inspector = reflection.Inspector.from_engine(engine)
constraints = inspector.get_unique_constraints('block_device_mapping')
constraint_names = [constraint['name'] for constraint in constraints]
self.assertIn('uniq_block_device_mapping0uuid', constraint_names)
class TestNovaMigrationsSQLite(NovaMigrationsCheckers,
test_base.DbTestCase,