From d03e80a390bd712e6d8df7cddc36f2f481d35255 Mon Sep 17 00:00:00 2001 From: Henry Gessau Date: Sat, 7 Feb 2015 21:19:06 -0500 Subject: [PATCH] Refactor radvd control in the l3-agent Several of parameters used by radvd are known when a router is created and do not need to be passed around every time an RA method is called. Also, we want to easily check the state of radvd for a router. Use an object to keep track of the data and state of an radvd process. Related-blueprint: restructure-l3-agent Change-Id: I6bcad9d84af5a5b148df7520f582392a8b56a2ec --- neutron/agent/l3/agent.py | 13 +-- neutron/agent/l3/router_info.py | 2 + neutron/agent/linux/ra.py | 133 +++++++++++++++------------- neutron/tests/unit/test_l3_agent.py | 56 +++++++----- 4 files changed, 114 insertions(+), 90 deletions(-) diff --git a/neutron/agent/l3/agent.py b/neutron/agent/l3/agent.py index 2374988b7..433a93c42 100644 --- a/neutron/agent/l3/agent.py +++ b/neutron/agent/l3/agent.py @@ -288,7 +288,8 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, def _destroy_router_namespace(self, ns): router_id = self.get_router_id(ns) - ra.disable_ipv6_ra(router_id, self.process_monitor) + if router_id in self.router_info: + self.router_info[router_id].radvd.disable() ns_ip = ip_lib.IPWrapper(self.root_helper, namespace=ns) for d in ns_ip.get_devices(exclude_loopback=True): if d.name.startswith(INTERNAL_DEV_PREFIX): @@ -374,6 +375,10 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, def _router_added(self, router_id, router): ri = self._create_router(router_id, router) + ri.radvd = ra.DaemonMonitor(router['id'], + ri.ns_name, + self.process_monitor, + self.get_internal_device_name) self.event_observers.notify( adv_svc.AdvancedService.before_router_added, ri) @@ -455,11 +460,7 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, # Enable RA if new_ipv6_port or old_ipv6_port: - ra.enable_ipv6_ra(ri.router_id, - ri.ns_name, - internal_ports, - self.get_internal_device_name, - self.process_monitor) + ri.radvd.enable(internal_ports) existing_devices = self._get_existing_devices(ri) current_internal_devs = set([n for n in existing_devices diff --git a/neutron/agent/l3/router_info.py b/neutron/agent/l3/router_info.py index 581163c32..e6cafe44b 100644 --- a/neutron/agent/l3/router_info.py +++ b/neutron/agent/l3/router_info.py @@ -47,6 +47,8 @@ class RouterInfo(object): self.routes = [] self.agent_conf = agent_conf self.driver = interface_driver + # radvd is a neutron.agent.linux.ra.DaemonMonitor + self.radvd = None @property def router(self): diff --git a/neutron/agent/linux/ra.py b/neutron/agent/linux/ra.py index ef3346958..1c757682f 100644 --- a/neutron/agent/linux/ra.py +++ b/neutron/agent/linux/ra.py @@ -61,64 +61,75 @@ CONFIG_TEMPLATE = jinja2.Template("""interface {{ interface_name }} """) -def _generate_radvd_conf(router_id, router_ports, dev_name_helper): - radvd_conf = utils.get_conf_file_name(cfg.CONF.ra_confs, - router_id, - 'radvd.conf', - True) - buf = six.StringIO() - for p in router_ports: - prefix = p['subnet']['cidr'] - if netaddr.IPNetwork(prefix).version == 6: - interface_name = dev_name_helper(p['id']) - ra_mode = p['subnet']['ipv6_ra_mode'] - buf.write('%s' % CONFIG_TEMPLATE.render( - ra_mode=ra_mode, - interface_name=interface_name, - prefix=prefix, - constants=constants)) - - utils.replace_file(radvd_conf, buf.getvalue()) - return radvd_conf - - -def _spawn_radvd(router_id, radvd_conf, router_ns, process_monitor): - def callback(pid_file): - # we need to use -m syslog and f.e. not -m stderr (the default) - # or -m stderr_syslog so that radvd 2.0+ will close stderr and - # exit after daemonization; otherwise, the current thread will - # be locked waiting for result from radvd that won't ever come - # until the process dies - radvd_cmd = [RADVD_SERVICE_CMD, - '-C', '%s' % radvd_conf, - '-p', '%s' % pid_file, - '-m', 'syslog'] - return radvd_cmd - - process_monitor.enable(uuid=router_id, - cmd_callback=callback, - namespace=router_ns, - service=RADVD_SERVICE_NAME, - reload_cfg=True) - LOG.debug("radvd enabled for router %s", router_id) - - -def enable_ipv6_ra(router_id, router_ns, router_ports, - dev_name_helper, process_monitor): - for p in router_ports: - if netaddr.IPNetwork(p['subnet']['cidr']).version == 6: - break - else: - # Kill the daemon if it's running - disable_ipv6_ra(router_id, process_monitor) - return - - LOG.debug("Enable IPv6 RA for router %s", router_id) - radvd_conf = _generate_radvd_conf(router_id, router_ports, dev_name_helper) - _spawn_radvd(router_id, radvd_conf, router_ns, process_monitor) - - -def disable_ipv6_ra(router_id, process_monitor): - process_monitor.disable(router_id, service=RADVD_SERVICE_NAME) - utils.remove_conf_files(cfg.CONF.ra_confs, router_id) - LOG.debug("radvd disabled for router %s", router_id) +class DaemonMonitor(object): + """Manage the data and state of an radvd process.""" + + def __init__(self, router_id, router_ns, process_monitor, dev_name_helper): + self._router_id = router_id + self._router_ns = router_ns + self._process_monitor = process_monitor + self._dev_name_helper = dev_name_helper + + def _generate_radvd_conf(self, router_ports): + radvd_conf = utils.get_conf_file_name(cfg.CONF.ra_confs, + self._router_id, + 'radvd.conf', + True) + buf = six.StringIO() + for p in router_ports: + prefix = p['subnet']['cidr'] + if netaddr.IPNetwork(prefix).version == 6: + interface_name = self._dev_name_helper(p['id']) + ra_mode = p['subnet']['ipv6_ra_mode'] + buf.write('%s' % CONFIG_TEMPLATE.render( + ra_mode=ra_mode, + interface_name=interface_name, + prefix=prefix, + constants=constants)) + + utils.replace_file(radvd_conf, buf.getvalue()) + return radvd_conf + + def _spawn_radvd(self, radvd_conf): + def callback(pid_file): + # we need to use -m syslog and f.e. not -m stderr (the default) + # or -m stderr_syslog so that radvd 2.0+ will close stderr and + # exit after daemonization; otherwise, the current thread will + # be locked waiting for result from radvd that won't ever come + # until the process dies + radvd_cmd = [RADVD_SERVICE_CMD, + '-C', '%s' % radvd_conf, + '-p', '%s' % pid_file, + '-m', 'syslog'] + return radvd_cmd + + self._process_monitor.enable(uuid=self._router_id, + cmd_callback=callback, + namespace=self._router_ns, + service=RADVD_SERVICE_NAME, + reload_cfg=True) + LOG.debug("radvd enabled for router %s", self._router_id) + + def enable(self, router_ports): + for p in router_ports: + if netaddr.IPNetwork(p['subnet']['cidr']).version == 6: + break + else: + # Kill the daemon if it's running + self.disable() + return + + LOG.debug("Enable IPv6 RA for router %s", self._router_id) + radvd_conf = self._generate_radvd_conf(router_ports) + self._spawn_radvd(radvd_conf) + + def disable(self): + self._process_monitor.disable(self._router_id, + service=RADVD_SERVICE_NAME) + utils.remove_conf_files(cfg.CONF.ra_confs, self._router_id) + LOG.debug("radvd disabled for router %s", self._router_id) + + @property + def enabled(self): + return self._process_monitor.is_active(self._router_id, + RADVD_SERVICE_NAME) diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py index 3b8016f45..c17b5b286 100644 --- a/neutron/tests/unit/test_l3_agent.py +++ b/neutron/tests/unit/test_l3_agent.py @@ -296,6 +296,15 @@ class BasicRouterOperationsFramework(base.BaseTestCase): return agent, ri, port + def _process_router_instance_for_agent(self, agent, ri, router): + ri.router = router + if not ri.radvd: + ri.radvd = ra.DaemonMonitor(router['id'], + ri.ns_name, + agent.process_monitor, + agent.get_internal_device_name) + agent.process_router(ri) + class TestBasicRouterOperations(BasicRouterOperationsFramework): def test_periodic_sync_routers_task_raise_exception(self): @@ -1112,7 +1121,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): router['gw_port'] = None ri = l3router.RouterInfo(router['id'], router, **self.ri_kwargs) agent.external_gateway_added = mock.Mock() - agent.process_router(ri) + self._process_router_instance_for_agent(agent, ri, router) orig_nat_rules = ri.iptables_manager.ipv4['nat'].rules[:] # Get NAT rules with the gw_port @@ -1121,7 +1130,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): with mock.patch.object( agent, 'external_gateway_nat_rules') as external_gateway_nat_rules: - agent.process_router(ri) + self._process_router_instance_for_agent(agent, ri, router) new_nat_rules = ri.iptables_manager.ipv4['nat'].rules[:] # There should be no change with the NAT rules @@ -1140,8 +1149,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): router_append_interface(router, count=1, ip_version=6, ra_mode=ra_mode, addr_mode=addr_mode) # Reassign the router object to RouterInfo - ri.router = router - agent.process_router(ri) + self._process_router_instance_for_agent(agent, ri, router) # IPv4 NAT rules should not be changed by adding an IPv6 interface nat_rules_delta = [r for r in ri.iptables_manager.ipv4['nat'].rules if r not in orig_nat_rules] @@ -1207,8 +1215,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): router_append_interface(router, count=1, ip_version=4) router_append_interface(router, count=1, ip_version=6) # Reassign the router object to RouterInfo - ri.router = router - agent.process_router(ri) + self._process_router_instance_for_agent(agent, ri, router) self._assert_ri_process_enabled(ri, 'radvd') def test_process_router_interface_removed(self): @@ -1231,17 +1238,16 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): router = prepare_router_data() ri = l3router.RouterInfo(router['id'], router, **self.ri_kwargs) agent.external_gateway_added = mock.Mock() - ri.router = router - agent.process_router(ri) + self._process_router_instance_for_agent(agent, ri, router) # Add an IPv6 interface and reprocess router_append_interface(router, count=1, ip_version=6) - agent.process_router(ri) + self._process_router_instance_for_agent(agent, ri, router) self._assert_ri_process_enabled(ri, 'radvd') # Reset the calls so we can check for disable radvd self.external_process.reset_mock() # Remove the IPv6 interface and reprocess del router[l3_constants.INTERFACE_KEY][1] - agent.process_router(ri) + self._process_router_instance_for_agent(agent, ri, router) self._assert_ri_process_disabled(ri, 'radvd') def test_process_router_internal_network_added_unexpected_error(self): @@ -1935,7 +1941,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): nat.clear_rules_by_tag.assert_called_once_with('floating_ip') def test_spawn_radvd(self): - router = prepare_router_data() + router = prepare_router_data(ip_version=6) conffile = '/fake/radvd.conf' pidfile = '/fake/radvd.pid' @@ -1947,17 +1953,23 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): self.ip_cls_p.stop() ensure_dir = 'neutron.agent.linux.utils.ensure_dir' + get_conf_file_name = 'neutron.agent.linux.utils.get_conf_file_name' get_pid_file_name = ('neutron.agent.linux.external_process.' 'ProcessManager.get_pid_file_name') - with mock.patch('neutron.agent.linux.utils.execute') as execute: - with mock.patch(get_pid_file_name) as get_pid: - with mock.patch(ensure_dir) as ensure_dir: - get_pid.return_value = pidfile - ra._spawn_radvd(router['id'], - conffile, - agent.get_ns_name(router['id']), - agent.process_monitor) - cmd = execute.call_args[0][0] + utils_execute = 'neutron.agent.linux.utils.execute' + + mock.patch(get_conf_file_name).start().return_value = conffile + mock.patch(get_pid_file_name).start().return_value = pidfile + mock.patch(ensure_dir).start() + execute = mock.patch(utils_execute).start() + + radvd = ra.DaemonMonitor(router['id'], + agent.get_ns_name(router['id']), + agent.process_monitor, + FakeDev) + radvd.enable(router['_interfaces']) + + cmd = execute.call_args[0][0] self.assertIn('radvd', cmd) @@ -1983,9 +1995,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): ri = self._process_router_ipv6_interface_added(router, ra_mode=ra_mode) - ra._generate_radvd_conf(ri.router['id'], - router[l3_constants.INTERFACE_KEY], - mock.Mock()) + ri.radvd._generate_radvd_conf(router[l3_constants.INTERFACE_KEY]) def assertFlag(flag): return (self.assertIn if flag else self.assertNotIn) -- 2.45.2