From 49964bdc649bab88813fdcd34a2e54616057ac8d Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Thu, 21 Nov 2019 10:11:28 -0500 Subject: [PATCH] Avoid spurious error logging in _get_compute_nodes_in_db When a compute service is using the ironic driver and that compute service is not yet managing any ironic nodes, the update_available_resource periodic will spew errors every time it runs, e.g.: 2019-10-07 07:32:21.268 6 ERROR nova.compute.manager [req-9de2cbda-9d8f-4a0f-a5be-de74d26077a2 - - - - -] No compute node record for host primary-ironic: nova.exception_Remote.ComputeHostNotFound_Remote: Compute host primary-ironic could not be found. 2019-10-07 07:33:22.454 6 ERROR nova.compute.manager [req-9de2cbda-9d8f-4a0f-a5be-de74d26077a2 - - - - -] No compute node record for host primary-ironic: nova.exception_Remote.ComputeHostNotFound_Remote: Compute host primary-ironic could not be found. 2019-10-07 07:34:22.416 6 ERROR nova.compute.manager [req-9de2cbda-9d8f-4a0f-a5be-de74d26077a2 - - - - -] No compute node record for host primary-ironic: nova.exception_Remote.ComputeHostNotFound_Remote: Compute host primary-ironic could not be found. This is a simple change to just not log anything if there are no compute node records in the database and the driver is not reporting any nodes to manage. Change-Id: Ic00c80fb89d08c19d97dad9de4061cac892ad7fb Closes-Bug: #1847054 --- nova/compute/manager.py | 30 ++++++++++++--------- nova/tests/unit/compute/test_compute.py | 2 +- nova/tests/unit/compute/test_compute_mgr.py | 21 ++++++++++++--- 3 files changed, 37 insertions(+), 16 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index a7b2f483cd..831beda309 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -9192,16 +9192,17 @@ class ComputeManager(manager.Manager): :param startup: True if this is being called when the nova-compute service is starting, False otherwise. """ - - compute_nodes_in_db = self._get_compute_nodes_in_db(context, - use_slave=True, - startup=startup) try: nodenames = set(self.driver.get_available_nodes()) except exception.VirtDriverNotReady: LOG.warning("Virt driver is not ready.") return + compute_nodes_in_db = self._get_compute_nodes_in_db(context, + nodenames, + use_slave=True, + startup=startup) + # Delete orphan compute node not reported by driver but still in db for cn in compute_nodes_in_db: if cn.hypervisor_hostname not in nodenames: @@ -9226,19 +9227,24 @@ class ComputeManager(manager.Manager): self._update_available_resource_for_node(context, nodename, startup=startup) - def _get_compute_nodes_in_db(self, context, use_slave=False, + def _get_compute_nodes_in_db(self, context, nodenames, use_slave=False, startup=False): try: return objects.ComputeNodeList.get_all_by_host(context, self.host, use_slave=use_slave) except exception.NotFound: - if startup: - LOG.warning( - "No compute node record found for host %s. If this is " - "the first time this service is starting on this " - "host, then you can ignore this warning.", self.host) - else: - LOG.error("No compute node record for host %s", self.host) + # If the driver is not reporting any nodenames we should not + # expect there to be compute nodes so we just return in that case. + # For example, this could be an ironic compute and it is not + # managing any nodes yet. + if nodenames: + if startup: + LOG.warning( + "No compute node record found for host %s. If this is " + "the first time this service is starting on this " + "host, then you can ignore this warning.", self.host) + else: + LOG.error("No compute node record for host %s", self.host) return [] @periodic_task.periodic_task( diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 8c15b30298..dc53ef1c4d 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -166,7 +166,7 @@ class BaseTestCase(test.TestCase): self.compute.driver) self.compute.rt = fake_rt - def fake_get_compute_nodes_in_db(self, context, **kwargs): + def fake_get_compute_nodes_in_db(self, context, *args, **kwargs): fake_compute_nodes = [{'local_gb': 259, 'uuid': uuids.fake_compute_node, 'vcpus_used': 0, diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 18b2f73609..01d50e44e1 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -336,8 +336,8 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, get_db_nodes.return_value = db_nodes get_avail_nodes.return_value = avail_nodes self.compute.update_available_resource(self.context, startup=True) - get_db_nodes.assert_called_once_with(self.context, use_slave=True, - startup=True) + get_db_nodes.assert_called_once_with(self.context, avail_nodes, + use_slave=True, startup=True) self.assertEqual(len(avail_nodes_l), update_mock.call_count) update_mock.assert_has_calls( [mock.call(self.context, node, startup=True) @@ -398,12 +398,27 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, compute node on startup since this may be expected. """ self.assertEqual([], self.compute._get_compute_nodes_in_db( - self.context, startup=True)) + self.context, {'fake-node'}, startup=True)) get_all_by_host.assert_called_once_with( self.context, self.compute.host, use_slave=False) self.assertTrue(mock_log.warning.called) self.assertFalse(mock_log.error.called) + @mock.patch.object(objects.ComputeNodeList, 'get_all_by_host', + side_effect=exception.NotFound) + @mock.patch('nova.compute.manager.LOG') + def test_get_compute_nodes_in_db_not_found_no_nodenames( + self, mock_log, get_all_by_host): + """Tests to make sure that _get_compute_nodes_in_db does not log + anything when ComputeNodeList.get_all_by_host raises NotFound and the + driver did not report any nodenames. + """ + self.assertEqual([], self.compute._get_compute_nodes_in_db( + self.context, set())) + get_all_by_host.assert_called_once_with( + self.context, self.compute.host, use_slave=False) + mock_log.assert_not_called() + def _trusted_certs_setup_instance(self, include_trusted_certs=True): instance = fake_instance.fake_instance_obj(self.context) if include_trusted_certs: