Configure floating IPs addresses after NAT rules

Change the behaviour of the L3 agent in order to set the IP addresses
for the floating IPs on the external gateway interface after the
relevant NAT rules have been applied.
This will avoid a transitory period in which the floating IP exists
and is reachable but it not yet wired to the actual target.

Partial-Bug: #1265505

Change-Id: Ib382fde021868bab2185f2fa5bdee86559148ba7

diff --git a/neutron/agent/l3_agent.py b/neutron/agent/l3_agent.py
index f78d55ce81f30dcb8bce23ba86d7490a9f66fbcb..69414d771d42902cf505542e52a3297201e9512b 100644 (file)
--- a/neutron/agent/l3_agent.py
+++ b/neutron/agent/l3_agent.py
@@ -447,13 +447,17 @@ 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
         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):
@@ -477,19 +481,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):
-        """Configure the router's floating IPs
-        Configures floating ips in iptables and on the router's gateway device.
+    def process_router_floating_ip_nat_rules(self, ri):
+        """Configure NAT rules for the router's floating IPs.
 
-        Cleans up floating ips that should not longer be configured.
+        Configures iptables rules for the floating ips of the given router
         """
-        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')
 
+        # 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())
         existing_cidrs = set([addr['cidr'] for addr in device.addr.list()])
         new_cidrs = set()
 
@@ -505,14 +524,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):

diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py
index 07db8d2c8feebd7b7f3fcabc114a77aec821fddf..f2008f637e0f4380fbab2a849d1893fefe2da70b 100644 (file)
--- a/neutron/tests/unit/test_l3_agent.py
+++ b/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_ips.reset_mock()
+        agent.process_router_floating_ip_addresses.assert_called_with(
+            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_ips.reset_mock()
+        agent.process_router_floating_ip_addresses.assert_called_with(
+            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_ips.reset_mock()
+        agent.process_router_floating_ip_addresses.assert_called_with(
+            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')