From 8da6831e6375d013f0235a542b6b91580a43f61f Mon Sep 17 00:00:00 2001 From: Surya Seetharaman Date: Thu, 8 Mar 2018 19:06:00 +0100 Subject: [PATCH] Make archive_deleted_rows handle a missing CONF.api_database.connection This change https://review.openstack.org/#/c/515034/ (added in queens) makes the archive_deleted_rows CLI remove instance mappings and request specs from the API database if there are instances archived from the main nova/cell database. However for this to work, the api database connection should be set in the config file. So in the case that the API database is not configured in the config file being used to run the CLI, we should gracefully handle the condition and and stop archiving thus prompting the user to set the api_db config and try the archival operation again. This patch fixes the graceful handling. Change-Id: I0c7b802a453aa423c7273ab724ce78eac0cfed4c Closes-Bug: #1753833 --- doc/source/cli/nova-manage.rst | 3 ++- nova/cmd/manage.py | 17 ++++++++++++++-- nova/tests/unit/test_nova_manage.py | 31 +++++++++++++++++++++++++---- 3 files changed, 44 insertions(+), 7 deletions(-) diff --git a/doc/source/cli/nova-manage.rst b/doc/source/cli/nova-manage.rst index 36f6fe189d60..3d0a4c1d7497 100644 --- a/doc/source/cli/nova-manage.rst +++ b/doc/source/cli/nova-manage.rst @@ -64,7 +64,8 @@ Nova Database Move deleted rows from production tables to shadow tables. Note that the corresponding rows in the instance_mappings and request_specs tables of the - API database are purged when instance records are archived. Specifying + API database are purged when instance records are archived and thus, + CONF.api_database.connection is required in the config file. Specifying --verbose will print the results of the archive operation for any tables that were changed. Specifying --until-complete will make the command run continuously until all deleted rows are archived. Use the --max_rows option, diff --git a/nova/cmd/manage.py b/nova/cmd/manage.py index 9a5975541d17..be399a0b7adb 100644 --- a/nova/cmd/manage.py +++ b/nova/cmd/manage.py @@ -495,7 +495,8 @@ Error: %s""") % six.text_type(e)) """Move deleted rows from production tables to shadow tables. Returns 0 if nothing was archived, 1 if some number of rows were - archived, 2 if max_rows is invalid. If automating, this should be + archived, 2 if max_rows is invalid, 3 if no connection could be + established to the API DB. If automating, this should be run continuously while the result is 1, stopping at 0. """ max_rows = int(max_rows) @@ -507,6 +508,19 @@ Error: %s""") % six.text_type(e)) {'max_value': db.MAX_INT}) return 2 + ctxt = context.get_admin_context() + try: + # NOTE(tssurya): This check has been added to validate if the API + # DB is reachable or not as this is essential for purging the + # instance_mappings and request_specs of the deleted instances. + objects.CellMappingList.get_all(ctxt) + except db_exc.CantStartEngineError: + print(_('Failed to connect to API DB so aborting this archival ' + 'attempt. Please check your config file to make sure that ' + 'CONF.api_database.connection is set and run this ' + 'command again.')) + return 3 + table_to_rows_archived = {} deleted_instance_uuids = [] if until_complete and verbose: @@ -525,7 +539,6 @@ Error: %s""") % six.text_type(e)) if deleted_instance_uuids: table_to_rows_archived.setdefault('instance_mappings', 0) table_to_rows_archived.setdefault('request_specs', 0) - ctxt = context.get_admin_context() deleted_mappings = objects.InstanceMappingList.destroy_bulk( ctxt, deleted_instance_uuids) table_to_rows_archived['instance_mappings'] += deleted_mappings diff --git a/nova/tests/unit/test_nova_manage.py b/nova/tests/unit/test_nova_manage.py index e4e14c7cf4b1..f355a77a1b97 100644 --- a/nova/tests/unit/test_nova_manage.py +++ b/nova/tests/unit/test_nova_manage.py @@ -389,7 +389,9 @@ class DBCommandsTestCase(test.NoDBTestCase): @mock.patch.object(db, 'archive_deleted_rows', return_value=(dict(instances=10, consoles=5), list())) - def _test_archive_deleted_rows(self, mock_db_archive, verbose=False): + @mock.patch.object(objects.CellMappingList, 'get_all') + def _test_archive_deleted_rows(self, mock_get_all, mock_db_archive, + verbose=False): result = self.commands.archive_deleted_rows(20, verbose=verbose) mock_db_archive.assert_called_once_with(20) output = self.output.getvalue() @@ -416,7 +418,9 @@ class DBCommandsTestCase(test.NoDBTestCase): self._test_archive_deleted_rows(verbose=True) @mock.patch.object(db, 'archive_deleted_rows') - def test_archive_deleted_rows_until_complete(self, mock_db_archive, + @mock.patch.object(objects.CellMappingList, 'get_all') + def test_archive_deleted_rows_until_complete(self, mock_get_all, + mock_db_archive, verbose=False): mock_db_archive.side_effect = [ ({'instances': 10, 'instance_extra': 5}, list()), @@ -449,7 +453,9 @@ Archiving.....complete @mock.patch('nova.db.sqlalchemy.api.purge_shadow_tables') @mock.patch.object(db, 'archive_deleted_rows') - def test_archive_deleted_rows_until_stopped(self, mock_db_archive, + @mock.patch.object(objects.CellMappingList, 'get_all') + def test_archive_deleted_rows_until_stopped(self, mock_get_all, + mock_db_archive, mock_db_purge, verbose=True): mock_db_archive.side_effect = [ @@ -486,7 +492,9 @@ Rows were archived, running purge... self.test_archive_deleted_rows_until_stopped(verbose=False) @mock.patch.object(db, 'archive_deleted_rows', return_value=({}, [])) - def test_archive_deleted_rows_verbose_no_results(self, mock_db_archive): + @mock.patch.object(objects.CellMappingList, 'get_all') + def test_archive_deleted_rows_verbose_no_results(self, mock_get_all, + mock_db_archive): result = self.commands.archive_deleted_rows(20, verbose=True, purge=True) mock_db_archive.assert_called_once_with(20) @@ -543,6 +551,21 @@ Rows were archived, running purge... else: self.assertEqual(0, len(output)) + @mock.patch.object(objects.CellMappingList, 'get_all', + side_effect=db_exc.CantStartEngineError) + def test_archive_deleted_rows_without_api_connection_configured(self, + mock_get_all): + result = self.commands.archive_deleted_rows(20, verbose=True) + mock_get_all.assert_called_once() + output = self.output.getvalue() + expected = '''\ +Failed to connect to API DB so aborting this archival attempt. \ +Please check your config file to make sure that CONF.api_database.connection \ +is set and run this command again. +''' + self.assertEqual(expected, output) + self.assertEqual(3, result) + @mock.patch('nova.db.sqlalchemy.api.purge_shadow_tables') def test_purge_all(self, mock_purge): mock_purge.return_value = 1