diff --git a/nova/compute/resource_tracker.py b/nova/compute/resource_tracker.py index 2ff1cf7e08d4..ede167e2da8e 100644 --- a/nova/compute/resource_tracker.py +++ b/nova/compute/resource_tracker.py @@ -366,6 +366,7 @@ class ResourceTracker(object): # to initialize if self.compute_node: self._copy_resources(resources) + self._setup_pci_tracker(context, resources) return # now try to get the compute node record from the @@ -373,6 +374,7 @@ class ResourceTracker(object): self.compute_node = self._get_compute_node(context) if self.compute_node: self._copy_resources(resources) + self._setup_pci_tracker(context, resources) return # there was no local copy and none in the database @@ -386,6 +388,20 @@ class ResourceTracker(object): '%(host)s:%(node)s'), {'host': self.host, 'node': self.nodename}) + self._setup_pci_tracker(context, resources) + + def _setup_pci_tracker(self, context, resources): + if not self.pci_tracker: + n_id = self.compute_node.id if self.compute_node else None + self.pci_tracker = pci_manager.PciDevTracker(context, node_id=n_id) + if 'pci_passthrough_devices' in resources: + dev_json = resources.pop('pci_passthrough_devices') + self.pci_tracker.update_devices_from_hypervisor_resources( + dev_json) + + dev_pools_obj = self.pci_tracker.stats.to_device_pools_obj() + self.compute_node.pci_device_pools = dev_pools_obj + def _copy_resources(self, resources): """Copy resource values to initialise compute_node and related data structures. @@ -487,15 +503,6 @@ class ResourceTracker(object): if self.disabled: return - if 'pci_passthrough_devices' in resources: - # TODO(jaypipes): Move this into _init_compute_node() - if not self.pci_tracker: - n_id = self.compute_node.id if self.compute_node else None - self.pci_tracker = pci_manager.PciDevTracker(context, - node_id=n_id) - dev_json = resources.pop('pci_passthrough_devices') - self.pci_tracker.update_devices_from_hypervisor_resources(dev_json) - # Grab all instances assigned to this node: instances = objects.InstanceList.get_by_host_and_node( context, self.host, self.nodename, @@ -522,12 +529,9 @@ class ResourceTracker(object): # this periodic task, and also because the resource tracker is not # notified when instances are deleted, we need remove all usages # from deleted instances. - if self.pci_tracker: - self.pci_tracker.clean_usage(instances, migrations, orphans) - dev_pools_obj = self.pci_tracker.stats.to_device_pools_obj() - self.compute_node.pci_device_pools = dev_pools_obj - else: - self.compute_node.pci_device_pools = objects.PciDevicePoolList() + self.pci_tracker.clean_usage(instances, migrations, orphans) + dev_pools_obj = self.pci_tracker.stats.to_device_pools_obj() + self.compute_node.pci_device_pools = dev_pools_obj self._report_final_resource_view() diff --git a/nova/tests/unit/compute/test_resource_tracker.py b/nova/tests/unit/compute/test_resource_tracker.py index 71df0565376d..367729950bb2 100644 --- a/nova/tests/unit/compute/test_resource_tracker.py +++ b/nova/tests/unit/compute/test_resource_tracker.py @@ -284,16 +284,6 @@ class BaseTestCase(test.TestCase): compute.update(values) return compute - def _create_compute_node_obj(self, context): - # Use the db representation of a compute node returned - # by _create_compute_node() to create an equivalent compute - # node object. - compute = self._create_compute_node() - compute_obj = objects.ComputeNode() - compute_obj = objects.ComputeNode._from_db_object( - context, compute_obj, compute) - return compute_obj - def _create_service(self, host="fakehost", compute=None): if compute: compute = [compute] @@ -448,7 +438,6 @@ class BaseTestCase(test.TestCase): driver = self._driver() tracker = resource_tracker.ResourceTracker(host, driver, node) - tracker.compute_node = self._create_compute_node_obj(self.context) return tracker diff --git a/nova/tests/unit/compute/test_tracker.py b/nova/tests/unit/compute/test_tracker.py index 1fa8c9cc2e03..dd629c8ee4c9 100644 --- a/nova/tests/unit/compute/test_tracker.py +++ b/nova/tests/unit/compute/test_tracker.py @@ -433,11 +433,16 @@ class TestUpdateAvailableResources(BaseTestCase): self.rt.update_available_resource(mock.sentinel.ctx) return update_mock + @mock.patch('nova.objects.InstancePCIRequests.get_by_instance', + return_value=objects.InstancePCIRequests(requests=[])) + @mock.patch('nova.objects.PciDeviceList.get_by_compute_node', + return_value=objects.PciDeviceList()) @mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename') @mock.patch('nova.objects.MigrationList.get_in_progress_by_host_and_node') @mock.patch('nova.objects.InstanceList.get_by_host_and_node') def test_no_instances_no_migrations_no_reserved(self, get_mock, migr_mock, - get_cn_mock): + get_cn_mock, pci_mock, + instance_pci_mock): self.flags(reserved_host_disk_mb=0, reserved_host_memory_mb=0) self._setup_rt() @@ -491,11 +496,16 @@ class TestUpdateAvailableResources(BaseTestCase): self.assertTrue(obj_base.obj_equal_prims(expected_resources, self.rt.compute_node)) + @mock.patch('nova.objects.InstancePCIRequests.get_by_instance', + return_value=objects.InstancePCIRequests(requests=[])) + @mock.patch('nova.objects.PciDeviceList.get_by_compute_node', + return_value=objects.PciDeviceList()) @mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename') @mock.patch('nova.objects.MigrationList.get_in_progress_by_host_and_node') @mock.patch('nova.objects.InstanceList.get_by_host_and_node') def test_no_instances_no_migrations_reserved_disk_and_ram( - self, get_mock, migr_mock, get_cn_mock): + self, get_mock, migr_mock, get_cn_mock, pci_mock, + instance_pci_mock): self.flags(reserved_host_disk_mb=1024, reserved_host_memory_mb=512) self._setup_rt() @@ -537,11 +547,16 @@ class TestUpdateAvailableResources(BaseTestCase): self.assertTrue(obj_base.obj_equal_prims(expected_resources, self.rt.compute_node)) + @mock.patch('nova.objects.InstancePCIRequests.get_by_instance', + return_value=objects.InstancePCIRequests(requests=[])) + @mock.patch('nova.objects.PciDeviceList.get_by_compute_node', + return_value=objects.PciDeviceList()) @mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename') @mock.patch('nova.objects.MigrationList.get_in_progress_by_host_and_node') @mock.patch('nova.objects.InstanceList.get_by_host_and_node') def test_some_instances_no_migrations(self, get_mock, migr_mock, - get_cn_mock): + get_cn_mock, pci_mock, + instance_pci_mock): self.flags(reserved_host_disk_mb=0, reserved_host_memory_mb=0) @@ -590,11 +605,16 @@ class TestUpdateAvailableResources(BaseTestCase): self.assertTrue(obj_base.obj_equal_prims(expected_resources, self.rt.compute_node)) + @mock.patch('nova.objects.InstancePCIRequests.get_by_instance', + return_value=objects.InstancePCIRequests(requests=[])) + @mock.patch('nova.objects.PciDeviceList.get_by_compute_node', + return_value=objects.PciDeviceList()) @mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename') @mock.patch('nova.objects.MigrationList.get_in_progress_by_host_and_node') @mock.patch('nova.objects.InstanceList.get_by_host_and_node') def test_orphaned_instances_no_migrations(self, get_mock, migr_mock, - get_cn_mock): + get_cn_mock, pci_mock, + instance_pci_mock): self.flags(reserved_host_disk_mb=0, reserved_host_memory_mb=0) @@ -663,12 +683,17 @@ class TestUpdateAvailableResources(BaseTestCase): self.assertTrue(obj_base.obj_equal_prims(expected_resources, self.rt.compute_node)) + @mock.patch('nova.objects.InstancePCIRequests.get_by_instance', + return_value=objects.InstancePCIRequests(requests=[])) + @mock.patch('nova.objects.PciDeviceList.get_by_compute_node', + return_value=objects.PciDeviceList()) @mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename') @mock.patch('nova.objects.MigrationList.get_in_progress_by_host_and_node') @mock.patch('nova.objects.Instance.get_by_uuid') @mock.patch('nova.objects.InstanceList.get_by_host_and_node') def test_no_instances_source_migration(self, get_mock, get_inst_mock, - migr_mock, get_cn_mock): + migr_mock, get_cn_mock, pci_mock, + instance_pci_mock): # We test the behavior of update_available_resource() when # there is an active migration that involves this compute node # as the source host not the destination host, and the resource @@ -734,12 +759,17 @@ class TestUpdateAvailableResources(BaseTestCase): self.assertTrue(obj_base.obj_equal_prims(expected_resources, self.rt.compute_node)) + @mock.patch('nova.objects.InstancePCIRequests.get_by_instance', + return_value=objects.InstancePCIRequests(requests=[])) + @mock.patch('nova.objects.PciDeviceList.get_by_compute_node', + return_value=objects.PciDeviceList()) @mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename') @mock.patch('nova.objects.MigrationList.get_in_progress_by_host_and_node') @mock.patch('nova.objects.Instance.get_by_uuid') @mock.patch('nova.objects.InstanceList.get_by_host_and_node') def test_no_instances_dest_migration(self, get_mock, get_inst_mock, - migr_mock, get_cn_mock): + migr_mock, get_cn_mock, pci_mock, + instance_pci_mock): # We test the behavior of update_available_resource() when # there is an active migration that involves this compute node # as the destination host not the source host, and the resource @@ -802,12 +832,17 @@ class TestUpdateAvailableResources(BaseTestCase): self.assertTrue(obj_base.obj_equal_prims(expected_resources, self.rt.compute_node)) + @mock.patch('nova.objects.InstancePCIRequests.get_by_instance', + return_value=objects.InstancePCIRequests(requests=[])) + @mock.patch('nova.objects.PciDeviceList.get_by_compute_node', + return_value=objects.PciDeviceList()) @mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename') @mock.patch('nova.objects.MigrationList.get_in_progress_by_host_and_node') @mock.patch('nova.objects.Instance.get_by_uuid') @mock.patch('nova.objects.InstanceList.get_by_host_and_node') def test_no_instances_dest_evacuation(self, get_mock, get_inst_mock, - migr_mock, get_cn_mock): + migr_mock, get_cn_mock, pci_mock, + instance_pci_mock): # We test the behavior of update_available_resource() when # there is an active evacuation that involves this compute node # as the destination host not the source host, and the resource @@ -867,6 +902,10 @@ class TestUpdateAvailableResources(BaseTestCase): self.assertTrue(obj_base.obj_equal_prims(expected_resources, self.rt.compute_node)) + @mock.patch('nova.objects.InstancePCIRequests.get_by_instance', + return_value=objects.InstancePCIRequests(requests=[])) + @mock.patch('nova.objects.PciDeviceList.get_by_compute_node', + return_value=objects.PciDeviceList()) @mock.patch('nova.objects.MigrationContext.get_by_instance_uuid', return_value=None) @mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename') @@ -876,7 +915,9 @@ class TestUpdateAvailableResources(BaseTestCase): def test_some_instances_source_and_dest_migration(self, get_mock, get_inst_mock, migr_mock, get_cn_mock, - get_mig_ctxt_mock): + get_mig_ctxt_mock, + pci_mock, + instance_pci_mock): # We test the behavior of update_available_resource() when # there is an active migration that involves this compute node # as the destination host AND the source host, and the resource @@ -946,11 +987,13 @@ class TestUpdateAvailableResources(BaseTestCase): class TestInitComputeNode(BaseTestCase): + @mock.patch('nova.objects.PciDeviceList.get_by_compute_node', + return_value=objects.PciDeviceList()) @mock.patch('nova.objects.ComputeNode.create') @mock.patch('nova.objects.Service.get_by_compute_host') @mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename') def test_no_op_init_compute_node(self, get_mock, service_mock, - create_mock): + create_mock, pci_mock): self._setup_rt() resources = copy.deepcopy(_VIRT_DRIVER_AVAIL_RESOURCES) @@ -962,11 +1005,15 @@ class TestInitComputeNode(BaseTestCase): self.assertFalse(service_mock.called) self.assertFalse(get_mock.called) self.assertFalse(create_mock.called) + self.assertTrue(pci_mock.called) self.assertFalse(self.rt.disabled) + @mock.patch('nova.objects.PciDeviceList.get_by_compute_node', + return_value=objects.PciDeviceList()) @mock.patch('nova.objects.ComputeNode.create') @mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename') - def test_compute_node_loaded(self, get_mock, create_mock): + def test_compute_node_loaded(self, get_mock, create_mock, + pci_mock): self._setup_rt() def fake_get_node(_ctx, host, node): @@ -983,9 +1030,12 @@ class TestInitComputeNode(BaseTestCase): self.assertFalse(create_mock.called) self.assertFalse(self.rt.disabled) + @mock.patch('nova.objects.PciDeviceList.get_by_compute_node', + return_value=objects.PciDeviceList(objects=[])) @mock.patch('nova.objects.ComputeNode.create') @mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename') - def test_compute_node_created_on_empty(self, get_mock, create_mock): + def test_compute_node_created_on_empty(self, get_mock, create_mock, + pci_tracker_mock): self._setup_rt() get_mock.side_effect = exc.NotFound @@ -1017,6 +1067,7 @@ class TestInitComputeNode(BaseTestCase): # The expected compute represents the initial values used # when creating a compute node. expected_compute = objects.ComputeNode( + id=42, host_ip=resources['host_ip'], vcpus=resources['vcpus'], memory_mb=resources['memory_mb'], @@ -1036,6 +1087,7 @@ class TestInitComputeNode(BaseTestCase): cpu_allocation_ratio=cpu_alloc_ratio, disk_allocation_ratio=disk_alloc_ratio, stats={}, + pci_device_pools=objects.PciDevicePoolList(objects=[]) ) # Forcing the flags to the values we know @@ -1043,6 +1095,13 @@ class TestInitComputeNode(BaseTestCase): self.rt.cpu_allocation_ratio = cpu_alloc_ratio self.rt.disk_allocation_ratio = disk_alloc_ratio + def set_cn_id(): + # The PCI tracker needs the compute node's ID when starting up, so + # make sure that we set the ID value so we don't get a Cannot load + # 'id' in base class error + self.rt.compute_node.id = 42 # Has to be a number, not a mock + + create_mock.side_effect = set_cn_id self.rt._init_compute_node(mock.sentinel.ctx, resources) self.assertFalse(self.rt.disabled) @@ -1051,6 +1110,8 @@ class TestInitComputeNode(BaseTestCase): create_mock.assert_called_once_with() self.assertTrue(obj_base.obj_equal_prims(expected_compute, self.rt.compute_node)) + pci_tracker_mock.assert_called_once_with(mock.sentinel.ctx, + 42) def test_copy_resources_adds_allocation_ratios(self): self.flags(cpu_allocation_ratio=4.0, ram_allocation_ratio=3.0, @@ -1058,8 +1119,10 @@ class TestInitComputeNode(BaseTestCase): self._setup_rt() resources = copy.deepcopy(_VIRT_DRIVER_AVAIL_RESOURCES) - compute_node = copy.deepcopy(_COMPUTE_NODE_FIXTURES[0]) - self.rt.compute_node = compute_node + # TODO(jaypipes): Remove this manual setting of the RT.compute_node + # attribute once the compute node object is always created in the + # RT's constructor. + self.rt.compute_node = copy.deepcopy(_COMPUTE_NODE_FIXTURES[0]) self.rt._copy_resources(resources) self.assertEqual(4.0, self.rt.compute_node.cpu_allocation_ratio) @@ -1516,7 +1579,6 @@ class TestMoveClaim(BaseTestCase): super(TestMoveClaim, self).setUp() self._setup_rt() - self.rt.compute_node = copy.deepcopy(_COMPUTE_NODE_FIXTURES[0]) self.instance = _INSTANCE_FIXTURES[0].obj_clone() self.flavor = _INSTANCE_TYPE_OBJ_FIXTURES[1] @@ -1527,15 +1589,20 @@ class TestMoveClaim(BaseTestCase): self.elevated = mock.MagicMock() self.ctx.elevated.return_value = self.elevated - # Initialise extensible resource trackers self.flags(reserved_host_disk_mb=0, reserved_host_memory_mb=0) with test.nested( - mock.patch('nova.objects.InstanceList.get_by_host_and_node'), + mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename', + return_value=copy.deepcopy(_COMPUTE_NODE_FIXTURES[0])), + mock.patch('nova.objects.InstancePCIRequests.get_by_instance', + return_value=objects.InstancePCIRequests(requests=[])), + mock.patch('nova.objects.PciDeviceList.get_by_compute_node', + return_value=objects.PciDeviceList()), + mock.patch('nova.objects.InstanceList.get_by_host_and_node', + return_value=objects.InstanceList()), mock.patch('nova.objects.MigrationList.' - 'get_in_progress_by_host_and_node') - ) as (inst_list_mock, migr_mock): - inst_list_mock.return_value = objects.InstanceList(objects=[]) - migr_mock.return_value = objects.MigrationList(objects=[]) + 'get_in_progress_by_host_and_node', + return_value=objects.MigrationList()) + ) as (cn_mock, inst_pci_mock, pci_dev_mock, inst_list_mock, migr_mock): self.rt.update_available_resource(self.ctx) def register_mocks(self, pci_mock, inst_list_mock, inst_by_uuid, @@ -1592,11 +1659,14 @@ class TestMoveClaim(BaseTestCase): expected = copy.deepcopy(self.rt.compute_node) self.adjust_expected(expected, self.flavor) - create_mig_mock = mock.patch.object(self.rt, '_create_migration') - mig_ctxt_mock = mock.patch('nova.objects.MigrationContext', - return_value=mig_context_obj) - with create_mig_mock as migr_mock, mig_ctxt_mock as ctxt_mock: - migr_mock.return_value = _MIGRATION_FIXTURES['source-only'] + with test.nested( + mock.patch('nova.objects.InstancePCIRequests.get_by_instance', + return_value=objects.InstancePCIRequests(requests=[])), + mock.patch.object(self.rt, '_create_migration', + return_value=_MIGRATION_FIXTURES['source-only']), + mock.patch('nova.objects.MigrationContext', + return_value=mig_context_obj) + ) as (int_pci_mock, migr_mock, ctxt_mock): claim = self.rt.resize_claim( self.ctx, self.instance, self.flavor, None) self.assertEqual(1, ctxt_mock.call_count) @@ -1710,9 +1780,21 @@ class TestMoveClaim(BaseTestCase): # update_available_resource to initialise extensible resource trackers src_rt = self.rt (dst_rt, _, _) = setup_rt("other-host", "other-node") - dst_rt.compute_node = copy.deepcopy(_COMPUTE_NODE_FIXTURES[0]) inst_list_mock.return_value = objects.InstanceList(objects=[]) - dst_rt.update_available_resource(self.ctx) + with test.nested( + mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename', + return_value=copy.deepcopy(_COMPUTE_NODE_FIXTURES[0])), + mock.patch('nova.objects.InstancePCIRequests.get_by_instance', + return_value=objects.InstancePCIRequests(requests=[])), + mock.patch('nova.objects.PciDeviceList.get_by_compute_node', + return_value=objects.PciDeviceList()), + mock.patch('nova.objects.InstanceList.get_by_host_and_node', + return_value=objects.InstanceList()), + mock.patch('nova.objects.MigrationList.' + 'get_in_progress_by_host_and_node', + return_value=objects.MigrationList()) + ) as (cn_mock, inst_pci_mock, pci_dev_mock, inst_list_mock, migr_mock): + dst_rt.update_available_resource(self.ctx) # Register the instance with dst_rt expected = copy.deepcopy(dst_rt.compute_node)