added logic to make the creation of networks (IPv4 only) validation a bit smarter:

  - detects if the cidr is already in use
  - detects if any existing smaller networks are within the range of requested cidr(s)
  - detects if splitting a supernet into # of num_networks && network_size will fit
  - detects if requested cidr(s) are within range of already existing supernet (larger cidr).

IPv6 logic remains intact yet had not been improved by this code.

2 files changed, 237 insertions, 5 deletions

diff --git a/nova/network/manager.py b/nova/network/manager.py
index 8fc6a295f..402049d44 100644
--- a/nova/network/manager.py
+++ b/nova/network/manager.py
@@ -614,6 +614,64 @@ class NetworkManager(manager.SchedulerDependentManager):
                 network_ref = self.db.fixed_ip_get_network(context, address)
                 self._setup_network(context, network_ref)
 
+    def _validate_cidrs(self, context, cidr, num_networks, network_size):
+        significant_bits = 32 - int(math.log(network_size, 2))
+        req_net = netaddr.IPNetwork(cidr)
+        req_net_ip = str(req_net.ip)
+        req_size = network_size * num_networks
+        if req_size > req_net.size:
+            msg = _("network_size * num_networks exceeds cidr size")
+            raise ValueError(msg)
+        adjusted_cidr_str = req_net_ip + '/' + str(significant_bits)
+        adjusted_cidr = netaddr.IPNetwork(adjusted_cidr_str)
+        try:
+            used_nets = self.db.network_get_all(context)
+        except exception.NoNetworksFound:
+            used_nets = []
+        used_cidrs = [netaddr.IPNetwork(net['cidr']) for net in used_nets]
+        if adjusted_cidr in used_cidrs:
+            raise ValueError(_("cidr already in use"))
+        for adjusted_cidr_supernet in adjusted_cidr.supernet():
+            if adjusted_cidr_supernet in used_cidrs:
+                msg = _("requested cidr (%s) conflicts with existing supernet")
+                raise ValueError(msg % str(adjusted_cidr))
+        # 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 index in range(num_networks):
+                if next_cidr.first > req_net.last:
+                    msg = _("Not enough subnets avail to satisfy requested "
+                            "num_net works - some subnets in requested range"
+                            " already in use")
+                    raise ValueError(msg)
+                while True:
+                    used_values = used_cidrs + used_supernets
+                    if next_cidr in used_values:
+                        next_cidr = next_cidr.next()
+                    else:
+                        all_req_nets.append(next_cidr)
+                        next_cidr = next_cidr.next()
+                        break
+        all_req_nets = sorted(list(set(all_req_nets)))
+        return all_req_nets
+
     def create_networks(self, context, label, cidr, multi_host, num_networks,
                         network_size, cidr_v6, gateway_v6, bridge,
                         bridge_interface, dns1=None, dns2=None, **kwargs):
@@ -624,8 +682,8 @@ class NetworkManager(manager.SchedulerDependentManager):
             network_size_v6 = 1 << 64
 
         if cidr:
-            fixed_net = netaddr.IPNetwork(cidr)
-            significant_bits = 32 - int(math.log(network_size, 2))
+            req_cidrs = self._validate_cidrs(context, cidr, num_networks,
+                                             network_size)
 
         for index in range(num_networks):
             net = {}
@@ -635,9 +693,7 @@ class NetworkManager(manager.SchedulerDependentManager):
             net['dns2'] = dns2
 
             if cidr:
-                start = index * network_size
-                project_net = netaddr.IPNetwork('%s/%s' % (fixed_net[start],
-                                                           significant_bits))
+                project_net = req_cidrs[index]
                 net['cidr'] = str(project_net)
                 net['multi_host'] = multi_host
                 net['netmask'] = str(project_net.netmask)
diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py
index 2ca8b64f4..547a7a1fa 100644
--- a/nova/tests/test_network.py
+++ b/nova/tests/test_network.py
@@ -247,6 +247,17 @@ class CommonNetworkTestCase(test.TestCase):
                 return [dict(address='10.0.0.0'),  dict(address='10.0.0.1'),
                         dict(address='10.0.0.2')]
 
