From 522695e9cd959f79a5fab003941cf52aa618821d Mon Sep 17 00:00:00 2001 From: Carl Baldwin Date: Mon, 22 Dec 2014 21:01:45 +0000 Subject: [PATCH] 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 | 35 ++++++++++++--- neutron/agent/l3/dvr.py | 12 ----- neutron/agent/l3/dvr_router.py | 26 +++++++++++ neutron/agent/l3/ha.py | 47 ------------------- neutron/agent/l3/ha_router.py | 70 +++++++++++++++++++++++++++++ neutron/agent/l3/legacy_router.py | 19 ++++++++ neutron/agent/l3/router_info.py | 11 ++--- neutron/common/exceptions.py | 4 ++ neutron/tests/unit/test_l3_agent.py | 28 ++++++------ 9 files changed, 168 insertions(+), 84 deletions(-) create mode 100644 neutron/agent/l3/dvr_router.py create mode 100644 neutron/agent/l3/ha_router.py create mode 100644 neutron/agent/l3/legacy_router.py diff --git a/neutron/agent/l3/agent.py b/neutron/agent/l3/agent.py index f973bc9f6..69e770292 100644 --- 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 a0fb93432..f55293c00 100644 --- 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 index 000000000..b948fcddf --- /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 bcc941495..4e006fa50 100644 --- 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 index 000000000..168f06443 --- /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 index 000000000..31dd0f326 --- /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 16827d53e..f110d7385 100644 --- 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 a8d50c3e3..3aee9680c 100644 --- 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 0a94eeae7..e15ed1b6d 100644 --- 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 -- 2.45.2