From 939a61d1c6054f24b6d47b5a6f1fa90fadc0c690 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Tue, 20 Nov 2018 17:30:36 -0500 Subject: [PATCH] Remove NovaException logging from scatter_gather_cells With change I861b223ee46b0f0a31f646a4b45f8a02410253cf the caller of the various scatter_gatter functions can get any exception result and decide what kind of logging is necessary, so this change removes the logging from the scatter_gather_cells method **for NovaExceptions**. This is needed because of change Iaea1cb4ed93bb98f451de4f993106d7891ca3682 where showing a server which is deleted, which results in an InstanceNotFound exception, gets logged as an exception in the nova-api logs and shouldn't be. Non-NovaExceptions will still get logged as before since those are likely due to unexpected errors, like DBErrors. Change-Id: I930ee897c46385c3c9719b393753c4484caf909f Closes-Bug: #1804325 --- nova/context.py | 4 +++- nova/tests/unit/test_context.py | 16 +++++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/nova/context.py b/nova/context.py index f290a02be7fb..624b908e976f 100644 --- a/nova/context.py +++ b/nova/context.py @@ -440,7 +440,9 @@ def scatter_gather_cells(context, cell_mappings, timeout, fn, *args, **kwargs): with target_cell(context, cell_mapping) as cctxt: result = fn(cctxt, *args, **kwargs) except Exception as e: - LOG.exception('Error gathering result from cell %s', cell_uuid) + # Only log the exception traceback for non-nova exceptions. + if not isinstance(e, exception.NovaException): + LOG.exception('Error gathering result from cell %s', cell_uuid) result = e.__class__(e.args) # The queue is already synchronized. queue.put((cell_uuid, result)) diff --git a/nova/tests/unit/test_context.py b/nova/tests/unit/test_context.py index b40d95bf5b11..4827757a40c6 100644 --- a/nova/tests/unit/test_context.py +++ b/nova/tests/unit/test_context.py @@ -430,9 +430,23 @@ class ContextTestCase(test.NoDBTestCase): ctxt, mappings, 30, objects.InstanceList.get_by_filters) self.assertEqual(2, len(results)) self.assertIn(mock.sentinel.instances, results.values()) - isinstance(results.values(), Exception) + self.assertIsInstance(results[mapping1.uuid], Exception) + # non-NovaException gets logged self.assertTrue(mock_log_exception.called) + # Now run it again with a NovaException to see it's not logged. + mock_log_exception.reset_mock() + mock_get_inst.side_effect = [mock.sentinel.instances, + exception.NotFound()] + + results = context.scatter_gather_cells( + ctxt, mappings, 30, objects.InstanceList.get_by_filters) + self.assertEqual(2, len(results)) + self.assertIn(mock.sentinel.instances, results.values()) + self.assertIsInstance(results[mapping1.uuid], exception.NovaException) + # NovaExceptions are not logged, the caller should handle them. + mock_log_exception.assert_not_called() + @mock.patch('nova.context.scatter_gather_cells') @mock.patch('nova.objects.CellMappingList.get_all') def test_scatter_gather_all_cells(self, mock_get_all, mock_scatter):