openstack/nova
Code Issues Proposed changes

Merge "Make get_instance_objects_sorted() be smart about cells"

Browse Source
This commit is contained in:
Zuul 2018-05-10 22:31:30 +00:00 committed by Gerrit Code Review
commit 305bc05fc5
7 changed files with 123 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
nova/compute/instance_list.py
View File

@ -13,6 +13,7 @@
import copy
from nova.compute import multi_cell_list
import nova.conf
from nova import context
from nova import db
from nova import exception
@ -20,6 +21,9 @@ from nova import objects
from nova.objects import instance as instance_obj
CONF = nova.conf.CONF
class InstanceSortContext(multi_cell_list.RecordSortContext):
def __init__(self, sort_keys, sort_dirs):
if not sort_keys:
@ -41,9 +45,9 @@ class InstanceSortContext(multi_cell_list.RecordSortContext):
class InstanceLister(multi_cell_list.CrossCellLister):
def __init__(self, sort_keys, sort_dirs):
def __init__(self, sort_keys, sort_dirs, cells=None):
super(InstanceLister, self).__init__(
InstanceSortContext(sort_keys, sort_dirs))
InstanceSortContext(sort_keys, sort_dirs), cells=cells)
@property
def marker_identifier(self):
@ -85,18 +89,31 @@ class InstanceLister(multi_cell_list.CrossCellLister):
# NOTE(danms): These methods are here for legacy glue reasons. We should not
# replicate these for every data type we implement.
def get_instances_sorted(ctx, filters, limit, marker, columns_to_join,
sort_keys, sort_dirs):
return InstanceLister(sort_keys, sort_dirs).get_records_sorted(
sort_keys, sort_dirs, cell_mappings=None):
return InstanceLister(sort_keys, sort_dirs,
cells=cell_mappings).get_records_sorted(
ctx, filters, limit, marker, columns_to_join=columns_to_join)
def get_instance_objects_sorted(ctx, filters, limit, marker, expected_attrs,
sort_keys, sort_dirs):
"""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
# this to get a subset of cell mappings.
if query_cell_subset and not ctx.is_admin:
cell_mappings = objects.CellMappingList.get_by_project_id(
ctx, ctx.project_id)
else:
# If we're admin then query all cells
cell_mappings = None
columns_to_join = instance_obj._expected_cols(expected_attrs)
instance_generator = get_instances_sorted(ctx, filters, limit, marker,
columns_to_join, sort_keys,
sort_dirs)
sort_dirs,
cell_mappings=cell_mappings)
if 'fault' in expected_attrs:
# We join fault above, so we need to make sure we don't ask
# make_instance_list to do it again for us

9
nova/compute/multi_cell_list.py
View File

@ -72,8 +72,9 @@ class CrossCellLister(object):
implement this if you need to efficiently list your data type from
cell databases.
"""
def __init__(self, sort_ctx):
def __init__(self, sort_ctx, cells=None):
self.sort_ctx = sort_ctx
self.cells = cells
@property
@abc.abstractmethod
@ -245,7 +246,11 @@ class CrossCellLister(object):
# that here gracefully. The below routine will provide sentinels
# to indicate that, which will crash the merge below, but we don't
# handle this anywhere yet anyway.
results = context.scatter_gather_all_cells(ctx, do_query)
if self.cells:
results = context.scatter_gather_cells(ctx, self.cells, 60,
do_query)
else:
results = context.scatter_gather_all_cells(ctx, do_query)
# If a limit was provided, it was passed to the per-cell query
# routines. That means we have NUM_CELLS * limit items across

11
nova/conf/api.py
View File

