From aaae38d1aa227a732b9aa8c93d77a909d0324696 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. Conflicts: nova/tests/unit/test_nova_manage.py NOTE(tssurya): The conflict is due to not having changes I6f87cf03d49be6bfad2c5e6f0c8accf0fab4e6ee and Ibd824a77b32cbceb60973a89a93ce09fe6d1050d in Queens. Change-Id: I0c7b802a453aa423c7273ab724ce78eac0cfed4c Closes-Bug: #1753833 (cherry picked from commit 8da6831e6375d013f0235a542b6b91580a43f61f) --- 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 1a25d5cc944d..5352d3563edd 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 c71120a47d63..34bbaf5ea57c 100644 --- a/nova/cmd/manage.py +++ b/nova/cmd/manage.py @@ -497,7 +497,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) @@ -509,6 +510,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: @@ -527,7 +541,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 c351cfab3e48..b53a8a6344af 100644 --- a/nova/tests/unit/test_nova_manage.py +++ b/nova/tests/unit/test_nova_manage.py @@ -388,7 +388,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() @@ -415,7 +417,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()), @@ -447,7 +451,9 @@ Archiving.....complete self.test_archive_deleted_rows_until_complete(verbose=False) @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, verbose=True): mock_db_archive.side_effect = [ ({'instances': 10, 'instance_extra': 5}, list()), @@ -479,7 +485,9 @@ Archiving.....stopped 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) mock_db_archive.assert_called_once_with(20) output = self.output.getvalue() @@ -534,6 +542,21 @@ Archiving.....stopped 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.object(migration, 'db_null_instance_uuid_scan', return_value={'foo': 0}) def test_null_instance_uuid_scan_no_records_found(self, mock_scan):