From bd0645153fb1b60a551c50c657a7837713da54a9 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Mon, 9 Aug 2010 15:34:05 -0700 Subject: initial cleanup of tests for network --- nova/network/model.py | 39 ++++++++------- nova/network/vpn.py | 26 ++++++---- nova/tests/network_unittest.py | 106 ++++++++++++++++++++++++++--------------- 3 files changed, 107 insertions(+), 64 deletions(-) diff --git a/nova/network/model.py b/nova/network/model.py index daac035e4..a70671632 100644 --- a/nova/network/model.py +++ b/nova/network/model.py @@ -141,7 +141,6 @@ class Vlan(datastore.BasicModel): class BaseNetwork(datastore.BasicModel): override_type = 'network' - NUM_STATIC_IPS = 3 # Network, Gateway, and CloudPipe @property def identifier(self): @@ -215,16 +214,19 @@ class BaseNetwork(datastore.BasicModel): @property def available(self): - # the .2 address is always CloudPipe - # and the top are for vpn clients - for idx in range(self.num_static_ips, len(self.network)-(1 + FLAGS.cnt_vpn_clients)): + for idx in range(self.num_bottom_reserved_ips, + len(self.network) - self.num_top_reserved_ips): address = str(self.network[idx]) if not address in self.hosts.keys(): yield address @property - def num_static_ips(self): - return BaseNetwork.NUM_STATIC_IPS + def num_bottom_reserved_ips(self): + return 2 # Network, Gateway + + @property + def num_top_reserved_ips(self): + return 1 # Broadcast def allocate_ip(self, user_id, project_id, mac): for address in self.available: @@ -306,9 +308,9 @@ class DHCPNetwork(BridgedNetwork): def __init__(self, *args, **kwargs): super(DHCPNetwork, self).__init__(*args, **kwargs) # logging.debug("Initing DHCPNetwork object...") - self.dhcp_listen_address = self.network[1] - self.dhcp_range_start = self.network[3] - self.dhcp_range_end = self.network[-(1 + FLAGS.cnt_vpn_clients)] + self.dhcp_listen_address = self.gateway + self.dhcp_range_start = self.network[self.num_bottom_reserved_ips] + self.dhcp_range_end = self.network[-self.num_top_reserved_ips] try: os.makedirs(FLAGS.networks_path) # NOTE(todd): I guess this is a lazy way to not have to check if the @@ -318,6 +320,16 @@ class DHCPNetwork(BridgedNetwork): except Exception, err: pass + @property + def num_bottom_reserved_ips(self): + # For cloudpipe + return super(DHCPNetwork, self).num_bottom_reserved_ips + 1 + + @property + def num_top_reserved_ips(self): + return super(DHCPNetwork, self).num_top_reserved_ips + \ + FLAGS.cnt_vpn_clients + def express(self, address=None): super(DHCPNetwork, self).express(address=address) if len(self.assigned) > 0: @@ -388,13 +400,6 @@ class PublicNetworkController(BaseNetwork): self.save() self.express() - @property - def available(self): - for idx in range(2, len(self.network)-1): - address = str(self.network[idx]) - if not address in self.hosts.keys(): - yield address - @property def host_objs(self): for address in self.assigned: @@ -415,7 +420,7 @@ class PublicNetworkController(BaseNetwork): def deallocate_ip(self, ip_str): # NOTE(vish): cleanup is now done on release by the parent class - self.release_ip(ip_str) + self.release_ip(ip_str) def associate_address(self, public_ip, private_ip, instance_id): if not public_ip in self.assigned: diff --git a/nova/network/vpn.py b/nova/network/vpn.py index cec84287c..1b6dd7a56 100644 --- a/nova/network/vpn.py +++ b/nova/network/vpn.py @@ -74,23 +74,31 @@ class NetworkData(datastore.BasicModel): # similar to an association, but we are just # storing a set of values instead of keys that # should be turned into objects. - redis = datastore.Redis.instance() - key = 'ip:%s:ports' % ip - # TODO(vish): these ports should be allocated through an admin - # command instead of a flag - if (not redis.exists(key) and - not redis.exists(cls._redis_association_name('ip', ip))): - for i in range(FLAGS.vpn_start_port, FLAGS.vpn_end_port + 1): - redis.sadd(key, i) + cls._ensure_set_exists(ip) - port = redis.spop(key) + port = datastore.Redis.instance().spop(cls._redis_ports_key(ip)) if not port: raise NoMorePorts() return port + @classmethod + def _redis_ports_key(cls, ip): + return 'ip:%s:ports' % ip + + @classmethod + def _ensure_set_exists(cls, ip): + # TODO(vish): these ports should be allocated through an admin + # command instead of a flag + redis = datastore.Redis.instance() + if (not redis.exists(cls._redis_ports_key(ip)) and + not redis.exists(cls._redis_association_name('ip', ip))): + for i in range(FLAGS.vpn_start_port, FLAGS.vpn_end_port + 1): + redis.sadd(cls._redis_ports_key(ip), i) + @classmethod def num_ports_for_ip(cls, ip): """Calculates the number of free ports for a given ip""" + cls._ensure_set_exists(ip) return datastore.Redis.instance().scard('ip:%s:ports' % ip) @property diff --git a/nova/tests/network_unittest.py b/nova/tests/network_unittest.py index 879ee02a4..94d10200e 100644 --- a/nova/tests/network_unittest.py +++ b/nova/tests/network_unittest.py @@ -54,6 +54,7 @@ class NetworkTestCase(test.TrialTestCase): self.projects.append(self.manager.create_project(name, 'netuser', name)) + vpn.NetworkData.create(self.projects[i].id) self.network = model.PublicNetworkController() self.service = service.VlanNetworkService() @@ -70,7 +71,7 @@ class NetworkTestCase(test.TrialTestCase): self.assertTrue(IPy.IP(address) in self.network.network) def test_allocate_deallocate_fixed_ip(self): - result = yield self.service.allocate_fixed_ip( + result = self.service.allocate_fixed_ip( self.user.id, self.projects[0].id) address = result['private_dns_name'] mac = result['mac_address'] @@ -89,11 +90,11 @@ class NetworkTestCase(test.TrialTestCase): def test_range_allocation(self): hostname = "test-host" - result = yield self.service.allocate_fixed_ip( + result = self.service.allocate_fixed_ip( self.user.id, self.projects[0].id) mac = result['mac_address'] address = result['private_dns_name'] - result = yield self.service.allocate_fixed_ip( + result = self.service.allocate_fixed_ip( self.user, self.projects[1].id) secondmac = result['mac_address'] secondaddress = result['private_dns_name'] @@ -123,21 +124,21 @@ class NetworkTestCase(test.TrialTestCase): self.assertEqual(False, is_in_project(secondaddress, self.projects[1].id)) def test_subnet_edge(self): - result = yield self.service.allocate_fixed_ip(self.user.id, + result = self.service.allocate_fixed_ip(self.user.id, self.projects[0].id) firstaddress = result['private_dns_name'] hostname = "toomany-hosts" for i in range(1,5): project_id = self.projects[i].id - result = yield self.service.allocate_fixed_ip( + result = self.service.allocate_fixed_ip( self.user, project_id) mac = result['mac_address'] address = result['private_dns_name'] - result = yield self.service.allocate_fixed_ip( + result = self.service.allocate_fixed_ip( self.user, project_id) mac2 = result['mac_address'] address2 = result['private_dns_name'] - result = yield self.service.allocate_fixed_ip( + result = self.service.allocate_fixed_ip( self.user, project_id) mac3 = result['mac_address'] address3 = result['private_dns_name'] @@ -155,8 +156,7 @@ class NetworkTestCase(test.TrialTestCase): rv = self.service.deallocate_fixed_ip(firstaddress) self.dnsmasq.release_ip(mac, firstaddress, hostname, net.bridge_name) - def test_212_vpn_ip_and_port_looks_valid(self): - vpn.NetworkData.create(self.projects[0].id) + def test_vpn_ip_and_port_looks_valid(self): self.assert_(self.projects[0].vpn_ip) self.assert_(self.projects[0].vpn_port >= FLAGS.vpn_start_port) self.assert_(self.projects[0].vpn_port <= FLAGS.vpn_end_port) @@ -169,55 +169,85 @@ class NetworkTestCase(test.TrialTestCase): for network_datum in vpns: network_datum.destroy() - def test_release_before_deallocate(self): - pass + def test_ips_are_reused(self): + """Makes sure that ip addresses that are deallocated get reused""" - def test_deallocate_before_issued(self): - pass + result = self.service.allocate_fixed_ip( + self.user.id, self.projects[0].id) + mac = result['mac_address'] + address = result['private_dns_name'] - def test_too_many_addresses(self): - """ - Here, we test that a proper NoMoreAddresses exception is raised. + hostname = "reuse-host" + net = model.get_project_network(self.projects[0].id, "default") + + self.dnsmasq.issue_ip(mac, address, hostname, net.bridge_name) + rv = self.service.deallocate_fixed_ip(address) + self.dnsmasq.release_ip(mac, address, hostname, net.bridge_name) - However, the number of available IP addresses depends on the test + result = self.service.allocate_fixed_ip( + self.user, self.projects[0].id) + secondmac = result['mac_address'] + secondaddress = result['private_dns_name'] + self.assertEqual(address, secondaddress) + rv = self.service.deallocate_fixed_ip(secondaddress) + self.dnsmasq.issue_ip(secondmac, + secondaddress, + hostname, + net.bridge_name) + self.dnsmasq.release_ip(secondmac, + secondaddress, + hostname, + net.bridge_name) + + def test_available_ips(self): + """Make sure the number of available ips for the network is correct + + The number of available IP addresses depends on the test environment's setup. Network size is set in test fixture's setUp method. - There are FLAGS.cnt_vpn_clients addresses reserved for VPN (NUM_RESERVED_VPN_IPS) - - And there are NUM_STATIC_IPS that are always reserved by Nova for the necessary - services (gateway, CloudPipe, etc) - - So we should get flags.network_size - (NUM_STATIC_IPS + - NUM_PREALLOCATED_IPS + - NUM_RESERVED_VPN_IPS) - usable addresses + There are ips reserved at the bottom and top of the range. + services (network, gateway, CloudPipe, broadcast) """ net = model.get_project_network(self.projects[0].id, "default") - - # Determine expected number of available IP addresses - num_static_ips = net.num_static_ips num_preallocated_ips = len(net.hosts.keys()) - num_reserved_vpn_ips = flags.FLAGS.cnt_vpn_clients - num_available_ips = flags.FLAGS.network_size - (num_static_ips + + num_available_ips = flags.FLAGS.network_size - (net.num_bottom_reserved_ips + num_preallocated_ips + - num_reserved_vpn_ips) + net.num_top_reserved_ips) + self.assertEqual(num_available_ips, len(list(net.available))) + + def test_too_many_addresses(self): + """Test for a NoMoreAddresses exception when all fixed ips are used. + """ + net = model.get_project_network(self.projects[0].id, "default") hostname = "toomany-hosts" macs = {} addresses = {} - for i in range(0, (num_available_ips - 1)): - result = yield self.service.allocate_fixed_ip(self.user.id, self.projects[0].id) + # Number of availaible ips is len of the available list + num_available_ips = len(list(net.available)) + for i in range(num_available_ips): + result = self.service.allocate_fixed_ip(self.user.id, + self.projects[0].id) macs[i] = result['mac_address'] addresses[i] = result['private_dns_name'] - self.dnsmasq.issue_ip(macs[i], addresses[i], hostname, net.bridge_name) + self.dnsmasq.issue_ip(macs[i], + addresses[i], + hostname, + net.bridge_name) - self.assertFailure(self.service.allocate_fixed_ip(self.user.id, self.projects[0].id), NoMoreAddresses) + self.assertEqual(len(list(net.available)), 0) + self.assertRaises(NoMoreAddresses, self.service.allocate_fixed_ip, + self.user.id, self.projects[0].id) - for i in range(0, (num_available_ips - 1)): + for i in range(len(addresses)): rv = self.service.deallocate_fixed_ip(addresses[i]) - self.dnsmasq.release_ip(macs[i], addresses[i], hostname, net.bridge_name) + self.dnsmasq.release_ip(macs[i], + addresses[i], + hostname, + net.bridge_name) + self.assertEqual(len(list(net.available)), num_available_ips) def is_in_project(address, project_id): return address in model.get_project_network(project_id).list_addresses() -- cgit From d8c1a74342af9af442e4ef0508fa1f66eac48bb5 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Mon, 9 Aug 2010 23:02:06 -0700 Subject: fix releasing to work properly --- nova/network/model.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/nova/network/model.py b/nova/network/model.py index a70671632..109ae64c7 100644 --- a/nova/network/model.py +++ b/nova/network/model.py @@ -243,12 +243,12 @@ class BaseNetwork(datastore.BasicModel): def release_ip(self, ip_str): if not ip_str in self.assigned: raise exception.AddressNotAllocated() - self.deexpress(address=ip_str) self._rem_host(ip_str) + self.deexpress(address=ip_str) def deallocate_ip(self, ip_str): - # Do nothing for now, cleanup on ip release - pass + # go ahead and remove ip + self.release_ip(ip_str) def list_addresses(self): for address in self.hosts: -- cgit From fadaf1d9842abb991b093b04c031fa9947d82fbc Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Tue, 10 Aug 2010 11:48:14 -0700 Subject: pep8 cleanup --- nova/network/exception.py | 5 ++++- nova/network/linux_net.py | 46 ++++++++++++++++++++++++++++---------- nova/network/model.py | 39 +++++++++++++++++++------------- nova/network/service.py | 13 +++++++---- nova/network/vpn.py | 2 +- nova/tests/network_unittest.py | 50 +++++++++++++++++++++++------------------- 6 files changed, 100 insertions(+), 55 deletions(-) diff --git a/nova/network/exception.py b/nova/network/exception.py index 5722e9672..884ea54b4 100644 --- a/nova/network/exception.py +++ b/nova/network/exception.py @@ -26,15 +26,18 @@ from nova.exception import Error class NoMoreAddresses(Error): pass + class AddressNotAllocated(Error): pass + class AddressAlreadyAssociated(Error): pass + class AddressNotAssociated(Error): pass + class NotValidNetworkSize(Error): pass - diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py index 4a4b4c8a8..35bfded49 100644 --- a/nova/network/linux_net.py +++ b/nova/network/linux_net.py @@ -23,15 +23,16 @@ import subprocess # todo(ja): does the definition of network_path belong here? +from nova import flags from nova import utils -from nova import flags -FLAGS=flags.FLAGS +FLAGS = flags.FLAGS flags.DEFINE_string('dhcpbridge_flagfile', '/etc/nova/nova-dhcpbridge.conf', 'location of flagfile for dhcpbridge') + def execute(cmd, addl_env=None): if FLAGS.fake_network: logging.debug("FAKE NET: %s" % cmd) @@ -39,11 +40,13 @@ def execute(cmd, addl_env=None): else: return utils.execute(cmd, addl_env=addl_env) + def runthis(desc, cmd): if FLAGS.fake_network: return execute(cmd) else: - return utils.runthis(desc,cmd) + return utils.runthis(desc, cmd) + def Popen(cmd): if FLAGS.fake_network: @@ -56,18 +59,25 @@ def device_exists(device): (out, err) = execute("ifconfig %s" % device) return not err + def confirm_rule(cmd): execute("sudo iptables --delete %s" % (cmd)) execute("sudo iptables -I %s" % (cmd)) + def remove_rule(cmd): execute("sudo iptables --delete %s" % (cmd)) + def bind_public_ip(ip, interface): - runthis("Binding IP to interface: %s", "sudo ip addr add %s dev %s" % (ip, interface)) + runthis("Binding IP to interface: %s", + "sudo ip addr add %s dev %s" % (ip, interface)) + def unbind_public_ip(ip, interface): - runthis("Binding IP to interface: %s", "sudo ip addr del %s dev %s" % (ip, interface)) + runthis("Binding IP to interface: %s", + "sudo ip addr del %s dev %s" % (ip, interface)) + def vlan_create(net): """ create a vlan on on a bridge device unless vlan already exists """ @@ -77,6 +87,7 @@ def vlan_create(net): execute("sudo vconfig add %s %s" % (FLAGS.bridge_dev, net['vlan'])) execute("sudo ifconfig vlan%s up" % (net['vlan'])) + def bridge_create(net): """ create a bridge on a vlan unless it already exists """ if not device_exists(net['bridge_name']): @@ -85,14 +96,17 @@ def bridge_create(net): execute("sudo brctl setfd %s 0" % (net.bridge_name)) # execute("sudo brctl setageing %s 10" % (net.bridge_name)) execute("sudo brctl stp %s off" % (net['bridge_name'])) - execute("sudo brctl addif %s vlan%s" % (net['bridge_name'], net['vlan'])) + execute("sudo brctl addif %s vlan%s" % (net['bridge_name'], + net['vlan'])) if net.bridge_gets_ip: execute("sudo ifconfig %s %s broadcast %s netmask %s up" % \ (net['bridge_name'], net.gateway, net.broadcast, net.netmask)) - confirm_rule("FORWARD --in-interface %s -j ACCEPT" % (net['bridge_name'])) + confirm_rule("FORWARD --in-interface %s -j ACCEPT" % + (net['bridge_name'])) else: execute("sudo ifconfig %s up" % net['bridge_name']) + def dnsmasq_cmd(net): cmd = ['sudo -E dnsmasq', ' --strict-order', @@ -107,12 +121,15 @@ def dnsmasq_cmd(net): ' --leasefile-ro'] return ''.join(cmd) + def hostDHCP(network, host, mac): - idx = host.split(".")[-1] # Logically, the idx of instances they've launched in this net + # Logically, the idx of instances they've launched in this net + idx = host.split(".")[-1] return "%s,%s-%s-%s.novalocal,%s" % \ (mac, network['user_id'], network['vlan'], idx, host) -# todo(ja): if the system has restarted or pid numbers have wrapped + +# TODO(ja): if the system has restarted or pid numbers have wrapped # then you cannot be certain that the pid refers to the # dnsmasq. As well, sending a HUP only reloads the hostfile, # so any configuration options (like dchp-range, vlan, ...) @@ -125,13 +142,15 @@ def start_dnsmasq(network): """ with open(dhcp_file(network['vlan'], 'conf'), 'w') as f: for host_name in network.hosts: - f.write("%s\n" % hostDHCP(network, host_name, network.hosts[host_name])) + f.write("%s\n" % hostDHCP(network, + host_name, + network.hosts[host_name])) pid = dnsmasq_pid_for(network) # if dnsmasq is already running, then tell it to reload if pid: - # todo(ja): use "/proc/%d/cmdline" % (pid) to determine if pid refers + # TODO(ja): use "/proc/%d/cmdline" % (pid) to determine if pid refers # correct dnsmasq process try: os.kill(pid, signal.SIGHUP) @@ -148,6 +167,7 @@ def start_dnsmasq(network): 'DNSMASQ_INTERFACE': network['bridge_name']} execute(dnsmasq_cmd(network), addl_env=env) + def stop_dnsmasq(network): """ stops the dnsmasq instance for a given network """ pid = dnsmasq_pid_for(network) @@ -158,14 +178,17 @@ def stop_dnsmasq(network): except Exception, e: logging.debug("Killing dnsmasq threw %s", e) + def dhcp_file(vlan, kind): """ return path to a pid, leases or conf file for a vlan """ return os.path.abspath("%s/nova-%s.%s" % (FLAGS.networks_path, vlan, kind)) + def bin_file(script): return os.path.abspath(os.path.join(__file__, "../../../bin", script)) + def dnsmasq_pid_for(network): """ the pid for prior dnsmasq instance for a vlan, returns None if no pid file exists @@ -178,4 +201,3 @@ def dnsmasq_pid_for(network): if os.path.exists(pid_file): with open(pid_file, 'r') as f: return int(f.read()) - diff --git a/nova/network/model.py b/nova/network/model.py index 2074a6d46..734a3f7a9 100644 --- a/nova/network/model.py +++ b/nova/network/model.py @@ -119,7 +119,9 @@ class Vlan(datastore.BasicModel): default way of saving into "vlan:ID" and adding to a set of "vlans". """ set_name = self._redis_set_name(self.__class__.__name__) - datastore.Redis.instance().hset(set_name, self.project_id, self.vlan_id) + datastore.Redis.instance().hset(set_name, + self.project_id, + self.vlan_id) @datastore.absorb_connection_error def destroy(self): @@ -129,17 +131,16 @@ class Vlan(datastore.BasicModel): def subnet(self): vlan = int(self.vlan_id) network = IPy.IP(FLAGS.private_range) - start = (vlan-FLAGS.vlan_start) * FLAGS.network_size + start = (vlan - FLAGS.vlan_start) * FLAGS.network_size # minus one for the gateway. return "%s-%s" % (network[start], network[start + FLAGS.network_size - 1]) + # CLEANUP: # TODO(ja): Save the IPs at the top of each subnet for cloudpipe vpn clients # TODO(ja): does vlanpool "keeper" need to know the min/max - # shouldn't FLAGS always win? -# TODO(joshua): Save the IPs at the top of each subnet for cloudpipe vpn clients - class BaseNetwork(datastore.BasicModel): override_type = 'network' @@ -223,11 +224,11 @@ class BaseNetwork(datastore.BasicModel): @property def num_bottom_reserved_ips(self): - return 2 # Network, Gateway + return 2 # Network, Gateway @property def num_top_reserved_ips(self): - return 1 # Broadcast + return 1 # Broadcast def allocate_ip(self, user_id, project_id, mac): for address in self.available: @@ -257,8 +258,11 @@ class BaseNetwork(datastore.BasicModel): for address in self.hosts: yield address - def express(self, address=None): pass - def deexpress(self, address=None): pass + def express(self, address=None): + pass + + def deexpress(self, address=None): + pass class BridgedNetwork(BaseNetwork): @@ -298,6 +302,7 @@ class BridgedNetwork(BaseNetwork): linux_net.vlan_create(self) linux_net.bridge_create(self) + class DHCPNetwork(BridgedNetwork): """ properties: @@ -365,6 +370,7 @@ class DHCPNetwork(BridgedNetwork): else: linux_net.start_dnsmasq(self) + class PublicAddress(datastore.BasicModel): override_type = "address" @@ -391,6 +397,8 @@ class PublicAddress(datastore.BasicModel): DEFAULT_PORTS = [("tcp", 80), ("tcp", 22), ("udp", 1194), ("tcp", 443)] + + class PublicNetworkController(BaseNetwork): override_type = 'network' @@ -400,7 +408,8 @@ class PublicNetworkController(BaseNetwork): FLAGS.public_range) self['user_id'] = "public" self['project_id'] = "public" - self["create_time"] = time.strftime('%Y-%m-%dT%H:%M:%SZ', time.gmtime()) + self["create_time"] = time.strftime('%Y-%m-%dT%H:%M:%SZ', + time.gmtime()) self["vlan"] = FLAGS.public_vlan self.save() self.express() @@ -458,7 +467,7 @@ class PublicNetworkController(BaseNetwork): if address: addresses = [self.get_host(address)] for addr in addresses: - if addr.get('private_ip','available') == 'available': + if addr.get('private_ip', 'available') == 'available': continue public_ip = addr['address'] private_ip = addr['private_ip'] @@ -490,8 +499,9 @@ class PublicNetworkController(BaseNetwork): % (private_ip, protocol, port)) -# FIXME(todd): does this present a race condition, or is there some piece of -# architecture that mitigates it (only one queue listener per net)? +# FIXME(todd): does this present a race condition, or is there some +# piece of architecture that mitigates it (only one queue +# listener per net)? def get_vlan_for_project(project_id): """ Allocate vlan IDs to individual users. @@ -502,7 +512,7 @@ def get_vlan_for_project(project_id): known_vlans = Vlan.dict_by_vlan() for vnum in range(FLAGS.vlan_start, FLAGS.vlan_end): vstr = str(vnum) - if not known_vlans.has_key(vstr): + if not vstr in known_vlans: return Vlan.create(project_id, vnum) old_project_id = known_vlans[vstr] if not manager.AuthManager().get_project(old_project_id): @@ -526,6 +536,7 @@ def get_vlan_for_project(project_id): return Vlan.create(project_id, vnum) raise exception.AddressNotAllocated("Out of VLANs") + def get_project_network(project_id, security_group='default'): """ get a project's private network, allocating one if needed """ project = manager.AuthManager().get_project(project_id) @@ -556,10 +567,8 @@ def get_network_by_interface(iface, security_group='default'): return get_project_network(project_id, security_group) - def get_public_ip_for_instance(instance_id): # FIXME: this should be a lookup - iteration won't scale for address_record in PublicAddress.all(): if address_record.get('instance_id', 'available') == instance_id: return address_record['address'] - diff --git a/nova/network/service.py b/nova/network/service.py index 1a61f49d4..f13324103 100644 --- a/nova/network/service.py +++ b/nova/network/service.py @@ -38,7 +38,7 @@ flags.DEFINE_string('network_type', flags.DEFINE_string('flat_network_bridge', 'br100', 'Bridge for simple network instances') flags.DEFINE_list('flat_network_ips', - ['192.168.0.2','192.168.0.3','192.168.0.4'], + ['192.168.0.2', '192.168.0.3', '192.168.0.4'], 'Available ips for simple network') flags.DEFINE_string('flat_network_network', '192.168.0.0', 'Network for simple network') @@ -51,17 +51,21 @@ flags.DEFINE_string('flat_network_broadcast', '192.168.0.255', flags.DEFINE_string('flat_network_dns', '8.8.4.4', 'Dns for simple network') + def type_to_class(network_type): if network_type == 'flat': return FlatNetworkService - elif network_type == 'vlan': + elif network_type == 'vlan': return VlanNetworkService raise NotFound("Couldn't find %s network type" % network_type) def setup_compute_network(network_type, user_id, project_id, security_group): srv = type_to_class(network_type) - srv.setup_compute_network(network_type, user_id, project_id, security_group) + srv.setup_compute_network(network_type, + user_id, + project_id, + security_group) def get_host_for_project(project_id): @@ -175,6 +179,7 @@ class FlatNetworkService(BaseNetworkService): """Returns an ip to the pool""" datastore.Redis.instance().sadd('ips', fixed_ip) + class VlanNetworkService(BaseNetworkService): """Vlan network with dhcp""" # NOTE(vish): A lot of the interactions with network/model.py can be @@ -194,7 +199,7 @@ class VlanNetworkService(BaseNetworkService): return {'network_type': FLAGS.network_type, 'bridge_name': net['bridge_name'], 'mac_address': mac, - 'private_dns_name' : fixed_ip} + 'private_dns_name': fixed_ip} def deallocate_fixed_ip(self, fixed_ip, *args, **kwargs): diff --git a/nova/network/vpn.py b/nova/network/vpn.py index 1b6dd7a56..74eebf9a8 100644 --- a/nova/network/vpn.py +++ b/nova/network/vpn.py @@ -33,6 +33,7 @@ flags.DEFINE_integer('vpn_start_port', 1000, flags.DEFINE_integer('vpn_end_port', 2000, 'End port for the cloudpipe VPN servers') + class NoMorePorts(exception.Error): pass @@ -121,4 +122,3 @@ class NetworkData(datastore.BasicModel): self.unassociate_with('ip', self.ip) datastore.Redis.instance().sadd('ip:%s:ports' % self.ip, self.port) super(NetworkData, self).destroy() - diff --git a/nova/tests/network_unittest.py b/nova/tests/network_unittest.py index 94d10200e..9aa39e516 100644 --- a/nova/tests/network_unittest.py +++ b/nova/tests/network_unittest.py @@ -31,6 +31,7 @@ from nova.network.exception import NoMoreAddresses FLAGS = flags.FLAGS + class NetworkTestCase(test.TrialTestCase): def setUp(self): super(NetworkTestCase, self).setUp() @@ -66,12 +67,14 @@ class NetworkTestCase(test.TrialTestCase): def test_public_network_allocation(self): pubnet = IPy.IP(flags.FLAGS.public_range) - address = self.network.allocate_ip(self.user.id, self.projects[0].id, "public") + address = self.network.allocate_ip(self.user.id, + self.projects[0].id, + "public") self.assertTrue(IPy.IP(address) in pubnet) self.assertTrue(IPy.IP(address) in self.network.network) def test_allocate_deallocate_fixed_ip(self): - result = self.service.allocate_fixed_ip( + result = self.service.allocate_fixed_ip( self.user.id, self.projects[0].id) address = result['private_dns_name'] mac = result['mac_address'] @@ -103,7 +106,8 @@ class NetworkTestCase(test.TrialTestCase): secondnet = model.get_project_network(self.projects[1].id, "default") self.assertEqual(True, is_in_project(address, self.projects[0].id)) - self.assertEqual(True, is_in_project(secondaddress, self.projects[1].id)) + self.assertEqual(True, is_in_project(secondaddress, + self.projects[1].id)) self.assertEqual(False, is_in_project(address, self.projects[1].id)) # Addresses are allocated before they're issued @@ -116,19 +120,21 @@ class NetworkTestCase(test.TrialTestCase): self.assertEqual(False, is_in_project(address, self.projects[0].id)) # First address release shouldn't affect the second - self.assertEqual(True, is_in_project(secondaddress, self.projects[1].id)) + self.assertEqual(True, is_in_project(secondaddress, + self.projects[1].id)) rv = self.service.deallocate_fixed_ip(secondaddress) self.dnsmasq.release_ip(secondmac, secondaddress, hostname, secondnet.bridge_name) - self.assertEqual(False, is_in_project(secondaddress, self.projects[1].id)) + self.assertEqual(False, is_in_project(secondaddress, + self.projects[1].id)) def test_subnet_edge(self): result = self.service.allocate_fixed_ip(self.user.id, self.projects[0].id) firstaddress = result['private_dns_name'] hostname = "toomany-hosts" - for i in range(1,5): + for i in range(1, 5): project_id = self.projects[i].id result = self.service.allocate_fixed_ip( self.user, project_id) @@ -142,9 +148,12 @@ class NetworkTestCase(test.TrialTestCase): self.user, project_id) mac3 = result['mac_address'] address3 = result['private_dns_name'] - self.assertEqual(False, is_in_project(address, self.projects[0].id)) - self.assertEqual(False, is_in_project(address2, self.projects[0].id)) - self.assertEqual(False, is_in_project(address3, self.projects[0].id)) + self.assertEqual(False, is_in_project(address, + self.projects[0].id)) + self.assertEqual(False, is_in_project(address2, + self.projects[0].id)) + self.assertEqual(False, is_in_project(address3, + self.projects[0].id)) rv = self.service.deallocate_fixed_ip(address) rv = self.service.deallocate_fixed_ip(address2) rv = self.service.deallocate_fixed_ip(address3) @@ -212,9 +221,10 @@ class NetworkTestCase(test.TrialTestCase): """ net = model.get_project_network(self.projects[0].id, "default") num_preallocated_ips = len(net.hosts.keys()) - num_available_ips = flags.FLAGS.network_size - (net.num_bottom_reserved_ips + - num_preallocated_ips + - net.num_top_reserved_ips) + net_size = flags.FLAGS.network_size + num_available_ips = net_size - (net.num_bottom_reserved_ips + + num_preallocated_ips + + net.num_top_reserved_ips) self.assertEqual(num_available_ips, len(list(net.available))) def test_too_many_addresses(self): @@ -249,25 +259,22 @@ class NetworkTestCase(test.TrialTestCase): net.bridge_name) self.assertEqual(len(list(net.available)), num_available_ips) + def is_in_project(address, project_id): return address in model.get_project_network(project_id).list_addresses() -def _get_project_addresses(project_id): - project_addresses = [] - for addr in model.get_project_network(project_id).list_addresses(): - project_addresses.append(addr) - return project_addresses def binpath(script): return os.path.abspath(os.path.join(__file__, "../../../bin", script)) + class FakeDNSMasq(object): def issue_ip(self, mac, ip, hostname, interface): cmd = "%s add %s %s %s" % (binpath('nova-dhcpbridge'), mac, ip, hostname) env = {'DNSMASQ_INTERFACE': interface, - 'TESTING' : '1', - 'FLAGFILE' : FLAGS.dhcpbridge_flagfile} + 'TESTING': '1', + 'FLAGFILE': FLAGS.dhcpbridge_flagfile} (out, err) = utils.execute(cmd, addl_env=env) logging.debug("ISSUE_IP: %s, %s " % (out, err)) @@ -275,8 +282,7 @@ class FakeDNSMasq(object): cmd = "%s del %s %s %s" % (binpath('nova-dhcpbridge'), mac, ip, hostname) env = {'DNSMASQ_INTERFACE': interface, - 'TESTING' : '1', - 'FLAGFILE' : FLAGS.dhcpbridge_flagfile} + 'TESTING': '1', + 'FLAGFILE': FLAGS.dhcpbridge_flagfile} (out, err) = utils.execute(cmd, addl_env=env) logging.debug("RELEASE_IP: %s, %s " % (out, err)) - -- cgit From c4f6500a4c33d4ad093d29f971c139b63984a0a5 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Tue, 10 Aug 2010 12:27:06 -0700 Subject: pylint cleanup --- bin/nova-dhcpbridge | 5 ++-- nova/network/exception.py | 5 ++++ nova/network/linux_net.py | 66 ++++++++++++++++++++++++++--------------------- 3 files changed, 44 insertions(+), 32 deletions(-) diff --git a/bin/nova-dhcpbridge b/bin/nova-dhcpbridge index 7789dac98..6a9115fcb 100755 --- a/bin/nova-dhcpbridge +++ b/bin/nova-dhcpbridge @@ -70,8 +70,9 @@ def init_leases(interface): net = model.get_network_by_interface(interface) res = "" for host_name in net.hosts: - res += "%s\n" % linux_net.hostDHCP(net, host_name, - net.hosts[host_name]) + res += "%s\n" % linux_net.host_dhcp(net, + host_name, + net.hosts[host_name]) return res diff --git a/nova/network/exception.py b/nova/network/exception.py index 884ea54b4..8d7aa1498 100644 --- a/nova/network/exception.py +++ b/nova/network/exception.py @@ -24,20 +24,25 @@ from nova.exception import Error class NoMoreAddresses(Error): + """No More Addresses are available in the network""" pass class AddressNotAllocated(Error): + """The specified address has not been allocated""" pass class AddressAlreadyAssociated(Error): + """The specified address has already been associated""" pass class AddressNotAssociated(Error): + """The specified address is not associated""" pass class NotValidNetworkSize(Error): + """The network size is not valid""" pass diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py index 35bfded49..2f6a9638d 100644 --- a/nova/network/linux_net.py +++ b/nova/network/linux_net.py @@ -15,11 +15,13 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. +""" +Implements vlans, bridges, and iptables rules using linux utilities. +""" import logging import signal import os -import subprocess # todo(ja): does the definition of network_path belong here? @@ -34,53 +36,53 @@ flags.DEFINE_string('dhcpbridge_flagfile', def execute(cmd, addl_env=None): + """Wrapper around utils.execute for fake_network""" if FLAGS.fake_network: - logging.debug("FAKE NET: %s" % cmd) + logging.debug("FAKE NET: %s", cmd) return "fake", 0 else: return utils.execute(cmd, addl_env=addl_env) def runthis(desc, cmd): + """Wrapper around utils.runthis for fake_network""" if FLAGS.fake_network: return execute(cmd) else: return utils.runthis(desc, cmd) -def Popen(cmd): - if FLAGS.fake_network: - execute(' '.join(cmd)) - else: - subprocess.Popen(cmd) - - def device_exists(device): - (out, err) = execute("ifconfig %s" % device) + """Check if ethernet device exists""" + (_out, err) = execute("ifconfig %s" % device) return not err def confirm_rule(cmd): + """Delete and re-add iptables rule""" execute("sudo iptables --delete %s" % (cmd)) execute("sudo iptables -I %s" % (cmd)) def remove_rule(cmd): + """Remove iptables rule""" execute("sudo iptables --delete %s" % (cmd)) -def bind_public_ip(ip, interface): +def bind_public_ip(public_ip, interface): + """Bind ip to an interface""" runthis("Binding IP to interface: %s", - "sudo ip addr add %s dev %s" % (ip, interface)) + "sudo ip addr add %s dev %s" % (public_ip, interface)) -def unbind_public_ip(ip, interface): +def unbind_public_ip(public_ip, interface): + """Unbind a public ip from an interface""" runthis("Binding IP to interface: %s", - "sudo ip addr del %s dev %s" % (ip, interface)) + "sudo ip addr del %s dev %s" % (public_ip, interface)) def vlan_create(net): - """ create a vlan on on a bridge device unless vlan already exists """ + """Create a vlan on on a bridge device unless vlan already exists""" if not device_exists("vlan%s" % net['vlan']): logging.debug("Starting VLAN inteface for %s network", (net['vlan'])) execute("sudo vconfig set_name_type VLAN_PLUS_VID_NO_PAD") @@ -89,7 +91,7 @@ def vlan_create(net): def bridge_create(net): - """ create a bridge on a vlan unless it already exists """ + """Create a bridge on a vlan unless it already exists""" if not device_exists(net['bridge_name']): logging.debug("Starting Bridge inteface for %s network", (net['vlan'])) execute("sudo brctl addbr %s" % (net['bridge_name'])) @@ -107,7 +109,8 @@ def bridge_create(net): execute("sudo ifconfig %s up" % net['bridge_name']) -def dnsmasq_cmd(net): +def _dnsmasq_cmd(net): + """Builds dnsmasq command""" cmd = ['sudo -E dnsmasq', ' --strict-order', ' --bind-interfaces', @@ -122,7 +125,8 @@ def dnsmasq_cmd(net): return ''.join(cmd) -def hostDHCP(network, host, mac): +def host_dhcp(network, host, mac): + """Return a host string for a network, host, and mac""" # Logically, the idx of instances they've launched in this net idx = host.split(".")[-1] return "%s,%s-%s-%s.novalocal,%s" % \ @@ -135,14 +139,14 @@ def hostDHCP(network, host, mac): # so any configuration options (like dchp-range, vlan, ...) # aren't reloaded def start_dnsmasq(network): - """ (re)starts a dnsmasq server for a given network + """(Re)starts a dnsmasq server for a given network if a dnsmasq instance is already running then send a HUP signal causing it to reload, otherwise spawn a new instance """ with open(dhcp_file(network['vlan'], 'conf'), 'w') as f: for host_name in network.hosts: - f.write("%s\n" % hostDHCP(network, + f.write("%s\n" % host_dhcp(network, host_name, network.hosts[host_name])) @@ -154,8 +158,8 @@ def start_dnsmasq(network): # correct dnsmasq process try: os.kill(pid, signal.SIGHUP) - except Exception, e: - logging.debug("Hupping dnsmasq threw %s", e) + except Exception as exc: # pylint: disable-msg=W0703 + logging.debug("Hupping dnsmasq threw %s", exc) # otherwise delete the existing leases file and start dnsmasq lease_file = dhcp_file(network['vlan'], 'leases') @@ -165,35 +169,37 @@ def start_dnsmasq(network): # FLAGFILE and DNSMASQ_INTERFACE in env env = {'FLAGFILE': FLAGS.dhcpbridge_flagfile, 'DNSMASQ_INTERFACE': network['bridge_name']} - execute(dnsmasq_cmd(network), addl_env=env) + execute(_dnsmasq_cmd(network), addl_env=env) def stop_dnsmasq(network): - """ stops the dnsmasq instance for a given network """ + """Stops the dnsmasq instance for a given network""" pid = dnsmasq_pid_for(network) if pid: try: os.kill(pid, signal.SIGTERM) - except Exception, e: - logging.debug("Killing dnsmasq threw %s", e) + except Exception as exc: # pylint: disable-msg=W0703 + logging.debug("Killing dnsmasq threw %s", exc) def dhcp_file(vlan, kind): - """ return path to a pid, leases or conf file for a vlan """ + """Return path to a pid, leases or conf file for a vlan""" return os.path.abspath("%s/nova-%s.%s" % (FLAGS.networks_path, vlan, kind)) def bin_file(script): + """Return the absolute path to scipt in the bin directory""" return os.path.abspath(os.path.join(__file__, "../../../bin", script)) def dnsmasq_pid_for(network): - """ the pid for prior dnsmasq instance for a vlan, - returns None if no pid file exists + """Returns he pid for prior dnsmasq instance for a vlan + + Returns None if no pid file exists - if machine has rebooted pid might be incorrect (caller should check) + If machine has rebooted pid might be incorrect (caller should check) """ pid_file = dhcp_file(network['vlan'], 'pid') -- cgit From e0983caad1c3ff7ca451094f8778b1a62bf91531 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Tue, 10 Aug 2010 12:46:40 -0700 Subject: Further pylint cleanup --- nova/endpoint/cloud.py | 10 +++++----- nova/network/linux_net.py | 4 ++-- nova/network/service.py | 24 ++++++++++++++++-------- 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index ad9188ff3..02969c8e9 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -103,7 +103,7 @@ class CloudController(object): result = {} for instance in self.instdir.all: if instance['project_id'] == project_id: - line = '%s slots=%d' % (instance['private_dns_name'], + line = '%s slots=%d' % (instance['private_dns_name'], INSTANCE_TYPES[instance['instance_type']]['vcpus']) if instance['key_name'] in result: result[instance['key_name']].append(line) @@ -423,7 +423,7 @@ class CloudController(object): i['key_name'] = instance.get('key_name', None) if context.user.is_admin(): i['key_name'] = '%s (%s, %s)' % (i['key_name'], - instance.get('project_id', None), + instance.get('project_id', None), instance.get('node_name', '')) i['product_codes_set'] = self._convert_to_set( instance.get('product_codes', None), 'product_code') @@ -560,15 +560,15 @@ class CloudController(object): # TODO: Get the real security group of launch in here security_group = "default" for num in range(int(kwargs['max_count'])): - vpn = False + is_vpn = False if image_id == FLAGS.vpn_image_id: - vpn = True + is_vpn = True allocate_result = yield rpc.call(network_topic, {"method": "allocate_fixed_ip", "args": {"user_id": context.user.id, "project_id": context.project.id, "security_group": security_group, - "vpn": vpn}}) + "is_vpn": is_vpn}}) allocate_data = allocate_result['result'] inst = self.instdir.new() inst['image_id'] = image_id diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py index 2f6a9638d..56b4a9dd2 100644 --- a/nova/network/linux_net.py +++ b/nova/network/linux_net.py @@ -158,7 +158,7 @@ def start_dnsmasq(network): # correct dnsmasq process try: os.kill(pid, signal.SIGHUP) - except Exception as exc: # pylint: disable-msg=W0703 + except Exception as exc: # pylint: disable=W0703 logging.debug("Hupping dnsmasq threw %s", exc) # otherwise delete the existing leases file and start dnsmasq @@ -179,7 +179,7 @@ def stop_dnsmasq(network): if pid: try: os.kill(pid, signal.SIGTERM) - except Exception as exc: # pylint: disable-msg=W0703 + except Exception as exc: # pylint: disable=W0703 logging.debug("Killing dnsmasq threw %s", exc) diff --git a/nova/network/service.py b/nova/network/service.py index f13324103..fd45496c9 100644 --- a/nova/network/service.py +++ b/nova/network/service.py @@ -17,7 +17,7 @@ # under the License. """ -Network Nodes are responsible for allocating ips and setting up network +Network Hosts are responsible for allocating ips and setting up network """ from nova import datastore @@ -53,6 +53,7 @@ flags.DEFINE_string('flat_network_dns', '8.8.4.4', def type_to_class(network_type): + """Convert a network_type string into an actual Python class""" if network_type == 'flat': return FlatNetworkService elif network_type == 'vlan': @@ -61,6 +62,7 @@ def type_to_class(network_type): def setup_compute_network(network_type, user_id, project_id, security_group): + """Sets up the network on a compute host""" srv = type_to_class(network_type) srv.setup_compute_network(network_type, user_id, @@ -69,12 +71,14 @@ def setup_compute_network(network_type, user_id, project_id, security_group): def get_host_for_project(project_id): + """Get host allocated to project from datastore""" redis = datastore.Redis.instance() return redis.get(_host_key(project_id)) def _host_key(project_id): - return "network_host:%s" % project_id + """Returns redis host key for network""" + return "networkhost:%s" % project_id class BaseNetworkService(service.Service): @@ -84,6 +88,7 @@ class BaseNetworkService(service.Service): """ def __init__(self, *args, **kwargs): self.network = model.PublicNetworkController() + super(BaseNetworkService, self).__init__(*args, **kwargs) def set_network_host(self, user_id, project_id, *args, **kwargs): """Safely sets the host of the projects network""" @@ -113,7 +118,7 @@ class BaseNetworkService(service.Service): pass @classmethod - def setup_compute_network(self, user_id, project_id, security_group, + def setup_compute_network(cls, user_id, project_id, security_group, *args, **kwargs): """Sets up matching network for compute hosts""" raise NotImplementedError() @@ -142,7 +147,7 @@ class FlatNetworkService(BaseNetworkService): """Basic network where no vlans are used""" @classmethod - def setup_compute_network(self, user_id, project_id, security_group, + def setup_compute_network(cls, user_id, project_id, security_group, *args, **kwargs): """Network is created manually""" pass @@ -186,13 +191,14 @@ class VlanNetworkService(BaseNetworkService): # simplified and improved. Also there it may be useful # to support vlans separately from dhcp, instead of having # both of them together in this class. + # pylint: disable=W0221 def allocate_fixed_ip(self, user_id, project_id, security_group='default', - vpn=False, *args, **kwargs): - """Gets a fixed ip from the pool """ + is_vpn=False, *args, **kwargs): + """Gets a fixed ip from the pool""" mac = utils.generate_mac() net = model.get_project_network(project_id) - if vpn: + if is_vpn: fixed_ip = net.allocate_vpn_ip(user_id, project_id, mac) else: fixed_ip = net.allocate_ip(user_id, project_id, mac) @@ -207,9 +213,11 @@ class VlanNetworkService(BaseNetworkService): return model.get_network_by_address(fixed_ip).deallocate_ip(fixed_ip) def lease_ip(self, address): + """Called by bridge when ip is leased""" return model.get_network_by_address(address).lease_ip(address) def release_ip(self, address): + """Called by bridge when ip is released""" return model.get_network_by_address(address).release_ip(address) def restart_nets(self): @@ -223,7 +231,7 @@ class VlanNetworkService(BaseNetworkService): vpn.NetworkData.create(project_id) @classmethod - def setup_compute_network(self, user_id, project_id, security_group, + def setup_compute_network(cls, user_id, project_id, security_group, *args, **kwargs): """Sets up matching network for compute hosts""" # NOTE(vish): Use BridgedNetwork instead of DHCPNetwork because -- cgit From 712b6e41d40303a7a3e9d0ce21dde628361417ae Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Tue, 10 Aug 2010 12:51:42 -0700 Subject: Pylint clean of vpn.py --- nova/network/vpn.py | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/nova/network/vpn.py b/nova/network/vpn.py index 74eebf9a8..a0e2a7fa1 100644 --- a/nova/network/vpn.py +++ b/nova/network/vpn.py @@ -35,6 +35,7 @@ flags.DEFINE_integer('vpn_end_port', 2000, class NoMorePorts(exception.Error): + """No ports available to allocate for the given ip""" pass @@ -68,42 +69,44 @@ class NetworkData(datastore.BasicModel): return network_data @classmethod - def find_free_port_for_ip(cls, ip): + def find_free_port_for_ip(cls, vpn_ip): """Finds a free port for a given ip from the redis set""" # TODO(vish): these redis commands should be generalized and # placed into a base class. Conceptually, it is # similar to an association, but we are just # storing a set of values instead of keys that # should be turned into objects. - cls._ensure_set_exists(ip) + cls._ensure_set_exists(vpn_ip) - port = datastore.Redis.instance().spop(cls._redis_ports_key(ip)) + port = datastore.Redis.instance().spop(cls._redis_ports_key(vpn_ip)) if not port: raise NoMorePorts() return port @classmethod - def _redis_ports_key(cls, ip): - return 'ip:%s:ports' % ip + def _redis_ports_key(cls, vpn_ip): + """Key that ports are stored under in redis""" + return 'ip:%s:ports' % vpn_ip @classmethod - def _ensure_set_exists(cls, ip): + def _ensure_set_exists(cls, vpn_ip): + """Creates the set of ports for the ip if it doesn't already exist""" # TODO(vish): these ports should be allocated through an admin # command instead of a flag redis = datastore.Redis.instance() - if (not redis.exists(cls._redis_ports_key(ip)) and - not redis.exists(cls._redis_association_name('ip', ip))): + if (not redis.exists(cls._redis_ports_key(vpn_ip)) and + not redis.exists(cls._redis_association_name('ip', vpn_ip))): for i in range(FLAGS.vpn_start_port, FLAGS.vpn_end_port + 1): - redis.sadd(cls._redis_ports_key(ip), i) + redis.sadd(cls._redis_ports_key(vpn_ip), i) @classmethod - def num_ports_for_ip(cls, ip): + def num_ports_for_ip(cls, vpn_ip): """Calculates the number of free ports for a given ip""" - cls._ensure_set_exists(ip) - return datastore.Redis.instance().scard('ip:%s:ports' % ip) + cls._ensure_set_exists(vpn_ip) + return datastore.Redis.instance().scard('ip:%s:ports' % vpn_ip) @property - def ip(self): + def ip(self): # pylint: disable=C0103 """The ip assigned to the project""" return self['ip'] -- cgit From 47bf3ed11f2f372a07ea3b1b8deb9f7684cc2e5d Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Tue, 10 Aug 2010 15:45:24 -0700 Subject: lots more pylint fixes --- nova/network/linux_net.py | 2 +- nova/network/model.py | 131 ++++++++++++++++++++++++------------ nova/tests/network_unittest.py | 147 ++++++++++++++++++++--------------------- 3 files changed, 160 insertions(+), 120 deletions(-) diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py index 56b4a9dd2..0e8ddcc6a 100644 --- a/nova/network/linux_net.py +++ b/nova/network/linux_net.py @@ -118,7 +118,7 @@ def _dnsmasq_cmd(net): ' --pid-file=%s' % dhcp_file(net['vlan'], 'pid'), ' --listen-address=%s' % net.dhcp_listen_address, ' --except-interface=lo', - ' --dhcp-range=%s,static,600s' % (net.dhcp_range_start), + ' --dhcp-range=%s,static,600s' % net.dhcp_range_start, ' --dhcp-hostsfile=%s' % dhcp_file(net['vlan'], 'conf'), ' --dhcp-script=%s' % bin_file('nova-dhcpbridge'), ' --leasefile-ro'] diff --git a/nova/network/model.py b/nova/network/model.py index 734a3f7a9..7b1e16f26 100644 --- a/nova/network/model.py +++ b/nova/network/model.py @@ -57,7 +57,8 @@ logging.getLogger().setLevel(logging.DEBUG) class Vlan(datastore.BasicModel): - def __init__(self, project, vlan): + """Tracks vlans assigned to project it the datastore""" + def __init__(self, project, vlan): # pylint: disable=W0231 """ Since we don't want to try and find a vlan by its identifier, but by a project id, we don't call super-init. @@ -67,10 +68,12 @@ class Vlan(datastore.BasicModel): @property def identifier(self): + """Datastore identifier""" return "%s:%s" % (self.project_id, self.vlan_id) @classmethod def create(cls, project, vlan): + """Create a Vlan object""" instance = cls(project, vlan) instance.save() return instance @@ -78,6 +81,7 @@ class Vlan(datastore.BasicModel): @classmethod @datastore.absorb_connection_error def lookup(cls, project): + """Returns object by project if it exists in datastore or None""" set_name = cls._redis_set_name(cls.__name__) vlan = datastore.Redis.instance().hget(set_name, project) if vlan: @@ -88,19 +92,19 @@ class Vlan(datastore.BasicModel): @classmethod @datastore.absorb_connection_error def dict_by_project(cls): - """a hash of project:vlan""" + """A hash of project:vlan""" set_name = cls._redis_set_name(cls.__name__) - return datastore.Redis.instance().hgetall(set_name) + return datastore.Redis.instance().hgetall(set_name) or {} @classmethod @datastore.absorb_connection_error def dict_by_vlan(cls): - """a hash of vlan:project""" + """A hash of vlan:project""" set_name = cls._redis_set_name(cls.__name__) retvals = {} - hashset = datastore.Redis.instance().hgetall(set_name) - for val in hashset.keys(): - retvals[hashset[val]] = val + hashset = datastore.Redis.instance().hgetall(set_name) or {} + for (key, val) in hashset.iteritems(): + retvals[val] = key return retvals @classmethod @@ -125,10 +129,12 @@ class Vlan(datastore.BasicModel): @datastore.absorb_connection_error def destroy(self): + """Removes the object from the datastore""" set_name = self._redis_set_name(self.__class__.__name__) datastore.Redis.instance().hdel(set_name, self.project_id) def subnet(self): + """Returns a string containing the subnet""" vlan = int(self.vlan_id) network = IPy.IP(FLAGS.private_range) start = (vlan - FLAGS.vlan_start) * FLAGS.network_size @@ -142,17 +148,22 @@ class Vlan(datastore.BasicModel): # TODO(ja): does vlanpool "keeper" need to know the min/max - # shouldn't FLAGS always win? class BaseNetwork(datastore.BasicModel): + """Implements basic logic for allocating ips in a network""" override_type = 'network' @property def identifier(self): + """Datastore identifier""" return self.network_id def default_state(self): + """Default values for new objects""" return {'network_id': self.network_id, 'network_str': self.network_str} @classmethod + # pylint: disable=R0913 def create(cls, user_id, project_id, security_group, vlan, network_str): + """Create a BaseNetwork object""" network_id = "%s:%s" % (project_id, security_group) net = cls(network_id, network_str) net['user_id'] = user_id @@ -170,52 +181,65 @@ class BaseNetwork(datastore.BasicModel): @property def network(self): + """Returns a string representing the network""" return IPy.IP(self['network_str']) @property def netmask(self): + """Returns the netmask of this network""" return self.network.netmask() @property def gateway(self): + """Returns the network gateway address""" return self.network[1] @property def broadcast(self): + """Returns the network broadcast address""" return self.network.broadcast() @property def bridge_name(self): + """Returns the bridge associated with this network""" return "br%s" % (self["vlan"]) @property def user(self): + """Returns the user associated with this network""" return manager.AuthManager().get_user(self['user_id']) @property def project(self): + """Returns the project associated with this network""" return manager.AuthManager().get_project(self['project_id']) @property def _hosts_key(self): + """Datastore key where hosts are stored""" return "network:%s:hosts" % (self['network_str']) @property def hosts(self): + """Returns a hash of all hosts allocated in this network""" return datastore.Redis.instance().hgetall(self._hosts_key) or {} def _add_host(self, _user_id, _project_id, host, target): + """Add a host to the datastore""" datastore.Redis.instance().hset(self._hosts_key, host, target) def _rem_host(self, host): + """Remove a host from the datastore""" datastore.Redis.instance().hdel(self._hosts_key, host) @property def assigned(self): + """Returns a list of all assigned keys""" return datastore.Redis.instance().hkeys(self._hosts_key) @property def available(self): + """Returns a list of all available addresses in the network""" for idx in range(self.num_bottom_reserved_ips, len(self.network) - self.num_top_reserved_ips): address = str(self.network[idx]) @@ -224,15 +248,18 @@ class BaseNetwork(datastore.BasicModel): @property def num_bottom_reserved_ips(self): + """Returns number of ips reserved at the bottom of the range""" return 2 # Network, Gateway @property def num_top_reserved_ips(self): + """Returns number of ips reserved at the top of the range""" return 1 # Broadcast def allocate_ip(self, user_id, project_id, mac): + """Allocates an ip to a mac address""" for address in self.available: - logging.debug("Allocating IP %s to %s" % (address, project_id)) + logging.debug("Allocating IP %s to %s", address, project_id) self._add_host(user_id, project_id, address, mac) self.express(address=address) return address @@ -240,28 +267,37 @@ class BaseNetwork(datastore.BasicModel): (project_id, str(self.network))) def lease_ip(self, ip_str): - logging.debug("Leasing allocated IP %s" % (ip_str)) + """Called when DHCP lease is activated""" + logging.debug("Leasing allocated IP %s", ip_str) def release_ip(self, ip_str): + """Called when DHCP lease expires + + Removes the ip from the assigned list""" if not ip_str in self.assigned: raise exception.AddressNotAllocated() self._rem_host(ip_str) self.deexpress(address=ip_str) + logging.debug("Releasing IP %s", ip_str) def deallocate_ip(self, ip_str): + """Deallocates an allocated ip""" # NOTE(vish): Perhaps we should put the ip into an intermediate # state, so we know that we are pending waiting for # dnsmasq to confirm that it has been released. - pass + logging.debug("Deallocating allocated IP %s", ip_str) def list_addresses(self): + """List all allocated addresses""" for address in self.hosts: yield address def express(self, address=None): + """Set up network. Implemented in subclasses""" pass def deexpress(self, address=None): + """Tear down network. Implemented in subclasses""" pass @@ -286,7 +322,11 @@ class BridgedNetwork(BaseNetwork): override_type = 'network' @classmethod - def get_network_for_project(cls, user_id, project_id, security_group): + def get_network_for_project(cls, + user_id, + project_id, + security_group='default'): + """Returns network for a given project""" vlan = get_vlan_for_project(project_id) network_str = vlan.subnet() return cls.create(user_id, project_id, security_group, vlan.vlan_id, @@ -304,29 +344,14 @@ class BridgedNetwork(BaseNetwork): class DHCPNetwork(BridgedNetwork): - """ - properties: - dhcp_listen_address: the ip of the gateway / dhcp host - dhcp_range_start: the first ip to give out - dhcp_range_end: the last ip to give out - """ + """Network supporting DHCP""" bridge_gets_ip = True override_type = 'network' def __init__(self, *args, **kwargs): super(DHCPNetwork, self).__init__(*args, **kwargs) - # logging.debug("Initing DHCPNetwork object...") - self.dhcp_listen_address = self.gateway - self.dhcp_range_start = self.network[self.num_bottom_reserved_ips] - self.dhcp_range_end = self.network[-self.num_top_reserved_ips] - try: + if not(os.path.exists(FLAGS.networks_path)): os.makedirs(FLAGS.networks_path) - # NOTE(todd): I guess this is a lazy way to not have to check if the - # directory exists, but shouldn't we be smarter about - # telling the difference between existing directory and - # permission denied? (Errno 17 vs 13, OSError) - except Exception, err: - pass @property def num_bottom_reserved_ips(self): @@ -338,6 +363,16 @@ class DHCPNetwork(BridgedNetwork): return super(DHCPNetwork, self).num_top_reserved_ips + \ FLAGS.cnt_vpn_clients + @property + def dhcp_listen_address(self): + """Address where dhcp server should listen""" + return self.gateway + + @property + def dhcp_range_start(self): + """Starting address dhcp server should use""" + return self.network[self.num_bottom_reserved_ips] + def express(self, address=None): super(DHCPNetwork, self).express(address=address) if len(self.assigned) > 0: @@ -346,15 +381,17 @@ class DHCPNetwork(BridgedNetwork): linux_net.start_dnsmasq(self) else: logging.debug("Not launching dnsmasq: no hosts.") - self.express_cloudpipe() + self.express_vpn() def allocate_vpn_ip(self, user_id, project_id, mac): + """Allocates the reserved ip to a vpn instance""" address = str(self.network[2]) self._add_host(user_id, project_id, address, mac) self.express(address=address) return address - def express_cloudpipe(self): + def express_vpn(self): + """Sets up routing rules for vpn""" private_ip = str(self.network[2]) linux_net.confirm_rule("FORWARD -d %s -p udp --dport 1194 -j ACCEPT" % (private_ip, )) @@ -372,6 +409,7 @@ class DHCPNetwork(BridgedNetwork): class PublicAddress(datastore.BasicModel): + """Represents an elastic ip in the datastore""" override_type = "address" def __init__(self, address): @@ -387,6 +425,7 @@ class PublicAddress(datastore.BasicModel): @classmethod def create(cls, user_id, project_id, address): + """Creates a PublicAddress object""" addr = cls(address) addr['user_id'] = user_id addr['project_id'] = project_id @@ -400,12 +439,13 @@ DEFAULT_PORTS = [("tcp", 80), ("tcp", 22), ("udp", 1194), ("tcp", 443)] class PublicNetworkController(BaseNetwork): + """Handles elastic ips""" override_type = 'network' def __init__(self, *args, **kwargs): network_id = "public:default" super(PublicNetworkController, self).__init__(network_id, - FLAGS.public_range) + FLAGS.public_range, *args, **kwargs) self['user_id'] = "public" self['project_id'] = "public" self["create_time"] = time.strftime('%Y-%m-%dT%H:%M:%SZ', @@ -416,12 +456,14 @@ class PublicNetworkController(BaseNetwork): @property def host_objs(self): + """Returns assigned addresses as PublicAddress objects""" for address in self.assigned: yield PublicAddress(address) - def get_host(self, host): - if host in self.assigned: - return PublicAddress(host) + def get_host(self, public_ip): + """Returns a specific public ip as PublicAddress object""" + if public_ip in self.assigned: + return PublicAddress(public_ip) return None def _add_host(self, user_id, project_id, host, _target): @@ -437,9 +479,10 @@ class PublicNetworkController(BaseNetwork): self.release_ip(ip_str) def associate_address(self, public_ip, private_ip, instance_id): + """Associates a public ip to a private ip and instance id""" if not public_ip in self.assigned: raise exception.AddressNotAllocated() - # TODO(joshua): Keep an index going both ways + # TODO(josh): Keep an index going both ways for addr in self.host_objs: if addr.get('private_ip', None) == private_ip: raise exception.AddressAlreadyAssociated() @@ -452,6 +495,7 @@ class PublicNetworkController(BaseNetwork): self.express(address=public_ip) def disassociate_address(self, public_ip): + """Disassociates a public ip with its private ip""" if not public_ip in self.assigned: raise exception.AddressNotAllocated() addr = self.get_host(public_ip) @@ -476,7 +520,7 @@ class PublicNetworkController(BaseNetwork): % (public_ip, private_ip)) linux_net.confirm_rule("POSTROUTING -t nat -s %s -j SNAT --to %s" % (private_ip, public_ip)) - # TODO: Get these from the secgroup datastore entries + # TODO(joshua): Get these from the secgroup datastore entries linux_net.confirm_rule("FORWARD -d %s -p icmp -j ACCEPT" % (private_ip)) for (protocol, port) in DEFAULT_PORTS: @@ -503,9 +547,7 @@ class PublicNetworkController(BaseNetwork): # piece of architecture that mitigates it (only one queue # listener per net)? def get_vlan_for_project(project_id): - """ - Allocate vlan IDs to individual users. - """ + """Allocate vlan IDs to individual users""" vlan = Vlan.lookup(project_id) if vlan: return vlan @@ -538,7 +580,7 @@ def get_vlan_for_project(project_id): def get_project_network(project_id, security_group='default'): - """ get a project's private network, allocating one if needed """ + """Gets a project's private network, allocating one if needed""" project = manager.AuthManager().get_project(project_id) if not project: raise nova_exception.NotFound("Project %s doesn't exist." % project_id) @@ -549,26 +591,29 @@ def get_project_network(project_id, security_group='default'): def get_network_by_address(address): + """Gets the network for a given private ip""" # TODO(vish): This is completely the wrong way to do this, but # I'm getting the network binary working before I # tackle doing this the right way. - logging.debug("Get Network By Address: %s" % address) + logging.debug("Get Network By Address: %s", address) for project in manager.AuthManager().get_projects(): net = get_project_network(project.id) if address in net.assigned: - logging.debug("Found %s in %s" % (address, project.id)) + logging.debug("Found %s in %s", address, project.id) return net raise exception.AddressNotAllocated() def get_network_by_interface(iface, security_group='default'): + """Gets the network for a given interface""" vlan = iface.rpartition("br")[2] project_id = Vlan.dict_by_vlan().get(vlan) return get_project_network(project_id, security_group) def get_public_ip_for_instance(instance_id): - # FIXME: this should be a lookup - iteration won't scale + """Gets the public ip for a given instance""" + # FIXME(josh): this should be a lookup - iteration won't scale for address_record in PublicAddress.all(): if address_record.get('instance_id', 'available') == instance_id: return address_record['address'] diff --git a/nova/tests/network_unittest.py b/nova/tests/network_unittest.py index 9aa39e516..5671a8886 100644 --- a/nova/tests/network_unittest.py +++ b/nova/tests/network_unittest.py @@ -15,7 +15,9 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. - +""" +Unit Tests for network code +""" import IPy import os import logging @@ -33,7 +35,8 @@ FLAGS = flags.FLAGS class NetworkTestCase(test.TrialTestCase): - def setUp(self): + """Test cases for network code""" + def setUp(self): # pylint: disable=C0103 super(NetworkTestCase, self).setUp() # NOTE(vish): if you change these flags, make sure to change the # flags in the corresponding section in nova-dhcpbridge @@ -44,7 +47,6 @@ class NetworkTestCase(test.TrialTestCase): network_size=32) logging.getLogger().setLevel(logging.DEBUG) self.manager = manager.AuthManager() - self.dnsmasq = FakeDNSMasq() self.user = self.manager.create_user('netuser', 'netuser', 'netuser') self.projects = [] self.projects.append(self.manager.create_project('netuser', @@ -56,49 +58,48 @@ class NetworkTestCase(test.TrialTestCase): 'netuser', name)) vpn.NetworkData.create(self.projects[i].id) - self.network = model.PublicNetworkController() self.service = service.VlanNetworkService() - def tearDown(self): + def tearDown(self): # pylint: disable=C0103 super(NetworkTestCase, self).tearDown() for project in self.projects: self.manager.delete_project(project) self.manager.delete_user(self.user) def test_public_network_allocation(self): + """Makes sure that we can allocaate a public ip""" pubnet = IPy.IP(flags.FLAGS.public_range) - address = self.network.allocate_ip(self.user.id, - self.projects[0].id, - "public") + address = self.service.allocate_elastic_ip(self.user.id, + self.projects[0].id) self.assertTrue(IPy.IP(address) in pubnet) - self.assertTrue(IPy.IP(address) in self.network.network) def test_allocate_deallocate_fixed_ip(self): + """Makes sure that we can allocate and deallocate a fixed ip""" result = self.service.allocate_fixed_ip( self.user.id, self.projects[0].id) address = result['private_dns_name'] mac = result['mac_address'] - logging.debug("Was allocated %s" % (address)) net = model.get_project_network(self.projects[0].id, "default") self.assertEqual(True, is_in_project(address, self.projects[0].id)) hostname = "test-host" - self.dnsmasq.issue_ip(mac, address, hostname, net.bridge_name) - rv = self.service.deallocate_fixed_ip(address) + issue_ip(mac, address, hostname, net.bridge_name) + self.service.deallocate_fixed_ip(address) # Doesn't go away until it's dhcp released self.assertEqual(True, is_in_project(address, self.projects[0].id)) - self.dnsmasq.release_ip(mac, address, hostname, net.bridge_name) + release_ip(mac, address, hostname, net.bridge_name) self.assertEqual(False, is_in_project(address, self.projects[0].id)) - def test_range_allocation(self): - hostname = "test-host" - result = self.service.allocate_fixed_ip( - self.user.id, self.projects[0].id) + def test_side_effects(self): + """Ensures allocating and releasing has no side effects""" + hostname = "side-effect-host" + result = self.service.allocate_fixed_ip(self.user.id, + self.projects[0].id) mac = result['mac_address'] address = result['private_dns_name'] - result = self.service.allocate_fixed_ip( - self.user, self.projects[1].id) + result = self.service.allocate_fixed_ip(self.user, + self.projects[1].id) secondmac = result['mac_address'] secondaddress = result['private_dns_name'] @@ -111,25 +112,24 @@ class NetworkTestCase(test.TrialTestCase): self.assertEqual(False, is_in_project(address, self.projects[1].id)) # Addresses are allocated before they're issued - self.dnsmasq.issue_ip(mac, address, hostname, net.bridge_name) - self.dnsmasq.issue_ip(secondmac, secondaddress, - hostname, secondnet.bridge_name) + issue_ip(mac, address, hostname, net.bridge_name) + issue_ip(secondmac, secondaddress, hostname, secondnet.bridge_name) - rv = self.service.deallocate_fixed_ip(address) - self.dnsmasq.release_ip(mac, address, hostname, net.bridge_name) + self.service.deallocate_fixed_ip(address) + release_ip(mac, address, hostname, net.bridge_name) self.assertEqual(False, is_in_project(address, self.projects[0].id)) # First address release shouldn't affect the second self.assertEqual(True, is_in_project(secondaddress, self.projects[1].id)) - rv = self.service.deallocate_fixed_ip(secondaddress) - self.dnsmasq.release_ip(secondmac, secondaddress, - hostname, secondnet.bridge_name) + self.service.deallocate_fixed_ip(secondaddress) + release_ip(secondmac, secondaddress, hostname, secondnet.bridge_name) self.assertEqual(False, is_in_project(secondaddress, self.projects[1].id)) def test_subnet_edge(self): + """Makes sure that private ips don't overlap""" result = self.service.allocate_fixed_ip(self.user.id, self.projects[0].id) firstaddress = result['private_dns_name'] @@ -148,29 +148,34 @@ class NetworkTestCase(test.TrialTestCase): self.user, project_id) mac3 = result['mac_address'] address3 = result['private_dns_name'] + net = model.get_project_network(project_id, "default") + issue_ip(mac, address, hostname, net.bridge_name) + issue_ip(mac2, address2, hostname, net.bridge_name) + issue_ip(mac3, address3, hostname, net.bridge_name) self.assertEqual(False, is_in_project(address, self.projects[0].id)) self.assertEqual(False, is_in_project(address2, self.projects[0].id)) self.assertEqual(False, is_in_project(address3, self.projects[0].id)) - rv = self.service.deallocate_fixed_ip(address) - rv = self.service.deallocate_fixed_ip(address2) - rv = self.service.deallocate_fixed_ip(address3) - net = model.get_project_network(project_id, "default") - self.dnsmasq.release_ip(mac, address, hostname, net.bridge_name) - self.dnsmasq.release_ip(mac2, address2, hostname, net.bridge_name) - self.dnsmasq.release_ip(mac3, address3, hostname, net.bridge_name) + self.service.deallocate_fixed_ip(address) + self.service.deallocate_fixed_ip(address2) + self.service.deallocate_fixed_ip(address3) + release_ip(mac, address, hostname, net.bridge_name) + release_ip(mac2, address2, hostname, net.bridge_name) + release_ip(mac3, address3, hostname, net.bridge_name) net = model.get_project_network(self.projects[0].id, "default") - rv = self.service.deallocate_fixed_ip(firstaddress) - self.dnsmasq.release_ip(mac, firstaddress, hostname, net.bridge_name) + self.service.deallocate_fixed_ip(firstaddress) + release_ip(mac, firstaddress, hostname, net.bridge_name) def test_vpn_ip_and_port_looks_valid(self): + """Ensure the vpn ip and port are reasonable""" self.assert_(self.projects[0].vpn_ip) self.assert_(self.projects[0].vpn_port >= FLAGS.vpn_start_port) self.assert_(self.projects[0].vpn_port <= FLAGS.vpn_end_port) def test_too_many_vpns(self): + """Ensure error is raised if we run out of vpn ports""" vpns = [] for i in xrange(vpn.NetworkData.num_ports_for_ip(FLAGS.vpn_ip)): vpns.append(vpn.NetworkData.create("vpnuser%s" % i)) @@ -180,7 +185,6 @@ class NetworkTestCase(test.TrialTestCase): def test_ips_are_reused(self): """Makes sure that ip addresses that are deallocated get reused""" - result = self.service.allocate_fixed_ip( self.user.id, self.projects[0].id) mac = result['mac_address'] @@ -189,24 +193,18 @@ class NetworkTestCase(test.TrialTestCase): hostname = "reuse-host" net = model.get_project_network(self.projects[0].id, "default") - self.dnsmasq.issue_ip(mac, address, hostname, net.bridge_name) - rv = self.service.deallocate_fixed_ip(address) - self.dnsmasq.release_ip(mac, address, hostname, net.bridge_name) + issue_ip(mac, address, hostname, net.bridge_name) + self.service.deallocate_fixed_ip(address) + release_ip(mac, address, hostname, net.bridge_name) result = self.service.allocate_fixed_ip( self.user, self.projects[0].id) secondmac = result['mac_address'] secondaddress = result['private_dns_name'] self.assertEqual(address, secondaddress) - rv = self.service.deallocate_fixed_ip(secondaddress) - self.dnsmasq.issue_ip(secondmac, - secondaddress, - hostname, - net.bridge_name) - self.dnsmasq.release_ip(secondmac, - secondaddress, - hostname, - net.bridge_name) + self.service.deallocate_fixed_ip(secondaddress) + issue_ip(secondmac, secondaddress, hostname, net.bridge_name) + release_ip(secondmac, secondaddress, hostname, net.bridge_name) def test_available_ips(self): """Make sure the number of available ips for the network is correct @@ -242,47 +240,44 @@ class NetworkTestCase(test.TrialTestCase): self.projects[0].id) macs[i] = result['mac_address'] addresses[i] = result['private_dns_name'] - self.dnsmasq.issue_ip(macs[i], - addresses[i], - hostname, - net.bridge_name) + issue_ip(macs[i], addresses[i], hostname, net.bridge_name) self.assertEqual(len(list(net.available)), 0) self.assertRaises(NoMoreAddresses, self.service.allocate_fixed_ip, self.user.id, self.projects[0].id) for i in range(len(addresses)): - rv = self.service.deallocate_fixed_ip(addresses[i]) - self.dnsmasq.release_ip(macs[i], - addresses[i], - hostname, - net.bridge_name) + self.service.deallocate_fixed_ip(addresses[i]) + release_ip(macs[i], addresses[i], hostname, net.bridge_name) self.assertEqual(len(list(net.available)), num_available_ips) def is_in_project(address, project_id): + """Returns true if address is in specified project""" return address in model.get_project_network(project_id).list_addresses() def binpath(script): + """Returns the absolute path to a script in bin""" return os.path.abspath(os.path.join(__file__, "../../../bin", script)) -class FakeDNSMasq(object): - def issue_ip(self, mac, ip, hostname, interface): - cmd = "%s add %s %s %s" % (binpath('nova-dhcpbridge'), - mac, ip, hostname) - env = {'DNSMASQ_INTERFACE': interface, - 'TESTING': '1', - 'FLAGFILE': FLAGS.dhcpbridge_flagfile} - (out, err) = utils.execute(cmd, addl_env=env) - logging.debug("ISSUE_IP: %s, %s " % (out, err)) - - def release_ip(self, mac, ip, hostname, interface): - cmd = "%s del %s %s %s" % (binpath('nova-dhcpbridge'), - mac, ip, hostname) - env = {'DNSMASQ_INTERFACE': interface, - 'TESTING': '1', - 'FLAGFILE': FLAGS.dhcpbridge_flagfile} - (out, err) = utils.execute(cmd, addl_env=env) - logging.debug("RELEASE_IP: %s, %s " % (out, err)) +def issue_ip(mac, private_ip, hostname, interface): + """Run add command on dhcpbridge""" + cmd = "%s add %s %s %s" % (binpath('nova-dhcpbridge'), + mac, private_ip, hostname) + env = {'DNSMASQ_INTERFACE': interface, + 'TESTING': '1', + 'FLAGFILE': FLAGS.dhcpbridge_flagfile} + (out, err) = utils.execute(cmd, addl_env=env) + logging.debug("ISSUE_IP: %s, %s ", out, err) + +def release_ip(mac, private_ip, hostname, interface): + """Run del command on dhcpbridge""" + cmd = "%s del %s %s %s" % (binpath('nova-dhcpbridge'), + mac, private_ip, hostname) + env = {'DNSMASQ_INTERFACE': interface, + 'TESTING': '1', + 'FLAGFILE': FLAGS.dhcpbridge_flagfile} + (out, err) = utils.execute(cmd, addl_env=env) + logging.debug("RELEASE_IP: %s, %s ", out, err) -- cgit