From 7954b2714eb767148dfb3fc318ff0141df549487 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Thu, 4 Apr 2019 13:36:57 +0100 Subject: [PATCH] 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 --- doc/source/cli/nova-manage.rst | 6 +- nova/cmd/manage.py | 164 +----------------- nova/tests/unit/test_nova_manage.py | 147 ---------------- .../remove-cells-v1-055028c270d06680.yaml | 12 +- 4 files changed, 13 insertions(+), 316 deletions(-) diff --git a/doc/source/cli/nova-manage.rst b/doc/source/cli/nova-manage.rst index 47c30ca0a93b..1989ec172dd8 100644 --- a/doc/source/cli/nova-manage.rst +++ b/doc/source/cli/nova-manage.rst @@ -184,13 +184,11 @@ Nova Cells v2 ~~~~~~~~~~~~~ ``nova-manage cell_v2 simple_cell_setup [--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 ]`` Create a cell mapping to the database connection for the cell0 database. diff --git a/nova/cmd/manage.py b/nova/cmd/manage.py index 9c2e796ebd85..a2cebb8e1efd 100644 --- a/nova/cmd/manage.py +++ b/nova/cmd/manage.py @@ -891,159 +891,6 @@ class ApiDbCommands(object): 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='', help='Name for the new cell') - @args('--cell_type', metavar='', - help='Whether the cell is parent/api or child/compute') - @args('--username', metavar='', - help='Username for the message broker in this cell') - @args('--password', metavar='', - help='Password for the message broker in this cell') - @args('--broker_hosts', metavar='', - 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='', - help='Address of the message broker in this cell') - @args('--port', metavar='', - help='Port number of the message broker in this cell') - @args('--virtual_host', metavar='', - help='The virtual host of the message broker in this cell') - @args('--woffset', metavar='') - @args('--wscale', metavar='') - 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='', - 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 @@ class CellV2Commands(object): """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 @@ class PlacementCommands(object): CATEGORIES = { 'api_db': ApiDbCommands, - 'cell': CellCommands, 'cell_v2': CellV2Commands, 'db': DbCommands, 'floating': FloatingIpCommands, diff --git a/nova/tests/unit/test_nova_manage.py b/nova/tests/unit/test_nova_manage.py index a437f2682078..a00e4b45bd44 100644 --- a/nova/tests/unit/test_nova_manage.py +++ b/nova/tests/unit/test_nova_manage.py @@ -879,149 +879,6 @@ class ApiDbCommandsTestCase(test.NoDBTestCase): 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:devstack@127.0.0.1:5432" - cell_tp_url += ",guest:devstack@127.0.0.2: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:the=password@127.0.0.1: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:devstack@127.0.0.1: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 @@ class CellV2CommandsTestCase(test.NoDBTestCase): 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) diff --git a/releasenotes/notes/remove-cells-v1-055028c270d06680.yaml b/releasenotes/notes/remove-cells-v1-055028c270d06680.yaml index d19915b7227f..4dbe4503cc83 100644 --- a/releasenotes/notes/remove-cells-v1-055028c270d06680.yaml +++ b/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``