openstack/nova
Code Issues Proposed changes

Add --by-service to discover_hosts

Browse Source 
This allows us to discover and map compute hosts by service instead of
by compute node, which will solve a major deployment ordering problem for
people using ironic. This also allows closing a really nasty race when
doing HA of nova-compute/ironic.

Conflicts:
	doc/source/cli/nova-manage.rst

NOTE(danms): Conflicts in nova-manage.rst due to queens not having recent
             changes to the man page.

Change-Id: Ie9f064cb9caf6dcba2414acb24d12b825df45fab
Closes-Bug: #1755602
(cherry picked from commit 005a66d7e0)
This commit is contained in:
Dan Smith 2018-03-13 14:42:09 -07:00
parent f41c24f21a
commit dd90b5dbf3
6 changed files with 142 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
doc/source/cli/nova-manage.rst
View File

@ -194,7 +194,7 @@ Nova Cells v2
database connection was missing, and 2 if a cell is already using that
transport url and database connection combination.
``nova-manage cell_v2 discover_hosts [--cell_uuid <cell_uuid>] [--verbose] [--strict]``
``nova-manage cell_v2 discover_hosts [--cell_uuid <cell_uuid>] [--verbose] [--strict] [--by-service]``
Searches cells, or a single cell, and maps found hosts. This command will
check the database for each cell (or a single one if passed in) and map any
@ -204,7 +204,11 @@ Nova Cells v2
there and the API will not list the new hosts). If the strict option is
provided the command will only be considered successful if an unmapped host
is discovered (exit code 0). Any other case is considered a failure (exit
code 1).
code 1). If --by-service is specified, this command will look in the
appropriate cell(s) for any nova-compute services and ensure there are host
mappings for them. This is less efficient and is only necessary when using
compute drivers that may manage zero or more actual compute nodes at any
given time (currently only ironic).
``nova-manage cell_v2 list_cells [--verbose]``

9
nova/cmd/manage.py
View File

