From 78b6c14f2cf375493f3fc268d589f2ba23f4f346 Mon Sep 17 00:00:00 2001 From: James Page Date: Wed, 14 Jun 2017 15:57:47 +0100 Subject: [PATCH] vif: redux interface wiring approach The nova-lxd driver has to take a slightly different approach to virtual interface wiring due to a lack of an equivalent to 'launch and pause' in LXD. For some interface types, the last mile tap device needs to be present for vif plugging to complete successfully which occurs prior to the instance being launched; This change refactors the vif module to create veth pairs directly in nova-lxd, rather than delegating this to LXD as part of a bridged network interface type. This allows vif plugging to complete prior to the instance being created in LXD. The side effect of this change is that all currently supported interface types are now configured as 'physical' interfaces in LXD profiles for instances - wiring to bridges is handled directly by the nova-lxd driver instead. This change has been validated with: ovs driver + iptables hybrid firewall driver ovs driver + openvswitch native firewall driver linuxbridge driver + iptables hybrid firewall driver The VIF wiring approach is described in detail in the VIF wiring documentation included in this change. Closes-Bug: 1681758 Change-Id: Ic268e989d1ee19f696298fb1e0db729a00352a12 --- doc/source/index.rst | 1 + doc/source/vif_wiring.rst | 59 ++++++ nova/tests/unit/virt/lxd/test_driver.py | 54 ++++- nova/tests/unit/virt/lxd/test_flavor.py | 26 +-- nova/tests/unit/virt/lxd/test_vif.py | 252 +++++++++++++++++++----- nova/virt/lxd/driver.py | 59 +++--- nova/virt/lxd/flavor.py | 24 +-- nova/virt/lxd/vif.py | 153 ++++++++++++-- 8 files changed, 486 insertions(+), 142 deletions(-) create mode 100644 doc/source/vif_wiring.rst diff --git a/doc/source/index.rst b/doc/source/index.rst index 1bc0b32e..cc0b1c26 100644 --- a/doc/source/index.rst +++ b/doc/source/index.rst @@ -14,6 +14,7 @@ Contents: usage contributing exclusive_machine + vif_wiring Indices and tables ================== diff --git a/doc/source/vif_wiring.rst b/doc/source/vif_wiring.rst new file mode 100644 index 00000000..3a783fd6 --- /dev/null +++ b/doc/source/vif_wiring.rst @@ -0,0 +1,59 @@ +Nova-LXD VIF Design Notes +========================= + +VIF plugging workflow +--------------------- + +Nova-LXD makes use of the os-vif interface plugging library to wire LXD +instances into underlying Neutron networking; however there are some +subtle differences between the Nova-Libvirt driver and the Nova-LXD driver +in terms of how the last mile wiring is done to the instances. + +In the Nova-Libvirt driver, Libvirt is used to start the instance in a +paused state, which creates the required tap device and any required wiring +to bridges created in previous os-vif plugging events. + +The concept of 'start-and-pause' does not exist in LXD, so the driver +creates a veth pair instead, allowing the last mile wiring to be created +in advance of the actual LXD container being created. + +This allows Neutron to complete the underlying VIF plugging at which +point it will notify Nova and the Nova-LXD driver will create the LXD +container and wire the pre-created veth pair into its profile. + +tap/tin veth pairs +------------------ + +The veth pair created to wire the LXD instance into the underlying Neutron +networking uses the tap and tin prefixes; the tap named device is present +on the host OS, allowing iptables based firewall rules to be applied as +they are for other virt drivers, and the tin named device is passed to +LXD as part of the container profile. LXD will rename this device +internally within the container to an ethNN style name. + +The LXD profile devices for network interfaces are created as 'physical' +rather than 'bridged' network devices as the driver handles creation of +the veth pair, rather than LXD (as would happen with a bridged device). + +LXD profile interface naming +---------------------------- + +The name of the interfaces in each containers LXD profile maps to the +devname provided by Neutron as part of VIF plugging - this will typically +be of the format tapXXXXXXX. This allows for easier identification of +the interface during detachment events later in instance lifecycle. + +Prior versions of the nova-lxd driver did not take this approach; interface +naming was not consistent depending on when the interface was attached. The +legacy code used to detach interfaces based on MAC address is used as a +fallback in the event that the new style device name is not found, supporting +upgraders from previous versions of the driver. + +Supported Interface Types +------------------------- + +The Nova-LXD driver has been validated with: + + - OpenvSwitch (ovs) hybrid bridge ports. + - OpenvSwitch (ovs) standard ports. + - Linuxbridge (bridge) ports diff --git a/nova/tests/unit/virt/lxd/test_driver.py b/nova/tests/unit/virt/lxd/test_driver.py index 1df3e66f..ed45e157 100644 --- a/nova/tests/unit/virt/lxd/test_driver.py +++ b/nova/tests/unit/virt/lxd/test_driver.py @@ -616,8 +616,8 @@ class LXDDriverTest(test.NoDBTestCase): def test_attach_interface(self): expected = { 'hwaddr': '00:11:22:33:44:55', - 'parent': 'qbr0123456789a', - 'nictype': 'bridged', + 'parent': 'tin0123456789a', + 'nictype': 'physical', 'type': 'nic', } @@ -645,7 +645,8 @@ class LXDDriverTest(test.NoDBTestCase): 'type': network_model.VIF_TYPE_OVS, 'address': '00:11:22:33:44:55', 'network': { - 'bridge': 'fakebr'}} + 'bridge': 'fakebr'}, + 'devname': 'tap0123456789a'} lxd_driver = driver.LXDDriver(None) lxd_driver.init_host(None) @@ -653,17 +654,52 @@ class LXDDriverTest(test.NoDBTestCase): lxd_driver.attach_interface(ctx, instance, image_meta, vif) - self.assertTrue('qbr0123456789a' in profile.devices) - self.assertEqual(expected, profile.devices['qbr0123456789a']) + self.assertTrue('tap0123456789a' in profile.devices) + self.assertEqual(expected, profile.devices['tap0123456789a']) + profile.save.assert_called_once_with(wait=True) + + def test_detach_interface_legacy(self): + profile = mock.Mock() + profile.devices = { + 'eth0': { + 'nictype': 'bridged', + 'parent': 'lxdbr0', + 'hwaddr': '00:11:22:33:44:55', + 'type': 'nic' + }, + 'root': { + 'path': '/', + 'type': 'disk' + }, + } + self.client.profiles.get.return_value = profile + + ctx = context.get_admin_context() + instance = fake_instance.fake_instance_obj( + ctx, name='test', memory_mb=0) + vif = { + 'id': '0123456789abcdef', + 'type': network_model.VIF_TYPE_OVS, + 'address': '00:11:22:33:44:55', + 'network': { + 'bridge': 'fakebr'}} + + lxd_driver = driver.LXDDriver(None) + lxd_driver.init_host(None) + + lxd_driver.detach_interface(ctx, instance, vif) + + self.vif_driver.unplug.assert_called_once_with( + instance, vif) + self.assertEqual(['root'], sorted(profile.devices.keys())) profile.save.assert_called_once_with(wait=True) def test_detach_interface(self): profile = mock.Mock() profile.devices = { - 'eth0': { - 'name': 'eth0', - 'nictype': 'bridged', - 'parent': 'lxdbr0', + 'tap0123456789a': { + 'nictype': 'physical', + 'parent': 'tin0123456789a', 'hwaddr': '00:11:22:33:44:55', 'type': 'nic' }, diff --git a/nova/tests/unit/virt/lxd/test_flavor.py b/nova/tests/unit/virt/lxd/test_flavor.py index 5494d5c7..bfb0c1cd 100644 --- a/nova/tests/unit/virt/lxd/test_flavor.py +++ b/nova/tests/unit/virt/lxd/test_flavor.py @@ -358,7 +358,8 @@ class ToProfileTest(test.NoDBTestCase): self.client.profiles.create.assert_called_once_with( instance.name, expected_config, expected_devices) - def test_to_profile_network_config_average(self): + @mock.patch('nova.virt.lxd.vif._is_no_op_firewall', return_value=False) + def test_to_profile_network_config_average(self, _is_no_op_firewall): ctx = context.get_admin_context() instance = fake_instance.fake_instance_obj( ctx, name='test', memory_mb=0) @@ -371,7 +372,8 @@ class ToProfileTest(test.NoDBTestCase): 'type': network_model.VIF_TYPE_OVS, 'address': '00:11:22:33:44:55', 'network': { - 'bridge': 'fakebr'}}] + 'bridge': 'fakebr'}, + 'devname': 'tap0123456789a'}] block_info = [] expected_config = { @@ -383,11 +385,10 @@ class ToProfileTest(test.NoDBTestCase): instance.name)), } expected_devices = { - 'qbr0123456789a': { - 'host_name': 'nic0123456789a', + 'tap0123456789a': { 'hwaddr': '00:11:22:33:44:55', - 'nictype': 'bridged', - 'parent': 'qbr0123456789a', + 'nictype': 'physical', + 'parent': 'tin0123456789a', 'type': 'nic', 'limits.egress': '16000Mbit', 'limits.ingress': '8000Mbit', @@ -404,7 +405,8 @@ class ToProfileTest(test.NoDBTestCase): self.client.profiles.create.assert_called_once_with( instance.name, expected_config, expected_devices) - def test_to_profile_network_config_peak(self): + @mock.patch('nova.virt.lxd.vif._is_no_op_firewall', return_value=False) + def test_to_profile_network_config_peak(self, _is_no_op_firewall): ctx = context.get_admin_context() instance = fake_instance.fake_instance_obj( ctx, name='test', memory_mb=0) @@ -417,7 +419,8 @@ class ToProfileTest(test.NoDBTestCase): 'type': network_model.VIF_TYPE_OVS, 'address': '00:11:22:33:44:55', 'network': { - 'bridge': 'fakebr'}}] + 'bridge': 'fakebr'}, + 'devname': 'tap0123456789a'}] block_info = [] expected_config = { @@ -429,11 +432,10 @@ class ToProfileTest(test.NoDBTestCase): instance.name)), } expected_devices = { - 'qbr0123456789a': { - 'host_name': 'nic0123456789a', + 'tap0123456789a': { 'hwaddr': '00:11:22:33:44:55', - 'nictype': 'bridged', - 'parent': 'qbr0123456789a', + 'nictype': 'physical', + 'parent': 'tin0123456789a', 'type': 'nic', 'limits.egress': '32000Mbit', 'limits.ingress': '24000Mbit', diff --git a/nova/tests/unit/virt/lxd/test_vif.py b/nova/tests/unit/virt/lxd/test_vif.py index d6b9f53f..6d415699 100644 --- a/nova/tests/unit/virt/lxd/test_vif.py +++ b/nova/tests/unit/virt/lxd/test_vif.py @@ -27,17 +27,30 @@ SUBNET = network_model.Subnet( cidr='101.168.1.0/24', dns=[DNS_BRIDGE], gateway=GATEWAY, routes=None, dhcp_server='191.168.1.1') NETWORK = network_model.Network( - id='network-id-xxx-yyy-zzz', bridge='br0', label=None, + id='ab7b876b-2c1c-4bb2-afa1-f9f4b6a28053', bridge='br0', label=None, subnets=[SUBNET], bridge_interface=None, vlan=99, mtu=1000) -VIF = network_model.VIF( - id='0123456789abcdef', address='ca:fe:de:ad:be:ef', +OVS_VIF = network_model.VIF( + id='da5cc4bf-f16c-4807-a0b6-911c7c67c3f8', 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') + devname='tapda5cc4bf-f1', + ovs_interfaceid='7b6812a6-b044-4596-b3c5-43a8ec431638', + details={network_model.VIF_DETAILS_OVS_HYBRID_PLUG: False}) +OVS_HYBRID_VIF = network_model.VIF( + id='da5cc4bf-f16c-4807-a0b6-911c7c67c3f8', address='ca:fe:de:ad:be:ef', + network=NETWORK, type=network_model.VIF_TYPE_OVS, + devname='tapda5cc4bf-f1', + ovs_interfaceid='7b6812a6-b044-4596-b3c5-43a8ec431638', + details={network_model.VIF_DETAILS_OVS_HYBRID_PLUG: True}) TAP_VIF = network_model.VIF( - id='0123456789abcdef', address='ca:fe:de:ad:be:ee', + id='da5cc4bf-f16c-4807-a0b6-911c7c67c3f8', address='ca:fe:de:ad:be:ee', network=NETWORK, type=network_model.VIF_TYPE_TAP, - devname='tap-014-345-678', + devname='tapda5cc4bf-f1', details={'mac_address': 'aa:bb:cc:dd:ee:ff'}) +LB_VIF = network_model.VIF( + id='da5cc4bf-f16c-4807-a0b6-911c7c67c3f8', address='ca:fe:de:ad:be:ed', + network=NETWORK, type=network_model.VIF_TYPE_BRIDGE, + devname='tapda5cc4bf-f1') + INSTANCE = fake_instance.fake_instance_obj( context.get_admin_context(), name='test') @@ -47,7 +60,7 @@ class GetVifDevnameTest(test.NoDBTestCase): def test_get_vif_devname_devname_exists(self): an_vif = { - 'id': '0123456789abcdef', + 'id': 'da5cc4bf-f16c-4807-a0b6-911c7c67c3f8', 'devname': 'oth1', } @@ -57,12 +70,12 @@ class GetVifDevnameTest(test.NoDBTestCase): def test_get_vif_devname_devname_nonexistent(self): an_vif = { - 'id': '0123456789abcdef', + 'id': 'da5cc4bf-f16c-4807-a0b6-911c7c67c3f8', } devname = vif.get_vif_devname(an_vif) - self.assertEqual('nic0123456789a', devname) + self.assertEqual('nicda5cc4bf-f1', devname) class GetConfigTest(test.NoDBTestCase): @@ -70,7 +83,7 @@ class GetConfigTest(test.NoDBTestCase): def setUp(self): super(GetConfigTest, self).setUp() - self.CONF_patcher = mock.patch('nova.virt.lxd.vif.conf.CONF') + self.CONF_patcher = mock.patch('nova.virt.lxd.vif.CONF') self.CONF = self.CONF_patcher.start() self.CONF.firewall_driver = 'nova.virt.firewall.NoopFirewallDriver' @@ -81,9 +94,11 @@ class GetConfigTest(test.NoDBTestCase): def test_get_config_bad_vif_type(self): """Unsupported vif types raise an exception.""" an_vif = network_model.VIF( - id='0123456789abcdef', address='ca:fe:de:ad:be:ef', + id='da5cc4bf-f16c-4807-a0b6-911c7c67c3f8', + address='ca:fe:de:ad:be:ef', network=NETWORK, type='invalid', - devname='tap-012-345-678', ovs_interfaceid='9abc-def-000') + devname='tapda5cc4bf-f1', + ovs_interfaceid='7b6812a6-b044-4596-b3c5-43a8ec431638') self.assertRaises( exception.NovaException, vif.get_config, an_vif) @@ -91,9 +106,11 @@ class GetConfigTest(test.NoDBTestCase): def test_get_config_bridge(self): expected = {'bridge': 'br0', 'mac_address': 'ca:fe:de:ad:be:ef'} an_vif = network_model.VIF( - id='0123456789abcdef', address='ca:fe:de:ad:be:ef', + id='da5cc4bf-f16c-4807-a0b6-911c7c67c3f8', + address='ca:fe:de:ad:be:ef', network=NETWORK, type='bridge', - devname='tap-012-345-678', ovs_interfaceid='9abc-def-000') + devname='tapda5cc4bf-f1', + ovs_interfaceid='7b6812a6-b044-4596-b3c5-43a8ec431638') config = vif.get_config(an_vif) @@ -103,9 +120,11 @@ class GetConfigTest(test.NoDBTestCase): expected = { 'bridge': 'br0', 'mac_address': 'ca:fe:de:ad:be:ef'} an_vif = network_model.VIF( - id='0123456789abcdef', address='ca:fe:de:ad:be:ef', + id='da5cc4bf-f16c-4807-a0b6-911c7c67c3f8', + address='ca:fe:de:ad:be:ef', network=NETWORK, type='ovs', - devname='tap-012-345-678', ovs_interfaceid='9abc-def-000') + devname='tapda5cc4bf-f1', + ovs_interfaceid='7b6812a6-b044-4596-b3c5-43a8ec431638') config = vif.get_config(an_vif) @@ -115,11 +134,13 @@ class GetConfigTest(test.NoDBTestCase): self.CONF.firewall_driver = 'AnFirewallDriver' expected = { - 'bridge': 'qbr0123456789a', 'mac_address': 'ca:fe:de:ad:be:ef'} + 'bridge': 'qbrda5cc4bf-f1', 'mac_address': 'ca:fe:de:ad:be:ef'} an_vif = network_model.VIF( - id='0123456789abcdef', address='ca:fe:de:ad:be:ef', + id='da5cc4bf-f16c-4807-a0b6-911c7c67c3f8', + address='ca:fe:de:ad:be:ef', network=NETWORK, type='ovs', - devname='tap-012-345-678', ovs_interfaceid='9abc-def-000') + devname='tapda5cc4bf-f1', + ovs_interfaceid='7b6812a6-b044-4596-b3c5-43a8ec431638') config = vif.get_config(an_vif) @@ -128,9 +149,11 @@ class GetConfigTest(test.NoDBTestCase): def test_get_config_tap(self): expected = {'mac_address': 'ca:fe:de:ad:be:ef'} an_vif = network_model.VIF( - id='0123456789abcdef', address='ca:fe:de:ad:be:ef', + id='da5cc4bf-f16c-4807-a0b6-911c7c67c3f8', + address='ca:fe:de:ad:be:ef', network=NETWORK, type='tap', - devname='tap-012-345-678', ovs_interfaceid='9abc-def-000') + devname='tapda5cc4bf-f1', + ovs_interfaceid='7b6812a6-b044-4596-b3c5-43a8ec431638') config = vif.get_config(an_vif) @@ -144,36 +167,165 @@ class LXDGenericVifDriverTest(test.NoDBTestCase): super(LXDGenericVifDriverTest, self).setUp() self.vif_driver = vif.LXDGenericVifDriver() - @mock.patch('nova.virt.lxd.vif.os_vif') - def test_plug_ovs(self, os_vif): - self.vif_driver.plug(INSTANCE, VIF) - - self.assertEqual( - 'tap-012-345-678', os_vif.plug.call_args[0][0].vif_name) - self.assertEqual( - 'instance-00000001', os_vif.plug.call_args[0][1].name) - - @mock.patch('nova.virt.lxd.vif.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.object(vif, '_post_plug_wiring') @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): + def test_plug_ovs(self, os_vif, linux_net, _post_plug_wiring): + self.vif_driver.plug(INSTANCE, OVS_VIF) + + self.assertEqual( + 'tapda5cc4bf-f1', os_vif.plug.call_args[0][0].vif_name) + self.assertEqual( + 'instance-00000001', os_vif.plug.call_args[0][1].name) + _post_plug_wiring.assert_called_with(INSTANCE, OVS_VIF) + + @mock.patch.object(vif, '_post_unplug_wiring') + @mock.patch('nova.virt.lxd.vif.linux_net') + @mock.patch('nova.virt.lxd.vif.os_vif') + def test_unplug_ovs(self, os_vif, linux_net, _post_unplug_wiring): + self.vif_driver.unplug(INSTANCE, OVS_VIF) + + self.assertEqual( + 'tapda5cc4bf-f1', os_vif.unplug.call_args[0][0].vif_name) + self.assertEqual( + 'instance-00000001', os_vif.unplug.call_args[0][1].name) + _post_unplug_wiring.assert_called_with(INSTANCE, OVS_VIF) + + @mock.patch.object(vif, '_post_plug_wiring') + @mock.patch.object(vif, '_create_veth_pair') + @mock.patch('nova.virt.lxd.vif.os_vif') + def test_plug_tap(self, os_vif, _create_veth_pair, _post_plug_wiring): + self.vif_driver.plug(INSTANCE, TAP_VIF) + os_vif.plug.assert_not_called() + _create_veth_pair.assert_called_with('tapda5cc4bf-f1', + 'tinda5cc4bf-f1', + 1000) + _post_plug_wiring.assert_called_with(INSTANCE, TAP_VIF) + + @mock.patch.object(vif, '_post_unplug_wiring') + @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, _post_unplug_wiring): 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') + linux_net.delete_net_dev.assert_called_with('tapda5cc4bf-f1') + _post_unplug_wiring.assert_called_with(INSTANCE, TAP_VIF) + + +class PostPlugTest(test.NoDBTestCase): + """Tests for post plug operations""" + + def setUp(self): + super(PostPlugTest, self).setUp() + + @mock.patch('nova.virt.lxd.vif._create_veth_pair') + @mock.patch('nova.virt.lxd.vif._add_bridge_port') + @mock.patch('nova.virt.lxd.vif.linux_net') + def test_post_plug_ovs_hybrid(self, + linux_net, + add_bridge_port, + create_veth_pair): + linux_net.device_exists.return_value = False + + vif._post_plug_wiring(INSTANCE, OVS_HYBRID_VIF) + + linux_net.device_exists.assert_called_with('tapda5cc4bf-f1') + create_veth_pair.assert_called_with('tapda5cc4bf-f1', + 'tinda5cc4bf-f1', + 1000) + add_bridge_port.assert_called_with('qbrda5cc4bf-f1', + 'tapda5cc4bf-f1') + + @mock.patch('nova.virt.lxd.vif._create_veth_pair') + @mock.patch('nova.virt.lxd.vif._add_bridge_port') + @mock.patch('nova.virt.lxd.vif.linux_net') + def test_post_plug_ovs(self, + linux_net, + add_bridge_port, + create_veth_pair): + + linux_net.device_exists.return_value = False + + vif._post_plug_wiring(INSTANCE, OVS_VIF) + + linux_net.device_exists.assert_called_with('tapda5cc4bf-f1') + create_veth_pair.assert_called_with('tapda5cc4bf-f1', + 'tinda5cc4bf-f1', + 1000) + add_bridge_port.assert_not_called() + linux_net.create_ovs_vif_port.assert_called_with( + 'br0', + 'tapda5cc4bf-f1', + 'da5cc4bf-f16c-4807-a0b6-911c7c67c3f8', + 'ca:fe:de:ad:be:ef', + INSTANCE.uuid, + 1000 + ) + + @mock.patch('nova.virt.lxd.vif._create_veth_pair') + @mock.patch('nova.virt.lxd.vif._add_bridge_port') + @mock.patch('nova.virt.lxd.vif.linux_net') + def test_post_plug_bridge(self, + linux_net, + add_bridge_port, + create_veth_pair): + linux_net.device_exists.return_value = False + + vif._post_plug_wiring(INSTANCE, LB_VIF) + + linux_net.device_exists.assert_called_with('tapda5cc4bf-f1') + create_veth_pair.assert_called_with('tapda5cc4bf-f1', + 'tinda5cc4bf-f1', + 1000) + add_bridge_port.assert_called_with('br0', + 'tapda5cc4bf-f1') + + @mock.patch('nova.virt.lxd.vif._create_veth_pair') + @mock.patch('nova.virt.lxd.vif._add_bridge_port') + @mock.patch('nova.virt.lxd.vif.linux_net') + def test_post_plug_tap(self, + linux_net, + add_bridge_port, + create_veth_pair): + linux_net.device_exists.return_value = False + + vif._post_plug_wiring(INSTANCE, TAP_VIF) + + linux_net.device_exists.assert_not_called() + + +class PostUnplugTest(test.NoDBTestCase): + """Tests for post unplug operations""" + + @mock.patch('nova.virt.lxd.vif.linux_net') + def test_post_unplug_ovs_hybrid(self, linux_net): + vif._post_unplug_wiring(INSTANCE, OVS_HYBRID_VIF) + linux_net.delete_net_dev.assert_called_with('tapda5cc4bf-f1') + + @mock.patch('nova.virt.lxd.vif.linux_net') + def test_post_unplug_ovs(self, linux_net): + vif._post_unplug_wiring(INSTANCE, OVS_VIF) + linux_net.delete_ovs_vif_port.assert_called_with('br0', + 'tapda5cc4bf-f1', + True) + + @mock.patch('nova.virt.lxd.vif.linux_net') + def test_post_unplug_bridge(self, linux_net): + vif._post_unplug_wiring(INSTANCE, LB_VIF) + linux_net.delete_net_dev.assert_called_with('tapda5cc4bf-f1') + + +class MiscHelpersTest(test.NoDBTestCase): + """Misc tests for vif module""" + + def test_is_ovs_vif_port(self): + self.assertTrue(vif._is_ovs_vif_port(OVS_VIF)) + self.assertFalse(vif._is_ovs_vif_port(OVS_HYBRID_VIF)) + self.assertFalse(vif._is_ovs_vif_port(TAP_VIF)) + + @mock.patch.object(vif, 'utils') + def test_add_bridge_port(self, utils): + vif._add_bridge_port('br-int', 'tapXYZ') + utils.execute.assert_called_with('brctl', 'addif', + 'br-int', 'tapXYZ', + run_as_root=True) diff --git a/nova/virt/lxd/driver.py b/nova/virt/lxd/driver.py index d7eac2ca..d750076a 100644 --- a/nova/virt/lxd/driver.py +++ b/nova/virt/lxd/driver.py @@ -662,46 +662,39 @@ class LXDDriver(driver.ComputeDriver): profile = self.client.profiles.get(instance.name) - 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', - 'hwaddr': vif['address'], - 'parent': network_config['bridge'], - 'type': 'nic', - } - } - else: - net_device = lxd_vif.get_vif_devname(vif) - config_update = { - net_device: { - 'nictype': 'physical', - 'hwaddr': vif['address'], - 'parent': lxd_vif.get_vif_internal_devname(vif), - 'type': 'nic', - } + net_device = lxd_vif.get_vif_devname(vif) + config_update = { + net_device: { + 'nictype': 'physical', + 'hwaddr': vif['address'], + 'parent': lxd_vif.get_vif_internal_devname(vif), + 'type': 'nic', } + } profile.devices.update(config_update) profile.save(wait=True) def detach_interface(self, context, 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 - break - del profile.devices[to_remove] - profile.save(wait=True) + devname = lxd_vif.get_vif_devname(vif) + + # NOTE(jamespage): Attempt to remove device using + # new style tap naming + if devname in profile.devices: + del profile.devices[devname] + profile.save(wait=True) + else: + # NOTE(jamespage): For upgrades, scan devices + # and attempt to identify + # using mac address as the + # device will *not* have a + # consistent name + for key, val in profile.devices.items(): + if val.get('hwaddr') == vif['address']: + del profile.devices[key] + profile.save(wait=True) + break self.vif_driver.unplug(instance, vif) diff --git a/nova/virt/lxd/flavor.py b/nova/virt/lxd/flavor.py index a1b9af0e..f94a44be 100644 --- a/nova/virt/lxd/flavor.py +++ b/nova/virt/lxd/flavor.py @@ -156,23 +156,13 @@ def _network(instance, _, network_info, __): for vifaddr in network_info: cfg = vif.get_config(vifaddr) devname = vif.get_vif_devname(vifaddr) - if 'bridge' in cfg: - key = bridge = str(cfg['bridge']) - devices[key] = { - 'nictype': 'bridged', - 'hwaddr': str(cfg['mac_address']), - 'parent': bridge, - 'host_name': devname, - 'type': 'nic' - } - else: - key = devname - devices[key] = { - 'nictype': 'physical', - 'hwaddr': str(cfg['mac_address']), - 'parent': vif.get_vif_internal_devname(vifaddr), - 'type': 'nic' - } + key = devname + devices[key] = { + 'nictype': 'physical', + 'hwaddr': str(cfg['mac_address']), + 'parent': vif.get_vif_internal_devname(vifaddr), + 'type': 'nic' + } 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 8092b2e8..445c4737 100644 --- a/nova/virt/lxd/vif.py +++ b/nova/virt/lxd/vif.py @@ -24,6 +24,9 @@ from nova.network import os_vif_util import os_vif + +CONF = conf.CONF + LOG = logging.getLogger(__name__) @@ -48,11 +51,24 @@ def _create_veth_pair(dev1_name, dev2_name, mtu=None): 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 _add_bridge_port(bridge, dev): + utils.execute('brctl', 'addif', bridge, dev, run_as_root=True) + + +def _is_no_op_firewall(): + return CONF.firewall_driver == "nova.virt.firewall.NoopFirewallDriver" + + +def _is_ovs_vif_port(vif): + return vif['type'] == 'ovs' and not vif.is_hybrid_plug_enabled() + + def _get_bridge_config(vif): return { 'bridge': vif['network']['bridge'], @@ -60,8 +76,7 @@ def _get_bridge_config(vif): def _get_ovs_config(vif): - if (conf.CONF.firewall_driver != 'nova.virt.firewall.NoopFirewallDriver' or - vif.is_hybrid_plug_enabled()): + if not _is_no_op_firewall() or vif.is_hybrid_plug_enabled(): return { 'bridge': ('qbr{}'.format(vif['id']))[:network_model.NIC_NAME_LEN], 'mac_address': vif['address']} @@ -93,6 +108,97 @@ def get_config(vif): 'Unsupported vif type: {}'.format(vif_type)) +# VIF_TYPE_OVS = 'ovs' +# VIF_TYPE_BRIDGE = 'bridge' +def _post_plug_wiring_veth_and_bridge(instance, vif): + config = get_config(vif) + network = vif.get('network') + mtu = network.get_meta('mtu') if network else None + v1_name = get_vif_devname(vif) + v2_name = get_vif_internal_devname(vif) + if not linux_net.device_exists(v1_name): + _create_veth_pair(v1_name, v2_name, mtu) + if _is_ovs_vif_port(vif): + # NOTE(jamespage): wire tap device directly to ovs bridge + linux_net.create_ovs_vif_port(vif['network']['bridge'], + v1_name, + vif['id'], + vif['address'], + instance.uuid, + mtu) + else: + # NOTE(jamespage): wire tap device linux bridge + _add_bridge_port(config['bridge'], v1_name) + else: + linux_net._set_device_mtu(v1_name, mtu) + + +POST_PLUG_WIRING = { + 'bridge': _post_plug_wiring_veth_and_bridge, + 'ovs': _post_plug_wiring_veth_and_bridge, +} + + +def _post_plug_wiring(instance, vif): + """Perform nova-lxd specific post os-vif plug processing + + :param vif: a nova.network.model.VIF instance + + Perform any post os-vif plug wiring requires to network + the instance LXD container with the underlying Neutron + network infrastructure + """ + + LOG.debug("Performing post plug wiring for VIF %s", vif) + vif_type = vif['type'] + + try: + POST_PLUG_WIRING[vif_type](instance, vif) + except KeyError: + LOG.debug("No post plug wiring step " + "for vif type: {}".format(vif_type)) + + +# VIF_TYPE_OVS = 'ovs' +# VIF_TYPE_BRIDGE = 'bridge' +def _post_unplug_wiring_delete_veth(instance, vif): + v1_name = get_vif_devname(vif) + try: + if _is_ovs_vif_port(vif): + linux_net.delete_ovs_vif_port(vif['network']['bridge'], + v1_name, True) + else: + linux_net.delete_net_dev(v1_name) + except processutils.ProcessExecutionError: + LOG.exception("Failed to delete veth for vif", + vif=vif) + + +POST_UNPLUG_WIRING = { + 'bridge': _post_unplug_wiring_delete_veth, + 'ovs': _post_unplug_wiring_delete_veth, +} + + +def _post_unplug_wiring(instance, vif): + """Perform nova-lxd specific post os-vif unplug processing + + :param vif: a nova.network.model.VIF instance + + Perform any post os-vif unplug wiring requires to remove + network interfaces assocaited with a lxd container. + """ + + LOG.debug("Performing post unplug wiring for VIF %s", vif) + vif_type = vif['type'] + + try: + POST_UNPLUG_WIRING[vif_type](instance, vif) + except KeyError: + LOG.debug("No post unplug wiring step " + "for vif type: {}".format(vif_type)) + + class LXDGenericVifDriver(object): """Generic VIF driver for LXD networking.""" @@ -107,15 +213,16 @@ class LXDGenericVifDriver(object): vif_obj = os_vif_util.nova_to_osvif_vif(vif) if vif_obj is not None: os_vif.plug(vif_obj, instance_info) - return + else: + # 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) - # 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) + _post_plug_wiring(instance, vif) def unplug(self, instance, vif): vif_type = vif['type'] @@ -125,26 +232,30 @@ class LXDGenericVifDriver(object): vif_obj = os_vif_util.nova_to_osvif_vif(vif) if vif_obj is not None: os_vif.unplug(vif_obj, instance_info) - return + else: + # 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) - # 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) + _post_unplug_wiring(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') + v1_name = get_vif_devname(vif) + v2_name = get_vif_internal_devname(vif) 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) + if not linux_net.device_exists(v1_name): + _create_veth_pair(v1_name, v2_name, mtu) + else: + linux_net._set_device_mtu(v1_name, mtu) def unplug_tap(self, instance, vif): """Unplug a VIF_TYPE_TAP virtual interface."""