Fix _restore_local_vlan_map race

Local vlan mappings provisioned by _restore_local_vlan_map
might not be cleaned up properly if the corresponding ports are
removed before we enter the main loop.

This commit fixes the problem by making _restore_local_vlan_map
just record local vlan IDs and leave the rest of provisioning logic
to the main loop.

Closes-Bug: #1507776
Change-Id: I84ff6684ae3fda7d839c14b1a7382871f45ce610

diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py
index c051e5d8357bfb08c72076fbdbce529b1ead1b67..5158c3b8c6fec9667f701aa48b6a55781765a2e3 100644 (file)
--- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py
+++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py
@@ -335,6 +335,7 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
             LOG.exception(_LE("Failed reporting state!"))
 
     def _restore_local_vlan_map(self):
+        self._local_vlan_hints = {}
         cur_ports = self.int_br.get_vif_ports()
         port_names = [p.port_name for p in cur_ports]
         port_info = self.int_br.get_ports_attributes(
@@ -352,20 +353,15 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
             if not local_vlan:
                 continue
             net_uuid = local_vlan_map.get('net_uuid')
-            if (net_uuid and net_uuid not in self.local_vlan_map
+            if (net_uuid and net_uuid not in self._local_vlan_hints
                 and local_vlan != DEAD_VLAN_TAG):
-                segmentation_id = local_vlan_map.get('segmentation_id')
-                if segmentation_id == 'None':
-                    # Backward compatible check when we used to store the
-                    # string 'None' in OVS
-                    segmentation_id = None
-                if segmentation_id is not None:
-                    segmentation_id = int(segmentation_id)
-                self.provision_local_vlan(local_vlan_map['net_uuid'],
-                                          local_vlan_map['network_type'],
-                                          local_vlan_map['physical_network'],
-                                          segmentation_id,
-                                          local_vlan)
+                self.available_local_vlans.remove(local_vlan)
+                self._local_vlan_hints[local_vlan_map['net_uuid']] = \
+                    local_vlan
+
+    def _dispose_local_vlan_hints(self):
+        self.available_local_vlans.update(self._local_vlan_hints.values())
+        self._local_vlan_hints = {}
 
     def setup_rpc(self):
         self.agent_id = 'ovs-agent-%s' % self.conf.host
@@ -624,7 +620,7 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
                                     segmentation_id=segmentation_id)
 
     def provision_local_vlan(self, net_uuid, network_type, physical_network,
-                             segmentation_id, local_vlan=None):
+                             segmentation_id):
         '''Provisions a local VLAN.
 
         :param net_uuid: the uuid of the network associated with this vlan.
@@ -641,10 +637,8 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
         if lvm:
             lvid = lvm.vlan
         else:
-            if local_vlan in self.available_local_vlans:
-                lvid = local_vlan
-                self.available_local_vlans.remove(local_vlan)
-            else:
+            lvid = self._local_vlan_hints.pop(net_uuid, None)
+            if lvid is None:
                 if not self.available_local_vlans:
                     LOG.error(_LE("No local VLAN available for net-id=%s"),
                               net_uuid)
@@ -1790,6 +1784,7 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
                     # so we can sure that no other Exception occurred.
                     if not sync:
                         ovs_restarted = False
+                        self._dispose_local_vlan_hints()
                 except Exception:
                     LOG.exception(_LE("Error while processing VIF ports"))
                     # Put the ports back in self.updated_port

diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py
index 0e5761d254ed56c9749a1687066ec4c3445e3d53..b841902f1c2f542254a45299ee8a9a53d177f72e 100644 (file)
--- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py
+++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py
@@ -242,24 +242,23 @@ class TestOvsNeutronAgent(object):
     def _test_restore_local_vlan_maps(self, tag, segmentation_id='1'):
         port = mock.Mock()
         port.port_name = 'fake_port'
-        local_vlan_map = {'net_uuid': 'fake_network_id',
+        net_uuid = 'fake_network_id'
+        local_vlan_map = {'net_uuid': net_uuid,
                           'network_type': 'vlan',
                           'physical_network': 'fake_network'}
         if segmentation_id is not None:
             local_vlan_map['segmentation_id'] = segmentation_id
-        with mock.patch.object(self.agent, 'int_br') as int_br, \
-            mock.patch.object(self.agent, 'provision_local_vlan') as \
-                provision_local_vlan:
+        with mock.patch.object(self.agent, 'int_br') as int_br:
             int_br.get_vif_ports.return_value = [port]
             int_br.get_ports_attributes.return_value = [{
                 'name': port.port_name, 'other_config': local_vlan_map,
                 'tag': tag
             }]
             self.agent._restore_local_vlan_map()
+            expected_hints = {}
             if tag:
-                self.assertTrue(provision_local_vlan.called)
-            else:
-                self.assertFalse(provision_local_vlan.called)
+                expected_hints[net_uuid] = tag
+            self.assertEqual(expected_hints, self.agent._local_vlan_hints)
 
     def test_restore_local_vlan_map_with_device_has_tag(self):
         self._test_restore_local_vlan_maps(2)