Make nova-manage handle completely missing flavor information
If you have a really (really) old instance without any flavor information
stashed in sysmeta, then nova-manage will not be able to convert it to
a proper flavor object in extra. This patch makes it handle that case
by looking up the flavor by id instead. Not only will this facilitate
the transition, but will also ensure that any such legacy instances are
properly brought up to date so that going forward we can just assume that
they have all been converted (and remove some other places where we
handle the has-no-flavor-info case).
This involves changing the flavor cache to contain flavor objects
instead of DB objects so that the two methods that use the cache can
coexist. It doesn't much affect the _augment_flavors_to_migrate() path,
other than some mechanics and test changes.
Conflicts:
nova/tests/unit/db/test_db_api.py
Change-Id: I54a056d339d98bc4092af8cf9f4f5d24b882506b
Closes-Bug: #1460673
(cherry picked from commit 240fb9c539)
This commit is contained in:
Dan Smith
parent
d203dd7f7f
commit
22adc62466
|
|
@ -6014,13 +6014,14 @@ def archive_deleted_rows(context, max_rows=None):
|
|||
return rows_archived
|
||||
|
||||
|
||||
def _augment_flavor_to_migrate(flavor_to_migrate, db_flavor):
|
||||
def _augment_flavor_to_migrate(flavor_to_migrate, full_flavor):
|
||||
"""Make sure that extra_specs on the flavor to migrate is updated."""
|
||||
if not flavor_to_migrate.obj_attr_is_set('extra_specs'):
|
||||
flavor_to_migrate.extra_specs = {}
|
||||
for key in db_flavor['extra_specs']:
|
||||
for key in full_flavor['extra_specs']:
|
||||
if key not in flavor_to_migrate.extra_specs:
|
||||
flavor_to_migrate.extra_specs[key] = db_flavor['extra_specs'][key]
|
||||
flavor_to_migrate.extra_specs[key] = \
|
||||
full_flavor.extra_specs[key]
|
||||
|
||||
|
||||
def _augment_flavors_to_migrate(instance, flavor_cache):
|
||||
|
|
@ -6030,17 +6031,40 @@ def _augment_flavors_to_migrate(instance, flavor_cache):
|
|||
:param flavor_cache: Dict to persist flavors we look up from the DB
|
||||
"""
|
||||
|
||||
# NOTE(danms): Avoid circular import
|
||||
from nova import objects
|
||||
|
||||
deleted_ctx = instance._context.elevated(read_deleted='yes')
|
||||
|
||||
for flavorprop in ['flavor', 'old_flavor', 'new_flavor']:
|
||||
flavor = getattr(instance, flavorprop)
|
||||
if flavor is None:
|
||||
continue
|
||||
flavorid = flavor.flavorid
|
||||
if flavorid not in flavor_cache:
|
||||
flavor_cache[flavorid] = flavor_get_by_flavor_id(
|
||||
instance._context, flavorid, 'yes')
|
||||
flavor_cache[flavorid] = objects.Flavor.get_by_flavor_id(
|
||||
deleted_ctx, flavorid)
|
||||
_augment_flavor_to_migrate(flavor, flavor_cache[flavorid])
|
||||
|
||||
|
||||
def _load_missing_flavor(instance, flavor_cache):
|
||||
# NOTE(danms): Avoid circular import
|
||||
from nova import objects
|
||||
|
||||
deleted_ctx = instance._context.elevated(read_deleted='yes')
|
||||
|
||||
flavor_cache_by_id = {flavor.id: flavor
|
||||
for flavor in flavor_cache.values()}
|
||||
if instance.instance_type_id in flavor_cache_by_id:
|
||||
instance.flavor = flavor_cache_by_id[instance.instance_type_id]
|
||||
else:
|
||||
instance.flavor = objects.Flavor.get_by_id(deleted_ctx,
|
||||
instance.instance_type_id)
|
||||
flavor_cache[instance.flavor.flavorid] = instance.flavor
|
||||
instance.old_flavor = None
|
||||
instance.new_flavor = None
|
||||
|
||||
|
||||
@require_admin_context
|
||||
def migrate_flavor_data(context, max_count, flavor_cache):
|
||||
# NOTE(danms): This is only ever run in nova-manage, and we need to avoid
|
||||
|
|
@ -6074,7 +6098,22 @@ def migrate_flavor_data(context, max_count, flavor_cache):
|
|||
if instance.vm_state in [vm_states.RESCUED, vm_states.RESIZED]:
|
||||
continue
|
||||
|
||||
_augment_flavors_to_migrate(instance, flavor_cache)
|
||||
# NOTE(danms): If we have a really old instance with no flavor
|
||||
# information at all, flavor will not have been set during load.
|
||||
# If that's the case, look up the flavor by id (which implies that
|
||||
# old_ and new_flavor are None). No need to augment with extra_specs
|
||||
# since we're doing the lookup from scratch.
|
||||
if not instance.obj_attr_is_set('flavor'):
|
||||
try:
|
||||
_load_missing_flavor(instance, flavor_cache)
|
||||
except exception.FlavorNotFound:
|
||||
LOG.error(_LE('Unable to lookup flavor for legacy instance; '
|
||||
'migration is not possible without manual '
|
||||
'intervention'),
|
||||
instance=instance)
|
||||
continue
|
||||
else:
|
||||
_augment_flavors_to_migrate(instance, flavor_cache)
|
||||
if instance.obj_what_changed():
|
||||
if db_instance.get('extra') is None:
|
||||
_instance_extra_create(context,
|
||||
|
|
|
|||
|
|
@ -7499,25 +7499,23 @@ class Ec2TestCase(test.TestCase):
|
|||
class FlavorMigrationTestCase(test.TestCase):
|
||||
def test_augment_flavor_to_migrate_no_extra_specs(self):
|
||||
flavor = objects.Flavor()
|
||||
db_flavor = {
|
||||
'extra_specs': {'foo': 'bar'}}
|
||||
sqlalchemy_api._augment_flavor_to_migrate(flavor, db_flavor)
|
||||
full_flavor = objects.Flavor(extra_specs={'foo': 'bar'})
|
||||
sqlalchemy_api._augment_flavor_to_migrate(flavor, full_flavor)
|
||||
self.assertTrue(flavor.obj_attr_is_set('extra_specs'))
|
||||
self.assertEqual(db_flavor['extra_specs'], flavor.extra_specs)
|
||||
self.assertEqual(full_flavor.extra_specs, flavor.extra_specs)
|
||||
|
||||
def test_augment_flavor_to_migrate_extra_specs_merge(self):
|
||||
flavor = objects.Flavor()
|
||||
flavor.extra_specs = {'foo': '1', 'bar': '2'}
|
||||
db_flavor = {
|
||||
'extra_specs': {'bar': '3', 'baz': '4'}
|
||||
}
|
||||
sqlalchemy_api._augment_flavor_to_migrate(flavor, db_flavor)
|
||||
full_flavor = objects.Flavor(extra_specs={'bar': '3', 'baz': '4'})
|
||||
sqlalchemy_api._augment_flavor_to_migrate(flavor, full_flavor)
|
||||
self.assertEqual({'foo': '1', 'bar': '2', 'baz': '4'},
|
||||
flavor.extra_specs)
|
||||
|
||||
@mock.patch('nova.db.sqlalchemy.api._augment_flavor_to_migrate')
|
||||
def test_augment_flavors_to_migrate(self, mock_augment):
|
||||
instance = objects.Instance()
|
||||
instance._context = context.get_admin_context()
|
||||
instance.flavor = objects.Flavor(flavorid='foo')
|
||||
instance.old_flavor = None
|
||||
instance.new_flavor = None
|
||||
|
|
@ -7526,7 +7524,7 @@ class FlavorMigrationTestCase(test.TestCase):
|
|||
mock_augment.assert_called_once_with(instance.flavor, 'bar')
|
||||
|
||||
@mock.patch('nova.db.sqlalchemy.api._augment_flavor_to_migrate')
|
||||
@mock.patch('nova.db.sqlalchemy.api.flavor_get_by_flavor_id')
|
||||
@mock.patch('nova.objects.Flavor.get_by_flavor_id')
|
||||
def test_augment_flavors_to_migrate_uses_cache(self, mock_get,
|
||||
mock_augment):
|
||||
instance = objects.Instance(context=context.get_admin_context())
|
||||
|
|
@ -7540,7 +7538,7 @@ class FlavorMigrationTestCase(test.TestCase):
|
|||
sqlalchemy_api._augment_flavors_to_migrate(instance, flavor_cache)
|
||||
self.assertIn('foo', flavor_cache)
|
||||
self.assertEqual('foo_flavor', flavor_cache['foo'])
|
||||
mock_get.assert_called_once_with(instance._context, 'foo', 'yes')
|
||||
mock_get.assert_called_once_with(mock.ANY, 'foo')
|
||||
|
||||
def test_migrate_flavor(self):
|
||||
ctxt = context.get_admin_context()
|
||||
|
|
@ -7669,6 +7667,142 @@ class FlavorMigrationTestCase(test.TestCase):
|
|||
self.assertIsNotNone(extra)
|
||||
self.assertIsNotNone(extra.flavor)
|
||||
|
||||
def test_migrate_flavor_with_deleted_things(self):
|
||||
ctxt = context.get_admin_context()
|
||||
flavor = flavors.get_default_flavor()
|
||||
sysmeta = flavors.save_flavor_info({}, flavor)
|
||||
|
||||
# Instance with some deleted metadata bits. We create
|
||||
# with a flavor, then change one of the keys so that
|
||||
# we end up with a soft-deleted row for that value.
|
||||
values = {'uuid': str(stdlib_uuid.uuid4()),
|
||||
'system_metadata': sysmeta,
|
||||
}
|
||||
inst1 = db.instance_create(ctxt, values)
|
||||
_sysmeta = dict(sysmeta, instance_type_id=123)
|
||||
db.instance_system_metadata_update(ctxt, inst1.uuid,
|
||||
_sysmeta, True)
|
||||
|
||||
# Deleted instance. Without a full flavor in sysmeta,
|
||||
# if this is hit in the migration, we'll explode trying
|
||||
# to construct the full flavor.
|
||||
values = {'uuid': str(stdlib_uuid.uuid4()),
|
||||
'system_metadata': sysmeta,
|
||||
}
|
||||
inst2 = db.instance_create(ctxt, values)
|
||||
inst2.soft_delete(session=None)
|
||||
|
||||
# Instance that has only deleted flavor metadata. This
|
||||
# looks like an instance that previously had flavor stuff
|
||||
# in sysmeta, but has since been converted. Since extra.flavor
|
||||
# is not a legitimate structure, we'll explode if the migration
|
||||
# code tries to hit this.
|
||||
values = {'uuid': str(stdlib_uuid.uuid4()),
|
||||
'system_metadata': {'instance_type_id': '123'},
|
||||
'extra': {'flavor': 'foobar'},
|
||||
}
|
||||
inst3 = db.instance_create(ctxt, values)
|
||||
db.instance_system_metadata_update(ctxt, inst3.uuid,
|
||||
{'foo': 'bar'},
|
||||
True)
|
||||
|
||||
match, done = db.migrate_flavor_data(ctxt, None, {})
|
||||
self.assertEqual(1, match)
|
||||
self.assertEqual(1, done)
|
||||
|
||||
@mock.patch('nova.objects.Flavor.get_by_id')
|
||||
def _test_migrate_flavor_with_no_sysmeta(self, flavor, inst,
|
||||
expected_count,
|
||||
mock_get_flavor):
|
||||
ctxt = context.get_admin_context()
|
||||
mock_get_flavor.return_value = flavor
|
||||
|
||||
flavor_cache = {}
|
||||
total, hit = db.migrate_flavor_data(ctxt, None, flavor_cache)
|
||||
self.assertEqual(expected_count, total)
|
||||
self.assertEqual(expected_count, hit)
|
||||
mock_get_flavor.assert_called_once_with(mock.ANY, 123)
|
||||
self.assertIn(flavor.flavorid, flavor_cache)
|
||||
|
||||
# NOTE(danms): Fetch the pieces to make sure the sans-sysmeta
|
||||
# instance is actually migrated in the database
|
||||
extra = db.instance_extra_get_by_instance_uuid(ctxt, inst['uuid'])
|
||||
self.assertIn('flavor', extra)
|
||||
extra_flavor = objects.Flavor.obj_from_primitive(
|
||||
jsonutils.loads(extra['flavor'])['cur'])
|
||||
self.assertEqual(flavor.id, extra_flavor.id)
|
||||
|
||||
sysmeta = db.instance_system_metadata_get(ctxt, inst['uuid'])
|
||||
self.assertNotIn('instance_type_id', sysmeta)
|
||||
|
||||
def test_migrate_flavor_with_no_sysmeta_first(self):
|
||||
flavor = flavors.get_default_flavor()
|
||||
sysmeta = flavors.save_flavor_info({}, flavor)
|
||||
ctxt = context.get_admin_context()
|
||||
|
||||
values = {'uuid': str(stdlib_uuid.uuid4()),
|
||||
'instance_type_id': 123,
|
||||
'system_metadata': sysmeta,
|
||||
}
|
||||
db.instance_create(ctxt, values)
|
||||
|
||||
values = {'uuid': str(stdlib_uuid.uuid4()),
|
||||
'instance_type_id': 123,
|
||||
'system_metadata': {},
|
||||
}
|
||||
bad_inst = db.instance_create(ctxt, values)
|
||||
|
||||
self._test_migrate_flavor_with_no_sysmeta(flavor, bad_inst, 2)
|
||||
|
||||
def test_migrate_flavor_with_no_sysmeta_last(self):
|
||||
flavor = flavors.get_default_flavor()
|
||||
sysmeta = flavors.save_flavor_info({}, flavor)
|
||||
ctxt = context.get_admin_context()
|
||||
|
||||
values = {'uuid': str(stdlib_uuid.uuid4()),
|
||||
'instance_type_id': 123,
|
||||
'system_metadata': {},
|
||||
}
|
||||
bad_inst = db.instance_create(ctxt, values)
|
||||
|
||||
values = {'uuid': str(stdlib_uuid.uuid4()),
|
||||
'instance_type_id': 123,
|
||||
'system_metadata': sysmeta,
|
||||
}
|
||||
db.instance_create(ctxt, values)
|
||||
|
||||
self._test_migrate_flavor_with_no_sysmeta(flavor, bad_inst, 2)
|
||||
|
||||
def test_migrate_flavor_with_no_sysmeta_alone(self):
|
||||
flavor = flavors.get_default_flavor()
|
||||
ctxt = context.get_admin_context()
|
||||
|
||||
values = {'uuid': str(stdlib_uuid.uuid4()),
|
||||
'instance_type_id': 123,
|
||||
'system_metadata': {},
|
||||
}
|
||||
bad_inst = db.instance_create(ctxt, values)
|
||||
|
||||
self._test_migrate_flavor_with_no_sysmeta(flavor, bad_inst, 1)
|
||||
|
||||
@mock.patch('nova.objects.Instance.save')
|
||||
@mock.patch('nova.objects.Flavor.get_by_id')
|
||||
def test_migrate_flavor_with_no_sysmeta_no_flavor(self, mock_get_flavor,
|
||||
mock_save):
|
||||
ctxt = context.get_admin_context()
|
||||
mock_get_flavor.side_effect = exception.FlavorNotFound(flavor_id=123)
|
||||
values = {'uuid': str(stdlib_uuid.uuid4()),
|
||||
'instance_type_id': 123,
|
||||
'system_metadata': {},
|
||||
}
|
||||
db.instance_create(ctxt, values)
|
||||
|
||||
flavor_cache = {}
|
||||
db.migrate_flavor_data(ctxt, None, flavor_cache)
|
||||
mock_get_flavor.assert_called_once_with(mock.ANY, 123)
|
||||
self.assertEqual({}, flavor_cache)
|
||||
self.assertFalse(mock_save.called)
|
||||
|
||||
|
||||
class ArchiveTestCase(test.TestCase):
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue