Merge "Plumbing for allowing the all-tenants filter with down cells"

This commit is contained in:
Zuul 2019-02-12 02:01:20 +00:00 committed by Gerrit Code Review
commit 5a09c81af3
5 changed files with 78 additions and 33 deletions

View File

@ -113,6 +113,9 @@ class ServersController(wsgi.Controller):
search_opts = {}
search_opts.update(req.GET)
# NOTE(tssurya): Will be enabled after bumping the microversion.
cell_down_support = False
context = req.environ['nova.context']
remove_invalid_options(context, search_opts,
self._get_server_search_options(req))
@ -207,7 +210,11 @@ class ServersController(wsgi.Controller):
all_tenants = common.is_all_tenants(search_opts)
# use the boolean from here on out so remove the entry from search_opts
# if it's present
# if it's present.
# NOTE(tssurya): In case we support handling down cells
# we need to know further down the stack whether the 'all_tenants'
# filter was passed with the true value or not, so we pass the flag
# further down the stack.
search_opts.pop('all_tenants', None)
elevated = None
@ -250,7 +257,8 @@ class ServersController(wsgi.Controller):
instance_list = self.compute_api.get_all(elevated or context,
search_opts=search_opts, limit=limit, marker=marker,
expected_attrs=expected_attrs, sort_keys=sort_keys,
sort_dirs=sort_dirs, cell_down_support=False)
sort_dirs=sort_dirs, cell_down_support=cell_down_support,
all_tenants=all_tenants)
except exception.MarkerNotFound:
msg = _('marker [%s] not found') % marker
raise exc.HTTPBadRequest(explanation=msg)

View File

