Actually pass in hostname and create a proper model for data in network code

8 files changed, 145 insertions, 133 deletions

diff --git a/bin/nova-dhcpbridge b/bin/nova-dhcpbridge
index 0dac2672a..b1ad1c8fe 100755
--- a/bin/nova-dhcpbridge
+++ b/bin/nova-dhcpbridge
@@ -69,8 +69,8 @@ def init_leases(interface):
     """Get the list of hosts for an interface."""
     net = model.get_network_by_interface(interface)
     res = ""
-    for fixed_ip in net.hosts:
-        res += "%s\n" % linux_net.host_dhcp(fixed_ip, net.hosts[fixed_ip])
+    for address in net.address_objs:
+        res += "%s\n" % linux_net.host_dhcp(address)
     return res
 
 
diff --git a/nova/compute/model.py b/nova/compute/model.py
index 94fe43c1a..266a93b9a 100644
--- a/nova/compute/model.py
+++ b/nova/compute/model.py
@@ -123,15 +123,7 @@ class Instance(datastore.BasicModel):
                 'node_name': 'unassigned',
                 'project_id': 'unassigned',
                 'user_id': 'unassigned',
-                'private_dns_name': 'unassigned',
-                'hostname': self.instance_id}
-
-
-    @property
-    def hostname(self):
-        # NOTE(vish): this is to be backward compatible with instances that may
-        #             not have been created with a hostname
-        return self.get('hostname', self.instance_id)
+                'private_dns_name': 'unassigned'}
 
     @property
     def identifier(self):
diff --git a/nova/datastore.py b/nova/datastore.py
index 51ef7a758..926e41f67 100644
--- a/nova/datastore.py
+++ b/nova/datastore.py
@@ -124,13 +124,17 @@ class BasicModel(object):
             yield cls(identifier)
 
     @classmethod
-    @absorb_connection_error
     def associated_to(cls, foreign_type, foreign_id):
-        redis_set = cls._redis_association_name(foreign_type, foreign_id)
-        for identifier in Redis.instance().smembers(redis_set):
+        for identifier in cls.associated_keys(foreign_type, foreign_id):
             yield cls(identifier)
 
     @classmethod
+    @absorb_connection_error
+    def associated_keys(cls, foreign_type, foreign_id):
+        redis_set = cls._redis_association_name(foreign_type, foreign_id)
+        return Redis.instance().smembers(redis_set) or []
+
+    @classmethod
     def _redis_set_name(cls, kls_name):
         # stupidly pluralize (for compatiblity with previous codebase)
         return kls_name.lower() + "s"
@@ -138,7 +142,7 @@ class BasicModel(object):
     @classmethod
     def _redis_association_name(cls, foreign_type, foreign_id):
         return cls._redis_set_name("%s:%s:%s" %
-                                   (foreign_type, foreign_id, cls.__name__))
+                                   (foreign_type, foreign_id, cls._redis_name()))
 
     @property
     def identifier(self):
diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py
index 26071cfed..c79e96f5d 100644
--- a/nova/endpoint/cloud.py
+++ b/nova/endpoint/cloud.py
@@ -125,6 +125,12 @@ class CloudController(object):
             }
         else:
             keys = ''
