lb-agent: handle exception when bridge slave already removed

An exception can happen when a network is deleted because the
lb-agent tries to removes the dhcp tap from the bridge at about
the same time as the dhcp-agent is deleting the tap. The unhandled
exception means the bridge does not get deleted and a log error.

Closes-Bug: #1611612
Change-Id: Ia9a6b5fc49e239769e850e9486454e81e3a4b96f
This commit is contained in:
Darragh O'Reilly 2016-08-10 05:58:50 +00:00
parent 87517709f2
commit 72720f9aa3
2 changed files with 38 additions and 16 deletions

View File

@ -559,19 +559,30 @@ class LinuxBridgeManager(amb.CommonAgentManagerBase):
def remove_interface(self, bridge_name, interface_name):
bridge_device = bridge_lib.BridgeDevice(bridge_name)
if bridge_device.exists():
if not bridge_lib.is_bridged_interface(interface_name):
if not bridge_device.owns_interface(interface_name):
return True
LOG.debug("Removing device %(interface_name)s from bridge "
"%(bridge_name)s",
{'interface_name': interface_name,
'bridge_name': bridge_name})
if bridge_device.delif(interface_name):
return False
LOG.debug("Done removing device %(interface_name)s from bridge "
"%(bridge_name)s",
{'interface_name': interface_name,
'bridge_name': bridge_name})
return True
try:
bridge_device.delif(interface_name)
LOG.debug("Done removing device %(interface_name)s from "
"bridge %(bridge_name)s",
{'interface_name': interface_name,
'bridge_name': bridge_name})
return True
except RuntimeError:
with excutils.save_and_reraise_exception() as ctxt:
if not bridge_device.owns_interface(interface_name):
# the exception was likely a side effect of the tap
# being deleted by some other agent during handling
ctxt.reraise = False
LOG.debug("Cannot remove %(interface_name)s from "
"%(bridge_name)s. It is not on the bridge.",
{'interface_name': interface_name,
'bridge_name': bridge_name})
return False
else:
LOG.debug("Cannot remove device %(interface_name)s bridge "
"%(bridge_name)s does not exist",

View File

@ -706,25 +706,36 @@ class TestLinuxBridgeManager(base.BaseTestCase):
def test_remove_interface(self):
with mock.patch.object(ip_lib.IPDevice, "exists") as de_fn,\
mock.patch.object(bridge_lib,
'is_bridged_interface') as isdev_fn,\
mock.patch.object(bridge_lib.BridgeDevice,
'owns_interface') as owns_fn,\
mock.patch.object(bridge_lib.BridgeDevice,
"delif") as delif_fn:
de_fn.return_value = False
self.assertFalse(self.lbm.remove_interface("br0", "eth0"))
self.assertFalse(isdev_fn.called)
self.assertFalse(owns_fn.called)
de_fn.return_value = True
isdev_fn.return_value = False
owns_fn.return_value = False
self.assertTrue(self.lbm.remove_interface("br0", "eth0"))
isdev_fn.return_value = True
delif_fn.return_value = True
self.assertFalse(self.lbm.remove_interface("br0", "eth0"))
delif_fn.return_value = False
self.assertTrue(self.lbm.remove_interface("br0", "eth0"))
def test_remove_interface_not_on_bridge(self):
bridge_device = mock.Mock()
with mock.patch.object(bridge_lib, "BridgeDevice",
return_value=bridge_device):
bridge_device.exists.return_value = True
bridge_device.delif.side_effect = RuntimeError
bridge_device.owns_interface.side_effect = [True, False]
self.lbm.remove_interface("br0", 'tap0')
self.assertEqual(2, bridge_device.owns_interface.call_count)
bridge_device.owns_interface.side_effect = [True, True]
self.assertRaises(RuntimeError,
self.lbm.remove_interface, "br0", 'tap0')
def test_delete_interface(self):
with mock.patch.object(ip_lib.IPDevice, "exists") as de_fn,\
mock.patch.object(ip_lib.IpLinkCommand, "set_down") as down_fn,\