]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Database exception causes UnboundLocalError in linuxbridge-agent
authorLi Ma <skywalker.nick@gmail.com>
Fri, 25 Apr 2014 07:37:46 +0000 (00:37 -0700)
committerLi Ma <skywalker.nick@gmail.com>
Sat, 26 Apr 2014 14:04:20 +0000 (07:04 -0700)
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

neutron/plugins/linuxbridge/agent/linuxbridge_neutron_agent.py
neutron/tests/unit/linuxbridge/test_lb_neutron_agent.py

index 962aba177342eb6bea9b2b0f65e7507881f209c1..30008822aca8b64470d95f96a35a28522c986259 100755 (executable)
@@ -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)
index 643dd850b413888feb7c50d805b19a48d7e51b05..eb10caaf07f621fcd288bbdf37293a011857613e 100644 (file)
@@ -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,