Merge "Fix performance of Server List with Neutron for Admins" into stable/havana

This commit is contained in:
Jenkins 2014-01-30 14:24:50 +00:00 committed by Gerrit Code Review
commit 442fb63048
8 changed files with 230 additions and 95 deletions

View File

@ -521,20 +521,15 @@ class SecurityGroupsOutputController(wsgi.Controller):
# neutron security groups the requested security groups for the
# instance are not in the db and have not been sent to neutron yet.
if req.method != 'POST':
if len(servers) == 1:
group = (self.security_group_api
.get_instance_security_groups(context,
servers[0]['id']))
if group:
servers[0][key] = group
else:
sg_instance_bindings = (
sg_instance_bindings = (
self.security_group_api
.get_instances_security_groups_bindings(context))
for server in servers:
groups = sg_instance_bindings.get(server['id'])
if groups:
server[key] = groups
.get_instances_security_groups_bindings(context,
servers))
for server in servers:
groups = sg_instance_bindings.get(server['id'])
if groups:
server[key] = groups
# In this section of code len(servers) == 1 as you can only POST
# one server in an API request.
else:

View File

@ -67,20 +67,15 @@ class SecurityGroupsOutputController(wsgi.Controller):
# neutron security groups the requested security groups for the
# instance are not in the db and have not been sent to neutron yet.
if req.method != 'POST':
if len(servers) == 1:
group = (self.security_group_api
.get_instance_security_groups(context,
servers[0]['id']))
if group:
servers[0][key] = group
else:
sg_instance_bindings = (
self.security_group_api
.get_instances_security_groups_bindings(context))
for server in servers:
groups = sg_instance_bindings.get(server['id'])
if groups:
server[key] = groups
sg_instance_bindings = (
self.security_group_api
.get_instances_security_groups_bindings(context,
servers))
for server in servers:
groups = sg_instance_bindings.get(server['id'])
if groups:
server[key] = groups
# In this section of code len(servers) == 1 as you can only POST
# one server in an API request.
else:

View File

