get rid of network indexes and make networks into a pool

8 files changed, 158 insertions, 139 deletions

diff --git a/bin/nova-manage b/bin/nova-manage
index baa1cb4db..40d690e30 100755
--- a/bin/nova-manage
+++ b/bin/nova-manage
@@ -72,6 +72,7 @@ from nova import flags
 from nova import quota
 from nova import utils
 from nova.auth import manager
+from nova.network import manager as network_manager
 from nova.cloudpipe import pipelib
 
 
@@ -321,6 +322,25 @@ class FloatingIpCommands(object):
                                   floating_ip['address'],
                                   instance)
 
+class NetworkCommands(object):
+    """Class for managing networks."""
+
+    def create(self, num_networks=None, network_size=None,
+               vlan_start=None, vpn_start=None):
+        """Creates floating ips for host by range
+        arguments: [num_networks=FLAG], [network_size=FLAG],
+                   [vlan_start=FLAG, vpn_start=FLAG]"""
+        if not num_networks:
+            num_networks = FLAGS.num_networks
+        if not network_size:
+            network_size = FLAGS.network_size
+        if not vlan_start:
+            vlan_start = FLAGS.vlan_start
+        if not vpn_start:
+            vpn_start = FLAGS.vpn_start
+        net_manager = network_manager.VlanManager()
+        net_manager.create_networks(None, int(num_networks), int(network_size),
+                                    int(vlan_start), int(vpn_start))
 
 CATEGORIES = [
     ('user', UserCommands),
@@ -328,7 +348,8 @@ CATEGORIES = [
     ('role', RoleCommands),
     ('shell', ShellCommands),
     ('vpn', VpnCommands),
-    ('floating', FloatingIpCommands)
+    ('floating', FloatingIpCommands),
+    ('network', NetworkCommands)
 ]
 
 
@@ -394,9 +415,9 @@ def main():
         fn(*argv)
         sys.exit(0)
     except TypeError:
-        print "Wrong number of arguments supplied"
+        print "Possible wrong number of arguments supplied"
         print "%s %s: %s" % (category, action, fn.__doc__)
-        sys.exit(2)
+        raise
 
 if __name__ == '__main__':
     main()
diff --git a/nova/auth/manager.py b/nova/auth/manager.py
index 55fbf42aa..8cccfe306 100644
--- a/nova/auth/manager.py
+++ b/nova/auth/manager.py
@@ -484,12 +484,6 @@ class AuthManager(object):
                                               member_users)
             if project_dict:
                 project = Project(**project_dict)
-                try:
-                    self.network_manager.allocate_network(context,
-                                                          project.id)
-                except:
-                    drv.delete_project(project.id)
-                    raise
                 return project
 
     def modify_project(self, project, manager_user=None, description=None):
@@ -558,13 +552,6 @@ class AuthManager(object):
 
     def delete_project(self, project, context=None):
         """Deletes a project"""
-        try:
-            network_ref = db.project_get_network(context,
-                                                 Project.safe_id(project))
-            db.network_destroy(context, network_ref['id'])
-        except:
-            logging.exception('Could not destroy network for %s',
-                              project)
         with self.driver() as drv:
             drv.delete_project(Project.safe_id(project))
 
diff --git a/nova/db/api.py b/nova/db/api.py
index 848f5febf..1e0a2a5c8 100644
--- a/nova/db/api.py
+++ b/nova/db/api.py
@@ -337,6 +337,11 @@ def key_pair_get_all_by_user(context, user_id):
 ####################
 
 
+def network_associate(context, project_id):
+    """Associate a free network to a project."""
+    return IMPL.network_associate(context, project_id)
+
+
 def network_count(context):
     """Return the number of networks."""
     return IMPL.network_count(context)
@@ -357,9 +362,12 @@ def network_count_reserved_ips(context, network_id):
     return IMPL.network_count_reserved_ips(context, network_id)
 
 
-def network_create(context, values):
-    """Create a network from the values dictionary."""
-    return IMPL.network_create(context, values)
+def network_create_safe(context, values):
+    """Create a network from the values dict
+
+    The network is only returned if the create succeeds. If the create violates
+    constraints because the network already exists, no exception is raised."""
+    return IMPL.network_create_safe(context, values)
 
 
 def network_create_fixed_ips(context, network_id, num_vpn_clients):
