From 16866d2cf8ce8c399a23e80668e4c4b53ff07f23 Mon Sep 17 00:00:00 2001 From: James Page Date: Wed, 31 May 2017 17:15:50 +0100 Subject: [PATCH] Refactor container VIF handling for linuxbridge Neutron recently changed behaviour to complete all bridge setup and configuration for the linuxbridge driver via the neutron linuxbridge agent, requiring Nova simply to setup the tap device that subsequently gets plugged into a linuxbridge. Rework plug/unplug handling based on libvirt driver to fallback to a legacy style plug/unplug driver for tap devices of this kind. In the case of LXD we actually still use a veth pair so that: a) security group rules are applied to the tap named device on the host. b) the container still gets part of a nic to use, named ethX internally to the container. c) the neutron linuxbridge agent can see the required tapXXX device prior to the container being created, allowing VIF plugging to be completed. This looks something like this once wired and running: Host | Container [bridge] <-> [tapXXX|tinXXX] <-> [ethX] The veth pair is mapping into a LXD container using the physical LXD nic type. As the drive now creates the veth pair for unbridged network types, unplug must occur after the device has been removed from the container during interface_detach. Rework LXD device profile naming for consistency: a) VIFs attaching to bridges will be named inline with the bridge (no-change) b) VIFs not being attached to a bridge will be named with the VIF devname (changed from 'unbridged' which did not support any multiplicity). Change-Id: I2fdf41e5640f5ca5e3bcd7df1aa159a65b706138 Closes-Bug: 1694719 --- nova/tests/unit/virt/lxd/test_driver.py | 4 +- nova/tests/unit/virt/lxd/test_vif.py | 25 +++++++- nova/virt/lxd/driver.py | 21 ++++--- nova/virt/lxd/flavor.py | 14 ++--- nova/virt/lxd/vif.py | 79 +++++++++++++++++++++++-- 5 files changed, 119 insertions(+), 24 deletions(-) diff --git a/nova/tests/unit/virt/lxd/test_driver.py b/nova/tests/unit/virt/lxd/test_driver.py index 5f176366..1df3e66f 100644 --- a/nova/tests/unit/virt/lxd/test_driver.py +++ b/nova/tests/unit/virt/lxd/test_driver.py @@ -653,8 +653,8 @@ class LXDDriverTest(test.NoDBTestCase): lxd_driver.attach_interface(ctx, instance, image_meta, vif) - self.assertTrue('eth1' in profile.devices) - self.assertEqual(expected, profile.devices['eth1']) + self.assertTrue('qbr0123456789a' in profile.devices) + self.assertEqual(expected, profile.devices['qbr0123456789a']) profile.save.assert_called_once_with(wait=True) def test_detach_interface(self): diff --git a/nova/tests/unit/virt/lxd/test_vif.py b/nova/tests/unit/virt/lxd/test_vif.py index 5875a5fc..d6b9f53f 100644 --- a/nova/tests/unit/virt/lxd/test_vif.py +++ b/nova/tests/unit/virt/lxd/test_vif.py @@ -33,6 +33,11 @@ VIF = network_model.VIF( id='0123456789abcdef', address='ca:fe:de:ad:be:ef', network=NETWORK, type=network_model.VIF_TYPE_OVS, devname='tap-012-345-678', ovs_interfaceid='9abc-def-000') +TAP_VIF = network_model.VIF( + id='0123456789abcdef', address='ca:fe:de:ad:be:ee', + network=NETWORK, type=network_model.VIF_TYPE_TAP, + devname='tap-014-345-678', + details={'mac_address': 'aa:bb:cc:dd:ee:ff'}) INSTANCE = fake_instance.fake_instance_obj( context.get_admin_context(), name='test') @@ -140,7 +145,7 @@ class LXDGenericVifDriverTest(test.NoDBTestCase): self.vif_driver = vif.LXDGenericVifDriver() @mock.patch('nova.virt.lxd.vif.os_vif') - def test_plug(self, os_vif): + def test_plug_ovs(self, os_vif): self.vif_driver.plug(INSTANCE, VIF) self.assertEqual( @@ -149,10 +154,26 @@ class LXDGenericVifDriverTest(test.NoDBTestCase): 'instance-00000001', os_vif.plug.call_args[0][1].name) @mock.patch('nova.virt.lxd.vif.os_vif') - def test_unplug(self, os_vif): + def test_unplug_ovs(self, os_vif): self.vif_driver.unplug(INSTANCE, VIF) self.assertEqual( 'tap-012-345-678', os_vif.unplug.call_args[0][0].vif_name) self.assertEqual( 'instance-00000001', os_vif.unplug.call_args[0][1].name) + + @mock.patch('nova.virt.lxd.vif.os_vif') + def test_plug_tap(self, os_vif): + with mock.patch.object(vif, '_create_veth_pair') as create_veth_pair: + self.vif_driver.plug(INSTANCE, TAP_VIF) + os_vif.plug.assert_not_called() + create_veth_pair.assert_called_with('tap-014-345-678', + 'tin-014-345-678', + 1000) + + @mock.patch('nova.virt.lxd.vif.linux_net') + @mock.patch('nova.virt.lxd.vif.os_vif') + def test_unplug_tap(self, os_vif, linux_net): + self.vif_driver.unplug(INSTANCE, TAP_VIF) + os_vif.plug.assert_not_called() + linux_net.delete_net_dev.assert_called_with('tap-014-345-678') diff --git a/nova/virt/lxd/driver.py b/nova/virt/lxd/driver.py index 757e02d3..d7eac2ca 100644 --- a/nova/virt/lxd/driver.py +++ b/nova/virt/lxd/driver.py @@ -662,14 +662,12 @@ class LXDDriver(driver.ComputeDriver): profile = self.client.profiles.get(instance.name) - interfaces = [] - for key, val in profile.devices.items(): - if key.startswith('eth'): - interfaces.append(key) - net_device = 'eth{}'.format(len(interfaces)) - network_config = lxd_vif.get_config(vif) + + # XXX(jamespage): Refactor into vif module so code is shared + # across hotplug and instance creation. if 'bridge' in network_config: + net_device = network_config['bridge'] config_update = { net_device: { 'nictype': 'bridged', @@ -679,10 +677,12 @@ class LXDDriver(driver.ComputeDriver): } } else: + net_device = lxd_vif.get_vif_devname(vif) config_update = { net_device: { - 'nictype': 'p2p', + 'nictype': 'physical', 'hwaddr': vif['address'], + 'parent': lxd_vif.get_vif_internal_devname(vif), 'type': 'nic', } } @@ -691,10 +691,11 @@ class LXDDriver(driver.ComputeDriver): profile.save(wait=True) def detach_interface(self, context, instance, vif): - self.vif_driver.unplug(instance, vif) - profile = self.client.profiles.get(instance.name) to_remove = None + # XXX(jamespage): refactor to use actual key + # which switch to consistent + # device naming in the profile. for key, val in profile.devices.items(): if val.get('hwaddr') == vif['address']: to_remove = key @@ -702,6 +703,8 @@ class LXDDriver(driver.ComputeDriver): del profile.devices[to_remove] profile.save(wait=True) + self.vif_driver.unplug(instance, vif) + def migrate_disk_and_power_off( self, context, instance, dest, _flavor, network_info, block_device_info=None, timeout=0, retry_interval=0): diff --git a/nova/virt/lxd/flavor.py b/nova/virt/lxd/flavor.py index 90134b3d..a1b9af0e 100644 --- a/nova/virt/lxd/flavor.py +++ b/nova/virt/lxd/flavor.py @@ -155,24 +155,24 @@ def _network(instance, _, network_info, __): devices = {} for vifaddr in network_info: cfg = vif.get_config(vifaddr) + devname = vif.get_vif_devname(vifaddr) if 'bridge' in cfg: - key = str(cfg['bridge']) + key = bridge = str(cfg['bridge']) devices[key] = { 'nictype': 'bridged', 'hwaddr': str(cfg['mac_address']), - 'parent': str(cfg['bridge']), + 'parent': bridge, + 'host_name': devname, 'type': 'nic' } else: - key = 'unbridged' + key = devname devices[key] = { - 'nictype': 'p2p', + 'nictype': 'physical', 'hwaddr': str(cfg['mac_address']), + 'parent': vif.get_vif_internal_devname(vifaddr), 'type': 'nic' } - host_device = vif.get_vif_devname(vifaddr) - if host_device: - devices[key]['host_name'] = host_device specs = instance.flavor.extra_specs # Since LXD does not implement average NIC IO and number of burst diff --git a/nova/virt/lxd/vif.py b/nova/virt/lxd/vif.py index a10036e9..8092b2e8 100644 --- a/nova/virt/lxd/vif.py +++ b/nova/virt/lxd/vif.py @@ -11,13 +11,21 @@ # 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 oslo_concurrency import processutils +from oslo_log import log as logging + from nova import conf from nova import exception +from nova import utils +from nova.network import linux_net from nova.network import model as network_model from nova.network import os_vif_util import os_vif +LOG = logging.getLogger(__name__) + def get_vif_devname(vif): """Get device name for a given vif.""" @@ -26,6 +34,25 @@ def get_vif_devname(vif): return ("nic" + vif['id'])[:network_model.NIC_NAME_LEN] +def get_vif_internal_devname(vif): + """Get the internal device name for a given vif.""" + return get_vif_devname(vif).replace('tap', 'tin') + + +def _create_veth_pair(dev1_name, dev2_name, mtu=None): + """Create a pair of veth devices with the specified names, + deleting any previous devices with those names. + """ + for dev in [dev1_name, dev2_name]: + linux_net.delete_net_dev(dev) + + utils.execute('ip', 'link', 'add', dev1_name, 'type', 'veth', 'peer', + 'name', dev2_name, run_as_root=True) + for dev in [dev1_name, dev2_name]: + utils.execute('ip', 'link', 'set', dev, 'up', run_as_root=True) + linux_net._set_device_mtu(dev, mtu) + + def _get_bridge_config(vif): return { 'bridge': vif['network']['bridge'], @@ -73,13 +100,57 @@ class LXDGenericVifDriver(object): os_vif.initialize() def plug(self, instance, vif): + vif_type = vif['type'] instance_info = os_vif_util.nova_to_osvif_instance(instance) - vif_obj = os_vif_util.nova_to_osvif_vif(vif) - os_vif.plug(vif_obj, instance_info) + # Try os-vif codepath first + vif_obj = os_vif_util.nova_to_osvif_vif(vif) + if vif_obj is not None: + os_vif.plug(vif_obj, instance_info) + return + + # Legacy non-os-vif codepath + func = getattr(self, 'plug_%s' % vif_type, None) + if not func: + raise exception.InternalError( + "Unexpected vif_type=%s" % vif_type + ) + func(instance, vif) def unplug(self, instance, vif): + vif_type = vif['type'] instance_info = os_vif_util.nova_to_osvif_instance(instance) - vif_obj = os_vif_util.nova_to_osvif_vif(vif) - os_vif.unplug(vif_obj, instance_info) + # Try os-vif codepath first + vif_obj = os_vif_util.nova_to_osvif_vif(vif) + if vif_obj is not None: + os_vif.unplug(vif_obj, instance_info) + return + + # Legacy non-os-vif codepath + func = getattr(self, 'unplug_%s' % vif_type, None) + if not func: + raise exception.InternalError( + "Unexpected vif_type=%s" % vif_type + ) + func(instance, vif) + + def plug_tap(self, instance, vif): + """Plug a VIF_TYPE_TAP virtual interface.""" + dev1_name = get_vif_devname(vif) + dev2_name = dev1_name.replace('tap', 'tin') + network = vif.get('network') + mtu = network.get_meta('mtu') if network else None + # NOTE(jamespage): For nova-lxd this is really a veth pair + # so that a) security rules get applied on the host + # and b) that the container can still be wired. + _create_veth_pair(dev1_name, dev2_name, mtu) + + def unplug_tap(self, instance, vif): + """Unplug a VIF_TYPE_TAP virtual interface.""" + dev = get_vif_devname(vif) + try: + linux_net.delete_net_dev(dev) + except processutils.ProcessExecutionError: + LOG.exception("Failed while unplugging vif", + instance=instance)