initial cleanup of tests for network

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 <n> 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:
@@ -389,13 +401,6 @@ class PublicNetworkController(BaseNetwork):
         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:
             yield PublicAddress(address)
@@ -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()