@@ -367,9 +375,14 @@ def network_create_fixed_ips(context, network_id, num_vpn_clients):
     return IMPL.network_create_fixed_ips(context, network_id, num_vpn_clients)
 
 
-def network_destroy(context, network_id):
-    """Destroy the network or raise if it does not exist."""
-    return IMPL.network_destroy(context, network_id)
+def network_disassociate(context, network_id):
+    """Disassociate the network from project or raise if it does not exist."""
+    return IMPL.network_disassociate(context, network_id)
+
+
+def network_disassociate_all(context):
+    """Disassociate all networks from projects."""
+    return IMPL.network_disassociate_all(context)
 
 
 def network_get(context, network_id):
@@ -398,16 +411,6 @@ def network_get_vpn_ip(context, network_id):
     return IMPL.network_get_vpn_ip(context, network_id)
 
 
-def network_index_count(context):
-    """Return count of network indexes"""
-    return IMPL.network_index_count(context)
-
-
-def network_index_create(context, values):
-    """Create a network index from the values dict"""
-    return IMPL.network_index_create(context, values)
-
-
 def network_set_cidr(context, network_id, cidr):
     """Set the Classless Inner Domain Routing for the network"""
     return IMPL.network_set_cidr(context, network_id, cidr)
diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py
index b5caae940..dc7001b93 100644
--- a/nova/db/sqlalchemy/api.py
+++ b/nova/db/sqlalchemy/api.py
@@ -19,14 +19,13 @@
 Implementation of SQLAlchemy backend
 """
 
-import sys
-
 from nova import db
 from nova import exception
 from nova import flags
 from nova.db.sqlalchemy import models
 from nova.db.sqlalchemy.session import get_session
 from sqlalchemy import or_
+from sqlalchemy.exc import IntegrityError
 from sqlalchemy.orm import joinedload_all
 from sqlalchemy.sql import func
 
@@ -536,6 +535,24 @@ def key_pair_get_all_by_user(_context, user_id):
 ###################
 
 
+def network_associate(_context, project_id):
+    session = get_session()
+    with session.begin():
+        network_ref = session.query(models.Network
+                             ).filter_by(deleted=False
+                             ).filter_by(project_id=None
+                             ).with_lockmode('update'
+                             ).first()
+        # NOTE(vish): if with_lockmode isn't supported, as in sqlite,
+        #             then this has concurrency issues
+        if not network_ref:
+            raise db.NoMoreNetworks()
+        network_ref['project_id'] = project_id
+        session.add(network_ref)
+    return network_ref
+
+
+
 def network_count(_context):
     return models.Network.count()
 
@@ -568,30 +585,24 @@ def network_count_reserved_ips(_context, network_id):
                  ).count()
 
 
-def network_create(_context, values):
+def network_create_safe(_context, values):
     network_ref = models.Network()
     for (key, value) in values.iteritems():
         network_ref[key] = value
-    network_ref.save()
-    return network_ref
+    try:
+        network_ref.save()
+        return network_ref
+    except IntegrityError:
+        return None
+
+
+def network_disassociate(context, network_id):
+    db.network.update(context, network_id, {'project_id': None})
 
 
-def network_destroy(_context, network_id):
+def network_disassociate_all(context):
     session = get_session()
-    with session.begin():
-        # TODO(vish): do we have to use sql here?
-        session.execute('update networks set deleted=1 where id=:id',
-                        {'id': network_id})
-        session.execute('update fixed_ips set deleted=1 where network_id=:id',
-                        {'id': network_id})
-        session.execute('update floating_ips set deleted=1 '
-                        'where fixed_ip_id in '
-                        '(select id from fixed_ips '
-                        'where network_id=:id)',
-                        {'id': network_id})
-        session.execute('update network_indexes set network_id=NULL '
-                        'where network_id=:id',
-                        {'id': network_id})
+    session.execute('update networks set project_id=NULL')
 
 
 def network_get(_context, network_id):
@@ -622,33 +633,6 @@ def network_get_by_bridge(_context, bridge):
     return rv
 
 
-def network_get_index(_context, network_id):
-    session = get_session()
-    with session.begin():
-        network_index = session.query(models.NetworkIndex
-                              ).filter_by(network_id=None
-                              ).filter_by(deleted=False
-                              ).with_lockmode('update'
-                              ).first()
-        if not network_index:
-            raise db.NoMoreNetworks()
-        network_index['network'] = models.Network.find(network_id,
-                                                       session=session)
-        session.add(network_index)
-    return network_index['index']
-
-
-def network_index_count(_context):
-    return models.NetworkIndex.count()
-
-
-def network_index_create(_context, values):
-    network_index_ref = models.NetworkIndex()
-    for (key, value) in values.iteritems():
-        network_index_ref[key] = value
-    network_index_ref.save()
-
-
 def network_set_host(_context, network_id, host_id):
     session = get_session()
     with session.begin():
@@ -680,14 +664,23 @@ def network_update(_context, network_id, values):
 ###################
 
 
-def project_get_network(_context, project_id):
+def project_get_network(context, project_id):
     session = get_session()
     rv = session.query(models.Network
                ).filter_by(project_id=project_id
                ).filter_by(deleted=False
                ).first()
     if not rv:
-        raise exception.NotFound('No network for project: %s' % project_id)
+        try:
+            return db.network_associate(context, project_id)
+        except IntegrityError:
+            # NOTE(vish): We hit this if there is a race and two
+            #             processes are attempting to allocate the
+            #             network at the same time
+            rv = session.query(models.Network
+                       ).filter_by(project_id=project_id
+                       ).filter_by(deleted=False
+                       ).first()
     return rv
 
 
diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py
index f6ba7953f..cfbe474c6 100644
--- a/nova/db/sqlalchemy/models.py
+++ b/nova/db/sqlalchemy/models.py
@@ -25,7 +25,7 @@ import datetime
 
 # TODO(vish): clean up these imports
 from sqlalchemy.orm import relationship, backref, exc, object_mapper
-from sqlalchemy import Column, Integer, String
+from sqlalchemy import Column, Integer, String, schema
 from sqlalchemy import ForeignKey, DateTime, Boolean, Text
 from sqlalchemy.ext.declarative import declarative_base
 
@@ -363,10 +363,13 @@ class KeyPair(BASE, NovaBase):
 class Network(BASE, NovaBase):
     """Represents a network"""
     __tablename__ = 'networks'
+    __table_args__ = (schema.UniqueConstraint("vpn_public_address",
+                                              "vpn_public_port"),
+                      {'mysql_engine': 'InnoDB'})
     id = Column(Integer, primary_key=True)
 
     injected = Column(Boolean, default=False)
-    cidr = Column(String(255))
+    cidr = Column(String(255), unique=True)
     netmask = Column(String(255))
     bridge = Column(String(255))
     gateway = Column(String(255))
@@ -379,27 +382,16 @@ class Network(BASE, NovaBase):
     vpn_private_address = Column(String(255))
     dhcp_start = Column(String(255))
 
-    project_id = Column(String(255))
+    # NOTE(vish): The unique constraint below helps avoid a race condition
+    #             when associating a network, but it also means that we
+    #             can't associate two networks with one project.
+    project_id = Column(String(255), unique=True)
     host = Column(String(255))  # , ForeignKey('hosts.id'))
 
 
-class NetworkIndex(BASE, NovaBase):
-    """Represents a unique offset for a network
-
-    Currently vlan number, vpn port, and fixed ip ranges are keyed off of
-    this index. These may ultimately need to be converted to separate
-    pools.
-    """
-    __tablename__ = 'network_indexes'
-    id = Column(Integer, primary_key=True)
-    index = Column(Integer)
-    network_id = Column(Integer, ForeignKey('networks.id'), nullable=True)
-    network = relationship(Network, backref=backref('network_index',
-                                                    uselist=False))
-
 class AuthToken(BASE, NovaBase):
-    """Represents an authorization token for all API transactions. Fields 
-    are a string representing the actual token and a user id for mapping 
+    """Represents an authorization token for all API transactions. Fields
+    are a string representing the actual token and a user id for mapping
     to the actual user"""
     __tablename__ = 'auth_tokens'
     token_hash = Column(String(255), primary_key=True)
