Create DvrRouter and HaRouter as a sub-class of Router

This commit creates them as simple sub-classes and instantiates them
when appropriate.  It also moves the basic mixin classes from
RouterInfo.  Since all of the properties and attributes from these
mixins are only used in the ha or dvr contexts, this is safe.  Future
refactoring will further tease things out until they are properly
encapsulated.

They inherit everything else from their base class so that they all
still share the same code.  Creating them up front provides a place
for dvr and ha specific logic to land as methods are moved from the L3
agent and mixins to the new router classes.  Eventually, all of the
specific logic will be teased in to the specific classes.

Change-Id: I8c485e74ae06b10fd284797359bc6d1115361713
Partially-Implements: blueprint restructure-l3-agent

neutron/agent/l3/agent.py

@@ -25,9 +25,11 @@
 
 from neutron.agent.common import config
 from neutron.agent.l3 import dvr
+from neutron.agent.l3 import dvr_router
 from neutron.agent.l3 import event_observers
 from neutron.agent.l3 import ha
-from neutron.agent.l3 import router_info
+from neutron.agent.l3 import ha_router
+from neutron.agent.l3 import legacy_router
 from neutron.agent.l3 import router_processing_queue as queue
 from neutron.agent.linux import ip_lib
 from neutron.agent.linux import ra
@@ -341,14 +343,33 @@ def _fetch_external_net_id(self, force=False):
                         "one external network.")
                     raise Exception(msg)
 
-    def _router_added(self, router_id, router):
+    def _create_router(self, router_id, router):
+        # TODO(Carl) We need to support a router that is both HA and DVR.  The
+        # patch that enables it will replace these lines.  See bug #1365473.
+        if router.get('distributed') and router.get('ha'):
+            raise n_exc.DvrHaRouterNotSupported(router_id=router_id)
+
         ns_name = (self.get_ns_name(router_id)
                    if self.conf.use_namespaces else None)
-        ri = router_info.RouterInfo(router_id=router_id,
-                                    root_helper=self.root_helper,
-                                    router=router,
-                                    use_ipv6=self.use_ipv6,
-                                    ns_name=ns_name)
+        args = []
+        kwargs = {
+            'router_id': router_id,
+            'root_helper': self.root_helper,
+            'router': router,
+            'use_ipv6': self.use_ipv6,
+            'ns_name': ns_name,
+        }
+
+        if router.get('distributed'):
+            return dvr_router.DvrRouter(*args, **kwargs)
+
+        if router.get('ha'):
+            return ha_router.HaRouter(*args, **kwargs)
+
+        return legacy_router.LegacyRouter(*args, **kwargs)
+
+    def _router_added(self, router_id, router):
+        ri = self._create_router(router_id, router)
         self.event_observers.notify(
             adv_svc.AdvancedService.before_router_added, ri)
 

neutron/agent/l3/dvr.py

@@ -38,18 +38,6 @@
 FIP_RT_TBL = 16
 
 
-class RouterMixin(object):
-    def __init__(self):
-        self.snat_ports = []
-        self.floating_ips_dict = {}
-
-        self.snat_iptables_manager = None
-        # DVR Data
-        # Linklocal subnet for router and floating IP namespace link
-        self.rtr_fip_subnet = None
-        self.dist_fip_count = 0
-
-
 class AgentMixin(object):
     def __init__(self, host):
         # dvr data

neutron/agent/l3/dvr_router.py

@@ -0,0 +1,26 @@
+# Copyright (c) 2015 Openstack Foundation
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from neutron.agent.l3 import router_info as router
+
+
+class DvrRouter(router.RouterInfo):
+    def __init__(self, *args, **kwargs):
+        super(DvrRouter, self).__init__(*args, **kwargs)
+
+        self.floating_ips_dict = {}
+        self.snat_iptables_manager = None
+        # Linklocal subnet for router and floating IP namespace link
+        self.rtr_fip_subnet = None
+        self.dist_fip_count = 0

neutron/agent/l3/ha.py

@@ -14,7 +14,6 @@
 #    under the License.
 
 import os
-import shutil
 import signal
 
 from oslo.config import cfg
@@ -47,52 +46,6 @@
 ]
 
 
