From: Carl Baldwin Date: Mon, 12 Jan 2015 16:36:40 +0000 (+0000) Subject: Create arping helper in ip_lib X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=7333678c1303fa57effb461fc0c1a00ca72c7033;p=openstack-build%2Fneutron-build.git Create arping helper in ip_lib In trying to restructure the L3 agent in to more modules, some helpers like arping will be used by several modules. It is better to relocate it to a common module which all of them will import and use. Since there is only one spot which passed 'distributed=True', I chose to break the utility in to two. Also, 'distributed' doesn't really describe what that argument is for. So, I named the second utility differently to indicate that it is for sending garps when proxyarp is in use for the address on the interface. Change-Id: Icfdf41917e9e2e0dcd2be19297aee5ac89e96e94 Partially-Implements: blueprint restructure-l3-agent --- diff --git a/neutron/agent/l3/agent.py b/neutron/agent/l3/agent.py index 6579566d7..e07c82fcb 100644 --- a/neutron/agent/l3/agent.py +++ b/neutron/agent/l3/agent.py @@ -714,8 +714,11 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, else: # As GARP is processed in a distinct thread the call below # won't raise an exception to be handled. - self._send_gratuitous_arp_packet( - ri.ns_name, interface_name, fip_ip) + ip_lib.send_gratuitous_arp(ri.ns_name, + interface_name, + fip_ip, + self.conf.send_arp_for_ha, + self.root_helper) return l3_constants.FLOATINGIP_STATUS_ACTIVE def _remove_floating_ip(self, ri, device, ip_cidr): @@ -770,33 +773,6 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, def _get_ex_gw_port(self, ri): return ri.router.get('gw_port') - def _arping(self, ns_name, interface_name, ip_address, distributed=False): - if distributed: - device = ip_lib.IPDevice(interface_name, self.root_helper, - namespace=ns_name) - ip_cidr = str(ip_address) + FLOATING_IP_CIDR_SUFFIX - net = netaddr.IPNetwork(ip_cidr) - device.addr.add(net.version, ip_cidr, str(net.broadcast)) - - arping_cmd = ['arping', '-A', - '-I', interface_name, - '-c', self.conf.send_arp_for_ha, - ip_address] - try: - ip_wrapper = ip_lib.IPWrapper(self.root_helper, - namespace=ns_name) - ip_wrapper.netns.execute(arping_cmd, check_exit_code=True) - except Exception: - LOG.exception(_LE("Failed sending gratuitous ARP.")) - if distributed: - device.addr.delete(net.version, ip_cidr) - - def _send_gratuitous_arp_packet(self, ns_name, interface_name, ip_address, - distributed=False): - if self.conf.send_arp_for_ha > 0: - eventlet.spawn_n(self._arping, ns_name, interface_name, ip_address, - distributed) - def get_internal_device_name(self, port_id): return (INTERNAL_DEV_PREFIX + port_id)[:self.driver.DEV_NAME_LEN] @@ -894,8 +870,11 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, extra_subnets=ex_gw_port.get('extra_subnets', []), preserve_ips=preserve_ips) ip_address = ex_gw_port['ip_cidr'].split('/')[0] - self._send_gratuitous_arp_packet(ns_name, - interface_name, ip_address) + ip_lib.send_gratuitous_arp(ns_name, + interface_name, + ip_address, + self.conf.send_arp_for_ha, + self.root_helper) def external_gateway_removed(self, ri, ex_gw_port, interface_name): if ri.router['distributed']: @@ -948,8 +927,11 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, self.driver.init_l3(interface_name, [internal_cidr], namespace=ns_name) ip_address = internal_cidr.split('/')[0] - self._send_gratuitous_arp_packet(ns_name, interface_name, - ip_address) + ip_lib.send_gratuitous_arp(ns_name, + interface_name, + ip_address, + self.conf.send_arp_for_ha, + self.root_helper) def internal_network_added(self, ri, port): network_id = port['network_id'] diff --git a/neutron/agent/l3/dvr.py b/neutron/agent/l3/dvr.py index db88b5a84..a0fb93432 100644 --- a/neutron/agent/l3/dvr.py +++ b/neutron/agent/l3/dvr.py @@ -242,7 +242,11 @@ class AgentMixin(object): self.driver.init_l3(interface_name, [ex_gw_port['ip_cidr']], namespace=ns_name) ip_address = ex_gw_port['ip_cidr'].split('/')[0] - self._send_gratuitous_arp_packet(ns_name, interface_name, ip_address) + ip_lib.send_gratuitous_arp(ns_name, + interface_name, + ip_address, + self.conf.send_arp_for_ha, + self.root_helper) gw_ip = ex_gw_port['subnet']['gateway_ip'] if gw_ip: @@ -332,9 +336,11 @@ class AgentMixin(object): device.route.add_route(fip_cidr, str(rtr_2_fip.ip)) interface_name = ( self.get_fip_ext_device_name(self.agent_gateway_port['id'])) - self._send_gratuitous_arp_packet(fip_ns_name, - interface_name, floating_ip, - distributed=True) + ip_lib.send_garp_for_proxyarp(fip_ns_name, + interface_name, + floating_ip, + self.conf.send_arp_for_ha, + self.root_helper) # update internal structures ri.dist_fip_count = ri.dist_fip_count + 1 diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py index 4ca7591df..dea22c466 100644 --- a/neutron/agent/linux/ip_lib.py +++ b/neutron/agent/linux/ip_lib.py @@ -13,6 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. +import eventlet import os import netaddr @@ -20,7 +21,10 @@ from oslo.config import cfg from neutron.agent.linux import utils from neutron.common import exceptions +from neutron.i18n import _LE +from neutron.openstack.common import log as logging +LOG = logging.getLogger(__name__) OPTS = [ cfg.BoolOpt('ip_lib_force_root', @@ -610,3 +614,50 @@ def iproute_arg_supported(command, arg, root_helper=None): stdout, stderr = utils.execute(command, root_helper=root_helper, check_exit_code=False, return_stderr=True) return any(arg in line for line in stderr.split('\n')) + + +def _arping(ns_name, iface_name, address, count, root_helper): + arping_cmd = ['arping', '-A', '-I', iface_name, '-c', count, address] + try: + ip_wrapper = IPWrapper(root_helper, namespace=ns_name) + ip_wrapper.netns.execute(arping_cmd, check_exit_code=True) + except Exception: + msg = _LE("Failed sending gratuitous ARP " + "to %(addr)s on %(iface)s in namespace %(ns)s") + LOG.exception(msg, {'addr': address, + 'iface': iface_name, + 'ns': ns_name}) + + +def send_gratuitous_arp(ns_name, iface_name, address, count, root_helper): + """Send a gratuitous arp using given namespace, interface, and address""" + + def arping(): + _arping(ns_name, iface_name, address, count, root_helper) + + if count > 0: + eventlet.spawn_n(arping) + + +def send_garp_for_proxyarp(ns_name, iface_name, address, count, root_helper): + """ + Send a gratuitous arp using given namespace, interface, and address + + This version should be used when proxy arp is in use since the interface + won't actually have the address configured. We actually need to configure + the address on the interface and then remove it when the proxy arp has been + sent. + """ + def arping_with_temporary_address(): + # Configure the address on the interface + device = IPDevice(iface_name, root_helper, namespace=ns_name) + net = netaddr.IPNetwork(str(address)) + device.addr.add(net.version, str(net), str(net.broadcast)) + + _arping(ns_name, iface_name, address, count, root_helper) + + # Delete the address from the interface + device.addr.delete(net.version, str(net)) + + if count > 0: + eventlet.spawn_n(arping_with_temporary_address) diff --git a/neutron/tests/functional/agent/test_l3_agent.py b/neutron/tests/functional/agent/test_l3_agent.py index c8270d8d6..48bf58e00 100644 --- a/neutron/tests/functional/agent/test_l3_agent.py +++ b/neutron/tests/functional/agent/test_l3_agent.py @@ -88,7 +88,7 @@ class L3AgentTestFramework(base.BaseOVSLinuxTestCase): '%s/external/pids' % temp_dir) conf.set_override('host', host) agent = l3_test_agent.TestL3NATAgent(host, conf) - mock.patch.object(agent, '_arping').start() + mock.patch.object(ip_lib, 'send_gratuitous_arp').start() return agent diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py index c64a30391..ae7bc0dc8 100644 --- a/neutron/tests/unit/test_l3_agent.py +++ b/neutron/tests/unit/test_l3_agent.py @@ -203,9 +203,13 @@ class TestBasicRouterOperations(base.BaseTestCase): self.external_process = self.external_process_p.start() self.send_arp_p = mock.patch( - 'neutron.agent.l3.agent.L3NATAgent._send_gratuitous_arp_packet') + 'neutron.agent.linux.ip_lib.send_gratuitous_arp') self.send_arp = self.send_arp_p.start() + self.send_arp_proxyarp_p = mock.patch( + 'neutron.agent.linux.ip_lib.send_garp_for_proxyarp') + self.send_arp_proxyarp = self.send_arp_proxyarp_p.start() + self.dvr_cls_p = mock.patch('neutron.agent.linux.interface.NullDriver') driver_cls = self.dvr_cls_p.start() self.mock_driver = mock.MagicMock() @@ -328,7 +332,8 @@ class TestBasicRouterOperations(base.BaseTestCase): self.assertEqual(self.mock_driver.plug.call_count, 1) self.assertEqual(self.mock_driver.init_l3.call_count, 1) self.send_arp.assert_called_once_with(ri.ns_name, interface_name, - '99.0.1.9') + '99.0.1.9', + mock.ANY, mock.ANY) elif action == 'remove': self.device_exists.return_value = True agent.internal_network_removed(ri, port) @@ -415,7 +420,8 @@ class TestBasicRouterOperations(base.BaseTestCase): self.assertEqual(self.mock_driver.init_l3.call_count, 1) self.send_arp.assert_called_once_with(ri.ns_name, interface_name, - '20.0.0.30') + '20.0.0.30', + mock.ANY, mock.ANY) kwargs = {'preserve_ips': ['192.168.1.34/32'], 'namespace': 'qrouter-' + router['id'], 'gateway': '20.0.0.1', @@ -468,7 +474,7 @@ class TestBasicRouterOperations(base.BaseTestCase): self.assertEqual(self.mock_driver.plug.call_count, 0) self.assertEqual(self.mock_driver.init_l3.call_count, 1) self.send_arp.assert_called_once_with(ri.ns_name, interface_name, - '20.0.0.30') + '20.0.0.30', mock.ANY, mock.ANY) kwargs = {'preserve_ips': ['192.168.1.34/32'], 'namespace': 'qrouter-' + router['id'], 'gateway': '20.0.0.1', @@ -520,30 +526,6 @@ class TestBasicRouterOperations(base.BaseTestCase): router['gw_port_host'] = HOSTNAME self._test_external_gateway_action('add', router) - def _test_arping(self, namespace): - if not namespace: - self.conf.set_override('use_namespaces', False) - - router_id = _uuid() - ri = l3router.RouterInfo(router_id, self.conf.root_helper, {}) - agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) - floating_ip = '20.0.0.101' - interface_name = agent.get_external_device_name(router_id) - agent._arping(ri, interface_name, floating_ip) - - arping_cmd = ['arping', '-A', - '-I', interface_name, - '-c', self.conf.send_arp_for_ha, - floating_ip] - self.mock_ip.netns.execute.assert_any_call( - arping_cmd, check_exit_code=True) - - def test_arping_namespace(self): - self._test_arping(namespace=True) - - def test_arping_no_namespace(self): - self._test_arping(namespace=False) - def test_agent_remove_external_gateway(self): router = prepare_router_data(num_internal_ports=2) self._test_external_gateway_action('remove', router) @@ -1856,7 +1838,8 @@ class TestBasicRouterOperations(base.BaseTestCase): self.assertEqual(self.mock_driver.init_l3.call_count, 1) if self.conf.use_namespaces: self.send_arp.assert_called_once_with(fip_ns_name, interface_name, - '20.0.0.30') + '20.0.0.30', + mock.ANY, mock.ANY) else: self.utils_exec.assert_any_call( check_exit_code=True, root_helper=self.conf.root_helper) diff --git a/neutron/tests/unit/test_linux_ip_lib.py b/neutron/tests/unit/test_linux_ip_lib.py index 27564821d..954dd7666 100644 --- a/neutron/tests/unit/test_linux_ip_lib.py +++ b/neutron/tests/unit/test_linux_ip_lib.py @@ -917,3 +917,70 @@ class TestIpNeighCommand(TestIPCmdBase): self.neigh_cmd.delete(4, '192.168.45.100', 'cc:dd:ee:ff:ab:cd') self._assert_sudo([4], ('del', '192.168.45.100', 'lladdr', 'cc:dd:ee:ff:ab:cd', 'dev', 'tap0')) + + +class TestArpPing(TestIPCmdBase): + def _test_arping(self, function, address, spawn_n, mIPWrapper): + spawn_n.side_effect = lambda f: f() + function(mock.sentinel.ns_name, + mock.sentinel.iface_name, + address, + mock.sentinel.count, + mock.sentinel.root_helper) + + self.assertTrue(spawn_n.called) + mIPWrapper.assert_called_once_with(mock.sentinel.root_helper, + namespace=mock.sentinel.ns_name) + + ip_wrapper = mIPWrapper(mock.sentinel.root_helper, + mock.sentinel.ns_name) + + # Just test that arping is called with the right arguments + arping_cmd = ['arping', '-A', + '-I', mock.sentinel.iface_name, + '-c', mock.sentinel.count, + address] + ip_wrapper.netns.execute.assert_any_call(arping_cmd, + check_exit_code=True) + + @mock.patch.object(ip_lib, 'IPWrapper') + @mock.patch('eventlet.spawn_n') + def test_send_gratuitous_arp(self, spawn_n, mIPWrapper): + self._test_arping( + ip_lib.send_gratuitous_arp, '20.0.0.1', spawn_n, mIPWrapper) + + @mock.patch.object(ip_lib, 'IPDevice') + @mock.patch.object(ip_lib, 'IPWrapper') + @mock.patch('eventlet.spawn_n') + def test_send_garp_for_proxy_arp(self, spawn_n, mIPWrapper, mIPDevice): + addr = '20.0.0.1' + ip_wrapper = mIPWrapper(mock.sentinel.root_helper, + mock.sentinel.ns_name) + mIPWrapper.reset_mock() + device = mIPDevice(mock.sentinel.iface_name, + mock.sentinel.root_helper, + namespace=mock.sentinel.ns_name) + mIPDevice.reset_mock() + + # Check that the address was added to the interface before arping + def check_added_address(*args, **kwargs): + mIPDevice.assert_called_once_with(mock.sentinel.iface_name, + mock.sentinel.root_helper, + namespace=mock.sentinel.ns_name) + device.addr.add.assert_called_once_with(4, addr + '/32', addr) + self.assertFalse(device.addr.delete.called) + device.addr.reset_mock() + + ip_wrapper.netns.execute.side_effect = check_added_address + + self._test_arping( + ip_lib.send_garp_for_proxyarp, addr, spawn_n, mIPWrapper) + + # Test that the address was removed after arping + device = mIPDevice(mock.sentinel.iface_name, + mock.sentinel.root_helper, + namespace=mock.sentinel.ns_name) + device.addr.delete.assert_called_once_with(4, addr + '/32') + + # If this was called then check_added_address probably had a assert + self.assertFalse(device.addr.add.called)