@@ -409,7 +401,6 @@ class AuthToken(BASE, NovaBase):
     cdn_management_url = Column(String(255))
 
 
-
 # TODO(vish): can these both come from the same baseclass?
 class FixedIp(BASE, NovaBase):
     """Represents a fixed ip for an instance"""
@@ -476,7 +467,7 @@ def register_models():
     """Register Models and create metadata"""
     from sqlalchemy import create_engine
     models = (Service, Instance, Volume, ExportDevice,
-              FixedIp, FloatingIp, Network, NetworkIndex,
+              FixedIp, FloatingIp, Network,
               AuthToken)  # , Image, Host)
     engine = create_engine(FLAGS.sql_connection, echo=False)
     for model in models:
diff --git a/nova/network/manager.py b/nova/network/manager.py
index b4c4a53b4..761a035a2 100644
--- a/nova/network/manager.py
+++ b/nova/network/manager.py
@@ -157,7 +157,7 @@ class NetworkManager(manager.Manager):
     def _create_fixed_ips(self, context, network_id):
         """Create all fixed ips for network"""
         network_ref = self.db.network_get(context, network_id)
-        # NOTE(vish): should these be properties of the network as opposed
+        # NOTE(vish): Should these be properties of the network as opposed
         #             to properties of the manager class?
         bottom_reserved = self._bottom_reserved_ips
         top_reserved = self._top_reserved_ips
