Fixes issues with allocation and deallocation of fixed and elastic addresses.

5 files changed, 45 insertions, 34 deletions

diff --git a/bin/nova-dhcpbridge b/bin/nova-dhcpbridge
index b1ad1c8fe..f70a4482c 100755
--- a/bin/nova-dhcpbridge
+++ b/bin/nova-dhcpbridge
@@ -69,7 +69,7 @@ def init_leases(interface):
     """Get the list of hosts for an interface."""
     net = model.get_network_by_interface(interface)
     res = ""
-    for address in net.address_objs:
+    for address in net.assigned_objs:
         res += "%s\n" % linux_net.host_dhcp(address)
     return res
 
diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py
index e17d60748..ba4b4b541 100644
--- a/nova/endpoint/cloud.py
+++ b/nova/endpoint/cloud.py
@@ -126,7 +126,7 @@ class CloudController(object):
         else:
             keys = ''
 
-        address_record = network_model.Address(i['private_dns_name'])
+        address_record = network_model.FixedIp(i['private_dns_name'])
         if address_record:
             hostname = address_record['hostname']
         else:
@@ -311,7 +311,7 @@ class CloudController(object):
 
     def _get_address(self, context, public_ip):
         # FIXME(vish) this should move into network.py
-        address = network_model.PublicAddress.lookup(public_ip)
+        address = network_model.ElasticIp.lookup(public_ip)
         if address and (context.user.is_admin() or address['project_id'] == context.project.id):
             return address
         raise exception.NotFound("Address at ip %s not found" % public_ip)
@@ -463,7 +463,7 @@ class CloudController(object):
 
     def format_addresses(self, context):
         addresses = []
-        for address in network_model.PublicAddress.all():
+        for address in network_model.ElasticIp.all():
             # TODO(vish): implement a by_project iterator for addresses
             if (context.user.is_admin() or
                 address['project_id'] == context.project.id):
diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index 4ebc2097b..15050adaf 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -116,7 +116,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,120s' % net.dhcp_range_start,
         ' --dhcp-hostsfile=%s' % dhcp_file(net['vlan'], 'conf'),
         ' --dhcp-script=%s' % bin_file('nova-dhcpbridge'),
         ' --leasefile-ro']
@@ -153,14 +153,10 @@ def start_dnsmasq(network):
         #           correct dnsmasq process
         try:
             os.kill(pid, signal.SIGHUP)
+            return
         except Exception as exc:  # pylint: disable=W0703
             logging.debug("Hupping dnsmasq threw %s", exc)
 
-    # otherwise delete the existing leases file and start dnsmasq
-    lease_file = dhcp_file(network['vlan'], 'leases')
-    if os.path.exists(lease_file):
-        os.unlink(lease_file)
-
     # FLAGFILE and DNSMASQ_INTERFACE in env
     env = {'FLAGFILE': FLAGS.dhcpbridge_flagfile,
            'DNSMASQ_INTERFACE': network['bridge_name']}
diff --git a/nova/network/model.py b/nova/network/model.py
index ce9345067..1a958b564 100644
--- a/nova/network/model.py
+++ b/nova/network/model.py
@@ -143,25 +143,26 @@ class Vlan(datastore.BasicModel):
                           network[start + FLAGS.network_size - 1])
 
 
-class Address(datastore.BasicModel):
+class FixedIp(datastore.BasicModel):
     """Represents a fixed ip in the datastore"""
-    override_type = "address"
 
     def __init__(self, address):
         self.address = address
-        super(Address, self).__init__()
+        super(FixedIp, self).__init__()
 
     @property
     def identifier(self):
         return self.address
 
+    # NOTE(vish): address states allocated, leased, deallocated
     def default_state(self):
-        return {'address': self.address}
+        return {'address': self.address,
+                'state': 'none'}
 
     @classmethod
     # pylint: disable=R0913
     def create(cls, user_id, project_id, address, mac, hostname, network_id):
-        """Creates an Address object"""
+        """Creates an FixedIp object"""
         addr = cls(address)
         addr['user_id'] = user_id
         addr['project_id'] = project_id
@@ -170,21 +171,22 @@ class Address(datastore.BasicModel):
             hostname = "ip-%s" % address.replace('.', '-')
         addr['hostname'] = hostname
         addr['network_id'] = network_id
+        addr['state'] = 'allocated'
         addr.save()
         return addr
 
     def save(self):
         is_new = self.is_new_record()
-        success = super(Address, self).save()
+        success = super(FixedIp, self).save()
         if success and is_new:
             self.associate_with("network", self['network_id'])
 
     def destroy(self):
         self.unassociate_with("network", self['network_id'])
