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

doc/source/cli/nova-manage.rst

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

nova/cmd/manage.py

@@ -1349,7 +1349,11 @@ def say(string):
           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 @@ def status_fn(msg):
                 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)

nova/objects/host_mapping.py

@@ -175,7 +175,7 @@ def get_all(cls, context):
         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

nova/tests/unit/objects/test_host_mapping.py

@@ -240,3 +240,73 @@ def _hm_get(context, host):
         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))

nova/tests/unit/test_nova_manage.py

@@ -1626,6 +1626,15 @@ def test_discover_hosts_strict(self, mock_discover_hosts):
         # 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)

releasenotes/notes/discover-hosts-by-service-06ee20365b895127.yaml

@@ -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.