x/vmware-nsx
Code Issues Proposed changes

Avoid performing extra query for fetching port security binding

Browse Source 
Bug 1201957

Add a relationship performing eager load in Port and Network
models, thus preventing the 'extend' function from performing
an extra database query.
Also fixes a comment in securitygroups_db.py

Change-Id: If0f0277191884aab4dcb1ee36826df7f7d66a8fa
This commit is contained in:
Salvatore Orlando 2013-07-15 00:21:12 +02:00
parent a28087d5e6
commit 69c9b32734
6 changed files with 82 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
neutron/api/v2/base.py
View File

@ -548,7 +548,6 @@ class Controller(object):
raise webob.exc.HTTPBadRequest(msg) raise webob.exc.HTTPBadRequest(msg)
Controller._populate_tenant_id(context, res_dict, is_create) Controller._populate_tenant_id(context, res_dict, is_create)
Controller._verify_attributes(res_dict, attr_info) Controller._verify_attributes(res_dict, attr_info)
if is_create: # POST if is_create: # POST

2
neutron/db/db_base_plugin_v2.py
View File

@ -995,7 +995,7 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2,
'status': constants.NET_STATUS_ACTIVE} 'status': constants.NET_STATUS_ACTIVE}
network = models_v2.Network(**args) network = models_v2.Network(**args)
context.session.add(network) context.session.add(network)
return self._make_network_dict(network) return self._make_network_dict(network, process_extensions=False)
def update_network(self, context, id, network): def update_network(self, context, id, network):
n = network['network'] n = network['network']

85
neutron/db/portsecurity_db.py
View File