+
+        address_record = network_model.Address(i['private_dns_name'])
+        if address_record:
+            hostname = address_record['hostname']
+        else:
+            hostname = 'ip-%s' % i['private_dns_name'].replace('.', '-')
         data = {
             'user-data': base64.b64decode(i['user_data']),
             'meta-data': {
@@ -137,17 +143,17 @@ class CloudController(object):
                     'root': '/dev/sda1',
                     'swap': 'sda3'
                 },
-                'hostname': i.hostname,
+                'hostname': hostname,
                 'instance-action': 'none',
                 'instance-id': i['instance_id'],
                 'instance-type': i.get('instance_type', ''),
-                'local-hostname': i.hostname,
+                'local-hostname': hostname,
                 'local-ipv4': i['private_dns_name'], # TODO: switch to IP
                 'kernel-id': i.get('kernel_id', ''),
                 'placement': {
                     'availaibility-zone': i.get('availability_zone', 'nova'),
                 },
-                'public-hostname': i.hostname,
+                'public-hostname': hostname,
                 'public-ipv4': i.get('dns_name', ''), # TODO: switch to IP
                 'public-keys': keys,
                 'ramdisk-id': i.get('ramdisk_id', ''),
@@ -563,14 +569,15 @@ class CloudController(object):
             is_vpn = False
             if image_id  == FLAGS.vpn_image_id:
                 is_vpn = True
+            inst = self.instdir.new()
             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,
-                               "is_vpn": is_vpn}})
+                               "is_vpn": is_vpn,
+                               "hostname": inst.instance_id}})
             allocate_data = allocate_result['result']
-            inst = self.instdir.new()
             inst['image_id'] = image_id
             inst['kernel_id'] = kernel_id
             inst['ramdisk_id'] = ramdisk_id
@@ -584,6 +591,7 @@ class CloudController(object):
             inst['project_id'] = context.project.id
             inst['ami_launch_index'] = num
             inst['security_group'] = security_group
+            inst['hostname'] = inst.instance_id
             for (key, value) in allocate_data.iteritems():
                 inst[key] = value
 
diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index 8a8fff225..4ebc2097b 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -1,5 +1,3 @@
-# vim: tabstop=4 shiftwidth=4 softtabstop=4
-
 # Copyright 2010 United States Government as represented by the
 # Administrator of the National Aeronautics and Space Administration.
 # All Rights Reserved.
@@ -27,7 +25,6 @@ import os
 
 from nova import flags
 from nova import utils
-from nova.compute import model
 
 FLAGS = flags.FLAGS
 
@@ -126,14 +123,11 @@ def _dnsmasq_cmd(net):
     return ''.join(cmd)
 
 
-def host_dhcp(fixed_ip, mac):
-    """Return a host string for a fixed_ip and mac"""
-    instance = model.InstanceDirectory().by_ip(fixed_ip)
-    if instance is None:
-        hostname = 'ip-%s' % fixed_ip.replace('.', '-')
-    else:
-        hostname = instance.hostname
-    return "%s,%s.novalocal,%s" % (mac, hostname, fixed_ip)
+def host_dhcp(address):
+    """Return a host string for an address object"""
+    return "%s,%s.novalocal,%s" % (address['mac'],
+                                   address['hostname'],
+                                   address.address)
 
 
 # TODO(ja): if the system has restarted or pid numbers have wrapped
@@ -148,8 +142,8 @@ def start_dnsmasq(network):
     signal causing it to reload, otherwise spawn a new instance
     """
     with open(dhcp_file(network['vlan'], 'conf'), 'w') as f:
-        for fixed_ip in network.hosts:
-            f.write("%s\n" % host_dhcp(fixed_ip, network.hosts[fixed_ip]))
+        for address in network.assigned_objs:
+            f.write("%s\n" % host_dhcp(address))
 
     pid = dnsmasq_pid_for(network)
 
diff --git a/nova/network/model.py b/nova/network/model.py
index 7b1e16f26..ce9345067 100644
--- a/nova/network/model.py
+++ b/nova/network/model.py
@@ -143,13 +143,64 @@ class Vlan(datastore.BasicModel):
                           network[start + FLAGS.network_size - 1])
 
 
+class Address(datastore.BasicModel):
+    """Represents a fixed ip in the datastore"""
+    override_type = "address"
+
+    def __init__(self, address):
+        self.address = address
+        super(Address, self).__init__()
+
+    @property
+    def identifier(self):
+        return self.address
+
+    def default_state(self):
+        return {'address': self.address}
+
+    @classmethod
+    # pylint: disable=R0913
+    def create(cls, user_id, project_id, address, mac, hostname, network_id):
+        """Creates an Address object"""
+        addr = cls(address)
+        addr['user_id'] = user_id
+        addr['project_id'] = project_id
+        addr['mac'] = mac
+        if hostname is None:
+            hostname = "ip-%s" % address.replace('.', '-')
+        addr['hostname'] = hostname
+        addr['network_id'] = network_id
+        addr.save()
+        return addr
+
+    def save(self):
+        is_new = self.is_new_record()
+        success = super(Address, 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()
+
+
+class PublicAddress(Address):
+    """Represents an elastic ip in the datastore"""
+    override_type = "address"
+
+    def default_state(self):
+        return {'address': self.address,
+                'instance_id': 'available',
+                'private_ip': 'available'}
+
+
 # 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?
 class BaseNetwork(datastore.BasicModel):
     """Implements basic logic for allocating ips in a network"""
     override_type = 'network'
+    address_class = Address
 
     @property
     def identifier(self):
@@ -214,28 +265,31 @@ class BaseNetwork(datastore.BasicModel):
         """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):
