]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Don't use iterator in search for tunnel type
authorJakub Libosvar <libosvar@redhat.com>
Wed, 29 Apr 2015 10:04:03 +0000 (12:04 +0200)
committerJakub Libosvar <libosvar@redhat.com>
Thu, 30 Apr 2015 07:35:29 +0000 (09:35 +0200)
Changing dictionary size while using iterator causes RuntimeError. This
can happen in local vlan mapping under certain program flows. This patch
changes iteritems() to values() that returns list and thus preventing
from failure if local vlan mapping changes during creating tunnels.

Change-Id: I8a858d5c53e85f83a582f34205f9afa214cb4d58
Closes-Bug: 1449944

neutron/plugins/openvswitch/agent/ovs_neutron_agent.py
neutron/tests/unit/plugins/openvswitch/agent/test_ovs_neutron_agent.py

index 9e510c70b369d44001153346448af66f1d0945db..e85dee371a8f25aaa28f53b10d1b0ed9dcb02ed4 100644 (file)
@@ -1186,7 +1186,7 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
         ofports = _ofport_set_to_str(self.tun_br_ofports[tunnel_type].values())
         if ofports and not self.l2_pop:
             # Update flooding flows to include the new tunnel
-            for network_id, vlan_mapping in self.local_vlan_map.iteritems():
+            for vlan_mapping in list(self.local_vlan_map.values()):
                 if vlan_mapping.network_type == tunnel_type:
                     br.mod_flow(table=constants.FLOOD_TO_TUN,
                                 dl_vlan=vlan_mapping.vlan,
index ee14e967a1435e0fc7003ee55d8122622b6cc884..817336e6e7915be8d4664b8d6e7448d8c9439c69 100644 (file)
@@ -1203,6 +1203,29 @@ class TestOvsNeutronAgent(base.BaseTestCase):
         self.agent.treat_devices_added_or_updated.assert_called_with(
             ['port1'], ovs_restarted=False)
 
+    def test__setup_tunnel_port_while_new_mapping_is_added(self):
+        """
+        Test that _setup_tunnel_port doesn't fail if new vlan mapping is
+        added in a different coroutine while iterating over existing mappings.
+        See bug 1449944 for more info.
+        """
+
+        def add_new_vlan_mapping(*args, **kwargs):
+            self.agent.local_vlan_map['bar'] = (
+                ovs_neutron_agent.LocalVLANMapping(1, 2, 3, 4))
+        bridge = mock.Mock()
+        tunnel_type = 'vxlan'
+        self.agent.tun_br_ofports = {tunnel_type: dict()}
+        self.agent.l2_pop = False
+        self.agent.local_vlan_map = {
+            'foo': ovs_neutron_agent.LocalVLANMapping(4, tunnel_type, 2, 1)}
+        bridge.mod_flow.side_effect = add_new_vlan_mapping
+        with mock.patch('neutron.plugins.openvswitch.agent.ovs_neutron_agent.'
+                        '_ofport_set_to_str', return_value=True):
+            self.agent._setup_tunnel_port(bridge, 1, 2,
+                                          tunnel_type=tunnel_type)
+        self.assertIn('bar', self.agent.local_vlan_map)
+
 
 class AncillaryBridgesTest(base.BaseTestCase):