@ -1279,7 +1279,11 @@ class CellV2Commands(object):
help=_('Considered successful (exit code 0) only when an unmapped '
'host is discovered. Any other outcome will be considered a '
'failure (exit code 1).'))
def discover_hosts(self, cell_uuid=None, verbose=False, strict=False):
@args('--by-service', action='store_true', default=False,
dest='by_service',
help=_('Discover hosts by service instead of compute node'))
def discover_hosts(self, cell_uuid=None, verbose=False, strict=False,
by_service=False):
"""Searches cells, or a single cell, and maps found hosts.
When a new host is added to a deployment it will add a service entry
@ -1292,7 +1296,8 @@ class CellV2Commands(object):
print(msg)
ctxt = context.RequestContext()
hosts = host_mapping_obj.discover_hosts(ctxt, cell_uuid, status_fn)
hosts = host_mapping_obj.discover_hosts(ctxt, cell_uuid, status_fn,
by_service)
# discover_hosts will return an empty list if no hosts are discovered
if strict:
return int(not hosts)

52
nova/objects/host_mapping.py
View File

@ -175,7 +175,7 @@ class HostMappingList(base.ObjectListBase, base.NovaObject):
return base.obj_make_list(context, cls(), HostMapping, db_mappings)
def _check_and_create_host_mappings(ctxt, cm, compute_nodes, status_fn):
def _check_and_create_node_host_mappings(ctxt, cm, compute_nodes, status_fn):
host_mappings = []
for compute in compute_nodes:
status_fn(_("Checking host mapping for compute host "
@ -197,7 +197,41 @@ def _check_and_create_host_mappings(ctxt, cm, compute_nodes, status_fn):
return host_mappings
def discover_hosts(ctxt, cell_uuid=None, status_fn=None):
def _check_and_create_service_host_mappings(ctxt, cm, services, status_fn):
host_mappings = []
for service in services:
try:
HostMapping.get_by_host(ctxt, service.host)
except exception.HostMappingNotFound:
status_fn(_('Creating host mapping for service %(srv)s') %
{'srv': service.host})
host_mapping = HostMapping(
ctxt, host=service.host,
cell_mapping=cm)
host_mapping.create()
host_mappings.append(host_mapping)
return host_mappings
def _check_and_create_host_mappings(ctxt, cm, status_fn, by_service):
from nova import objects
if by_service:
services = objects.ServiceList.get_by_binary(
ctxt, 'nova-compute', include_disabled=True)
added_hm = _check_and_create_service_host_mappings(ctxt, cm,
services,
status_fn)
else:
compute_nodes = objects.ComputeNodeList.get_all_by_not_mapped(
ctxt, 1)
added_hm = _check_and_create_node_host_mappings(ctxt, cm,
compute_nodes,
status_fn)
return added_hm
def discover_hosts(ctxt, cell_uuid=None, status_fn=None, by_service=False):
# TODO(alaski): If this is not run on a host configured to use the API
# database most of the lookups below will fail and may not provide a
# great error message. Add a check which will raise a useful error
@ -220,21 +254,19 @@ def discover_hosts(ctxt, cell_uuid=None, status_fn=None):
status_fn(_('Skipping cell0 since it does not contain hosts.'))
continue
if 'name' in cm and cm.name:
status_fn(_("Getting compute nodes from cell '%(name)s': "
status_fn(_("Getting computes from cell '%(name)s': "
"%(uuid)s") % {'name': cm.name,
'uuid': cm.uuid})
else:
status_fn(_("Getting compute nodes from cell: %(uuid)s") %
status_fn(_("Getting computes from cell: %(uuid)s") %
{'uuid': cm.uuid})
with context.target_cell(ctxt, cm) as cctxt:
compute_nodes = objects.ComputeNodeList.get_all_by_not_mapped(
cctxt, 1)
added_hm = _check_and_create_host_mappings(cctxt, cm, status_fn,
by_service)
status_fn(_('Found %(num)s unmapped computes in cell: %(uuid)s') %
{'num': len(compute_nodes),
{'num': len(added_hm),
'uuid': cm.uuid})
added_hm = _check_and_create_host_mappings(cctxt, cm,
compute_nodes,
status_fn)
host_mappings.extend(added_hm)
return host_mappings

70
nova/tests/unit/objects/test_host_mapping.py
View File

@ -240,3 +240,73 @@ class TestHostMappingDiscovery(test.NoDBTestCase):
self.assertTrue(mock_hm_create.called)
self.assertEqual(['d'],
[hm.host for hm in hms])
@mock.patch('nova.objects.CellMappingList.get_all')
@mock.patch('nova.objects.HostMapping.get_by_host')
@mock.patch('nova.objects.HostMapping.create')
@mock.patch('nova.objects.ServiceList.get_by_binary')
def test_discover_services(self, mock_srv, mock_hm_create,
mock_hm_get, mock_cm):
mock_cm.return_value = [
objects.CellMapping(uuid=uuids.cell1),
objects.CellMapping(uuid=uuids.cell2),
]
mock_srv.side_effect = [
[objects.Service(host='host1'),
objects.Service(host='host2')],
[objects.Service(host='host3')],
]
def fake_get_host_mapping(ctxt, host):
if host == 'host2':
return
else:
raise exception.HostMappingNotFound(name=host)
mock_hm_get.side_effect = fake_get_host_mapping
ctxt = context.get_admin_context()
mappings = host_mapping.discover_hosts(ctxt, by_service=True)
self.assertEqual(2, len(mappings))
self.assertEqual(['host1', 'host3'],
sorted([m.host for m in mappings]))
@mock.patch('nova.objects.CellMapping.get_by_uuid')
@mock.patch('nova.objects.HostMapping.get_by_host')
@mock.patch('nova.objects.HostMapping.create')
@mock.patch('nova.objects.ServiceList.get_by_binary')
def test_discover_services_one_cell(self, mock_srv, mock_hm_create,
mock_hm_get, mock_cm):
mock_cm.return_value = objects.CellMapping(uuid=uuids.cell1)
mock_srv.return_value = [
objects.Service(host='host1'),
objects.Service(host='host2'),
]
def fake_get_host_mapping(ctxt, host):
if host == 'host2':
return
else:
raise exception.HostMappingNotFound(name=host)
mock_hm_get.side_effect = fake_get_host_mapping
lines = []
def fake_status(msg):
lines.append(msg)
ctxt = context.get_admin_context()
mappings = host_mapping.discover_hosts(ctxt, cell_uuid=uuids.cell1,
status_fn=fake_status,
by_service=True)
self.assertEqual(1, len(mappings))
self.assertEqual(['host1'],
sorted([m.host for m in mappings]))
expected = """\
Getting computes from cell: %(cell)s
Creating host mapping for service host1
Found 1 unmapped computes in cell: %(cell)s""" % {'cell': uuids.cell1}
self.assertEqual(expected, '\n'.join(lines))

9
nova/tests/unit/test_nova_manage.py
View File

@ -1534,6 +1534,15 @@ class CellV2CommandsTestCase(test.NoDBTestCase):
# Check the return when strict=False
self.assertIsNone(self.commands.discover_hosts())
@mock.patch('nova.objects.host_mapping.discover_hosts')
def test_discover_hosts_by_service(self, mock_discover_hosts):
mock_discover_hosts.return_value = ['fake']
ret = self.commands.discover_hosts(by_service=True, strict=True)
self.assertEqual(0, ret)
mock_discover_hosts.assert_called_once_with(mock.ANY, None,
mock.ANY,
True)
def test_validate_transport_url_in_conf(self):
from_conf = 'fake://user:pass@host:port/'
self.flags(transport_url=from_conf)

8
releasenotes/notes/discover-hosts-by-service-06ee20365b895127.yaml Normal file
View File

@ -0,0 +1,8 @@
---
fixes:
- |
The nova-manage discover_hosts command now has a ``--by-service`` option which
allows discovering hosts in a cell purely by the presence of a nova-compute
binary. At this point, there is no need to use this unless you're using ironic,
as it is less efficient. However, if you are using ironic, this allows discovery
and mapping of hosts even when no ironic nodes are present.