Remove 'nova-manage cell' commands

These are no longer necessary with the removal of cells v1. A check for
cells v1 in 'nova-manage cell_v2 simple_cell_setup' is also removed,
meaning this can no longer return the '2' exit code.

Part of blueprint remove-cells-v1

Change-Id: I8c2bfb31224300bc639d5089c4dfb62143d04b7f
Signed-off-by: Stephen Finucane <[email protected]>

Author: stephenfin

doc/source/cli/nova-manage.rst

@@ -184,13 +184,11 @@ Nova Cells v2
 ~~~~~~~~~~~~~
 
 ``nova-manage cell_v2 simple_cell_setup [--transport-url <transport_url>]``
-    Setup a fresh cells v2 environment; this should not be used if you
-    currently have a cells v1 environment. If a transport_url is not
+    Setup a fresh cells v2 environment. If a ``transport_url`` is not
     specified, it will use the one defined by ``[DEFAULT]/transport_url``
     in the configuration file. Returns 0 if setup is completed
     (or has already been done), 1 if no hosts are reporting (and cannot be
-    mapped), 1 if the transport url is missing or invalid, and 2 if run in a
-    cells v1 environment.
+    mapped) and 1 if the transport url is missing or invalid.
 
 ``nova-manage cell_v2 map_cell0 [--database_connection <database_connection>]``
     Create a cell mapping to the database connection for the cell0 database.

nova/cmd/manage.py

@@ -891,159 +891,6 @@ def version(self):
         print(migration.db_version(database='api'))
 
 
