openstack
/
nova
Code Issues Proposed changes

Join pci_devices when getting all servers in API 

The PCI API extension goes back and hits the database for every instance
when getting a list of all instances through the API.  This is causing
problems in testing because it's possible for the instance to be deleted
in between getting the initial list and when the API extension runs.

The API extension really shouldn't be going back to hit the database
anyway, so this patch will resolve that problem.

Change-Id: Id3c8a0b187e399ce2acecd4aaa37ac95e731d46c
Closes-bug: #1270680
This commit is contained in:
Russell Bryant 2014-01-20 13:54:29 -05:00
parent b89f1320cd
commit 119e2a690a
3 changed files with 74 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
nova/api/openstack/compute/plugins/v3/servers.py
View File

@ -548,10 +548,8 @@ class ServersController(wsgi.Controller):
limit, marker = common.get_limit_and_marker(req)
try:
instance_list = self.compute_api.get_all(context,
search_opts=search_opts,
limit=limit,
marker=marker,
want_objects=True)
search_opts=search_opts, limit=limit, marker=marker,
want_objects=True, expected_attrs=['pci_devices'])
except exception.MarkerNotFound:
msg = _('marker [%s] not found') % marker
raise exc.HTTPBadRequest(explanation=msg)

13
nova/compute/api.py
View File

