From 0c8824b44d2f841d610296893f9a316f4f083d43 Mon Sep 17 00:00:00 2001 From: Surya Seetharaman Date: Wed, 6 Feb 2019 11:31:11 +0100 Subject: [PATCH] Plumbing for allowing the all-tenants filter with down cells This patch lays the underground work for supporting the ``all-tenants`` filter. Related to blueprint handling-down-cell Change-Id: I7dcef20aed0178c81d6580aa9534288eaa383dab --- nova/api/openstack/compute/servers.py | 12 +++- nova/compute/api.py | 10 +++- .../api/openstack/compute/test_serversV21.py | 56 +++++++++---------- nova/tests/unit/api/openstack/fakes.py | 3 +- nova/tests/unit/compute/test_compute_api.py | 30 ++++++++++ 5 files changed, 78 insertions(+), 33 deletions(-) diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py index 58efc8f18dc3..ea04517e91bc 100644 --- a/nova/api/openstack/compute/servers.py +++ b/nova/api/openstack/compute/servers.py @@ -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) diff --git a/nova/compute/api.py b/nova/compute/api.py index 286d29feaed0..e4fa8c0454d5 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -2350,7 +2350,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 """ @@ -2513,7 +2513,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 @@ -2533,6 +2533,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: @@ -2695,6 +2696,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) diff --git a/nova/tests/unit/api/openstack/compute/test_serversV21.py b/nova/tests/unit/api/openstack/compute/test_serversV21.py index f33e19d073de..5acfadb9bdb3 100644 --- a/nova/tests/unit/api/openstack/compute/test_serversV21.py +++ b/nova/tests/unit/api/openstack/compute/test_serversV21.py @@ -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, diff --git a/nova/tests/unit/api/openstack/fakes.py b/nova/tests/unit/api/openstack/fakes.py index a959ef47bda1..120f863dd1ef 100644 --- a/nova/tests/unit/api/openstack/fakes.py +++ b/nova/tests/unit/api/openstack/fakes.py @@ -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) diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index 4e520df8e885..2db7ae3183e8 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -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, @@ -7018,6 +7045,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,