-class CellCommands(object):
-    """Commands for managing cells v1 functionality."""
-
-    # TODO(stephenfin): Remove this when cells v1 is removed
-    description = ('DEPRECATED: The cell commands, which configure cells v1 '
-                   'functionality, are deprecated as Cells v1 itself has '
-                   'been deprecated. They will be removed in an upcoming '
-                   'release.')
-
-    @staticmethod
-    def _parse_server_string(server_str):
-        """Parses the given server_string and returns a tuple of host and port.
-        If it's not a combination of host part and port, the port element is an
-        empty string. If the input is invalid expression, return a tuple of two
-        empty strings.
-        """
-        try:
-            # First of all, exclude pure IPv6 address (w/o port).
-            if netaddr.valid_ipv6(server_str):
-                return (server_str, '')
-
-            # Next, check if this is IPv6 address with a port number
-            # combination.
-            if server_str.find("]:") != -1:
-                (address, port) = server_str.replace('[', '', 1).split(']:')
-                return (address, port)
-
-            # Third, check if this is a combination of an address and a port
-            if server_str.find(':') == -1:
-                return (server_str, '')
-
-            # This must be a combination of an address and a port
-            (address, port) = server_str.split(':')
-            return (address, port)
-
-        except (ValueError, netaddr.AddrFormatError):
-            print('Invalid server_string: %s' % server_str)
-            return ('', '')
-
-    def _create_transport_hosts(self, username, password,
-                                broker_hosts=None, hostname=None, port=None):
-        """Returns a list of oslo.messaging.TransportHost objects."""
-        transport_hosts = []
-        # Either broker-hosts or hostname should be set
-        if broker_hosts:
-            hosts = broker_hosts.split(',')
-            for host in hosts:
-                host = host.strip()
-                broker_hostname, broker_port = self._parse_server_string(host)
-                if not broker_port:
-                    msg = _('Invalid broker_hosts value: %s. It should be'
-                            ' in hostname:port format') % host
-                    raise ValueError(msg)
-                try:
-                    broker_port = int(broker_port)
-                except ValueError:
-                    msg = _('Invalid port value: %s. It should be '
-                             'an integer') % broker_port
-                    raise ValueError(msg)
-                transport_hosts.append(
-                               messaging.TransportHost(
-                                   hostname=broker_hostname,
-                                   port=broker_port,
-                                   username=username,
-                                   password=password))
-        else:
-            try:
-                port = int(port)
-            except ValueError:
-                msg = _("Invalid port value: %s. Should be an integer") % port
-                raise ValueError(msg)
-            transport_hosts.append(
-                           messaging.TransportHost(
-                               hostname=hostname,
-                               port=port,
-                               username=username,
-                               password=password))
-        return transport_hosts
-
-    @args('--name', metavar='<name>', help='Name for the new cell')
-    @args('--cell_type', metavar='<parent|api|child|compute>',
-         help='Whether the cell is parent/api or child/compute')
-    @args('--username', metavar='<username>',
-         help='Username for the message broker in this cell')
-    @args('--password', metavar='<password>',
-         help='Password for the message broker in this cell')
-    @args('--broker_hosts', metavar='<broker_hosts>',
-         help='Comma separated list of message brokers in this cell. '
-              'Each Broker is specified as hostname:port with both '
-              'mandatory. This option overrides the --hostname '
-              'and --port options (if provided). ')
-    @args('--hostname', metavar='<hostname>',
-         help='Address of the message broker in this cell')
-    @args('--port', metavar='<number>',
-         help='Port number of the message broker in this cell')
-    @args('--virtual_host', metavar='<virtual_host>',
-         help='The virtual host of the message broker in this cell')
-    @args('--woffset', metavar='<float>')
-    @args('--wscale', metavar='<float>')
-    def create(self, name, cell_type='child', username=None, broker_hosts=None,
-               password=None, hostname=None, port=None, virtual_host=None,
-               woffset=None, wscale=None):
-
-        if cell_type not in ['parent', 'child', 'api', 'compute']:
-            print("Error: cell type must be 'parent'/'api' or "
-                "'child'/'compute'")
-            return 2
-
-        # Set up the transport URL
-        transport_hosts = self._create_transport_hosts(
-                                                 username, password,
-                                                 broker_hosts, hostname,
-                                                 port)
-        transport_url = rpc.get_transport_url()
-        transport_url.hosts.extend(transport_hosts)
-        transport_url.virtual_host = virtual_host
-
-        is_parent = False
-        if cell_type in ['api', 'parent']:
-            is_parent = True
-        values = {'name': name,
-                  'is_parent': is_parent,
-                  'transport_url': urlparse.unquote(str(transport_url)),
-                  'weight_offset': float(woffset),
-                  'weight_scale': float(wscale)}
-        ctxt = context.get_admin_context()
-        db.cell_create(ctxt, values)
-
-    @args('--cell_name', metavar='<cell_name>',
-          help='Name of the cell to delete')
-    def delete(self, cell_name):
-        ctxt = context.get_admin_context()
-        db.cell_delete(ctxt, cell_name)
-
-    def list(self):
-        ctxt = context.get_admin_context()
-        cells = db.cell_get_all(ctxt)
-        fmt = "%3s  %-10s  %-6s  %-10s  %-15s  %-5s  %-10s"
-        print(fmt % ('Id', 'Name', 'Type', 'Username', 'Hostname',
-                'Port', 'VHost'))
-        print(fmt % ('-' * 3, '-' * 10, '-' * 6, '-' * 10, '-' * 15,
-                '-' * 5, '-' * 10))
-        for cell in cells:
-            url = rpc.get_transport_url(cell.transport_url)
-            host = url.hosts[0] if url.hosts else messaging.TransportHost()
-            print(fmt % (cell.id, cell.name,
-                    'parent' if cell.is_parent else 'child',
-                    host.username, host.hostname,
-                    host.port, url.virtual_host))
-        print(fmt % ('-' * 3, '-' * 10, '-' * 6, '-' * 10, '-' * 15,
-                '-' * 5, '-' * 10))
-
-
 class CellV2Commands(object):
     """Commands for managing cells v2."""
 
@@ -1085,14 +932,10 @@ def simple_cell_setup(self, transport_url=None):
         """Simple cellsv2 setup.
 
         This simplified command is for use by existing non-cells users to
-        configure the default environment. If you are using CellsV1, this
-        will not work for you. Returns 0 if setup is completed (or has
-        already been done), 1 if no hosts are reporting (and this cannot
-        be mapped) and 2 if run in a CellsV1 environment.
+        configure the default environment. Returns 0 if setup is completed (or
+        has already been done) and 1 if no hosts are reporting (and this cannot
+        be mapped).
         """
