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

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)

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)

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)
+        net_db = self._get_network(context, id)
-        tenant_id = net['tenant_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
+        # delete all packet_filters of the network from the controller
-        if self.packet_filter_enabled:
+        for pf in net_db.packetfilters:
-            filters = dict(network_id=[id])
+            self.delete_packet_filter(context, pf['id'])
-            pfs = self.get_packet_filters(context, filters=filters)
-            for pf in pfs:
-                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
+        # delete all packet_filters of the port from the controller
-        if self.packet_filter_enabled:
+        for pf in port_db.packetfilters:
-            filters = dict(port_id=[id])
+            self.delete_packet_filter(context, pf['id'])
-            pfs = self.get_packet_filters(context, filters=filters)
-            for packet_filter in pfs:
-                self.delete_packet_filter(context, packet_filter['id'])
 
         # if needed, check to see if this is a port owned by
         # and l3-router.  If so, we should prevent deletion.

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']