@ -1728,7 +1728,8 @@ class API(base.Base):
return instance
def get_all(self, context, search_opts=None, sort_key='created_at',
sort_dir='desc', limit=None, marker=None, want_objects=False):
sort_dir='desc', limit=None, marker=None, want_objects=False,
expected_attrs=None):
"""Get all instances filtered by one of the given parameters.
If there is no filter and the context is an admin, it will retrieve
@ -1800,9 +1801,9 @@ class API(base.Base):
return []
inst_models = self._get_instances_by_filters(context, filters,
sort_key, sort_dir,
limit=limit,
marker=marker)
sort_key, sort_dir, limit=limit, marker=marker,
expected_attrs=expected_attrs)
if want_objects:
return inst_models
@ -1816,7 +1817,7 @@ class API(base.Base):
def _get_instances_by_filters(self, context, filters,
sort_key, sort_dir,
limit=None,
marker=None):
marker=None, expected_attrs=None):
if 'ip6' in filters or 'ip' in filters:
res = self.network_api.get_instance_uuids_by_ip_filter(context,
filters)
@ -1827,6 +1828,8 @@ class API(base.Base):
fields = ['metadata', 'system_metadata', 'info_cache',
'security_groups']
if expected_attrs:
fields.extend(expected_attrs)
return instance_obj.InstanceList.get_by_filters(
context, filters=filters, sort_key=sort_key, sort_dir=sort_dir,
limit=limit, marker=marker, expected_attrs=fields)

87
nova/tests/api/openstack/compute/plugins/v3/test_servers.py
View File

@ -641,7 +641,8 @@ class ServersControllerTest(ControllerTest):
def fake_get_all(compute_self, context, search_opts=None,
sort_key=None, sort_dir='desc',
limit=None, marker=None, want_objects=False):
limit=None, marker=None, want_objects=False,
expected_attrs=[]):
db_list = [fakes.stub_instance(100, uuid=server_uuid)]
return instance_obj._make_instance_list(
context, instance_obj.InstanceList(), db_list, FIELDS)
@ -659,7 +660,8 @@ class ServersControllerTest(ControllerTest):
def fake_get_all(compute_self, context, search_opts=None,
sort_key=None, sort_dir='desc',
limit=None, marker=None, want_objects=False):
limit=None, marker=None, want_objects=False,
expected_attrs=[]):
self.assertIsNotNone(search_opts)
self.assertIn('image', search_opts)
self.assertEqual(search_opts['image'], '12345')
@ -678,7 +680,8 @@ class ServersControllerTest(ControllerTest):
def test_tenant_id_filter_converts_to_project_id_for_admin(self):
def fake_get_all(context, filters=None, sort_key=None,
sort_dir='desc', limit=None, marker=None,
columns_to_join=None, use_slave=False):
columns_to_join=None, use_slave=False,
expected_attrs=[]):
self.assertIsNotNone(filters)
self.assertEqual(filters['project_id'], 'newfake')
self.assertFalse(filters.get('tenant_id'))
@ -697,7 +700,8 @@ class ServersControllerTest(ControllerTest):
def test_tenant_id_filter_no_admin_context(self):
def fake_get_all(context, filters=None, sort_key=None,
sort_dir='desc', limit=None, marker=None,
columns_to_join=None, use_slave=False):
columns_to_join=None, use_slave=False,
expected_attrs=[]):
self.assertNotEqual(filters, None)
self.assertEqual(filters['project_id'], 'fake')
return [fakes.stub_instance(100)]
@ -712,7 +716,8 @@ class ServersControllerTest(ControllerTest):
def test_tenant_id_filter_implies_all_tenants(self):
def fake_get_all(context, filters=None, sort_key=None,
sort_dir='desc', limit=None, marker=None,
columns_to_join=None, use_slave=False):
columns_to_join=None, use_slave=False,
expected_attrs=[]):
self.assertNotEqual(filters, None)
# The project_id assertion checks that the project_id
# filter is set to that specified in the request url and
@ -733,7 +738,8 @@ class ServersControllerTest(ControllerTest):
def test_all_tenants_param_normal(self):
def fake_get_all(context, filters=None, sort_key=None,
sort_dir='desc', limit=None, marker=None,
columns_to_join=None, use_slave=False):
columns_to_join=None, use_slave=False,
expected_attrs=[]):
self.assertNotIn('project_id', filters)
return [fakes.stub_instance(100)]
@ -749,7 +755,8 @@ class ServersControllerTest(ControllerTest):
def test_all_tenants_param_one(self):
def fake_get_all(context, filters=None, sort_key=None,
sort_dir='desc', limit=None, marker=None,
columns_to_join=None, use_slave=False):
columns_to_join=None, use_slave=False,
expected_attrs=[]):
self.assertNotIn('project_id', filters)
return [fakes.stub_instance(100)]
@ -765,7 +772,8 @@ class ServersControllerTest(ControllerTest):
def test_all_tenants_param_zero(self):
def fake_get_all(context, filters=None, sort_key=None,
sort_dir='desc', limit=None, marker=None,
columns_to_join=None, use_slave=False):
columns_to_join=None, use_slave=False,
expected_attrs=[]):
self.assertNotIn('all_tenants', filters)
return [fakes.stub_instance(100)]
@ -781,7 +789,8 @@ class ServersControllerTest(ControllerTest):
def test_all_tenants_param_false(self):
def fake_get_all(context, filters=None, sort_key=None,
sort_dir='desc', limit=None, marker=None,
columns_to_join=None, use_slave=False):
columns_to_join=None, use_slave=False,
expected_attrs=[]):
self.assertNotIn('all_tenants', filters)
return [fakes.stub_instance(100)]
@ -797,7 +806,8 @@ class ServersControllerTest(ControllerTest):
def test_all_tenants_param_invalid(self):
def fake_get_all(context, filters=None, sort_key=None,
sort_dir='desc', limit=None, marker=None,
columns_to_join=None):
columns_to_join=None,
expected_attrs=[]):
self.assertNotIn('all_tenants', filters)
return [fakes.stub_instance(100)]
@ -812,7 +822,8 @@ class ServersControllerTest(ControllerTest):
def test_admin_restricted_tenant(self):
def fake_get_all(context, filters=None, sort_key=None,
sort_dir='desc', limit=None, marker=None,
columns_to_join=None, use_slave=False):
columns_to_join=None, use_slave=False,
expected_attrs=[]):
self.assertIsNotNone(filters)
self.assertEqual(filters['project_id'], 'fake')
return [fakes.stub_instance(100)]
@ -829,7 +840,8 @@ class ServersControllerTest(ControllerTest):
def test_all_tenants_pass_policy(self):
def fake_get_all(context, filters=None, sort_key=None,
sort_dir='desc', limit=None, marker=None,
columns_to_join=None, use_slave=False):
columns_to_join=None, use_slave=False,
expected_attrs=[]):
self.assertIsNotNone(filters)
self.assertNotIn('project_id', filters)
return [fakes.stub_instance(100)]
@ -878,7 +890,8 @@ class ServersControllerTest(ControllerTest):
def fake_get_all(compute_self, context, search_opts=None,
sort_key=None, sort_dir='desc',
limit=None, marker=None, want_objects=False):
limit=None, marker=None, want_objects=False,
expected_attrs=[]):
self.assertIsNotNone(search_opts)
self.assertIn('flavor', search_opts)
# flavor is an integer ID
@ -906,7 +919,8 @@ class ServersControllerTest(ControllerTest):
def fake_get_all(compute_self, context, search_opts=None,
sort_key=None, sort_dir='desc',
limit=None, marker=None, want_objects=False):
limit=None, marker=None, want_objects=False,
expected_attrs=[]):
self.assertIsNotNone(search_opts)
self.assertIn('vm_state', search_opts)
self.assertEqual(search_opts['vm_state'], [vm_states.ACTIVE])
@ -928,7 +942,8 @@ class ServersControllerTest(ControllerTest):
def fake_get_all(compute_self, context, search_opts=None,
sort_key=None, sort_dir='desc',
limit=None, marker=None, want_objects=False):
limit=None, marker=None, want_objects=False,
expected_attrs=[]):
self.assertIsNotNone(search_opts)
self.assertIn('task_state', search_opts)
self.assertEqual(search_opts['task_state'], [task_state])
@ -951,7 +966,8 @@ class ServersControllerTest(ControllerTest):
def fake_get_all(compute_self, context, search_opts=None,
sort_key=None, sort_dir='desc',
limit=None, marker=None, want_objects=False):
limit=None, marker=None, want_objects=False,
expected_attrs=[]):
self.assertIn('vm_state', search_opts)
self.assertEqual(search_opts['vm_state'],
[vm_states.ACTIVE, vm_states.STOPPED])
@ -986,7 +1002,8 @@ class ServersControllerTest(ControllerTest):
def fake_get_all(compute_self, context, search_opts=None,
sort_key=None, sort_dir='desc',
limit=None, marker=None, want_objects=False):
limit=None, marker=None, want_objects=False,
expected_attrs=[]):
self.assertIn('vm_state', search_opts)
self.assertEqual(search_opts['vm_state'], ['deleted'])
@ -1008,7 +1025,8 @@ class ServersControllerTest(ControllerTest):
def fake_get_all(compute_self, context, search_opts=None,
sort_key=None, sort_dir='desc',
limit=None, marker=None, want_objects=False):
limit=None, marker=None, want_objects=False,
expected_attrs=[]):
self.assertIsNotNone(search_opts)
self.assertIn('name', search_opts)
self.assertEqual(search_opts['name'], 'whee.*')
@ -1029,7 +1047,8 @@ class ServersControllerTest(ControllerTest):
def fake_get_all(compute_self, context, search_opts=None,
sort_key=None, sort_dir='desc',
limit=None, marker=None, want_objects=False):
limit=None, marker=None, want_objects=False,
expected_attrs=[]):
self.assertIsNotNone(search_opts)
self.assertIn('changes-since', search_opts)
changes_since = datetime.datetime(2011, 1, 24, 17, 8, 1,
@ -1063,7 +1082,8 @@ class ServersControllerTest(ControllerTest):
def fake_get_all(compute_self, context, search_opts=None,
sort_key=None, sort_dir='desc',
limit=None, marker=None, want_objects=False):
limit=None, marker=None, want_objects=False,
expected_attrs=[]):
self.assertIsNotNone(search_opts)
# Allowed by user
self.assertIn('name', search_opts)
@ -1094,7 +1114,8 @@ class ServersControllerTest(ControllerTest):
def fake_get_all(compute_self, context, search_opts=None,
sort_key=None, sort_dir='desc',
limit=None, marker=None, want_objects=False):
limit=None, marker=None, want_objects=False,
expected_attrs=[]):
self.assertIsNotNone(search_opts)
# Allowed by user
self.assertIn('name', search_opts)
@ -1124,7 +1145,8 @@ class ServersControllerTest(ControllerTest):
def fake_get_all(compute_self, context, search_opts=None,
sort_key=None, sort_dir='desc',
limit=None, marker=None, want_objects=False):
limit=None, marker=None, want_objects=False,
expected_attrs=[]):
self.assertIsNotNone(search_opts)
self.assertIn('ip', search_opts)
self.assertEqual(search_opts['ip'], '10\..*')
@ -1148,7 +1170,8 @@ class ServersControllerTest(ControllerTest):
def fake_get_all(compute_self, context, search_opts=None,
sort_key=None, sort_dir='desc',
limit=None, marker=None, want_objects=False):
limit=None, marker=None, want_objects=False,
expected_attrs=[]):
self.assertIsNotNone(search_opts)
self.assertIn('ip6', search_opts)
self.assertEqual(search_opts['ip6'], 'ffff.*')
@ -1225,6 +1248,24 @@ class ServersControllerTest(ControllerTest):
self.assertEqual(s['host_id'], host_ids[i % 2])
self.assertEqual(s['name'], 'server%d' % (i + 1))
def test_get_servers_joins_pci_devices(self):
server_uuid = str(uuid.uuid4())
self.called = False
def fake_get_all(compute_self, context, search_opts=None,
sort_key=None, sort_dir='desc',
limit=None, marker=None, want_objects=False,
expected_attrs=[]):
self.called = True
self.assertTrue('pci_devices' in expected_attrs)
return []
self.stubs.Set(compute_api.API, 'get_all', fake_get_all)
req = fakes.HTTPRequestV3.blank('/servers', use_admin_context=True)
servers = self.controller.index(req)['servers']
self.assertTrue(self.called)
class ServersControllerDeleteTest(ControllerTest):