-class RouterMixin(object):
-    def __init__(self):
-        self.ha_port = None
-        self.keepalived_manager = None
-
-    def _verify_ha(self):
-        if not self.is_ha:
-            raise ValueError(_('Router %s is not a HA router') %
-                             self.router_id)
-
-    @property
-    def is_ha(self):
-        return self.router is not None and self.router.get('ha')
-
-    @property
-    def ha_priority(self):
-        self._verify_ha()
-        return self.router is not None and self.router.get(
-            'priority', keepalived.HA_DEFAULT_PRIORITY)
-
-    @property
-    def ha_vr_id(self):
-        self._verify_ha()
-        return self.router is not None and self.router.get('ha_vr_id')
-
-    @property
-    def ha_state(self):
-        self._verify_ha()
-        ha_state_path = self.keepalived_manager._get_full_config_file_path(
-            'state')
-        try:
-            with open(ha_state_path, 'r') as f:
-                return f.read()
-        except (OSError, IOError):
-            LOG.debug('Error while reading HA state for %s', self.router_id)
-            return None
-
-    def spawn_keepalived(self):
-        self.keepalived_manager.spawn_or_restart()
-
-    def disable_keepalived(self):
-        self.keepalived_manager.disable()
-        conf_dir = self.keepalived_manager.get_conf_dir()
-        shutil.rmtree(conf_dir)
-
-
 class AgentMixin(object):
     def __init__(self, host):
         self._init_ha_conf_path()

neutron/agent/l3/ha_router.py

@@ -0,0 +1,70 @@
+# Copyright (c) 2015 Openstack Foundation
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import shutil
+
+from neutron.agent.l3 import router_info as router
+from neutron.agent.linux import keepalived
+from neutron.openstack.common import log as logging
+
+LOG = logging.getLogger(__name__)
+
+
+class HaRouter(router.RouterInfo):
+    def __init__(self, *args, **kwargs):
+        super(HaRouter, self).__init__(*args, **kwargs)
+
+        self.ha_port = None
+        self.keepalived_manager = None
+
+    def _verify_ha(self):
+        # TODO(Carl) Remove when is_ha below is removed.
+        if not self.is_ha:
+            raise ValueError(_('Router %s is not a HA router') %
+                             self.router_id)
+
+    @property
+    def is_ha(self):
+        # TODO(Carl) Remove when refactoring to use sub-classes is complete.
+        return self.router is not None
+
+    @property
+    def ha_priority(self):
+        self._verify_ha()
+        return self.router.get('priority', keepalived.HA_DEFAULT_PRIORITY)
+
+    @property
+    def ha_vr_id(self):
+        self._verify_ha()
+        return self.router.get('ha_vr_id')
+
+    @property
+    def ha_state(self):
+        self._verify_ha()
+        ha_state_path = self.keepalived_manager._get_full_config_file_path(
+            'state')
+        try:
+            with open(ha_state_path, 'r') as f:
+                return f.read()
+        except (OSError, IOError):
+            LOG.debug('Error while reading HA state for %s', self.router_id)
+            return None
+
+    def spawn_keepalived(self):
+        self.keepalived_manager.spawn_or_restart()
+
+    def disable_keepalived(self):
+        self.keepalived_manager.disable()
+        conf_dir = self.keepalived_manager.get_conf_dir()
+        shutil.rmtree(conf_dir)

neutron/agent/l3/legacy_router.py

@@ -0,0 +1,19 @@
+# Copyright (c) 2015 Openstack Foundation
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from neutron.agent.l3 import router_info as router
+
+
+class LegacyRouter(router.RouterInfo):
+    pass

neutron/agent/l3/router_info.py

@@ -12,12 +12,10 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-from neutron.agent.l3 import dvr
-from neutron.agent.l3 import ha
 from neutron.agent.linux import iptables_manager
 
 
-class RouterInfo(dvr.RouterMixin, ha.RouterMixin):
+class RouterInfo(object):
 
     def __init__(self, router_id, root_helper, router,
                  use_ipv6=False, ns_name=None):
@@ -37,8 +35,6 @@ def __init__(self, router_id, root_helper, router,
             namespace=self.ns_name)
         self.routes = []
 
-        super(RouterInfo, self).__init__()
-
     @property
     def router(self):
         return self._router
@@ -58,6 +54,11 @@ def router(self, value):
             # Gateway port was removed, remove rules
             self._snat_action = 'remove_rules'
 
+    @property
+    def is_ha(self):
+        # TODO(Carl) Refactoring should render this obsolete.  Remove it.
+        return False
+
     def perform_snat_action(self, snat_callback, *args):
         # Process SNAT rules for attached subnets
         if self._snat_action:

neutron/common/exceptions.py

@@ -341,6 +341,10 @@ class RouterNotCompatibleWithAgent(NeutronException):
     message = _("Router '%(router_id)s' is not compatible with this agent")
 
 
+class DvrHaRouterNotSupported(NeutronException):
+    message = _("Router '%(router_id)s' cannot be both DVR and HA")
+
+
 class FailToDropPrivilegesExit(SystemExit):
     """Exit exception raised when a drop privileges action fails."""
     code = 99

neutron/tests/unit/test_l3_agent.py

@@ -27,6 +27,7 @@
 from neutron.agent.l3 import agent as l3_agent
 from neutron.agent.l3 import config as l3_config
 from neutron.agent.l3 import dvr
