Move _set_subnet_arp_info to dvr_router

Further teasing DVR apart from the agent and other router types.  This
change illustrates the need to use plugin_rpc and context from the
agent.  It leaves this as a dependence of dvr_router on the instance
of the agent.  So, when the agent creates a dvr_router, it passes
itself in as an argument.  It doesn't do this with other router types.

I would like to eliminate the dependence of RPC while processing a
router.  However, this is just a refactor and at least decouples it as
much as possible and isolates the dependence to DVR only.

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

diff --git a/neutron/agent/l3/agent.py b/neutron/agent/l3/agent.py
index b0651f1cc370e4cc0492fee539bdc77bf29ff75e..8ac4b02b21672ec6201e7df1d12f0aa681bad0f7 100644 (file)
--- a/neutron/agent/l3/agent.py
+++ b/neutron/agent/l3/agent.py
@@ -276,6 +276,7 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
         }
 
         if router.get('distributed'):
+            kwargs['agent'] = self
             kwargs['host'] = self.host
             return dvr_router.DvrRouter(*args, **kwargs)
 
@@ -354,7 +355,8 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
             self._set_subnet_info(p)
             self.internal_network_added(ri, p)
             ri.internal_ports.append(p)
-            self._set_subnet_arp_info(ri, p)
+            if ri.router['distributed']:
+                ri._set_subnet_arp_info(p)
             if (not new_ipv6_port and
                     netaddr.IPNetwork(p['subnet']['cidr']).version == 6):
                 new_ipv6_port = True

diff --git a/neutron/agent/l3/dvr.py b/neutron/agent/l3/dvr.py
index 56460aa4437e3b11d2e9acc8f2ab2d3ce456f820..cb5e10d558945f9ace663cdaa9c003be552648f6 100644 (file)
--- a/neutron/agent/l3/dvr.py
+++ b/neutron/agent/l3/dvr.py
@@ -61,22 +61,8 @@ class AgentMixin(object):
         fip_ns = self.get_fip_ns(ex_net_id)
         fip_ns.delete()
 