@ -42,6 +42,11 @@ wrap_check_security_groups_policy = compute_api.policy_decorator(
CONF = cfg.CONF
LOG = logging.getLogger(__name__)
# NOTE: Neutron client has a max URL length of 8192, so we have
# to limit the number of IDs we include in any single search. Really
# doesn't seem to be any point in making this a config value.
MAX_SEARCH_IDS = 150
class SecurityGroupAPI(security_group_base.SecurityGroupBase):
@ -279,35 +284,92 @@ class SecurityGroupAPI(security_group_base.SecurityGroupBase):
raise exc_info[0], exc_info[1], exc_info[2]
return self._convert_to_nova_security_group_rule_format(rule)
def get_instances_security_groups_bindings(self, context):
def _get_ports_from_server_list(self, servers, neutron):
"""Returns a list of ports used by the servers."""
def _chunk_by_ids(servers, limit):
ids = []
for server in servers:
ids.append(server['id'])
if len(ids) >= limit:
yield ids
ids = []
if ids:
yield ids
# Note: Have to split the query up as the search criteria
# form part of the URL, which has a fixed max size
ports = []
for ids in _chunk_by_ids(servers, MAX_SEARCH_IDS):
search_opts = {'device_id': ids}
ports.extend(neutron.list_ports(**search_opts).get('ports'))
return ports
def _get_secgroups_from_port_list(self, ports, neutron):
"""Returns a dict of security groups keyed by thier ids."""
def _chunk_by_ids(sg_ids, limit):
sg_id_list = []
for sg_id in sg_ids:
sg_id_list.append(sg_id)
if len(sg_id_list) >= limit:
yield sg_id_list
sg_id_list = []
if sg_id_list:
yield sg_id_list
# Find the set of unique SecGroup IDs to search for
sg_ids = set()
for port in ports:
sg_ids.update(port.get('security_groups', []))
# Note: Have to split the query up as the search criteria
# form part of the URL, which has a fixed max size
security_groups = {}
for sg_id_list in _chunk_by_ids(sg_ids, MAX_SEARCH_IDS):
sg_search_opts = {'id': sg_id_list}
search_results = neutron.list_security_groups(**sg_search_opts)
for sg in search_results.get('security_groups'):
security_groups[sg['id']] = sg
return security_groups
def get_instances_security_groups_bindings(self, context, servers,
detailed=False):
"""Returns a dict(instance_id, [security_groups]) to allow obtaining
all of the instances and their security groups in one shot.
"""
neutron = neutronv2.get_client(context)
ports = neutron.list_ports().get('ports')
security_groups = neutron.list_security_groups().get('security_groups')
security_group_lookup = {}
instances_security_group_bindings = {}
for security_group in security_groups:
security_group_lookup[security_group['id']] = security_group
neutron = neutronv2.get_client(context)
ports = self._get_ports_from_server_list(servers, neutron)
security_groups = self._get_secgroups_from_port_list(ports, neutron)
instances_security_group_bindings = {}
for port in ports:
for port_security_group in port.get('security_groups', []):
try:
sg = security_group_lookup[port_security_group]
# name is optional in neutron so if not specified return id
if sg.get('name'):
sg_entry = {'name': sg['name']}
for port_sg_id in port.get('security_groups', []):
# Note: have to check we found port_sg as its possible
# the port has an SG that this user doesn't have access to
port_sg = security_groups.get(port_sg_id)
if port_sg:
if detailed:
sg_entry = self._convert_to_nova_security_group_format(
port_sg)
instances_security_group_bindings.setdefault(
port['device_id'], []).append(sg_entry)
else:
sg_entry = {'name': sg['id']}
instances_security_group_bindings.setdefault(
port['device_id'], []).append(sg_entry)
except KeyError:
# This should only happen due to a race condition
# if the security group on a port was deleted after the
# ports were returned. We pass since this security
# group is no longer on the port.
pass
# name is optional in neutron so if not specified
# return id
name = port_sg.get('name')
if not name:
name = port_sg.get('id')
sg_entry = {'name': name}
instances_security_group_bindings.setdefault(
port['device_id'], []).append(sg_entry)
return instances_security_group_bindings
def get_instance_security_groups(self, context, instance_uuid,
@ -316,38 +378,10 @@ class SecurityGroupAPI(security_group_base.SecurityGroupBase):
If detailed is True then it also returns the full details of the
security groups associated with an instance.
"""
neutron = neutronv2.get_client(context)
params = {'device_id': instance_uuid}
ports = neutron.list_ports(**params)
security_groups = neutron.list_security_groups().get('security_groups')
security_group_lookup = {}
for security_group in security_groups:
security_group_lookup[security_group['id']] = security_group
ret = []
for port in ports['ports']:
for security_group in port.get('security_groups', []):
try:
if detailed:
ret.append(self._convert_to_nova_security_group_format(
security_group_lookup[security_group]))
else:
name = security_group_lookup[security_group].get(
'name')
# Since the name is optional for
# neutron security groups
if not name:
name = security_group
ret.append({'name': name})
except KeyError:
# This should only happen due to a race condition
# if the security group on a port was deleted after the
# ports were returned. We pass since this security
# group is no longer on the port.
pass
return ret
servers = [{'id': instance_uuid}]
sg_bindings = self.get_instances_security_groups_bindings(
context, servers, detailed)
return sg_bindings.get(instance_uuid)
def _has_security_group_requirements(self, port):
port_security_enabled = port.get('port_security_enabled', True)

View File

@ -270,6 +270,8 @@ class TestNeutronSecurityGroups(
self.manager._removeSecurityGroup(req, '1', body)
def test_get_instances_security_groups_bindings(self):
servers = [{'id': test_security_groups.FAKE_UUID1},
{'id': test_security_groups.FAKE_UUID2}]
sg1 = self._create_sg_template(name='test1').get('security_group')
sg2 = self._create_sg_template(name='test2').get('security_group')
# test name='' is replaced with id
@ -290,8 +292,8 @@ class TestNeutronSecurityGroups(
security_group_api = self.controller.security_group_api
bindings = (
security_group_api.get_instances_security_groups_bindings(
context.get_admin_context()))
self.assertEquals(bindings, expected)
context.get_admin_context(), servers))
self.assertEqual(bindings, expected)
def test_get_instance_security_groups(self):
sg1 = self._create_sg_template(name='test1').get('security_group')
@ -763,7 +765,7 @@ class MockClient(object):
device_id = _params.get('device_id')
for port in self._fake_ports.values():
if device_id:
if device_id == port['device_id']:
if port['device_id'] in device_id:
ret.append(port)
else:
ret.append(port)

View File

@ -1510,9 +1510,14 @@ def fake_compute_create(*args, **kwargs):
return ([fake_compute_get(*args, **kwargs)], '')
def fake_get_instances_security_groups_bindings(inst, context):
return {UUID1: [{'name': 'fake-0-0'}, {'name': 'fake-0-1'}],
UUID2: [{'name': 'fake-1-0'}, {'name': 'fake-1-1'}]}
def fake_get_instances_security_groups_bindings(inst, context, servers):
groups = {UUID1: [{'name': 'fake-0-0'}, {'name': 'fake-0-1'}],
UUID2: [{'name': 'fake-1-0'}, {'name': 'fake-1-1'}],
UUID3: [{'name': 'fake-2-0'}, {'name': 'fake-2-1'}]}
result = {}
for server in servers:
result[server['id']] = groups.get(server['id'])
return result
class SecurityGroupsOutputTest(test.TestCase):

View File

@ -96,9 +96,14 @@ def fake_get_instance_security_groups(*args, **kwargs):
return [{'name': 'fake'}]
def fake_get_instances_security_groups_bindings(inst, context):
return {UUID1: [{'name': 'fake-0-0'}, {'name': 'fake-0-1'}],
UUID2: [{'name': 'fake-1-0'}, {'name': 'fake-1-1'}]}
def fake_get_instances_security_groups_bindings(inst, context, servers):
groups = {UUID1: [{'name': 'fake-0-0'}, {'name': 'fake-0-1'}],
UUID2: [{'name': 'fake-1-0'}, {'name': 'fake-1-1'}],
UUID3: [{'name': 'fake-2-0'}, {'name': 'fake-2-1'}]}
result = {}
for server in servers:
result[server['id']] = groups.get(server['id'])
return result
class SecurityGroupsOutputTest(test.TestCase):

View File

@ -28,8 +28,11 @@ def fake_get(*args, **kwargs):
return nova_group
def fake_get_instance_security_groups(*args, **kwargs):
return [{'name': 'test'}]
def fake_get_instances_security_groups_bindings(self, context, servers):
result = {}
for s in servers:
result[s.get('id')] = [{'name': 'test'}]
return result
class SecurityGroupsJsonTest(test_servers.ServersSampleBase):
@ -40,8 +43,8 @@ class SecurityGroupsJsonTest(test_servers.ServersSampleBase):
super(SecurityGroupsJsonTest, self).setUp()
self.stubs.Set(neutron_driver.SecurityGroupAPI, 'get', fake_get)
self.stubs.Set(neutron_driver.SecurityGroupAPI,
'get_instance_security_groups',
fake_get_instance_security_groups)
'get_instances_security_groups_bindings',
fake_get_instances_security_groups_bindings)
def test_server_create(self):
self._post_server()

View File

@ -83,3 +83,99 @@ class TestNeutronDriver(test.NoDBTestCase):
sg_api = security_groups.NativeNeutronSecurityGroupAPI()
self.assertRaises(exception.SecurityGroupLimitExceeded,
sg_api.add_rules, self.context, None, name, [vals])
def test_instances_security_group_bindings(self):
server_id = 'c5a20e8d-c4b0-47cf-9dca-ebe4f758acb1'
port1_id = '4c505aec-09aa-47bc-bcc0-940477e84dc0'
port2_id = 'b3b31a53-6e29-479f-ae5c-00b7b71a6d44'
sg1_id = '2f7ce969-1a73-4ef9-bbd6-c9a91780ecd4'
sg2_id = '20c89ce5-9388-4046-896e-64ffbd3eb584'
servers = [{'id': server_id}]
ports = [{'id': port1_id, 'device_id': server_id,
'security_groups': [sg1_id]},
{'id': port2_id, 'device_id': server_id,
'security_groups': [sg2_id]}]
port_list = {'ports': ports}
sg1 = {'id': sg1_id, 'name': 'wol'}
sg2 = {'id': sg2_id, 'name': 'eor'}
security_groups_list = {'security_groups': [sg1, sg2]}
sg_bindings = {server_id: [{'name': 'wol'}, {'name': 'eor'}]}
self.moxed_client.list_ports(device_id=[server_id]).AndReturn(
port_list)
self.moxed_client.list_security_groups(id=[sg2_id, sg1_id]).AndReturn(
security_groups_list)
self.mox.ReplayAll()
sg_api = neutron_driver.SecurityGroupAPI()
result = sg_api.get_instances_security_groups_bindings(
self.context, servers)
self.assertEquals(result, sg_bindings)
def _test_instances_security_group_bindings_scale(self, num_servers):
max_query = 150
sg1_id = '2f7ce969-1a73-4ef9-bbd6-c9a91780ecd4'
sg2_id = '20c89ce5-9388-4046-896e-64ffbd3eb584'
sg1 = {'id': sg1_id, 'name': 'wol'}
sg2 = {'id': sg2_id, 'name': 'eor'}
security_groups_list = {'security_groups': [sg1, sg2]}
servers = []
device_ids = []
ports = []
sg_bindings = {}
for i in xrange(0, num_servers):
server_id = "server-%d" % i
port_id = "port-%d" % i
servers.append({'id': server_id})
device_ids.append(server_id)
ports.append({'id': port_id,
'device_id': server_id,
'security_groups': [sg1_id, sg2_id]})
sg_bindings[server_id] = [{'name': 'wol'}, {'name': 'eor'}]
for x in xrange(0, num_servers, max_query):
self.moxed_client.list_ports(
device_id=device_ids[x:x + max_query]).\
AndReturn({'ports': ports[x:x + max_query]})
self.moxed_client.list_security_groups(id=[sg2_id, sg1_id]).AndReturn(
security_groups_list)
self.mox.ReplayAll()
sg_api = neutron_driver.SecurityGroupAPI()
result = sg_api.get_instances_security_groups_bindings(
self.context, servers)
self.assertEquals(result, sg_bindings)
def test_instances_security_group_bindings_less_than_max(self):
self._test_instances_security_group_bindings_scale(100)
def test_instances_security_group_bindings_max(self):
self._test_instances_security_group_bindings_scale(150)
def test_instances_security_group_bindings_more_then_max(self):
self._test_instances_security_group_bindings_scale(300)
def test_instances_security_group_bindings_with_hidden_sg(self):
servers = [{'id': 'server_1'}]
ports = [{'id': '1', 'device_id': 'dev_1', 'security_groups': ['1']},
{'id': '2', 'device_id': 'dev_1', 'security_groups': ['2']}]
port_list = {'ports': ports}
sg1 = {'id': '1', 'name': 'wol'}
sg2 = {'id': '2', 'name': 'eor'}
# User doesn't have access to sg2
security_groups_list = {'security_groups': [sg1]}
sg_bindings = {'dev_1': [{'name': 'wol'}]}
self.moxed_client.list_ports(device_id=['server_1']).AndReturn(
port_list)
self.moxed_client.list_security_groups(id=['1', '2']).AndReturn(
security_groups_list)
self.mox.ReplayAll()
sg_api = neutron_driver.SecurityGroupAPI()
result = sg_api.get_instances_security_groups_bindings(
self.context, servers)
self.assertEquals(result, sg_bindings)