Add --by-service to discover_hosts

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.

Change-Id: Ie9f064cb9caf6dcba2414acb24d12b825df45fab
Closes-Bug: #1755602
This commit is contained in:
Dan Smith 2018-03-13 14:42:09 -07:00
parent eb637b9de7
commit 005a66d7e0
6 changed files with 142 additions and 14 deletions

View File

@ -199,7 +199,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
hosts which are not currently mapped. If a host is already mapped nothing
@ -208,7 +208,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]``
Lists the v2 cells in the deployment. By default only the cell name and

View File

@ -1349,7 +1349,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
@ -1362,7 +1366,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)

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

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))

View File

@ -1626,6 +1626,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)

View File

@ -0,0 +1,8 @@
---
features:
- |
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.