Minor improvements to cell commands

It adds a bit more verbosity to cell update command, so that it
is more obvious for an operator which values are being used as
transport URL or database connection URL for a cell.

Change-Id: Ie567ae7da4508a4b6f797d4bc77347c84702a74e

nova/cmd/manage.py

@@ -703,12 +703,19 @@ class ApiDbCommands(object):
 class CellV2Commands(object):
     """Commands for managing cells v2."""
 
-    def _validate_transport_url(self, transport_url):
-        transport_url = transport_url or CONF.transport_url
+    def _validate_transport_url(self, transport_url, warn_about_none=True):
         if not transport_url:
-            print('Must specify --transport-url if [DEFAULT]/transport_url '
-                  'is not set in the configuration file.')
-            return None
+            if not CONF.transport_url:
+                if warn_about_none:
+                    print(_(
+                        'Must specify --transport-url if '
+                        '[DEFAULT]/transport_url is not set in the '
+                        'configuration file.'))
+                return None
+            print(_('--transport-url not provided in the command line, '
+                    'using the value [DEFAULT]/transport_url from the '
+                    'configuration file'))
+            transport_url = CONF.transport_url
 
         try:
             messaging.TransportURL.parse(conf=CONF,
@@ -720,6 +727,22 @@ class CellV2Commands(object):
 
         return transport_url
 
+    def _validate_database_connection(
+            self, database_connection, warn_about_none=True):
+        if not database_connection:
+            if not CONF.database.connection:
+                if warn_about_none:
+                    print(_(
+                        'Must specify --database_connection if '
+                        '[database]/connection is not set in the '
+                        'configuration file.'))
+                return None
+            print(_('--database_connection not provided in the command line, '
+                    'using the value [database]/connection from the '
+                    'configuration file'))
+            return CONF.database.connection
+        return database_connection
+
     def _non_unique_transport_url_database_connection_checker(self, ctxt,
                             cell_mapping, transport_url, database_connection):
         for cell in objects.CellMappingList.get_all(ctxt):
@@ -1151,11 +1174,9 @@ class CellV2Commands(object):
         if not transport_url:
             return 1
 
-        database_connection = database_connection or CONF.database.connection
+        database_connection = self._validate_database_connection(
+            database_connection)
         if not database_connection:
-            print(_('Must specify --database_connection '
-                    'if [database]/connection is not set '
-                    'in the configuration file.'))
             return 1
         if (self._non_unique_transport_url_database_connection_checker(ctxt,
             None, transport_url, database_connection)):
@@ -1338,8 +1359,12 @@ class CellV2Commands(object):
         if name:
             cell_mapping.name = name
 
-        transport_url = transport_url or CONF.transport_url
-        db_connection = db_connection or CONF.database.connection
+        # Having empty transport_url and db_connection means leaving the
+        # existing values
+        transport_url = self._validate_transport_url(
+            transport_url, warn_about_none=False)
+        db_connection = self._validate_database_connection(
+            db_connection, warn_about_none=False)
 
         if (self._non_unique_transport_url_database_connection_checker(ctxt,
                 cell_mapping, transport_url, db_connection)):

nova/tests/unit/cmd/test_manage.py

@@ -640,8 +640,8 @@ Cell %s: 456
             mock_target_cell.assert_called_once_with(ctxt, 'map')
 
             db_sync_calls = [
-                    mock.call(4, context=cell_ctxt),
-                    mock.call(4)
+                mock.call(4, context=cell_ctxt),
+                mock.call(4)
             ]
             mock_db_sync.assert_has_calls(db_sync_calls)
 
@@ -1689,7 +1689,7 @@ class CellV2CommandsTestCase(test.NoDBTestCase):
 
         status = self.commands.create_cell(verbose=True)
         self.assertEqual(0, status)
-        cell1_uuid = self.output.getvalue().strip()
+        cell1_uuid = self.output.getvalue().split('\n')[-2].strip()
         cell1 = objects.CellMapping.get_by_uuid(ctxt, cell1_uuid)
         self.assertIsNone(cell1.name)
         self.assertEqual(settings['database_connection'],
@@ -2033,7 +2033,10 @@ class CellV2CommandsTestCase(test.NoDBTestCase):
         expected_db_connection = CONF.database.connection or 'fake:///db'
         self.assertEqual(expected_db_connection, cm.database_connection)
         output = self.output.getvalue().strip()
-        self.assertEqual('', output)
+        lines = output.split('\n')
+        self.assertIn('using the value [DEFAULT]/transport_url', lines[0])
+        self.assertIn('using the value [database]/connection', lines[1])
+        self.assertEqual(2, len(lines))
 
     def test_update_cell_disable_and_enable(self):
         ctxt = context.get_admin_context()
@@ -2045,8 +2048,8 @@ class CellV2CommandsTestCase(test.NoDBTestCase):
                                                       disable=True,
                                                       enable=True))
         output = self.output.getvalue().strip()
-        self.assertEqual('Cell cannot be disabled and enabled at the same '
-                         'time.', output)
+        self.assertIn('Cell cannot be disabled and enabled at the same '
+                      'time.', output)
 
     def test_update_cell_disable_cell0(self):
         ctxt = context.get_admin_context()
@@ -2056,7 +2059,7 @@ class CellV2CommandsTestCase(test.NoDBTestCase):
                             database_connection='fake:///db').create()
         self.assertEqual(5, self.commands.update_cell(uuid0, disable=True))
         output = self.output.getvalue().strip()
-        self.assertEqual('Cell0 cannot be disabled.', output)
+        self.assertIn('Cell0 cannot be disabled.', output)
 
     def test_update_cell_disable_success(self):
         ctxt = context.get_admin_context()
@@ -2071,7 +2074,10 @@ class CellV2CommandsTestCase(test.NoDBTestCase):
         cm = objects.CellMapping.get_by_uuid(ctxt, uuid)
         self.assertTrue(cm.disabled)
         output = self.output.getvalue().strip()
-        self.assertEqual('', output)
+        lines = output.split('\n')
+        self.assertIn('using the value [DEFAULT]/transport_url', lines[0])
+        self.assertIn('using the value [database]/connection', lines[1])
+        self.assertEqual(2, len(lines))
 
     def test_update_cell_enable_success(self):
         ctxt = context.get_admin_context()
@@ -2087,7 +2093,10 @@ class CellV2CommandsTestCase(test.NoDBTestCase):
         cm = objects.CellMapping.get_by_uuid(ctxt, uuid)
         self.assertFalse(cm.disabled)
         output = self.output.getvalue().strip()
-        self.assertEqual('', output)
+        lines = output.split('\n')
+        self.assertIn('using the value [DEFAULT]/transport_url', lines[0])
+        self.assertIn('using the value [database]/connection', lines[1])
+        self.assertEqual(2, len(lines))
 
     def test_update_cell_disable_already_disabled(self):
         ctxt = context.get_admin_context()