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

diff --git a/neutron/agent/l3/agent.py b/neutron/agent/l3/agent.py
index f973bc9f69ee0f4c549cfd10aadb4ea7ccc04070..69e770292ece5ea886f2b9a9ffb0a7f727bdfd24 100644 (file)
--- a/neutron/agent/l3/agent.py
+++ b/neutron/agent/l3/agent.py
@@ -25,9 +25,11 @@ from oslo.utils import timeutils
 
 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 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
                         "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)
 

diff --git a/neutron/agent/l3/dvr.py b/neutron/agent/l3/dvr.py
index a0fb9343261914752c1a86bcded908822d651f67..f55293c00f7c2f9c4f5f32f867b31cf18f20f938 100644 (file)
--- a/neutron/agent/l3/dvr.py
+++ b/neutron/agent/l3/dvr.py
@@ -38,18 +38,6 @@ FIP_PR_END = FIP_PR_START + 40000
 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

diff --git a/neutron/agent/l3/dvr_router.py b/neutron/agent/l3/dvr_router.py
new file mode 100644 (file)
index 0000000..b948fcd
--- /dev/null
+++ b/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

diff --git a/neutron/agent/l3/ha.py b/neutron/agent/l3/ha.py
index bcc9414955a5ba85afe6d764739cce7958cb6bd9..4e006fa50976c5270d3370f3eed68c7425c141ab 100644 (file)
--- a/neutron/agent/l3/ha.py
+++ b/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 @@ OPTS = [
 ]
 
 
-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()

diff --git a/neutron/agent/l3/ha_router.py b/neutron/agent/l3/ha_router.py
new file mode 100644 (file)
index 0000000..168f064
--- /dev/null
+++ b/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)

diff --git a/neutron/agent/l3/legacy_router.py b/neutron/agent/l3/legacy_router.py
new file mode 100644 (file)
index 0000000..31dd0f3
--- /dev/null
+++ b/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

diff --git a/neutron/agent/l3/router_info.py b/neutron/agent/l3/router_info.py
index 16827d53ea876666096e4ca3e2aceb1cb27d6e77..f110d73858566ac1291947f525d696cc43d02b72 100644 (file)
--- a/neutron/agent/l3/router_info.py
+++ b/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 @@ class RouterInfo(dvr.RouterMixin, ha.RouterMixin):
             namespace=self.ns_name)
         self.routes = []
 
-        super(RouterInfo, self).__init__()
-
     @property
     def router(self):
         return self._router
@@ -58,6 +54,11 @@ class RouterInfo(dvr.RouterMixin, ha.RouterMixin):
             # 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:

diff --git a/neutron/common/exceptions.py b/neutron/common/exceptions.py
index a8d50c3e3c4f1b4a1f07e5d6ba87532507776d02..3aee9680cfb693b64542033eb4f988483f5d6b7a 100644 (file)
--- a/neutron/common/exceptions.py
+++ b/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

diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py
index 0a94eeae731e01d1a9b89fd0ee7394fa9ab73936..e15ed1b6d765c8c7ee0d4a51062d4d51df26c70f 100644 (file)
--- a/neutron/tests/unit/test_l3_agent.py
+++ b/neutron/tests/unit/test_l3_agent.py
@@ -27,6 +27,7 @@ from neutron.agent.common import config as agent_config
 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 @@ class TestBasicRouterOperations(base.BaseTestCase):
         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 @@ class TestBasicRouterOperations(base.BaseTestCase):
         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 @@ class TestBasicRouterOperations(base.BaseTestCase):
                 {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 @@ class TestBasicRouterOperations(base.BaseTestCase):
                '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 @@ class TestBasicRouterOperations(base.BaseTestCase):
         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 @@ class TestBasicRouterOperations(base.BaseTestCase):
     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 @@ class TestBasicRouterOperations(base.BaseTestCase):
                          '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 @@ class TestBasicRouterOperations(base.BaseTestCase):
         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