+    # pylint: disable=R0913
+    def _add_host(self, user_id, project_id, ip_address, mac, hostname):
         """Add a host to the datastore"""
-        datastore.Redis.instance().hset(self._hosts_key, host, target)
+        Address.create(user_id, project_id, ip_address,
+                       mac, hostname, self.identifier)
 
-    def _rem_host(self, host):
+    def _rem_host(self, ip_address):
         """Remove a host from the datastore"""
-        datastore.Redis.instance().hdel(self._hosts_key, host)
+        Address(ip_address).destroy()
 
     @property
     def assigned(self):
-        """Returns a list of all assigned keys"""
-        return datastore.Redis.instance().hkeys(self._hosts_key)
+        """Returns a list of all assigned addresses"""
+        return self.address_class.associated_keys('network', self.identifier)
+
+    @property
+    def assigned_objs(self):
+        """Returns a list of all assigned addresses as objects"""
+        return self.address_class.associated_to('network', self.identifier)
+
+    def get_address(self, ip_address):
+        """Returns a specific ip as an object"""
+        if ip_address in self.assigned:
+            return self.address_class(ip_address)
+        return None
 
     @property
     def available(self):
@@ -243,7 +297,7 @@ class BaseNetwork(datastore.BasicModel):
         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():
+            if not address in self.assigned:
                 yield address
 
     @property
@@ -256,11 +310,11 @@ class BaseNetwork(datastore.BasicModel):
         """Returns number of ips reserved at the top of the range"""
         return 1  # Broadcast
 
-    def allocate_ip(self, user_id, project_id, mac):
+    def allocate_ip(self, user_id, project_id, mac, hostname=None):
         """Allocates an ip to a mac address"""
         for address in self.available:
             logging.debug("Allocating IP %s to %s", address, project_id)
