diff --git a/nova/cmd/manage.py b/nova/cmd/manage.py index 48034ce9e0d9..e24103b61c8d 100644 --- a/nova/cmd/manage.py +++ b/nova/cmd/manage.py @@ -82,6 +82,7 @@ from nova.i18n import _ from nova import objects from nova.objects import flavor as flavor_obj from nova.objects import instance as instance_obj +from nova.objects import request_spec from nova import quota from nova import rpc from nova import utils @@ -726,6 +727,7 @@ class DbCommands(object): flavor_obj.migrate_flavors, flavor_obj.migrate_flavor_reset_autoincrement, instance_obj.migrate_instance_keypairs, + request_spec.migrate_instances_add_request_spec, ) def __init__(self): diff --git a/nova/objects/request_spec.py b/nova/objects/request_spec.py index 573d96c0a73d..8c5ffcd749cb 100644 --- a/nova/objects/request_spec.py +++ b/nova/objects/request_spec.py @@ -22,6 +22,7 @@ from nova import objects from nova.objects import base from nova.objects import fields from nova.objects import instance as obj_instance +from nova.scheduler import utils as scheduler_utils from nova.virt import hardware @@ -463,6 +464,90 @@ class RequestSpec(base.NovaObject): self.obj_reset_changes(['force_hosts', 'force_nodes']) +# NOTE(sbauza): Since verifying a huge list of instances can be a performance +# impact, we need to use a marker for only checking a set of them. +# As the current model doesn't expose a way to persist that marker, we propose +# here to use the request_specs table with a fake (and impossible) instance +# UUID where the related spec field (which is Text) would be the marker, ie. +# the last instance UUID we checked. +# TODO(sbauza): Remove the CRUD helpers and the migration script in Ocata. + +# NOTE(sbauza): RFC4122 (4.1.7) allows a Nil UUID to be semantically accepted. +FAKE_UUID = '00000000-0000-0000-0000-000000000000' + + +@db.api_context_manager.reader +def _get_marker_for_migrate_instances(context): + req_spec = (context.session.query(api_models.RequestSpec).filter_by( + instance_uuid=FAKE_UUID)).first() + marker = req_spec['spec'] if req_spec else None + return marker + + +@db.api_context_manager.writer +def _set_or_delete_marker_for_migrate_instances(context, marker=None): + # We need to delete the old marker anyway, which no longer corresponds to + # the last instance we checked (if there was a marker)... + # NOTE(sbauza): delete() deletes rows that match the query and if none + # are found, returns 0 hits. + context.session.query(api_models.RequestSpec).filter_by( + instance_uuid=FAKE_UUID).delete() + if marker is not None: + # ... but there can be a new marker to set + db_mapping = api_models.RequestSpec() + db_mapping.update({'instance_uuid': FAKE_UUID, 'spec': marker}) + db_mapping.save(context.session) + + +def _create_minimal_request_spec(context, instance): + image = instance.image_meta + # TODO(sbauza): Modify that once setup_instance_group() accepts a + # RequestSpec object + request_spec = {'instance_properties': {'uuid': instance.uuid}} + filter_properties = {} + scheduler_utils.setup_instance_group(context, request_spec, + filter_properties) + # This is an old instance. Let's try to populate a RequestSpec + # object using the existing information we have previously saved. + request_spec = objects.RequestSpec.from_components( + context, instance.uuid, image, + instance.flavor, instance.numa_topology, + instance.pci_requests, + filter_properties, None, instance.availability_zone + ) + request_spec.create() + + +def migrate_instances_add_request_spec(context, max_count): + """Creates and persists a RequestSpec per instance not yet having it.""" + marker = _get_marker_for_migrate_instances(context) + # Prevent lazy-load of those fields for every instance later. + attrs = ['system_metadata', 'flavor', 'pci_requests', 'numa_topology', + 'availability_zone'] + instances = objects.InstanceList.get_by_filters(context, + filters={'deleted': False}, + sort_key='created_at', + sort_dir='asc', + limit=max_count, + marker=marker, + expected_attrs=attrs) + count_all = len(instances) + count_hit = 0 + for instance in instances: + try: + RequestSpec.get_by_instance_uuid(context, instance.uuid) + except exception.RequestSpecNotFound: + _create_minimal_request_spec(context, instance) + count_hit += 1 + if count_all > 0: + # We want to persist which last instance was checked in order to make + # sure we don't review it again in a next call. + marker = instances[-1].uuid + + _set_or_delete_marker_for_migrate_instances(context, marker) + return count_all, count_hit + + @base.NovaObjectRegistry.register class SchedulerRetries(base.NovaObject): # Version 1.0: Initial version diff --git a/nova/tests/functional/db/test_request_spec.py b/nova/tests/functional/db/test_request_spec.py index 460570932908..c012172bf132 100644 --- a/nova/tests/functional/db/test_request_spec.py +++ b/nova/tests/functional/db/test_request_spec.py @@ -10,14 +10,23 @@ # License for the specific language governing permissions and limitations # under the License. +from oslo_config import cfg + from nova import context +from nova.db.sqlalchemy import api as db +from nova.db.sqlalchemy import api_models from nova import exception +from nova import objects from nova.objects import base as obj_base from nova.objects import request_spec from nova import test from nova.tests import fixtures +from nova.tests.functional import integrated_helpers +from nova.tests.unit import fake_network from nova.tests.unit import fake_request_spec +CONF = cfg.CONF + class RequestSpecTestCase(test.NoDBTestCase): USES_DB_SELF = True @@ -62,3 +71,84 @@ class RequestSpecTestCase(test.NoDBTestCase): def test_double_create(self): spec = self._create_spec() self.assertRaises(exception.ObjectActionError, spec.create) + + +@db.api_context_manager.writer +def _delete_request_spec(context, instance_uuid): + """Deletes a RequestSpec by the instance_uuid.""" + context.session.query(api_models.RequestSpec).filter_by( + instance_uuid=instance_uuid).delete() + + +class RequestSpecInstanceMigrationTestCase( + integrated_helpers._IntegratedTestBase): + api_major_version = 'v2.1' + _image_ref_parameter = 'imageRef' + _flavor_ref_parameter = 'flavorRef' + + def setUp(self): + super(RequestSpecInstanceMigrationTestCase, self).setUp() + + self.context = context.get_admin_context() + fake_network.set_stub_network_methods(self) + + def _create_instances(self, old=2, total=5): + request = self._build_minimal_create_server_request() + # Create all instances that would set a RequestSpec object + request.update({'max_count': total}) + self.api.post_server({'server': request}) + + self.instances = objects.InstanceList.get_all(self.context) + # Make sure that we have all the needed instances + self.assertEqual(total, len(self.instances)) + + # Fake the legacy behaviour by removing the RequestSpec for some old. + for i in range(0, old): + _delete_request_spec(self.context, self.instances[i].uuid) + + # Just add a deleted instance to make sure we don't create + # a RequestSpec object for it. + del request['max_count'] + server = self.api.post_server({'server': request}) + self.api.delete_server(server['id']) + # Make sure we have the deleted instance only soft-deleted in DB + deleted_instances = objects.InstanceList.get_by_filters( + self.context, filters={'deleted': True}) + self.assertEqual(1, len(deleted_instances)) + + def test_migration(self): + self._create_instances(old=2, total=5) + + match, done = request_spec.migrate_instances_add_request_spec( + self.context, 2) + self.assertEqual(2, match) + self.assertEqual(2, done) + + # Run again the migration call for making sure that we don't check + # again the same instances + match, done = request_spec.migrate_instances_add_request_spec( + self.context, 3) + self.assertEqual(3, match) + self.assertEqual(0, done) + + # Make sure we ran over all the instances + match, done = request_spec.migrate_instances_add_request_spec( + self.context, 50) + self.assertEqual(0, match) + self.assertEqual(0, done) + + # Make sure all instances have now a related RequestSpec + for uuid in [instance.uuid for instance in self.instances]: + try: + objects.RequestSpec.get_by_instance_uuid(self.context, uuid) + except exception.RequestSpecNotFound: + self.fail("RequestSpec not found for instance UUID :%s ", uuid) + + def test_migration_with_none_old(self): + self._create_instances(old=0, total=5) + + # Make sure no migrations can be found + match, done = request_spec.migrate_instances_add_request_spec( + self.context, 50) + self.assertEqual(5, match) + self.assertEqual(0, done)