Add missing unit tests for FilterScheduler._get_all_host_states

This addresses a review comment from change
I8666e0af3f057314f6b06939a108411b8a88d64b for some
missing tests. This code is tightly coupled between
how the SchedulerManager calls FilterScheduler.select_destinations
and how the provider_summaries variable is interpreted differently
when it's None or {}, and is also tightly coupled to how
HostManager._get_computes_for_cells considers compute_uuids being
None or not when grabbing all compute nodes from a cell. Because
of this coupling and complexity, the NOTE in the method is also
updated for clarity.

Change-Id: I3e994d4ad23bf1dd2e46f5d69ff7fdd1a6afdb0e

nova/scheduler/filter_scheduler.py

@@ -321,10 +321,14 @@ class FilterScheduler(driver.Scheduler):
 
     def _get_all_host_states(self, context, spec_obj, provider_summaries):
         """Template method, so a subclass can implement caching."""
-        # NOTE(jaypipes): None is treated differently from an empty dict. We
-        # pass None when we want to grab all compute nodes (for instance, when
-        # using the caching scheduler. We pass an empty dict when the Placement
-        # API found no providers that match the requested constraints.
+        # NOTE(jaypipes): provider_summaries being None is treated differently
+        # from an empty dict. provider_summaries is None when we want to grab
+        # all compute nodes, for instance when using the caching scheduler.
+        # The provider_summaries variable will be an empty dict when the
+        # Placement API found no providers that match the requested
+        # constraints, which in turn makes compute_uuids an empty list and
+        # get_host_states_by_uuids will return an empty tuple also, which will
+        # eventually result in a NoValidHost error.
         compute_uuids = None
         if provider_summaries is not None:
             compute_uuids = list(provider_summaries.keys())

nova/scheduler/host_manager.py

@@ -596,6 +596,14 @@ class HostManager(object):
     def _get_computes_for_cells(self, context, cells, compute_uuids=None):
         """Get a tuple of compute node and service information.
 
+        :param context: request context
+        :param cells: list of CellMapping objects
+        :param compute_uuids: list of ComputeNode UUIDs. If this is None, all
+            compute nodes from each specified cell will be returned, otherwise
+            only the ComputeNode objects with a UUID in the list of UUIDs in
+            any given cell is returned. If this is an empty list, the returned
+            compute_nodes tuple item will be an empty dict.
+
         Returns a tuple (compute_nodes, services) where:
          - compute_nodes is cell-uuid keyed dict of compute node lists
          - services is a dict of services indexed by hostname

nova/tests/unit/scheduler/test_filter_scheduler.py

@@ -599,3 +599,29 @@ class FilterSchedulerTestCase(test_scheduler.SchedulerTestCase):
                 mock.call(self.context, 'scheduler.select_destinations.end',
                  dict(request_spec=expected))]
             self.assertEqual(expected, mock_info.call_args_list)
+
+    def test_get_all_host_states_provider_summaries_is_none(self):
+        """Tests that HostManager.get_host_states_by_uuids is called with
+        compute_uuids being None when the incoming provider_summaries is None.
+        """
+        with mock.patch.object(self.driver.host_manager,
+                               'get_host_states_by_uuids') as get_host_states:
+            self.driver._get_all_host_states(
+                mock.sentinel.ctxt, mock.sentinel.spec_obj, None)
+        # Make sure get_host_states_by_uuids was called with
+        # compute_uuids being None.
+        get_host_states.assert_called_once_with(
+            mock.sentinel.ctxt, None, mock.sentinel.spec_obj)
+
+    def test_get_all_host_states_provider_summaries_is_empty(self):
+        """Tests that HostManager.get_host_states_by_uuids is called with
+        compute_uuids being [] when the incoming provider_summaries is {}.
+        """
+        with mock.patch.object(self.driver.host_manager,
+                               'get_host_states_by_uuids') as get_host_states:
+            self.driver._get_all_host_states(
+                mock.sentinel.ctxt, mock.sentinel.spec_obj, {})
+        # Make sure get_host_states_by_uuids was called with
+        # compute_uuids being [].
+        get_host_states.assert_called_once_with(
+            mock.sentinel.ctxt, [], mock.sentinel.spec_obj)