@ -17,9 +17,13 @@
# @author: Aaron Rosen, Nicira, Inc # @author: Aaron Rosen, Nicira, Inc
import sqlalchemy as sa import sqlalchemy as sa
from sqlalchemy import orm
from sqlalchemy.orm import exc from sqlalchemy.orm import exc
from neutron.api.v2 import attributes as attrs
from neutron.db import db_base_plugin_v2
from neutron.db import model_base from neutron.db import model_base
from neutron.db import models_v2
from neutron.extensions import portsecurity as psec from neutron.extensions import portsecurity as psec
from neutron.openstack.common import log as logging from neutron.openstack.common import log as logging
@ -32,6 +36,13 @@ class PortSecurityBinding(model_base.BASEV2):
primary_key=True) primary_key=True)
port_security_enabled = sa.Column(sa.Boolean(), nullable=False) port_security_enabled = sa.Column(sa.Boolean(), nullable=False)
# Add a relationship to the Port model in order to be to able to
# instruct SQLAlchemy to eagerly load port security binding
port = orm.relationship(
models_v2.Port,
backref=orm.backref("port_security", uselist=False,
cascade='delete', lazy='joined'))
class NetworkSecurityBinding(model_base.BASEV2): class NetworkSecurityBinding(model_base.BASEV2):
network_id = sa.Column(sa.String(36), network_id = sa.Column(sa.String(36),
@ -39,25 +50,43 @@ class NetworkSecurityBinding(model_base.BASEV2):
primary_key=True) primary_key=True)
port_security_enabled = sa.Column(sa.Boolean(), nullable=False) port_security_enabled = sa.Column(sa.Boolean(), nullable=False)
# Add a relationship to the Port model in order to be able to instruct
# SQLAlchemy to eagerly load default port security setting for ports
# on this network
network = orm.relationship(
models_v2.Network,
backref=orm.backref("port_security", uselist=False,
cascade='delete', lazy='joined'))
class PortSecurityDbMixin(object): class PortSecurityDbMixin(object):
"""Mixin class to add port security.""" """Mixin class to add port security."""
def _process_network_create_port_security(self, context, network): def _process_network_port_security_create(
self, context, network_req, network_res):
with context.session.begin(subtransactions=True): with context.session.begin(subtransactions=True):
db = NetworkSecurityBinding( db = NetworkSecurityBinding(
network_id=network['id'], network_id=network_res['id'],
port_security_enabled=network[psec.PORTSECURITY]) port_security_enabled=network_req[psec.PORTSECURITY])
context.session.add(db) context.session.add(db)
network_res[psec.PORTSECURITY] = network_req[psec.PORTSECURITY]
return self._make_network_port_security_dict(db) return self._make_network_port_security_dict(db)
def _extend_network_port_security_dict(self, context, network): def _process_port_port_security_create(
network[psec.PORTSECURITY] = self._get_network_security_binding( self, context, port_req, port_res):
context, network['id']) with context.session.begin(subtransactions=True):
db = PortSecurityBinding(
port_id=port_res['id'],
port_security_enabled=port_req[psec.PORTSECURITY])
context.session.add(db)
port_res[psec.PORTSECURITY] = port_req[psec.PORTSECURITY]
return self._make_port_security_dict(db)
def _extend_port_port_security_dict(self, context, port): def _extend_port_security_dict(self, response_data, db_data):
port[psec.PORTSECURITY] = self._get_port_security_binding( if ('port-security' in
context, port['id']) getattr(self, 'supported_extension_aliases', [])):
psec_value = db_data['port_security'][psec.PORTSECURITY]
response_data[psec.PORTSECURITY] = psec_value
def _get_network_security_binding(self, context, network_id): def _get_network_security_binding(self, context, network_id):
try: try:
@ -77,25 +106,37 @@ class PortSecurityDbMixin(object):
raise psec.PortSecurityBindingNotFound() raise psec.PortSecurityBindingNotFound()
return binding[psec.PORTSECURITY] return binding[psec.PORTSECURITY]
def _update_port_security_binding(self, context, port_id, def _process_port_port_security_update(
port_security_enabled): self, context, port_req, port_res):
if psec.PORTSECURITY in port_req:
port_security_enabled = port_req[psec.PORTSECURITY]
else:
return
try: try:
query = self._model_query(context, PortSecurityBinding) query = self._model_query(context, PortSecurityBinding)
port_id = port_res['id']
binding = query.filter( binding = query.filter(
PortSecurityBinding.port_id == port_id).one() PortSecurityBinding.port_id == port_id).one()
binding.update({psec.PORTSECURITY: port_security_enabled}) binding.port_security_enabled = port_security_enabled
port_res[psec.PORTSECURITY] = port_security_enabled
except exc.NoResultFound: except exc.NoResultFound:
raise psec.PortSecurityBindingNotFound() raise psec.PortSecurityBindingNotFound()
def _update_network_security_binding(self, context, network_id, def _process_network_port_security_update(
port_security_enabled): self, context, network_req, network_res):
if psec.PORTSECURITY in network_req:
port_security_enabled = network_req[psec.PORTSECURITY]
else:
return
try: try:
query = self._model_query(context, NetworkSecurityBinding) query = self._model_query(context, NetworkSecurityBinding)
network_id = network_res['id']
binding = query.filter( binding = query.filter(
NetworkSecurityBinding.network_id == network_id).one() NetworkSecurityBinding.network_id == network_id).one()
binding.update({psec.PORTSECURITY: port_security_enabled}) binding.port_security_enabled = port_security_enabled
network_res[psec.PORTSECURITY] = port_security_enabled
except exc.NoResultFound: except exc.NoResultFound:
raise psec.PortSecurityBindingNotFound() raise psec.PortSecurityBindingNotFound()
@ -126,14 +167,6 @@ class PortSecurityDbMixin(object):
return (port_security_enabled, has_ip) return (port_security_enabled, has_ip)
def _process_port_security_create(self, context, port):
with context.session.begin(subtransactions=True):
port_security_binding = PortSecurityBinding(
port_id=port['id'],
port_security_enabled=port[psec.PORTSECURITY])
context.session.add(port_security_binding)
return self._make_port_security_dict(port_security_binding)
def _make_port_security_dict(self, port, fields=None): def _make_port_security_dict(self, port, fields=None):
res = {'port_id': port['port_id'], res = {'port_id': port['port_id'],
psec.PORTSECURITY: port[psec.PORTSECURITY]} psec.PORTSECURITY: port[psec.PORTSECURITY]}
@ -141,3 +174,9 @@ class PortSecurityDbMixin(object):
def _ip_on_port(self, port): def _ip_on_port(self, port):
return bool(port.get('fixed_ips')) return bool(port.get('fixed_ips'))
# Register dict extend functions for ports and networks
db_base_plugin_v2.NeutronDbPluginV2.register_dict_extend_funcs(
attrs.NETWORKS, [_extend_port_security_dict])
db_base_plugin_v2.NeutronDbPluginV2.register_dict_extend_funcs(
attrs.PORTS, [_extend_port_security_dict])

6
neutron/db/securitygroups_db.py
View File

