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 0000000..9ee5afb --- /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 b30959b..755e258 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 6697887..316f427 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 a12d81f..380c683 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 2534a61..4ed208a 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 5593555..8da0b3b 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 = {