diff --git a/neutron/agent/l3/dvr.py b/neutron/agent/l3/dvr.py index bb4c1a43a58..38f01b4a69f 100644 --- a/neutron/agent/l3/dvr.py +++ b/neutron/agent/l3/dvr.py @@ -74,4 +74,5 @@ class AgentMixin(object): """Delete fip namespace after external network removed.""" fip_ns = self.get_fip_ns(ext_net_id) if fip_ns.agent_gateway_port and not fip_ns.destroyed: + fip_ns.unsubscribe(ext_net_id) fip_ns.delete() diff --git a/neutron/agent/l3/dvr_fip_ns.py b/neutron/agent/l3/dvr_fip_ns.py index 6d4cf7bc3dc..654a7d24365 100644 --- a/neutron/agent/l3/dvr_fip_ns.py +++ b/neutron/agent/l3/dvr_fip_ns.py @@ -79,13 +79,13 @@ class FipNamespace(namespaces.Namespace): def has_subscribers(self): return len(self._subscribers) != 0 - def subscribe(self, router_id): + def subscribe(self, external_net_id): is_first = not self.has_subscribers() - self._subscribers.add(router_id) + self._subscribers.add(external_net_id) return is_first - def unsubscribe(self, router_id): - self._subscribers.discard(router_id) + def unsubscribe(self, external_net_id): + self._subscribers.discard(external_net_id) return not self.has_subscribers() def allocate_rule_priority(self, floating_ip): diff --git a/neutron/agent/l3/dvr_local_router.py b/neutron/agent/l3/dvr_local_router.py index 563c88a9220..93101edf66a 100644 --- a/neutron/agent/l3/dvr_local_router.py +++ b/neutron/agent/l3/dvr_local_router.py @@ -133,7 +133,6 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): self.fip_ns.local_subnets.release(self.router_id) self.rtr_fip_subnet = None ns_ip.del_veth(fip_2_rtr_name) - self.fip_ns.unsubscribe(self.router_id) # NOTE (Swami): The fg interface and the namespace will be deleted # when the external gateway port is removed. This will be # initiated from the server through an RPC call. @@ -429,7 +428,7 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): "plugin: %s", fip_agent_port) is_first = False if floating_ips: - is_first = self.fip_ns.subscribe(self.router_id) + is_first = self.fip_ns.subscribe(ex_gw_port['network_id']) if is_first and not fip_agent_port: LOG.debug("No FloatingIP agent gateway port possibly due to " "late binding of the private port to the host, " @@ -448,7 +447,7 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): self.fip_ns.create_gateway_port(fip_agent_port) if (self.fip_ns.agent_gateway_port and - (self.dist_fip_count == 0 or is_first)): + (self.dist_fip_count == 0)): self.fip_ns.create_rtr_2_fip_link(self) # kicks the FW Agent to add rules for the IR namespace if diff --git a/neutron/tests/functional/agent/l3/test_dvr_router.py b/neutron/tests/functional/agent/l3/test_dvr_router.py index e7c5beb59bb..9ef421714b1 100644 --- a/neutron/tests/functional/agent/l3/test_dvr_router.py +++ b/neutron/tests/functional/agent/l3/test_dvr_router.py @@ -19,7 +19,6 @@ import mock import netaddr from neutron.agent.l3 import agent as neutron_l3_agent -from neutron.agent.l3 import dvr_fip_ns from neutron.agent.l3 import dvr_snat_ns from neutron.agent.l3 import namespaces from neutron.agent.linux import ip_lib @@ -673,8 +672,12 @@ class TestDvrRouter(framework.L3AgentTestFramework): def _assert_fip_namespace_deleted(self, ext_gateway_port): ext_net_id = ext_gateway_port['network_id'] + fip_ns = self.agent.get_fip_ns(ext_net_id) + fip_ns.unsubscribe = mock.Mock() self.agent.fipnamespace_delete_on_ext_net( self.agent.context, ext_net_id) self._assert_interfaces_deleted_from_ovs() - fip_ns_name = dvr_fip_ns.FipNamespace._get_ns_name(ext_net_id) + fip_ns_name = fip_ns.get_name() self.assertFalse(self._namespace_exists(fip_ns_name)) + self.assertTrue(fip_ns.destroyed) + self.assertTrue(fip_ns.unsubscribe.called) diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py index 229114a2156..0f01570a0d6 100644 --- a/neutron/tests/unit/agent/l3/test_agent.py +++ b/neutron/tests/unit/agent/l3/test_agent.py @@ -889,6 +889,72 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): ri.add_floating_ip.assert_called_once_with( floating_ips[0], mock.sentinel.interface_name, device) + @mock.patch.object(lla.LinkLocalAllocator, '_write') + def test_create_dvr_fip_interfaces_if_fipnamespace_exist(self, lla_write): + fake_network_id = _uuid() + subnet_id = _uuid() + fake_floatingips = {'floatingips': [ + {'id': _uuid(), + 'floating_ip_address': '20.0.0.3', + 'fixed_ip_address': '192.168.0.1', + 'floating_network_id': _uuid(), + 'port_id': _uuid(), + 'host': HOSTNAME}]} + agent_gateway_port = ( + [{'fixed_ips': [ + {'ip_address': '20.0.0.30', + 'prefixlen': 24, + 'subnet_id': subnet_id}], + 'subnets': [ + {'id': subnet_id, + 'gateway_ip': '20.0.0.1'}], + 'id': _uuid(), + 'network_id': fake_network_id, + 'mac_address': 'ca:fe:de:ad:be:ef'}] + ) + + router = l3_test_common.prepare_router_data(enable_snat=True) + router[l3_constants.FLOATINGIP_KEY] = fake_floatingips['floatingips'] + router[l3_constants.FLOATINGIP_AGENT_INTF_KEY] = agent_gateway_port + router['distributed'] = True + agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) + ri = dvr_router.DvrEdgeRouter( + agent, HOSTNAME, router['id'], router, **self.ri_kwargs) + ext_gw_port = ri.router.get('gw_port') + ri.fip_ns = agent.get_fip_ns(ext_gw_port['network_id']) + ri.dist_fip_count = 0 + agent.process_router_add = mock.Mock() + ri.fip_ns.create_rtr_2_fip_link = mock.Mock() + with mock.patch.object(ri, 'get_floating_ips') as fips, \ + mock.patch.object(ri.fip_ns, + 'create') as create_fip, \ + mock.patch.object(ri, 'get_floating_agent_gw_interface' + ) as fip_gw_port: + fips.return_value = fake_floatingips + fip_gw_port.return_value = agent_gateway_port[0] + ri.create_dvr_fip_interfaces(ext_gw_port) + self.assertTrue(fip_gw_port.called) + self.assertTrue(fips.called) + self.assertTrue(create_fip.called) + self.assertEqual(agent_gateway_port[0], + ri.fip_ns.agent_gateway_port) + # Now let us associate the fip to the router + ri.floating_ip_added_dist(fips, "192.168.0.1/32") + self.assertEqual(1, ri.dist_fip_count) + # Now let us disassociate the fip from the router + ri.floating_ip_removed_dist("192.168.0.1/32") + self.assertEqual(0, ri.dist_fip_count) + # Calling create_dvr_fip_interfaces again to make sure + # that the fip namespace create is not called again. + # If the create is not called again, that would contain + # the duplicate rules configuration in the fip namespace. + ri.create_dvr_fip_interfaces(ext_gw_port) + self.assertTrue(fip_gw_port.called) + self.assertTrue(fips.called) + create_fip.assert_called_once_with() + self.assertEqual(2, agent.process_router_add.call_count) + self.assertEqual(2, ri.fip_ns.create_rtr_2_fip_link.call_count) + @mock.patch.object(lla.LinkLocalAllocator, '_write') def test_create_dvr_fip_interfaces_for_late_binding(self, lla_write): fake_network_id = _uuid() @@ -1020,7 +1086,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): ri.fip_ns.subscribe = mock.Mock(return_value=True) ri.fip_ns.agent_router_gateway = mock.Mock() ri.rtr_fip_subnet = None - ri.dist_fip_count = 1 + ri.dist_fip_count = 0 with mock.patch.object(ri, 'get_floating_ips') as fips,\ mock.patch.object(ri, 'get_floating_agent_gw_interface' diff --git a/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py b/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py index e5b2f9ac188..5b67e56787d 100644 --- a/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py +++ b/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py @@ -39,23 +39,23 @@ class TestDvrFipNs(base.BaseTestCase): use_ipv6=True) def test_subscribe(self): - is_first = self.fip_ns.subscribe(mock.sentinel.router_id) + is_first = self.fip_ns.subscribe(mock.sentinel.external_net_id) self.assertTrue(is_first) def test_subscribe_not_first(self): - self.fip_ns.subscribe(mock.sentinel.router_id) - is_first = self.fip_ns.subscribe(mock.sentinel.router_id2) + self.fip_ns.subscribe(mock.sentinel.external_net_id) + is_first = self.fip_ns.subscribe(mock.sentinel.external_net_id2) self.assertFalse(is_first) def test_unsubscribe(self): - self.fip_ns.subscribe(mock.sentinel.router_id) - is_last = self.fip_ns.unsubscribe(mock.sentinel.router_id) + self.fip_ns.subscribe(mock.sentinel.external_net_id) + is_last = self.fip_ns.unsubscribe(mock.sentinel.external_net_id) self.assertTrue(is_last) def test_unsubscribe_not_last(self): - self.fip_ns.subscribe(mock.sentinel.router_id) - self.fip_ns.subscribe(mock.sentinel.router_id2) - is_last = self.fip_ns.unsubscribe(mock.sentinel.router_id2) + self.fip_ns.subscribe(mock.sentinel.external_net_id) + self.fip_ns.subscribe(mock.sentinel.external_net_id2) + is_last = self.fip_ns.unsubscribe(mock.sentinel.external_net_id2) self.assertFalse(is_last) def test_allocate_rule_priority(self): diff --git a/neutron/tests/unit/agent/l3/test_dvr_local_router.py b/neutron/tests/unit/agent/l3/test_dvr_local_router.py index b200cf57e87..95229a93c77 100644 --- a/neutron/tests/unit/agent/l3/test_dvr_local_router.py +++ b/neutron/tests/unit/agent/l3/test_dvr_local_router.py @@ -252,7 +252,6 @@ class TestDvrRouterOperations(base.BaseTestCase): mIPRule().rule.delete.assert_called_with( ip=str(netaddr.IPNetwork(fip_cidr).ip), table=16, priority=FIP_PRI) mIPDevice().route.delete_route.assert_called_with(fip_cidr, str(s.ip)) - self.assertFalse(ri.fip_ns.unsubscribe.called) ri.fip_ns.local_subnets.allocate.assert_not_called() ri.dist_fip_count = 1 @@ -267,7 +266,7 @@ class TestDvrRouterOperations(base.BaseTestCase): fip_ns.get_int_device_name(router['id'])) mIPDevice().route.delete_gateway.assert_called_once_with( str(fip_to_rtr.ip), table=16) - fip_ns.unsubscribe.assert_called_once_with(ri.router_id) + self.assertFalse(ri.fip_ns.unsubscribe.called) ri.fip_ns.local_subnets.allocate.assert_called_once_with(ri.router_id) def _test_add_floating_ip(self, ri, fip, is_failure):