openstack/nova
Code Issues Proposed changes

Log the exception returned from a cell during API.get()

Browse Source 
When getting an instance using the compute.API we call
scatter_gather_single_cell() to be able to capture details when we fail
to retrieve a result from a cell such as timeouts and exceptions.

Currently however, we aren't logging the content of an exception if
scatter_gather_single_cell() returns an exception as the result. The
scatter gather method itself logs exceptions that are not of type
NovaException as these represent definite unexpected errors such as
database errors but NovaException handling are left for the caller to
decide whether they want to log it or re-raise it and so on.

It can be difficult to debug a situation where a cell is returning a
NovaException result so this adds logging of the exception content in
the compute API when we encounter an unexpected NovaException.

The existing log message has been updated to more accurately reflect
what has happened (did not respond vs exception). The assignment of the
exception object in scatter gather has also been updated to not
unnecessarily construct a new exception object because it (a) wasn't
necessary and (b) made asserting the LOG.exception() call argument in
the unit test difficult.

Related-Bug: #1970087

Change-Id: Iae1c61c72be5b6017b934293e3dc079a24eeb0e7
This commit is contained in:
melanie witt 2022-05-03 01:48:36 +00:00
parent b8cc570455
commit 1d4dbfd468
3 changed files with 16 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
nova/compute/api.py
View File

@ -2857,9 +2857,11 @@ class API:
# spec has been archived is being queried.
raise exception.InstanceNotFound(instance_id=uuid)
else:
if isinstance(result[cell_uuid], exception.NovaException):
LOG.exception(result[cell_uuid])
raise exception.NovaException(
_("Cell %s is not responding and hence instance "
"info is not available.") % cell_uuid)
_("Cell %s is not responding or returned an exception, "
"hence instance info is not available.") % cell_uuid)
def _get_instance(self, context, instance_uuid, expected_attrs,
cell_down_support=False):

2
nova/context.py
View File

@ -424,7 +424,7 @@ def scatter_gather_cells(context, cell_mappings, timeout, fn, *args, **kwargs):
# 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)
result = e
# The queue is already synchronized.
queue.put((cell_uuid, result))

12
nova/tests/unit/compute/test_api.py
View File

@ -6371,8 +6371,9 @@ class _ComputeAPIUnitTestMixIn(object):
self.assertEqual(uuids.inst, result.uuid)
mock_get_inst.assert_called_once()
@mock.patch('nova.compute.api.LOG.exception')
@mock.patch.object(objects.Instance, 'get_by_uuid')
def test_get_instance_from_cell_failure(self, mock_get_inst):
def test_get_instance_from_cell_failure(self, mock_get_inst, mock_log_exp):
# Make sure InstanceNotFound is bubbled up and not treated like
# other errors
mock_get_inst.side_effect = exception.InstanceNotFound(
@ -6385,6 +6386,15 @@ class _ComputeAPIUnitTestMixIn(object):
self.compute_api._get_instance_from_cell, self.context,
im, [], False)
self.assertIn('could not be found', str(exp))
# Make sure other unexpected NovaException are logged for debugging
mock_get_inst.side_effect = exception.NovaException()
exp = self.assertRaises(
exception.NovaException, self.compute_api._get_instance_from_cell,
self.context, im, [], False)
msg = (f'Cell {cell_mapping.uuid} is not responding or returned an '
'exception, hence instance info is not available.')
self.assertIn(msg, str(exp))
mock_log_exp.assert_called_once_with(mock_get_inst.side_effect)
@mock.patch('nova.compute.api.API._save_user_id_in_instance_mapping')
@mock.patch.object(objects.RequestSpec, 'get_by_instance_uuid')