From: Carl Baldwin <carl.baldwin@hp.com>
Date: Sat, 31 Jan 2015 00:27:52 +0000 (+0000)
Subject: Don't pass the port down to the floating ip processing
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=f97a9cb366f09d33400efb9a172fe462e27aadbd;p=openstack-build%2Fneutron-build.git

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 ab0b84cfb..844911d22 100644
--- 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 6f7876363..287dd33b5 100644
--- 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()