Add protocol to port_forwarding uniq constraint

Floating IP port forwardings with different protocols can not have
the same internal or external port number to the same VM port. But
we can have different application servers, for instance TCP server
and UDP server, listen to the same port at same time.

This patch adds the protocol attribute to the DB uniq constraint
to allow creating different protocol port forwardings with same
internal or external port number.

Co-Authored-By: LIU Yulong <i@liuyulong.me>
Closes-Bug: #1799155
Change-Id: Ifbb5f3ee2473aac98982bff0d2e6bb9b3e5ab5d6

neutron/db/migration/alembic_migrations/versions/EXPAND_HEAD

@@ -1 +1 @@
-867d39095bf4
+d72db3e25539

neutron/db/migration/alembic_migrations/versions/stein/expand/d72db3e25539_modify_uniq_port_forwarding.py

@@ -0,0 +1,83 @@
+# Copyright 2018 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.
+#
+
+"""modify uniq port forwarding
+
+Revision ID: d72db3e25539
+Revises: 867d39095bf4
+Create Date: 2018-10-12 19:51:11.981394
+
+"""
+
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy.engine import reflection
+
+from neutron.db import migration
+
+# revision identifiers, used by Alembic.
+revision = 'd72db3e25539'
+down_revision = '867d39095bf4'
+
+TABLE_NAME = 'portforwardings'
+
+
+def upgrade():
+    inspector = reflection.Inspector.from_engine(op.get_bind())
+    foreign_keys = inspector.get_foreign_keys(TABLE_NAME)
+    migration.remove_foreign_keys(TABLE_NAME, foreign_keys)
+
+    unique_constraints = inspector.get_unique_constraints(TABLE_NAME)
+    for constraint in unique_constraints:
+        op.drop_constraint(
+            constraint_name=constraint['name'],
+            table_name=TABLE_NAME,
+            type_="unique"
+        )
+
+    op.create_unique_constraint(
+        constraint_name=('uniq_port_forwardings0floatingip_id0'
+                         'external_port0protocol'),
+        table_name=TABLE_NAME,
+        columns=['floatingip_id', 'external_port', 'protocol']
+    )
+    op.create_unique_constraint(
+        constraint_name=('uniq_port_forwardings0internal_neutron_port_id0'
+                         'socket0protocol'),
+        table_name=TABLE_NAME,
+        columns=['internal_neutron_port_id', 'socket', 'protocol']
+    )
+
+    migration.create_foreign_keys(TABLE_NAME, foreign_keys)
+
+
+def expand_drop_exceptions():
+    """Drop and replace the unique constraints for table portforwardings
+
+    Drop the existing portforwardings foreign key uniq constraints and then
+    replace them with new unique constraints with column ``protocol``.
+    This is needed to use drop in expand migration to pass test_branches.
+    """
+
+    return {
+        sa.Constraint: [
+            "portforwardings_ibfk_1",
+            "portforwardings_ibfk_2",
+            "uniq_port_forwardings0floatingip_id0external_port",
+            "uniq_port_forwardings0internal_neutron_port_id0socket",
+            "portforwardings_floatingip_id_fkey",
+            "portforwardings_internal_neutron_port_id_fkey",
+        ]
+    }

neutron/db/models/port_forwarding.py

@@ -25,12 +25,13 @@ from neutron_lib.db import constants as db_const
 class PortForwarding(model_base.BASEV2, model_base.HasId):
 
     __table_args__ = (
-        sa.UniqueConstraint('floatingip_id', 'external_port',
+        sa.UniqueConstraint('floatingip_id', 'external_port', 'protocol',
                             name='uniq_port_forwardings0floatingip_id0'
-                                 'external_port'),
+                                 'external_port0protocol'),
-        sa.UniqueConstraint('internal_neutron_port_id', 'socket',
+        sa.UniqueConstraint('internal_neutron_port_id', 'socket', 'protocol',
                             name='uniq_port_forwardings0'
-                                 'internal_neutron_port_id0socket'),
+                                 'internal_neutron_port_id0socket0'
+                                 'protocol')
     )
 
     floatingip_id = sa.Column(sa.String(db_const.UUID_FIELD_SIZE),

neutron/objects/port_forwarding.py

@@ -30,7 +30,8 @@ FIELDS_NOT_SUPPORT_FILTER = ['internal_ip_address', 'internal_port']
 @base.NeutronObjectRegistry.register
 class PortForwarding(base.NeutronDbObject):
     # Version 1.0: Initial version
-    VERSION = '1.0'
+    # Version 1.1: Change unique constraint
+    VERSION = '1.1'
 
     db_model = models.PortForwarding
 

neutron/services/portforwarding/pf_plugin.py

@@ -403,10 +403,12 @@ class PortForwardingPlugin(fip_pf.PortForwardingPluginBase):
         if not specify_params:
             specify_params = [
                 {'floatingip_id': floatingip_id,
-                 'external_port': port_forwarding['external_port']},
+                 'external_port': port_forwarding['external_port'],
+                 'protocol': port_forwarding['protocol']},
                 {'internal_port_id': port_forwarding['internal_port_id'],
                  'internal_ip_address': port_forwarding['internal_ip_address'],
-                 'internal_port': port_forwarding['internal_port']}]
+                 'internal_port': port_forwarding['internal_port'],
+                 'protocol': port_forwarding['protocol']}]
         for param in specify_params:
             objs = pf.PortForwarding.get_objects(context, **param)
             if objs:

neutron/tests/unit/extensions/test_expose_port_forwarding_in_fip.py

@@ -14,6 +14,7 @@
 import mock
 from neutron_lib.api.definitions import external_net as extnet_apidef
 from neutron_lib.api.definitions import floating_ip_port_forwarding as apidef
+from neutron_lib import constants
 from neutron_lib import context
 
 from neutron.extensions import floating_ip_port_forwarding as pf_ext
@@ -65,6 +66,42 @@ class TestExtendFipPortForwardingExtension(
             plugin=CORE_PLUGIN, ext_mgr=ext_mgr, service_plugins=svc_plugins)
         self.pf_plugin = pf_plugin.PortForwardingPlugin()
 
+    def test_create_floatingip_port_forwarding_same_port_diff_protocol(self):
+        port_forwarding = {
+            apidef.RESOURCE_NAME:
+                {apidef.EXTERNAL_PORT: 2225,
+                 apidef.INTERNAL_PORT: 25,
+                 apidef.INTERNAL_PORT_ID: None,
+                 apidef.PROTOCOL: constants.PROTO_NAME_TCP,
+                 apidef.INTERNAL_IP_ADDRESS: None}}
+        ctx = context.get_admin_context()
+        kwargs = {'arg_list': (extnet_apidef.EXTERNAL,),
+                  extnet_apidef.EXTERNAL: True}
+        with self.network(**kwargs) as extnet, self.network() as innet:
+            with self.subnet(network=extnet, cidr='200.0.0.0/22'), \
+                 self.subnet(network=innet, cidr='10.0.0.0/24') as insub, \
+                    self.router() as router:
+                fip = self._make_floatingip(self.fmt, extnet['network']['id'])
+                self._add_external_gateway_to_router(router['router']['id'],
+                                                     extnet['network']['id'])
+                self._router_interface_action('add', router['router']['id'],
+                                              insub['subnet']['id'], None)
+                with self.port(subnet=insub) as port1:
+                    update_dict1 = {
+                        apidef.INTERNAL_PORT_ID: port1['port']['id'],
+                        apidef.INTERNAL_IP_ADDRESS:
+                            port1['port']['fixed_ips'][0]['ip_address']}
+                    port_forwarding[apidef.RESOURCE_NAME].update(update_dict1)
+                    self.pf_plugin.create_floatingip_port_forwarding(
+                        ctx, fip['floatingip']['id'], port_forwarding)
+
+                    update_dict2 = {
+                        apidef.PROTOCOL: constants.PROTO_NAME_UDP
+                    }
+                    port_forwarding[apidef.RESOURCE_NAME].update(update_dict2)
+                    self.pf_plugin.create_floatingip_port_forwarding(
+                        ctx, fip['floatingip']['id'], port_forwarding)
+
     def test_get_fip_after_port_forwarding_create(self):
         port_forwarding = {
             apidef.RESOURCE_NAME:

neutron/tests/unit/objects/test_objects.py

@@ -67,7 +67,7 @@ object_data = {
     'PortBindingLevel': '1.1-50d47f63218f87581b6cd9a62db574e5',
     'PortDataPlaneStatus': '1.0-25be74bda46c749653a10357676c0ab2',
     'PortDNS': '1.1-c5ca2dc172bdd5fafee3fc986d1d7023',
-    'PortForwarding': '1.0-db61273978c497239be5389a8aeb1c61',
+    'PortForwarding': '1.1-db61273978c497239be5389a8aeb1c61',
     'PortSecurity': '1.0-b30802391a87945ee9c07582b4ff95e3',
     'ProviderResourceAssociation': '1.0-05ab2d5a3017e5ce9dd381328f285f34',
     'ProvisioningBlock': '1.0-c19d6d05bfa8143533471c1296066125',

releasenotes/notes/change-port-forwarding-uniq-constraint-78ba3db20bce5fd2.yaml

@@ -0,0 +1,12 @@
+---
+upgrade:
+  - |
+    Adds Floating IP port forwarding table column ``protocol`` to the uniq
+    constraints. In one expand script, we drop the original uniq constraints
+    first, then create the new uniq constraints with column ``protocol``.
+fixes:
+  - |
+    Floating IP port forwardings with different protocols could not have the
+    same internal or external port number to the same VM port. After this
+    fix we will allow creating port forwardings with same internal or
+    external port number in different protocols.