-            self._add_host(user_id, project_id, address, mac)
+            self._add_host(user_id, project_id, address, mac, hostname)
             self.express(address=address)
             return address
         raise exception.NoMoreAddresses("Project %s with network %s" %
@@ -287,11 +341,6 @@ class BaseNetwork(datastore.BasicModel):
         #             dnsmasq to confirm that it has been released.
         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
@@ -383,10 +432,10 @@ class DHCPNetwork(BridgedNetwork):
             logging.debug("Not launching dnsmasq: no hosts.")
         self.express_vpn()
 
-    def allocate_vpn_ip(self, user_id, project_id, mac):
+    def allocate_vpn_ip(self, user_id, project_id, mac, hostname=None):
         """Allocates the reserved ip to a vpn instance"""
         address = str(self.network[2])
-        self._add_host(user_id, project_id, address, mac)
+        self._add_host(user_id, project_id, address, mac, hostname)
         self.express(address=address)
         return address
 
@@ -407,40 +456,13 @@ class DHCPNetwork(BridgedNetwork):
         else:
             linux_net.start_dnsmasq(self)
 
-
-class PublicAddress(datastore.BasicModel):
-    """Represents an elastic ip in the datastore"""
-    override_type = "address"
-
-    def __init__(self, address):
-        self.address = address
-        super(PublicAddress, self).__init__()
-
-    @property
-    def identifier(self):
-        return self.address
-
-    def default_state(self):
-        return {'address': self.address}
-
-    @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
-        addr['instance_id'] = 'available'
-        addr['private_ip'] = 'available'
-        addr.save()
-        return addr
-
-
 DEFAULT_PORTS = [("tcp", 80), ("tcp", 22), ("udp", 1194), ("tcp", 443)]
 
 
 class PublicNetworkController(BaseNetwork):
     """Handles elastic ips"""
     override_type = 'network'
+    address_class = PublicAddress
 
     def __init__(self, *args, **kwargs):
         network_id = "public:default"
@@ -454,26 +476,6 @@ class PublicNetworkController(BaseNetwork):
         self.save()
         self.express()
 
-    @property
-    def host_objs(self):
-        """Returns assigned addresses as PublicAddress objects"""
-        for address in self.assigned:
-            yield PublicAddress(address)
-
-    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):
-        datastore.Redis.instance().hset(self._hosts_key, host, project_id)
-        PublicAddress.create(user_id, project_id, host)
-
-    def _rem_host(self, host):
-        PublicAddress(host).destroy()
-        datastore.Redis.instance().hdel(self._hosts_key, host)
-
     def deallocate_ip(self, ip_str):
         # NOTE(vish): cleanup is now done on release by the parent class
         self.release_ip(ip_str)
@@ -483,10 +485,10 @@ class PublicNetworkController(BaseNetwork):
         if not public_ip in self.assigned:
             raise exception.AddressNotAllocated()
         # TODO(josh): Keep an index going both ways
-        for addr in self.host_objs:
+        for addr in self.assigned_objs:
             if addr.get('private_ip', None) == private_ip:
                 raise exception.AddressAlreadyAssociated()
-        addr = self.get_host(public_ip)
+        addr = self.get_address(public_ip)
         if addr.get('private_ip', 'available') != 'available':
             raise exception.AddressAlreadyAssociated()
         addr['private_ip'] = private_ip
@@ -498,7 +500,7 @@ class PublicNetworkController(BaseNetwork):
         """Disassociates a public ip with its private ip"""
         if not public_ip in self.assigned:
             raise exception.AddressNotAllocated()
-        addr = self.get_host(public_ip)
+        addr = self.get_address(public_ip)
         if addr.get('private_ip', 'available') == 'available':
             raise exception.AddressNotAssociated()
         self.deexpress(address=public_ip)
@@ -507,9 +509,12 @@ class PublicNetworkController(BaseNetwork):
         addr.save()
 
     def express(self, address=None):
-        addresses = self.host_objs
         if address:
-            addresses = [self.get_host(address)]
+            if not address in self.assigned:
+                raise exception.AddressNotAllocated()
+            addresses = [self.get_address(address)]
+        else:
+            addresses = self.assigned_objs
         for addr in addresses:
             if addr.get('private_ip', 'available') == 'available':
                 continue
@@ -529,7 +534,7 @@ class PublicNetworkController(BaseNetwork):
                     % (private_ip, protocol, port))
 
     def deexpress(self, address=None):
-        addr = self.get_host(address)
+        addr = self.get_address(address)
         private_ip = addr['private_ip']
         linux_net.unbind_public_ip(address, FLAGS.public_interface)
         linux_net.remove_rule("PREROUTING -t nat -d %s -j DNAT --to %s"
@@ -592,16 +597,10 @@ 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)
-    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)
-            return net
-    raise exception.AddressNotAllocated()
+    address_record = Address.lookup(address)
+    if not address_record:
+        raise exception.AddressNotAllocated()
+    return get_project_network(address_record['project_id'])
 
 
 def get_network_by_interface(iface, security_group='default'):
