From 6f01194e9c30afc2656cef9b15fea5ae688e93d7 Mon Sep 17 00:00:00 2001 From: Salvatore Orlando Date: Mon, 29 Apr 2013 01:40:47 +0200 Subject: [PATCH] Perform a joined query for ports and security group associations bug 1174111 Instead of loading security group associations with a separate query each time a port is loaded from the database, perform the load operation with a join using a joined sqlalchemy relationship. Also, this patch removes the need for invoking the mixin method 'extend_port_dict_security_group' from the plugin code. Change-Id: I40b22281d45ff4f4bf8149211883799a9051c1a0 --- quantum/api/v2/attributes.py | 23 ++++++---- quantum/db/db_base_plugin_v2.py | 38 +++++++++++---- quantum/db/securitygroups_db.py | 46 ++++++++++++------- quantum/db/securitygroups_rpc_base.py | 6 ++- quantum/plugins/brocade/QuantumPlugin.py | 7 ++- .../plugins/linuxbridge/lb_quantum_plugin.py | 5 +- quantum/plugins/midonet/plugin.py | 7 ++- quantum/plugins/nec/nec_plugin.py | 8 +--- quantum/plugins/nicira/QuantumPlugin.py | 16 +++---- .../plugins/openvswitch/ovs_quantum_plugin.py | 10 +--- quantum/plugins/ryu/ryu_quantum_plugin.py | 7 +-- .../tests/unit/test_extension_portsecurity.py | 15 +++--- .../unit/test_extension_security_group.py | 19 ++++---- 13 files changed, 112 insertions(+), 95 deletions(-) diff --git a/quantum/api/v2/attributes.py b/quantum/api/v2/attributes.py index 1eb6df1aa..ea38e5f54 100644 --- a/quantum/api/v2/attributes.py +++ b/quantum/api/v2/attributes.py @@ -451,6 +451,13 @@ validators = {'type:dict': _validate_dict, 'type:uuid_list': _validate_uuid_list, 'type:values': _validate_values} +# Define constants for base resource name +NETWORK = 'network' +NETWORKS = '%ss' % NETWORK +PORT = 'port' +PORTS = '%ss' % PORT +SUBNET = 'subnet' +SUBNETS = '%ss' % SUBNET # Note: a default of ATTR_NOT_SPECIFIED indicates that an # attribute is not required, but will be generated by the plugin # if it is not specified. Particularly, a value of ATTR_NOT_SPECIFIED @@ -475,7 +482,7 @@ validators = {'type:dict': _validate_dict, # mechanism, ie: there might be rules which refer to this attribute. RESOURCE_ATTRIBUTE_MAP = { - 'networks': { + NETWORKS: { 'id': {'allow_post': False, 'allow_put': False, 'validate': {'type:uuid': None}, 'is_visible': True, @@ -504,7 +511,7 @@ RESOURCE_ATTRIBUTE_MAP = { 'required_by_policy': True, 'enforce_policy': True}, }, - 'ports': { + PORTS: { 'id': {'allow_post': False, 'allow_put': False, 'validate': {'type:uuid': None}, 'is_visible': True, @@ -546,7 +553,7 @@ RESOURCE_ATTRIBUTE_MAP = { 'status': {'allow_post': False, 'allow_put': False, 'is_visible': True}, }, - 'subnets': { + SUBNETS: { 'id': {'allow_post': False, 'allow_put': False, 'validate': {'type:uuid': None}, 'is_visible': True, @@ -606,13 +613,13 @@ RESOURCE_ATTRIBUTE_MAP = { # Resources without parents, such as networks, are not in this list RESOURCE_HIERARCHY_MAP = { - 'ports': {'parent': 'networks', 'identified_by': 'network_id'}, - 'subnets': {'parent': 'networks', 'identified_by': 'network_id'} + PORTS: {'parent': NETWORKS, 'identified_by': 'network_id'}, + SUBNETS: {'parent': NETWORKS, 'identified_by': 'network_id'} } -PLURALS = {'networks': 'network', - 'ports': 'port', - 'subnets': 'subnet', +PLURALS = {NETWORKS: NETWORK, + PORTS: PORT, + SUBNETS: SUBNET, 'dns_nameservers': 'dns_nameserver', 'host_routes': 'host_route', 'allocation_pools': 'allocation_pool', diff --git a/quantum/db/db_base_plugin_v2.py b/quantum/db/db_base_plugin_v2.py index e671f97b9..5a2f15255 100644 --- a/quantum/db/db_base_plugin_v2.py +++ b/quantum/db/db_base_plugin_v2.py @@ -70,6 +70,10 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2): # To this aim, the register_model_query_hook and unregister_query_hook # from this class should be invoked _model_query_hooks = {} + # This dictionary will store methods for extending attributes of + # api resources. Mixins can use this dict for adding their own methods + # TODO(salvatore-orlando): Avoid using class-level variables + _dict_extend_functions = {} def __init__(self): # NOTE(jkoelker) This is an incomlete implementation. Subclasses @@ -117,6 +121,12 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2): query = query.filter(query_filter) return query + @classmethod + def register_dict_extend_funcs(cls, resource, funcs): + cur_funcs = cls._dict_extend_functions.get(resource, []) + cur_funcs.extend(funcs) + cls._dict_extend_functions[resource] = cur_funcs + @classmethod def register_model_query_hook(cls, model, name, query_hook, filter_hook, result_filters=None): @@ -142,6 +152,14 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2): model_hooks[name] = {'query': query_hook, 'filter': filter_hook, 'result_filters': result_filters} + def _filter_non_model_columns(self, data, model): + """Remove all the attributes from data which are not columns of + the model passed as second parameter. + """ + columns = [c.name for c in model.__table__.columns] + return dict((k, v) for (k, v) in + data.iteritems() if k in columns) + def _get_by_id(self, context, model, id): query = self._model_query(context, model) return query.filter(model.id == id).one() @@ -909,7 +927,8 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2): } return self._fields(res, fields) - def _make_port_dict(self, port, fields=None): + def _make_port_dict(self, port, fields=None, + process_extensions=True): res = {"id": port["id"], 'name': port['name'], "network_id": port["network_id"], @@ -922,6 +941,11 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2): for ip in port["fixed_ips"]], "device_id": port["device_id"], "device_owner": port["device_owner"]} + # Call auxiliary extend functions, if any + if process_extensions: + for func in self._dict_extend_functions.get(attributes.PORTS, + []): + func(self, res, port) return self._fields(res, fields) def _create_bulk(self, resource, context, request_items): @@ -1331,7 +1355,7 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2): ) context.session.add(allocated) - return self._make_port_dict(port) + return self._make_port_dict(port, process_extensions=False) def update_port(self, context, id, port): p = port['port'] @@ -1342,15 +1366,12 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2): if 'fixed_ips' in p: self._recycle_expired_ip_allocations(context, port['network_id']) - original = self._make_port_dict(port) + original = self._make_port_dict(port, process_extensions=False) ips = self._update_ips_for_port(context, port["network_id"], id, original["fixed_ips"], p['fixed_ips']) - # 'fixed_ip's not part of DB so it is deleted - del p['fixed_ips'] - # Update ips if necessary for ip in ips: allocated = models_v2.IPAllocation( @@ -1358,8 +1379,9 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2): ip_address=ip['ip_address'], subnet_id=ip['subnet_id'], expiration=self._default_allocation_expiration()) context.session.add(allocated) - - port.update(p) + # Remove all attributes in p which are not in the port DB model + # and then update the port + port.update(self._filter_non_model_columns(p, models_v2.Port)) return self._make_port_dict(port) diff --git a/quantum/db/securitygroups_db.py b/quantum/db/securitygroups_db.py index b1c5f9a32..f54ba9ad4 100644 --- a/quantum/db/securitygroups_db.py +++ b/quantum/db/securitygroups_db.py @@ -22,6 +22,7 @@ from sqlalchemy.orm import exc from sqlalchemy.orm import scoped_session from quantum.api.v2 import attributes as attr +from quantum.db import db_base_plugin_v2 from quantum.db import model_base from quantum.db import models_v2 from quantum.extensions import securitygroup as ext_sg @@ -46,6 +47,13 @@ class SecurityGroupPortBinding(model_base.BASEV2): sa.ForeignKey("securitygroups.id"), primary_key=True) + # Add a relationship to the Port model in order to instruct SQLAlchemy to + # eagerly load security group bindings + ports = orm.relationship( + models_v2.Port, + backref=orm.backref("security_groups", + lazy='joined', cascade='delete')) + class SecurityGroupRule(model_base.BASEV2, models_v2.HasId, models_v2.HasTenant): @@ -385,25 +393,29 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase): rule = self._get_security_group_rule(context, id) context.session.delete(rule) - def _extend_port_dict_security_group(self, context, port): - filters = {'port_id': [port['id']]} - fields = {'security_group_id': None} - security_group_id = self._get_port_security_group_bindings( - context, filters, fields) + 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_ids = [sec_group_mapping['security_group_id'] for + sec_group_mapping in port_db.security_groups] + port_res[ext_sg.SECURITYGROUPS] = security_group_ids + return port_res - port[ext_sg.SECURITYGROUPS] = [] - for security_group_id in security_group_id: - port[ext_sg.SECURITYGROUPS].append( - security_group_id['security_group_id']) - return port + # Register dict extend functions for ports + db_base_plugin_v2.QuantumDbPluginV2.register_dict_extend_funcs( + attr.PORTS, [_extend_port_dict_security_group]) - def _process_port_create_security_group(self, context, port_id, - security_group_id): - if not attr.is_attr_set(security_group_id): - return - for security_group_id in security_group_id: - self._create_port_security_group_binding(context, port_id, - security_group_id) + def _process_port_create_security_group(self, context, port, + security_group_ids): + if attr.is_attr_set(security_group_ids): + for security_group_id in security_group_ids: + self._create_port_security_group_binding(context, port['id'], + security_group_id) + # Convert to list as a set might be passed here and + # this has to be serialized + port[ext_sg.SECURITYGROUPS] = (security_group_ids and + list(security_group_ids) or []) def _ensure_default_security_group(self, context, tenant_id): """Create a default security group if one doesn't exist. diff --git a/quantum/db/securitygroups_rpc_base.py b/quantum/db/securitygroups_rpc_base.py index a9665b1ec..3d9afe992 100644 --- a/quantum/db/securitygroups_rpc_base.py +++ b/quantum/db/securitygroups_rpc_base.py @@ -77,10 +77,12 @@ class SecurityGroupServerRpcMixin(sg_db.SecurityGroupDbMixin): self._delete_port_security_group_bindings(context, id) self._process_port_create_security_group( context, - id, + updated_port, port['port'][ext_sg.SECURITYGROUPS]) need_notify = True - self._extend_port_dict_security_group(context, updated_port) + else: + updated_port[ext_sg.SECURITYGROUPS] = ( + original_port[ext_sg.SECURITYGROUPS]) return need_notify def is_security_group_member_updated(self, context, diff --git a/quantum/plugins/brocade/QuantumPlugin.py b/quantum/plugins/brocade/QuantumPlugin.py index f24aa622a..895ca1c14 100644 --- a/quantum/plugins/brocade/QuantumPlugin.py +++ b/quantum/plugins/brocade/QuantumPlugin.py @@ -382,15 +382,16 @@ class BrocadePluginV2(db_base_plugin_v2.QuantumDbPluginV2, port['port'][ext_sg.SECURITYGROUPS] = ( self._get_security_groups_on_port(context, port)) self._delete_port_security_group_bindings(context, port_id) + # process_port_create_security_group also needs port id + port['port']['id'] = port_id self._process_port_create_security_group( context, - port_id, + port['port'], port['port'][ext_sg.SECURITYGROUPS]) port_updated = True port = super(BrocadePluginV2, self).update_port( context, port_id, port) - self._extend_port_dict_security_group(context, port) if original_port['admin_state_up'] != port['admin_state_up']: port_updated = True @@ -411,7 +412,6 @@ class BrocadePluginV2(db_base_plugin_v2.QuantumDbPluginV2, with context.session.begin(subtransactions=True): port = super(BrocadePluginV2, self).get_port( context, port_id, fields) - self._extend_port_dict_security_group(context, port) self._extend_port_dict_binding(context, port) return self._fields(port, fields) @@ -423,7 +423,6 @@ class BrocadePluginV2(db_base_plugin_v2.QuantumDbPluginV2, filters, fields) for port in ports: - self._extend_port_dict_security_group(context, port) self._extend_port_dict_binding(context, port) res_ports.append(self._fields(port, fields)) diff --git a/quantum/plugins/linuxbridge/lb_quantum_plugin.py b/quantum/plugins/linuxbridge/lb_quantum_plugin.py index 8955ea0cb..45d942bb7 100644 --- a/quantum/plugins/linuxbridge/lb_quantum_plugin.py +++ b/quantum/plugins/linuxbridge/lb_quantum_plugin.py @@ -471,7 +471,6 @@ class LinuxBridgePluginV2(db_base_plugin_v2.QuantumDbPluginV2, port = super(LinuxBridgePluginV2, self).get_port(context, id, fields) - self._extend_port_dict_security_group(context, port) self._extend_port_dict_binding(context, port), return self._fields(port, fields) @@ -484,7 +483,6 @@ class LinuxBridgePluginV2(db_base_plugin_v2.QuantumDbPluginV2, limit, marker, page_reverse) #TODO(nati) filter by security group for port in ports: - self._extend_port_dict_security_group(context, port) self._extend_port_dict_binding(context, port) res_ports.append(self._fields(port, fields)) return res_ports @@ -500,8 +498,7 @@ class LinuxBridgePluginV2(db_base_plugin_v2.QuantumDbPluginV2, port = super(LinuxBridgePluginV2, self).create_port(context, port) self._process_port_create_security_group( - context, port['id'], sgids) - self._extend_port_dict_security_group(context, port) + context, port, sgids) self.notify_security_groups_member_updated(context, port) return self._extend_port_dict_binding(context, port) diff --git a/quantum/plugins/midonet/plugin.py b/quantum/plugins/midonet/plugin.py index 34e46808b..e9656c273 100644 --- a/quantum/plugins/midonet/plugin.py +++ b/quantum/plugins/midonet/plugin.py @@ -422,7 +422,9 @@ class MidonetPluginV2(db_base_plugin_v2.QuantumDbPluginV2, with session.begin(subtransactions=True): port_db_entry = super(MidonetPluginV2, self).create_port(context, port) - self._extend_port_dict_security_group(context, port_db_entry) + # Caveat: port_db_entry is not a db model instance + sg_ids = self._get_security_groups_on_port(context, port) + self._process_port_create_security_group(context, port, sg_ids) if is_compute_interface: # Create a DHCP entry if needed. if 'ip_address' in (port_db_entry['fixed_ips'] or [{}])[0]: @@ -453,8 +455,6 @@ class MidonetPluginV2(db_base_plugin_v2.QuantumDbPluginV2, # get the quantum port from DB. port_db_entry = super(MidonetPluginV2, self).get_port(context, id, fields) - self._extend_port_dict_security_group(context, port_db_entry) - # verify that corresponding port exists in MidoNet. try: self.mido_api.get_port(id) @@ -477,7 +477,6 @@ class MidonetPluginV2(db_base_plugin_v2.QuantumDbPluginV2, try: for port in ports_db_entry: self.mido_api.get_port(port['id']) - self._extend_port_dict_security_group(context, port) except w_exc.HTTPNotFound: raise MidonetResourceNotFound(resource_type='Port', id=port['id']) diff --git a/quantum/plugins/nec/nec_plugin.py b/quantum/plugins/nec/nec_plugin.py index 2117132a3..2ae5d2b7b 100644 --- a/quantum/plugins/nec/nec_plugin.py +++ b/quantum/plugins/nec/nec_plugin.py @@ -380,8 +380,7 @@ class NECPluginV2(nec_plugin_base.NECPluginV2Base, sgids = self._get_security_groups_on_port(context, port) port = super(NECPluginV2, self).create_port(context, port) self._process_port_create_security_group( - context, port['id'], sgids) - self._extend_port_dict_security_group(context, port) + context, port, sgids) self.notify_security_groups_member_updated(context, port) self._update_resource_status(context, "port", port['id'], OperationalStatus.BUILD) @@ -416,9 +415,6 @@ class NECPluginV2(nec_plugin_base.NECPluginV2Base, else: self.deactivate_port(context, old_port) - # NOTE: _extend_port_dict_security_group() is called in - # update_security_group_on_port() above, so we don't need to - # call it here. return self._extend_port_dict_binding(context, new_port) def delete_port(self, context, id, l3_port_check=True): @@ -452,7 +448,6 @@ class NECPluginV2(nec_plugin_base.NECPluginV2Base, def get_port(self, context, id, fields=None): with context.session.begin(subtransactions=True): port = super(NECPluginV2, self).get_port(context, id, fields) - self._extend_port_dict_security_group(context, port) self._extend_port_dict_binding(context, port) return self._fields(port, fields) @@ -462,7 +457,6 @@ class NECPluginV2(nec_plugin_base.NECPluginV2Base, fields) # TODO(amotoki) filter by security group for port in ports: - self._extend_port_dict_security_group(context, port) self._extend_port_dict_binding(context, port) return [self._fields(port, fields) for port in ports] diff --git a/quantum/plugins/nicira/QuantumPlugin.py b/quantum/plugins/nicira/QuantumPlugin.py index b9814dee3..52879c2a2 100644 --- a/quantum/plugins/nicira/QuantumPlugin.py +++ b/quantum/plugins/nicira/QuantumPlugin.py @@ -183,6 +183,7 @@ class NvpPluginV2(db_base_plugin_v2.QuantumDbPluginV2, self.nvp_opts.nvp_gen_timeout) db.configure_db() + # Extend the fault map self._extend_fault_map() # Set up RPC interface for DHCP agent self.setup_rpc() @@ -1041,7 +1042,6 @@ class NvpPluginV2(db_base_plugin_v2.QuantumDbPluginV2, context, filters) for quantum_lport in quantum_lports: self._extend_port_port_security_dict(context, quantum_lport) - self._extend_port_dict_security_group(context, quantum_lport) if (filters.get('network_id') and len(filters.get('network_id')) and self._network_is_external(context, filters['network_id'][0])): # Do not perform check on NVP platform @@ -1173,7 +1173,7 @@ class NvpPluginV2(db_base_plugin_v2.QuantumDbPluginV2, port_data[ext_sg.SECURITYGROUPS] = ( self._get_security_groups_on_port(context, port)) self._process_port_create_security_group( - context, quantum_db['id'], port_data[ext_sg.SECURITYGROUPS]) + context, port_data, port_data[ext_sg.SECURITYGROUPS]) # QoS extension checks port_data[ext_qos.QUEUE] = self._check_for_queue_and_create( context, port_data) @@ -1201,7 +1201,6 @@ class NvpPluginV2(db_base_plugin_v2.QuantumDbPluginV2, # 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_dict_security_group(context, port_data) self._extend_port_qos_queue(context, port_data) net = self.get_network(context, port_data['network_id']) self.schedule_network(context, net) @@ -1218,7 +1217,9 @@ class NvpPluginV2(db_base_plugin_v2.QuantumDbPluginV2, with context.session.begin(subtransactions=True): ret_port = super(NvpPluginV2, self).update_port( context, id, port) - # copy values over + # copy values over - except fixed_ips as + # they've alreaby been processed + 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 @@ -1246,7 +1247,8 @@ class NvpPluginV2(db_base_plugin_v2.QuantumDbPluginV2, # delete the port binding and read it with the new rules. self._delete_port_security_group_bindings(context, id) sgids = self._get_security_groups_on_port(context, port) - self._process_port_create_security_group(context, id, sgids) + self._process_port_create_security_group(context, ret_port, + sgids) if psec.PORTSECURITY in port['port']: self._update_port_security_binding( @@ -1257,8 +1259,7 @@ class NvpPluginV2(db_base_plugin_v2.QuantumDbPluginV2, 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) - self._extend_port_dict_security_group(context, ret_port) - LOG.debug(_("Update port request: %s"), port) + LOG.warn(_("Update port request: %s"), port) nvp_port_id = self._nvp_get_port_id( context, self.cluster, ret_port) nvplib.update_port(self.cluster, @@ -1335,7 +1336,6 @@ class NvpPluginV2(db_base_plugin_v2.QuantumDbPluginV2, quantum_db_port = super(NvpPluginV2, self).get_port(context, id, fields) self._extend_port_port_security_dict(context, quantum_db_port) - self._extend_port_dict_security_group(context, quantum_db_port) self._extend_port_qos_queue(context, quantum_db_port) if self._network_is_external(context, diff --git a/quantum/plugins/openvswitch/ovs_quantum_plugin.py b/quantum/plugins/openvswitch/ovs_quantum_plugin.py index be937395a..819be0be1 100644 --- a/quantum/plugins/openvswitch/ovs_quantum_plugin.py +++ b/quantum/plugins/openvswitch/ovs_quantum_plugin.py @@ -563,9 +563,7 @@ class OVSQuantumPluginV2(db_base_plugin_v2.QuantumDbPluginV2, self._ensure_default_security_group_on_port(context, port) sgids = self._get_security_groups_on_port(context, port) port = super(OVSQuantumPluginV2, self).create_port(context, port) - self._process_port_create_security_group( - context, port['id'], sgids) - self._extend_port_dict_security_group(context, port) + self._process_port_create_security_group(context, port, sgids) self.notify_security_groups_member_updated(context, port) return self._extend_port_dict_binding(context, port) @@ -573,7 +571,6 @@ class OVSQuantumPluginV2(db_base_plugin_v2.QuantumDbPluginV2, with context.session.begin(subtransactions=True): port = super(OVSQuantumPluginV2, self).get_port(context, id, fields) - self._extend_port_dict_security_group(context, port) self._extend_port_dict_binding(context, port) return self._fields(port, fields) @@ -586,13 +583,11 @@ class OVSQuantumPluginV2(db_base_plugin_v2.QuantumDbPluginV2, page_reverse) #TODO(nati) filter by security group for port in ports: - self._extend_port_dict_security_group(context, port) self._extend_port_dict_binding(context, port) return [self._fields(port, fields) for port in ports] def update_port(self, context, id, port): session = context.session - need_port_update_notify = False with session.begin(subtransactions=True): original_port = super(OVSQuantumPluginV2, self).get_port( @@ -601,10 +596,8 @@ class OVSQuantumPluginV2(db_base_plugin_v2.QuantumDbPluginV2, context, id, port) need_port_update_notify = self.update_security_group_on_port( context, id, port, original_port, updated_port) - need_port_update_notify |= self.is_security_group_member_updated( context, original_port, updated_port) - if original_port['admin_state_up'] != updated_port['admin_state_up']: need_port_update_notify = True @@ -615,7 +608,6 @@ class OVSQuantumPluginV2(db_base_plugin_v2.QuantumDbPluginV2, binding.network_type, binding.segmentation_id, binding.physical_network) - return self._extend_port_dict_binding(context, updated_port) def delete_port(self, context, id, l3_port_check=True): diff --git a/quantum/plugins/ryu/ryu_quantum_plugin.py b/quantum/plugins/ryu/ryu_quantum_plugin.py index ddd6029bb..74468cfeb 100644 --- a/quantum/plugins/ryu/ryu_quantum_plugin.py +++ b/quantum/plugins/ryu/ryu_quantum_plugin.py @@ -100,7 +100,6 @@ class RyuQuantumPluginV2(db_base_plugin_v2.QuantumDbPluginV2, def __init__(self, configfile=None): db.configure_db() - self.tunnel_key = db_api_v2.TunnelKey( cfg.CONF.OVS.tunnel_key_min, cfg.CONF.OVS.tunnel_key_max) self.ofp_api_host = cfg.CONF.OVS.openflow_rest_api @@ -203,8 +202,7 @@ class RyuQuantumPluginV2(db_base_plugin_v2.QuantumDbPluginV2, sgids = self._get_security_groups_on_port(context, port) port = super(RyuQuantumPluginV2, self).create_port(context, port) self._process_port_create_security_group( - context, port['id'], sgids) - self._extend_port_dict_security_group(context, port) + context, port, sgids) self.notify_security_groups_member_updated(context, port) self.iface_client.create_network_id(port['id'], port['network_id']) return port @@ -253,13 +251,10 @@ class RyuQuantumPluginV2(db_base_plugin_v2.QuantumDbPluginV2, with context.session.begin(subtransactions=True): port = super(RyuQuantumPluginV2, self).get_port(context, id, fields) - self._extend_port_dict_security_group(context, port) return self._fields(port, fields) def get_ports(self, context, filters=None, fields=None): with context.session.begin(subtransactions=True): ports = super(RyuQuantumPluginV2, self).get_ports( context, filters, fields) - for port in ports: - self._extend_port_dict_security_group(context, port) return [self._fields(port, fields) for port in ports] diff --git a/quantum/tests/unit/test_extension_portsecurity.py b/quantum/tests/unit/test_extension_portsecurity.py index 38bf670ba..737347d82 100644 --- a/quantum/tests/unit/test_extension_portsecurity.py +++ b/quantum/tests/unit/test_extension_portsecurity.py @@ -116,9 +116,8 @@ class PortSecurityTestPlugin(db_base_plugin_v2.QuantumDbPluginV2, if (p.get(ext_sg.SECURITYGROUPS) and p[psec.PORTSECURITY]): self._process_port_create_security_group( - context, p['id'], p[ext_sg.SECURITYGROUPS]) + context, p, p[ext_sg.SECURITYGROUPS]) - self._extend_port_dict_security_group(context, p) self._extend_port_port_security_dict(context, p) return port['port'] @@ -132,7 +131,8 @@ class PortSecurityTestPlugin(db_base_plugin_v2.QuantumDbPluginV2, with context.session.begin(subtransactions=True): ret_port = super(PortSecurityTestPlugin, self).update_port( context, id, port) - # copy values over + # copy values over - but not fixed_ips + port['port'].pop('fixed_ips', None) ret_port.update(port['port']) # populate port_security setting @@ -164,14 +164,16 @@ class PortSecurityTestPlugin(db_base_plugin_v2.QuantumDbPluginV2, # delete the port binding and read it with the new rules. self._delete_port_security_group_bindings(context, id) sgids = self._get_security_groups_on_port(context, port) - self._process_port_create_security_group(context, id, sgids) + # process port create sec groups needs port id + port['id'] = id + self._process_port_create_security_group(context, + 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._extend_port_dict_security_group(context, ret_port) return ret_port @@ -301,13 +303,12 @@ class TestPortSecurity(PortSecurityDBTestCase): psec.PORTSECURITY: False}} req = self.new_update_request('ports', update_port, port['port']['id']) - port = self.deserialize('json', req.get_response(self.api)) self.assertEqual(port['port'][psec.PORTSECURITY], False) self.assertEqual(len(port['port'][ext_sg.SECURITYGROUPS]), 0) self._delete('ports', port['port']['id']) - def test_update_port_remove_port_security_security_group_readd(self): + def test_update_port_remove_port_security_security_group_read(self): if self._skip_security_group: self.skipTest("Plugin does not support security groups") with self.network() as net: diff --git a/quantum/tests/unit/test_extension_security_group.py b/quantum/tests/unit/test_extension_security_group.py index 788cfc66d..cbff55f02 100644 --- a/quantum/tests/unit/test_extension_security_group.py +++ b/quantum/tests/unit/test_extension_security_group.py @@ -192,9 +192,8 @@ class SecurityGroupTestPlugin(db_base_plugin_v2.QuantumDbPluginV2, sgids = self._get_security_groups_on_port(context, port) port = super(SecurityGroupTestPlugin, self).create_port(context, port) - self._process_port_create_security_group(context, port['id'], + self._process_port_create_security_group(context, port, sgids) - self._extend_port_dict_security_group(context, port) return port def update_port(self, context, id, port): @@ -205,11 +204,12 @@ class SecurityGroupTestPlugin(db_base_plugin_v2.QuantumDbPluginV2, self._get_security_groups_on_port(context, port)) # delete the port binding and read it with the new rules self._delete_port_security_group_bindings(context, id) + port['port']['id'] = id self._process_port_create_security_group( - context, id, port['port'].get(ext_sg.SECURITYGROUPS)) + context, port['port'], + port['port'].get(ext_sg.SECURITYGROUPS)) port = super(SecurityGroupTestPlugin, self).update_port( context, id, port) - self._extend_port_dict_security_group(context, port) return port def create_network(self, context, network): @@ -224,8 +224,6 @@ class SecurityGroupTestPlugin(db_base_plugin_v2.QuantumDbPluginV2, quantum_lports = super(SecurityGroupTestPlugin, self).get_ports( context, filters, sorts=sorts, limit=limit, marker=marker, page_reverse=page_reverse) - for quantum_lport in quantum_lports: - self._extend_port_dict_security_group(context, quantum_lport) return quantum_lports @@ -721,11 +719,10 @@ class TestSecurityGroups(SecurityGroupDBTestCase): def test_list_ports_security_group(self): with self.network() as n: with self.subnet(n): - res = self._create_port(self.fmt, n['network']['id']) - self.deserialize(self.fmt, res) - res = self.new_list_request('ports') - ports = self.deserialize(self.fmt, - res.get_response(self.api)) + self._create_port(self.fmt, n['network']['id']) + req = self.new_list_request('ports') + res = req.get_response(self.api) + ports = self.deserialize(self.fmt, res) port = ports['ports'][0] self.assertEqual(len(port[ext_sg.SECURITYGROUPS]), 1) self._delete('ports', port['id'])