From 7732f0e6f3691025a06f2abfc5a57c71d83e5e72 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Wed, 7 Aug 2019 12:23:15 -0400 Subject: [PATCH] Add useful error log when _determine_version_cap raises DBNotAllowed Change Icddbe4760eaff30e4e13c1e8d3d5d3f489dac3c4 was intended for the API service to check all cells for the minimum nova-compute service version when [upgrade_levels]/compute=auto. That worked in the gate with devstack because we don't configure nova-compute with access to the database and run nova-compute with a separate nova-cpu.conf so even if nova-compute is on the same host as the nova-api service, they aren't using the same config file (nova-api runs with nova.conf which has access to the API DB obviously). The problem is when nova-compute is configured with [upgrade_levels]/compute=auto and an [api_database]/connection, there are flows that can try to hit the API database directly because of the _determine_version_cap method. For example, the _sync_power_states periodic task trying to stop an instance, or even simple inter-compute communication over RPC like during a resize. This change simply catches the DBNotAllowed exception, logs a more useful error message, and re-raises the exception. In addition, the config help for the [api_database] group and "configuration" option specifically are updated to mention they should not be set on the nova-compute service. Change-Id: Iac2911a7a305a9d14bc6dadb364998f3ecb9ce42 Related-Bug: #1807044 Closes-Bug: #1839360 (cherry picked from commit 7d7d58509d5e60ec19c6310931dc62eeff033595) (cherry picked from commit bd03723a0c1a16d67433658fb486a84bb1bddf02) --- nova/compute/rpcapi.py | 19 +++++++++++++++++-- nova/conf/database.py | 5 ++++- nova/tests/unit/compute/test_rpcapi.py | 26 ++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/nova/compute/rpcapi.py b/nova/compute/rpcapi.py index 2f0c080ece80..9584cdfe90bf 100644 --- a/nova/compute/rpcapi.py +++ b/nova/compute/rpcapi.py @@ -18,6 +18,7 @@ Client side of the compute RPC API. from oslo_log import log as logging import oslo_messaging as messaging from oslo_serialization import jsonutils +from oslo_utils import excutils import nova.conf from nova import context @@ -389,8 +390,22 @@ class ComputeAPI(object): # NOTE(danms): If we have a connection to the api database, # we should iterate all cells. If not, we must only look locally. if CONF.api_database.connection: - service_version = service_obj.get_minimum_version_all_cells( - context.get_admin_context(), ['nova-compute']) + try: + service_version = service_obj.get_minimum_version_all_cells( + context.get_admin_context(), ['nova-compute']) + except exception.DBNotAllowed: + # This most likely means we are in a nova-compute service + # configured with [upgrade_levels]/compute=auto and a + # connection to the API database. We should not be attempting + # to "get out" of our cell to look at the minimum versions of + # nova-compute services in other cells, so DBNotAllowed was + # raised. Log a user-friendly message and re-raise the error. + with excutils.save_and_reraise_exception(): + LOG.error('This service is configured for access to the ' + 'API database but is not allowed to directly ' + 'access the database. You should run this ' + 'service without the [api_database]/connection ' + 'config option.') else: service_version = objects.Service.get_minimum_version( context.get_admin_context(), 'nova-compute') diff --git a/nova/conf/database.py b/nova/conf/database.py index 9355becc8e72..c85689f89656 100644 --- a/nova/conf/database.py +++ b/nova/conf/database.py @@ -36,13 +36,16 @@ api_db_group = cfg.OptGroup('api_database', The *Nova API Database* is a separate database which is used for information which is used across *cells*. This database is mandatory since the Mitaka release (13.0.0). + +This group should **not** be configured for the ``nova-compute`` service. """) api_db_opts = [ # TODO(markus_z): This should probably have a required=True attribute cfg.StrOpt('connection', secret=True, - help=''), + # This help gets appended to the oslo.db help so prefix with a space. + help=' Do not set this for the ``nova-compute`` service.'), cfg.StrOpt('connection_parameters', default='', help=''), diff --git a/nova/tests/unit/compute/test_rpcapi.py b/nova/tests/unit/compute/test_rpcapi.py index 0905ac988543..2610cb2ea58a 100644 --- a/nova/tests/unit/compute/test_rpcapi.py +++ b/nova/tests/unit/compute/test_rpcapi.py @@ -615,3 +615,29 @@ class ComputeRpcAPITestCase(test.NoDBTestCase): compute_rpcapi.ComputeAPI() mock_allcells.assert_called_once_with(mock.ANY, ['nova-compute']) mock_minver.assert_not_called() + + @mock.patch('nova.compute.rpcapi.LOG.error') + @mock.patch('nova.objects.Service.get_minimum_version') + @mock.patch('nova.objects.service.get_minimum_version_all_cells', + side_effect=exception.DBNotAllowed(binary='nova-compute')) + def test_version_cap_all_cells_no_access(self, mock_allcells, mock_minver, + mock_log_error): + """Tests a scenario where nova-compute is configured with a connection + to the API database and fails trying to get the minium nova-compute + service version across all cells because nova-compute is configured to + not allow direct database access. + """ + self.flags(connection='sqlite:///', group='api_database') + compute_rpcapi.LAST_VERSION = None + self.assertRaises(exception.DBNotAllowed, + compute_rpcapi.ComputeAPI()._determine_version_cap, + mock.Mock()) + mock_allcells.assert_called_once_with(mock.ANY, ['nova-compute']) + mock_minver.assert_not_called() + # Make sure the expected error was logged. + mock_log_error.assert_called_once_with( + 'This service is configured for access to the ' + 'API database but is not allowed to directly ' + 'access the database. You should run this ' + 'service without the [api_database]/connection ' + 'config option.')