@ -2353,7 +2353,7 @@ class API(base.Base):
:param context: RequestContext
:param down_cell_uuids: A list of cell UUIDs that did not respond
:param project: A project ID to filter mappings
:param project: A project ID to filter mappings, or None
:param limit: A numeric limit on the number of results, or None
:returns: An InstanceList() of partial Instance() objects
"""
@ -2516,7 +2516,7 @@ class API(base.Base):
def get_all(self, context, search_opts=None, limit=None, marker=None,
expected_attrs=None, sort_keys=None, sort_dirs=None,
cell_down_support=False):
cell_down_support=False, all_tenants=False):
"""Get all instances filtered by one of the given parameters.
If there is no filter and the context is an admin, it will retrieve
@ -2536,6 +2536,7 @@ class API(base.Base):
construct if the relevant cell is
down. If False, instances from
unreachable cells will be omitted.
:param all_tenants: True if the "all_tenants" filter was passed.
"""
if search_opts is None:
@ -2698,6 +2699,11 @@ class API(base.Base):
# that didn't return, so generate and prefix those to the actual
# results.
project = search_opts.get('project_id', context.project_id)
if all_tenants:
# NOTE(tssurya): The only scenario where project has to be None
# is when using "all_tenants" in which case we do not want
# the query to be restricted based on the project_id.
project = None
limit = (orig_limit - len(instances)) if limit else limit
return (self._generate_minimal_construct_for_down_cells(context,
down_cell_uuids, project, limit) + instances)

View File

@ -713,7 +713,7 @@ class ServersControllerTest(ControllerTest):
req.environ['nova.context'], expected_attrs=[], limit=1000,
marker=None, search_opts={'deleted': False, 'project_id': 'fake'},
sort_dirs=['desc'], sort_keys=['created_at'],
cell_down_support=False)
cell_down_support=False, all_tenants=False)
def test_get_server_list_with_reservation_id(self):
req = self.req('/fake/servers?reservation_id=foo')
@ -806,7 +806,7 @@ class ServersControllerTest(ControllerTest):
limit=1000, marker=None,
search_opts={'deleted': False, 'project_id': 'fake'},
sort_dirs=['desc'], sort_keys=['created_at'],
cell_down_support=False)
cell_down_support=False, all_tenants=False)
def test_get_server_details_with_bad_name(self):
req = self.req('/fake/servers/detail?name=%2Binstance')
@ -925,7 +925,7 @@ class ServersControllerTest(ControllerTest):
self.mock_get_all.assert_called_once_with(
mock.ANY, search_opts=mock.ANY, limit=mock.ANY, marker=mock.ANY,
expected_attrs=mock.ANY, sort_keys=[], sort_dirs=[],
cell_down_support=False)
cell_down_support=False, all_tenants=False)
def test_get_servers_ignore_sort_key_only_one_dir(self):
req = self.req(
@ -934,7 +934,7 @@ class ServersControllerTest(ControllerTest):
self.mock_get_all.assert_called_once_with(
mock.ANY, search_opts=mock.ANY, limit=mock.ANY, marker=mock.ANY,
expected_attrs=mock.ANY, sort_keys=['user_id'],
sort_dirs=['asc'], cell_down_support=False)
sort_dirs=['asc'], cell_down_support=False, all_tenants=False)
def test_get_servers_ignore_sort_key_with_no_sort_dir(self):
req = self.req('/fake/servers?sort_key=vcpus&sort_key=user_id')
@ -942,7 +942,7 @@ class ServersControllerTest(ControllerTest):
self.mock_get_all.assert_called_once_with(
mock.ANY, search_opts=mock.ANY, limit=mock.ANY, marker=mock.ANY,
expected_attrs=mock.ANY, sort_keys=['user_id'], sort_dirs=[],
cell_down_support=False)
cell_down_support=False, all_tenants=False)
def test_get_servers_ignore_sort_key_with_bad_sort_dir(self):
req = self.req('/fake/servers?sort_key=vcpus&sort_dir=bad_dir')
@ -950,7 +950,7 @@ class ServersControllerTest(ControllerTest):
self.mock_get_all.assert_called_once_with(
mock.ANY, search_opts=mock.ANY, limit=mock.ANY, marker=mock.ANY,
expected_attrs=mock.ANY, sort_keys=[], sort_dirs=[],
cell_down_support=False)
cell_down_support=False, all_tenants=False)
def test_get_servers_non_admin_with_admin_only_sort_key(self):
req = self.req('/fake/servers?sort_key=host&sort_dir=desc')
@ -964,13 +964,13 @@ class ServersControllerTest(ControllerTest):
self.mock_get_all.assert_called_once_with(
mock.ANY, search_opts=mock.ANY, limit=mock.ANY, marker=mock.ANY,
expected_attrs=mock.ANY, sort_keys=['node'], sort_dirs=['desc'],
cell_down_support=False)
cell_down_support=False, all_tenants=False)
def test_get_servers_with_bad_option(self):
def fake_get_all(context, search_opts=None,
limit=None, marker=None,
expected_attrs=None, sort_keys=None, sort_dirs=None,
cell_down_support=False):
cell_down_support=False, all_tenants=False):
db_list = [fakes.stub_instance(100, uuid=uuids.fake)]
return instance_obj._make_instance_list(
context, objects.InstanceList(), db_list, FIELDS)
@ -987,13 +987,13 @@ class ServersControllerTest(ControllerTest):
limit=1000, marker=None,
search_opts={'deleted': False, 'project_id': 'fake'},
sort_dirs=['desc'], sort_keys=['created_at'],
cell_down_support=False)
cell_down_support=False, all_tenants=False)
def test_get_servers_allows_image(self):
def fake_get_all(context, search_opts=None,
limit=None, marker=None,
expected_attrs=None, sort_keys=None, sort_dirs=None,
cell_down_support=False):
cell_down_support=False, all_tenants=False):
self.assertIsNotNone(search_opts)
self.assertIn('image', search_opts)
self.assertEqual(search_opts['image'], '12345')
@ -1147,7 +1147,7 @@ class ServersControllerTest(ControllerTest):
def fake_get_all(context, search_opts=None,
limit=None, marker=None,
expected_attrs=None, sort_keys=None, sort_dirs=None,
cell_down_support=False):
cell_down_support=False, all_tenants=False):
self.assertIsNotNone(search_opts)
self.assertIn('flavor', search_opts)
# flavor is an integer ID
@ -1183,7 +1183,7 @@ class ServersControllerTest(ControllerTest):
def fake_get_all(context, search_opts=None,
limit=None, marker=None,
expected_attrs=None, sort_keys=None, sort_dirs=None,
cell_down_support=False):
cell_down_support=False, all_tenants=False):
self.assertIsNotNone(search_opts)
self.assertIn('vm_state', search_opts)
self.assertEqual(search_opts['vm_state'], [vm_states.ACTIVE])
@ -1202,7 +1202,7 @@ class ServersControllerTest(ControllerTest):
def fake_get_all(context, search_opts=None,
limit=None, marker=None,
expected_attrs=None, sort_keys=None, sort_dirs=None,
cell_down_support=False):
cell_down_support=False, all_tenants=False):
self.assertIsNotNone(search_opts)
self.assertIn('task_state', search_opts)
self.assertEqual([task_states.REBOOT_PENDING,
@ -1226,7 +1226,7 @@ class ServersControllerTest(ControllerTest):
def fake_get_all(context, search_opts=None,
limit=None, marker=None,
expected_attrs=None, sort_keys=None, sort_dirs=None,
cell_down_support=False):
cell_down_support=False, all_tenants=False):
self.assertIn('vm_state', search_opts)
self.assertEqual(search_opts['vm_state'],
[vm_states.ACTIVE, vm_states.STOPPED])
@ -1259,7 +1259,7 @@ class ServersControllerTest(ControllerTest):
def fake_get_all(context, search_opts=None,
limit=None, marker=None,
expected_attrs=None, sort_keys=None, sort_dirs=None,
cell_down_support=False):
cell_down_support=False, all_tenants=False):
self.assertIn('vm_state', search_opts)
self.assertEqual(search_opts['vm_state'], ['deleted'])
@ -1318,7 +1318,7 @@ class ServersControllerTest(ControllerTest):
def fake_get_all(context, search_opts=None,
limit=None, marker=None,
expected_attrs=None, sort_keys=None, sort_dirs=None,
cell_down_support=False):
cell_down_support=False, all_tenants=False):
self.assertIsNotNone(search_opts)
self.assertIn('name', search_opts)
self.assertEqual(search_opts['name'], 'whee.*')
@ -1346,7 +1346,7 @@ class ServersControllerTest(ControllerTest):
def fake_get_all(context, search_opts=None,
limit=None, marker=None,
expected_attrs=None, sort_keys=None, sort_dirs=None,
cell_down_support=False):
cell_down_support=False, all_tenants=False):
self.assertIsNotNone(search_opts)
self.assertIn('changes-since', search_opts)
changes_since = datetime.datetime(2011, 1, 24, 17, 8, 1,
@ -1386,7 +1386,7 @@ class ServersControllerTest(ControllerTest):
def fake_get_all(context, search_opts=None,
limit=None, marker=None,
expected_attrs=None, sort_keys=None, sort_dirs=None,
cell_down_support=False):
cell_down_support=False, all_tenants=False):
self.assertIsNotNone(search_opts)
# Allowed by user
self.assertIn('name', search_opts)
@ -1415,7 +1415,7 @@ class ServersControllerTest(ControllerTest):
def fake_get_all(context, search_opts=None,
limit=None, marker=None,
expected_attrs=None, sort_keys=None, sort_dirs=None,
cell_down_support=False):
cell_down_support=False, all_tenants=False):
self.assertIsNotNone(search_opts)
# Allowed by user
self.assertIn('name', search_opts)
@ -1448,7 +1448,7 @@ class ServersControllerTest(ControllerTest):
def fake_get_all(context, search_opts=None,
limit=None, marker=None,
expected_attrs=None, sort_keys=None, sort_dirs=None,
cell_down_support=False):
cell_down_support=False, all_tenants=False):
self.assertIsNotNone(search_opts)
# Allowed by user
self.assertIn('name', search_opts)
@ -1482,7 +1482,7 @@ class ServersControllerTest(ControllerTest):
def fake_get_all(context, search_opts=None,
limit=None, marker=None,
expected_attrs=None, sort_keys=None, sort_dirs=None,
cell_down_support=False):
cell_down_support=False, all_tenants=False):
self.assertIsNotNone(search_opts)
self.assertIn('ip', search_opts)
self.assertEqual(search_opts['ip'], '10\..*')
@ -1504,7 +1504,7 @@ class ServersControllerTest(ControllerTest):
def fake_get_all(context, search_opts=None,
limit=None, marker=None,
expected_attrs=None, sort_keys=None, sort_dirs=None,
cell_down_support=False):
cell_down_support=False, all_tenants=False):
self.assertIsNotNone(search_opts)
self.assertIn('ip6', search_opts)
self.assertEqual(search_opts['ip6'], 'ffff.*')
@ -1527,7 +1527,7 @@ class ServersControllerTest(ControllerTest):
def fake_get_all(context, search_opts=None,
limit=None, marker=None,
expected_attrs=None, sort_keys=None, sort_dirs=None,
cell_down_support=False):
cell_down_support=False, all_tenants=False):
self.assertIsNotNone(search_opts)
self.assertIn('ip6', search_opts)
self.assertEqual(search_opts['ip6'], 'ffff.*')
@ -1550,7 +1550,7 @@ class ServersControllerTest(ControllerTest):
def fake_get_all(context, search_opts=None,
limit=None, marker=None,
expected_attrs=None, sort_keys=None, sort_dirs=None,
cell_down_support=False):
cell_down_support=False, all_tenants=False):
self.assertIsNotNone(search_opts)
self.assertIn('access_ip_v4', search_opts)
self.assertEqual(search_opts['access_ip_v4'], 'ffff.*')
@ -1573,7 +1573,7 @@ class ServersControllerTest(ControllerTest):
def fake_get_all(context, search_opts=None,
limit=None, marker=None,
expected_attrs=None, sort_keys=None, sort_dirs=None,
cell_down_support=False):
cell_down_support=False, all_tenants=False):
self.assertIsNotNone(search_opts)
self.assertIn('access_ip_v6', search_opts)
self.assertEqual(search_opts['access_ip_v6'], 'ffff.*')
@ -1712,7 +1712,7 @@ class ServersControllerTest(ControllerTest):
def fake_get_all(context, search_opts=None,
limit=None, marker=None,
expected_attrs=None, sort_keys=None, sort_dirs=None,
cell_down_support=False):
cell_down_support=False, all_tenants=False):
cur = api_version_request.APIVersionRequest(self.wsgi_api_version)
v216 = api_version_request.APIVersionRequest('2.16')
if cur >= v216:
@ -2436,7 +2436,7 @@ class ServerControllerTestV266(ControllerTest):
def fake_get_all(context, search_opts=None,
limit=None, marker=None,
expected_attrs=None, sort_keys=None, sort_dirs=None,
cell_down_support=False):
cell_down_support=False, all_tenants=False):
self.assertIsNotNone(search_opts)
self.assertIn('changes-before', search_opts)
changes_before = datetime.datetime(2011, 1, 24, 17, 8, 1,
@ -2474,7 +2474,7 @@ class ServerControllerTestV266(ControllerTest):
def fake_get_all(context, search_opts=None,
limit=None, marker=None,
expected_attrs=None, sort_keys=None, sort_dirs=None,
cell_down_support=False):
cell_down_support=False, all_tenants=False):
self.assertIsNotNone(search_opts)
self.assertIn('changes-since', search_opts)
changes_since = datetime.datetime(2011, 1, 23, 17, 8, 1,

View File

@ -418,7 +418,8 @@ def fake_instance_get_all_by_filters(num_servers=5, **kwargs):
def fake_compute_get_all(num_servers=5, **kwargs):
def _return_servers_objs(context, search_opts=None, limit=None,
marker=None, expected_attrs=None, sort_keys=None,
sort_dirs=None, cell_down_support=False):
sort_dirs=None, cell_down_support=False,
all_tenants=False):
db_insts = fake_instance_get_all_by_filters()(None,
limit=limit,
marker=marker)

View File

@ -5719,6 +5719,33 @@ class _ComputeAPIUnitTestMixIn(object):
self.context.project_id,
limit=1)
@mock.patch.object(objects.BuildRequestList, 'get_by_filters')
@mock.patch.object(objects.InstanceMappingList,
'get_not_deleted_by_cell_and_project')
def test_get_all_with_cell_down_support_all_tenants(self, mock_get_ims,
mock_buildreq_get):
mock_buildreq_get.return_value = objects.BuildRequestList()
im = objects.InstanceMapping(context=self.context,
instance_uuid=uuids.inst1, cell_id=1,
project_id='fake', created_at=None, queued_for_delete=False)
mock_get_ims.return_value = [im]
inst = objects.Instance(context=self.context, uuid=im.instance_uuid,
project_id=im.project_id, created_at=im.created_at)
partial_instances = objects.InstanceList(self.context, objects=[inst])
with mock.patch('nova.compute.instance_list.'
'get_instance_objects_sorted') as mock_inst_get:
mock_inst_get.return_value = objects.InstanceList(
partial_instances), [uuids.cell1]
insts = self.compute_api.get_all(self.context, limit=3,
cell_down_support=True, all_tenants=True)
for i, instance in enumerate(partial_instances):
self.assertTrue(obj_base.obj_equal_prims(instance, insts[i]))
# get_not_deleted_by_cell_and_project is called with None
# project_id because of the all_tenants case.
mock_get_ims.assert_called_once_with(self.context, uuids.cell1,
None,
limit=3)
@mock.patch.object(objects.Instance, 'get_by_uuid')
def test_get_instance_from_cell_success(self, mock_get_inst):
cell_mapping = objects.CellMapping(uuid=uuids.cell1,
@ -6974,6 +7001,9 @@ class Cellsv1DeprecatedTestMixIn(object):
def test_get_all_without_cell_down_support(self):
self.skipTest("Cell down handling is not supported for cells_v1.")
def test_get_all_with_cell_down_support_all_tenants(self):
self.skipTest("Cell down handling is not supported for cells_v1.")
class ComputeAPIAPICellUnitTestCase(Cellsv1DeprecatedTestMixIn,
_ComputeAPIUnitTestMixIn,