Make simple_cell_setup fully idempotent

Previously this command would treat the presence of a cell0 mapping
as indication that cellsv2 is fully setup. This could be a false
indication for a variety of situations, including having set up a
cell0 mapping manually, as well as failing to sync the database and
thus not continuing on to host/instance mapping.

This patch makes us continue on, even if the mapping is present,
and also even if the sync of the cell0 database fails.

Change-Id: Ic05e1a998d7807d58fc35637742059d61f7669e9
Closes-Bug: #1649341
(cherry picked from commit 34761e678f)

nova/cmd/manage.py

@@ -1222,11 +1222,17 @@ class CellV2Commands(object):
         try:
             cell0_mapping = self.map_cell0()
         except db_exc.DBDuplicateEntry:
-            print('Already setup, nothing to do.')
-            return 0
+            print(_('Cell0 is already setup'))
+            cell0_mapping = objects.CellMapping.get_by_uuid(
+                ctxt, objects.CellMapping.CELL0_UUID)
+
         # Run migrations so cell0 is usable
         with context.target_cell(ctxt, cell0_mapping):
-            migration.db_sync(None, context=ctxt)
+            try:
+                migration.db_sync(None, context=ctxt)
+            except db_exc.DBError as ex:
+                print(_('Unable to sync cell0 schema: %s') % ex)
+
         cell_uuid = self._map_cell_and_hosts(transport_url)
         if cell_uuid is None:
             # There are no compute hosts which means no cell_mapping was

nova/tests/unit/test_nova_manage.py

@@ -17,6 +17,7 @@ import sys
 
 import fixtures
 import mock
+from oslo_db import exception as db_exc
 from oslo_utils import uuidutils
 from six.moves import StringIO
 
@@ -1124,7 +1125,7 @@ class CellV2CommandsTestCase(test.TestCase):
         self.assertEqual('fake://netloc/nova_api_cell0',
                          cell_mapping.database_connection)
 
-    def _test_migrate_simple_command(self, first_call=True):
+    def _test_migrate_simple_command(self, cell0_sync_fail=False):
         ctxt = context.RequestContext()
         CONF.set_default('connection',
                          'fake://netloc/nova_api',
@@ -1152,12 +1153,11 @@ class CellV2CommandsTestCase(test.TestCase):
         @mock.patch.object(uuidutils, 'generate_uuid',
                 return_value=cell_uuid)
         def _test(mock_gen_uuid, mock_db_sync):
+            if cell0_sync_fail:
+                mock_db_sync.side_effect = db_exc.DBError
             result = self.commands.simple_cell_setup(transport_url)
-            if first_call:
-                mock_db_sync.assert_called_once_with(
-                    None, context=test.MatchType(context.RequestContext))
-            else:
-                mock_db_sync.assert_not_called()
+            mock_db_sync.assert_called_once_with(
+                None, context=test.MatchType(context.RequestContext))
             return result
 
         r = _test()
@@ -1183,11 +1183,15 @@ class CellV2CommandsTestCase(test.TestCase):
     def test_simple_command_single(self):
         self._test_migrate_simple_command()
 
+    def test_simple_command_cell0_fail(self):
+        # Make sure that if db_sync fails, we still do all the other
+        # bits
+        self._test_migrate_simple_command(cell0_sync_fail=True)
+
     def test_simple_command_multiple(self):
+        # Make sure that the command is idempotent
+        self._test_migrate_simple_command()
         self._test_migrate_simple_command()
-        with mock.patch.object(self.commands, '_map_cell_and_hosts') as m:
-            self._test_migrate_simple_command(first_call=False)
-            self.assertFalse(m.called)
 
     def test_simple_command_cellsv1(self):
         self.flags(enable=True, group='cells')