]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
fix issue with OVS plugin VLAN allocation after a quantum-server restart
authorDan Wendlandt <dan@nicira.com>
Tue, 3 Apr 2012 03:14:37 +0000 (20:14 -0700)
committerDan Wendlandt <dan@nicira.com>
Tue, 3 Apr 2012 03:14:37 +0000 (20:14 -0700)
bug 962853

Also use constants for VLAN_MIN/VLAN_MAX and clean-up VlanMap unit tests.

Change-Id: Id7b580d604092b5fc16d4c87ae866d419aad4d1f

quantum/plugins/openvswitch/ovs_quantum_plugin.py
quantum/plugins/openvswitch/tests/unit/test_vlan_map.py

index c5091c579dea8219730d7b4a79c76be5312e0e8d..aced08c30117c04c584faa50f4c4dc3e17c2b275 100644 (file)
@@ -40,15 +40,26 @@ LOG.basicConfig(level=LOG.WARN)
 LOG.getLogger("ovs_quantum_plugin")
 
 
+# Exception thrown if no more VLANs are available
+class NoFreeVLANException(Exception):
+    pass
+
+
 class VlanMap(object):
     vlans = {}
     net_ids = {}
     free_vlans = set()
+    VLAN_MIN = 1
+    VLAN_MAX = 4094
 
     def __init__(self):
         self.vlans.clear()
         self.net_ids.clear()
-        self.free_vlans = set(xrange(2, 4094))
+        self.free_vlans = set(xrange(self.VLAN_MIN, self.VLAN_MAX + 1))
+
+    def already_used(self, vlan_id, network_id):
+        self.free_vlans.remove(vlan_id)
+        self.set_vlan(vlan_id, network_id)
 
     def set_vlan(self, vlan_id, network_id):
         self.vlans[vlan_id] = network_id
@@ -58,13 +69,11 @@ class VlanMap(object):
         if len(self.free_vlans):
             vlan = self.free_vlans.pop()
             self.set_vlan(vlan, network_id)
-            # LOG.debug("VlanMap::acquire %s -> %s", x, network_id)
+            LOG.debug("Allocated VLAN %s for network %s" % (vlan, network_id))
             return vlan
         else:
-            raise Exception("No free vlans..")
-
-    def get(self, vlan_id):
-        return self.vlans.get(vlan_id, None)
+            raise NoFreeVLANException("No VLAN free for network %s" %
+                                      network_id)
 
     def release(self, network_id):
         vlan = self.net_ids.get(network_id, None)
@@ -72,7 +81,8 @@ class VlanMap(object):
             self.free_vlans.add(vlan)
             del self.vlans[vlan]
             del self.net_ids[network_id]
-            # LOG.debug("VlanMap::release %s", vlan)
+            LOG.debug("Deallocated VLAN %s (used by network %s)"
+                      % (vlan, network_id))
         else:
             LOG.error("No vlan found with network \"%s\"", network_id)
 
@@ -103,9 +113,9 @@ class OVSQuantumPlugin(QuantumPluginBase):
         vlans = ovs_db.get_vlans()
         for x in vlans:
             vlan_id, network_id = x
-            LOG.debug("Adding already populated vlan %s -> %s"
-            #                                   % (vlan_id, network_id))
-            self.vmap.set_vlan(vlan_id, network_id)
+            LOG.debug("Adding already populated vlan %s -> %s"
+                      % (vlan_id, network_id))
+            self.vmap.already_used(vlan_id, network_id)
 
     def get_all_networks(self, tenant_id, **kwargs):
         nets = []
index f0dde727c04cdc2e65a9bc5527c6a760e2d30a61..4e4b62406ddc8e751df592257345a0c7c66b68a8 100644 (file)
@@ -15,7 +15,7 @@
 #    under the License.
 
 import unittest
-from ovs_quantum_plugin import VlanMap
+from ovs_quantum_plugin import VlanMap, NoFreeVLANException
 
 
 class VlanMapTest(unittest.TestCase):
@@ -28,18 +28,35 @@ class VlanMapTest(unittest.TestCase):
 
     def testAddVlan(self):
         vlan_id = self.vmap.acquire("foobar")
-        self.assertTrue(vlan_id == 2)
+        self.assertTrue(vlan_id >= VlanMap.VLAN_MIN)
+        self.assertTrue(vlan_id <= VlanMap.VLAN_MAX)
 
     def testReleaseVlan(self):
         vlan_id = self.vmap.acquire("foobar")
         self.vmap.release("foobar")
-        self.assertTrue(self.vmap.get(vlan_id) is None)
 
     def testAddRelease4kVlans(self):
         vlan_id = None
-        for id in range(2, 4000):
-            vlan_id = self.vmap.acquire(id)
-            self.assertTrue(vlan_id == id)
-        for id in range(2, 4000):
-            self.vmap.release(id)
-            self.assertTrue(self.vmap.get(id) is None)
+        num_vlans = VlanMap.VLAN_MAX - VlanMap.VLAN_MIN
+        for id in xrange(num_vlans):
+            vlan_id = self.vmap.acquire("net-%s" % id)
+            self.assertTrue(vlan_id >= VlanMap.VLAN_MIN)
+            self.assertTrue(vlan_id <= VlanMap.VLAN_MAX)
+        for id in xrange(num_vlans):
+            self.vmap.release("net-%s" % id)
+
+    def testAlreadyUsed(self):
+        existing_vlan = 2
+        self.vmap.already_used(existing_vlan, "net1")
+        try:
+            # this value is high enough that we will exhaust
+            # all VLANs.  We want to make sure 'existing_vlan'
+            # is never reallocated.
+            num_vlans = VlanMap.VLAN_MAX - VlanMap.VLAN_MIN + 1
+            for x in xrange(num_vlans):
+                vlan_id = self.vmap.acquire("net-%x" % x)
+                self.assertTrue(vlan_id != existing_vlan)
+
+            self.fail("Did not run out of VLANs as expected")
+        except NoFreeVLANException:
+            pass  # Expected exit