From 4539ff26d903174c844d6571533dfe719195e107 Mon Sep 17 00:00:00 2001 From: Li Ma Date: Fri, 25 Apr 2014 00:37:46 -0700 Subject: [PATCH] Database exception causes UnboundLocalError in linuxbridge-agent When database exception raises from update_device_down, the linuxbridge-agent doesn't deal with them in a proper way that causes accessing not-existed local variables. A straightforward workaround is to set it as None by default and verify its value then. Change-Id: I32a9467876a619a7d0ad53ff3d5d95ff977e54f4 Closes-Bug: #1311971 --- .../agent/linuxbridge_neutron_agent.py | 3 +- .../unit/linuxbridge/test_lb_neutron_agent.py | 57 +++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/neutron/plugins/linuxbridge/agent/linuxbridge_neutron_agent.py b/neutron/plugins/linuxbridge/agent/linuxbridge_neutron_agent.py index 962aba177..30008822a 100755 --- a/neutron/plugins/linuxbridge/agent/linuxbridge_neutron_agent.py +++ b/neutron/plugins/linuxbridge/agent/linuxbridge_neutron_agent.py @@ -966,6 +966,7 @@ class LinuxBridgeNeutronAgentRPC(sg_rpc.SecurityGroupAgentRpcMixin): self.remove_devices_filter(devices) for device in devices: LOG.info(_("Attachment %s removed"), device) + details = None try: details = self.plugin_rpc.update_device_down(self.context, device, @@ -975,7 +976,7 @@ class LinuxBridgeNeutronAgentRPC(sg_rpc.SecurityGroupAgentRpcMixin): LOG.debug(_("port_removed failed for %(device)s: %(e)s"), {'device': device, 'e': e}) resync = True - if details['exists']: + if details and details['exists']: LOG.info(_("Port %s updated."), device) else: LOG.debug(_("Device %s not defined on plugin"), device) diff --git a/neutron/tests/unit/linuxbridge/test_lb_neutron_agent.py b/neutron/tests/unit/linuxbridge/test_lb_neutron_agent.py index 643dd850b..eb10caaf0 100644 --- a/neutron/tests/unit/linuxbridge/test_lb_neutron_agent.py +++ b/neutron/tests/unit/linuxbridge/test_lb_neutron_agent.py @@ -32,6 +32,7 @@ from neutron.plugins.linuxbridge.common import constants as lconst from neutron.tests import base LOCAL_IP = '192.168.0.33' +DEVICE_1 = 'tapabcdef01-12' class FakeIpLinkCommand(object): @@ -111,6 +112,62 @@ class TestLinuxBridgeAgent(base.BaseTestCase): self.get_mac = self.get_mac_p.start() self.get_mac.return_value = '00:00:00:00:00:01' + def test_treat_devices_removed_with_existed_device(self): + agent = linuxbridge_neutron_agent.LinuxBridgeNeutronAgentRPC({}, + 0, + None) + devices = [DEVICE_1] + with contextlib.nested( + mock.patch.object(agent.plugin_rpc, "update_device_down"), + mock.patch.object(agent, "remove_devices_filter") + ) as (fn_udd, fn_rdf): + fn_udd.return_value = {'device': DEVICE_1, + 'exists': True} + with mock.patch.object(linuxbridge_neutron_agent.LOG, + 'info') as log: + resync = agent.treat_devices_removed(devices) + self.assertEqual(2, log.call_count) + self.assertFalse(resync) + self.assertTrue(fn_udd.called) + self.assertTrue(fn_rdf.called) + + def test_treat_devices_removed_with_not_existed_device(self): + agent = linuxbridge_neutron_agent.LinuxBridgeNeutronAgentRPC({}, + 0, + None) + devices = [DEVICE_1] + with contextlib.nested( + mock.patch.object(agent.plugin_rpc, "update_device_down"), + mock.patch.object(agent, "remove_devices_filter") + ) as (fn_udd, fn_rdf): + fn_udd.return_value = {'device': DEVICE_1, + 'exists': False} + with mock.patch.object(linuxbridge_neutron_agent.LOG, + 'debug') as log: + resync = agent.treat_devices_removed(devices) + self.assertEqual(1, log.call_count) + self.assertFalse(resync) + self.assertTrue(fn_udd.called) + self.assertTrue(fn_rdf.called) + + def test_treat_devices_removed_failed(self): + agent = linuxbridge_neutron_agent.LinuxBridgeNeutronAgentRPC({}, + 0, + None) + devices = [DEVICE_1] + with contextlib.nested( + mock.patch.object(agent.plugin_rpc, "update_device_down"), + mock.patch.object(agent, "remove_devices_filter") + ) as (fn_udd, fn_rdf): + fn_udd.side_effect = Exception() + with mock.patch.object(linuxbridge_neutron_agent.LOG, + 'debug') as log: + resync = agent.treat_devices_removed(devices) + self.assertEqual(2, log.call_count) + self.assertTrue(resync) + self.assertTrue(fn_udd.called) + self.assertTrue(fn_rdf.called) + def test_update_devices_failed(self): agent = linuxbridge_neutron_agent.LinuxBridgeNeutronAgentRPC({}, 0,