Make discover_hosts only query for unmapped ComputeNode records

This should be much more efficient and capable of being run at a higher
frequency on more deployments.

Related to blueprint discover-hosts-faster
Change-Id: I8cbc27291c5f9cbeaef01340572974fdbd8d2656

nova/conf/scheduler.py

@@ -133,12 +133,11 @@ This value controls how often (in seconds) the scheduler should attempt
 to discover new hosts that have been added to cells. If negative (the
 default), no automatic discovery will occur.
 
-Small deployments may want this periodic task enabled, as surveying the
+Deployments where compute nodes come and go frequently may want this
-cells for new hosts is likely to be lightweight enough to not cause undue
+enabled, where others may prefer to manually discover hosts when one
-burdon to the scheduler. However, larger clouds (and those that are not
+is added to avoid any overhead from constantly checking. If enabled,
-adding hosts regularly) will likely want to disable this automatic
+every time this runs, we will select any unmapped hosts out of each
-behavior and instead use the `nova-manage cell_v2 discover_hosts` command
+cell database on every run.
-when hosts have been added to a cell.
 """),
 ]
 

nova/objects/host_mapping.py

@@ -167,6 +167,28 @@ 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):
+    host_mappings = []
+    for compute in compute_nodes:
+        status_fn(_("Checking host mapping for compute host "
+                    "'%(host)s': %(uuid)s") %
+                  {'host': compute.host, 'uuid': compute.uuid})
+        try:
+            HostMapping.get_by_host(ctxt, compute.host)
+        except exception.HostMappingNotFound:
+            status_fn(_("Creating host mapping for compute host "
+                        "'%(host)s': %(uuid)s") %
+                      {'host': compute.host, 'uuid': compute.uuid})
+            host_mapping = HostMapping(
+                ctxt, host=compute.host,
+                cell_mapping=cm)
+            host_mapping.create()
+            host_mappings.append(host_mapping)
+            compute.mapped = 1
+            compute.save()
+    return host_mappings
+
+
 def discover_hosts(ctxt, cell_uuid=None, status_fn=None):
     # 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
@@ -197,23 +219,13 @@ def discover_hosts(ctxt, cell_uuid=None, status_fn=None):
             status_fn(_("Getting compute nodes from cell: %(uuid)s") %
                       {'uuid': cm.uuid})
         with context.target_cell(ctxt, cm):
-            compute_nodes = objects.ComputeNodeList.get_all(ctxt)
+            compute_nodes = objects.ComputeNodeList.get_all_by_not_mapped(
-            status_fn(_('Found %(num)s computes in cell: %(uuid)s') %
+                ctxt, 1)
+            status_fn(_('Found %(num)s unmapped computes in cell: %(uuid)s') %
                       {'num': len(compute_nodes),
                        'uuid': cm.uuid})
-        for compute in compute_nodes:
+            added_hm = _check_and_create_host_mappings(ctxt, cm, compute_nodes,
-            status_fn(_("Checking host mapping for compute host "
+                                                       status_fn)
-                        "'%(host)s': %(uuid)s") %
+            host_mappings.extend(added_hm)
-                      {'host': compute.host, 'uuid': compute.uuid})
+
-            try:
-                objects.HostMapping.get_by_host(ctxt, compute.host)
-            except exception.HostMappingNotFound:
-                status_fn(_("Creating host mapping for compute host "
-                            "'%(host)s': %(uuid)s") %
-                          {'host': compute.host, 'uuid': compute.uuid})
-                host_mapping = objects.HostMapping(
-                    ctxt, host=compute.host,
-                    cell_mapping=cm)
-                host_mapping.create()
-                host_mappings.append(host_mapping)
     return host_mappings

nova/tests/functional/db/test_host_mapping.py

@@ -10,14 +10,17 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import mock
 from oslo_utils import uuidutils
 
 from nova import context
 from nova import exception
+from nova import objects
 from nova.objects import cell_mapping
 from nova.objects import host_mapping
 from nova import test
 from nova.tests import fixtures
+from nova.tests import uuidsentinel as uuids
 
 
 sample_mapping = {'host': 'fake-host',
@@ -134,3 +137,81 @@ class HostMappingTestCase(test.NoDBTestCase):
             self.context, db_host_mapping['cell_id'])
         self.assertEqual(1, len(host_mapping_list))
         self.assertEqual(db_host_mapping['id'], host_mapping_list[0].id)
+
+
+class HostMappingDiscoveryTest(test.TestCase):
+    def _setup_cells(self):
+        ctxt = context.get_admin_context()
+        self.celldbs = fixtures.CellDatabases()
+        cells = []
+        for uuid in (uuids.cell1, uuids.cell2, uuids.cell3):
+            cm = objects.CellMapping(context=ctxt,
+                                     uuid=uuid,
+                                     database_connection=uuid,
+                                     transport_url='fake://')
+            cm.create()
+            cells.append(cm)
+            self.celldbs.add_cell_database(uuid)
+        self.useFixture(self.celldbs)
+
+        for cell in cells:
+            for i in (1, 2, 3):
+                # Make one host in each cell unmapped
+                mapped = 0 if i == 2 else 1
+
+                host = 'host-%s-%i' % (cell.uuid, i)
+                if mapped:
+                    hm = objects.HostMapping(context=ctxt,
+                                             cell_mapping=cell,
+                                             host=host)
+                    hm.create()
+
+                with context.target_cell(ctxt, cell):
+                    cn = objects.ComputeNode(
+                        context=ctxt, vcpus=1, memory_mb=1, local_gb=1,
+                        vcpus_used=0, memory_mb_used=0, local_gb_used=0,
+                        hypervisor_type='danvm', hypervisor_version='1',
+                        cpu_info='foo',
+                        cpu_allocation_ratio=1.0,
+                        ram_allocation_ratio=1.0,
+                        disk_allocation_ratio=1.0,
+                        mapped=mapped, host=host)
+                    cn.create()
+
+    def test_discover_hosts(self):
+        status = lambda m: None
+
+        ctxt = context.get_admin_context()
+
+        # NOTE(danms): Three cells, one unmapped host per cell
+        mappings = host_mapping.discover_hosts(ctxt, status_fn=status)
+        self.assertEqual(3, len(mappings))
+
+        # NOTE(danms): All hosts should be mapped now, so we should do
+        # no lookups for them
+        with mock.patch('nova.objects.HostMapping.get_by_host') as mock_gbh:
+            mappings = host_mapping.discover_hosts(ctxt, status_fn=status)
+            self.assertFalse(mock_gbh.called)
+        self.assertEqual(0, len(mappings))
+
+    def test_discover_hosts_one_cell(self):
+        status = lambda m: None
+
+        ctxt = context.get_admin_context()
+        cells = objects.CellMappingList.get_all(ctxt)
+
+        # NOTE(danms): One cell, one unmapped host per cell
+        mappings = host_mapping.discover_hosts(ctxt, cells[1].uuid,
+                                               status_fn=status)
+        self.assertEqual(1, len(mappings))
+
+        # NOTE(danms): Three cells, two with one more unmapped host
+        mappings = host_mapping.discover_hosts(ctxt, status_fn=status)
+        self.assertEqual(2, len(mappings))
+
+        # NOTE(danms): All hosts should be mapped now, so we should do
+        # no lookups for them
+        with mock.patch('nova.objects.HostMapping.get_by_host') as mock_gbh:
+            mappings = host_mapping.discover_hosts(ctxt, status_fn=status)
+            self.assertFalse(mock_gbh.called)
+        self.assertEqual(0, len(mappings))

nova/tests/unit/objects/test_host_mapping.py

@@ -154,7 +154,7 @@ class TestHostMappingDiscovery(test.NoDBTestCase):
     @mock.patch('nova.objects.CellMappingList.get_all')
     @mock.patch('nova.objects.HostMapping.create')
     @mock.patch('nova.objects.HostMapping.get_by_host')
-    @mock.patch('nova.objects.ComputeNodeList.get_all')
+    @mock.patch('nova.objects.ComputeNodeList.get_all_by_not_mapped')
     def test_discover_hosts_all(self, mock_cn_get, mock_hm_get, mock_hm_create,
                                 mock_cm):
         def _hm_get(context, host):
@@ -174,7 +174,9 @@ class TestHostMappingDiscovery(test.NoDBTestCase):
                                              uuid=uuids.cm2)]
         mock_cm.return_value = cell_mappings
         ctxt = context.get_admin_context()
-        hms = host_mapping.discover_hosts(ctxt)
+        with mock.patch('nova.objects.ComputeNode.save') as mock_save:
+            hms = host_mapping.discover_hosts(ctxt)
+            mock_save.assert_has_calls([mock.call(), mock.call()])
         self.assertEqual(2, len(hms))
         self.assertTrue(mock_hm_create.called)
         self.assertEqual(['d', 'e'],
@@ -183,7 +185,7 @@ class TestHostMappingDiscovery(test.NoDBTestCase):
     @mock.patch('nova.objects.CellMapping.get_by_uuid')
     @mock.patch('nova.objects.HostMapping.create')
     @mock.patch('nova.objects.HostMapping.get_by_host')
-    @mock.patch('nova.objects.ComputeNodeList.get_all')
+    @mock.patch('nova.objects.ComputeNodeList.get_all_by_not_mapped')
     def test_discover_hosts_one(self, mock_cn_get, mock_hm_get, mock_hm_create,
                                 mock_cm):
         def _hm_get(context, host):
@@ -202,7 +204,9 @@ class TestHostMappingDiscovery(test.NoDBTestCase):
         mock_cm.return_value = objects.CellMapping(name='foo',
                                                    uuid=uuids.cm1)
         ctxt = context.get_admin_context()
-        hms = host_mapping.discover_hosts(ctxt, uuids.cm1)
+        with mock.patch('nova.objects.ComputeNode.save') as mock_save:
+            hms = host_mapping.discover_hosts(ctxt, uuids.cm1)
+            mock_save.assert_called_once_with()
         self.assertEqual(1, len(hms))
         self.assertTrue(mock_hm_create.called)
         self.assertEqual(['d'],

releasenotes/notes/discover-hosts-periodic-is-more-efficient-6c55b606a7831750.yaml

@@ -0,0 +1,6 @@
+---
+features:
+  - The `discover_hosts_in_cells_interval` periodic task in the scheduler is now
+    more efficient in that it can specifically query unmapped compute nodes from
+    the cell databases instead of having to query them all and compare against
+    existing host mappings.