+from neutron.agent.l3 import dvr_router
 from neutron.agent.l3 import ha
 from neutron.agent.l3 import link_local_allocator as lla
 from neutron.agent.l3 import router_info as l3router
@@ -900,7 +901,8 @@ def _test_process_router_floating_ip_addresses_add(self, ri,
         ex_gw_port = {'id': _uuid()}
 
         with mock.patch.object(lla.LinkLocalAllocator, '_write'):
-            agent.create_dvr_fip_interfaces(ri, ex_gw_port)
+            if ri.router['distributed']:
+                agent.create_dvr_fip_interfaces(ri, ex_gw_port)
             fip_statuses = agent.process_router_floating_ip_addresses(
                 ri, ex_gw_port)
         self.assertEqual({fip_id: l3_constants.FLOATINGIP_STATUS_ACTIVE},
@@ -963,8 +965,8 @@ def test_process_router_dist_floating_ip_add(self):
         router = prepare_router_data(enable_snat=True)
         router[l3_constants.FLOATINGIP_KEY] = fake_floatingips['floatingips']
         router['distributed'] = True
-        ri = l3router.RouterInfo(router['id'], self.conf.root_helper,
-                                 router=router)
+        ri = dvr_router.DvrRouter(router['id'], self.conf.root_helper,
+                                  router=router)
         ri.iptables_manager.ipv4['nat'] = mock.MagicMock()
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
         agent.host = HOSTNAME
@@ -1392,7 +1394,7 @@ def test_process_router_floatingip_exception(self):
                 {fip_id: l3_constants.FLOATINGIP_STATUS_ERROR})
 
     def test_handle_router_snat_rules_distributed_without_snat_manager(self):
-        ri = l3router.RouterInfo(
+        ri = dvr_router.DvrRouter(
             'foo_router_id', mock.ANY, {'distributed': True})
         ri.iptables_manager = mock.Mock()
 
@@ -1871,8 +1873,8 @@ def test_create_rtr_2_fip_link(self):
                'floating_network_id': _uuid(),
                'port_id': _uuid()}
 
-        ri = l3router.RouterInfo(router['id'], self.conf.root_helper,
-                                 router=router)
+        ri = dvr_router.DvrRouter(router['id'], self.conf.root_helper,
+                                  router=router)
 
         rtr_2_fip_name = agent.get_rtr_int_device_name(ri.router_id)
         fip_2_rtr_name = agent.get_fip_int_device_name(ri.router_id)
@@ -1893,8 +1895,8 @@ def test_create_rtr_2_fip_link_already_exists(self):
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
         router = prepare_router_data()
 
-        ri = l3router.RouterInfo(router['id'], self.conf.root_helper,
-                                 router=router)
+        ri = dvr_router.DvrRouter(router['id'], self.conf.root_helper,
+                                  router=router)
         self.device_exists.return_value = True
         with mock.patch.object(lla.LinkLocalAllocator, '_write'):
             agent.create_rtr_2_fip_link(ri, {})
@@ -1903,8 +1905,8 @@ def test_create_rtr_2_fip_link_already_exists(self):
     def test_floating_ip_added_dist(self):
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
         router = prepare_router_data()
-        ri = l3router.RouterInfo(router['id'], self.conf.root_helper,
-                                 router=router)
+        ri = dvr_router.DvrRouter(router['id'], self.conf.root_helper,
+                                  router=router)
         agent_gw_port = {'fixed_ips': [{'ip_address': '20.0.0.30',
                                         'subnet_id': _uuid()}],
                          'subnet': {'gateway_ip': '20.0.0.1'},
@@ -1942,8 +1944,8 @@ def test_floating_ip_removed_dist(self, write, unsubscribe):
                          'ip_cidr': '20.0.0.30/24'}
         fip_cidr = '11.22.33.44/24'
 
-        ri = l3router.RouterInfo(router['id'], self.conf.root_helper,
-                                 router=router)
+        ri = dvr_router.DvrRouter(router['id'], self.conf.root_helper,
+                                  router=router)
         ri.dist_fip_count = 2
         agent.fip_ns_subscribers.add(ri.router_id)
         ri.floating_ips_dict['11.22.33.44'] = FIP_PRI
@@ -2060,7 +2062,7 @@ def test_external_gateway_removed_ext_gw_port_and_fip(self):
         router['distributed'] = True
         router['gw_port_host'] = HOSTNAME
 
-        ri = l3router.RouterInfo(router['id'], self.conf.root_helper, router)
+        ri = dvr_router.DvrRouter(router['id'], self.conf.root_helper, router)
         vm_floating_ip = '19.4.4.2'
         ri.floating_ips_dict[vm_floating_ip] = FIP_PRI
         ri.dist_fip_count = 1