From ee4bae211309e0f1fcee5565ddd2379997e1de13 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 10 Sep 2014 16:59:14 -0700 Subject: [PATCH] Initialize dist_fip_count after agent restart Runtime router variable dist_fip_count has been used to keep track of FIPs for DVR routers. This variable is not re-initialized correctly on agent restart and can get stale from other errors which cause problems with namespace and port cleanup. This patch will initialize the ri.dist_fip_count once in process_router for dvr routers only. This method was selected instead of the _router_added or _router_removed path because it is the one central entry point for rotuer add, delete, and update. The object self.agent_gateway_port also needs to be properly handled after an agent restart and this patch will handle that as well. When needed, the system will be read via system calls to determine the state of namespaces and ports since the variables cannot be relied on. System calls will be kept to a minimum to reduce and possible performance hits. Change-Id: Iae5ebf5249f8e16ab57df78e042293ca2855ddf1 Closes-bug: #1367039 --- neutron/agent/l3/agent.py | 16 +++++++---- neutron/agent/l3/dvr.py | 27 ++++++++++++++++--- neutron/agent/l3/dvr_router.py | 2 +- neutron/tests/unit/test_l3_agent.py | 42 +++++++++++++++++++++++++++-- 4 files changed, 75 insertions(+), 12 deletions(-) diff --git a/neutron/agent/l3/agent.py b/neutron/agent/l3/agent.py index d05b5e723..6496454cd 100644 --- a/neutron/agent/l3/agent.py +++ b/neutron/agent/l3/agent.py @@ -581,6 +581,7 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, # TODO(mrsmith) - we shouldn't need to check here if 'distributed' not in ri.router: ri.router['distributed'] = False + self.scan_fip_ports(ri) self._process_internal_ports(ri) self._process_external(ri) # Process static routes for router @@ -657,14 +658,19 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, self._create_agent_gateway_port(ri, floating_ips[0] ['floating_network_id']) - if self.agent_gateway_port: - if floating_ips and ri.dist_fip_count == 0: - self.create_rtr_2_fip_link(ri, floating_ips[0] - ['floating_network_id']) + if self.agent_gateway_port and floating_ips: + fip_net_id = floating_ips[0]['floating_network_id'] + self.create_rtr_2_fip_link(ri, fip_net_id) def _get_external_device_interface_name(self, ri, ex_gw_port): if ri.router['distributed']: - if self.agent_gateway_port: + fip_int = self.get_fip_int_device_name(ri.router_id) + # TODO(mrsmith) refactor for multiple ext nets + fip_ns = self.get_fip_ns_name(str(self._fetch_external_net_id())) + + if ip_lib.device_exists(fip_int, + root_helper=self.root_helper, + namespace=fip_ns): return self.get_rtr_int_device_name(ri.router_id) else: return self.get_external_device_name(ex_gw_port['id']) diff --git a/neutron/agent/l3/dvr.py b/neutron/agent/l3/dvr.py index f55293c00..ba357eef9 100644 --- a/neutron/agent/l3/dvr.py +++ b/neutron/agent/l3/dvr.py @@ -19,6 +19,7 @@ from neutron.agent.l3 import link_local_allocator as lla from neutron.agent.linux import ip_lib from neutron.agent.linux import iptables_manager from neutron.common import constants as l3_constants +from neutron.common import utils as common_utils from neutron.i18n import _LE from neutron.openstack.common import log as logging @@ -119,6 +120,24 @@ class AgentMixin(object): if f['subnet_id'] == subnet_id: return port + def scan_fip_ports(self, ri): + # don't scan if not dvr or count is not None + if not ri.router.get('distributed') or ri.dist_fip_count is not None: + return + + # scan system for any existing fip ports + ri.dist_fip_count = 0 + rtr_2_fip_interface = self.get_rtr_int_device_name(ri.router_id) + if ip_lib.device_exists(rtr_2_fip_interface, + root_helper=self.root_helper, + namespace=ri.ns_name): + device = ip_lib.IPDevice(rtr_2_fip_interface, self.root_helper, + namespace=ri.ns_name) + existing_cidrs = [addr['cidr'] for addr in device.addr.list()] + fip_cidrs = [c for c in existing_cidrs if + common_utils.is_cidr_host(c)] + ri.dist_fip_count = len(fip_cidrs) + def get_fip_ext_device_name(self, port_id): return (FIP_EXT_DEV_PREFIX + port_id)[:self.driver.DEV_NAME_LEN] @@ -337,17 +356,17 @@ class AgentMixin(object): floating_ip = fip_cidr.split('/')[0] rtr_2_fip_name = self.get_rtr_int_device_name(ri.router_id) fip_2_rtr_name = self.get_fip_int_device_name(ri.router_id) + if ri.rtr_fip_subnet is None: + ri.rtr_fip_subnet = self.local_subnets.allocate(ri.router_id) rtr_2_fip, fip_2_rtr = ri.rtr_fip_subnet.get_pair() fip_ns_name = self.get_fip_ns_name(str(self._fetch_external_net_id())) ip_rule_rtr = ip_lib.IpRule(self.root_helper, namespace=ri.ns_name) if floating_ip in ri.floating_ips_dict: rule_pr = ri.floating_ips_dict[floating_ip] + ip_rule_rtr.delete_rule_priority(rule_pr) + self.fip_priorities.add(rule_pr) #TODO(rajeev): Handle else case - exception/log? - else: - rule_pr = None - ip_rule_rtr.delete_rule_priority(rule_pr) - self.fip_priorities.add(rule_pr) device = ip_lib.IPDevice(fip_2_rtr_name, self.root_helper, namespace=fip_ns_name) diff --git a/neutron/agent/l3/dvr_router.py b/neutron/agent/l3/dvr_router.py index b948fcddf..6822a15aa 100644 --- a/neutron/agent/l3/dvr_router.py +++ b/neutron/agent/l3/dvr_router.py @@ -23,4 +23,4 @@ class DvrRouter(router.RouterInfo): self.snat_iptables_manager = None # Linklocal subnet for router and floating IP namespace link self.rtr_fip_subnet = None - self.dist_fip_count = 0 + self.dist_fip_count = None diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py index 4db7511ad..163aa69c0 100644 --- a/neutron/tests/unit/test_l3_agent.py +++ b/neutron/tests/unit/test_l3_agent.py @@ -794,6 +794,42 @@ class TestBasicRouterOperations(base.BaseTestCase): 4, '1.5.25.15', '00:44:33:22:11:55') agent.router_deleted(None, router['id']) + @mock.patch('neutron.agent.linux.ip_lib.IPDevice') + def _test_scan_fip_ports(self, ri, ip_list, IPDevice): + agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) + self.device_exists.return_value = True + IPDevice.return_value = device = mock.Mock() + device.addr.list.return_value = ip_list + agent.scan_fip_ports(ri) + + def test_scan_fip_ports_restart_fips(self): + router = prepare_router_data() + ri = dvr_router.DvrRouter(router['id'], self.conf.root_helper, + router=router) + ri.router['distributed'] = True + ip_list = [{'cidr': '111.2.3.4/32'}, {'cidr': '111.2.3.5/32'}] + self._test_scan_fip_ports(ri, ip_list) + self.assertEqual(ri.dist_fip_count, 2) + + def test_scan_fip_ports_restart_none(self): + router = prepare_router_data() + ri = dvr_router.DvrRouter(router['id'], self.conf.root_helper, + router=router) + ri.router['distributed'] = True + ip_list = [] + self._test_scan_fip_ports(ri, ip_list) + self.assertEqual(ri.dist_fip_count, 0) + + def test_scan_fip_ports_restart_zero(self): + router = prepare_router_data() + ri = dvr_router.DvrRouter(router['id'], self.conf.root_helper, + router=router) + ri.router['distributed'] = True + ri.dist_fip_count = 0 + ip_list = None + self._test_scan_fip_ports(ri, ip_list) + self.assertEqual(ri.dist_fip_count, 0) + def test_process_cent_router(self): router = prepare_router_data() ri = l3router.RouterInfo(router['id'], self.conf.root_helper, @@ -802,8 +838,8 @@ class TestBasicRouterOperations(base.BaseTestCase): def test_process_dist_router(self): 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) subnet_id = _get_subnet_id(router[l3_constants.INTERFACE_KEY][0]) ri.router['distributed'] = True ri.router['_snat_router_interfaces'] = [{ @@ -969,6 +1005,7 @@ class TestBasicRouterOperations(base.BaseTestCase): ri = dvr_router.DvrRouter(router['id'], self.conf.root_helper, router=router) ri.iptables_manager.ipv4['nat'] = mock.MagicMock() + ri.dist_fip_count = 0 agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) agent.host = HOSTNAME agent.agent_gateway_port = ( @@ -1924,6 +1961,7 @@ class TestBasicRouterOperations(base.BaseTestCase): 'port_id': _uuid()} agent.agent_gateway_port = agent_gw_port ri.rtr_fip_subnet = lla.LinkLocalAddressPair('169.254.30.42/31') + ri.dist_fip_count = 0 ip_cidr = common_utils.ip_to_cidr(fip['floating_ip_address']) agent.floating_ip_added_dist(ri, fip, ip_cidr) self.mock_rule.add_rule_from.assert_called_with('192.168.0.1', -- 2.45.2