diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 0f388ef0e74c..dfd2a68984d8 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -6428,6 +6428,32 @@ class ComputeManager(manager.Manager): "instance: %s"), e, instance=instance) + def update_available_resource_for_node(self, context, nodename): + + rt = self._get_resource_tracker(nodename) + try: + rt.update_available_resource(context) + except exception.ComputeHostNotFound: + # NOTE(comstud): We can get to this case if a node was + # marked 'deleted' in the DB and then re-added with a + # different auto-increment id. The cached resource + # tracker tried to update a deleted record and failed. + # Don't add this resource tracker to the new dict, so + # that this will resolve itself on the next run. + LOG.info(_LI("Compute node '%s' not found in " + "update_available_resource."), nodename) + self._resource_tracker_dict.pop(nodename, None) + return + except Exception: + LOG.exception(_LE("Error updating resources for node " + "%(node)s."), {'node': nodename}) + + # NOTE(comstud): Replace the RT cache before looping through + # compute nodes to delete below, as we can end up doing greenthread + # switches there. Best to have everyone using the newest cache + # ASAP. + self._resource_tracker_dict[nodename] = rt + @periodic_task.periodic_task(spacing=CONF.update_resources_interval) def update_available_resource(self, context): """See driver.get_available_resource() @@ -6437,35 +6463,16 @@ class ComputeManager(manager.Manager): :param context: security context """ - new_resource_tracker_dict = {} compute_nodes_in_db = self._get_compute_nodes_in_db(context, use_slave=True) nodenames = set(self.driver.get_available_nodes()) for nodename in nodenames: - rt = self._get_resource_tracker(nodename) - try: - rt.update_available_resource(context) - except exception.ComputeHostNotFound: - # NOTE(comstud): We can get to this case if a node was - # marked 'deleted' in the DB and then re-added with a - # different auto-increment id. The cached resource - # tracker tried to update a deleted record and failed. - # Don't add this resource tracker to the new dict, so - # that this will resolve itself on the next run. - LOG.info(_LI("Compute node '%s' not found in " - "update_available_resource."), nodename) - continue - except Exception: - LOG.exception(_LE("Error updating resources for node " - "%(node)s."), {'node': nodename}) - new_resource_tracker_dict[nodename] = rt + self.update_available_resource_for_node(context, nodename) - # NOTE(comstud): Replace the RT cache before looping through - # compute nodes to delete below, as we can end up doing greenthread - # switches there. Best to have everyone using the newest cache - # ASAP. - self._resource_tracker_dict = new_resource_tracker_dict + self._resource_tracker_dict = { + k: v for k, v in self._resource_tracker_dict.items() + if k in nodenames} # Delete orphan compute node not reported by driver but still in db for cn in compute_nodes_in_db: diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index eac8d8b81d87..954d24745fe8 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -158,57 +158,84 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): '_shutdown_instance', 'delete'], methods_called) - @mock.patch.object(manager.ComputeManager, '_get_resource_tracker') - @mock.patch.object(fake_driver.FakeDriver, 'get_available_nodes') - @mock.patch.object(manager.ComputeManager, '_get_compute_nodes_in_db') - def test_update_available_resource(self, get_db_nodes, get_avail_nodes, - get_rt): - info = {'cn_id': 1} - - def _make_compute_node(hyp_hostname): + def _make_compute_node(self, hyp_hostname, cn_id): cn = mock.Mock(spec_set=['hypervisor_hostname', 'id', 'destroy']) - cn.id = info['cn_id'] - info['cn_id'] += 1 + cn.id = cn_id cn.hypervisor_hostname = hyp_hostname return cn - def _make_rt(node): + def _make_rt(self, node): n = mock.Mock(spec_set=['update_available_resource', 'nodename']) n.nodename = node return n - ctxt = mock.Mock() - db_nodes = [_make_compute_node('node1'), - _make_compute_node('node2'), - _make_compute_node('node3'), - _make_compute_node('node4')] + @mock.patch.object(manager.ComputeManager, '_get_resource_tracker') + @mock.patch.object(fake_driver.FakeDriver, 'get_available_nodes') + @mock.patch.object(manager.ComputeManager, '_get_compute_nodes_in_db') + def test_update_available_resource_for_node( + self, get_db_nodes, get_avail_nodes, get_rt): + db_nodes = [] + + db_nodes = [self._make_compute_node('node%s' % i, i) + for i in range(1, 5)] avail_nodes = set(['node2', 'node3', 'node4', 'node5']) avail_nodes_l = list(avail_nodes) - rts = [_make_rt(node) for node in avail_nodes_l] + rts = [self._make_rt(node) for node in avail_nodes_l] # Make the 2nd and 3rd ones raise exc = exception.ComputeHostNotFound(host='fake') rts[1].update_available_resource.side_effect = exc exc = test.TestingException() rts[2].update_available_resource.side_effect = exc - rts_iter = iter(rts) + get_db_nodes.return_value = db_nodes + get_avail_nodes.return_value = avail_nodes + get_rt.side_effect = rts - def _get_rt_side_effect(*args, **kwargs): - return next(rts_iter) + self.compute.update_available_resource_for_node(self.context, + avail_nodes_l[0]) + self.assertEqual(self.compute._resource_tracker_dict[avail_nodes_l[0]], + rts[0]) + + # Update ComputeHostNotFound + self.compute.update_available_resource_for_node(self.context, + avail_nodes_l[1]) + self.assertNotIn(self.compute._resource_tracker_dict, avail_nodes_l[1]) + + # Update TestException + self.compute.update_available_resource_for_node(self.context, + avail_nodes_l[2]) + self.assertEqual(self.compute._resource_tracker_dict[ + avail_nodes_l[2]], rts[2]) + + @mock.patch.object(manager.ComputeManager, '_get_resource_tracker') + @mock.patch.object(fake_driver.FakeDriver, 'get_available_nodes') + @mock.patch.object(manager.ComputeManager, '_get_compute_nodes_in_db') + def test_update_available_resource(self, get_db_nodes, get_avail_nodes, + get_rt): + db_nodes = [self._make_compute_node('node%s' % i, i) + for i in range(1, 5)] + avail_nodes = set(['node2', 'node3', 'node4', 'node5']) + avail_nodes_l = list(avail_nodes) + rts = [self._make_rt(node) for node in avail_nodes_l] + # Make the 2nd and 3rd ones raise + exc = exception.ComputeHostNotFound(host='fake') + rts[1].update_available_resource.side_effect = exc + exc = test.TestingException() + rts[2].update_available_resource.side_effect = exc expected_rt_dict = {avail_nodes_l[0]: rts[0], avail_nodes_l[2]: rts[2], avail_nodes_l[3]: rts[3]} get_db_nodes.return_value = db_nodes get_avail_nodes.return_value = avail_nodes - get_rt.side_effect = _get_rt_side_effect - self.compute.update_available_resource(ctxt) - get_db_nodes.assert_called_once_with(ctxt, use_slave=True) + get_rt.side_effect = rts + self.compute.update_available_resource(self.context) + get_db_nodes.assert_called_once_with(self.context, use_slave=True) self.assertEqual(sorted([mock.call(node) for node in avail_nodes]), sorted(get_rt.call_args_list)) for rt in rts: - rt.update_available_resource.assert_called_once_with(ctxt) + rt.update_available_resource.assert_called_once_with(self.context) self.assertEqual(expected_rt_dict, self.compute._resource_tracker_dict) # First node in set should have been removed from DB