@ -433,9 +433,9 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase):
context.session.delete(rule) context.session.delete(rule)
def _extend_port_dict_security_group(self, port_res, port_db): def _extend_port_dict_security_group(self, port_res, port_db):
# If port_db is provided, security groups will be accessed via # Security group bindings will be retrieved from the sqlalchemy
# sqlalchemy models. As they're loaded together with ports this # model. As they're loaded eagerly with ports because of the
# will not cause an extra query. # joined load they will not cause an extra query.
security_group_ids = [sec_group_mapping['security_group_id'] for security_group_ids = [sec_group_mapping['security_group_id'] for
sec_group_mapping in port_db.security_groups] sec_group_mapping in port_db.security_groups]
port_res[ext_sg.SECURITYGROUPS] = security_group_ids port_res[ext_sg.SECURITYGROUPS] = security_group_ids

30
neutron/plugins/nicira/NeutronPlugin.py
View File

@ -868,7 +868,8 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
# Ensure there's an id in net_data # Ensure there's an id in net_data
net_data['id'] = new_net['id'] net_data['id'] = new_net['id']
# Process port security extension # Process port security extension
self._process_network_create_port_security(context, net_data) self._process_network_port_security_create(
context, net_data, new_net)
# DB Operations for setting the network as external # DB Operations for setting the network as external
self._process_l3_create(context, new_net, net_data) self._process_l3_create(context, new_net, net_data)
# Process QoS queue extension # Process QoS queue extension
@ -887,7 +888,6 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
net_data.get(pnet.SEGMENTATION_ID, 0)) net_data.get(pnet.SEGMENTATION_ID, 0))
self._extend_network_dict_provider(context, new_net, self._extend_network_dict_provider(context, new_net,
net_binding) net_binding)
self._extend_network_port_security_dict(context, new_net)
self.schedule_network(context, new_net) self.schedule_network(context, new_net)
return new_net return new_net
@ -976,9 +976,8 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
raise nvp_exc.NvpPluginException(err_msg=err_msg) raise nvp_exc.NvpPluginException(err_msg=err_msg)
# Don't do field selection here otherwise we won't be able # Don't do field selection here otherwise we won't be able
# to add provider networks fields # to add provider networks fields
net_result = self._make_network_dict(network, None) net_result = self._make_network_dict(network)
self._extend_network_dict_provider(context, net_result) self._extend_network_dict_provider(context, net_result)
self._extend_network_port_security_dict(context, net_result)
self._extend_network_qos_queue(context, net_result) self._extend_network_qos_queue(context, net_result)
return self._fields(net_result, fields) return self._fields(net_result, fields)
@ -990,7 +989,6 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
super(NvpPluginV2, self).get_networks(context, filters)) super(NvpPluginV2, self).get_networks(context, filters))
for net in neutron_lswitches: for net in neutron_lswitches:
self._extend_network_dict_provider(context, net) self._extend_network_dict_provider(context, net)
self._extend_network_port_security_dict(context, net)
self._extend_network_qos_queue(context, net) self._extend_network_qos_queue(context, net)
tenant_ids = filters and filters.get('tenant_id') or None tenant_ids = filters and filters.get('tenant_id') or None
@ -1027,7 +1025,6 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
nvp_lswitches = dict( nvp_lswitches = dict(
(uuid, ls) for (uuid, ls) in nvp_lswitches.iteritems() (uuid, ls) for (uuid, ls) in nvp_lswitches.iteritems()
if uuid in set(filters['id'])) if uuid in set(filters['id']))
for neutron_lswitch in neutron_lswitches: for neutron_lswitch in neutron_lswitches:
# Skip external networks as they do not exist in NVP # Skip external networks as they do not exist in NVP
if neutron_lswitch[l3.EXTERNAL]: if neutron_lswitch[l3.EXTERNAL]:
@ -1063,7 +1060,6 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
row[field] = neutron_lswitch[field] row[field] = neutron_lswitch[field]
ret_fields.append(row) ret_fields.append(row)
return ret_fields return ret_fields
return neutron_lswitches return neutron_lswitches
def update_network(self, context, id, network): def update_network(self, context, id, network):
@ -1076,13 +1072,12 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
with context.session.begin(subtransactions=True): with context.session.begin(subtransactions=True):
net = super(NvpPluginV2, self).update_network(context, id, network) net = super(NvpPluginV2, self).update_network(context, id, network)
if psec.PORTSECURITY in network['network']: if psec.PORTSECURITY in network['network']:
self._update_network_security_binding( self._process_network_port_security_update(
context, id, network['network'][psec.PORTSECURITY]) context, network['network'], net)
if network['network'].get(ext_qos.QUEUE): if network['network'].get(ext_qos.QUEUE):
net[ext_qos.QUEUE] = network['network'][ext_qos.QUEUE] net[ext_qos.QUEUE] = network['network'][ext_qos.QUEUE]
self._delete_network_queue_mapping(context, id) self._delete_network_queue_mapping(context, id)
self._process_network_queue_mapping(context, net) self._process_network_queue_mapping(context, net)
self._extend_network_port_security_dict(context, net)
self._process_l3_update(context, net, network['network']) self._process_l3_update(context, net, network['network'])
self._extend_network_dict_provider(context, net) self._extend_network_dict_provider(context, net)
self._extend_network_qos_queue(context, net) self._extend_network_qos_queue(context, net)
@ -1094,7 +1089,6 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
neutron_lports = super(NvpPluginV2, self).get_ports( neutron_lports = super(NvpPluginV2, self).get_ports(
context, filters) context, filters)
for neutron_lport in neutron_lports: for neutron_lport in neutron_lports:
self._extend_port_port_security_dict(context, neutron_lport)
self._extend_port_mac_learning_state(context, neutron_lport) self._extend_port_mac_learning_state(context, neutron_lport)
if (filters.get('network_id') and len(filters.get('network_id')) and if (filters.get('network_id') and len(filters.get('network_id')) and
self._network_is_external(context, filters['network_id'][0])): self._network_is_external(context, filters['network_id'][0])):
@ -1218,7 +1212,8 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
(port_security, has_ip) = self._determine_port_security_and_has_ip( (port_security, has_ip) = self._determine_port_security_and_has_ip(
context, port_data) context, port_data)
port_data[psec.PORTSECURITY] = port_security port_data[psec.PORTSECURITY] = port_security
self._process_port_security_create(context, port_data) self._process_port_port_security_create(
context, port_data, neutron_db)
# security group extension checks # security group extension checks
if port_security and has_ip: if port_security and has_ip:
self._ensure_default_security_group_on_port(context, port) self._ensure_default_security_group_on_port(context, port)
@ -1263,7 +1258,6 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
# remove since it will be added in extend based on policy # remove since it will be added in extend based on policy
del port_data[ext_qos.QUEUE] del port_data[ext_qos.QUEUE]
self._extend_port_port_security_dict(context, port_data)
self._extend_port_qos_queue(context, port_data) self._extend_port_qos_queue(context, port_data)
self._process_portbindings_create_and_update(context, self._process_portbindings_create_and_update(context,
port, port_data) port, port_data)
@ -1287,10 +1281,6 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
port['port'].pop('fixed_ips', None) port['port'].pop('fixed_ips', None)
ret_port.update(port['port']) ret_port.update(port['port'])
tenant_id = self._get_tenant_id_for_create(context, ret_port) tenant_id = self._get_tenant_id_for_create(context, ret_port)
# populate port_security setting
if psec.PORTSECURITY not in port['port']:
ret_port[psec.PORTSECURITY] = self._get_port_security_binding(
context, id)
has_ip = self._ip_on_port(ret_port) has_ip = self._ip_on_port(ret_port)
# checks if security groups were updated adding/modifying # checks if security groups were updated adding/modifying
@ -1316,8 +1306,8 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
sgids) sgids)
if psec.PORTSECURITY in port['port']: if psec.PORTSECURITY in port['port']:
self._update_port_security_binding( self._process_port_port_security_update(
context, id, ret_port[psec.PORTSECURITY]) context, port['port'], ret_port)
ret_port[ext_qos.QUEUE] = self._check_for_queue_and_create( ret_port[ext_qos.QUEUE] = self._check_for_queue_and_create(
context, ret_port) context, ret_port)
@ -1334,7 +1324,6 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
ret_port[mac_ext.MAC_LEARNING] = old_mac_learning_state ret_port[mac_ext.MAC_LEARNING] = old_mac_learning_state
self._delete_port_queue_mapping(context, ret_port['id']) self._delete_port_queue_mapping(context, ret_port['id'])
self._process_port_queue_mapping(context, ret_port) self._process_port_queue_mapping(context, ret_port)
self._extend_port_port_security_dict(context, ret_port)
LOG.warn(_("Update port request: %s"), port) LOG.warn(_("Update port request: %s"), port)
nvp_port_id = self._nvp_get_port_id( nvp_port_id = self._nvp_get_port_id(
context, self.cluster, ret_port) context, self.cluster, ret_port)
@ -1425,7 +1414,6 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
with context.session.begin(subtransactions=True): with context.session.begin(subtransactions=True):
neutron_db_port = super(NvpPluginV2, self).get_port(context, neutron_db_port = super(NvpPluginV2, self).get_port(context,
id, fields) id, fields)
self._extend_port_port_security_dict(context, neutron_db_port)
self._extend_port_qos_queue(context, neutron_db_port) self._extend_port_qos_queue(context, neutron_db_port)
self._extend_port_mac_learning_state(context, neutron_db_port) self._extend_port_mac_learning_state(context, neutron_db_port)

