Refactor the _get_external_device_interface_name method

Currently this method does two things. It returns the
external interface name as its name indicates and
creates some interfaces in the case of the dvr mode.
This patch split these two things into two methods and
refactor a bit the code according to this change.

Related-Bug: #1375994
Change-Id: I7dc2c03457cbb209c84584c2badffd12048e9617

diff --git a/neutron/agent/l3/agent.py b/neutron/agent/l3/agent.py
index 566cc1c61bf4ace20673c228ab36638b9d77a1ac..f973bc9f69ee0f4c549cfd10aadb4ea7ccc04070 100644 (file)
--- a/neutron/agent/l3/agent.py
+++ b/neutron/agent/l3/agent.py
@@ -545,6 +545,8 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
 
                 # Process SNAT/DNAT rules and addresses for floating IPs
                 existing_floating_ips = ri.floating_ips
+                if ri.router['distributed']:
+                    self.create_dvr_fip_interfaces(ri, ex_gw_port)
                 self._process_snat_dnat_for_fip(ri)
 
             # Once NAT rules for floating IPs are safely in place
@@ -630,27 +632,25 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
 
         ri.iptables_manager.apply()
 
-    def _get_external_device_interface_name(self, ri, ex_gw_port,
-                                            floating_ips):
+    def create_dvr_fip_interfaces(self, ri, ex_gw_port):
+        floating_ips = self.get_floating_ips(ri)
+        if floating_ips:
+            is_first = self._fip_ns_subscribe(ri.router_id)
+            if is_first:
+                self._create_agent_gateway_port(ri, floating_ips[0]
+                                                ['floating_network_id'])
+
+        if self.agent_gateway_port:
+            if floating_ips and ri.dist_fip_count == 0:
+                self.create_rtr_2_fip_link(ri, floating_ips[0]
+                                           ['floating_network_id'])
+
+    def _get_external_device_interface_name(self, ri, ex_gw_port):
         if ri.router['distributed']:
-            # filter out only FIPs for this host/agent
-            floating_ips = [i for i in floating_ips if i['host'] == self.host]
-            if floating_ips:
-                is_first = self._fip_ns_subscribe(ri.router_id)
-                if is_first:
-                    self._create_agent_gateway_port(ri, floating_ips[0]
-                                                    ['floating_network_id'])
-
             if self.agent_gateway_port:
-                if floating_ips and ri.dist_fip_count == 0:
-                    self.create_rtr_2_fip_link(ri, floating_ips[0]
-                                               ['floating_network_id'])
                 return self.get_rtr_int_device_name(ri.router_id)
-            else:
-                # there are no fips or agent port, no work to do
-                return None
-
-        return self.get_external_device_name(ex_gw_port['id'])
+        else:
+            return self.get_external_device_name(ex_gw_port['id'])
 
     def _add_floating_ip(self, ri, fip, interface_name, device):
         fip_ip = fip['floating_ip_address']
@@ -702,9 +702,8 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
         """
 
         fip_statuses = {}
-        floating_ips = self.get_floating_ips(ri)
         interface_name = self._get_external_device_interface_name(
-            ri, ex_gw_port, floating_ips)
+            ri, ex_gw_port)
         if interface_name is None:
             return fip_statuses
 
@@ -713,6 +712,7 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
         existing_cidrs = set([addr['cidr'] for addr in device.addr.list()])
         new_cidrs = set()
 
+        floating_ips = self.get_floating_ips(ri)
         # Loop once to ensure that floating ips are configured.
         for fip in floating_ips:
             fip_ip = fip['floating_ip_address']

diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py
index 865072ca9d780776bad9a14b9e4f817f35247fb0..0a94eeae731e01d1a9b89fd0ee7394fa9ab73936 100644 (file)
--- a/neutron/tests/unit/test_l3_agent.py
+++ b/neutron/tests/unit/test_l3_agent.py
@@ -813,7 +813,9 @@ class TestBasicRouterOperations(base.BaseTestCase):
     def _test_process_router(self, ri):
         router = ri.router
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
+        agent.host = HOSTNAME
         fake_fip_id = 'fake_fip_id'
+        agent.create_dvr_fip_interfaces = mock.Mock()
         agent.process_router_floating_ip_addresses = mock.Mock()
         agent.process_router_floating_ip_nat_rules = mock.Mock()
         agent.process_router_floating_ip_addresses.return_value = {
@@ -824,7 +826,8 @@ class TestBasicRouterOperations(base.BaseTestCase):
             {'id': fake_fip_id,
              'floating_ip_address': '8.8.8.8',
              'fixed_ip_address': '7.7.7.7',
-             'port_id': _uuid()}]}
+             'port_id': _uuid(),
+             'host': HOSTNAME}]}
         agent.process_router(ri)
         ex_gw_port = agent._get_ex_gw_port(ri)
         agent.process_router_floating_ip_addresses.assert_called_with(
@@ -889,15 +892,17 @@ class TestBasicRouterOperations(base.BaseTestCase):
     @mock.patch('neutron.agent.linux.ip_lib.IPDevice')
     def _test_process_router_floating_ip_addresses_add(self, ri,
                                                        agent, IPDevice):
-        floating_ips = ri.router.get(l3_constants.FLOATINGIP_KEY, [])
+        floating_ips = agent.get_floating_ips(ri)
         fip_id = floating_ips[0]['id']
         IPDevice.return_value = device = mock.Mock()
         device.addr.list.return_value = []
         ri.iptables_manager.ipv4['nat'] = mock.MagicMock()
+        ex_gw_port = {'id': _uuid()}
 
         with mock.patch.object(lla.LinkLocalAllocator, '_write'):
+            agent.create_dvr_fip_interfaces(ri, ex_gw_port)
             fip_statuses = agent.process_router_floating_ip_addresses(
-                ri, {'id': _uuid()})
+                ri, ex_gw_port)
         self.assertEqual({fip_id: l3_constants.FLOATINGIP_STATUS_ACTIVE},
                          fip_statuses)
         device.addr.add.assert_called_once_with(4, '15.1.2.3/32', '15.1.2.3')
@@ -910,10 +915,10 @@ class TestBasicRouterOperations(base.BaseTestCase):
         }
 
         ri = mock.MagicMock()
-        ri.router.get.return_value = [fip]
         ri.router['distributed'].__nonzero__ = lambda self: False
 
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
+        agent.get_floating_ips = mock.Mock(return_value=[fip])
 
         agent.process_router_floating_ip_nat_rules(ri)
 
@@ -928,7 +933,9 @@ class TestBasicRouterOperations(base.BaseTestCase):
             {'id': _uuid(),
              'floating_ip_address': '15.1.2.3',
              'fixed_ip_address': '192.168.0.1',
-             'port_id': _uuid()}]}
+             'floating_network_id': _uuid(),
+             'port_id': _uuid(),
+             'host': HOSTNAME}]}
 
         router = prepare_router_data(enable_snat=True)
         router[l3_constants.FLOATINGIP_KEY] = fake_floatingips['floatingips']
@@ -2175,4 +2182,5 @@ class TestBasicRouterOperations(base.BaseTestCase):
                           mock.sentinel.ex_gw_port)
 
         agent.process_router_floating_ip_addresses.assert_called_with(
-            mock.sentinel.ri, mock.sentinel.ex_gw_port)
+            mock.sentinel.ri,
+            mock.sentinel.ex_gw_port)