From 90b96170d3f269165f649e8b61739cf31ffb78b8 Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Wed, 16 May 2018 11:57:32 +0200 Subject: [PATCH] Add requested_resources field to RequestSpec The new RequestSpec.resource_requests stores a list of RequestGroup objects each representing a granular resource request group requesting resource for the given instance. This will be used in a later patch to communicate the Neutron port's resource request to the scheduler. Also this can be used later to refactor flavor-based resource request handling. This new field is intentionally not persisted to the DB for two reasons: * the port's resource request is already persisted in neutron so this would be a data duplication that could lead to data inconsistencies * this field is only used to carry information from the the nova-conductor to the nova-scheduler for each select_destinations() request blueprint: bandwidth-resource-provider Change-Id: I53e5debcffd6de2b3a2ff838e7f5da33fa1a82b8 --- nova/objects/request_spec.py | 34 +++++++-- nova/tests/unit/objects/test_objects.py | 2 +- nova/tests/unit/objects/test_request_spec.py | 74 ++++++++++++++++++++ 3 files changed, 105 insertions(+), 5 deletions(-) diff --git a/nova/objects/request_spec.py b/nova/objects/request_spec.py index eb3be91f95a7..64f906026e60 100644 --- a/nova/objects/request_spec.py +++ b/nova/objects/request_spec.py @@ -28,7 +28,8 @@ from nova.virt import hardware REQUEST_SPEC_OPTIONAL_ATTRS = ['requested_destination', 'security_groups', - 'network_metadata'] + 'network_metadata', + 'requested_resources'] @base.NovaObjectRegistry.register @@ -45,7 +46,8 @@ class RequestSpec(base.NovaObject): # Version 1.9: Added user_id # Version 1.10: Added network_metadata # Version 1.11: Added is_bfv - VERSION = '1.11' + # Version 1.12: Added requested_resources + VERSION = '1.12' fields = { 'id': fields.IntegerField(), @@ -84,11 +86,17 @@ class RequestSpec(base.NovaObject): 'security_groups': fields.ObjectField('SecurityGroupList'), 'network_metadata': fields.ObjectField('NetworkMetadata'), 'is_bfv': fields.BooleanField(), + 'requested_resources': fields.ListOfObjectsField('RequestGroup', + nullable=True, + default=None) } def obj_make_compatible(self, primitive, target_version): super(RequestSpec, self).obj_make_compatible(primitive, target_version) target_version = versionutils.convert_version_to_tuple(target_version) + if target_version < (1, 12): + if 'requested_resources' in primitive: + del primitive['requested_resources'] if target_version < (1, 11) and 'is_bfv' in primitive: del primitive['is_bfv'] if target_version < (1, 10): @@ -410,7 +418,7 @@ class RequestSpec(base.NovaObject): def from_components(cls, context, instance_uuid, image, flavor, numa_topology, pci_requests, filter_properties, instance_group, availability_zone, security_groups=None, project_id=None, - user_id=None): + user_id=None, port_resource_requests=None): """Returns a new RequestSpec object hydrated by various components. This helper is useful in creating the RequestSpec from the various @@ -433,6 +441,9 @@ class RequestSpec(base.NovaObject): the instance project_id). :param user_id: The user_id for the requestspec (should match the instance user_id). + :param port_resource_requests: a list of RequestGroup objects + representing the resource needs of the + neutron ports """ spec_obj = cls(context) spec_obj.num_instances = 1 @@ -458,6 +469,13 @@ class RequestSpec(base.NovaObject): spec_obj.requested_destination = filter_properties.get( 'requested_destination') + # TODO(gibi): do the creation of the unnumbered group and any + # numbered group from the flavor by moving the logic from + # nova.scheduler.utils.resources_from_request_spec() here. + spec_obj.requested_resources = [] + if port_resource_requests: + spec_obj.requested_resources.extend(port_resource_requests) + # NOTE(sbauza): Default the other fields that are not part of the # original contract spec_obj.obj_set_defaults() @@ -497,6 +515,13 @@ class RequestSpec(base.NovaObject): # though they should match. if key in ['id', 'instance_uuid']: setattr(spec, key, db_spec[key]) + elif key == 'requested_resources': + # Do not override what we already have in the object as this + # field is not persisted. If save() is called after + # requested_resources is populated, it will reset the field to + # None and we'll lose what is set (but not persisted) on the + # object. + continue elif key in spec_obj: setattr(spec, key, getattr(spec_obj, key)) spec._context = context @@ -560,7 +585,8 @@ class RequestSpec(base.NovaObject): spec.instance_group.hosts = None # NOTE(mriedem): Don't persist retries or requested_destination # since those are per-request - for excluded in ('retry', 'requested_destination'): + for excluded in ('retry', 'requested_destination', + 'requested_resources'): if excluded in spec and getattr(spec, excluded): setattr(spec, excluded, None) # NOTE(stephenfin): Don't persist network metadata since we have diff --git a/nova/tests/unit/objects/test_objects.py b/nova/tests/unit/objects/test_objects.py index 218c6eca6b40..0c8b2a8524f7 100644 --- a/nova/tests/unit/objects/test_objects.py +++ b/nova/tests/unit/objects/test_objects.py @@ -1143,7 +1143,7 @@ object_data = { 'Quotas': '1.3-40fcefe522111dddd3e5e6155702cf4e', 'QuotasNoOp': '1.3-347a039fc7cfee7b225b68b5181e0733', 'RequestGroup': '1.0-5f694d4237c00c7b01136a4e4bcacd6d', - 'RequestSpec': '1.11-851a690dbf116fb5165cc94ea6c85629', + 'RequestSpec': '1.12-25010470f219af9b6163f2a457a513f5', 'S3ImageMapping': '1.0-7dd7366a890d82660ed121de9092276e', 'SchedulerLimits': '1.0-249c4bd8e62a9b327b7026b7f19cc641', 'SchedulerRetries': '1.1-3c9c8b16143ebbb6ad7030e999d14cc0', diff --git a/nova/tests/unit/objects/test_request_spec.py b/nova/tests/unit/objects/test_request_spec.py index 77e0df68a94a..afbce11cd8e3 100644 --- a/nova/tests/unit/objects/test_request_spec.py +++ b/nova/tests/unit/objects/test_request_spec.py @@ -393,6 +393,24 @@ class _TestRequestSpecObject(object): filter_properties, None, instance.availability_zone) self.assertNotIn('security_groups', spec) + def test_from_components_with_port_resource_request(self, ): + ctxt = context.RequestContext(fakes.FAKE_USER_ID, + fakes.FAKE_PROJECT_ID) + instance = fake_instance.fake_instance_obj(ctxt) + image = {'id': uuids.image_id, 'properties': {'mappings': []}, + 'status': 'fake-status', 'location': 'far-away'} + flavor = fake_flavor.fake_flavor_obj(ctxt) + filter_properties = {'fake': 'property'} + + rg = request_spec.RequestGroup() + + spec = objects.RequestSpec.from_components(ctxt, instance.uuid, image, + flavor, instance.numa_topology, instance.pci_requests, + filter_properties, None, instance.availability_zone, + port_resource_requests=[rg]) + + self.assertListEqual([rg], spec.requested_resources) + def test_get_scheduler_hint(self): spec_obj = objects.RequestSpec(scheduler_hints={'foo_single': ['1'], 'foo_mul': ['1', '2']}) @@ -616,6 +634,52 @@ class _TestRequestSpecObject(object): self.assertRaises(exception.ObjectActionError, req_obj.create) + def test_create_does_not_persist_requested_resources(self): + req_obj = fake_request_spec.fake_spec_obj(remove_id=True) + rg = request_spec.RequestGroup(resources={'fake-rc': 13}) + req_obj.requested_resources = [rg] + orig_create_in_db = request_spec.RequestSpec._create_in_db + with mock.patch.object(request_spec.RequestSpec, '_create_in_db') \ + as mock_create_in_db: + mock_create_in_db.side_effect = orig_create_in_db + req_obj.create() + mock_create_in_db.assert_called_once() + updates = mock_create_in_db.mock_calls[0][1][1] + # assert that the requested_resources field is not stored in the db + data = jsonutils.loads(updates['spec'])['nova_object.data'] + self.assertIsNone(data['requested_resources']) + self.assertIsNotNone(data['instance_uuid']) + + # also we expect that requested_resources field does not reset after + # create + self.assertEqual( + 13, req_obj.requested_resources[0].resources['fake-rc']) + + def test_save_does_not_persist_requested_resources(self): + req_obj = fake_request_spec.fake_spec_obj(remove_id=True) + rg = request_spec.RequestGroup(resources={'fake-rc': 13}) + req_obj.requested_resources = [rg] + req_obj.create() + # change something to make sure _save_in_db is called + req_obj.num_instances = 2 + + orig_save_in_db = request_spec.RequestSpec._save_in_db + with mock.patch.object(request_spec.RequestSpec, '_save_in_db') \ + as mock_save_in_db: + mock_save_in_db.side_effect = orig_save_in_db + req_obj.save() + mock_save_in_db.assert_called_once() + updates = mock_save_in_db.mock_calls[0][1][2] + # assert that the requested_resources field is not stored in the db + data = jsonutils.loads(updates['spec'])['nova_object.data'] + self.assertIsNone(data['requested_resources']) + self.assertIsNotNone(data['instance_uuid']) + + # also we expect that requested_resources field does not reset after + # save + self.assertEqual(13, req_obj.requested_resources[0].resources + ['fake-rc']) + def test_save(self): req_obj = fake_request_spec.fake_spec_obj() # Make sure the requested_destination is not persisted since it is @@ -702,6 +766,16 @@ class _TestRequestSpecObject(object): self.assertNotIn('network_metadata', primitive) self.assertIn('user_id', primitive) + def test_compat_requested_resources(self): + req_obj = objects.RequestSpec(requested_resources=[], + instance_uuid=uuids.instance) + versions = ovo_base.obj_tree_get_versions('RequestSpec') + primitive = req_obj.obj_to_primitive(target_version='1.11', + version_manifest=versions) + primitive = primitive['nova_object.data'] + self.assertNotIn('requested_resources', primitive) + self.assertIn('instance_uuid', primitive) + def test_default_requested_destination(self): req_obj = objects.RequestSpec() self.assertIsNone(req_obj.requested_destination)