From e72fafbf76ed456039426a96dd65d2c148dffa29 Mon Sep 17 00:00:00 2001 From: John Tran Date: Mon, 1 Aug 2011 12:37:12 -0700 Subject: adding a function with logic to make the creation of networks validation a bit smarter: - detects if the cidr is already in use - when specifying a supernet to be split into smaller subnets via num_networks && network_size, ensures none of the returned subnets are in use by either a subnet of the same size and range, nor a SMALLER size within the same range. - detects if splitting a supernet into # of num_networks && network_size will fit - detects if the supernet/cidr specified is conflicting with a network cidr that currently exists that may be a larger supernet already encompassing the specified cidr. " --- nova/network/manager.py | 48 +++++++++++++++++++++ nova/tests/test_network.py | 102 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 150 insertions(+) (limited to 'nova') diff --git a/nova/network/manager.py b/nova/network/manager.py index 4a3791d8a..fafb75c9e 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -609,6 +609,54 @@ 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: + raise ValueError(_("network_size * num_networks exceeds cidr size")) + adjusted_cidr = netaddr.IPNetwork(req_net_ip+'/'+str(significant_bits)) + all_req_nets = [adjusted_cidr] + 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: + raise ValueError(_("requested cidr (%s) conflicts with existing supernet (%s)" % (str(adjusted_cidr), str(adjusted_cidr_supernet)))) + # split supernet into subnets + if num_networks >= 2: + 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): + while True: + next_cidr = next_cidr.next() + if next_cidr in used_cidrs: + continue + else: + all_req_nets.append(next_cidr) + break + all_req_nets = list(set(all_req_nets)) + if not used_nets: + return all_req_nets + # after splitting ensure there were enough to satisfy the num_networks + if len(all_req_nets) < num_networks: + raise ValueError(_("Not enough subnets avail to satisfy requested num_networks")) + # if one of the split subnets were already defined, remove from create list + for req_cidr in all_req_nets: + if req_cidr in used_cidrs: + all_req_nets.remove(req_cidr) + 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): diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py index 28f50d328..8a851cf76 100644 --- a/nova/tests/test_network.py +++ b/nova/tests/test_network.py @@ -15,6 +15,7 @@ # License for the specific language governing permissions and limitations # under the License. +from nova import context from nova import db from nova import exception from nova import flags @@ -249,6 +250,15 @@ 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): + return {'foo': 'bar'} + + def network_get_all(self, context): + raise exception.NoNetworksFound() + def __init__(self): self.db = self.FakeDB() self.deallocate_called = None @@ -267,3 +277,95 @@ 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', '192.168.1.0', '192.168.3.0', '192.168.4.0'] + for exp_cidr in exp_cidrs: + 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_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/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', '192.168.1.0', '192.168.3.0', '192.168.4.0'] + for exp_cidr in exp_cidrs: + self.assertTrue(exp_cidr+'/24' in cidrs) + self.assertFalse('192.168.2.0/24' in cidrs) + + 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] + print returned_cidrs + 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() + manager.db.network_get_all(ctxt).AndReturn([{'id': 1, 'cidr': '192.168.0.0/8'}]) + self.mox.ReplayAll() + args = [None, '192.168.0.0/24', 1, 256] + # ValueError: requested cidr (192.168.0.0/24) conflicts with existing supernet (192.0.0.0/8) + self.assertRaises(ValueError, manager._validate_cidrs, *args) +# +# def test_create_networks_cidr_already_used(self): +# mockany = self.mox.CreateMockAnything() +# manager = self.FakeNetworkManager() +# self.mox.StubOutWithMock(manager.db, 'network_get_by_cidr') +# manager.db.network_get_by_cidr(mox.IgnoreArg(), '192.168.0.0/24').AndReturn(mockany) +# 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) -- cgit From 951114be20065044e7f12e37188eb30e859ff2cb Mon Sep 17 00:00:00 2001 From: John Tran Date: Mon, 1 Aug 2011 12:47:41 -0700 Subject: removed redundant logic --- nova/network/manager.py | 6 ------ 1 file changed, 6 deletions(-) (limited to 'nova') diff --git a/nova/network/manager.py b/nova/network/manager.py index fafb75c9e..f6c7e35e0 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -646,15 +646,9 @@ class NetworkManager(manager.SchedulerDependentManager): all_req_nets.append(next_cidr) break all_req_nets = list(set(all_req_nets)) - if not used_nets: - return all_req_nets # after splitting ensure there were enough to satisfy the num_networks if len(all_req_nets) < num_networks: raise ValueError(_("Not enough subnets avail to satisfy requested num_networks")) - # if one of the split subnets were already defined, remove from create list - for req_cidr in all_req_nets: - if req_cidr in used_cidrs: - all_req_nets.remove(req_cidr) return all_req_nets def create_networks(self, context, label, cidr, multi_host, num_networks, -- cgit From ab4bfcf6c458ab6bf6ead126a91413b92aa543b8 Mon Sep 17 00:00:00 2001 From: John Tran Date: Mon, 1 Aug 2011 16:27:17 -0700 Subject: pep8 fixes --- nova/network/manager.py | 12 ++++++++---- nova/tests/test_network.py | 36 +++++++++++++++++++++++------------- 2 files changed, 31 insertions(+), 17 deletions(-) (limited to 'nova') diff --git a/nova/network/manager.py b/nova/network/manager.py index f6c7e35e0..9d0f080ba 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -615,8 +615,10 @@ class NetworkManager(manager.SchedulerDependentManager): req_net_ip = str(req_net.ip) req_size = network_size * num_networks if req_size > req_net.size: - raise ValueError(_("network_size * num_networks exceeds cidr size")) - adjusted_cidr = netaddr.IPNetwork(req_net_ip+'/'+str(significant_bits)) + 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) all_req_nets = [adjusted_cidr] try: used_nets = self.db.network_get_all(context) @@ -627,7 +629,8 @@ class NetworkManager(manager.SchedulerDependentManager): raise ValueError(_("cidr already in use")) for adjusted_cidr_supernet in adjusted_cidr.supernet(): if adjusted_cidr_supernet in used_cidrs: - raise ValueError(_("requested cidr (%s) conflicts with existing supernet (%s)" % (str(adjusted_cidr), str(adjusted_cidr_supernet)))) + msg = "requested cidr (%s) conflicts with existing supernet" + raise ValueError(_(msg % str(adjusted_cidr))) # split supernet into subnets if num_networks >= 2: next_cidr = adjusted_cidr @@ -648,7 +651,8 @@ class NetworkManager(manager.SchedulerDependentManager): all_req_nets = list(set(all_req_nets)) # after splitting ensure there were enough to satisfy the num_networks if len(all_req_nets) < num_networks: - raise ValueError(_("Not enough subnets avail to satisfy requested num_networks")) + msg = "Not enough subnets avail to satisfy requested num_networks" + raise ValueError(_(msg)) return all_req_nets def create_networks(self, context, label, cidr, multi_host, num_networks, diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py index 8a851cf76..1f5305b1d 100644 --- a/nova/tests/test_network.py +++ b/nova/tests/test_network.py @@ -297,28 +297,32 @@ class CommonNetworkTestCase(test.TestCase): 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'}]) + 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', '192.168.1.0', '192.168.3.0', '192.168.4.0'] + exp_cidrs = ['192.168.0.0', '192.168.1.0', '192.168.3.0', + '192.168.4.0'] for exp_cidr in exp_cidrs: - self.assertTrue(exp_cidr+'/24' in cidrs) + 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_middle_of_range(self): + def test__validate_cidrs_split_cidr_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.0/25'}]) + 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', '192.168.1.0', '192.168.3.0', '192.168.4.0'] + exp_cidrs = ['192.168.0.0', '192.168.1.0', '192.168.3.0', + '192.168.4.0'] for exp_cidr in exp_cidrs: - self.assertTrue(exp_cidr+'/24' in cidrs) + self.assertTrue(exp_cidr + '/24' in cidrs) self.assertFalse('192.168.2.0/24' in cidrs) def test__validate_cidrs_one_in_use(self): @@ -331,7 +335,8 @@ class CommonNetworkTestCase(test.TestCase): 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'}]) + 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] @@ -340,7 +345,8 @@ class CommonNetworkTestCase(test.TestCase): 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 + # ValueError: Not enough subnets avail to satisfy requested + # num_networks self.assertRaises(ValueError, manager._validate_cidrs, *args) def test__validate_cidrs_split_partial(self): @@ -355,17 +361,21 @@ class CommonNetworkTestCase(test.TestCase): 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/8'}]) + manager.db.network_get_all(ctxt).AndReturn([{'id': 1, + 'cidr': '192.168.0.0/8'}]) self.mox.ReplayAll() args = [None, '192.168.0.0/24', 1, 256] - # ValueError: requested cidr (192.168.0.0/24) conflicts with existing supernet (192.0.0.0/8) + # ValueError: requested cidr (192.168.0.0/24) conflicts + # with existing supernet self.assertRaises(ValueError, manager._validate_cidrs, *args) # # def test_create_networks_cidr_already_used(self): # mockany = self.mox.CreateMockAnything() # manager = self.FakeNetworkManager() # self.mox.StubOutWithMock(manager.db, 'network_get_by_cidr') -# manager.db.network_get_by_cidr(mox.IgnoreArg(), '192.168.0.0/24').AndReturn(mockany) +# manager.db.network_get_by_cidr(mox.IgnoreArg(), '192.168.0.0/24')\ +# .AndReturn(mockany) # self.mox.ReplayAll() -# args = [None, 'foo', '192.168.0.0/24', None, 1, 256, 'fd00::/48', None, None, None] +# args = [None, 'foo', '192.168.0.0/24', None, 1, 256, +# 'fd00::/48', None, None, None] # self.assertRaises(ValueError, manager.create_networks, *args) -- cgit From bcfd8f5e1e0c3b53a2ad4a5bb533d94dcf5ef18c Mon Sep 17 00:00:00 2001 From: John Tran Date: Mon, 1 Aug 2011 21:34:43 -0700 Subject: added some tests for network create & moved the ipv6 logic back into the function --- nova/network/manager.py | 43 ++++++++++++++++++------------------------- nova/tests/test_network.py | 41 ++++++++++++++++++++++++++++------------- 2 files changed, 46 insertions(+), 38 deletions(-) (limited to 'nova') diff --git a/nova/network/manager.py b/nova/network/manager.py index 9d0f080ba..8b86bc036 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -659,71 +659,64 @@ class NetworkManager(manager.SchedulerDependentManager): network_size, cidr_v6, gateway_v6, bridge, bridge_interface, dns1=None, dns2=None, **kwargs): """Create networks based on parameters.""" - fixed_net = netaddr.IPNetwork(cidr) + req_cidrs = self._validate_cidrs(context, cidr, num_networks, network_size) + if FLAGS.use_ipv6: fixed_net_v6 = netaddr.IPNetwork(cidr_v6) significant_bits_v6 = 64 network_size_v6 = 1 << 64 - for index in range(num_networks): - start = index * network_size - significant_bits = 32 - int(math.log(network_size, 2)) - cidr = '%s/%s' % (fixed_net[start], significant_bits) - project_net = netaddr.IPNetwork(cidr) + count = 0 + for req_cidr in req_cidrs: + count = count + 1 net = {} net['bridge'] = bridge net['bridge_interface'] = bridge_interface net['dns1'] = dns1 net['dns2'] = dns2 - net['cidr'] = cidr - net['multi_host'] = multi_host - net['netmask'] = str(project_net.netmask) - net['gateway'] = str(project_net[1]) - net['broadcast'] = str(project_net.broadcast) - net['dhcp_start'] = str(project_net[2]) + net['cidr'] = str(req_cidr) + net['netmask'] = str(req_cidr.netmask) + net['gateway'] = str(req_cidr[1]) + net['broadcast'] = str(req_cidr.broadcast) + net['dhcp_start'] = str(req_cidr[2]) if num_networks > 1: - net['label'] = '%s_%d' % (label, index) + net['label'] = '%s_%d' % (label, count) else: net['label'] = label if FLAGS.use_ipv6: - start_v6 = index * network_size_v6 + start_v6 = count * network_size_v6 cidr_v6 = '%s/%s' % (fixed_net_v6[start_v6], significant_bits_v6) net['cidr_v6'] = cidr_v6 - project_net_v6 = netaddr.IPNetwork(cidr_v6) - if gateway_v6: # use a pre-defined gateway if one is provided net['gateway_v6'] = str(gateway_v6) else: net['gateway_v6'] = str(project_net_v6[1]) - net['netmask_v6'] = str(project_net_v6._prefixlen) if kwargs.get('vpn', False): # this bit here is for vlan-manager del net['dns1'] del net['dns2'] - vlan = kwargs['vlan_start'] + index - net['vpn_private_address'] = str(project_net[2]) - net['dhcp_start'] = str(project_net[3]) + vlan = kwargs['vlan_start'] + count + net['vpn_private_address'] = str(req_cidr[2]) + net['dhcp_start'] = str(req_cidr[3]) net['vlan'] = vlan net['bridge'] = 'br%s' % vlan # NOTE(vish): This makes ports unique accross the cloud, a more # robust solution would be to make them uniq per ip - net['vpn_public_port'] = kwargs['vpn_start'] + index + net['vpn_public_port'] = kwargs['vpn_start'] + count - # None if network with cidr or cidr_v6 already exists network = self.db.network_create_safe(context, net) - if network: self._create_fixed_ips(context, network['id']) else: - raise ValueError(_('Network with cidr %s already exists') % - cidr) + msg = "Error creating cidr %s, see log" + raise ValueError(_(msg) % str(cidr)) @property def _bottom_reserved_ips(self): # pylint: disable=R0201 diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py index 1f5305b1d..424b5f098 100644 --- a/nova/tests/test_network.py +++ b/nova/tests/test_network.py @@ -361,21 +361,36 @@ class CommonNetworkTestCase(test.TestCase): 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/8'}]) + 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_cidr_already_used(self): -# mockany = self.mox.CreateMockAnything() -# manager = self.FakeNetworkManager() -# self.mox.StubOutWithMock(manager.db, 'network_get_by_cidr') -# manager.db.network_get_by_cidr(mox.IgnoreArg(), '192.168.0.0/24')\ -# .AndReturn(mockany) -# 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(self): + cidr = '192.168.0.0/24' + manager = self.FakeNetworkManager() + self.mox.StubOutWithMock(manager.db, 'network_create_safe') + ignore = mox.IgnoreArg() + fakecidr= {'id': 1, 'cidr': cidr} + manager.db.network_create_safe(ignore, ignore).AndReturn(fakecidr) + self.mox.StubOutWithMock(manager, '_create_fixed_ips') + manager._create_fixed_ips(ignore, ignore).AndReturn('foo') + self.mox.ReplayAll() + args = [None, 'foo', cidr, None, 1, 256, 'fd00::/48', None, None, + None] + # making it to the end, without err, is enough validation + 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) -- cgit From 51f0cbf9221b461eb92beae2497e871bf2a7f45f Mon Sep 17 00:00:00 2001 From: John Tran Date: Tue, 2 Aug 2011 10:06:22 -0700 Subject: refactored tests --- nova/network/manager.py | 13 ++++++------- nova/tests/test_network.py | 27 ++++++++++++++++++--------- 2 files changed, 24 insertions(+), 16 deletions(-) (limited to 'nova') diff --git a/nova/network/manager.py b/nova/network/manager.py index 8b86bc036..b21667def 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -666,9 +666,8 @@ class NetworkManager(manager.SchedulerDependentManager): significant_bits_v6 = 64 network_size_v6 = 1 << 64 - count = 0 - for req_cidr in req_cidrs: - count = count + 1 + for index in range(len(req_cidrs)): + req_cidr = req_cidrs[index] net = {} net['bridge'] = bridge net['bridge_interface'] = bridge_interface @@ -680,12 +679,12 @@ class NetworkManager(manager.SchedulerDependentManager): net['broadcast'] = str(req_cidr.broadcast) net['dhcp_start'] = str(req_cidr[2]) if num_networks > 1: - net['label'] = '%s_%d' % (label, count) + net['label'] = '%s_%d' % (label, index) else: net['label'] = label if FLAGS.use_ipv6: - start_v6 = count * network_size_v6 + start_v6 = index * network_size_v6 cidr_v6 = '%s/%s' % (fixed_net_v6[start_v6], significant_bits_v6) net['cidr_v6'] = cidr_v6 @@ -701,7 +700,7 @@ class NetworkManager(manager.SchedulerDependentManager): # this bit here is for vlan-manager del net['dns1'] del net['dns2'] - vlan = kwargs['vlan_start'] + count + vlan = kwargs['vlan_start'] + index net['vpn_private_address'] = str(req_cidr[2]) net['dhcp_start'] = str(req_cidr[3]) net['vlan'] = vlan @@ -709,7 +708,7 @@ class NetworkManager(manager.SchedulerDependentManager): # NOTE(vish): This makes ports unique accross the cloud, a more # robust solution would be to make them uniq per ip - net['vpn_public_port'] = kwargs['vpn_start'] + count + net['vpn_public_port'] = kwargs['vpn_start'] + index network = self.db.network_create_safe(context, net) if network: diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py index 424b5f098..9c808c6db 100644 --- a/nova/tests/test_network.py +++ b/nova/tests/test_network.py @@ -254,7 +254,9 @@ class CommonNetworkTestCase(test.TestCase): raise exception.NetworkNotFoundForCidr() def network_create_safe(self, context, net): - return {'foo': 'bar'} + fakenet = {} + fakenet['id'] = 999 + return fakenet def network_get_all(self, context): raise exception.NoNetworksFound() @@ -266,6 +268,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') @@ -372,16 +377,11 @@ class CommonNetworkTestCase(test.TestCase): def test_create_networks(self): cidr = '192.168.0.0/24' manager = self.FakeNetworkManager() - self.mox.StubOutWithMock(manager.db, 'network_create_safe') - ignore = mox.IgnoreArg() - fakecidr= {'id': 1, 'cidr': cidr} - manager.db.network_create_safe(ignore, ignore).AndReturn(fakecidr) - self.mox.StubOutWithMock(manager, '_create_fixed_ips') - manager._create_fixed_ips(ignore, ignore).AndReturn('foo') - self.mox.ReplayAll() + self.stubs.Set(manager, '_create_fixed_ips', + self.fake_create_fixed_ips) args = [None, 'foo', cidr, None, 1, 256, 'fd00::/48', None, None, None] - # making it to the end, without err, is enough validation + result = manager.create_networks(*args) self.assertEqual(manager.create_networks(*args), None) def test_create_networks_cidr_already_used(self): @@ -394,3 +394,12 @@ class CommonNetworkTestCase(test.TestCase): 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) -- cgit From 8c7b71f65e54d67615e52927591e12a43b8b3991 Mon Sep 17 00:00:00 2001 From: John Tran Date: Thu, 4 Aug 2011 16:05:08 -0700 Subject: re-integrated my changes after merging trunk. fixed some pep8 issues. sorting the list of cidrs to create, so that it will create x.x.0.0 with a lower 'id' than x.x.1.0 (as an example). <- was causing libvirtd test to fail --- nova/network/manager.py | 54 +++++++++++++++++++++++++++++++++++++++++----- nova/tests/test_network.py | 6 +++--- 2 files changed, 52 insertions(+), 8 deletions(-) (limited to 'nova') diff --git a/nova/network/manager.py b/nova/network/manager.py index 8fc6a295f..3c29417d7 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -614,6 +614,52 @@ 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) + all_req_nets = [adjusted_cidr] + 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))) + # split supernet into subnets + if num_networks >= 2: + 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): + while True: + next_cidr = next_cidr.next() + if next_cidr in used_cidrs: + continue + else: + all_req_nets.append(next_cidr) + break + all_req_nets = sorted(list(set(all_req_nets))) + # after splitting ensure there were enough to satisfy the num_networks + if len(all_req_nets) < num_networks: + msg = "Not enough subnets avail to satisfy requested num_networks" + raise ValueError(_(msg)) + 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 +670,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 +681,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 9c808c6db..ceb12e890 100644 --- a/nova/tests/test_network.py +++ b/nova/tests/test_network.py @@ -269,7 +269,7 @@ class CommonNetworkTestCase(test.TestCase): self.deallocate_called = address def fake_create_fixed_ips(self, context, network_id): - return None + return None def test_remove_fixed_ip_from_instance(self): manager = self.FakeNetworkManager() @@ -366,7 +366,7 @@ class CommonNetworkTestCase(test.TestCase): manager = self.FakeNetworkManager() self.mox.StubOutWithMock(manager.db, 'network_get_all') ctxt = mox.IgnoreArg() - fakecidr= [{'id': 1, 'cidr': '192.168.0.0/8'}] + 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] @@ -388,7 +388,7 @@ class CommonNetworkTestCase(test.TestCase): manager = self.FakeNetworkManager() self.mox.StubOutWithMock(manager.db, 'network_get_all') ctxt = mox.IgnoreArg() - fakecidr= [{'id': 1, 'cidr': '192.168.0.0/24'}] + 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, -- cgit From f58d441b55e143de35aefd039b80e0b27dad9ce2 Mon Sep 17 00:00:00 2001 From: John Tran Date: Thu, 4 Aug 2011 16:27:55 -0700 Subject: removed unnecessary context from test I had left there from prior --- nova/tests/test_network.py | 1 - 1 file changed, 1 deletion(-) (limited to 'nova') diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py index ceb12e890..1ff3f0c01 100644 --- a/nova/tests/test_network.py +++ b/nova/tests/test_network.py @@ -15,7 +15,6 @@ # License for the specific language governing permissions and limitations # under the License. -from nova import context from nova import db from nova import exception from nova import flags -- cgit From 89ec28c70d7795d427ecd4242cb1856eabdca104 Mon Sep 17 00:00:00 2001 From: John Tran Date: Thu, 4 Aug 2011 18:01:07 -0700 Subject: fixed bug, wasn't detecting smaller subnet conflict properly added test for it --- nova/network/manager.py | 35 ++++++++++++++++++++++++----------- nova/tests/test_network.py | 30 +++++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 12 deletions(-) (limited to 'nova') 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] -- cgit From 38eb72be5f15731ba34a7dc0f8a28aa0fb63ea90 Mon Sep 17 00:00:00 2001 From: John Tran Date: Thu, 4 Aug 2011 18:37:36 -0700 Subject: fixed bug , when logic searched for next avail cidr it would return cidrs that were out of range of original requested cidr block. added test for it --- nova/network/manager.py | 4 ++++ nova/tests/test_network.py | 17 +++++++++++++++-- 2 files changed, 19 insertions(+), 2 deletions(-) (limited to 'nova') diff --git a/nova/network/manager.py b/nova/network/manager.py index 873fcadf5..0bb107268 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -655,6 +655,10 @@ class NetworkManager(manager.SchedulerDependentManager): # 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: if next_cidr in used_cidrs: next_cidr = next_cidr.next() diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py index e3a677c97..b4c5a7584 100644 --- a/nova/tests/test_network.py +++ b/nova/tests/test_network.py @@ -351,12 +351,26 @@ class CommonNetworkTestCase(test.TestCase): 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_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] @@ -385,7 +399,6 @@ class CommonNetworkTestCase(test.TestCase): manager = self.FakeNetworkManager() nets = manager._validate_cidrs(None, '192.168.0.0/16', 2, 256) returned_cidrs = [str(net) for net in nets] - print returned_cidrs self.assertTrue('192.168.0.0/24' in returned_cidrs) self.assertTrue('192.168.1.0/24' in returned_cidrs) -- cgit From b557b6366b21a0d3795369785037ee29c8cef377 Mon Sep 17 00:00:00 2001 From: John Tran Date: Thu, 4 Aug 2011 18:52:15 -0700 Subject: fix pep8 issues --- nova/network/manager.py | 14 +++++++++----- nova/tests/test_network.py | 5 +++-- 2 files changed, 12 insertions(+), 7 deletions(-) (limited to 'nova') diff --git a/nova/network/manager.py b/nova/network/manager.py index 0bb107268..1b92effb2 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -647,7 +647,8 @@ class NetworkManager(manager.SchedulerDependentManager): all_req_nets = [] if num_networks == 1: if adjusted_cidr in used_supernets: - msg = "requested cidr (%s) conflicts with existing smaller cidr" + msg = "requested cidr (%s) conflicts with existing smaller" \ + " cidr" raise ValueError(_(msg % str(adjusted_cidr))) else: all_req_nets.append(adjusted_cidr) @@ -656,8 +657,9 @@ class NetworkManager(manager.SchedulerDependentManager): 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" + msg = "Not enough subnets avail to satisfy requested " \ + "num_net works - some subnets in requested range" \ + " already in use" raise ValueError(_(msg)) while True: if next_cidr in used_cidrs: @@ -671,9 +673,11 @@ class NetworkManager(manager.SchedulerDependentManager): 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 + # after splitting ensure there were enough to satisfy the + # num_networks if len(all_req_nets) < num_networks: - msg = "Not enough subnets avail to satisfy requested num_networks" + msg = "Not enough subnets avail to satisfy requested " \ + "num_networks" raise ValueError(_(msg)) return all_req_nets diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py index b4c5a7584..8c84ea938 100644 --- a/nova/tests/test_network.py +++ b/nova/tests/test_network.py @@ -367,8 +367,9 @@ class CommonNetworkTestCase(test.TestCase): 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 + # 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): -- cgit From b7167b21d615f8617d588a1656aa341fd226ded9 Mon Sep 17 00:00:00 2001 From: John Tran Date: Thu, 4 Aug 2011 20:13:23 -0700 Subject: removed redundant logic --- nova/network/manager.py | 6 ------ 1 file changed, 6 deletions(-) (limited to 'nova') diff --git a/nova/network/manager.py b/nova/network/manager.py index 1b92effb2..3d915423f 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -673,12 +673,6 @@ class NetworkManager(manager.SchedulerDependentManager): 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 - if len(all_req_nets) < num_networks: - msg = "Not enough subnets avail to satisfy requested " \ - "num_networks" - raise ValueError(_(msg)) return all_req_nets def create_networks(self, context, label, cidr, multi_host, num_networks, -- cgit From 2935bebd718e770d0f2c9d1ab5dca76cc7d5f76a Mon Sep 17 00:00:00 2001 From: John Tran Date: Fri, 5 Aug 2011 09:50:11 -0700 Subject: fix typo --- nova/tests/test_network.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'nova') diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py index 40cc96147..1f27643e5 100644 --- a/nova/tests/test_network.py +++ b/nova/tests/test_network.py @@ -360,8 +360,7 @@ class CommonNetworkTestCase(test.TestCase): 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'} - ] + {'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] -- cgit From 3812d22b7a6f5d74418a7a99dc69c68a5b9f9046 Mon Sep 17 00:00:00 2001 From: John Tran Date: Fri, 5 Aug 2011 11:36:00 -0700 Subject: fixed per peer review --- nova/network/manager.py | 27 ++++++++++++--------------- nova/tests/test_network.py | 40 ++++++++++++++++++++-------------------- 2 files changed, 32 insertions(+), 35 deletions(-) (limited to 'nova') diff --git a/nova/network/manager.py b/nova/network/manager.py index 3d915423f..5677df670 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -620,8 +620,8 @@ class NetworkManager(manager.SchedulerDependentManager): 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)) + 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: @@ -633,8 +633,8 @@ class NetworkManager(manager.SchedulerDependentManager): 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))) + 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: @@ -647,9 +647,9 @@ class NetworkManager(manager.SchedulerDependentManager): 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))) + 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: @@ -657,17 +657,14 @@ class NetworkManager(manager.SchedulerDependentManager): 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 " \ + msg = ("Not enough subnets avail to satisfy requested " \ "num_net works - some subnets in requested range" \ - " already in use" - raise ValueError(_(msg)) + " already in use") + raise ValueError(msg) while True: - if next_cidr in used_cidrs: + used_values = used_cidrs + used_supernets + if next_cidr in used_values: 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() diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py index 1f27643e5..547a7a1fa 100644 --- a/nova/tests/test_network.py +++ b/nova/tests/test_network.py @@ -280,14 +280,14 @@ class CommonNetworkTestCase(test.TestCase): manager.remove_fixed_ip_from_instance, None, 99, 'bad input') - def test__validate_cidrs(self): + 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): + 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)) @@ -295,7 +295,7 @@ class CommonNetworkTestCase(test.TestCase): 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): + 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() @@ -305,13 +305,13 @@ class CommonNetworkTestCase(test.TestCase): 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', '192.168.1.0', '192.168.3.0', - '192.168.4.0'] + 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 + '/24' in 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): + def test_validate_cidrs_smaller_subnet_in_use(self): manager = self.FakeNetworkManager() self.mox.StubOutWithMock(manager.db, 'network_get_all') ctxt = mox.IgnoreArg() @@ -323,7 +323,7 @@ class CommonNetworkTestCase(test.TestCase): 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): + def test_validate_cidrs_split_smaller_cidr_in_use(self): manager = self.FakeNetworkManager() self.mox.StubOutWithMock(manager.db, 'network_get_all') ctxt = mox.IgnoreArg() @@ -333,13 +333,13 @@ class CommonNetworkTestCase(test.TestCase): 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', '192.168.1.0', '192.168.3.0', - '192.168.4.0'] + 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 + '/24' in 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): + def test_validate_cidrs_split_smaller_cidr_in_use2(self): manager = self.FakeNetworkManager() self.mox.StubOutWithMock(manager.db, 'network_get_all') ctxt = mox.IgnoreArg() @@ -349,12 +349,12 @@ class CommonNetworkTestCase(test.TestCase): 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', '192.168.2.64', '192.168.2.96'] + 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 + '/27' in 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): + def test_validate_cidrs_split_all_in_use(self): manager = self.FakeNetworkManager() self.mox.StubOutWithMock(manager.db, 'network_get_all') ctxt = mox.IgnoreArg() @@ -369,13 +369,13 @@ class CommonNetworkTestCase(test.TestCase): # in use self.assertRaises(ValueError, manager._validate_cidrs, *args) - def test__validate_cidrs_one_in_use(self): + 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): + def test_validate_cidrs_already_used(self): manager = self.FakeNetworkManager() self.mox.StubOutWithMock(manager.db, 'network_get_all') ctxt = mox.IgnoreArg() @@ -386,21 +386,21 @@ class CommonNetworkTestCase(test.TestCase): args = [None, '192.168.0.0/24', 1, 256] self.assertRaises(ValueError, manager._validate_cidrs, *args) - def test__validate_cidrs_too_many(self): + 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): + 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): + def test_validate_cidrs_conflict_existing_supernet(self): manager = self.FakeNetworkManager() self.mox.StubOutWithMock(manager.db, 'network_get_all') ctxt = mox.IgnoreArg() -- cgit From 353fa4871069cf0b926f09aa00496002f65584cb Mon Sep 17 00:00:00 2001 From: John Tran Date: Fri, 5 Aug 2011 11:48:06 -0700 Subject: fixed per peer review --- nova/network/manager.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'nova') diff --git a/nova/network/manager.py b/nova/network/manager.py index 5677df670..402049d44 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -647,8 +647,8 @@ class NetworkManager(manager.SchedulerDependentManager): all_req_nets = [] if num_networks == 1: if adjusted_cidr in used_supernets: - msg = _("requested cidr (%s) conflicts with existing smaller" \ - " cidr") + msg = _("requested cidr (%s) conflicts with existing smaller" + " cidr") raise ValueError(msg % str(adjusted_cidr)) else: all_req_nets.append(adjusted_cidr) @@ -657,9 +657,9 @@ class NetworkManager(manager.SchedulerDependentManager): 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") + 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 -- cgit