From 3d62d8e23c99bdf4fcfaab15c6dd2e341d5c9bfa Mon Sep 17 00:00:00 2001 From: Rawlin Peters Date: Thu, 16 Jun 2016 17:07:36 -0600 Subject: [PATCH] Ensure the OVS bridge exists when plugging When a vif will be plugged into an OVS bridge, ensure that the bridge exists during the plugging operation. In most cases, the OVS bridge will already exist at the time of port creation, so this will be a no-op. However, the OVS agent implementation for vlan-aware VMs will require the os-vif plugin to create the OVS bridge if it doesn't already exist, because vifs that correspond to trunk ports won't be directly plugged into br-int. Change-Id: I3cb61a5e842177a26f38c85d7782ecced4ac07da --- .../ensure-ovs-bridge-a0c1b51f469c92d0.yaml | 7 ++++ vif_plug_ovs/constants.py | 3 ++ vif_plug_ovs/linux_net.py | 10 ++++++ vif_plug_ovs/ovs.py | 7 +++- vif_plug_ovs/tests/test_linux_net.py | 18 ++++++++++ vif_plug_ovs/tests/test_plugin.py | 36 +++++++++++++------ 6 files changed, 69 insertions(+), 12 deletions(-) create mode 100644 releasenotes/notes/ensure-ovs-bridge-a0c1b51f469c92d0.yaml diff --git a/releasenotes/notes/ensure-ovs-bridge-a0c1b51f469c92d0.yaml b/releasenotes/notes/ensure-ovs-bridge-a0c1b51f469c92d0.yaml new file mode 100644 index 00000000..9ee5afbd --- /dev/null +++ b/releasenotes/notes/ensure-ovs-bridge-a0c1b51f469c92d0.yaml @@ -0,0 +1,7 @@ +--- +features: + - The ovs plugin has been modified to ensure that the + specified OVS bridge that the vif will be attached to + has been created. If the OVS bridge does not exist, it + will be created with the proper datapath_type. + diff --git a/vif_plug_ovs/constants.py b/vif_plug_ovs/constants.py index b30959ba..755e2586 100644 --- a/vif_plug_ovs/constants.py +++ b/vif_plug_ovs/constants.py @@ -11,3 +11,6 @@ # under the License. OVS_VHOSTUSER_INTERFACE_TYPE = 'dpdkvhostuser' + +OVS_DATAPATH_SYSTEM = 'system' +OVS_DATAPATH_NETDEV = 'netdev' diff --git a/vif_plug_ovs/linux_net.py b/vif_plug_ovs/linux_net.py index 6697887f..316f4273 100644 --- a/vif_plug_ovs/linux_net.py +++ b/vif_plug_ovs/linux_net.py @@ -60,6 +60,11 @@ def _create_ovs_vif_cmd(bridge, dev, iface_id, mac, return cmd +def _create_ovs_bridge_cmd(bridge, datapath_type): + return ['--', '--may-exist', 'add-br', bridge, + '--', 'set', 'Bridge', bridge, 'datapath_type=%s' % datapath_type] + + @privsep.vif_plug.entrypoint def create_ovs_vif_port(bridge, dev, iface_id, mac, instance_id, mtu=None, interface_type=None): @@ -117,6 +122,11 @@ def create_veth_pair(dev1_name, dev2_name, mtu): _set_device_mtu(dev, mtu) +@privsep.vif_plug.entrypoint +def ensure_ovs_bridge(bridge, datapath_type): + _ovs_vsctl(_create_ovs_bridge_cmd(bridge, datapath_type)) + + @privsep.vif_plug.entrypoint def ensure_bridge(bridge): if not device_exists(bridge): diff --git a/vif_plug_ovs/ovs.py b/vif_plug_ovs/ovs.py index a12d81fc..380c6834 100644 --- a/vif_plug_ovs/ovs.py +++ b/vif_plug_ovs/ovs.py @@ -81,6 +81,8 @@ class OvsPlugin(plugin.PluginBase): ]) def _plug_vhostuser(self, vif, instance_info): + linux_net.ensure_ovs_bridge(vif.network.bridge, + constants.OVS_DATAPATH_NETDEV) linux_net.create_ovs_vif_port( vif.network.bridge, OvsPlugin.gen_port_name("vhu", vif.id), @@ -107,6 +109,8 @@ class OvsPlugin(plugin.PluginBase): linux_net.create_veth_pair(v1_name, v2_name, self.config.network_device_mtu) linux_net.add_bridge_port(vif.bridge_name, v1_name) + linux_net.ensure_ovs_bridge(vif.network.bridge, + constants.OVS_DATAPATH_SYSTEM) linux_net.create_ovs_vif_port( vif.network.bridge, v2_name, @@ -124,7 +128,8 @@ class OvsPlugin(plugin.PluginBase): profile=vif.port_profile.__class__.__name__) if isinstance(vif, objects.vif.VIFOpenVSwitch): - pass # no special plugging required + linux_net.ensure_ovs_bridge(vif.network.bridge, + constants.OVS_DATAPATH_SYSTEM) elif isinstance(vif, objects.vif.VIFBridge): self._plug_bridge(vif, instance_info) elif isinstance(vif, objects.vif.VIFVHostUser): diff --git a/vif_plug_ovs/tests/test_linux_net.py b/vif_plug_ovs/tests/test_linux_net.py index 2534a61d..4ed208a1 100644 --- a/vif_plug_ovs/tests/test_linux_net.py +++ b/vif_plug_ovs/tests/test_linux_net.py @@ -141,6 +141,24 @@ class LinuxNetTest(testtools.TestCase): 'fake-type') self.assertEqual(expected, cmd) + @mock.patch.object(linux_net, '_create_ovs_bridge_cmd') + @mock.patch.object(linux_net, '_ovs_vsctl') + def test_ensure_ovs_bridge(self, mock_vsctl, mock_create_ovs_bridge): + bridge = 'fake-bridge' + dp_type = 'fake-type' + linux_net.ensure_ovs_bridge(bridge, dp_type) + mock_create_ovs_bridge.assert_called_once_with(bridge, dp_type) + self.assertTrue(mock_vsctl.called) + + def test_create_ovs_bridge_cmd(self): + bridge = 'fake-bridge' + dp_type = 'fake-type' + expected = ['--', '--may-exist', 'add-br', bridge, + '--', 'set', 'Bridge', bridge, + 'datapath_type=%s' % dp_type] + actual = linux_net._create_ovs_bridge_cmd(bridge, dp_type) + self.assertEqual(expected, actual) + @mock.patch.object(linux_net, '_ovs_vsctl') @mock.patch.object(linux_net, '_create_ovs_vif_cmd') @mock.patch.object(linux_net, '_set_device_mtu') diff --git a/vif_plug_ovs/tests/test_plugin.py b/vif_plug_ovs/tests/test_plugin.py index 55935553..8da0b3b9 100644 --- a/vif_plug_ovs/tests/test_plugin.py +++ b/vif_plug_ovs/tests/test_plugin.py @@ -17,6 +17,7 @@ import testtools from os_vif import objects +from vif_plug_ovs import constants from vif_plug_ovs import linux_net from vif_plug_ovs import ovs @@ -88,11 +89,15 @@ class PluginTest(testtools.TestCase): 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) + with mock.patch.object(linux_net, 'ensure_ovs_bridge') as ( + ensure_ovs_bridge): + 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) + ensure_ovs_bridge.assert_called_once_with( + self.vif_ovs.network.bridge, constants.OVS_DATAPATH_SYSTEM) def test_plug_ovs_bridge(self): calls = { @@ -109,7 +114,9 @@ class PluginTest(testtools.TestCase): 'ca:fe:de:ad:be:ef', 'f0000000-0000-0000-0000-000000000001', 1500, - timeout=120)] + timeout=120)], + 'ensure_ovs_bridge': [mock.call('br0', + constants.OVS_DATAPATH_SYSTEM)] } with nested( @@ -118,9 +125,10 @@ class PluginTest(testtools.TestCase): return_value=False), mock.patch.object(linux_net, 'create_veth_pair'), mock.patch.object(linux_net, 'add_bridge_port'), - mock.patch.object(linux_net, 'create_ovs_vif_port') + mock.patch.object(linux_net, 'create_ovs_vif_port'), + mock.patch.object(linux_net, 'ensure_ovs_bridge') ) as (ensure_bridge, device_exists, create_veth_pair, - add_bridge_port, create_ovs_vif_port): + add_bridge_port, create_ovs_vif_port, ensure_ovs_bridge): plugin = ovs.OvsPlugin.load("ovs") plugin.plug(self.vif_ovs_hybrid, self.instance) ensure_bridge.assert_has_calls(calls['ensure_bridge']) @@ -128,6 +136,7 @@ class PluginTest(testtools.TestCase): 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']) + ensure_ovs_bridge.assert_has_calls(calls['ensure_ovs_bridge']) def test_unplug_ovs(self): unplug_bridge_mock = mock.Mock() @@ -160,14 +169,19 @@ class PluginTest(testtools.TestCase): 'f0000000-0000-0000-0000-000000000001', 1500, interface_type='dpdkvhostuser', - timeout=120)] + timeout=120)], + 'ensure_ovs_bridge': [mock.call('br0', + constants.OVS_DATAPATH_NETDEV)] } - with mock.patch.object(linux_net, 'create_ovs_vif_port') \ - as (create_ovs_vif_port): + with nested( + mock.patch.object(linux_net, 'create_ovs_vif_port'), + mock.patch.object(linux_net, 'ensure_ovs_bridge') + ) as (create_ovs_vif_port, ensure_ovs_bridge): plugin = ovs.OvsPlugin.load("ovs") plugin.plug(self.vif_vhostuser, self.instance) create_ovs_vif_port.assert_has_calls(calls['create_ovs_vif_port']) + ensure_ovs_bridge.assert_has_calls(calls['ensure_ovs_bridge']) def test_unplug_ovs_vhostuser(self): calls = {