@ -254,6 +254,17 @@ Possible values:
* Any string, including an empty string (the default).
"""),
cfg.BoolOpt("instance_list_per_project_cells",
default=False,
help="""
When enabled, this will cause the API to only query cell databases
in which the tenant has mapped instances. This requires an additional
(fast) query in the API database before each list, but also
(potentially) limits the number of cell databases that must be queried
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. """),
]
# NOTE(edleafe): I would like to import the value directly from

3
nova/tests/fixtures.py
View File

@ -291,6 +291,9 @@ class SingleCellSimple(fixtures.Fixture):
self.useFixture(fixtures.MonkeyPatch(
'nova.objects.CellMappingList._get_all_from_db',
self._fake_cell_list))
self.useFixture(fixtures.MonkeyPatch(
'nova.objects.CellMappingList._get_by_project_id_from_db',
self._fake_cell_list))
self.useFixture(fixtures.MonkeyPatch(
'nova.objects.CellMapping._get_by_uuid_from_db',
self._fake_cell_get))

17
nova/tests/functional/compute/test_instance_list.py
View File

@ -427,6 +427,23 @@ class InstanceListTestCase(test.TestCase):
faults = [inst['fault'] for inst in insts]
self.assertEqual(expected_no_fault, faults.count(None))
def test_instance_list_minimal_cells(self):
"""Get a list of instances with a subset of cell mappings."""
last_cell = self.cells[-1]
with context.target_cell(self.context, last_cell) as cctxt:
last_cell_instances = db.instance_get_all(cctxt)
last_cell_uuids = [inst['uuid'] for inst in last_cell_instances]
instances = list(
instance_list.get_instances_sorted(self.context, {},
None, None, [],
['uuid'], ['asc'],
cell_mappings=self.cells[:-1]))
found_uuids = [inst['hostname'] for inst in instances]
had_uuids = [inst['hostname'] for inst in self.instances
if inst['uuid'] not in last_cell_uuids]
self.assertEqual(sorted(had_uuids), sorted(found_uuids))
class TestInstanceListObjects(test.TestCase):
def setUp(self):

3
nova/tests/unit/api/openstack/fakes.py
View File

@ -373,6 +373,9 @@ def fake_instance_get_all_by_filters(num_servers=5, **kwargs):
if 'sort_dirs' in kwargs:
kwargs.pop('sort_dirs')
if 'cell_mappings' in kwargs:
kwargs.pop('cell_mappings')
for i in range(num_servers):
uuid = get_fake_uuid(i)
server = stub_instance(id=i + 1, uuid=uuid,

60
nova/tests/unit/compute/test_instance_list.py
View File

@ -13,6 +13,7 @@
import mock
from nova.compute import instance_list
from nova import context as nova_context
from nova import objects
from nova import test
from nova.tests import fixtures
@ -77,3 +78,62 @@ class TestInstanceList(test.NoDBTestCase):
insts_two = [inst['hostname'] for inst in insts]
self.assertEqual(insts_one, insts_two)
@mock.patch('nova.objects.BuildRequestList.get_by_filters')
@mock.patch('nova.compute.instance_list.get_instances_sorted')
@mock.patch('nova.objects.CellMappingList.get_by_project_id')
def test_user_gets_subset_of_cells(self, mock_cm, mock_gi, mock_br):
self.flags(instance_list_per_project_cells=True, group='api')
mock_gi.return_value = []
mock_br.return_value = []
user_context = nova_context.RequestContext('fake', 'fake')
instance_list.get_instance_objects_sorted(
user_context, {}, None, None, [], None, None)
mock_gi.assert_called_once_with(user_context, {}, None, None, [],
None, None,
cell_mappings=mock_cm.return_value)
@mock.patch('nova.objects.BuildRequestList.get_by_filters')
@mock.patch('nova.compute.instance_list.get_instances_sorted')
@mock.patch('nova.objects.CellMappingList.get_by_project_id')
def test_admin_gets_all_cells(self, mock_cm, mock_gi, mock_br):
mock_gi.return_value = []
mock_br.return_value = []
admin_context = nova_context.RequestContext('fake', 'fake',
is_admin=True)
instance_list.get_instance_objects_sorted(
admin_context, {}, None, None, [], None, None)
mock_gi.assert_called_once_with(admin_context, {}, None, None, [],
None, None,
cell_mappings=None)
self.assertFalse(mock_cm.called)
@mock.patch('nova.objects.BuildRequestList.get_by_filters')
@mock.patch('nova.compute.instance_list.get_instances_sorted')
@mock.patch('nova.objects.CellMappingList.get_by_project_id')
def test_user_gets_all_cells(self, mock_cm, mock_gi, mock_br):
self.flags(instance_list_per_project_cells=False, group='api')
mock_gi.return_value = []
mock_br.return_value = []
user_context = nova_context.RequestContext('fake', 'fake')
instance_list.get_instance_objects_sorted(
user_context, {}, None, None, [], None, None)
mock_gi.assert_called_once_with(user_context, {}, None, None, [],
None, None,
cell_mappings=None)
@mock.patch('nova.objects.BuildRequestList.get_by_filters')
@mock.patch('nova.compute.instance_list.get_instances_sorted')
@mock.patch('nova.objects.CellMappingList.get_by_project_id')
def test_admin_gets_all_cells_anyway(self, mock_cm, mock_gi, mock_br):
self.flags(instance_list_per_project_cells=True, group='api')
mock_gi.return_value = []
mock_br.return_value = []
admin_context = nova_context.RequestContext('fake', 'fake',
is_admin=True)
instance_list.get_instance_objects_sorted(
admin_context, {}, None, None, [], None, None)
mock_gi.assert_called_once_with(admin_context, {}, None, None, [],
None, None,
cell_mappings=None)
self.assertFalse(mock_cm.called)