From d5b119ba37d5c724d1279cb2292dce05c32b51a0 Mon Sep 17 00:00:00 2001 From: Rawlin Peters Date: Thu, 23 Jun 2016 10:34:25 -0600 Subject: [PATCH] Don't create extraneous linux bridge/veth pair for VIFOpenVSwitch VIFOpenVSwitch vifs will be booted by libvirt directly on the target bridge. Don't create an extraneous linux bridge with a veth pair connection to the target bridge. VIFOpenVSwitch inherits from VIFBridge, so the following check causes _plug_bridge() to be incorrectly executed for vifs of type VIFOpenVSwitch: if isinstance(vif, objects.vif.VIFBridge): self._plug_bridge(vif, instance_info) Fix this to check for instances of VIFOpenVSwitch first and do nothing in that case. Closes-Bug: #1595614 Change-Id: I447395cc50aa8fc21fa714b76f4cb1f5e79846ae --- .../fix-vif-openvswitch-fa0d19be9dd668e1.yaml | 6 +++++ vif_plug_ovs/ovs.py | 8 ++++-- vif_plug_ovs/tests/test_plugin.py | 27 ++++++++++++++++--- 3 files changed, 36 insertions(+), 5 deletions(-) create mode 100644 releasenotes/notes/fix-vif-openvswitch-fa0d19be9dd668e1.yaml diff --git a/releasenotes/notes/fix-vif-openvswitch-fa0d19be9dd668e1.yaml b/releasenotes/notes/fix-vif-openvswitch-fa0d19be9dd668e1.yaml new file mode 100644 index 00000000..f473e42e --- /dev/null +++ b/releasenotes/notes/fix-vif-openvswitch-fa0d19be9dd668e1.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - The ovs plugin now handles vifs of type VIFOpenVSwitch + properly. Before, it would improperly create an extraneous + linux bridge and veth pair attached to the target OVS bridge. + diff --git a/vif_plug_ovs/ovs.py b/vif_plug_ovs/ovs.py index a854607c..a12d81fc 100644 --- a/vif_plug_ovs/ovs.py +++ b/vif_plug_ovs/ovs.py @@ -123,7 +123,9 @@ class OvsPlugin(plugin.PluginBase): raise exception.WrongPortProfile( profile=vif.port_profile.__class__.__name__) - if isinstance(vif, objects.vif.VIFBridge): + if isinstance(vif, objects.vif.VIFOpenVSwitch): + pass # no special plugging required + elif isinstance(vif, objects.vif.VIFBridge): self._plug_bridge(vif, instance_info) elif isinstance(vif, objects.vif.VIFVHostUser): self._plug_vhostuser(vif, instance_info) @@ -155,7 +157,9 @@ class OvsPlugin(plugin.PluginBase): raise exception.WrongPortProfile( profile=vif.port_profile.__class__.__name__) - if isinstance(vif, objects.vif.VIFBridge): + if isinstance(vif, objects.vif.VIFOpenVSwitch): + pass # no special unplugging required + elif isinstance(vif, objects.vif.VIFBridge): self._unplug_bridge(vif, instance_info) elif isinstance(vif, objects.vif.VIFVHostUser): self._unplug_vhostuser(vif, instance_info) diff --git a/vif_plug_ovs/tests/test_plugin.py b/vif_plug_ovs/tests/test_plugin.py index 3f190e4c..55935553 100644 --- a/vif_plug_ovs/tests/test_plugin.py +++ b/vif_plug_ovs/tests/test_plugin.py @@ -60,7 +60,7 @@ class PluginTest(testtools.TestCase): self.profile_ovs = objects.vif.VIFPortProfileOpenVSwitch( interface_id='e65867e0-9340-4a7f-a256-09af6eb7a3aa') - self.vif_ovs = objects.vif.VIFBridge( + self.vif_ovs_hybrid = objects.vif.VIFBridge( id='b679325f-ca89-4ee0-a8be-6db1409b69ea', address='ca:fe:de:ad:be:ef', network=self.network_ovs, @@ -68,6 +68,13 @@ class PluginTest(testtools.TestCase): bridge_name="qbrvif-xxx-yyy", port_profile=self.profile_ovs) + self.vif_ovs = objects.vif.VIFOpenVSwitch( + id='b679325f-ca89-4ee0-a8be-6db1409b69ea', + address='ca:fe:de:ad:be:ef', + network=self.network_ovs, + dev_name='tap-xxx-yyy-zzz', + port_profile=self.profile_ovs) + self.vif_vhostuser = objects.vif.VIFVHostUser( id='b679325f-ca89-4ee0-a8be-6db1409b69ea', address='ca:fe:de:ad:be:ef', @@ -80,6 +87,13 @@ class PluginTest(testtools.TestCase): name='demo', uuid='f0000000-0000-0000-0000-000000000001') + def test_plug_ovs(self): + plug_bridge_mock = mock.Mock() + plugin = ovs.OvsPlugin.load("ovs") + plugin._plug_bridge = plug_bridge_mock + plugin.plug(self.vif_ovs, self.instance) + self.assertFalse(plug_bridge_mock.called) + def test_plug_ovs_bridge(self): calls = { 'device_exists': [mock.call('qvob679325f-ca')], @@ -108,13 +122,20 @@ class PluginTest(testtools.TestCase): ) as (ensure_bridge, device_exists, create_veth_pair, add_bridge_port, create_ovs_vif_port): plugin = ovs.OvsPlugin.load("ovs") - plugin.plug(self.vif_ovs, self.instance) + plugin.plug(self.vif_ovs_hybrid, self.instance) ensure_bridge.assert_has_calls(calls['ensure_bridge']) device_exists.assert_has_calls(calls['device_exists']) create_veth_pair.assert_has_calls(calls['create_veth_pair']) add_bridge_port.assert_has_calls(calls['add_bridge_port']) create_ovs_vif_port.assert_has_calls(calls['create_ovs_vif_port']) + def test_unplug_ovs(self): + unplug_bridge_mock = mock.Mock() + plugin = ovs.OvsPlugin.load("ovs") + plugin._unplug_bridge = unplug_bridge_mock + plugin.unplug(self.vif_ovs, self.instance) + self.assertFalse(unplug_bridge_mock.called) + def test_unplug_ovs_bridge(self): calls = { 'delete_bridge': [mock.call('qbrvif-xxx-yyy', 'qvbb679325f-ca')], @@ -126,7 +147,7 @@ class PluginTest(testtools.TestCase): mock.patch.object(linux_net, 'delete_ovs_vif_port') ) as (delete_bridge, delete_ovs_vif_port): plugin = ovs.OvsPlugin.load("ovs") - plugin.unplug(self.vif_ovs, self.instance) + plugin.unplug(self.vif_ovs_hybrid, self.instance) delete_bridge.assert_has_calls(calls['delete_bridge']) delete_ovs_vif_port.assert_has_calls(calls['delete_ovs_vif_port'])