Merge "Address feedback from instance_list smart-cell behavior"

nova/compute/instance_list.py

@@ -100,13 +100,15 @@ def get_instance_objects_sorted(ctx, filters, limit, marker, expected_attrs,
     """Same as above, but return an InstanceList."""
     query_cell_subset = CONF.api.instance_list_per_project_cells
     # NOTE(danms): Replicated in part from instance_get_all_by_sort_filters(),
-    # where if we're not admin we're restricted to our context's project. Use
+    # where if we're not admin we're restricted to our context's project
-    # this to get a subset of cell mappings.
     if query_cell_subset and not ctx.is_admin:
+        # We are not admin, and configured to only query the subset of cells
+        # we could possibly have instances in.
         cell_mappings = objects.CellMappingList.get_by_project_id(
             ctx, ctx.project_id)
     else:
-        # If we're admin then query all cells
+        # Either we are admin, or configured to always hit all cells,
+        # so don't limit the list to a subset.
         cell_mappings = None
     columns_to_join = instance_obj._expected_cols(expected_attrs)
     instance_generator = get_instances_sorted(ctx, filters, limit, marker,

nova/compute/multi_cell_list.py

@@ -67,10 +67,11 @@ class CrossCellLister(object):
     """An implementation of a cross-cell efficient lister.
 
     This primarily provides a listing implementation for fetching
-    records from across all cells, paginated and sorted appropriately.
+    records from across multiple cells, paginated and sorted
-    The external interface is the get_records_sorted() method. You should
+    appropriately.  The external interface is the get_records_sorted()
-    implement this if you need to efficiently list your data type from
+    method. You should implement this if you need to efficiently list
-    cell databases.
+    your data type from cell databases.
+
     """
     def __init__(self, sort_ctx, cells=None):
         self.sort_ctx = sort_ctx

nova/conf/api.py

@@ -264,7 +264,8 @@ in which the tenant has mapped instances. This requires an additional
 to provide the result. If you have a small number of cells, or tenants
 are likely to have instances in all cells, then this should be
 False. If you have many cells, especially if you confine tenants to a
-small subset of those cells, this should be True.  """),
+small subset of those cells, this should be True.
+"""),
 ]
 
 # NOTE(edleafe): I would like to import the value directly from

nova/tests/functional/db/test_cell_mapping.py

@@ -146,7 +146,8 @@ class CellMappingListTestCase(test.NoDBTestCase):
                                          project_id='proj2')
             im.create()
 
-        # One mapping has no cell because it's a BuildRequest
+        # One mapping has no cell. This helps ensure that our query
+        # filters out any mappings that aren't tied to a cell.
         im = objects.InstanceMapping(context=ctxt,
                                      instance_uuid=uuidutils.generate_uuid(),
                                      cell_mapping=None,

nova/tests/unit/compute/test_instance_list.py

@@ -106,7 +106,7 @@ class TestInstanceList(test.NoDBTestCase):
         mock_gi.assert_called_once_with(admin_context, {}, None, None, [],
                                         None, None,
                                         cell_mappings=None)
-        self.assertFalse(mock_cm.called)
+        mock_cm.assert_not_called()
 
     @mock.patch('nova.objects.BuildRequestList.get_by_filters')
     @mock.patch('nova.compute.instance_list.get_instances_sorted')
@@ -136,4 +136,4 @@ class TestInstanceList(test.NoDBTestCase):
         mock_gi.assert_called_once_with(admin_context, {}, None, None, [],
                                         None, None,
                                         cell_mappings=None)
-        self.assertFalse(mock_cm.called)
+        mock_cm.assert_not_called()

releasenotes/notes/instance-list-limit-to-cells-config-f72701ac68444e95.yaml

@@ -0,0 +1,16 @@
+---
+other:
+  - |
+    The `[api]/instance_list_per_project_cells` configuration option
+    was added, which controls whether or not an instance list for
+    non-admin users checks all cell databases for results.  If
+    disabled (the default), then a list will always contact each cell
+    database looking for instances. This is appropriate if you have a
+    small number of cells, and/or if you spread instances from tenants
+    evenly across cells. If you confine tenants to a subset of cells,
+    then enabling this will result in fewer cell database calls, as
+    nova will only query the cells for which the tenant has instances
+    mapped. Doing this requires one more (fast) call to the API
+    database to get the relevant subset of cells, so if that is likely
+    to always be the same, disabling this feature will provide better
+    performance.