Make BlockDeviceMapping object support 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: I3b25debb0bcfd4e211734307c8d363f2b5dbc655
This commit is contained in:
parent
cd3901c067
commit
8df136dab5
|
@ -12,13 +12,18 @@
|
|||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
from oslo_db import api as oslo_db_api
|
||||
from oslo_db.sqlalchemy import update_match
|
||||
from oslo_log import log as logging
|
||||
from oslo_utils import uuidutils
|
||||
from oslo_utils import versionutils
|
||||
|
||||
from nova import block_device
|
||||
from nova.cells import opts as cells_opts
|
||||
from nova.cells import rpcapi as cells_rpcapi
|
||||
from nova import db
|
||||
from nova.db.sqlalchemy import api as db_api
|
||||
from nova.db.sqlalchemy import models as db_models
|
||||
from nova import exception
|
||||
from nova.i18n import _
|
||||
from nova import objects
|
||||
|
@ -63,10 +68,12 @@ class BlockDeviceMapping(base.NovaPersistentObject, base.NovaObject,
|
|||
# get_by_volume() and get_by_volume_and_instance()
|
||||
# Version 1.17: Added tag field
|
||||
# Version 1.18: Added attachment_id
|
||||
VERSION = '1.18'
|
||||
# Version 1.19: Added uuid
|
||||
VERSION = '1.19'
|
||||
|
||||
fields = {
|
||||
'id': fields.IntegerField(),
|
||||
'uuid': fields.UUIDField(),
|
||||
'instance_uuid': fields.UUIDField(),
|
||||
'instance': fields.ObjectField('Instance', nullable=True),
|
||||
'source_type': fields.BlockDeviceSourceTypeField(nullable=True),
|
||||
|
@ -90,19 +97,64 @@ class BlockDeviceMapping(base.NovaPersistentObject, base.NovaObject,
|
|||
|
||||
def obj_make_compatible(self, primitive, target_version):
|
||||
target_version = versionutils.convert_version_to_tuple(target_version)
|
||||
if target_version < (1, 19) and 'uuid' in primitive:
|
||||
del primitive['uuid']
|
||||
if target_version < (1, 18) and 'attachment_id' in primitive:
|
||||
del primitive['attachment_id']
|
||||
if target_version < (1, 17) and 'tag' in primitive:
|
||||
del primitive['tag']
|
||||
|
||||
@staticmethod
|
||||
def _from_db_object(context, block_device_obj,
|
||||
@oslo_db_api.wrap_db_retry(max_retries=1, retry_on_deadlock=True)
|
||||
def _create_uuid(context, bdm_id):
|
||||
# NOTE(mdbooth): This method is only required until uuid is made
|
||||
# non-nullable in a future release.
|
||||
|
||||
# NOTE(mdbooth): We wrap this method in a retry loop because it can
|
||||
# fail (safely) on multi-master galera if concurrent updates happen on
|
||||
# different masters. It will never fail on single-master. We can only
|
||||
# ever need one retry.
|
||||
|
||||
uuid = uuidutils.generate_uuid()
|
||||
values = {'uuid': uuid}
|
||||
compare = db_models.BlockDeviceMapping(id=bdm_id, uuid=None)
|
||||
|
||||
# NOTE(mdbooth): We explicitly use an independent transaction context
|
||||
# here so as not to fail if:
|
||||
# 1. We retry.
|
||||
# 2. We're in a read transaction.This is an edge case of what's
|
||||
# normally a read operation. Forcing everything (transitively)
|
||||
# which reads a BDM to be in a write transaction for a narrow
|
||||
# temporary edge case is undesirable.
|
||||
tctxt = db_api.get_context_manager(context).writer.independent
|
||||
with tctxt.using(context):
|
||||
query = context.session.query(db_models.BlockDeviceMapping).\
|
||||
filter_by(id=bdm_id)
|
||||
|
||||
try:
|
||||
query.update_on_match(compare, 'id', values)
|
||||
except update_match.NoRowsMatched:
|
||||
# We can only get here if we raced, and another writer already
|
||||
# gave this bdm a uuid
|
||||
result = query.one()
|
||||
uuid = result['uuid']
|
||||
assert(uuid is not None)
|
||||
|
||||
return uuid
|
||||
|
||||
@classmethod
|
||||
def _from_db_object(cls, context, block_device_obj,
|
||||
db_block_device, expected_attrs=None):
|
||||
if expected_attrs is None:
|
||||
expected_attrs = []
|
||||
for key in block_device_obj.fields:
|
||||
if key in BLOCK_DEVICE_OPTIONAL_ATTRS:
|
||||
continue
|
||||
if key == 'uuid' and not db_block_device.get(key):
|
||||
# NOTE(danms): While the records could be nullable,
|
||||
# generate a UUID on read since the object requires it
|
||||
bdm_id = db_block_device['id']
|
||||
db_block_device[key] = cls._create_uuid(context, bdm_id)
|
||||
block_device_obj[key] = db_block_device[key]
|
||||
if 'instance' in expected_attrs:
|
||||
my_inst = objects.Instance(context)
|
||||
|
|
|
@ -39,6 +39,7 @@ class FakeDbBlockDeviceDict(block_device.BlockDeviceDict):
|
|||
def __init__(self, bdm_dict=None, anon=False, **kwargs):
|
||||
bdm_dict = bdm_dict or {}
|
||||
db_id = bdm_dict.pop('id', 1)
|
||||
db_uuid = bdm_dict.pop('uuid', uuids.bdm)
|
||||
instance_uuid = bdm_dict.pop('instance_uuid', uuids.fake)
|
||||
|
||||
super(FakeDbBlockDeviceDict, self).__init__(bdm_dict=bdm_dict,
|
||||
|
@ -48,6 +49,7 @@ class FakeDbBlockDeviceDict(block_device.BlockDeviceDict):
|
|||
'deleted': 0}
|
||||
if not anon:
|
||||
fake_db_fields['id'] = db_id
|
||||
fake_db_fields['uuid'] = db_uuid
|
||||
fake_db_fields['attachment_id'] = None
|
||||
fake_db_fields['created_at'] = timeutils.utcnow()
|
||||
fake_db_fields['updated_at'] = timeutils.utcnow()
|
||||
|
|
|
@ -17,6 +17,8 @@ import mock
|
|||
from nova.cells import rpcapi as cells_rpcapi
|
||||
from nova import context
|
||||
from nova import db
|
||||
from nova.db.sqlalchemy import api as db_api
|
||||
from nova.db.sqlalchemy import models as db_models
|
||||
from nova import exception
|
||||
from nova import objects
|
||||
from nova.objects import block_device as block_device_obj
|
||||
|
@ -32,6 +34,7 @@ class _TestBlockDeviceMappingObject(object):
|
|||
instance = instance or {}
|
||||
fake_bdm = fake_block_device.FakeDbBlockDeviceDict({
|
||||
'id': 123,
|
||||
'uuid': uuids.bdm,
|
||||
'instance_uuid': instance.get('uuid') or uuids.instance,
|
||||
'attachment_id': None,
|
||||
'device_name': '/dev/sda2',
|
||||
|
@ -406,6 +409,80 @@ class _TestBlockDeviceMappingObject(object):
|
|||
primitive = bdm.obj_to_primitive(target_version='1.17')
|
||||
self.assertNotIn('attachment_id', primitive)
|
||||
|
||||
def test_obj_make_compatible_pre_1_19(self):
|
||||
values = {'source_type': 'volume', 'volume_id': 'fake-vol-id',
|
||||
'destination_type': 'volume',
|
||||
'instance_uuid': uuids.instance, 'uuid': uuids.bdm}
|
||||
bdm = objects.BlockDeviceMapping(context=self.context, **values)
|
||||
primitive = bdm.obj_to_primitive(target_version='1.18')
|
||||
self.assertNotIn('uuid', primitive)
|
||||
|
||||
|
||||
class TestBlockDeviceMappingUUIDMigration(test.TestCase):
|
||||
def setUp(self):
|
||||
super(TestBlockDeviceMappingUUIDMigration, self).setUp()
|
||||
self.context = context.RequestContext('fake-user-id',
|
||||
'fake-project-id')
|
||||
|
||||
self.orig_create_uuid = \
|
||||
objects.BlockDeviceMapping._create_uuid
|
||||
|
||||
@staticmethod
|
||||
@db_api.pick_context_manager_writer
|
||||
def _create_legacy_bdm(context):
|
||||
# Create a BDM with no uuid
|
||||
values = {'instance_uuid': uuids.instance_uuid}
|
||||
bdm_ref = db_models.BlockDeviceMapping()
|
||||
bdm_ref.update(values)
|
||||
bdm_ref.save(context.session)
|
||||
return bdm_ref
|
||||
|
||||
@mock.patch.object(objects.BlockDeviceMapping, '_create_uuid')
|
||||
def test_populate_uuid(self, mock_create_uuid):
|
||||
mock_create_uuid.side_effect = self.orig_create_uuid
|
||||
|
||||
self._create_legacy_bdm(self.context)
|
||||
bdms = objects.BlockDeviceMappingList.get_by_instance_uuid(
|
||||
self.context, uuids.instance_uuid)
|
||||
|
||||
# UUID should have been populated
|
||||
uuid = bdms[0].uuid
|
||||
self.assertIsNotNone(uuid)
|
||||
|
||||
bdms = objects.BlockDeviceMappingList.get_by_instance_uuid(
|
||||
self.context, uuids.instance_uuid)
|
||||
|
||||
# UUID should not have changed
|
||||
self.assertEqual(uuid, bdms[0].uuid)
|
||||
self.assertEqual(1, mock_create_uuid.call_count)
|
||||
|
||||
def test_create_uuid_race(self):
|
||||
# If threads read a legacy BDM object concurrently, we can end up
|
||||
# calling _create_uuid multiple times. Assert that calling _create_uuid
|
||||
# multiple times yields the same uuid.
|
||||
|
||||
# NOTE(mdbooth): _create_uuid handles all forms of race, including any
|
||||
# amount of overlapping. I have not attempted to write unit tests for
|
||||
# all potential execution orders. This test is sufficient to
|
||||
# demonstrate that the compare-and-swap works correctly, and we trust
|
||||
# the correctness of the database for the rest.
|
||||
|
||||
db_bdm = self._create_legacy_bdm(self.context)
|
||||
uuid1 = objects.BlockDeviceMapping._create_uuid(self.context,
|
||||
db_bdm['id'])
|
||||
|
||||
bdm = objects.BlockDeviceMappingList.get_by_instance_uuid(
|
||||
self.context, uuids.instance_uuid)[0]
|
||||
self.assertEqual(uuid1, bdm.uuid)
|
||||
|
||||
# We would only ever call this twice if we raced
|
||||
# This is also testing that the compare-and-swap doesn't overwrite an
|
||||
# existing uuid if we hit that race.
|
||||
uuid2 = objects.BlockDeviceMapping._create_uuid(self.context,
|
||||
bdm['id'])
|
||||
|
||||
self.assertEqual(uuid1, uuid2)
|
||||
|
||||
|
||||
class TestBlockDeviceMappingObject(test_objects._LocalTest,
|
||||
_TestBlockDeviceMappingObject):
|
||||
|
|
|
@ -1064,7 +1064,7 @@ object_data = {
|
|||
'AggregateList': '1.2-fb6e19f3c3a3186b04eceb98b5dadbfa',
|
||||
'BandwidthUsage': '1.2-c6e4c779c7f40f2407e3d70022e3cd1c',
|
||||
'BandwidthUsageList': '1.2-5fe7475ada6fe62413cbfcc06ec70746',
|
||||
'BlockDeviceMapping': '1.18-ad87cece6f84c65f5ec21615755bc6d3',
|
||||
'BlockDeviceMapping': '1.19-407e75274f48e60a76e56283333c9dbc',
|
||||
'BlockDeviceMappingList': '1.17-1e568eecb91d06d4112db9fd656de235',
|
||||
'BuildRequest': '1.3-077dee42bed93f8a5b62be77657b7152',
|
||||
'BuildRequestList': '1.0-cd95608eccb89fbc702c8b52f38ec738',
|
||||
|
|
Loading…
Reference in New Issue