From 35d76c7cf5b122c9a8b24e62adc73510bbd6d94c Mon Sep 17 00:00:00 2001 From: ricolin Date: Mon, 7 Oct 2019 18:32:24 +0800 Subject: [PATCH] [Gate fix] Avoid use cell_uuid before assignment Found this error in heat gate during running grenade test job. Should not asking to provide cell_uuid if queue.get is timeout. Closes-Bug: #1847131 Change-Id: I7f9edc9a4b4930f4dce98df271888fa8082a1701 --- nova/context.py | 4 ++-- nova/tests/unit/test_context.py | 34 ++++++++++++++++++++++++++++++++- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/nova/context.py b/nova/context.py index b87a74b217f1..a3b9dc8b79e5 100644 --- a/nova/context.py +++ b/nova/context.py @@ -445,8 +445,8 @@ def scatter_gather_cells(context, cell_mappings, timeout, fn, *args, **kwargs): except exception.CellTimeout: # NOTE(melwitt): We'll fill in did_not_respond_sentinels at the # same time we kill/wait for the green threads. - LOG.warning('Timed out waiting for response from cell %s', - cell_uuid, exc_info=True) + LOG.warning('Timed out waiting for response from cell', + exc_info=True) # Kill the green threads still pending and wait on those we know are done. for cell_uuid, greenthread in greenthreads: diff --git a/nova/tests/unit/test_context.py b/nova/tests/unit/test_context.py index 1481714695c0..2bc8d1aee172 100644 --- a/nova/tests/unit/test_context.py +++ b/nova/tests/unit/test_context.py @@ -385,13 +385,45 @@ class ContextTestCase(test.NoDBTestCase): exception.CellTimeout()] results = context.scatter_gather_cells( - ctxt, mappings, 30, objects.InstanceList.get_by_filters) + ctxt, mappings, 30, objects.InstanceList.get_by_filters, {}) self.assertEqual(2, len(results)) self.assertIn(mock.sentinel.instances, results.values()) self.assertIn(context.did_not_respond_sentinel, results.values()) mock_timeout.assert_called_once_with(30, exception.CellTimeout) self.assertTrue(mock_log_warning.called) + @mock.patch('nova.context.LOG.warning') + @mock.patch('eventlet.timeout.Timeout') + @mock.patch('eventlet.queue.LightQueue.get') + @mock.patch('nova.objects.InstanceList.get_by_filters') + def test_scatter_gather_cells_all_timeout(self, mock_get_inst, + mock_get_result, mock_timeout, + mock_log_warning): + """This is a regression test for bug 1847131. + test_scatter_gather_cells_timeout did not catch the issue because it + yields a result which sets the cell_uuid variable in scope before the + CellTimeout is processed and logged. In this test we only raise the + CellTimeout so cell_uuid will not be in scope for the log message. + """ + # This is needed because we're mocking get_by_filters. + self.useFixture(nova_fixtures.SpawnIsSynchronousFixture()) + ctxt = context.get_context() + mapping0 = objects.CellMapping(database_connection='fake://db0', + transport_url='none:///', + uuid=objects.CellMapping.CELL0_UUID) + mappings = objects.CellMappingList(objects=[mapping0]) + + # Simulate cell0 not responding. + mock_get_result.side_effect = exception.CellTimeout() + + results = context.scatter_gather_cells( + ctxt, mappings, 30, objects.InstanceList.get_by_filters, {}) + self.assertEqual(1, len(results)) + self.assertIn(context.did_not_respond_sentinel, results.values()) + mock_timeout.assert_called_once_with(30, exception.CellTimeout) + mock_log_warning.assert_called_once_with( + 'Timed out waiting for response from cell', exc_info=True) + @mock.patch('nova.context.LOG.exception') @mock.patch('nova.objects.InstanceList.get_by_filters') def test_scatter_gather_cells_exception(self, mock_get_inst,