@@ -308,35 +308,19 @@ class VlanManager(NetworkManager):
                 network_ref = self.db.fixed_ip_get_network(context, address)
                 self.driver.update_dhcp(context, network_ref['id'])
 
-    def allocate_network(self, context, project_id):
-        """Set up the network"""
-        self._ensure_indexes(context)
-        network_ref = db.network_create(context, {'project_id': project_id})
-        network_id = network_ref['id']
-        private_net = IPy.IP(FLAGS.private_range)
-        index = db.network_get_index(context, network_id)
-        vlan = FLAGS.vlan_start + index
-        start = index * FLAGS.network_size
-        significant_bits = 32 - int(math.log(FLAGS.network_size, 2))
-        cidr = "%s/%s" % (private_net[start], significant_bits)
-        project_net = IPy.IP(cidr)
 
-        net = {}
-        net['cidr'] = cidr
-        # NOTE(vish): we could turn these into properties
-        net['netmask'] = str(project_net.netmask())
-        net['gateway'] = str(project_net[1])
-        net['broadcast'] = str(project_net.broadcast())
-        net['vpn_private_address'] = str(project_net[2])
-        net['dhcp_start'] = str(project_net[3])
-        net['vlan'] = vlan
-        net['bridge'] = 'br%s' % vlan
-        net['vpn_public_address'] = FLAGS.vpn_ip
-        net['vpn_public_port'] = FLAGS.vpn_start + index
-        db.network_update(context, network_id, net)
-        self._create_fixed_ips(context, network_id)
+    def associate_network(self, context, project_id):
+        """Associates a network to a project"""
+        network_ref = db.network_associate(context, project_id)
+        network_id = network_ref['id']
         return network_id
 
+
+    def disassociate_network(self, context, network_id):
+        """Disassociates a newtwork from a project"""
+        db.network_disassocate(context, network_id)
+
+
     def setup_compute_network(self, context, project_id):
         """Sets up matching network for compute hosts"""
         network_ref = self.db.project_get_network(context, project_id)
@@ -348,17 +332,40 @@ class VlanManager(NetworkManager):
         # TODO(vish): Implement this
         pass
 
-    def _ensure_indexes(self, context):
-        """Ensure the indexes for the network exist
 
-        This could use a manage command instead of keying off of a flag"""
-        if not self.db.network_index_count(context):
-            for index in range(FLAGS.num_networks):
-                self.db.network_index_create(context, {'index': index})
+    def create_networks(self, context, num_networks, network_size,
+                        vlan_start, vpn_start):
+        """Create networks based on parameters"""
+        for index in range(num_networks):
+            private_net = IPy.IP(FLAGS.private_range)
+            vlan = vlan_start + index
+            start = index * network_size
+            significant_bits = 32 - int(math.log(network_size, 2))
+            cidr = "%s/%s" % (private_net[start], significant_bits)
+            project_net = IPy.IP(cidr)
+            net = {}
+            net['cidr'] = cidr
+            net['netmask'] = str(project_net.netmask())
+            net['gateway'] = str(project_net[1])
+            net['broadcast'] = str(project_net.broadcast())
+            net['vpn_private_address'] = str(project_net[2])
+            net['dhcp_start'] = str(project_net[3])
+            net['vlan'] = vlan
+            net['bridge'] = 'br%s' % vlan
+            # NOTE(vish): This makes ports unique accross the cloud, a more
+            #             robust solution would be to make them unique per ip
+            net['vpn_public_port'] = vpn_start + index
+            network_ref = self.db.network_create_safe(context, net)
+            if network_ref:
+                self._create_fixed_ips(context, network_ref['id'])
+
 
     def _on_set_network_host(self, context, network_id):
         """Called when this host becomes the host for a project"""
         network_ref = self.db.network_get(context, network_id)
