From 6ef28110c0c2b11ec63266e4d22a1b7c75cf7f5f Mon Sep 17 00:00:00 2001 From: Akihiro MOTOKI Date: Mon, 23 Sep 2013 18:03:39 +0900 Subject: [PATCH] Fix port deletion in NEC Plugin Cascade on delete from ports.id to packetfitlers.in_port is added to ensure packet filter entries associated with a port. Also joined query of packetfilter with port query is added to avoid additional packetfilter query by port_id. Fixes: bug #1212102 Change-Id: I8a67649f3361480eda6377b1d8a30bebd18aa714 --- .../versions/1064e98b7917_nec_pf_port_del.py | 63 +++++++++++++++++++ neutron/plugins/nec/db/packetfilter.py | 20 +++++- neutron/plugins/nec/nec_plugin.py | 30 ++++----- neutron/tests/unit/nec/test_packet_filter.py | 17 +++++ 4 files changed, 110 insertions(+), 20 deletions(-) create mode 100644 neutron/db/migration/alembic_migrations/versions/1064e98b7917_nec_pf_port_del.py diff --git a/neutron/db/migration/alembic_migrations/versions/1064e98b7917_nec_pf_port_del.py b/neutron/db/migration/alembic_migrations/versions/1064e98b7917_nec_pf_port_del.py new file mode 100644 index 00000000000..a4ddbab5a0c --- /dev/null +++ b/neutron/db/migration/alembic_migrations/versions/1064e98b7917_nec_pf_port_del.py @@ -0,0 +1,63 @@ +# vim: tabstop=4 shiftwidth=4 softtabstop=4 +# +# Copyright 2013 OpenStack Foundation +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +# + +"""nec-pf-port-del + +Revision ID: 1064e98b7917 +Revises: 3d6fae8b70b0 +Create Date: 2013-09-24 05:33:54.602618 + +""" + +# revision identifiers, used by Alembic. +revision = '1064e98b7917' +down_revision = '3d6fae8b70b0' + +# Change to ['*'] if this migration applies to all plugins + +migration_for_plugins = [ + 'neutron.plugins.nec.nec_plugin.NECPluginV2' +] + +from alembic import op +import sqlalchemy as sa + +from neutron.db import migration + + +def upgrade(active_plugins=None, options=None): + if not migration.should_run(active_plugins, migration_for_plugins): + return + + op.alter_column('packetfilters', 'in_port', + existing_type=sa.String(length=36), + nullable=True) + op.create_foreign_key( + 'packetfilters_ibfk_2', + source='packetfilters', referent='ports', + local_cols=['in_port'], remote_cols=['id'], + ondelete='CASCADE') + + +def downgrade(active_plugins=None, options=None): + if not migration.should_run(active_plugins, migration_for_plugins): + return + + op.drop_constraint('packetfilters_ibfk_2', 'packetfilters', 'foreignkey') + op.alter_column('packetfilters', 'in_port', + existing_type=sa.String(length=36), + nullable=False) diff --git a/neutron/plugins/nec/db/packetfilter.py b/neutron/plugins/nec/db/packetfilter.py index a45bb26fe71..724af322ade 100644 --- a/neutron/plugins/nec/db/packetfilter.py +++ b/neutron/plugins/nec/db/packetfilter.py @@ -16,6 +16,7 @@ # @author: Ryota MIBU import sqlalchemy as sa +from sqlalchemy import orm from sqlalchemy.orm import exc as sa_exc from neutron.api.v2 import attributes @@ -43,7 +44,9 @@ class PacketFilter(model_base.BASEV2, models_v2.HasId, models_v2.HasTenant): priority = sa.Column(sa.Integer, nullable=False) action = sa.Column(sa.String(16), nullable=False) # condition - in_port = sa.Column(sa.String(36), nullable=False) + in_port = sa.Column(sa.String(36), + sa.ForeignKey('ports.id', ondelete="CASCADE"), + nullable=True) src_mac = sa.Column(sa.String(32), nullable=False) dst_mac = sa.Column(sa.String(32), nullable=False) eth_type = sa.Column(sa.Integer, nullable=False) @@ -56,6 +59,16 @@ class PacketFilter(model_base.BASEV2, models_v2.HasId, models_v2.HasTenant): admin_state_up = sa.Column(sa.Boolean(), nullable=False) status = sa.Column(sa.String(16), nullable=False) + network = orm.relationship( + models_v2.Network, + backref=orm.backref('packetfilters', lazy='joined', cascade='delete'), + uselist=False) + in_port_ref = orm.relationship( + models_v2.Port, + backref=orm.backref('packetfilters', lazy='joined', cascade='delete'), + primaryjoin="Port.id==PacketFilter.in_port", + uselist=False) + class PacketFilterDbMixin(object): @@ -127,7 +140,10 @@ class PacketFilterDbMixin(object): 'protocol': pf_dict['protocol']} for key, default in params.items(): if params[key] == attributes.ATTR_NOT_SPECIFIED: - params.update({key: ''}) + if key == 'in_port': + params[key] = None + else: + params[key] = '' with context.session.begin(subtransactions=True): pf_entry = PacketFilter(**params) diff --git a/neutron/plugins/nec/nec_plugin.py b/neutron/plugins/nec/nec_plugin.py index 0f11ffec0db..a9caa0f7c3e 100644 --- a/neutron/plugins/nec/nec_plugin.py +++ b/neutron/plugins/nec/nec_plugin.py @@ -336,8 +336,8 @@ class NECPluginV2(db_base_plugin_v2.NeutronDbPluginV2, of the tenant, delete unnessary ofc_tenant. """ LOG.debug(_("NECPluginV2.delete_network() called, id=%s ."), id) - net = super(NECPluginV2, self).get_network(context, id) - tenant_id = net['tenant_id'] + net_db = self._get_network(context, id) + tenant_id = net_db['tenant_id'] ports = self.get_ports(context, filters={'network_id': [id]}) # check if there are any tenant owned ports in-use @@ -358,19 +358,16 @@ class NECPluginV2(db_base_plugin_v2.NeutronDbPluginV2, ','.join(_error_ports)) raise nexc.OFCException(reason=reason) - # delete all packet_filters of the network - if self.packet_filter_enabled: - filters = dict(network_id=[id]) - pfs = self.get_packet_filters(context, filters=filters) - for pf in pfs: - self.delete_packet_filter(context, pf['id']) + # delete all packet_filters of the network from the controller + for pf in net_db.packetfilters: + self.delete_packet_filter(context, pf['id']) try: - self.ofc.delete_ofc_network(context, id, net) + self.ofc.delete_ofc_network(context, id, net_db) except (nexc.OFCException, nexc.OFCConsistencyBroken) as exc: reason = _("delete_network() failed due to %s") % exc LOG.error(reason) - self._update_resource_status(context, "network", net['id'], + self._update_resource_status(context, "network", net_db['id'], const.NET_STATUS_ERROR) raise @@ -591,21 +588,18 @@ class NECPluginV2(db_base_plugin_v2.NeutronDbPluginV2, # ext_sg.SECURITYGROUPS attribute for the port is required # since notifier.security_groups_member_updated() need the attribute. # Thus we need to call self.get_port() instead of super().get_port() - port = self.get_port(context, id) + port_db = self._get_port(context, id) + port = self._make_port_dict(port_db) handler = self._get_port_handler('delete', port['device_owner']) port = handler(context, port) - # port = self.deactivate_port(context, port) if port['status'] == const.PORT_STATUS_ERROR: reason = _("Failed to delete port=%s from OFC.") % id raise nexc.OFCException(reason=reason) - # delete all packet_filters of the port - if self.packet_filter_enabled: - filters = dict(port_id=[id]) - pfs = self.get_packet_filters(context, filters=filters) - for packet_filter in pfs: - self.delete_packet_filter(context, packet_filter['id']) + # delete all packet_filters of the port from the controller + for pf in port_db.packetfilters: + self.delete_packet_filter(context, pf['id']) # if needed, check to see if this is a port owned by # and l3-router. If so, we should prevent deletion. diff --git a/neutron/tests/unit/nec/test_packet_filter.py b/neutron/tests/unit/nec/test_packet_filter.py index ca318be4540..b58c96e0dea 100644 --- a/neutron/tests/unit/nec/test_packet_filter.py +++ b/neutron/tests/unit/nec/test_packet_filter.py @@ -466,6 +466,23 @@ class TestNecPluginPacketFilter(test_nec_plugin.NecPluginV2TestCase): self._show('packet_filters', pf_id, expected_code=webob.exc.HTTPNotFound.code) + def test_auto_delete_pf_in_port_deletion(self): + with self.port(no_delete=True) as port: + network = self._show('networks', port['port']['network_id']) + + with self.packet_filter_on_network(network=network) as pfn: + with self.packet_filter_on_port(port=port, do_delete=False, + set_portinfo=False) as pf: + pf_id = pf['packet_filter']['id'] + in_port_id = pf['packet_filter']['in_port'] + + self._delete('ports', in_port_id) + # Check the packet filter on the port is deleted. + self._show('packet_filters', pf_id, + expected_code=webob.exc.HTTPNotFound.code) + # Check the packet filter on the network is not deleted. + self._show('packet_filters', pfn['packet_filter']['id']) + def test_no_pf_activation_while_port_operations(self): with self.packet_filter_on_port() as pf: in_port_id = pf['packet_filter']['in_port']