-        super(Address, self).destroy()
+        super(FixedIp, self).destroy()
 
 
-class PublicAddress(Address):
+class ElasticIp(FixedIp):
     """Represents an elastic ip in the datastore"""
     override_type = "address"
 
@@ -200,7 +202,7 @@ class PublicAddress(Address):
 class BaseNetwork(datastore.BasicModel):
     """Implements basic logic for allocating ips in a network"""
     override_type = 'network'
-    address_class = Address
+    address_class = FixedIp
 
     @property
     def identifier(self):
@@ -268,12 +270,12 @@ class BaseNetwork(datastore.BasicModel):
     # pylint: disable=R0913
     def _add_host(self, user_id, project_id, ip_address, mac, hostname):
         """Add a host to the datastore"""
-        Address.create(user_id, project_id, ip_address,
+        self.address_class.create(user_id, project_id, ip_address,
                        mac, hostname, self.identifier)
 
     def _rem_host(self, ip_address):
         """Remove a host from the datastore"""
-        Address(ip_address).destroy()
+        self.address_class(ip_address).destroy()
 
     @property
     def assigned(self):
@@ -322,7 +324,13 @@ class BaseNetwork(datastore.BasicModel):
 
     def lease_ip(self, ip_str):
         """Called when DHCP lease is activated"""
-        logging.debug("Leasing allocated IP %s", ip_str)
+        if not ip_str in self.assigned:
+            raise exception.AddressNotAllocated()
+        address = self.get_address(ip_str)
+        if address:
+            logging.debug("Leasing allocated IP %s", ip_str)
+            address['state'] = 'leased'
+            address.save()
 
     def release_ip(self, ip_str):
         """Called when DHCP lease expires
@@ -330,16 +338,23 @@ class BaseNetwork(datastore.BasicModel):
         Removes the ip from the assigned list"""
         if not ip_str in self.assigned:
             raise exception.AddressNotAllocated()
+        logging.debug("Releasing IP %s", ip_str)
         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.
-        logging.debug("Deallocating allocated IP %s", ip_str)
+        if not ip_str in self.assigned:
+            raise exception.AddressNotAllocated()
+        address = self.get_address(ip_str)
+        if address:
+            if address['state'] != 'leased':
+                # NOTE(vish): address hasn't been leased, so release it
+                self.release_ip(ip_str)
+            else:
+                logging.debug("Deallocating allocated IP %s", ip_str)
+                address['state'] == 'deallocated'
+                address.save()
 
     def express(self, address=None):
         """Set up network.  Implemented in subclasses"""
@@ -462,7 +477,7 @@ DEFAULT_PORTS = [("tcp", 80), ("tcp", 22), ("udp", 1194), ("tcp", 443)]
 class PublicNetworkController(BaseNetwork):
     """Handles elastic ips"""
     override_type = 'network'
-    address_class = PublicAddress
+    address_class = ElasticIp
 
     def __init__(self, *args, **kwargs):
         network_id = "public:default"
@@ -597,7 +612,7 @@ def get_project_network(project_id, security_group='default'):
 
 def get_network_by_address(address):
     """Gets the network for a given private ip"""
-    address_record = Address.lookup(address)
+    address_record = FixedIp.lookup(address)
     if not address_record:
         raise exception.AddressNotAllocated()
     return get_project_network(address_record['project_id'])
@@ -613,6 +628,6 @@ def get_network_by_interface(iface, security_group='default'):
 def get_public_ip_for_instance(instance_id):
     """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():
+    for address_record in ElasticIp.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 9c0f5520b..625f20dd4 100644
--- a/nova/network/service.py
+++ b/nova/network/service.py
@@ -226,13 +226,13 @@ class VlanNetworkService(BaseNetworkService):
         """Returns an ip to the pool"""
         return model.get_network_by_address(fixed_ip).deallocate_ip(fixed_ip)
 
-    def lease_ip(self, address):
+    def lease_ip(self, fixed_ip):
         """Called by bridge when ip is leased"""
-        return model.get_network_by_address(address).lease_ip(address)
+        return model.get_network_by_address(fixed_ip).lease_ip(fixed_ip)
 
-    def release_ip(self, address):
+    def release_ip(self, fixed_ip):
         """Called by bridge when ip is released"""
-        return model.get_network_by_address(address).release_ip(address)
+        return model.get_network_by_address(fixed_ip).release_ip(fixed_ip)
 
     def restart_nets(self):
         """Ensure the network for each user is enabled"""