Don't pass the port down to the floating ip processing

This is justified by the Law of Demeter.  I ran in to this while
trying to refactor the floating ip processing here [1].  The scope of
that patch can be reduced significantly with this refactor.  Also, it
helps us to decouple the floating ip NAT processing from the details
of the port which is a long-term goal.

[1] https://review.openstack.org/#/c/142863/

Change-Id: I606dfd43977934f1accea60911e10e71c1cbba73
Partially-Implements: bp/restructure-l3-agent

diff --git a/neutron/agent/l3/agent.py b/neutron/agent/l3/agent.py
index ab0b84cfbea4cdf49bc73cd24878c1cc0c71b24c..844911d22782fe4f9af8e31524f27b0b2a1b4d5d 100644 (file)
--- a/neutron/agent/l3/agent.py
+++ b/neutron/agent/l3/agent.py
@@ -533,10 +533,10 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
             raise n_exc.FloatingIpSetupException('L3 agent failure to setup '
                 'NAT for floating IPs')
 
-    def _configure_fip_addresses(self, ri, ex_gw_port):
+    def _configure_fip_addresses(self, ri, interface_name):
         try:
             return self.process_router_floating_ip_addresses(
-                ri, ex_gw_port)
+                ri, interface_name)
         except Exception:
             # TODO(salv-orlando): Less broad catching
             raise n_exc.FloatingIpSetupException('L3 agent failure to setup '
@@ -576,7 +576,9 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
 
             # Once NAT rules for floating IPs are safely in place
             # configure their addresses on the external gateway port
-            fip_statuses = self._configure_fip_addresses(ri, ex_gw_port)
+            interface_name = self._get_external_device_interface_name(
+                ri, ex_gw_port)
+            fip_statuses = self._configure_fip_addresses(ri, interface_name)
 
         except (n_exc.FloatingIpSetupException, n_exc.IpTablesApplyException):
                 # All floating IPs must be put in error state
@@ -741,7 +743,7 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
         else:
             return set([addr['cidr'] for addr in device.addr.list()])
 
-    def process_router_floating_ip_addresses(self, ri, ex_gw_port):
+    def process_router_floating_ip_addresses(self, ri, interface_name):
         """Configure IP addresses on router's external gateway interface.
 
         Ensures addresses for existing floating IPs and cleans up
@@ -749,8 +751,6 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
         """
 
         fip_statuses = {}
-        interface_name = self._get_external_device_interface_name(
-            ri, ex_gw_port)
         if interface_name is None:
             return fip_statuses
 
@@ -888,7 +888,9 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
     def external_gateway_removed(self, ri, ex_gw_port, interface_name):
         if ri.router['distributed']:
             self.process_router_floating_ip_nat_rules(ri)
-            self.process_router_floating_ip_addresses(ri, ex_gw_port)
+            interface_name = self._get_external_device_interface_name(
+                ri, ex_gw_port)
+            self.process_router_floating_ip_addresses(ri, interface_name)
             for p in ri.internal_ports:
                 internal_interface = self.get_internal_device_name(p['id'])
                 self._snat_redirect_remove(ri, p, internal_interface)

diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py
index 6f78763638d764ab1371a6ed6a755eef597222d9..287dd33b5b29385b2f70859d26e09f2b55bd674e 100644 (file)
--- a/neutron/tests/unit/test_l3_agent.py
+++ b/neutron/tests/unit/test_l3_agent.py
@@ -863,9 +863,8 @@ class TestBasicRouterOperations(base.BaseTestCase):
              '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(
-            ri, ex_gw_port)
+            ri, mock.ANY)
         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()
@@ -877,9 +876,8 @@ 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_ip_addresses.assert_called_with(
-            ri, ex_gw_port)
+            ri, mock.ANY)
         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()
@@ -896,7 +894,6 @@ class TestBasicRouterOperations(base.BaseTestCase):
         ri.router['gw_port']['fixed_ips'][0]['ip_address'] = str(old_ip + 1)
 
         agent.process_router(ri)
-        ex_gw_port = agent._get_ex_gw_port(ri)
         agent.process_router_floating_ip_addresses.reset_mock()
         agent.process_router_floating_ip_nat_rules.reset_mock()
         self.assertEqual(agent.external_gateway_added.call_count, 0)
@@ -905,9 +902,8 @@ class TestBasicRouterOperations(base.BaseTestCase):
         # 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_ip_addresses.assert_called_with(
-            ri, ex_gw_port)
+            ri, mock.ANY)
         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()