Merge "Configure floating IPs addresses after NAT rules"

neutron/agent/l3_agent.py

@@ -428,14 +428,18 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager):
         ri.perform_snat_action(self._handle_router_snat_rules,
                                internal_cidrs, interface_name)
 
-        # Process DNAT rules for floating IPs
+        # Process SNAT/DNAT rules for floating IPs
         if ex_gw_port:
-            self.process_router_floating_ips(ri, ex_gw_port)
+            self.process_router_floating_ip_nat_rules(ri)
 
         ri.ex_gw_port = ex_gw_port
         ri.enable_snat = ri.router.get('enable_snat')
         self.routes_updated(ri)
         ri.iptables_manager.defer_apply_off()
+        # Once NAT rules for floating IPs are safely in place
+        # configure their addresses on the external gateway port
+        if ex_gw_port:
+            self.process_router_floating_ip_addresses(ri, ex_gw_port)
 
     def _handle_router_snat_rules(self, ri, ex_gw_port, internal_cidrs,
                                   interface_name, action):
@@ -459,19 +463,34 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager):
                 ri.iptables_manager.ipv4['nat'].add_rule(*rule)
         ri.iptables_manager.apply()
 
-    def process_router_floating_ips(self, ri, ex_gw_port):
+    def process_router_floating_ip_nat_rules(self, ri):
-        """Configure the router's floating IPs
+        """Configure NAT rules for the router's floating IPs.
-        Configures floating ips in iptables and on the router's gateway device.
 
