From: Salvatore Orlando <salv.orlando@gmail.com>
Date: Mon, 10 Feb 2014 21:03:08 +0000 (-0800)
Subject: NSX plugin: fix floatingip re-association
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=37cc1fc89b22316ae7de18c41c3b710b234a3014;p=openstack-build%2Fneutron-build.git

NSX plugin: fix floatingip re-association

The NSX plugin does not allow to reassociate a floating IP to
a different internal IP address on the same port where it's
currently associated.
This patch fixes this behaviour and adds a unit test to ensure
re-association on the same port with a different IP is possible.

A few tweaks to the unit test aux functions were necessary to
accomodate the newly introduced unit test.

Change-Id: Iafbc3c54ebc4509ca75155ef138cc6da869df7bd
Closes-Bug: #1278581
---

diff --git a/neutron/plugins/nicira/NeutronPlugin.py b/neutron/plugins/nicira/NeutronPlugin.py
index 5fe118f80..ded70319f 100644
--- a/neutron/plugins/nicira/NeutronPlugin.py
+++ b/neutron/plugins/nicira/NeutronPlugin.py
@@ -1780,9 +1780,16 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin,
             raise q_exc.BadRequest(resource='floatingip', msg=msg)
         port_id = internal_ip = router_id = None
         if 'port_id' in fip and fip['port_id']:
-            port_qry = context.session.query(l3_db.FloatingIP)
+            fip_qry = context.session.query(l3_db.FloatingIP)
+            port_id, internal_ip, router_id = self.get_assoc_data(
+                context,
+                fip,
+                floatingip_db['floating_network_id'])
             try:
-                port_qry.filter_by(fixed_port_id=fip['port_id']).one()
+                fip_qry.filter_by(
+                    fixed_port_id=fip['port_id'],
+                    floating_network_id=floatingip_db['floating_network_id'],
+                    fixed_ip_address=internal_ip).one()
                 raise l3.FloatingIPPortAlreadyAssociated(
                     port_id=fip['port_id'],
                     fip_id=floatingip_db['id'],
@@ -1791,10 +1798,6 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin,
                     net_id=floatingip_db['floating_network_id'])
             except sa_exc.NoResultFound:
                 pass
-            port_id, internal_ip, router_id = self.get_assoc_data(
-                context,
-                fip,
-                floatingip_db['floating_network_id'])
         return (port_id, internal_ip, router_id)
 
     def _update_fip_assoc(self, context, fip, floatingip_db, external_port):
diff --git a/neutron/tests/unit/test_l3_plugin.py b/neutron/tests/unit/test_l3_plugin.py
index 1938285f4..2d7fd6821 100644
--- a/neutron/tests/unit/test_l3_plugin.py
+++ b/neutron/tests/unit/test_l3_plugin.py
@@ -22,6 +22,7 @@ import contextlib
 import copy
 
 import mock
+import netaddr
 from oslo.config import cfg
 from webob import exc
 import webtest
@@ -444,11 +445,15 @@ class L3NatTestCaseMixin(object):
                          fip['floatingip']['id'])
 
     @contextlib.contextmanager
-    def floatingip_with_assoc(self, port_id=None, fmt=None,
+    def floatingip_with_assoc(self, port_id=None, fmt=None, fixed_ip=None,
                               set_context=False):
         with self.subnet(cidr='11.0.0.0/24') as public_sub:
             self._set_net_external(public_sub['subnet']['network_id'])
-            with self.port() as private_port:
+            private_port = None
+            if port_id:
+                private_port = self._show('ports', port_id)
+            with test_db_plugin.optional_ctx(private_port,
+                                             self.port) as private_port:
                 with self.router() as r:
                     sid = private_port['port']['fixed_ips'][0]['subnet_id']
                     private_sub = {'subnet': {'id': sid}}
@@ -465,6 +470,7 @@ class L3NatTestCaseMixin(object):
                             fmt or self.fmt,
                             public_sub['subnet']['network_id'],
                             port_id=private_port['port']['id'],
+                            fixed_ip=fixed_ip,
                             set_context=False)
                         yield floatingip
                     finally:
@@ -1255,6 +1261,33 @@ class L3NatTestCaseBase(L3NatTestCaseMixin):
                 self.assertEqual(body['floatingip']['fixed_ip_address'],
                                  ip_address)
 
+    def test_floatingip_update_different_fixed_ip_same_port(self):
+        with self.subnet() as s:
+            ip_range = list(netaddr.IPNetwork(s['subnet']['cidr']))
+            fixed_ips = [{'ip_address': str(ip_range[-3])},
+                         {'ip_address': str(ip_range[-2])}]
+            with self.port(subnet=s, fixed_ips=fixed_ips) as p:
+                with self.floatingip_with_assoc(
+                    port_id=p['port']['id'],
+                    fixed_ip=str(ip_range[-3])) as fip:
+                    body = self._show('floatingips', fip['floatingip']['id'])
+                    self.assertEqual(fip['floatingip']['id'],
+                                     body['floatingip']['id'])
+                    self.assertEqual(fip['floatingip']['port_id'],
+                                     body['floatingip']['port_id'])
+                    self.assertEqual(str(ip_range[-3]),
+                                     body['floatingip']['fixed_ip_address'])
+                    self.assertIsNotNone(body['floatingip']['router_id'])
+                    body_2 = self._update(
+                        'floatingips', fip['floatingip']['id'],
+                        {'floatingip': {'port_id': p['port']['id'],
+                                        'fixed_ip_address': str(ip_range[-2])}
+                         })
+                    self.assertEqual(fip['floatingip']['port_id'],
+                                     body_2['floatingip']['port_id'])
+                    self.assertEqual(str(ip_range[-2]),
+                                     body_2['floatingip']['fixed_ip_address'])
+
     def test_floatingip_update_different_router(self):
         # Create subnet with different CIDRs to account for plugins which
         # do not support overlapping IPs