22
neutron/tests/unit/test_extension_portsecurity.py
View File

@ -60,9 +60,8 @@ class PortSecurityTestPlugin(db_base_plugin_v2.NeutronDbPluginV2,
neutron_db = super(PortSecurityTestPlugin, self).create_network( neutron_db = super(PortSecurityTestPlugin, self).create_network(
context, network) context, network)
neutron_db.update(network['network']) neutron_db.update(network['network'])
self._process_network_create_port_security( self._process_network_port_security_create(
context, neutron_db) context, network['network'], neutron_db)
self._extend_network_port_security_dict(context, neutron_db)
return neutron_db return neutron_db
def update_network(self, context, id, network): def update_network(self, context, id, network):
@ -70,17 +69,14 @@ class PortSecurityTestPlugin(db_base_plugin_v2.NeutronDbPluginV2,
neutron_db = super(PortSecurityTestPlugin, self).update_network( neutron_db = super(PortSecurityTestPlugin, self).update_network(
context, id, network) context, id, network)
if psec.PORTSECURITY in network['network']: if psec.PORTSECURITY in network['network']:
self._update_network_security_binding( self._process_network_port_security_update(
context, id, network['network'][psec.PORTSECURITY]) context, network['network'], neutron_db)
self._extend_network_port_security_dict(
context, neutron_db)
return neutron_db return neutron_db
def get_network(self, context, id, fields=None): def get_network(self, context, id, fields=None):
with context.session.begin(subtransactions=True): with context.session.begin(subtransactions=True):
net = super(PortSecurityTestPlugin, self).get_network( net = super(PortSecurityTestPlugin, self).get_network(
context, id) context, id)
self._extend_network_port_security_dict(context, net)
return self._fields(net, fields) return self._fields(net, fields)
def create_port(self, context, port): def create_port(self, context, port):
@ -95,7 +91,7 @@ class PortSecurityTestPlugin(db_base_plugin_v2.NeutronDbPluginV2,
(port_security, has_ip) = self._determine_port_security_and_has_ip( (port_security, has_ip) = self._determine_port_security_and_has_ip(
context, p) context, p)
p[psec.PORTSECURITY] = port_security p[psec.PORTSECURITY] = port_security
self._process_port_security_create(context, p) self._process_port_port_security_create(context, p, neutron_db)
if (attr.is_attr_set(p.get(ext_sg.SECURITYGROUPS)) and if (attr.is_attr_set(p.get(ext_sg.SECURITYGROUPS)) and
not (port_security and has_ip)): not (port_security and has_ip)):
@ -109,8 +105,6 @@ class PortSecurityTestPlugin(db_base_plugin_v2.NeutronDbPluginV2,
self._process_port_create_security_group( self._process_port_create_security_group(
context, p, p[ext_sg.SECURITYGROUPS]) context, p, p[ext_sg.SECURITYGROUPS])
self._extend_port_port_security_dict(context, p)
return port['port'] return port['port']
def update_port(self, context, id, port): def update_port(self, context, id, port):
@ -159,10 +153,8 @@ class PortSecurityTestPlugin(db_base_plugin_v2.NeutronDbPluginV2,
ret_port, sgids) ret_port, sgids)
if psec.PORTSECURITY in port['port']: if psec.PORTSECURITY in port['port']:
self._update_port_security_binding( self._process_port_port_security_update(
context, id, ret_port[psec.PORTSECURITY]) context, port['port'], ret_port)
self._extend_port_port_security_dict(context, ret_port)
return ret_port return ret_port