diff --git a/nova/cmd/manage.py b/nova/cmd/manage.py index 6ecaf53edadb..2b3e56d0429f 100644 --- a/nova/cmd/manage.py +++ b/nova/cmd/manage.py @@ -980,24 +980,6 @@ class DbCommands(object): print(_('There were no records found where ' 'instance_uuid was NULL.')) - @args('--max-number', metavar='', dest='max_number', - help='Maximum number of instances to consider') - @args('--force', action='store_true', dest='force', - help='Force instances to migrate (even if they may be performing ' - 'another operation). Warning, this is potentially dangerous.') - def migrate_flavor_data(self, max_number=None, force=False): - if max_number is not None: - max_number = int(max_number) - if max_number < 0: - print(_('Must supply a positive value for max_number')) - return(1) - admin_context = context.get_admin_context() - flavor_cache = {} - match, done = db.migrate_flavor_data(admin_context, max_number, - flavor_cache, force) - print(_('%(total)i instances matched query, %(done)i completed') % - {'total': match, 'done': done}) - class ApiDbCommands(object): """Class for managing the api database.""" diff --git a/nova/db/api.py b/nova/db/api.py index e47b366ac85c..490ce632edf7 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -1907,21 +1907,6 @@ def archive_deleted_rows_for_table(context, tablename, max_rows=None): max_rows=max_rows) -def migrate_flavor_data(context, max_count, flavor_cache, force=False): - """Migrate instance flavor data from system_metadata to instance_extra. - - :param max_count: The maximum number of instances to consider in this - run. - :param flavor_cache: A dict to persist flavor information in across - calls (just pass an empty dict here) - :param force: Boolean whether or not to force migration of instances that - are performing another operation. - :returns: number of instances needing migration, number of instances - migrated (both will always be less than max_count) - """ - return IMPL.migrate_flavor_data(context, max_count, flavor_cache, force) - - #################### diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 152bb3e136f1..b75f5b7dde94 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -6019,127 +6019,6 @@ def archive_deleted_rows(context, max_rows=None): return rows_archived -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 full_flavor['extra_specs']: - if key not in flavor_to_migrate.extra_specs: - flavor_to_migrate.extra_specs[key] = \ - full_flavor.extra_specs[key] - - -def _augment_flavors_to_migrate(instance, flavor_cache): - """Add extra_specs to instance flavors. - - :param instance: Instance to be mined - :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: - try: - flavor_cache[flavorid] = objects.Flavor.get_by_flavor_id( - deleted_ctx, flavorid) - except exception.FlavorNotFound: - LOG.warn(_LW('Flavor %(flavorid)s not found for instance ' - 'during migration; extra_specs will not be ' - 'available'), - {'flavorid': flavorid}, instance=instance) - continue - _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, force=False): - # NOTE(danms): This is only ever run in nova-manage, and we need to avoid - # a circular import - from nova import objects - - query = _instance_get_all_query(context, joins=['extra', 'extra.flavor']).\ - outerjoin(models.Instance.extra).\ - filter(models.InstanceExtra.flavor.is_(None)) - if max_count is not None: - instances = query.limit(max_count) - else: - instances = query.all() - - instances = _instances_fill_metadata(context, instances, - manual_joins=['system_metadata']) - - count_all = 0 - count_hit = 0 - for db_instance in instances: - count_all += 1 - instance = objects.Instance._from_db_object( - context, objects.Instance(), db_instance, - expected_attrs=['system_metadata', 'flavor']) - # NOTE(danms): Don't touch instances that are likely in the - # middle of some other operation. This is just a guess and not - # a lock. There is still a race here, although it's the same - # race as the normal code, since we use expected_task_state below. - if not force: - if instance.task_state is not None: - continue - if instance.vm_state in [vm_states.RESCUED, vm_states.RESIZED]: - continue - - # 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, - {'instance_uuid': db_instance['uuid']}) - LOG.debug( - 'Created instance_extra for %s' % db_instance['uuid']) - instance.save(expected_task_state=[instance.task_state], - expected_vm_state=[instance.vm_state]) - count_hit += 1 - - return count_all, count_hit - - #################### diff --git a/nova/tests/unit/db/test_db_api.py b/nova/tests/unit/db/test_db_api.py index 18be49db352a..5e1e3a6d7a5e 100644 --- a/nova/tests/unit/db/test_db_api.py +++ b/nova/tests/unit/db/test_db_api.py @@ -48,8 +48,6 @@ from sqlalchemy import Table from nova import block_device from nova.compute import arch -from nova.compute import flavors -from nova.compute import task_states from nova.compute import vm_states from nova import context from nova import db @@ -59,7 +57,6 @@ from nova.db.sqlalchemy import types as col_types from nova.db.sqlalchemy import utils as db_utils from nova import exception from nova import objects -from nova.objects import base as obj_base from nova import quota from nova import test from nova.tests.unit import matchers @@ -7624,351 +7621,6 @@ class Ec2TestCase(test.TestCase): self.ctxt, 100500) -class FlavorMigrationTestCase(test.TestCase): - def test_augment_flavor_to_migrate_no_extra_specs(self): - flavor = objects.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(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'} - 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 - sqlalchemy_api._augment_flavors_to_migrate(instance, - {'foo': 'bar'}) - mock_augment.assert_called_once_with(instance.flavor, 'bar') - - @mock.patch('nova.db.sqlalchemy.api._augment_flavor_to_migrate') - @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()) - instance.flavor = objects.Flavor(flavorid='foo') - instance.old_flavor = objects.Flavor(flavorid='foo') - instance.new_flavor = objects.Flavor(flavorid='bar') - - flavor_cache = {'bar': 'bar_flavor'} - mock_get.return_value = 'foo_flavor' - - 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(mock.ANY, 'foo') - - def test_migrate_flavor(self): - ctxt = context.get_admin_context() - flavor = flavors.get_default_flavor() - sysmeta = flavors.save_flavor_info({}, flavor) - db.flavor_extra_specs_update_or_create(ctxt, flavor.flavorid, - {'new_spec': 'foo'}) - - values = {'uuid': str(stdlib_uuid.uuid4()), - 'system_metadata': sysmeta, - 'extra': {'flavor': 'foobar'}, - } - db.instance_create(ctxt, values) - - values = {'uuid': str(stdlib_uuid.uuid4()), - 'system_metadata': sysmeta, - 'extra': {'flavor': None}, - } - instance = db.instance_create(ctxt, values) - - match, done = db.migrate_flavor_data(ctxt, None, {}) - self.assertEqual(1, match) - self.assertEqual(1, done) - extra = db.instance_extra_get_by_instance_uuid(ctxt, instance['uuid'], - columns=['flavor']) - flavorinfo = jsonutils.loads(extra.flavor) - self.assertIsNone(flavorinfo['old']) - self.assertIsNone(flavorinfo['new']) - curflavor = obj_base.NovaObject.obj_from_primitive(flavorinfo['cur']) - self.assertEqual(flavor.flavorid, curflavor.flavorid) - - def test_migrate_flavor_honors_limit(self): - ctxt = context.get_admin_context() - flavor = flavors.get_default_flavor() - sysmeta = flavors.save_flavor_info({}, flavor) - db.flavor_extra_specs_update_or_create(ctxt, flavor.flavorid, - {'new_spec': 'foo'}) - - for i in (1, 2, 3, 4, 5): - values = {'uuid': str(stdlib_uuid.uuid4()), - 'system_metadata': sysmeta, - 'extra': {'flavor': 'foobar'}, - } - db.instance_create(ctxt, values) - - values = {'uuid': str(stdlib_uuid.uuid4()), - 'system_metadata': sysmeta, - 'extra': {'flavor': None}, - } - db.instance_create(ctxt, values) - - match, done = db.migrate_flavor_data(ctxt, 2, {}) - self.assertEqual(2, match) - self.assertEqual(2, done) - - match, done = db.migrate_flavor_data(ctxt, 1, {}) - self.assertEqual(1, match) - self.assertEqual(1, done) - - match, done = db.migrate_flavor_data(ctxt, None, {}) - self.assertEqual(2, match) - self.assertEqual(2, done) - - match, done = db.migrate_flavor_data(ctxt, None, {}) - self.assertEqual(0, match) - self.assertEqual(0, done) - - def _test_migrate_flavor_states(self, force=False): - ctxt = context.get_admin_context() - flavor = flavors.get_default_flavor() - sysmeta = flavors.save_flavor_info({}, flavor) - values = {'uuid': str(stdlib_uuid.uuid4()), - 'system_metadata': sysmeta, - 'extra': {'flavor': None}, - } - db.instance_create(ctxt, values) - values = {'uuid': str(stdlib_uuid.uuid4()), - 'task_state': task_states.SPAWNING, - 'system_metadata': sysmeta, - 'extra': {'flavor': None}, - } - db.instance_create(ctxt, values) - values = {'uuid': str(stdlib_uuid.uuid4()), - 'vm_state': vm_states.RESCUED, - 'system_metadata': sysmeta, - 'extra': {'flavor': None}, - } - db.instance_create(ctxt, values) - values = {'uuid': str(stdlib_uuid.uuid4()), - 'vm_state': vm_states.RESIZED, - 'system_metadata': sysmeta, - 'extra': {'flavor': None}, - } - db.instance_create(ctxt, values) - - match, done = db.migrate_flavor_data(ctxt, None, {}, force) - - if not force: - self.assertEqual(4, match) - self.assertEqual(1, done) - - match, done = db.migrate_flavor_data(ctxt, None, {}, force) - self.assertEqual(3, match) - self.assertEqual(0, done) - else: - self.assertEqual(4, match) - self.assertEqual(4, done) - - match, done = db.migrate_flavor_data(ctxt, None, {}, force) - self.assertEqual(0, match) - self.assertEqual(0, done) - - def test_migrate_flavor_honors_states(self): - self._test_migrate_flavor_states(force=False) - - def test_migrate_flavor_force_states(self): - self._test_migrate_flavor_states(force=True) - - def test_migrate_flavor_gets_missing_extra_rows(self): - ctxt = context.get_admin_context() - flavor = flavors.get_default_flavor() - sysmeta = flavors.save_flavor_info({}, flavor) - values1 = {'uuid': str(stdlib_uuid.uuid4()), - 'system_metadata': sysmeta, - 'extra': {'flavor': None}, - } - db.instance_create(ctxt, values1) - values2 = {'uuid': str(stdlib_uuid.uuid4()), - 'system_metadata': sysmeta, - } - inst2 = db.instance_create(ctxt, values2) - sqlalchemy_api.model_query(ctxt, models.InstanceExtra).\ - filter_by(instance_uuid=inst2.uuid).delete() - - self.assertIsNone(db.instance_extra_get_by_instance_uuid( - ctxt, inst2.uuid)) - match, done = db.migrate_flavor_data(ctxt, None, {}) - self.assertEqual(2, match) - self.assertEqual(2, done) - extra = db.instance_extra_get_by_instance_uuid(ctxt, inst2.uuid) - 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) - - @mock.patch('nova.objects.Flavor.get_by_flavor_id') - @mock.patch('nova.db.sqlalchemy.api._augment_flavor_to_migrate') - @mock.patch('nova.objects.Instance.save') - def test_migrate_flavor_with_missing_flavor(self, mock_save, - mock_augment, - mock_get_flavor): - ctxt = context.get_admin_context() - flavor = flavors.get_default_flavor() - sysmeta = flavors.save_flavor_info({}, flavor) - mock_get_flavor.side_effect = exception.FlavorNotFound(flavor_id=123) - values = {'uuid': str(stdlib_uuid.uuid4()), - 'instance_type_id': 123, - 'system_metadata': sysmeta, - } - db.instance_create(ctxt, values) - - flavor_cache = {} - db.migrate_flavor_data(ctxt, None, flavor_cache) - self.assertFalse(mock_augment.called) - self.assertTrue(mock_save.called) - self.assertEqual({}, flavor_cache) - - class ArchiveTestCase(test.TestCase): def setUp(self): diff --git a/nova/tests/unit/test_nova_manage.py b/nova/tests/unit/test_nova_manage.py index 1edbfa5c2182..9e08ef029ad1 100644 --- a/nova/tests/unit/test_nova_manage.py +++ b/nova/tests/unit/test_nova_manage.py @@ -413,9 +413,6 @@ class DBCommandsTestCase(test.TestCase): def test_null_instance_uuid_scan_delete(self): self._test_null_instance_uuid_scan(delete=True) - def test_migrate_flavor_data_negative(self): - self.assertEqual(1, self.commands.migrate_flavor_data(-1)) - @mock.patch.object(sqla_migration, 'db_version', return_value=2) def test_version(self, sqla_migrate): self.commands.version()