From 081e1016debcae388b383e10302f72287fe4f0fe Mon Sep 17 00:00:00 2001 From: Dan Wendlandt Date: Sat, 11 Aug 2012 19:42:59 -0700 Subject: [PATCH] Linux Agent improvements for L3 prereq for bp quantum-l3-fw-nat - make init_l3 take cidrs, rather than assuming an augmented port object - make namespace for agent operations optional and allow the namespace name to be configured. - allow plug() operation to take an optional bridge parameter indicating which bridge to plug into - add namespace support for iptables manager - make OVS plug() set the IP address, etc. of a device even if it already exists. Change-Id: Id4fec9bf7cda30c45b94eccd25e9e54dc5af97b7 --- quantum/agent/dhcp_agent.py | 14 ++- quantum/agent/linux/interface.py | 99 +++++++++-------- quantum/agent/linux/ip_lib.py | 2 +- quantum/agent/linux/iptables_manager.py | 13 ++- quantum/agent/linux/ovs_lib.py | 2 +- quantum/tests/unit/test_dhcp_agent.py | 43 ++++++-- quantum/tests/unit/test_linux_interface.py | 120 ++++++++++++++------- quantum/tests/unit/test_linux_ip_lib.py | 2 +- 8 files changed, 191 insertions(+), 104 deletions(-) diff --git a/quantum/agent/dhcp_agent.py b/quantum/agent/dhcp_agent.py index b8e203434d..9e3e48d1c3 100644 --- a/quantum/agent/dhcp_agent.py +++ b/quantum/agent/dhcp_agent.py @@ -22,6 +22,7 @@ import sys import time import uuid +import netaddr from sqlalchemy.ext import sqlsoup from quantum.agent.common import config @@ -243,8 +244,17 @@ class DeviceManager(object): self.driver.plug(network.id, port.id, interface_name, - port.mac_address) - self.driver.init_l3(port, interface_name) + port.mac_address, + namespace=network.id) + ip_cidrs = [] + for fixed_ip in port.fixed_ips: + subnet = fixed_ip.subnet + net = netaddr.IPNetwork(subnet.cidr) + ip_cidr = '%s/%s' % (fixed_ip.ip_address, net.prefixlen) + ip_cidrs.append(ip_cidr) + + self.driver.init_l3(interface_name, ip_cidrs, + namespace=network.id) def destroy(self, network): self.driver.unplug(self.get_interface_name(network)) diff --git a/quantum/agent/linux/interface.py b/quantum/agent/linux/interface.py index b622d1e96a..023c76c382 100644 --- a/quantum/agent/linux/interface.py +++ b/quantum/agent/linux/interface.py @@ -50,22 +50,21 @@ class LinuxInterfaceDriver(object): def __init__(self, conf): self.conf = conf - def init_l3(self, port, device_name): - """Set the L3 settings for the interface using data from the port.""" - device = ip_lib.IPDevice(device_name, - self.conf.root_helper, - port.network.id) + def init_l3(self, device_name, ip_cidrs, namespace=None): + """Set the L3 settings for the interface using data from the port. + ip_cidrs: list of 'X.X.X.X/YY' strings + """ + device = ip_lib.IPDevice(device_name, self.conf.root_helper, + namespace=namespace) previous = {} for address in device.addr.list(scope='global', filters=['permanent']): previous[address['cidr']] = address['ip_version'] # add new addresses - for fixed_ip in port.fixed_ips: - subnet = fixed_ip.subnet - net = netaddr.IPNetwork(subnet.cidr) - ip_cidr = '%s/%s' % (fixed_ip.ip_address, net.prefixlen) + for ip_cidr in ip_cidrs: + net = netaddr.IPNetwork(ip_cidr) if ip_cidr in previous: del previous[ip_cidr] continue @@ -78,40 +77,44 @@ class LinuxInterfaceDriver(object): def check_bridge_exists(self, bridge): if not ip_lib.device_exists(bridge): - raise exception.BridgeDoesNotExist(bridge=bridge) + raise exceptions.BridgeDoesNotExist(bridge=bridge) def get_device_name(self, port): return (self.DEV_NAME_PREFIX + port.id)[:self.DEV_NAME_LEN] @abc.abstractmethod - def plug(self, network_id, port_id, device_name, mac_address): + def plug(self, network_id, port_id, device_name, mac_address, + bridge=None, namespace=None): """Plug in the interface.""" @abc.abstractmethod - def unplug(self, device_name): + def unplug(self, device_name, bridge=None): """Unplug the interface.""" class NullDriver(LinuxInterfaceDriver): - def plug(self, network_id, port_id, device_name, mac_address): + def plug(self, network_id, port_id, device_name, mac_address, + bridge=None, namespace=None): pass - def unplug(self, device_name): + def unplug(self, device_name, bridge=None): pass class OVSInterfaceDriver(LinuxInterfaceDriver): - """Driver for creating an OVS interface.""" + """Driver for creating an internal interface on an OVS bridge.""" - def plug(self, network_id, port_id, device_name, mac_address): + def plug(self, network_id, port_id, device_name, mac_address, + bridge=None, namespace=None): """Plug in the interface.""" - bridge = self.conf.ovs_integration_bridge + if not bridge: + bridge = self.conf.ovs_integration_bridge self.check_bridge_exists(bridge) if not ip_lib.device_exists(device_name, self.conf.root_helper, - namespace=network_id): + namespace=namespace): utils.execute(['ovs-vsctl', '--', '--may-exist', 'add-port', bridge, @@ -127,24 +130,24 @@ class OVSInterfaceDriver(LinuxInterfaceDriver): mac_address], self.conf.root_helper) - ip = ip_lib.IPWrapper(self.conf.root_helper) - device = ip.device(device_name) - device.link.set_address(mac_address) - if self.conf.network_device_mtu: - device.link.set_mtu(self.conf.network_device_mtu) + ip = ip_lib.IPWrapper(self.conf.root_helper) + device = ip.device(device_name) + device.link.set_address(mac_address) + if self.conf.network_device_mtu: + device.link.set_mtu(self.conf.network_device_mtu) - namespace = ip.ensure_namespace(network_id) - namespace.add_device_to_namespace(device) - device.link.set_up() - else: - LOG.error(_('Device %s already exists') % device) + if namespace: + namespace_obj = ip.ensure_namespace(namespace) + namespace_obj.add_device_to_namespace(device) + device.link.set_up() - def unplug(self, device_name): + def unplug(self, device_name, bridge=None): """Unplug the interface.""" - bridge_name = self.conf.ovs_integration_bridge + if not bridge: + bridge = self.conf.ovs_integration_bridge - self.check_bridge_exists(bridge_name) - bridge = ovs_lib.OVSBridge(bridge_name, self.conf.root_helper) + self.check_bridge_exists(bridge) + bridge = ovs_lib.OVSBridge(bridge, self.conf.root_helper) bridge.delete_port(device_name) @@ -153,19 +156,21 @@ class BridgeInterfaceDriver(LinuxInterfaceDriver): DEV_NAME_PREFIX = 'dhc' - def plug(self, network_id, port_id, device_name, mac_address): + def plug(self, network_id, port_id, device_name, mac_address, + bridge=None, namespace=None): """Plugin the interface.""" if not ip_lib.device_exists(device_name, self.conf.root_helper, - namespace=network_id): + namespace=namespace): ip = ip_lib.IPWrapper(self.conf.root_helper) tap_name = device_name.replace(self.DEV_NAME_PREFIX, 'tap') root_veth, dhcp_veth = ip.add_veth(tap_name, device_name) root_veth.link.set_address(mac_address) - namespace = ip.ensure_namespace(network_id) - namespace.add_device_to_namespace(dhcp_veth) + if namespace: + namespace_obj = ip.ensure_namespace(namespace) + namespace_obj.add_device_to_namespace(dhcp_veth) root_veth.link.set_up() dhcp_veth.link.set_up() @@ -173,7 +178,7 @@ class BridgeInterfaceDriver(LinuxInterfaceDriver): else: LOG.warn(_("Device %s already exists") % device_name) - def unplug(self, device_name): + def unplug(self, device_name, bridge=None): """Unplug the interface.""" device = ip_lib.IPDevice(device_name, self.conf.root_helper) try: @@ -194,15 +199,17 @@ class RyuInterfaceDriver(OVSInterfaceDriver): LOG.debug('ryu rest host %s', self.conf.ryu_api_host) self.ryu_client = OFPClient(self.conf.ryu_api_host) - self.check_bridge_exists(self.conf.ovs_integration_bridge) - self.ovs_br = ovs_lib.OVSBridge(self.conf.ovs_integration_bridge, - self.conf.root_helper) - self.datapath_id = self.ovs_br.get_datapath_id() - - def plug(self, network_id, port_id, device_name, mac_address): + def plug(self, network_id, port_id, device_name, mac_address, + bridge=None, namespace=None): """Plug in the interface.""" super(RyuInterfaceDriver, self).plug(network_id, port_id, device_name, - mac_address) + mac_address, bridge=bridge, + namespace=namespace) + if not bridge: + bridge = self.conf.ovs_integration_bridge - port_no = self.ovs_br.get_port_ofport(device_name) - self.ryu_client.create_port(network_id, self.datapath_id, port_no) + self.check_bridge_exists(bridge) + ovs_br = ovs_lib.OVSBridge(bridge, self.conf.root_helper) + datapath_id = ovs_br.get_datapath_id() + port_no = ovs_br.get_port_ofport(device_name) + self.ryu_client.create_port(network_id, datapath_id, port_no) diff --git a/quantum/agent/linux/ip_lib.py b/quantum/agent/linux/ip_lib.py index 2eb254f48d..f03cc66e98 100644 --- a/quantum/agent/linux/ip_lib.py +++ b/quantum/agent/linux/ip_lib.py @@ -99,7 +99,7 @@ class IPWrapper(SubProcessBase): @classmethod def get_namespaces(cls, root_helper): - output = cls._execute('netns', ('list',), root_helper=root_helper) + output = cls._execute('', 'netns', ('list',), root_helper=root_helper) return [l.strip() for l in output.split('\n')] diff --git a/quantum/agent/linux/iptables_manager.py b/quantum/agent/linux/iptables_manager.py index fa865b35bd..2d707616c5 100755 --- a/quantum/agent/linux/iptables_manager.py +++ b/quantum/agent/linux/iptables_manager.py @@ -194,7 +194,7 @@ class IptablesManager(object): """ def __init__(self, _execute=None, state_less=False, - root_helper=None, use_ipv6=False): + root_helper=None, use_ipv6=False, namespace=None): if _execute: self.execute = _execute else: @@ -202,6 +202,7 @@ class IptablesManager(object): self.use_ipv6 = use_ipv6 self.root_helper = root_helper + self.namespace = namespace self.ipv4 = {'filter': IptablesTable()} self.ipv6 = {'filter': IptablesTable()} @@ -276,12 +277,18 @@ class IptablesManager(object): for cmd, tables in s: for table in tables: - current_table = (self.execute(['%s-save' % cmd, '-t', table], + args = ['%s-save' % cmd, '-t', table] + if self.namespace: + args = ['ip', 'netns', 'exec', self.namespace] + args + current_table = (self.execute(args, root_helper=self.root_helper)) current_lines = current_table.split('\n') new_filter = self._modify_rules(current_lines, tables[table]) - self.execute(['%s-restore' % (cmd)], + args = ['%s-restore' % (cmd)] + if self.namespace: + args = ['ip', 'netns', 'exec', self.namespace] + args + self.execute(args, process_input='\n'.join(new_filter), root_helper=self.root_helper) LOG.debug(("IPTablesManager.apply completed with success")) diff --git a/quantum/agent/linux/ovs_lib.py b/quantum/agent/linux/ovs_lib.py index cfb2d784ae..204c981572 100644 --- a/quantum/agent/linux/ovs_lib.py +++ b/quantum/agent/linux/ovs_lib.py @@ -89,7 +89,7 @@ class OVSBridge: def _build_flow_expr_arr(self, **kwargs): flow_expr_arr = [] is_delete_expr = kwargs.get('delete', False) - print "kwargs = %s" % kwargs + if not is_delete_expr: prefix = ("hard_timeout=%s,idle_timeout=%s,priority=%s" % (kwargs.get('hard_timeout', '0'), diff --git a/quantum/tests/unit/test_dhcp_agent.py b/quantum/tests/unit/test_dhcp_agent.py index 3e2312ea83..b1c822f800 100644 --- a/quantum/tests/unit/test_dhcp_agent.py +++ b/quantum/tests/unit/test_dhcp_agent.py @@ -34,6 +34,23 @@ class FakeModel: return str(self.__dict__) +class FakePortModel(FakeModel): + fixed_ips = [] + + +class FakeFixedIPModel(object): + + def __init__(self, ip_address, cidr): + self.subnet = FakeSubnetModel(cidr) + self.ip_address = ip_address + + +class FakeSubnetModel(object): + + def __init__(self, cidr): + self.cidr = cidr + + class TestDhcpAgent(unittest.TestCase): def setUp(self): self.conf = config.setup_conf() @@ -384,17 +401,22 @@ class TestDeviceManager(unittest.TestCase): self.client_cls_p.stop() def test_setup(self): + port_id = '12345678-1234-aaaa-1234567890ab' + network_id = '12345678-1234-5678-1234567890ab' fake_subnets = [FakeModel('12345678-aaaa-aaaa-1234567890ab'), FakeModel('12345678-bbbb-bbbb-1234567890ab')] - fake_network = FakeModel('12345678-1234-5678-1234567890ab', + fake_network = FakeModel(network_id, tenant_id='aaaaaaaa-aaaa-aaaa-aaaaaaaaaaaa', subnets=fake_subnets) - fake_port = FakeModel('12345678-1234-aaaa-1234567890ab', - mac_address='aa:bb:cc:dd:ee:ff') - - port_dict = dict(mac_address='aa:bb:cc:dd:ee:ff', allocations=[], id=1) + fake_port = FakePortModel(port_id, mac_address='aa:bb:cc:dd:ee:ff', + network_id=network_id, + allocations=[]) + fake_port.fixed_ips.append(FakeFixedIPModel('172.9.9.9', + '172.9.9.0/24')) + port_dict = dict(mac_address='aa:bb:cc:dd:ee:ff', + allocations=[], id=1) self.client_inst.create_port.return_value = dict(port=port_dict) self.device_exists.return_value = False @@ -427,11 +449,14 @@ class TestDeviceManager(unittest.TestCase): mock.call.create_port(mock.ANY)]) self.mock_driver.assert_has_calls([ - mock.call.plug('12345678-1234-5678-1234567890ab', - '12345678-1234-aaaa-1234567890ab', + mock.call.get_device_name(mock.ANY), + mock.call.plug(network_id, + port_id, 'tap12345678-12', - 'aa:bb:cc:dd:ee:ff'), - mock.call.init_l3(mock.ANY, 'tap12345678-12')] + 'aa:bb:cc:dd:ee:ff', + namespace=network_id), + mock.call.init_l3('tap12345678-12', ['172.9.9.9/24'], + namespace=network_id)] ) def test_destroy(self): diff --git a/quantum/tests/unit/test_linux_interface.py b/quantum/tests/unit/test_linux_interface.py index 8de30e28ae..13fe3da5ab 100644 --- a/quantum/tests/unit/test_linux_interface.py +++ b/quantum/tests/unit/test_linux_interface.py @@ -92,21 +92,37 @@ class TestABCDriver(TestBase): self.ip_dev().addr.list = mock.Mock(return_value=addresses) bc = BaseChild(self.conf) - bc.init_l3(FakePort(), 'tap0') + ns = '12345678-1234-5678-90ab-ba0987654321' + bc.init_l3('tap0', ['192.168.1.2/24'], namespace=ns) self.ip_dev.assert_has_calls( - [mock.call('tap0', 'sudo', '12345678-1234-5678-90ab-ba0987654321'), + [mock.call('tap0', 'sudo', namespace=ns), mock.call().addr.list(scope='global', filters=['permanent']), mock.call().addr.add(4, '192.168.1.2/24', '192.168.1.255'), mock.call().addr.delete(4, '172.16.77.240/24')]) class TestOVSInterfaceDriver(TestBase): - def test_plug(self, additional_expectation=[]): + + def test_plug_no_ns(self): + self._test_plug() + + def test_plug_with_ns(self): + self._test_plug(namespace='01234567-1234-1234-99') + + def test_plug_alt_bridge(self): + self._test_plug(bridge='br-foo') + + def _test_plug(self, additional_expectation=[], bridge=None, + namespace=None): + + if not bridge: + bridge = 'br-int' + def device_exists(dev, root_helper=None, namespace=None): - return dev == 'br-int' + return dev == bridge vsctl_cmd = ['ovs-vsctl', '--', '--may-exist', 'add-port', - 'br-int', 'tap0', '--', 'set', 'Interface', 'tap0', + bridge, 'tap0', '--', 'set', 'Interface', 'tap0', 'type=internal', '--', 'set', 'Interface', 'tap0', 'external-ids:iface-id=port-1234', '--', 'set', 'Interface', 'tap0', @@ -120,29 +136,35 @@ class TestOVSInterfaceDriver(TestBase): ovs.plug('01234567-1234-1234-99', 'port-1234', 'tap0', - 'aa:bb:cc:dd:ee:ff') + 'aa:bb:cc:dd:ee:ff', + bridge=bridge, + namespace=namespace) execute.assert_called_once_with(vsctl_cmd, 'sudo') expected = [mock.call('sudo'), mock.call().device('tap0'), mock.call().device().link.set_address('aa:bb:cc:dd:ee:ff')] expected.extend(additional_expectation) - expected.extend( - [mock.call().ensure_namespace('01234567-1234-1234-99'), - mock.call().ensure_namespace().add_device_to_namespace(mock.ANY), - mock.call().device().link.set_up()]) + if namespace: + expected.extend( + [mock.call().ensure_namespace(namespace), + mock.call().ensure_namespace().add_device_to_namespace( + mock.ANY)]) + expected.extend([mock.call().device().link.set_up()]) self.ip.assert_has_calls(expected) def test_plug_mtu(self): self.conf.set_override('network_device_mtu', 9000) - self.test_plug([mock.call().device().link.set_mtu(9000)]) + self._test_plug([mock.call().device().link.set_mtu(9000)]) - def test_unplug(self): + def test_unplug(self, bridge=None): + if not bridge: + bridge = 'br-int' with mock.patch('quantum.agent.linux.ovs_lib.OVSBridge') as ovs_br: ovs = interface.OVSInterfaceDriver(self.conf) ovs.unplug('tap0') - ovs_br.assert_has_calls([mock.call('br-int', 'sudo'), + ovs_br.assert_has_calls([mock.call(bridge, 'sudo'), mock.call().delete_port('tap0')]) @@ -152,7 +174,13 @@ class TestBridgeInterfaceDriver(TestBase): device_name = br.get_device_name(FakePort()) self.assertEqual('dhcabcdef01-12', device_name) - def test_plug(self): + def test_plug_no_ns(self): + self._test_plug() + + def test_plug_with_ns(self): + self._test_plug(namespace='01234567-1234-1234-99') + + def _test_plug(self, namespace=None): def device_exists(device, root_helper=None, namespace=None): return device.startswith('brq') @@ -172,13 +200,17 @@ class TestBridgeInterfaceDriver(TestBase): br.plug('01234567-1234-1234-99', 'port-1234', 'dhc0', - 'aa:bb:cc:dd:ee:ff') + 'aa:bb:cc:dd:ee:ff', + namespace=namespace) - self.ip.assert_has_calls( - [mock.call('sudo'), - mock.call().add_veth('tap0', 'dhc0'), - mock.call().ensure_namespace('01234567-1234-1234-99'), - mock.call().ensure_namespace().add_device_to_namespace(ns_veth)]) + ip_calls = [mock.call('sudo'), mock.call().add_veth('tap0', 'dhc0')] + if namespace: + ip_calls.extend([ + mock.call().ensure_namespace('01234567-1234-1234-99'), + mock.call().ensure_namespace().add_device_to_namespace( + ns_veth)]) + + self.ip.assert_has_calls(ip_calls) root_veth.assert_has_calls([mock.call.link.set_up()]) ns_veth.assert_has_calls([mock.call.link.set_up()]) @@ -240,25 +272,24 @@ class TestRyuInterfaceDriver(TestBase): self.ryu_mod_p.stop() super(TestRyuInterfaceDriver, self).tearDown() - @staticmethod - def _device_exists(dev, root_helper=None, namespace=None): - return dev == 'br-int' + def test_plug_no_ns(self): + self._test_plug() - _vsctl_cmd_init = ['ovs-vsctl', '--timeout=2', - 'get', 'Bridge', 'br-int', 'datapath_id'] + def test_plug_with_ns(self): + self._test_plug(namespace='01234567-1234-1234-99') - def test_init(self): - with mock.patch.object(utils, 'execute') as execute: - self.device_exists.side_effect = self._device_exists - interface.RyuInterfaceDriver(self.conf) - execute.assert_called_once_with(self._vsctl_cmd_init, - root_helper='sudo') + def test_plug_alt_bridge(self): + self._test_plug(bridge='br-foo') - self.ryu_app_client.OFPClient.assert_called_once_with('127.0.0.1:8080') + def _test_plug(self, namespace=None, bridge=None): + if not bridge: + bridge = 'br-int' + + def _device_exists(dev, root_helper=None, namespace=None): + return dev == bridge - def test_plug(self): vsctl_cmd_plug = ['ovs-vsctl', '--', '--may-exist', 'add-port', - 'br-int', 'tap0', '--', 'set', 'Interface', 'tap0', + bridge, 'tap0', '--', 'set', 'Interface', 'tap0', 'type=internal', '--', 'set', 'Interface', 'tap0', 'external-ids:iface-id=port-1234', '--', 'set', 'Interface', 'tap0', @@ -267,17 +298,21 @@ class TestRyuInterfaceDriver(TestBase): 'external-ids:attached-mac=aa:bb:cc:dd:ee:ff'] vsctl_cmd_ofport = ['ovs-vsctl', '--timeout=2', 'get', 'Interface', 'tap0', 'ofport'] + vsctl_cmd_dp = ['ovs-vsctl', '--timeout=2', + 'get', 'Bridge', bridge, 'datapath_id'] with mock.patch.object(utils, 'execute') as execute: - self.device_exists.side_effect = self._device_exists + self.device_exists.side_effect = _device_exists ryu = interface.RyuInterfaceDriver(self.conf) ryu.plug('01234567-1234-1234-99', 'port-1234', 'tap0', - 'aa:bb:cc:dd:ee:ff') + 'aa:bb:cc:dd:ee:ff', + bridge=bridge, + namespace=namespace) - execute.assert_has_calls([mock.call(self._vsctl_cmd_init, + execute.assert_has_calls([mock.call(vsctl_cmd_dp, root_helper='sudo')]) execute.assert_has_calls([mock.call(vsctl_cmd_plug, 'sudo')]) execute.assert_has_calls([mock.call(vsctl_cmd_ofport, @@ -288,9 +323,12 @@ class TestRyuInterfaceDriver(TestBase): expected = [ mock.call('sudo'), mock.call().device('tap0'), - mock.call().device().link.set_address('aa:bb:cc:dd:ee:ff'), - mock.call().ensure_namespace('01234567-1234-1234-99'), - mock.call().ensure_namespace().add_device_to_namespace(mock.ANY), - mock.call().device().link.set_up()] + mock.call().device().link.set_address('aa:bb:cc:dd:ee:ff')] + if namespace: + expected.extend([ + mock.call().ensure_namespace(namespace), + mock.call().ensure_namespace().add_device_to_namespace( + mock.ANY)]) + expected.extend([mock.call().device().link.set_up()]) self.ip.assert_has_calls(expected) diff --git a/quantum/tests/unit/test_linux_ip_lib.py b/quantum/tests/unit/test_linux_ip_lib.py index ba6cb00e33..068aaf1e1d 100644 --- a/quantum/tests/unit/test_linux_ip_lib.py +++ b/quantum/tests/unit/test_linux_ip_lib.py @@ -149,7 +149,7 @@ class TestIpWrapper(unittest.TestCase): 'bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb', 'cccccccc-cccc-cccc-cccc-cccccccccccc']) - self.execute.assert_called_once_with('netns', ('list',), + self.execute.assert_called_once_with('', 'netns', ('list',), root_helper='sudo') def test_add_tuntap(self):