diff --git a/nova/network/service.py b/nova/network/service.py
index fd45496c9..9c0f5520b 100644
--- a/nova/network/service.py
+++ b/nova/network/service.py
@@ -152,7 +152,9 @@ class FlatNetworkService(BaseNetworkService):
         """Network is created manually"""
         pass
 
-    def allocate_fixed_ip(self, user_id, project_id,
+    def allocate_fixed_ip(self,
+                          user_id,
+                          project_id,
                           security_group='default',
                           *args, **kwargs):
         """Gets a fixed ip from the pool
@@ -161,7 +163,7 @@ class FlatNetworkService(BaseNetworkService):
         """
         # NOTE(vish): Some automation could be done here.  For example,
         #             creating the flat_network_bridge and setting up
-        #             a gateway.  This is all done manually atm
+        #             a gateway.  This is all done manually atm.
         redis = datastore.Redis.instance()
         if not redis.exists('ips') and not len(redis.keys('instances:*')):
             for fixed_ip in FLAGS.flat_network_ips:
@@ -169,6 +171,8 @@ class FlatNetworkService(BaseNetworkService):
         fixed_ip = redis.spop('ips')
         if not fixed_ip:
             raise exception.NoMoreAddresses()
+        # TODO(vish): some sort of dns handling for hostname should
+        #             probably be done here.
         return {'inject_network': True,
                 'network_type': FLAGS.network_type,
                 'mac_address': utils.generate_mac(),
@@ -192,16 +196,26 @@ class VlanNetworkService(BaseNetworkService):
     #             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,
+    def allocate_fixed_ip(self,
+                          user_id,
+                          project_id,
                           security_group='default',
-                          is_vpn=False, *args, **kwargs):
+                          is_vpn=False,
+                          hostname=None,
+                          *args, **kwargs):
         """Gets a fixed ip from the pool"""
         mac = utils.generate_mac()
         net = model.get_project_network(project_id)
         if is_vpn:
-            fixed_ip = net.allocate_vpn_ip(user_id, project_id, mac)
+            fixed_ip = net.allocate_vpn_ip(user_id,
+                                           project_id,
+                                           mac,
+                                           hostname)
         else:
-            fixed_ip = net.allocate_ip(user_id, project_id, mac)
+            fixed_ip = net.allocate_ip(user_id,
+                                       project_id,
+                                       mac,
+                                       hostname)
         return {'network_type': FLAGS.network_type,
                 'bridge_name': net['bridge_name'],
                 'mac_address': mac,
diff --git a/nova/tests/network_unittest.py b/nova/tests/network_unittest.py
index 5671a8886..039509809 100644
--- a/nova/tests/network_unittest.py
+++ b/nova/tests/network_unittest.py
@@ -202,8 +202,8 @@ class NetworkTestCase(test.TrialTestCase):
         secondmac = result['mac_address']
         secondaddress = result['private_dns_name']
         self.assertEqual(address, secondaddress)
-        self.service.deallocate_fixed_ip(secondaddress)
         issue_ip(secondmac, secondaddress, hostname, net.bridge_name)
+        self.service.deallocate_fixed_ip(secondaddress)
         release_ip(secondmac, secondaddress, hostname, net.bridge_name)
 
     def test_available_ips(self):
@@ -218,7 +218,7 @@ class NetworkTestCase(test.TrialTestCase):
         services (network, gateway, CloudPipe, broadcast)
         """
         net = model.get_project_network(self.projects[0].id, "default")
-        num_preallocated_ips = len(net.hosts.keys())
+        num_preallocated_ips = len(net.assigned)
         net_size = flags.FLAGS.network_size
         num_available_ips = net_size - (net.num_bottom_reserved_ips +
                                         num_preallocated_ips +
@@ -254,7 +254,7 @@ class NetworkTestCase(test.TrialTestCase):
 
 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()
+    return address in model.get_project_network(project_id).assigned
 
 
 def binpath(script):
@@ -272,6 +272,7 @@ def issue_ip(mac, private_ip, hostname, interface):
     (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'),