diff --git a/nova/compute/resource_tracker.py b/nova/compute/resource_tracker.py index cef14ea638f..1775f2f700f 100644 --- a/nova/compute/resource_tracker.py +++ b/nova/compute/resource_tracker.py @@ -934,18 +934,13 @@ def _update_available_resource(self, context, resources, startup=False): context, self.compute_nodes[nodename], migrations, instance_by_uuid) - # Detect and account for orphaned instances that may exist on the - # hypervisor, but are not in the DB: - orphans = self._find_orphaned_instances() - self._update_usage_from_orphans(orphans, nodename) - cn = self.compute_nodes[nodename] # NOTE(yjiang5): Because pci device tracker status is not cleared in # this periodic task, and also because the resource tracker is not # notified when instances are deleted, we need remove all usages # from deleted instances. - self.pci_tracker.clean_usage(instances, migrations, orphans) + self.pci_tracker.clean_usage(instances, migrations) dev_pools_obj = self.pci_tracker.stats.to_device_pools_obj() cn.pci_device_pools = dev_pools_obj @@ -1630,40 +1625,6 @@ def delete_allocation_for_evacuated_instance(self, context, instance, node, "instance on the %s node %s", node_type, cn_uuid, instance=instance) - def _find_orphaned_instances(self): - """Given the set of instances and migrations already account for - by resource tracker, sanity check the hypervisor to determine - if there are any "orphaned" instances left hanging around. - - Orphans could be consuming memory and should be accounted for in - usage calculations to guard against potential out of memory - errors. - """ - uuids1 = frozenset(self.tracked_instances) - uuids2 = frozenset(self.tracked_migrations.keys()) - uuids = uuids1 | uuids2 - - usage = self.driver.get_per_instance_usage() - vuuids = frozenset(usage.keys()) - - orphan_uuids = vuuids - uuids - orphans = [usage[uuid] for uuid in orphan_uuids] - - return orphans - - def _update_usage_from_orphans(self, orphans, nodename): - """Include orphaned instances in usage.""" - for orphan in orphans: - memory_mb = orphan['memory_mb'] - - LOG.warning("Detected running orphan instance: %(uuid)s " - "(consuming %(memory_mb)s MB memory)", - {'uuid': orphan['uuid'], 'memory_mb': memory_mb}) - - # just record memory usage for the orphan - usage = {'memory_mb': memory_mb} - self._update_usage(usage, nodename) - def delete_allocation_for_shelve_offloaded_instance(self, context, instance): self.reportclient.delete_allocation_for_instance(context, diff --git a/nova/pci/manager.py b/nova/pci/manager.py index 3084643f5e8..b9eea0363bc 100644 --- a/nova/pci/manager.py +++ b/nova/pci/manager.py @@ -363,14 +363,13 @@ def update_pci_for_instance(self, context, instance, sign): if sign == 1: self.allocate_instance(instance) - def clean_usage(self, instances, migrations, orphans): + def clean_usage(self, instances, migrations): """Remove all usages for instances not passed in the parameter. The caller should hold the COMPUTE_RESOURCE_SEMAPHORE lock """ existed = set(inst['uuid'] for inst in instances) existed |= set(mig['instance_uuid'] for mig in migrations) - existed |= set(inst['uuid'] for inst in orphans) # need to copy keys, because the dict is modified in the loop body for uuid in list(self.claims): diff --git a/nova/tests/unit/compute/test_resource_tracker.py b/nova/tests/unit/compute/test_resource_tracker.py index cd1db470903..b6465d87c9c 100644 --- a/nova/tests/unit/compute/test_resource_tracker.py +++ b/nova/tests/unit/compute/test_resource_tracker.py @@ -815,70 +815,6 @@ def test_some_instances_no_migrations(self, get_mock, migr_mock, actual_resources)) update_mock.assert_called_once() - @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_and_error') - @mock.patch('nova.objects.InstanceList.get_by_host_and_node') - def test_orphaned_instances_no_migrations(self, get_mock, migr_mock, - get_cn_mock, pci_mock, - instance_pci_mock): - # Setup virt resources to match used resources to number - # of defined instances on the hypervisor - virt_resources = copy.deepcopy(_VIRT_DRIVER_AVAIL_RESOURCES) - virt_resources.update(memory_mb_used=64) - self._setup_rt(virt_resources=virt_resources) - - get_mock.return_value = [] - migr_mock.return_value = [] - get_cn_mock.return_value = _COMPUTE_NODE_FIXTURES[0] - - # Orphaned instances are those that the virt driver has on - # record as consuming resources on the compute node, but the - # Nova database has no record of the instance being active - # on the host. For some reason, the resource tracker only - # considers orphaned instance's memory usage in its calculations - # of free resources... - orphaned_usages = { - '71ed7ef6-9d2e-4c65-9f4e-90bb6b76261d': { - # Yes, the return result format of get_per_instance_usage - # is indeed this stupid and redundant. Also note that the - # libvirt driver just returns an empty dict always for this - # method and so who the heck knows whether this stuff - # actually works. - 'uuid': '71ed7ef6-9d2e-4c65-9f4e-90bb6b76261d', - 'memory_mb': 64 - } - } - vd = self.driver_mock - vd.get_per_instance_usage.return_value = orphaned_usages - - update_mock = self._update_available_resources() - - get_cn_mock.assert_called_once_with(mock.ANY, _HOSTNAME, _NODENAME) - expected_resources = copy.deepcopy(_COMPUTE_NODE_FIXTURES[0]) - vals = { - 'free_disk_gb': 6, - 'local_gb': 6, - 'free_ram_mb': 448, # 512 - 64 orphaned usage - 'memory_mb_used': 64, - 'vcpus_used': 0, - 'local_gb_used': 0, - 'memory_mb': 512, - 'current_workload': 0, - 'vcpus': 4, - # Yep, for some reason, orphaned instances are not counted - # as running VMs... - 'running_vms': 0 - } - _update_compute_node(expected_resources, **vals) - actual_resources = update_mock.call_args[0][1] - self.assertTrue(obj_base.obj_equal_prims(expected_resources, - actual_resources)) - update_mock.assert_called_once() - @mock.patch('nova.compute.utils.is_volume_backed_instance', return_value=False) @mock.patch('nova.objects.InstancePCIRequests.get_by_instance', diff --git a/nova/tests/unit/pci/test_manager.py b/nova/tests/unit/pci/test_manager.py index fe7d918e27b..4fd56eec09d 100644 --- a/nova/tests/unit/pci/test_manager.py +++ b/nova/tests/unit/pci/test_manager.py @@ -473,7 +473,6 @@ def test_clean_usage(self): inst_2 = copy.copy(self.inst) inst_2.uuid = uuidsentinel.instance2 migr = {'instance_uuid': 'uuid2', 'vm_state': vm_states.BUILDING} - orph = {'uuid': 'uuid3', 'vm_state': vm_states.BUILDING} pci_requests_obj = self._create_pci_requests_object( [{'count': 1, 'spec': [{'vendor_id': 'v'}]}]) @@ -490,7 +489,7 @@ def test_clean_usage(self): self.assertEqual(len(free_devs), 1) self.assertEqual(free_devs[0].vendor_id, 'v') - self.tracker.clean_usage([self.inst], [migr], [orph]) + self.tracker.clean_usage([self.inst], [migr]) free_devs = self.tracker.pci_stats.get_free_devs() self.assertEqual(len(free_devs), 2) self.assertEqual( @@ -504,7 +503,7 @@ def test_clean_usage_no_request_match_no_claims(self): self.tracker.update_pci_for_instance(None, self.inst, sign=1) free_devs = self.tracker.pci_stats.get_free_devs() self.assertEqual(3, len(free_devs)) - self.tracker.clean_usage([], [], []) + self.tracker.clean_usage([], []) free_devs = self.tracker.pci_stats.get_free_devs() self.assertEqual(3, len(free_devs)) self.assertEqual( diff --git a/nova/virt/driver.py b/nova/virt/driver.py index 5fe612f56cd..fa51e5c1adc 100644 --- a/nova/virt/driver.py +++ b/nova/virt/driver.py @@ -1577,15 +1577,6 @@ def node_is_available(self, nodename): # Refresh and check again. return nodename in self.get_available_nodes(refresh=True) - # TODO(stephenfin): This was only implemented (properly) for XenAPI and - # should be removed. - def get_per_instance_usage(self): - """Get information about instance resource usage. - - :returns: dict of nova uuid => dict of usage info - """ - return {} - def instance_on_disk(self, instance): """Checks access of instance files on the host.