From be9dd3d9dbf19ff50998178f83b1de40e9acc09a Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Mon, 12 Oct 2020 16:25:55 +0200 Subject: [PATCH] Refactor update_pci_request_spec_with_allocated_interface_name Make update_pci_request_spec_with_allocated_interface_name only depend on a list of IntancePCIRequest o.vos instead of a whole Instance object. This will come in handy for the qos interface attach case where we only need to make the changes on the Instance o.vo after we are sure that the both the resource allocation and the pci claim is succeeded for the request. Change-Id: I5a6c6d3eed61895b00f9e9c3fb3b5d09d6786e9c blueprint: support-interface-attach-with-qos-ports --- nova/compute/manager.py | 13 +++-- nova/compute/utils.py | 8 +-- nova/conductor/tasks/live_migrate.py | 4 +- nova/tests/unit/compute/test_compute.py | 4 +- nova/tests/unit/compute/test_compute_mgr.py | 4 +- nova/tests/unit/compute/test_compute_utils.py | 54 ++++++------------- nova/tests/unit/compute/test_shelve.py | 4 +- .../unit/conductor/tasks/test_live_migrate.py | 4 +- 8 files changed, 39 insertions(+), 56 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index a4bf9547b64f..92352d3cc242 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -2341,7 +2341,8 @@ class ComputeManager(manager.Manager): try: compute_utils\ .update_pci_request_spec_with_allocated_interface_name( - context, self.reportclient, instance, provider_mapping) + context, self.reportclient, + instance.pci_requests.requests, provider_mapping) except (exception.AmbiguousResourceProviderForPCIRequest, exception.UnexpectedResourceProviderNameForPCIRequest ) as e: @@ -3482,7 +3483,8 @@ class ComputeManager(manager.Manager): if provider_mapping: compute_utils.\ update_pci_request_spec_with_allocated_interface_name( - context, self.reportclient, instance, provider_mapping) + context, self.reportclient, + instance.pci_requests.requests, provider_mapping) claim_context = rebuild_claim( context, instance, scheduled_node, allocations, @@ -5119,7 +5121,8 @@ class ComputeManager(manager.Manager): try: compute_utils.\ update_pci_request_spec_with_allocated_interface_name( - context, self.reportclient, instance, provider_mapping) + context, self.reportclient, + instance.pci_requests.requests, provider_mapping) except (exception.AmbiguousResourceProviderForPCIRequest, exception.UnexpectedResourceProviderNameForPCIRequest ) as e: @@ -6581,7 +6584,9 @@ class ComputeManager(manager.Manager): update = ( compute_utils. update_pci_request_spec_with_allocated_interface_name) - update(context, self.reportclient, instance, provider_mappings) + update( + context, self.reportclient, instance.pci_requests.requests, + provider_mappings) self.network_api.setup_instance_network_on_host( context, instance, self.host, diff --git a/nova/compute/utils.py b/nova/compute/utils.py index 1617f1dbe34b..2482a1ff3140 100644 --- a/nova/compute/utils.py +++ b/nova/compute/utils.py @@ -1477,13 +1477,13 @@ def notify_about_instance_delete(notifier, context, instance, def update_pci_request_spec_with_allocated_interface_name( - context, report_client, instance, provider_mapping): + context, report_client, pci_requests, provider_mapping): """Update the instance's PCI request based on the request group - resource provider mapping and the device RP name from placement. :param context: the request context :param report_client: a SchedulerReportClient instance - :param instance: an Instance object to be updated + :param pci_requests: A list of InstancePCIRequest objects to be updated :param provider_mapping: the request group - resource provider mapping in the form returned by the RequestSpec.get_request_group_mapping() call. @@ -1494,14 +1494,14 @@ def update_pci_request_spec_with_allocated_interface_name( have a well formatted name so we cannot parse the parent interface name out of it. """ - if not instance.pci_requests: + if not pci_requests: return def needs_update(pci_request, mapping): return (pci_request.requester_id and pci_request.requester_id in mapping) - for pci_request in instance.pci_requests.requests: + for pci_request in pci_requests: if needs_update(pci_request, provider_mapping): provider_uuids = provider_mapping[pci_request.requester_id] diff --git a/nova/conductor/tasks/live_migrate.py b/nova/conductor/tasks/live_migrate.py index d8531bad86fb..275a49cee1bc 100644 --- a/nova/conductor/tasks/live_migrate.py +++ b/nova/conductor/tasks/live_migrate.py @@ -545,8 +545,8 @@ class LiveMigrationTask(base.TaskBase): # runs. compute_utils.\ update_pci_request_spec_with_allocated_interface_name( - self.context, self.report_client, self.instance, - provider_mapping) + self.context, self.report_client, + self.instance.pci_requests.requests, provider_mapping) try: self._check_compatible_with_source_hypervisor(host) self._call_livem_checks_on_host(host, provider_mapping) diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index dbe3b17b7341..974c23b1185d 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -12935,8 +12935,8 @@ class EvacuateHostTestCase(BaseTestCase): self.inst.uuid) mock_update_pci_req.assert_called_once_with( - ctxt, self.compute.reportclient, self.inst, - mock.sentinel.mapping) + ctxt, self.compute.reportclient, + self.inst.pci_requests.requests, mock.sentinel.mapping) _test_rebuild(vm_is_stopped=vm_states_is_stopped) diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 3f99a498b89a..c3df7386c7a0 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -8060,6 +8060,8 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase): objects.RequestGroup( requester_id=uuids.port1, provider_uuids=[uuids.rp1])]) + self.instance.pci_requests = objects.InstancePCIRequests(requests=[]) + with test.nested( mock.patch.object(self.compute.driver, 'spawn'), mock.patch.object( @@ -10597,7 +10599,7 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase, clean_shutdown=True, migration=self.migration, host_list=[]) mock_update_pci.assert_called_once_with( - self.context, self.compute.reportclient, instance, + self.context, self.compute.reportclient, [], {uuids.port_id: [uuids.rp_uuid]}) mock_save.assert_called_once_with() diff --git a/nova/tests/unit/compute/test_compute_utils.py b/nova/tests/unit/compute/test_compute_utils.py index 98d567de56ca..9e7aee03fc14 100644 --- a/nova/tests/unit/compute/test_compute_utils.py +++ b/nova/tests/unit/compute/test_compute_utils.py @@ -1556,94 +1556,70 @@ class PciRequestUpdateTestCase(test.NoDBTestCase): self.context = context.RequestContext('fake', 'fake') def test_no_pci_request(self): - instance = objects.Instance( - pci_requests=objects.InstancePCIRequests(requests=[])) provider_mapping = {} compute_utils.update_pci_request_spec_with_allocated_interface_name( - self.context, mock.sentinel.report_client, instance, - provider_mapping) + self.context, mock.sentinel.report_client, [], provider_mapping) def test_pci_request_from_flavor(self): - instance = objects.Instance( - pci_requests=objects.InstancePCIRequests(requests=[ - objects.InstancePCIRequest(requester_id=None) - ])) + pci_requests = [objects.InstancePCIRequest(requester_id=None)] provider_mapping = {} compute_utils.update_pci_request_spec_with_allocated_interface_name( - self.context, mock.sentinel.report_client, instance, + self.context, mock.sentinel.report_client, pci_requests, provider_mapping) def test_pci_request_has_no_mapping(self): - instance = objects.Instance( - pci_requests=objects.InstancePCIRequests(requests=[ - objects.InstancePCIRequest(requester_id=uuids.port_1) - ])) + pci_requests = [objects.InstancePCIRequest(requester_id=uuids.port_1)] provider_mapping = {} compute_utils.update_pci_request_spec_with_allocated_interface_name( - self.context, mock.sentinel.report_client, instance, + self.context, mock.sentinel.report_client, pci_requests, provider_mapping) def test_pci_request_ambiguous_mapping(self): - instance = objects.Instance( - pci_requests=objects.InstancePCIRequests(requests=[ - objects.InstancePCIRequest(requester_id=uuids.port_1) - ])) + pci_requests = [objects.InstancePCIRequest(requester_id=uuids.port_1)] provider_mapping = {uuids.port_1: [uuids.rp1, uuids.rp2]} self.assertRaises( exception.AmbiguousResourceProviderForPCIRequest, (compute_utils. update_pci_request_spec_with_allocated_interface_name), - self.context, mock.sentinel.report_client, instance, + self.context, mock.sentinel.report_client, pci_requests, provider_mapping) def test_unexpected_provider_name(self): report_client = mock.Mock(spec=report.SchedulerReportClient) report_client.get_resource_provider_name.return_value = 'unexpected' - instance = objects.Instance( - pci_requests=objects.InstancePCIRequests(requests=[ - objects.InstancePCIRequest( - requester_id=uuids.port_1, - spec=[{}]) - ])) + pci_requests = [objects.InstancePCIRequest( + requester_id=uuids.port_1, spec=[{}])] provider_mapping = {uuids.port_1: [uuids.rp1]} self.assertRaises( exception.UnexpectedResourceProviderNameForPCIRequest, (compute_utils. update_pci_request_spec_with_allocated_interface_name), - self.context, report_client, instance, + self.context, report_client, pci_requests, provider_mapping) report_client.get_resource_provider_name.assert_called_once_with( self.context, uuids.rp1) - self.assertNotIn( - 'parent_ifname', instance.pci_requests.requests[0].spec[0]) + self.assertNotIn('parent_ifname', pci_requests[0].spec[0]) def test_pci_request_updated(self): report_client = mock.Mock(spec=report.SchedulerReportClient) report_client.get_resource_provider_name.return_value = ( 'host:agent:enp0s31f6') - instance = objects.Instance( - pci_requests=objects.InstancePCIRequests(requests=[ - objects.InstancePCIRequest( - requester_id=uuids.port_1, - spec=[{}], - ) - ])) + pci_requests = [objects.InstancePCIRequest( + requester_id=uuids.port_1, spec=[{}],)] provider_mapping = {uuids.port_1: [uuids.rp1]} compute_utils.update_pci_request_spec_with_allocated_interface_name( - self.context, report_client, instance, provider_mapping) + self.context, report_client, pci_requests, provider_mapping) report_client.get_resource_provider_name.assert_called_once_with( self.context, uuids.rp1) - self.assertEqual( - 'enp0s31f6', - instance.pci_requests.requests[0].spec[0]['parent_ifname']) + self.assertEqual('enp0s31f6', pci_requests[0].spec[0]['parent_ifname']) class AcceleratorRequestTestCase(test.NoDBTestCase): diff --git a/nova/tests/unit/compute/test_shelve.py b/nova/tests/unit/compute/test_shelve.py index eeda476c6273..0df00c463c78 100644 --- a/nova/tests/unit/compute/test_shelve.py +++ b/nova/tests/unit/compute/test_shelve.py @@ -641,7 +641,7 @@ class ShelveComputeManagerTestCase(test_compute.BaseTestCase): filter_properties={}, node='fake-node', request_spec=request_spec) mock_update_pci.assert_called_once_with( - self.context, self.compute.reportclient, instance, + self.context, self.compute.reportclient, [], {uuids.port_1: [uuids.rp1]}) mock_setup_network.assert_called_once_with( self.context, instance, self.compute.host, @@ -670,7 +670,7 @@ class ShelveComputeManagerTestCase(test_compute.BaseTestCase): filter_properties={}, node='fake-node', request_spec=request_spec) mock_update_pci.assert_called_once_with( - self.context, self.compute.reportclient, instance, + self.context, self.compute.reportclient, [], {uuids.port_1: [uuids.rp1]}) @mock.patch.object(objects.InstanceList, 'get_by_filters') diff --git a/nova/tests/unit/conductor/tasks/test_live_migrate.py b/nova/tests/unit/conductor/tasks/test_live_migrate.py index e8bb2310c95c..0c3852eab734 100644 --- a/nova/tests/unit/conductor/tasks/test_live_migrate.py +++ b/nova/tests/unit/conductor/tasks/test_live_migrate.py @@ -683,6 +683,7 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase): mock_fill_provider_mapping, mock_update_pci_req): resource_req = [objects.RequestGroup(requester_id=uuids.port_id)] self.mock_get_res_req.return_value = resource_req + self.instance.pci_requests = objects.InstancePCIRequests(requests=[]) self.assertEqual(("host1", "node1", fake_limits1), self.task._find_destination()) @@ -709,8 +710,7 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase): mock_fill_provider_mapping.assert_called_once_with( self.task.request_spec, fake_selection1) mock_update_pci_req.assert_called_once_with( - self.context, self.task.report_client, self.instance, - {uuids.port_id: []}) + self.context, self.task.report_client, [], {uuids.port_id: []}) @mock.patch.object(objects.InstanceMapping, 'get_by_instance_uuid', side_effect=exception.InstanceMappingNotFound(