-        if CONF.cells.enable:
-            print('CellsV1 users cannot use this simplified setup command')
-            return 2
         transport_url = self._validate_transport_url(transport_url)
         if not transport_url:
             return 1
@@ -2332,7 +2175,6 @@ def sync_aggregates(self, verbose=False):
 
 CATEGORIES = {
     'api_db': ApiDbCommands,
-    'cell': CellCommands,
     'cell_v2': CellV2Commands,
     'db': DbCommands,
     'floating': FloatingIpCommands,

nova/tests/unit/test_nova_manage.py

@@ -879,149 +879,6 @@ def test_sync(self, sqla_sync):
                                           version=4, database='api')
 
 
-class CellCommandsTestCase(test.NoDBTestCase):
-    def setUp(self):
-        super(CellCommandsTestCase, self).setUp()
-        self.output = StringIO()
-        self.useFixture(fixtures.MonkeyPatch('sys.stdout', self.output))
-        self.commands = manage.CellCommands()
-
-    def test_create_transport_hosts_multiple(self):
-        """Test the _create_transport_hosts method
-        when broker_hosts is set.
-        """
-        brokers = "127.0.0.1:5672,127.0.0.2:5671"
-        thosts = self.commands._create_transport_hosts(
-                                           'guest', 'devstack',
-                                           broker_hosts=brokers)
-        self.assertEqual(2, len(thosts))
-        self.assertEqual('127.0.0.1', thosts[0].hostname)
-        self.assertEqual(5672, thosts[0].port)
-        self.assertEqual('127.0.0.2', thosts[1].hostname)
-        self.assertEqual(5671, thosts[1].port)
-
-    def test_create_transport_hosts_single(self):
-        """Test the _create_transport_hosts method when hostname is passed."""
-        thosts = self.commands._create_transport_hosts('guest', 'devstack',
-                                                       hostname='127.0.0.1',
-                                                       port=80)
-        self.assertEqual(1, len(thosts))
-        self.assertEqual('127.0.0.1', thosts[0].hostname)
-        self.assertEqual(80, thosts[0].port)
-
-    def test_create_transport_hosts_single_broker(self):
-        """Test the _create_transport_hosts method for single broker_hosts."""
-        thosts = self.commands._create_transport_hosts(
-                                              'guest', 'devstack',
-                                              broker_hosts='127.0.0.1:5672')
-        self.assertEqual(1, len(thosts))
-        self.assertEqual('127.0.0.1', thosts[0].hostname)
-        self.assertEqual(5672, thosts[0].port)
-
-    def test_create_transport_hosts_both(self):
-        """Test the _create_transport_hosts method when both broker_hosts
-        and hostname/port are passed.
-        """
-        thosts = self.commands._create_transport_hosts(
-                                              'guest', 'devstack',
-                                              broker_hosts='127.0.0.1:5672',
-                                              hostname='127.0.0.2', port=80)
-        self.assertEqual(1, len(thosts))
-        self.assertEqual('127.0.0.1', thosts[0].hostname)
-        self.assertEqual(5672, thosts[0].port)
-
-    def test_create_transport_hosts_wrong_val(self):
-        """Test the _create_transport_hosts method when broker_hosts
-        is wrongly specified
-        """
-        self.assertRaises(ValueError,
-                          self.commands._create_transport_hosts,
-                          'guest', 'devstack',
-                          broker_hosts='127.0.0.1:5672,127.0.0.1')
-
-    def test_create_transport_hosts_wrong_port_val(self):
-        """Test the _create_transport_hosts method when port in
-        broker_hosts is wrongly specified
-        """
-        self.assertRaises(ValueError,
-                          self.commands._create_transport_hosts,
-                          'guest', 'devstack',
-                          broker_hosts='127.0.0.1:')
-
-    def test_create_transport_hosts_wrong_port_arg(self):
-        """Test the _create_transport_hosts method when port
-        argument is wrongly specified
-        """
-        self.assertRaises(ValueError,
-                          self.commands._create_transport_hosts,
-                          'guest', 'devstack',
-                          hostname='127.0.0.1', port='ab')
-
-    @mock.patch.object(context, 'get_admin_context')
-    @mock.patch.object(db, 'cell_create')
-    def test_create_broker_hosts(self, mock_db_cell_create, mock_ctxt):
-        """Test the create function when broker_hosts is
-        passed
-        """
-        cell_tp_url = "fake://guest:[email protected]:5432"
-        cell_tp_url += ",guest:[email protected]:9999/"
-        ctxt = mock.sentinel
-        mock_ctxt.return_value = mock.sentinel
-        self.commands.create("test",
-                             broker_hosts='127.0.0.1:5432,127.0.0.2:9999',
-                             woffset=0, wscale=0,
-                             username="guest", password="devstack")
-        exp_values = {'name': "test",
-                      'is_parent': False,
-                      'transport_url': cell_tp_url,
-                      'weight_offset': 0.0,
-                      'weight_scale': 0.0}
-        mock_db_cell_create.assert_called_once_with(ctxt, exp_values)
-
-    @mock.patch.object(context, 'get_admin_context')
-    @mock.patch.object(db, 'cell_create')
-    def test_create_broker_hosts_with_url_decoding_fix(self,
-                                                       mock_db_cell_create,
-                                                       mock_ctxt):
-        """Test the create function when broker_hosts is
-        passed
-        """
-        cell_tp_url = "fake://the=user:[email protected]:5432/"
-        ctxt = mock.sentinel
-        mock_ctxt.return_value = mock.sentinel
-        self.commands.create("test",
-                             broker_hosts='127.0.0.1:5432',
-                             woffset=0, wscale=0,
-                             username="the=user",
-                             password="the=password")
-        exp_values = {'name': "test",
-                      'is_parent': False,
-                      'transport_url': cell_tp_url,
-                      'weight_offset': 0.0,
-                      'weight_scale': 0.0}
-        mock_db_cell_create.assert_called_once_with(ctxt, exp_values)
-
-    @mock.patch.object(context, 'get_admin_context')
-    @mock.patch.object(db, 'cell_create')
-    def test_create_hostname(self, mock_db_cell_create, mock_ctxt):
-        """Test the create function when hostname and port is
-        passed
-        """
-        cell_tp_url = "fake://guest:[email protected]:9999/"
-        ctxt = mock.sentinel
-        mock_ctxt.return_value = mock.sentinel
-        self.commands.create("test",
-                             hostname='127.0.0.1', port="9999",
-                             woffset=0, wscale=0,
-                             username="guest", password="devstack")
-        exp_values = {'name': "test",
-                      'is_parent': False,
-                      'transport_url': cell_tp_url,
-                      'weight_offset': 0.0,
-                      'weight_scale': 0.0}
-        mock_db_cell_create.assert_called_once_with(ctxt, exp_values)
-
-
 @ddt.ddt
 class CellV2CommandsTestCase(test.NoDBTestCase):
     USES_DB_SELF = True
