diff --git a/neutron/plugins/nicira/NeutronPlugin.py b/neutron/plugins/nicira/NeutronPlugin.py index 8b4a468314c..ae052a66db4 100644 --- a/neutron/plugins/nicira/NeutronPlugin.py +++ b/neutron/plugins/nicira/NeutronPlugin.py @@ -1775,16 +1775,6 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, context, fip, floatingip_db['floating_network_id']) - - # Retrieve and delete existing NAT rules, if any - if not router_id and floatingip_db.get('fixed_port_id'): - # This happens if we're disassociating. Need to explicitly - # find the router serving this floating IP - tmp_fip = fip.copy() - tmp_fip['port_id'] = floatingip_db['fixed_port_id'] - _pid, internal_ip, router_id = self.get_assoc_data( - context, tmp_fip, floatingip_db['floating_network_id']) - return (port_id, internal_ip, router_id) def _update_fip_assoc(self, context, fip, floatingip_db, external_port): @@ -1793,6 +1783,8 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, Overrides method from base class. The method is augmented for creating NAT rules in the process. """ + # Store router currently serving the floating IP + old_router_id = floatingip_db.router_id port_id, internal_ip, router_id = self._get_fip_assoc_data( context, fip, floatingip_db) floating_ip = floatingip_db['floating_ip_address'] @@ -1803,14 +1795,31 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, internal_ip, router_id) # Fetch logical port of router's external gateway + # Fetch logical port of router's external gateway + nvp_floating_ips = self._build_ip_address_list( + context.elevated(), external_port['fixed_ips']) + floating_ip = floatingip_db['floating_ip_address'] + # Retrieve and delete existing NAT rules, if any + if old_router_id: + # Retrieve the current internal ip + _p, _s, old_internal_ip = self._internal_fip_assoc_data( + context, {'id': floatingip_db.id, + 'port_id': floatingip_db.fixed_port_id, + 'fixed_ip_address': floatingip_db.fixed_ip_address, + 'tenant_id': floatingip_db.tenant_id}) + nvp_gw_port_id = nvplib.find_router_gw_port( + context, self.cluster, old_router_id)['uuid'] + self._retrieve_and_delete_nat_rules( + context, floating_ip, old_internal_ip, old_router_id) + nvplib.update_lrouter_port_ips( + self.cluster, old_router_id, nvp_gw_port_id, + ips_to_add=[], ips_to_remove=nvp_floating_ips) + + if router_id: nvp_gw_port_id = nvplib.find_router_gw_port( context, self.cluster, router_id)['uuid'] - nvp_floating_ips = self._build_ip_address_list( - context.elevated(), external_port['fixed_ips']) - LOG.debug(_("Address list for NVP logical router " - "port:%s"), nvp_floating_ips) # Re-create NAT rules only if a port id is specified - if 'port_id' in fip and fip['port_id']: + if fip.get('port_id'): try: # Create new NAT rules nvplib.create_lrouter_dnat_rule( @@ -1847,15 +1856,6 @@ class NvpPluginV2(addr_pair_db.AllowedAddressPairsMixin, 'internal_ip': internal_ip}) msg = _("Failed to update NAT rules for floatingip update") raise nvp_exc.NvpPluginException(err_msg=msg) - elif floatingip_db['fixed_port_id']: - # This is a disassociation. - # Remove floating IP address from logical router port - internal_ip = None - nvplib.update_lrouter_port_ips(self.cluster, - router_id, - nvp_gw_port_id, - ips_to_add=[], - ips_to_remove=nvp_floating_ips) floatingip_db.update({'fixed_ip_address': internal_ip, 'fixed_port_id': port_id, diff --git a/neutron/tests/unit/test_l3_plugin.py b/neutron/tests/unit/test_l3_plugin.py index 19fc8949670..0c7aef7002e 100644 --- a/neutron/tests/unit/test_l3_plugin.py +++ b/neutron/tests/unit/test_l3_plugin.py @@ -474,35 +474,43 @@ class L3NatTestCaseMixin(object): r['router']['id'], public_sub['subnet']['network_id']) + @contextlib.contextmanager + def floatingip_no_assoc_with_public_sub( + self, private_sub, fmt=None, set_context=False, public_sub=None): + self._set_net_external(public_sub['subnet']['network_id']) + with self.router() as r: + floatingip = None + try: + self._add_external_gateway_to_router( + r['router']['id'], + public_sub['subnet']['network_id']) + self._router_interface_action('add', r['router']['id'], + private_sub['subnet']['id'], + None) + + floatingip = self._make_floatingip( + fmt or self.fmt, + public_sub['subnet']['network_id'], + set_context=set_context) + yield floatingip, r + finally: + if floatingip: + self._delete('floatingips', + floatingip['floatingip']['id']) + self._router_interface_action('remove', r['router']['id'], + private_sub['subnet']['id'], + None) + self._remove_external_gateway_from_router( + r['router']['id'], + public_sub['subnet']['network_id']) + @contextlib.contextmanager def floatingip_no_assoc(self, private_sub, fmt=None, set_context=False): with self.subnet(cidr='12.0.0.0/24') as public_sub: - self._set_net_external(public_sub['subnet']['network_id']) - with self.router() as r: - floatingip = None - try: - self._add_external_gateway_to_router( - r['router']['id'], - public_sub['subnet']['network_id']) - self._router_interface_action('add', r['router']['id'], - private_sub['subnet']['id'], - None) - - floatingip = self._make_floatingip( - fmt or self.fmt, - public_sub['subnet']['network_id'], - set_context=set_context) - yield floatingip - finally: - if floatingip: - self._delete('floatingips', - floatingip['floatingip']['id']) - self._router_interface_action('remove', r['router']['id'], - private_sub['subnet']['id'], - None) - self._remove_external_gateway_from_router( - r['router']['id'], - public_sub['subnet']['network_id']) + with self.floatingip_no_assoc_with_public_sub( + private_sub, fmt, set_context, public_sub) as (f, r): + # Yield only the floating ip object + yield f class L3NatTestCaseBase(L3NatTestCaseMixin): @@ -1243,6 +1251,68 @@ class L3NatTestCaseBase(L3NatTestCaseMixin): self.assertEqual(body['floatingip']['fixed_ip_address'], ip_address) + def test_floatingip_update_different_router(self): + # Create subnet with different CIDRs to account for plugins which + # do not support overlapping IPs + with contextlib.nested(self.subnet(cidr='10.0.0.0/24'), + self.subnet(cidr='10.0.1.0/24')) as ( + s1, s2): + with contextlib.nested(self.port(subnet=s1), + self.port(subnet=s2)) as (p1, p2): + private_sub1 = {'subnet': + {'id': + p1['port']['fixed_ips'][0]['subnet_id']}} + private_sub2 = {'subnet': + {'id': + p2['port']['fixed_ips'][0]['subnet_id']}} + with self.subnet(cidr='12.0.0.0/24') as public_sub: + with contextlib.nested( + self.floatingip_no_assoc_with_public_sub( + private_sub1, public_sub=public_sub), + self.floatingip_no_assoc_with_public_sub( + private_sub2, public_sub=public_sub)) as ( + (fip1, r1), (fip2, r2)): + + def assert_no_assoc(fip): + body = self._show('floatingips', + fip['floatingip']['id']) + self.assertEqual(body['floatingip']['port_id'], + None) + self.assertIsNone( + body['floatingip']['fixed_ip_address'], None) + + assert_no_assoc(fip1) + assert_no_assoc(fip2) + + def associate_and_assert(fip, port): + port_id = port['port']['id'] + ip_address = (port['port']['fixed_ips'] + [0]['ip_address']) + body = self._update( + 'floatingips', fip['floatingip']['id'], + {'floatingip': {'port_id': port_id}}) + self.assertEqual(body['floatingip']['port_id'], + port_id) + self.assertEqual( + body['floatingip']['fixed_ip_address'], + ip_address) + return body['floatingip']['router_id'] + + fip1_r1_res = associate_and_assert(fip1, p1) + self.assertEqual(fip1_r1_res, r1['router']['id']) + # The following operation will associate the floating + # ip to a different router + fip1_r2_res = associate_and_assert(fip1, p2) + self.assertEqual(fip1_r2_res, r2['router']['id']) + fip2_r1_res = associate_and_assert(fip2, p1) + self.assertEqual(fip2_r1_res, r1['router']['id']) + # disassociate fip1 + self._update( + 'floatingips', fip1['floatingip']['id'], + {'floatingip': {'port_id': None}}) + fip2_r2_res = associate_and_assert(fip2, p2) + self.assertEqual(fip2_r2_res, r2['router']['id']) + def test_floatingip_with_assoc(self): with self.floatingip_with_assoc() as fip: body = self._show('floatingips', fip['floatingip']['id'])