-    def _set_subnet_arp_info(self, ri, port):
-        """Set ARP info retrieved from Plugin for existing ports."""
-        if 'id' not in port['subnet'] or not ri.router['distributed']:
-            return
-        subnet_id = port['subnet']['id']
-        subnet_ports = (
-            self.plugin_rpc.get_ports_by_subnet(self.context,
-                                                subnet_id))
-
-        for p in subnet_ports:
-            if p['device_owner'] not in l3_constants.ROUTER_INTERFACE_OWNERS:
-                for fixed_ip in p['fixed_ips']:
-                    ri._update_arp_entry(fixed_ip['ip_address'],
-                                         p['mac_address'],
-                                         subnet_id,
-                                         'add')
+    def get_ports_by_subnet(self, subnet_id):
+        return self.plugin_rpc.get_ports_by_subnet(self.context, subnet_id)
 
     def get_snat_int_device_name(self, port_id):
         return (SNAT_INT_DEV_PREFIX +

diff --git a/neutron/agent/l3/dvr_router.py b/neutron/agent/l3/dvr_router.py
index 82371a8a5b8d7fa59f81fed549ca5262c43d260b..15425652cdf624b7af6e09b5e22c8fea3c8677ec 100644 (file)
--- a/neutron/agent/l3/dvr_router.py
+++ b/neutron/agent/l3/dvr_router.py
@@ -28,9 +28,10 @@ LOG = logging.getLogger(__name__)
 
 
 class DvrRouter(router.RouterInfo):
-    def __init__(self, host, *args, **kwargs):
+    def __init__(self, agent, host, *args, **kwargs):
         super(DvrRouter, self).__init__(*args, **kwargs)
 
+        self.agent = agent
         self.host = host
 
         self.floating_ips_dict = {}
@@ -194,3 +195,22 @@ class DvrRouter(router.RouterInfo):
         except Exception:
             with excutils.save_and_reraise_exception():
                 LOG.exception(_LE("DVR: Failed updating arp entry"))
+
+    def _set_subnet_arp_info(self, port):
+        """Set ARP info retrieved from Plugin for existing ports."""
+        if 'id' not in port['subnet']:
+            return
+
+        subnet_id = port['subnet']['id']
+
+        # TODO(Carl) Can we eliminate the need to make this RPC while
+        # processing a router.
+        subnet_ports = self.agent.get_ports_by_subnet(subnet_id)
+
+        for p in subnet_ports:
+            if p['device_owner'] not in l3_constants.ROUTER_INTERFACE_OWNERS:
+                for fixed_ip in p['fixed_ips']:
+                    self._update_arp_entry(fixed_ip['ip_address'],
+                                           p['mac_address'],
+                                           subnet_id,
+                                           'add')

diff --git a/neutron/tests/unit/agent/l3/test_dvr_router.py b/neutron/tests/unit/agent/l3/test_dvr_router.py
index 24009d2aa4603683b6e47a577447e32291ec2c97..adfc50a5c67370782f6ff26ffa4c334c0790d624 100644 (file)
--- a/neutron/tests/unit/agent/l3/test_dvr_router.py
+++ b/neutron/tests/unit/agent/l3/test_dvr_router.py
@@ -38,7 +38,8 @@ class TestDvrRouterOperations(base.BaseTestCase):
         self.router_id = _uuid()
         if not router:
             router = mock.MagicMock()
-        return dvr_router.DvrRouter(mock.sentinel.myhost,
+        return dvr_router.DvrRouter(mock.sentinel.agent,
+                                    mock.sentinel.myhost,
                                     self.router_id,
                                     router,
                                     agent_conf,

diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py
index 77bf190021953383703430bf7b84116675702c7c..968e70211ce470cc32af37c459b689f41fc16abb 100644 (file)
--- a/neutron/tests/unit/test_l3_agent.py
+++ b/neutron/tests/unit/test_l3_agent.py
@@ -424,7 +424,8 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
             agent.host = HOSTNAME
             agent._create_dvr_gateway = mock.Mock()
             agent.get_snat_interfaces = mock.Mock(return_value=self.snat_ports)
-            ri = dvr_router.DvrRouter(HOSTNAME,
+            ri = dvr_router.DvrRouter(agent,
+                                      HOSTNAME,
                                       router['id'],
                                       router,
                                       **self.ri_kwargs)
@@ -526,7 +527,8 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
                                             agent_mode, expected_call_count):
         router = prepare_router_data(num_internal_ports=2)
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
-        ri = dvr_router.DvrRouter(HOSTNAME,
+        ri = dvr_router.DvrRouter(agent,
+                                  HOSTNAME,
                                   router['id'],
                                   router,
                                   **self.ri_kwargs)
@@ -648,7 +650,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
         router = prepare_router_data(num_internal_ports=2)
         router['distributed'] = True
         ri = dvr_router.DvrRouter(
-            HOSTNAME, router['id'], router, **self.ri_kwargs)
+            agent, HOSTNAME, router['id'], router, **self.ri_kwargs)
         ports = ri.router.get(l3_constants.INTERFACE_KEY, [])
         test_ports = [{'mac_address': '00:11:22:33:44:55',
                       'device_owner': 'network:dhcp',
@@ -659,13 +661,13 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
 
         # Test basic case
         ports[0]['subnet']['id'] = _get_subnet_id(ports[0])
-        agent._set_subnet_arp_info(ri, ports[0])
+        ri._set_subnet_arp_info(ports[0])
         self.mock_ip_dev.neigh.add.assert_called_once_with(
             4, '1.2.3.4', '00:11:22:33:44:55')
 
         # Test negative case
         router['distributed'] = False
-        agent._set_subnet_arp_info(ri, ports[0])
+        ri._set_subnet_arp_info(ports[0])
         self.mock_ip_dev.neigh.add.never_called()
 
     def test_add_arp_entry(self):
@@ -697,6 +699,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
 
     def test__update_arp_entry_with_no_subnet(self):
         ri = dvr_router.DvrRouter(
+            mock.sentinel.agent,
             HOSTNAME,
             'foo_router_id',
             {'distributed': True, 'gw_port_host': HOSTNAME},
@@ -733,7 +736,8 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
     def test_process_dist_router(self):
         router = prepare_router_data()
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
-        ri = dvr_router.DvrRouter(HOSTNAME,
+        ri = dvr_router.DvrRouter(agent,
+                                  HOSTNAME,
                                   router['id'],
                                   router,
                                   **self.ri_kwargs)
@@ -855,9 +859,9 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
         router = prepare_router_data(enable_snat=True)
         router[l3_constants.FLOATINGIP_AGENT_INTF_KEY] = agent_gateway_port
         router['distributed'] = True
-        ri = dvr_router.DvrRouter(
-            HOSTNAME, router['id'], router, **self.ri_kwargs)
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
+        ri = dvr_router.DvrRouter(
+            agent, HOSTNAME, router['id'], router, **self.ri_kwargs)
         self.assertEqual(
             agent_gateway_port[0],
             agent.get_floating_agent_gw_interface(ri, fake_network_id))
@@ -886,10 +890,10 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
         router[l3_constants.FLOATINGIP_KEY] = fake_floatingips['floatingips']
         router[l3_constants.FLOATINGIP_AGENT_INTF_KEY] = agent_gateway_port
         router['distributed'] = True
+        agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
         ri = dvr_router.DvrRouter(
-            HOSTNAME, router['id'], router, **self.ri_kwargs)
+            agent, HOSTNAME, router['id'], router, **self.ri_kwargs)
 
-        agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
         ext_gw_port = ri.router.get('gw_port')
         ri.fip_ns = agent.get_fip_ns(ext_gw_port['network_id'])
         ri.dist_fip_count = 0
@@ -947,13 +951,14 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
         router = prepare_router_data(enable_snat=True)
         router[l3_constants.FLOATINGIP_KEY] = fake_floatingips['floatingips']
         router['distributed'] = True
-        ri = dvr_router.DvrRouter(HOSTNAME,
+        agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
+        ri = dvr_router.DvrRouter(agent,
+                                  HOSTNAME,
                                   router['id'],
                                   router,
                                   **self.ri_kwargs)
         ri.iptables_manager.ipv4['nat'] = mock.MagicMock()
         ri.dist_fip_count = 0
-        agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
         fip_ns = agent.get_fip_ns(mock.sentinel.ext_net_id)
         fip_ns.agent_gateway_port = (
             {'fixed_ips': [{'ip_address': '20.0.0.30',
@@ -1270,14 +1275,15 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
                 {fip_id: l3_constants.FLOATINGIP_STATUS_ERROR})
 
     def test_handle_router_snat_rules_distributed_without_snat_manager(self):
+        agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
         ri = dvr_router.DvrRouter(
+            agent,
             HOSTNAME,
             'foo_router_id',
             {'distributed': True},
             **self.ri_kwargs)
         ri.iptables_manager = mock.Mock()
 
-        agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
         with mock.patch.object(l3_agent.LOG, 'debug') as log_debug:
             agent._handle_router_snat_rules(
                 ri, mock.ANY, mock.ANY, mock.ANY)
@@ -1709,7 +1715,8 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
     def test_create_dvr_gateway(self):
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
         router = prepare_router_data()
-        ri = dvr_router.DvrRouter(HOSTNAME,
+        ri = dvr_router.DvrRouter(agent,
+                                  HOSTNAME,
                                   router['id'],
                                   router,
                                   **self.ri_kwargs)
@@ -1781,7 +1788,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
 
         external_net_id = router['gw_port']['network_id']
         ri = dvr_router.DvrRouter(
-            HOSTNAME, router['id'], router, **self.ri_kwargs)
+            agent, HOSTNAME, router['id'], router, **self.ri_kwargs)
         ri.remove_floating_ip = mock.Mock()
         agent._fetch_external_net_id = mock.Mock(return_value=external_net_id)
         ri.ex_gw_port = ri.router['gw_port']