From c25ce5a852d97f8159e77cb7ed71df31c482ce76 Mon Sep 17 00:00:00 2001 From: Kevin Benton Date: Fri, 26 Aug 2016 15:56:24 -0700 Subject: [PATCH] Check for concurrent bridge creation in bridge add With Linux bridge, either Neutron or Nova may create the bridge associated with a network so this process must be robust to concurrent creations of the same bridge. This change just has the addbr call capture exceptions and avoid reraising if the device already exists. Closes-Bug: #1617447 Change-Id: Ib0266086e0caffecf3f9f2a8291369cfa155f386 --- vif_plug_linux_bridge/linux_net.py | 7 ++++++- vif_plug_linux_bridge/tests/test_linux_net.py | 20 +++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/vif_plug_linux_bridge/linux_net.py b/vif_plug_linux_bridge/linux_net.py index 795474f6..9792dc66 100644 --- a/vif_plug_linux_bridge/linux_net.py +++ b/vif_plug_linux_bridge/linux_net.py @@ -24,6 +24,7 @@ import os from oslo_concurrency import lockutils from oslo_concurrency import processutils from oslo_log import log as logging +from oslo_utils import excutils from vif_plug_linux_bridge import privsep @@ -122,7 +123,11 @@ def _ensure_bridge_privileged(bridge, interface, net_attrs, gateway, """ if not device_exists(bridge): LOG.debug('Starting Bridge %s', bridge) - processutils.execute('brctl', 'addbr', bridge) + try: + processutils.execute('brctl', 'addbr', bridge) + except Exception: + with excutils.save_and_reraise_exception() as ectx: + ectx.reraise = not device_exists(bridge) processutils.execute('brctl', 'setfd', bridge, 0) # processutils.execute('brctl setageing %s 10' % bridge) processutils.execute('brctl', 'stp', bridge, 'off') diff --git a/vif_plug_linux_bridge/tests/test_linux_net.py b/vif_plug_linux_bridge/tests/test_linux_net.py index 3fbacbce..652ffc04 100644 --- a/vif_plug_linux_bridge/tests/test_linux_net.py +++ b/vif_plug_linux_bridge/tests/test_linux_net.py @@ -94,6 +94,26 @@ class LinuxNetTest(testtools.TestCase): self.assertEqual([], mock_exec.mock_calls) mock_dev_exists.assert_called_once_with("br0") + @mock.patch.object(processutils, "execute") + @mock.patch.object(linux_net, "device_exists", return_value=False) + def test_ensure_bridge_addbr_exception(self, mock_dev_exists, mock_exec): + mock_exec.side_effect = ValueError() + with testtools.ExpectedException(ValueError): + linux_net.ensure_bridge("br0", None, filtering=False) + + @mock.patch.object(processutils, "execute") + @mock.patch.object(linux_net, "device_exists", side_effect=[False, True]) + def test_ensure_bridge_concurrent_add(self, mock_dev_exists, mock_exec): + mock_exec.side_effect = [ValueError(), 0, 0, 0] + linux_net.ensure_bridge("br0", None, filtering=False) + + calls = [mock.call('brctl', 'addbr', 'br0'), + mock.call('brctl', 'setfd', 'br0', 0), + mock.call('brctl', 'stp', 'br0', "off"), + mock.call('ip', 'link', 'set', 'br0', "up")] + self.assertEqual(calls, mock_exec.mock_calls) + mock_dev_exists.assert_has_calls([mock.call("br0"), mock.call("br0")]) + @mock.patch.object(os.path, "exists", return_value=False) @mock.patch.object(processutils, "execute") @mock.patch.object(linux_net, "device_exists", return_value=False)