-        Cleans up floating ips that should not longer be configured.
+        Configures iptables rules for the floating ips of the given router
+        """
+        # Clear out all iptables rules for floating ips
+        ri.iptables_manager.ipv4['nat'].clear_rules_by_tag('floating_ip')
+
+        # Loop once to ensure that floating ips are configured.
+        for fip in ri.router.get(l3_constants.FLOATINGIP_KEY, []):
+            # Rebuild iptables rules for the floating ip.
+            fixed = fip['fixed_ip_address']
+            fip_ip = fip['floating_ip_address']
+            for chain, rule in self.floating_forward_rules(fip_ip, fixed):
+                ri.iptables_manager.ipv4['nat'].add_rule(chain, rule,
+                                                         tag='floating_ip')
+
+        ri.iptables_manager.apply()
+
+    def process_router_floating_ip_addresses(self, ri, ex_gw_port):
+        """Configure IP addresses on router's external gateway interface.
+
+        Ensures addresses for existing floating IPs and cleans up
+        those that should not longer be configured.
         """
         interface_name = self.get_external_device_name(ex_gw_port['id'])
         device = ip_lib.IPDevice(interface_name, self.root_helper,
                                  namespace=ri.ns_name())
-
-        # Clear out all iptables rules for floating ips
-        ri.iptables_manager.ipv4['nat'].clear_rules_by_tag('floating_ip')
-
         existing_cidrs = set([addr['cidr'] for addr in device.addr.list()])
         new_cidrs = set()
 
@@ -487,14 +506,6 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager):
                 device.addr.add(net.version, ip_cidr, str(net.broadcast))
                 self._send_gratuitous_arp_packet(ri, interface_name, fip_ip)
 
-            # Rebuild iptables rules for the floating ip.
-            fixed = fip['fixed_ip_address']
-            for chain, rule in self.floating_forward_rules(fip_ip, fixed):
-                ri.iptables_manager.ipv4['nat'].add_rule(chain, rule,
-                                                         tag='floating_ip')
-
-        ri.iptables_manager.apply()
-
         # Clean up addresses that no longer belong on the gateway interface.
         for ip_cidr in existing_cidrs - new_cidrs:
             if ip_cidr.endswith(FLOATING_IP_CIDR_SUFFIX):

neutron/tests/unit/test_l3_agent.py

@@ -382,7 +382,8 @@ class TestBasicRouterOperations(base.BaseTestCase):
 
     def test_process_router(self):
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
-        agent.process_router_floating_ips = mock.Mock()
+        agent.process_router_floating_ip_addresses = mock.Mock()
+        agent.process_router_floating_ip_nat_rules = mock.Mock()
         router = self._prepare_router_data()
         fake_floatingips1 = {'floatingips': [
             {'id': _uuid(),
@@ -393,8 +394,11 @@ class TestBasicRouterOperations(base.BaseTestCase):
                                  self.conf.use_namespaces, router=router)
         agent.process_router(ri)
         ex_gw_port = agent._get_ex_gw_port(ri)
-        agent.process_router_floating_ips.assert_called_with(ri, ex_gw_port)
+        agent.process_router_floating_ip_addresses.assert_called_with(
-        agent.process_router_floating_ips.reset_mock()
+            ri, ex_gw_port)
+        agent.process_router_floating_ip_addresses.reset_mock()
+        agent.process_router_floating_ip_nat_rules.assert_called_with(ri)
+        agent.process_router_floating_ip_nat_rules.reset_mock()
 
         # remap floating IP to a new fixed ip
         fake_floatingips2 = copy.deepcopy(fake_floatingips1)
@@ -403,25 +407,32 @@ class TestBasicRouterOperations(base.BaseTestCase):
         router[l3_constants.FLOATINGIP_KEY] = fake_floatingips2['floatingips']
         agent.process_router(ri)
         ex_gw_port = agent._get_ex_gw_port(ri)
-        agent.process_router_floating_ips.assert_called_with(ri, ex_gw_port)
+        agent.process_router_floating_ip_addresses.assert_called_with(
-        agent.process_router_floating_ips.reset_mock()
+            ri, ex_gw_port)
+        agent.process_router_floating_ip_addresses.reset_mock()
+        agent.process_router_floating_ip_nat_rules.assert_called_with(ri)
+        agent.process_router_floating_ip_nat_rules.reset_mock()
 
         # remove just the floating ips
         del router[l3_constants.FLOATINGIP_KEY]
         agent.process_router(ri)
         ex_gw_port = agent._get_ex_gw_port(ri)
-        agent.process_router_floating_ips.assert_called_with(ri, ex_gw_port)
+        agent.process_router_floating_ip_addresses.assert_called_with(
-        agent.process_router_floating_ips.reset_mock()
+            ri, ex_gw_port)
+        agent.process_router_floating_ip_addresses.reset_mock()
+        agent.process_router_floating_ip_nat_rules.assert_called_with(ri)
+        agent.process_router_floating_ip_nat_rules.reset_mock()
 
         # now no ports so state is torn down
         del router[l3_constants.INTERFACE_KEY]
         del router['gw_port']
         agent.process_router(ri)
         self.send_arp.assert_called_once()
-        self.assertFalse(agent.process_router_floating_ips.called)
+        self.assertFalse(agent.process_router_floating_ip_addresses.called)
+        self.assertFalse(agent.process_router_floating_ip_nat_rules.called)
 
     @mock.patch('neutron.agent.linux.ip_lib.IPDevice')
-    def test_process_router_floating_ip_add(self, IPDevice):
+    def test_process_router_floating_ip_addresses_add(self, IPDevice):
         fip = {
             'id': _uuid(), 'port_id': _uuid(),
             'floating_ip_address': '15.1.2.3',
@@ -436,10 +447,24 @@ class TestBasicRouterOperations(base.BaseTestCase):
 
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
 
-        agent.process_router_floating_ips(ri, {'id': _uuid()})
+        agent.process_router_floating_ip_addresses(ri, {'id': _uuid()})
 
         device.addr.add.assert_called_once_with(4, '15.1.2.3/32', '15.1.2.3')
 
+    def test_process_router_floating_ip_nat_rules_add(self):
+        fip = {
+            'id': _uuid(), 'port_id': _uuid(),
+            'floating_ip_address': '15.1.2.3',
+            'fixed_ip_address': '192.168.0.1'
+        }
+
+        ri = mock.MagicMock()
+        ri.router.get.return_value = [fip]
+
+        agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
+
+        agent.process_router_floating_ip_nat_rules(ri)
+
         nat = ri.iptables_manager.ipv4['nat']
         nat.clear_rules_by_tag.assert_called_once_with('floating_ip')
         rules = agent.floating_forward_rules('15.1.2.3', '192.168.0.1')
@@ -447,7 +472,7 @@ class TestBasicRouterOperations(base.BaseTestCase):
             nat.add_rule.assert_any_call(chain, rule, tag='floating_ip')
 
     @mock.patch('neutron.agent.linux.ip_lib.IPDevice')
-    def test_process_router_floating_ip_remove(self, IPDevice):
+    def test_process_router_floating_ip_addresses_remove(self, IPDevice):
         IPDevice.return_value = device = mock.Mock()
         device.addr.list.return_value = [{'cidr': '15.1.2.3/32'}]
 
@@ -456,16 +481,24 @@ class TestBasicRouterOperations(base.BaseTestCase):
 
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
 
-        agent.process_router_floating_ips(ri, {'id': _uuid()})
+        agent.process_router_floating_ip_addresses(ri, {'id': _uuid()})
 
         device.addr.delete.assert_called_once_with(4, '15.1.2.3/32')
 
+    def test_process_router_floating_ip_nat_rules_remove(self):
+        ri = mock.MagicMock()
+        ri.router.get.return_value = []
+
+        agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
+
+        agent.process_router_floating_ip_nat_rules(ri)
+
         nat = ri.iptables_manager.ipv4['nat']
         nat = ri.iptables_manager.ipv4['nat']
         nat.clear_rules_by_tag.assert_called_once_with('floating_ip')
 
     @mock.patch('neutron.agent.linux.ip_lib.IPDevice')
-    def test_process_router_floating_ip_remap(self, IPDevice):
+    def test_process_router_floating_ip_addresses_remap(self, IPDevice):
         fip = {
             'id': _uuid(), 'port_id': _uuid(),
             'floating_ip_address': '15.1.2.3',
@@ -480,11 +513,26 @@ class TestBasicRouterOperations(base.BaseTestCase):
 
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
 
-        agent.process_router_floating_ips(ri, {'id': _uuid()})
+        agent.process_router_floating_ip_addresses(ri, {'id': _uuid()})
 
         self.assertFalse(device.addr.add.called)
         self.assertFalse(device.addr.delete.called)
 
+    def test_process_router_floating_ip_nat_rules_remap(self):
+        fip = {
+            'id': _uuid(), 'port_id': _uuid(),
+            'floating_ip_address': '15.1.2.3',
+            'fixed_ip_address': '192.168.0.2'
+        }
+
+        ri = mock.MagicMock()
+
+        ri.router.get.return_value = [fip]
+
+        agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
+
+        agent.process_router_floating_ip_nat_rules(ri)
+
         nat = ri.iptables_manager.ipv4['nat']
         nat.clear_rules_by_tag.assert_called_once_with('floating_ip')
         rules = agent.floating_forward_rules('15.1.2.3', '192.168.0.2')