+        net = {}
+        net['vpn_public_address'] = FLAGS.vpn_ip
+        db.network_update(context, network_id, net)
         self.driver.ensure_vlan_bridge(network_ref['vlan'],
                                        network_ref['bridge'],
                                        network_ref)
diff --git a/nova/test.py b/nova/test.py
index c392c8a84..a491c2902 100644
--- a/nova/test.py
+++ b/nova/test.py
@@ -31,8 +31,10 @@ from tornado import ioloop
 from twisted.internet import defer
 from twisted.trial import unittest
 
+from nova import db
 from nova import fakerabbit
 from nova import flags
+from nova.network import manager as network_manager
 
 
 FLAGS = flags.FLAGS
@@ -56,6 +58,13 @@ class TrialTestCase(unittest.TestCase):
     def setUp(self): # pylint: disable-msg=C0103
         """Run before each test method to initialize test environment"""
         super(TrialTestCase, self).setUp()
+        # NOTE(vish): We need a better method for creating fixtures for tests
+        #             now that we have some required db setup for the system
+        #             to work properly.
+        if db.network_count(None) != 5:
+            network_manager.VlanManager().create_networks(None, 5, 16,
+                                                          FLAGS.vlan_start,
+                                                          FLAGS.vpn_start)
 
         # emulate some of the mox stuff, we can't use the metaclass
         # because it screws with our generators
@@ -71,6 +80,7 @@ class TrialTestCase(unittest.TestCase):
         self.stubs.UnsetAll()
         self.stubs.SmartUnsetAll()
         self.mox.VerifyAll()
+        db.network_disassociate_all(None)
 
         if FLAGS.fake_rabbit:
             fakerabbit.reset_all()
diff --git a/nova/tests/network_unittest.py b/nova/tests/network_unittest.py
index da65b50a2..5ceb336ec 100644
--- a/nova/tests/network_unittest.py
+++ b/nova/tests/network_unittest.py
@@ -94,6 +94,7 @@ class NetworkTestCase(test.TrialTestCase):
         float_addr = self.network.allocate_floating_ip(self.context,
                                                        self.projects[0].id)
         fix_addr = self._create_address(0)
+        lease_ip(fix_addr)
         self.assertEqual(float_addr, str(pubnet[0]))
         self.network.associate_floating_ip(self.context, float_addr, fix_addr)
         address = db.instance_get_floating_address(None, self.instance_id)
@@ -103,6 +104,7 @@ class NetworkTestCase(test.TrialTestCase):
         self.assertEqual(address, None)
         self.network.deallocate_floating_ip(self.context, float_addr)
         self.network.deallocate_fixed_ip(self.context, fix_addr)
+        release_ip(fix_addr)
 
     def test_allocate_deallocate_fixed_ip(self):
         """Makes sure that we can allocate and deallocate a fixed ip"""
@@ -180,8 +182,8 @@ class NetworkTestCase(test.TrialTestCase):
             release_ip(address3)
         for instance_id in instance_ids:
             db.instance_destroy(None, instance_id)
-        release_ip(first)
         self.network.deallocate_fixed_ip(self.context, first)
+        release_ip(first)
 
     def test_vpn_ip_and_port_looks_valid(self):
         """Ensure the vpn ip and port are reasonable"""
@@ -197,10 +199,13 @@ class NetworkTestCase(test.TrialTestCase):
         for i in range(networks_left):
             project = self.manager.create_project('many%s' % i, self.user)
             projects.append(project)
+            db.project_get_network(None, project.id)
+        project = self.manager.create_project('last', self.user)
+        projects.append(project)
         self.assertRaises(db.NoMoreNetworks,
-                          self.manager.create_project,
-                          'boom',
-                          self.user)
+                          db.project_get_network,
+                          None,
+                          project.id)
         for project in projects:
             self.manager.delete_project(project)
 
@@ -213,7 +218,9 @@ class NetworkTestCase(test.TrialTestCase):
 
         address2 = self._create_address(0)
         self.assertEqual(address, address2)
+        lease_ip(address)
         self.network.deallocate_fixed_ip(self.context, address2)
+        release_ip(address)
 
     def test_available_ips(self):
         """Make sure the number of available ips for the network is correct