+            def network_get_by_cidr(self, context, cidr):
+                raise exception.NetworkNotFoundForCidr()
+
+            def network_create_safe(self, context, net):
+                fakenet = {}
+                fakenet['id'] = 999
+                return fakenet
+
+            def network_get_all(self, context):
+                raise exception.NoNetworksFound()
+
         def __init__(self):
             self.db = self.FakeDB()
             self.deallocate_called = None
@@ -254,6 +265,9 @@ class CommonNetworkTestCase(test.TestCase):
         def deallocate_fixed_ip(self, context, address):
             self.deallocate_called = address
 
+    def fake_create_fixed_ips(self, context, network_id):
+        return None
+
     def test_remove_fixed_ip_from_instance(self):
         manager = self.FakeNetworkManager()
         manager.remove_fixed_ip_from_instance(None, 99, '10.0.0.1')
@@ -265,3 +279,165 @@ class CommonNetworkTestCase(test.TestCase):
         self.assertRaises(exception.FixedIpNotFoundForSpecificInstance,
                           manager.remove_fixed_ip_from_instance,
                           None, 99, 'bad input')
+
+    def test_validate_cidrs(self):
+        manager = self.FakeNetworkManager()
+        nets = manager._validate_cidrs(None, '192.168.0.0/24', 1, 256)
+        self.assertEqual(1, len(nets))
+        cidrs = [str(net) for net in nets]
+        self.assertTrue('192.168.0.0/24' in cidrs)
+
+    def test_validate_cidrs_split_exact_in_half(self):
+        manager = self.FakeNetworkManager()
+        nets = manager._validate_cidrs(None, '192.168.0.0/24', 2, 128)
+        self.assertEqual(2, len(nets))
+        cidrs = [str(net) for net in nets]
+        self.assertTrue('192.168.0.0/25' in cidrs)
+        self.assertTrue('192.168.0.128/25' in cidrs)
+
+    def test_validate_cidrs_split_cidr_in_use_middle_of_range(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.0/24'}])
+        self.mox.ReplayAll()
+        nets = manager._validate_cidrs(None, '192.168.0.0/16', 4, 256)
+        self.assertEqual(4, len(nets))
+        cidrs = [str(net) for net in nets]
+        exp_cidrs = ['192.168.0.0/24', '192.168.1.0/24', '192.168.3.0/24',
+                     '192.168.4.0/24']
+        for exp_cidr in exp_cidrs:
+            self.assertTrue(exp_cidr in cidrs)
+        self.assertFalse('192.168.2.0/24' in cidrs)
+
+    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()
+        manager.db.network_get_all(ctxt).AndReturn([{'id': 1,
+                                     'cidr': '192.168.2.0/25'}])
+        self.mox.ReplayAll()
+        nets = manager._validate_cidrs(None, '192.168.0.0/16', 4, 256)
+        self.assertEqual(4, len(nets))
+        cidrs = [str(net) for net in nets]
+        exp_cidrs = ['192.168.0.0/24', '192.168.1.0/24', '192.168.3.0/24',
+                     '192.168.4.0/24']
+        for exp_cidr in exp_cidrs:
+            self.assertTrue(exp_cidr 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]
+        exp_cidrs = ['192.168.2.32/27', '192.168.2.64/27', '192.168.2.96/27']
+        for exp_cidr in exp_cidrs:
+            self.assertTrue(exp_cidr in cidrs)
+        self.assertFalse('192.168.2.0/27' in cidrs)
+
+    def test_validate_cidrs_split_all_in_use(self):
+        manager = self.FakeNetworkManager()
+        self.mox.StubOutWithMock(manager.db, 'network_get_all')
+        ctxt = mox.IgnoreArg()
+        in_use = [{'id': 1, 'cidr': '192.168.2.9/29'},
+                  {'id': 2, 'cidr': '192.168.2.64/26'},
+                  {'id': 3, 'cidr': '192.168.2.128/26'}]
+        manager.db.network_get_all(ctxt).AndReturn(in_use)
+        self.mox.ReplayAll()
+        args = [None, '192.168.2.0/24', 3, 64]
+        # ValueError: Not enough subnets avail to satisfy requested num_
+        #             networks - some subnets in requested range already
+        #             in use
+        self.assertRaises(ValueError, manager._validate_cidrs, *args)
+
+    def test_validate_cidrs_one_in_use(self):
+        manager = self.FakeNetworkManager()
+        args = [None, '192.168.0.0/24', 2, 256]
+        # ValueError: network_size * num_networks exceeds cidr size
+        self.assertRaises(ValueError, manager._validate_cidrs, *args)
+
+    def test_validate_cidrs_already_used(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.0.0/24'}])
+        self.mox.ReplayAll()
+        # ValueError: cidr already in use
+        args = [None, '192.168.0.0/24', 1, 256]
+        self.assertRaises(ValueError, manager._validate_cidrs, *args)
+
+    def test_validate_cidrs_too_many(self):
+        manager = self.FakeNetworkManager()
+        args = [None, '192.168.0.0/24', 200, 256]
+        # ValueError: Not enough subnets avail to satisfy requested
+        #             num_networks
+        self.assertRaises(ValueError, manager._validate_cidrs, *args)
+
+    def test_validate_cidrs_split_partial(self):
+        manager = self.FakeNetworkManager()
+        nets = manager._validate_cidrs(None, '192.168.0.0/16', 2, 256)
+        returned_cidrs = [str(net) for net in nets]
+        self.assertTrue('192.168.0.0/24' in returned_cidrs)
+        self.assertTrue('192.168.1.0/24' in returned_cidrs)
+
+    def test_validate_cidrs_conflict_existing_supernet(self):
+        manager = self.FakeNetworkManager()
+        self.mox.StubOutWithMock(manager.db, 'network_get_all')
+        ctxt = mox.IgnoreArg()
+        fakecidr = [{'id': 1, 'cidr': '192.168.0.0/8'}]
+        manager.db.network_get_all(ctxt).AndReturn(fakecidr)
+        self.mox.ReplayAll()
+        args = [None, '192.168.0.0/24', 1, 256]
+        # ValueError: requested cidr (192.168.0.0/24) conflicts
+        #             with existing supernet
+        self.assertRaises(ValueError, manager._validate_cidrs, *args)
+
+    def test_create_networks(self):
+        cidr = '192.168.0.0/24'
+        manager = self.FakeNetworkManager()
+        self.stubs.Set(manager, '_create_fixed_ips',
+                                self.fake_create_fixed_ips)
+        args = [None, 'foo', cidr, None, 1, 256, 'fd00::/48', None, None,
+                None]
+        result = manager.create_networks(*args)
+        self.assertEqual(manager.create_networks(*args), None)
+
+    def test_create_networks_cidr_already_used(self):
+        manager = self.FakeNetworkManager()
+        self.mox.StubOutWithMock(manager.db, 'network_get_all')
+        ctxt = mox.IgnoreArg()
+        fakecidr = [{'id': 1, 'cidr': '192.168.0.0/24'}]
+        manager.db.network_get_all(ctxt).AndReturn(fakecidr)
+        self.mox.ReplayAll()
+        args = [None, 'foo', '192.168.0.0/24', None, 1, 256,
+                 'fd00::/48', None, None, None]
+        self.assertRaises(ValueError, manager.create_networks, *args)
+
+    def test_create_networks_many(self):
+        cidr = '192.168.0.0/16'
+        manager = self.FakeNetworkManager()
+        self.stubs.Set(manager, '_create_fixed_ips',
+                                self.fake_create_fixed_ips)
+        args = [None, 'foo', cidr, None, 10, 256, 'fd00::/48', None, None,
+                None]
+        self.assertEqual(manager.create_networks(*args), None)