Merge "Add protocol to port_forwarding uniq constraint"

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

@@ -419,10 +419,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 oslo_utils import uuidutils
 
@@ -90,6 +91,42 @@ class TestExtendFipPortForwardingExtension(
                                          rules=[])
         self.policy_2.create()
 
+    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.