From f581b2faf11b49852b0e1d6f2ddd8d19b8b69cdf Mon Sep 17 00:00:00 2001 From: Salvatore Orlando Date: Mon, 15 Jul 2013 00:21:12 +0200 Subject: [PATCH] Avoid performing extra query for fetching port security binding 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 --- neutron/api/v2/base.py | 1 - neutron/db/db_base_plugin_v2.py | 2 +- neutron/db/portsecurity_db.py | 85 ++++++++++++++----- neutron/db/securitygroups_db.py | 6 +- neutron/plugins/nicira/NeutronPlugin.py | 30 ++----- .../tests/unit/test_extension_portsecurity.py | 22 ++--- 6 files changed, 82 insertions(+), 64 deletions(-) diff --git a/neutron/api/v2/base.py b/neutron/api/v2/base.py index 0749d148b..f212d3703 100644 --- a/neutron/api/v2/base.py +++ b/neutron/api/v2/base.py @@ -548,7 +548,6 @@ class Controller(object): raise webob.exc.HTTPBadRequest(msg) Controller._populate_tenant_id(context, res_dict, is_create) - Controller._verify_attributes(res_dict, attr_info) if is_create: # POST diff --git a/neutron/db/db_base_plugin_v2.py b/neutron/db/db_base_plugin_v2.py index 932d5c08a..5b3352440 100644 --- a/neutron/db/db_base_plugin_v2.py +++ b/neutron/db/db_base_plugin_v2.py @@ -995,7 +995,7 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2, 'status': constants.NET_STATUS_ACTIVE} network = models_v2.Network(**args) 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): n = network['network'] diff --git a/neutron/db/portsecurity_db.py b/neutron/db/portsecurity_db.py index 0b1616750..f2eff2aae 100644 --- a/neutron/db/portsecurity_db.py +++ b/neutron/db/portsecurity_db.py @@ -17,9 +17,13 @@ # @author: Aaron Rosen, Nicira, Inc import sqlalchemy as sa +from sqlalchemy import orm 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 models_v2 from neutron.extensions import portsecurity as psec from neutron.openstack.common import log as logging @@ -32,6 +36,13 @@ class PortSecurityBinding(model_base.BASEV2): primary_key=True) 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): network_id = sa.Column(sa.String(36), @@ -39,25 +50,43 @@ class NetworkSecurityBinding(model_base.BASEV2): primary_key=True) 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): """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): db = NetworkSecurityBinding( - network_id=network['id'], - port_security_enabled=network[psec.PORTSECURITY]) + network_id=network_res['id'], + port_security_enabled=network_req[psec.PORTSECURITY]) context.session.add(db) + network_res[psec.PORTSECURITY] = network_req[psec.PORTSECURITY] return self._make_network_port_security_dict(db) - def _extend_network_port_security_dict(self, context, network): - network[psec.PORTSECURITY] = self._get_network_security_binding( - context, network['id']) + def _process_port_port_security_create( + self, context, port_req, port_res): + 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): - port[psec.PORTSECURITY] = self._get_port_security_binding( - context, port['id']) + def _extend_port_security_dict(self, response_data, db_data): + if ('port-security' in + 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): try: @@ -77,25 +106,37 @@ class PortSecurityDbMixin(object): raise psec.PortSecurityBindingNotFound() return binding[psec.PORTSECURITY] - def _update_port_security_binding(self, context, port_id, - port_security_enabled): + def _process_port_port_security_update( + self, context, port_req, port_res): + if psec.PORTSECURITY in port_req: + port_security_enabled = port_req[psec.PORTSECURITY] + else: + return try: query = self._model_query(context, PortSecurityBinding) + port_id = port_res['id'] binding = query.filter( 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: raise psec.PortSecurityBindingNotFound() - def _update_network_security_binding(self, context, network_id, - port_security_enabled): + def _process_network_port_security_update( + self, context, network_req, network_res): + if psec.PORTSECURITY in network_req: + port_security_enabled = network_req[psec.PORTSECURITY] + else: + return try: query = self._model_query(context, NetworkSecurityBinding) + network_id = network_res['id'] binding = query.filter( 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: raise psec.PortSecurityBindingNotFound() @@ -126,14 +167,6 @@ class PortSecurityDbMixin(object): 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): res = {'port_id': port['port_id'], psec.PORTSECURITY: port[psec.PORTSECURITY]} @@ -141,3 +174,9 @@ class PortSecurityDbMixin(object): def _ip_on_port(self, port): 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]) diff --git a/neutron/db/securitygroups_db.py b/neutron/db/securitygroups_db.py index 198c231a8..8e44ebec1 100644 --- a/neutron/db/securitygroups_db.py +++ b/neutron/db/securitygroups_db.py @@ -433,9 +433,9 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase): context.session.delete(rule) def _extend_port_dict_security_group(self, port_res, port_db): - # If port_db is provided, security groups will be accessed via - # sqlalchemy models. As they're loaded together with ports this - # will not cause an extra query. + # Security group bindings will be retrieved from the sqlalchemy + # model. As they're loaded eagerly with ports because of the + # joined load they will not cause an extra query. security_group_ids = [sec_group_mapping['security_group_id'] for sec_group_mapping in port_db.security_groups] port_res[ext_sg.SECURITYGROUPS] = security_group_ids diff --git a/neutron/plugins/nicira/NeutronPlugin.py b/neutron/plugins/nicira/NeutronPlugin.py index 597c5aa75..b33c5fcb8 100644 --- a/neutron/plugins/nicira/NeutronPlugin.py +++ b/neutron/plugins/nicira/NeutronPlugin.py @@ -868,7 +868,8 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2, # Ensure there's an id in net_data net_data['id'] = new_net['id'] # 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 self._process_l3_create(context, new_net, net_data) # Process QoS queue extension @@ -887,7 +888,6 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2, net_data.get(pnet.SEGMENTATION_ID, 0)) self._extend_network_dict_provider(context, new_net, net_binding) - self._extend_network_port_security_dict(context, new_net) self.schedule_network(context, new_net) return new_net @@ -976,9 +976,8 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2, raise nvp_exc.NvpPluginException(err_msg=err_msg) # Don't do field selection here otherwise we won't be able # 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_port_security_dict(context, net_result) self._extend_network_qos_queue(context, net_result) return self._fields(net_result, fields) @@ -990,7 +989,6 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2, super(NvpPluginV2, self).get_networks(context, filters)) for net in neutron_lswitches: self._extend_network_dict_provider(context, net) - self._extend_network_port_security_dict(context, net) self._extend_network_qos_queue(context, net) tenant_ids = filters and filters.get('tenant_id') or None @@ -1027,7 +1025,6 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2, nvp_lswitches = dict( (uuid, ls) for (uuid, ls) in nvp_lswitches.iteritems() if uuid in set(filters['id'])) - for neutron_lswitch in neutron_lswitches: # Skip external networks as they do not exist in NVP if neutron_lswitch[l3.EXTERNAL]: @@ -1063,7 +1060,6 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2, row[field] = neutron_lswitch[field] ret_fields.append(row) return ret_fields - return neutron_lswitches def update_network(self, context, id, network): @@ -1076,13 +1072,12 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2, with context.session.begin(subtransactions=True): net = super(NvpPluginV2, self).update_network(context, id, network) if psec.PORTSECURITY in network['network']: - self._update_network_security_binding( - context, id, network['network'][psec.PORTSECURITY]) + self._process_network_port_security_update( + context, network['network'], net) if network['network'].get(ext_qos.QUEUE): net[ext_qos.QUEUE] = network['network'][ext_qos.QUEUE] self._delete_network_queue_mapping(context, id) self._process_network_queue_mapping(context, net) - self._extend_network_port_security_dict(context, net) self._process_l3_update(context, net, network['network']) self._extend_network_dict_provider(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( context, filters) for neutron_lport in neutron_lports: - self._extend_port_port_security_dict(context, neutron_lport) self._extend_port_mac_learning_state(context, neutron_lport) if (filters.get('network_id') and len(filters.get('network_id')) and 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( context, port_data) 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 if port_security and has_ip: 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 del port_data[ext_qos.QUEUE] - self._extend_port_port_security_dict(context, port_data) self._extend_port_qos_queue(context, port_data) self._process_portbindings_create_and_update(context, port, port_data) @@ -1287,10 +1281,6 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2, port['port'].pop('fixed_ips', None) ret_port.update(port['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) # checks if security groups were updated adding/modifying @@ -1316,8 +1306,8 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2, sgids) if psec.PORTSECURITY in port['port']: - self._update_port_security_binding( - context, id, ret_port[psec.PORTSECURITY]) + self._process_port_port_security_update( + context, port['port'], ret_port) ret_port[ext_qos.QUEUE] = self._check_for_queue_and_create( context, ret_port) @@ -1334,7 +1324,6 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2, ret_port[mac_ext.MAC_LEARNING] = old_mac_learning_state self._delete_port_queue_mapping(context, ret_port['id']) self._process_port_queue_mapping(context, ret_port) - self._extend_port_port_security_dict(context, ret_port) LOG.warn(_("Update port request: %s"), port) nvp_port_id = self._nvp_get_port_id( context, self.cluster, ret_port) @@ -1425,7 +1414,6 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2, with context.session.begin(subtransactions=True): neutron_db_port = super(NvpPluginV2, self).get_port(context, id, fields) - self._extend_port_port_security_dict(context, neutron_db_port) self._extend_port_qos_queue(context, neutron_db_port) self._extend_port_mac_learning_state(context, neutron_db_port) diff --git a/neutron/tests/unit/test_extension_portsecurity.py b/neutron/tests/unit/test_extension_portsecurity.py index 19335c6ce..8ba50ff80 100644 --- a/neutron/tests/unit/test_extension_portsecurity.py +++ b/neutron/tests/unit/test_extension_portsecurity.py @@ -60,9 +60,8 @@ class PortSecurityTestPlugin(db_base_plugin_v2.NeutronDbPluginV2, neutron_db = super(PortSecurityTestPlugin, self).create_network( context, network) neutron_db.update(network['network']) - self._process_network_create_port_security( - context, neutron_db) - self._extend_network_port_security_dict(context, neutron_db) + self._process_network_port_security_create( + context, network['network'], neutron_db) return neutron_db 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( context, id, network) if psec.PORTSECURITY in network['network']: - self._update_network_security_binding( - context, id, network['network'][psec.PORTSECURITY]) - self._extend_network_port_security_dict( - context, neutron_db) + self._process_network_port_security_update( + context, network['network'], neutron_db) return neutron_db def get_network(self, context, id, fields=None): with context.session.begin(subtransactions=True): net = super(PortSecurityTestPlugin, self).get_network( context, id) - self._extend_network_port_security_dict(context, net) return self._fields(net, fields) 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( context, p) 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 not (port_security and has_ip)): @@ -109,8 +105,6 @@ class PortSecurityTestPlugin(db_base_plugin_v2.NeutronDbPluginV2, self._process_port_create_security_group( context, p, p[ext_sg.SECURITYGROUPS]) - self._extend_port_port_security_dict(context, p) - return port['port'] def update_port(self, context, id, port): @@ -159,10 +153,8 @@ class PortSecurityTestPlugin(db_base_plugin_v2.NeutronDbPluginV2, ret_port, sgids) if psec.PORTSECURITY in port['port']: - self._update_port_security_binding( - context, id, ret_port[psec.PORTSECURITY]) - - self._extend_port_port_security_dict(context, ret_port) + self._process_port_port_security_update( + context, port['port'], ret_port) return ret_port