fixed bug, wasn't detecting smaller subnet conflict properly added test for it

2 files changed, 53 insertions, 12 deletions

diff --git a/nova/network/manager.py b/nova/network/manager.py
index 3c29417d7..873fcadf5 100644
--- a/nova/network/manager.py
+++ b/nova/network/manager.py
@@ -624,7 +624,6 @@ class NetworkManager(manager.SchedulerDependentManager):
             raise ValueError(_(msg))
         adjusted_cidr_str = req_net_ip + '/' + str(significant_bits)
         adjusted_cidr = netaddr.IPNetwork(adjusted_cidr_str)
-        all_req_nets = [adjusted_cidr]
         try:
             used_nets = self.db.network_get_all(context)
         except exception.NoNetworksFound:
@@ -636,22 +635,36 @@ class NetworkManager(manager.SchedulerDependentManager):
             if adjusted_cidr_supernet in used_cidrs:
                 msg = "requested cidr (%s) conflicts with existing supernet"
                 raise ValueError(_(msg % str(adjusted_cidr)))
-        # split supernet into subnets
-        if num_networks >= 2:
+        # watch for smaller subnets conflicting
+        used_supernets = []
+        for used_cidr in used_cidrs:
+            if not used_cidr:
+                continue
+            if used_cidr.size < network_size:
+                for ucsupernet in used_cidr.supernet():
+                    if ucsupernet.size == network_size:
+                        used_supernets.append(ucsupernet)
+        all_req_nets = []
+        if num_networks == 1:
+            if adjusted_cidr in used_supernets:
+                msg = "requested cidr (%s) conflicts with existing smaller cidr"
+                raise ValueError(_(msg % str(adjusted_cidr)))
+            else:
+                all_req_nets.append(adjusted_cidr)
+        elif num_networks >= 2:
+            # split supernet into subnets
             next_cidr = adjusted_cidr
-            for used_cidr in used_cidrs:
-                # watch for smaller subnets conflicting
-                if used_cidr.size < next_cidr.size:
-                    for ucsupernet in used_cidr.supernet():
-                        if ucsupernet.size == next_cidr.size:
-                            used_cidrs.append(ucsupernet)
-            for index in range(1, num_networks):
+            for index in range(num_networks):
                 while True:
-                    next_cidr = next_cidr.next()
                     if next_cidr in used_cidrs:
+                        next_cidr = next_cidr.next()
+                        continue
+                    elif next_cidr in used_supernets:
+                        next_cidr = next_cidr.next()
                         continue
                     else:
                         all_req_nets.append(next_cidr)
+                        next_cidr = next_cidr.next()
                         break
         all_req_nets = sorted(list(set(all_req_nets)))
         # after splitting ensure there were enough to satisfy the num_networks
diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py
index 1ff3f0c01..e3a677c97 100644
--- a/nova/tests/test_network.py
+++ b/nova/tests/test_network.py
@@ -313,7 +313,19 @@ class CommonNetworkTestCase(test.TestCase):
             self.assertTrue(exp_cidr + '/24' in cidrs)
         self.assertFalse('192.168.2.0/24' in cidrs)
 
-    def test__validate_cidrs_split_cidr_smaller_subnet_in_use(self):
+    def test__validate_cidrs_smaller_subnet_in_use(self):
+        manager = self.FakeNetworkManager()
+        self.mox.StubOutWithMock(manager.db, 'network_get_all')
+        ctxt = mox.IgnoreArg()
+        manager.db.network_get_all(ctxt).AndReturn([{'id': 1,
+                                     'cidr': '192.168.2.9/25'}])
+        self.mox.ReplayAll()
+        # ValueError: requested cidr (192.168.2.0/24) conflicts with
+        #             existing smaller cidr
+        args = [None, '192.168.2.0/24', 1, 256]
+        self.assertRaises(ValueError, manager._validate_cidrs, *args)
+
+    def test__validate_cidrs_split_smaller_cidr_in_use(self):
         manager = self.FakeNetworkManager()
         self.mox.StubOutWithMock(manager.db, 'network_get_all')
         ctxt = mox.IgnoreArg()
@@ -329,6 +341,22 @@ class CommonNetworkTestCase(test.TestCase):
             self.assertTrue(exp_cidr + '/24' in cidrs)
         self.assertFalse('192.168.2.0/24' in cidrs)
 
+    def test__validate_cidrs_split_smaller_cidr_in_use2(self):
+        manager = self.FakeNetworkManager()
+        self.mox.StubOutWithMock(manager.db, 'network_get_all')
+        ctxt = mox.IgnoreArg()
+        manager.db.network_get_all(ctxt).AndReturn([{'id': 1,
+                                     'cidr': '192.168.2.9/29'}])
+        self.mox.ReplayAll()
+        nets = manager._validate_cidrs(None, '192.168.2.0/24', 3, 32)
+        self.assertEqual(3, len(nets))
+        cidrs = [str(net) for net in nets]
+        print cidrs
+        exp_cidrs = ['192.168.2.32', '192.168.2.64', '192.168.2.96']
+        for exp_cidr in exp_cidrs:
+            self.assertTrue(exp_cidr + '/27' in cidrs)
+        self.assertFalse('192.168.2.0/27' in cidrs)
+
     def test__validate_cidrs_one_in_use(self):
         manager = self.FakeNetworkManager()
         args = [None, '192.168.0.0/24', 2, 256]