@@ -1560,10 +1417,6 @@ def test_simple_command_multiple(self):
         self._test_migrate_simple_command()
         self._test_migrate_simple_command()
 
-    def test_simple_command_cellsv1(self):
-        self.flags(enable=True, group='cells')
-        self.assertEqual(2, self.commands.simple_cell_setup('foo'))
-
     def test_instance_verify_no_mapping(self):
         r = self.commands.verify_instance(uuidsentinel.instance)
         self.assertEqual(1, r)

releasenotes/notes/remove-cells-v1-055028c270d06680.yaml

@@ -2,10 +2,14 @@
 upgrade:
   - |
     The *cells v1* feature has been deprecated since the 16.0.0 Pike release
-    and has now been removed. The ``nova-cells`` service has been removed. The
-    *cells v1* specific REST APIs have been removed along with their related
-    policy rules. Calling these APIs will now result in a ``410 (Gone)`` error
-    response.
+    and has now been removed. The ``nova-cells`` service and ``nova-manage
+    cells`` commands have been removed, while the ``nova-manage cell_v2
+    simple_cell_setup`` command will no longer check if cells v1 is enabled and
+    therefore can no longer exit with ``2``.
+
+    The *cells v1* specific REST APIs have
+    been removed along with their related policy rules. Calling these APIs will
+    now result in a ``410 (Gone)`` error response.
 
     * ``GET /os-cells``
     * ``POST /os-cells``