Merge lp:~vishvananda/nova/remove-network-index into lp:~hudson-openstack/nova/trunk

[Vish Ishaya]

This patch removes the ugly network_index that is used by VlanManager and turns network itself into a pool. It adds support for creating the networks through an api command:
    nova-manage network create # creates all of the networks defined by flags
or
    nova-manage network create 5 # create the first five networks

This moves the network out of project.create, associating a network to a project lazily in project_get_network. This allows for a bit greater control over networks. There are a couple outstanding issues:
   1) Where do networks get disassociated? Right now I'm not disassociating at all. We're sharing ldap users across different deploys, whereas sql db is unique per deploy. Perhaps the solution is to have a flag like disassociate_on_project_destroy and make it optional?
   2) How do we handle creation of networks for tests? It is really slow to create a pool of networks and fixed_ips for each test case. Right now I'm taking advantage of the fact that the db isn't cleaned between tests, and creating them if they don't exist in BaseTestCase.SetUp() and disassociating in TearDown(). Ideally this should be replaced with fixtures of some sort.

Comments on these two issues are welcome.

[Jay Pipes (jaypipes)]

"1) Where do networks get disassociated? Right now I'm not disassociating at all. We're sharing ldap users across different deploys, whereas sql db is unique per deploy. Perhaps the solution is to have a flag like disassociate_on_project_destroy and make it optional?"

I think that is an excellent option. ++ on a flag to determine when disassociation occurs.

"2) How do we handle creation of networks for tests? It is really slow to create a pool of networks and fixed_ips for each test case. Right now I'm taking advantage of the fact that the db isn't cleaned between tests, and creating them if they don't exist in BaseTestCase.SetUp() and disassociating in TearDown(). Ideally this should be replaced with fixtures of some sort."

Well, there's a ton to talk about with the current testing systems at the next summit :) I say "systems" because the new rackspace stuff is using nosetests, and the rest is using the nova.test.BaseTestCase or unittest.TestCase.

I'd love to do a discuss the future of how we test OS projects at the summit...including stuff like how we establish fixtures and configuration-dependent environments.

-jay

[Jay Pipes (jaypipes)]

lgtm.

review: Approve

[lp:~vishvananda/nova/remove-network-index]

297. By Vish Ishaya on 2010-10-01

merged trunk

298. By Vish Ishaya on 2010-10-01

Fixed flat network manager with network index gone.

Both managers use ips created through nova manage.

Use of project_get_network is minimized to make way for managers that
would prefer to use cluste or host based ips instead of project based ips.

299. By Vish Ishaya on 2010-10-02

fix typo in setup_compute_network

[Jay Pipes (jaypipes)]

ping. vishy, looks like you need to merge with trunk and resolve conflicts from devin's auth work...

review: Needs Fixing

[lp:~vishvananda/nova/remove-network-index]

300. By Vish Ishaya on 2010-10-05

merged trunk and fixed tests

301. By Vish Ishaya on 2010-10-13

merged trunk

302. By Vish Ishaya on 2010-10-13

fix tests

303. By Vish Ishaya on 2010-10-13

super teardown

304. By Vish Ishaya on 2010-10-13

cleanup leftover addresses

[Rick Clark (dendrobates)]

lgtm

review: Approve

=== modified file 'bin/nova-manage'
--- bin/nova-manage	2010-10-12 14:23:24 +0000
+++ bin/nova-manage	2010-10-13 07:50:58 +0000
@@ -73,6 +73,7 @@
 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
 
 
@@ -377,6 +378,29 @@
                                   floating_ip['address'],
                                   instance)
 
+class NetworkCommands(object):
+    """Class for managing networks."""
+
+    def create(self, fixed_range=None, num_networks=None,
+               network_size=None, vlan_start=None, vpn_start=None):
+        """Creates fixed ips for host by range
+        arguments: [fixed_range=FLAG], [num_networks=FLAG],
+                   [network_size=FLAG], [vlan_start=FLAG],
+                   [vpn_start=FLAG]"""
+        if not fixed_range:
+            fixed_range = FLAGS.fixed_range
+        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 = utils.import_object(FLAGS.network_manager)
+        net_manager.create_networks(None, fixed_range, int(num_networks),
+                                    int(network_size), int(vlan_start),
+                                    int(vpn_start))
 
 CATEGORIES = [
     ('user', UserCommands),
@@ -384,7 +408,8 @@
     ('role', RoleCommands),
     ('shell', ShellCommands),
     ('vpn', VpnCommands),
-    ('floating', FloatingIpCommands)
+    ('floating', FloatingIpCommands),
+    ('network', NetworkCommands)
 ]
 
 
@@ -454,9 +479,9 @@
         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()

=== modified file 'nova/api/ec2/cloud.py'
--- nova/api/ec2/cloud.py	2010-10-12 07:24:33 +0000
+++ nova/api/ec2/cloud.py	2010-10-13 07:50:58 +0000
@@ -734,13 +734,13 @@
 
     def _get_network_topic(self, context):
         """Retrieves the network host for a project"""
-        network_ref = db.project_get_network(context, context.project.id)
+        network_ref = self.network_manager.get_network(context)
         host = network_ref['host']
         if not host:
             host = rpc.call(FLAGS.network_topic,
                                   {"method": "set_network_host",
                                    "args": {"context": None,
-                                            "project_id": context.project.id}})
+                                            "network_id": network_ref['id']}})
         return db.queue_get_for(context, FLAGS.network_topic, host)
 
     def _ensure_default_security_group(self, context):
@@ -851,12 +851,13 @@
             ec2_id = internal_id_to_ec2_id(internal_id)
             inst['hostname'] = ec2_id
             db.instance_update(context, inst_id, inst)
+            # TODO(vish): This probably should be done in the scheduler
+            #             or in compute as a call.  The network should be
+            #             allocated after the host is assigned and setup
+            #             can happen at the same time.
             address = self.network_manager.allocate_fixed_ip(context,
                                                              inst_id,
                                                              vpn)
-
-            # TODO(vish): This probably should be done in the scheduler
-            #             network is setup when host is assigned
             network_topic = self._get_network_topic(context)
             rpc.call(network_topic,
                      {"method": "setup_fixed_ip",

=== modified file 'nova/api/openstack/servers.py'
--- nova/api/openstack/servers.py	2010-10-08 21:08:48 +0000
+++ nova/api/openstack/servers.py	2010-10-13 07:50:58 +0000
@@ -48,9 +48,9 @@
     return dict(servers=entities)
 
 def _entity_detail(inst):
-    """ Maps everything to valid attributes for return"""
-    power_mapping = { 
-        power_state.NOSTATE:  'build', 
+    """ Maps everything to Rackspace-like attributes for return"""
+    power_mapping = {
+        power_state.NOSTATE:  'build',
         power_state.RUNNING:  'active',
         power_state.BLOCKED:  'active',
         power_state.PAUSED:   'suspended',
@@ -60,7 +60,7 @@
     }
     inst_dict = {}
 
-    mapped_keys = dict(status='state', imageId='image_id', 
+    mapped_keys = dict(status='state', imageId='image_id',
         flavorId='instance_type', name='server_name', id='id')
 
     for k, v in mapped_keys.iteritems():
@@ -83,7 +83,7 @@
     _serialization_metadata = {
         'application/xml': {
             "attributes": {
-                "server": [ "id", "imageId", "name", "flavorId", "hostId", 
+                "server": [ "id", "imageId", "name", "flavorId", "hostId",
                             "status", "progress", "progress" ]
             }
         }
@@ -155,7 +155,7 @@
         user_id = req.environ['nova.context']['user']['id']
 
         inst_dict = self._deserialize(req.body, req)
-        
+
         if not inst_dict:
             return faults.Fault(exc.HTTPUnprocessableEntity())
 
@@ -163,12 +163,12 @@
         if not instance or instance.user_id != user_id:
             return faults.Fault(exc.HTTPNotFound())
 
-        self.db_driver.instance_update(None, int(id), 
+        self.db_driver.instance_update(None, int(id),
             _filter_params(inst_dict['server']))
         return faults.Fault(exc.HTTPNoContent())
 
     def action(self, req, id):
-        """ multi-purpose method used to reboot, rebuild, and 
+        """ multi-purpose method used to reboot, rebuild, and
         resize a server """
         user_id = req.environ['nova.context']['user']['id']
         input_dict = self._deserialize(req.body, req)
@@ -195,12 +195,11 @@
             if v['flavorid'] == flavor_id][0]
 
         image_id = env['server']['imageId']
-        
         img_service = utils.import_object(FLAGS.image_service)
 
         image = img_service.show(image_id)
 
-        if not image: 
+        if not image:
             raise Exception, "Image not found"
 
         inst['server_name'] = env['server']['name']
@@ -236,15 +235,14 @@
 
         ref = self.db_driver.instance_create(None, inst)
         inst['id'] = ref.internal_id
-        
         # TODO(dietz): this isn't explicitly necessary, but the networking
         # calls depend on an object with a project_id property, and therefore
         # should be cleaned up later
         api_context = context.APIRequestContext(user_id)
-    
+
         inst['mac_address'] = utils.generate_mac()
-        
-        #TODO(dietz) is this necessary? 
+
+        #TODO(dietz) is this necessary?
         inst['launch_index'] = 0
 
         inst['hostname'] = str(ref.internal_id)
@@ -256,21 +254,20 @@
 
         # TODO(vish): This probably should be done in the scheduler
         #             network is setup when host is assigned
-        network_topic = self._get_network_topic(user_id)
+        network_topic = self._get_network_topic(None)
         rpc.call(network_topic,
                  {"method": "setup_fixed_ip",
                   "args": {"context": None,
                            "address": address}})
         return inst
 
-    def _get_network_topic(self, user_id):
+    def _get_network_topic(self, context):
         """Retrieves the network host for a project"""
-        network_ref = self.db_driver.project_get_network(None, 
-            user_id)
+        network_ref = self.network_manager.get_network(context)
         host = network_ref['host']
         if not host:
             host = rpc.call(FLAGS.network_topic,
                                   {"method": "set_network_host",
                                    "args": {"context": None,
-                                            "project_id": user_id}})
+                                            "network_id": network_ref['id']}})
         return self.db_driver.queue_get_for(None, FLAGS.network_topic, host)

=== modified file 'nova/auth/manager.py'
--- nova/auth/manager.py	2010-10-12 20:04:46 +0000
+++ nova/auth/manager.py	2010-10-13 07:50:58 +0000
@@ -484,13 +484,6 @@
                                               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):
@@ -559,14 +552,6 @@
 
     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))
 

=== modified file 'nova/compute/manager.py'
--- nova/compute/manager.py	2010-10-12 20:18:29 +0000
+++ nova/compute/manager.py	2010-10-13 07:50:58 +0000
@@ -76,7 +76,7 @@
             raise exception.Error("Instance has already been created")
         logging.debug("instance %s: starting...", instance_id)
         project_id = instance_ref['project_id']
-        self.network_manager.setup_compute_network(context, project_id)
+        self.network_manager.setup_compute_network(context, instance_id)
         self.db.instance_update(context,
                                 instance_id,
                                 {'host': self.host})

=== modified file 'nova/db/api.py'
--- nova/db/api.py	2010-10-12 20:04:46 +0000
+++ nova/db/api.py	2010-10-13 07:50:58 +0000
@@ -340,6 +340,11 @@
 ####################
 
 
+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)
@@ -360,9 +365,12 @@
     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):
@@ -370,9 +378,14 @@
     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):
@@ -387,10 +400,15 @@
 
 
 def network_get_by_bridge(context, bridge):
-    """Get an network or raise if it does not exist."""
+    """Get a network by bridge or raise if it does not exist."""
     return IMPL.network_get_by_bridge(context, bridge)
 
 
+def network_get_by_instance(context, instance_id):
+    """Get a network by instance id or raise if it does not exist."""
+    return IMPL.network_get_by_instance(context, instance_id)
+
+
 def network_get_index(context, network_id):
     """Get non-conflicting index for network"""
     return IMPL.network_get_index(context, network_id)
@@ -401,19 +419,6 @@
     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_safe(context, values):
-    """Create a network index from the values dict
-
-    The index is not returned. If the create violates the unique
-    constraints because the index already exists, no exception is raised."""
-    return IMPL.network_index_create_safe(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)

=== modified file 'nova/db/sqlalchemy/api.py'
--- nova/db/sqlalchemy/api.py	2010-10-12 05:21:25 +0000
+++ nova/db/sqlalchemy/api.py	2010-10-13 07:50:58 +0000
@@ -805,6 +805,24 @@
 
 
 @require_admin_context
+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
+
+
+@require_admin_context
 def network_count(context):
     session = get_session()
     return session.query(models.Network
@@ -844,31 +862,26 @@
 
 
 @require_admin_context
-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
-
-
-@require_admin_context
-def network_destroy(context, network_id):
+    try:
+        network_ref.save()
+        return network_ref
+    except IntegrityError:
+        return None
+
+
+@require_admin_context
+def network_disassociate(context, network_id):
+    network_update(context, network_id, {'project_id': None})
+
+
+@require_admin_context
+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')
 
 
 @require_context
@@ -918,48 +931,21 @@
 
     if not result:
         raise exception.NotFound('No network for bridge %s' % bridge)
-
     return result
 
 
 @require_admin_context
-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'] = network_get(context,
-                                               network_id,
-                                               session=session)
-        session.add(network_index)
-
-    return network_index['index']
-
-
-@require_admin_context
-def network_index_count(context):
-    session = get_session()
-    return session.query(models.NetworkIndex
-                 ).filter_by(deleted=can_read_deleted(context)
-                 ).count()
-
-
-@require_admin_context
-def network_index_create_safe(context, values):
-    network_index_ref = models.NetworkIndex()
-    for (key, value) in values.iteritems():
-        network_index_ref[key] = value
-    try:
-        network_index_ref.save()
-    except IntegrityError:
-        pass
+def network_get_by_instance(_context, instance_id):
+    session = get_session()
+    rv = session.query(models.Network
+               ).filter_by(deleted=False
+               ).join(models.Network.fixed_ips
+               ).filter_by(instance_id=instance_id
+               ).filter_by(deleted=False
+               ).first()
+    if not rv:
+        raise exception.NotFound('No network for instance %s' % instance_id)
+    return rv
 
 
 @require_admin_context
@@ -999,15 +985,22 @@
 @require_context
 def project_get_network(context, project_id):
     session = get_session()
-    result= session.query(models.Network
+    rv = session.query(models.Network
                ).filter_by(project_id=project_id
                ).filter_by(deleted=False
                ).first()
-
-    if not result:
-        raise exception.NotFound('No network for project: %s' % project_id)
-
-    return result
+    if not rv:
+        try:
+            return 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
 
 
 ###################

=== modified file 'nova/db/sqlalchemy/models.py'
--- nova/db/sqlalchemy/models.py	2010-10-12 22:43:37 +0000
+++ nova/db/sqlalchemy/models.py	2010-10-13 07:50:58 +0000
@@ -25,7 +25,7 @@
 
 # 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.exc import IntegrityError
 from sqlalchemy.ext.declarative import declarative_base
@@ -363,10 +363,13 @@
 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,28 +382,13 @@
     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, unique=True)
-    network_id = Column(Integer, ForeignKey('networks.id'), nullable=True)
-    network = relationship(Network,
-                           backref=backref('network_index', uselist=False),
-                           foreign_keys=network_id,
-                           primaryjoin='and_(NetworkIndex.network_id==Network.id,'
-                                            'NetworkIndex.deleted==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
@@ -413,7 +401,6 @@
     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"""
@@ -517,7 +504,7 @@
     """Register Models and create metadata"""
     from sqlalchemy import create_engine
     models = (Service, Instance, Volume, ExportDevice, FixedIp,
-              FloatingIp, Network, NetworkIndex, SecurityGroup,
+              FloatingIp, Network, SecurityGroup,
               SecurityGroupIngressRule, SecurityGroupInstanceAssociation,
               AuthToken, User, Project) # , Image, Host
     engine = create_engine(FLAGS.sql_connection, echo=False)

=== modified file 'nova/network/linux_net.py'
--- nova/network/linux_net.py	2010-10-02 10:39:47 +0000
+++ nova/network/linux_net.py	2010-10-13 07:50:58 +0000
@@ -65,12 +65,12 @@
     # SNAT rule for outbound traffic.
     _confirm_rule("POSTROUTING", "-t nat -s %s "
              "-j SNAT --to-source %s"
-             % (FLAGS.private_range, FLAGS.routing_source_ip))
+             % (FLAGS.fixed_range, FLAGS.routing_source_ip))
 
     _confirm_rule("POSTROUTING", "-t nat -s %s -j MASQUERADE" %
-                  FLAGS.private_range)
+                  FLAGS.fixed_range)
     _confirm_rule("POSTROUTING", "-t nat -s %(range)s -d %(range)s -j ACCEPT" %
-                  {'range': FLAGS.private_range})
+                  {'range': FLAGS.fixed_range})
 
 def bind_floating_ip(floating_ip):
     """Bind ip to public interface"""

=== modified file 'nova/network/manager.py'
--- nova/network/manager.py	2010-10-04 10:53:55 +0000
+++ nova/network/manager.py	2010-10-13 07:50:58 +0000
@@ -37,17 +37,6 @@
 FLAGS = flags.FLAGS
 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'],
-                  'Available ips for simple network')
-flags.DEFINE_string('flat_network_network', '192.168.0.0',
-                    'Network for simple network')
-flags.DEFINE_string('flat_network_netmask', '255.255.255.0',
-                    'Netmask for simple network')
-flags.DEFINE_string('flat_network_gateway', '192.168.0.1',
-                    'Broadcast for simple network')
-flags.DEFINE_string('flat_network_broadcast', '192.168.0.255',
-                    'Broadcast for simple network')
 flags.DEFINE_string('flat_network_dns', '8.8.4.4',
                     'Dns for simple network')
 flags.DEFINE_integer('vlan_start', 100, 'First VLAN for private networks')
@@ -57,8 +46,8 @@
 flags.DEFINE_integer('vpn_start', 1000, 'First Vpn port for private networks')
 flags.DEFINE_integer('network_size', 256,
                         'Number of addresses in each private subnet')
-flags.DEFINE_string('public_range', '4.4.4.0/24', 'Public IP address block')
-flags.DEFINE_string('private_range', '10.0.0.0/8', 'Private IP address block')
+flags.DEFINE_string('floating_range', '4.4.4.0/24', 'Floating IP address block')
+flags.DEFINE_string('fixed_range', '10.0.0.0/8', 'Fixed IP address block')
 flags.DEFINE_integer('cnt_vpn_clients', 5,
                      'Number of addresses reserved for vpn clients')
 flags.DEFINE_string('network_driver', 'nova.network.linux_net',
@@ -91,13 +80,9 @@
         for network in self.db.host_get_networks(None, self.host):
             self._on_set_network_host(None, network['id'])
 
-    def set_network_host(self, context, project_id):
-        """Safely sets the host of the projects network"""
+    def set_network_host(self, context, network_id):
+        """Safely sets the host of the network"""
         logging.debug("setting network host")
-        network_ref = self.db.project_get_network(context, project_id)
-        # TODO(vish): can we minimize db access by just getting the
-        #             id here instead of the ref?
-        network_id = network_ref['id']
         host = self.db.network_set_host(None,
                                         network_id,
                                         self.host)
@@ -117,10 +102,10 @@
         raise NotImplementedError()
 
     def _on_set_network_host(self, context, network_id):
-        """Called when this host becomes the host for a project"""
+        """Called when this host becomes the host for a network"""
         raise NotImplementedError()
 
-    def setup_compute_network(self, context, project_id):
+    def setup_compute_network(self, context, instance_id):
         """Sets up matching network for compute hosts"""
         raise NotImplementedError()
 
@@ -150,6 +135,16 @@
         """Returns an floating ip to the pool"""
         self.db.floating_ip_deallocate(context, floating_address)
 
+    def get_network(self, context):
+        """Get the network for the current context"""
+        raise NotImplementedError()
+
+    def create_networks(self, context, num_networks, network_size,
+                        *args, **kwargs):
+        """Create networks based on parameters"""
+        raise NotImplementedError()
+
+
     @property
     def _bottom_reserved_ips(self):  # pylint: disable-msg=R0201
         """Number of reserved ips at the bottom of the range"""
@@ -163,7 +158,7 @@
     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
@@ -185,8 +180,13 @@
 
     def allocate_fixed_ip(self, context, instance_id, *args, **kwargs):
         """Gets a fixed ip from the pool"""
-        network_ref = self.db.project_get_network(context, context.project.id)
-        address = self.db.fixed_ip_associate_pool(context,
+        # TODO(vish): when this is called by compute, we can associate compute
+        #             with a network, or a cluster of computes with a network
+        #             and use that network here with a method like
+        #             network_get_by_compute_host
+        network_ref = self.db.network_get_by_bridge(None,
+                                                    FLAGS.flat_network_bridge)
+        address = self.db.fixed_ip_associate_pool(None,
                                                   network_ref['id'],
                                                   instance_id)
         self.db.fixed_ip_update(context, address, {'allocated': True})
@@ -195,9 +195,9 @@
     def deallocate_fixed_ip(self, context, address, *args, **kwargs):
         """Returns a fixed ip to the pool"""
         self.db.fixed_ip_update(context, address, {'allocated': False})
-        self.db.fixed_ip_disassociate(context, address)
+        self.db.fixed_ip_disassociate(None, address)
 
-    def setup_compute_network(self, context, project_id):
+    def setup_compute_network(self, context, instance_id):
         """Network is created manually"""
         pass
 
@@ -205,24 +205,42 @@
         """Currently no setup"""
         pass
 
+    def create_networks(self, context, cidr, num_networks, network_size,
+                        *args, **kwargs):
+        """Create networks based on parameters"""
+        fixed_net = IPy.IP(cidr)
+        for index in range(num_networks):
+            start = index * network_size
+            significant_bits = 32 - int(math.log(network_size, 2))
+            cidr = "%s/%s" % (fixed_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['dhcp_start'] = str(project_net[2])
+            network_ref = self.db.network_create_safe(context, net)
+            if network_ref:
+                self._create_fixed_ips(context, network_ref['id'])
+
+    def get_network(self, context):
+        """Get the network for the current context"""
+        # NOTE(vish): To support mutilple network hosts, This could randomly
+        #             select from multiple networks instead of just
+        #             returning the one. It could also potentially be done
+        #             in the scheduler.
+        return self.db.network_get_by_bridge(context,
+                                             FLAGS.flat_network_bridge)
+
     def _on_set_network_host(self, context, network_id):
-        """Called when this host becomes the host for a project"""
-        # NOTE(vish): should there be two types of network objects
-        #             in the datastore?
+        """Called when this host becomes the host for a network"""
         net = {}
         net['injected'] = True
-        net['netmask'] = FLAGS.flat_network_netmask
         net['bridge'] = FLAGS.flat_network_bridge
-        net['gateway'] = FLAGS.flat_network_gateway
-        net['broadcast'] = FLAGS.flat_network_broadcast
         net['dns'] = FLAGS.flat_network_dns
         self.db.network_update(context, network_id, net)
-        # NOTE(vish): Rignt now we are putting  all of the fixed ips in
-        #             one large pool, but ultimately it may be better to
-        #             have each network manager have its own network that
-        #             it is responsible for and its own pool of ips.
-        for address in FLAGS.flat_network_ips:
-            self.db.fixed_ip_create(context, {'address': address})
+
 
 
 class VlanManager(NetworkManager):
@@ -250,10 +268,13 @@
 
     def allocate_fixed_ip(self, context, instance_id, *args, **kwargs):
         """Gets a fixed ip from the pool"""
+        # TODO(vish): This should probably be getting project_id from
+        #             the instance, but it is another trip to the db.
+        #             Perhaps this method should take an instance_ref.
         network_ref = self.db.project_get_network(context, context.project.id)
         if kwargs.get('vpn', None):
             address = network_ref['vpn_private_address']
-            self.db.fixed_ip_associate(context, address, instance_id)
+            self.db.fixed_ip_associate(None, address, instance_id)
         else:
             address = self.db.fixed_ip_associate_pool(None,
                                                       network_ref['id'],
@@ -319,38 +340,9 @@
                 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)
-        return network_id
-
-    def setup_compute_network(self, context, project_id):
+    def setup_compute_network(self, context, instance_id):
         """Sets up matching network for compute hosts"""
-        network_ref = self.db.project_get_network(context, project_id)
+        network_ref = db.network_get_by_instance(context, instance_id)
         self.driver.ensure_vlan_bridge(network_ref['vlan'],
                                        network_ref['bridge'])
 
@@ -359,17 +351,42 @@
         # TODO(vish): Implement this
         pass
 
-    def _ensure_indexes(self, context):
-        """Ensure the indexes for the network exist
+    def create_networks(self, context, cidr, num_networks, network_size,
+                        vlan_start, vpn_start):
+        """Create networks based on parameters"""
+        fixed_net = IPy.IP(cidr)
+        for index in range(num_networks):
+            vlan = vlan_start + index
+            start = index * network_size
+            significant_bits = 32 - int(math.log(network_size, 2))
+            cidr = "%s/%s" % (fixed_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'])
 
-        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_safe(context, {'index': index})
+    def get_network(self, context):
+        """Get the network for the current context"""
+        return self.db.project_get_network(None, context.project.id)
 
     def _on_set_network_host(self, context, network_id):
-        """Called when this host becomes the host for a project"""
+        """Called when this host becomes the host for a network"""
         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)

=== modified file 'nova/test.py'
--- nova/test.py	2010-09-29 11:39:09 +0000
+++ nova/test.py	2010-10-13 07:50:58 +0000
@@ -24,6 +24,7 @@
 
 import sys
 import time
+import datetime
 
 import mox
 import stubout
@@ -35,6 +36,7 @@
 from nova import fakerabbit
 from nova import flags
 from nova import rpc
+from nova.network import manager as network_manager
 
 
 FLAGS = flags.FLAGS
@@ -58,6 +60,16 @@
     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.
+        self.start = datetime.datetime.utcnow()
+        if db.network_count(None) != 5:
+            network_manager.VlanManager().create_networks(None,
+                                                          FLAGS.fixed_range,
+                                                          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
@@ -74,7 +86,9 @@
         self.stubs.UnsetAll()
         self.stubs.SmartUnsetAll()
         self.mox.VerifyAll()
-        
+        # NOTE(vish): Clean up any ips associated during the test.
+        db.fixed_ip_disassociate_all_by_timeout(None, FLAGS.host, self.start)
+        db.network_disassociate_all(None)
         rpc.Consumer.attach_to_twisted = self.originalAttach
         for x in self.injected:
             try:
@@ -140,7 +154,7 @@
 class BaseTestCase(TrialTestCase):
     # TODO(jaypipes): Can this be moved into the TrialTestCase class?
     """Base test case class for all unit tests.
-    
+
     DEPRECATED: This is being removed once Tornado is gone, use TrialTestCase.
     """
     def setUp(self): # pylint: disable-msg=C0103

=== modified file 'nova/tests/cloud_unittest.py'
--- nova/tests/cloud_unittest.py	2010-10-11 11:39:33 +0000
+++ nova/tests/cloud_unittest.py	2010-10-13 07:50:58 +0000
@@ -64,12 +64,12 @@
         self.cloud = cloud.CloudController()
 
         # set up a service
-        self.compute = utils.import_class(FLAGS.compute_manager)()
+        self.compute = utils.import_object(FLAGS.compute_manager)
         self.compute_consumer = rpc.AdapterConsumer(connection=self.conn,
                                                     topic=FLAGS.compute_topic,
                                                     proxy=self.compute)
         self.compute_consumer.attach_to_eventlet()
-        self.network = utils.import_class(FLAGS.network_manager)()
+        self.network = utils.import_object(FLAGS.network_manager)
         self.network_consumer = rpc.AdapterConsumer(connection=self.conn,
                                                     topic=FLAGS.network_topic,
                                                     proxy=self.network)

=== modified file 'nova/tests/compute_unittest.py'
--- nova/tests/compute_unittest.py	2010-09-30 09:47:05 +0000
+++ nova/tests/compute_unittest.py	2010-10-13 07:50:58 +0000
@@ -40,7 +40,8 @@
     def setUp(self):  # pylint: disable-msg=C0103
         logging.getLogger().setLevel(logging.DEBUG)
         super(ComputeTestCase, self).setUp()
-        self.flags(connection_type='fake')
+        self.flags(connection_type='fake',
+                   network_manager='nova.network.manager.FlatManager')
         self.compute = utils.import_object(FLAGS.compute_manager)
         self.manager = manager.AuthManager()
         self.user = self.manager.create_user('fake', 'fake', 'fake')

=== modified file 'nova/tests/network_unittest.py'
--- nova/tests/network_unittest.py	2010-10-03 18:22:35 +0000
+++ nova/tests/network_unittest.py	2010-10-13 07:50:58 +0000
@@ -52,13 +52,14 @@
         self.context = context.APIRequestContext(project=None, user=self.user)
         for i in range(5):
             name = 'project%s' % i
-            self.projects.append(self.manager.create_project(name,
-                                                             'netuser',
-                                                             name))
+            project = self.manager.create_project(name, 'netuser', name)
+            self.projects.append(project)
             # create the necessary network data for the project
-            user_context = context.get_admin_context(user=self.user)
-
-            self.network.set_network_host(user_context, self.projects[i].id)
+            user_context = context.APIRequestContext(project=self.projects[i],
+                                                     user=self.user)
+            network_ref = self.network.get_network(user_context)
+            self.network.set_network_host(context.get_admin_context(),
+                                          network_ref['id'])
         instance_ref = self._create_instance(0)
         self.instance_id = instance_ref['id']
         instance_ref = self._create_instance(1)
@@ -99,7 +100,7 @@
         """Makes sure that we can allocaate a public ip"""
         # TODO(vish): better way of adding floating ips
         self.context.project = self.projects[0]
-        pubnet = IPy.IP(flags.FLAGS.public_range)
+        pubnet = IPy.IP(flags.FLAGS.floating_range)
         address = str(pubnet[0])
         try:
             db.floating_ip_get_by_address(None, address)
@@ -109,6 +110,7 @@
         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)
@@ -118,6 +120,7 @@
         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"""
@@ -190,8 +193,10 @@
             release_ip(address3)
         for instance_id in instance_ids:
             db.instance_destroy(None, instance_id)
+        self.context.project = self.projects[0]
+        self.network.deallocate_fixed_ip(self.context, first)
+        self._deallocate_address(0, first)
         release_ip(first)
-        self._deallocate_address(0, first)
 
     def test_vpn_ip_and_port_looks_valid(self):
         """Ensure the vpn ip and port are reasonable"""
@@ -207,10 +212,13 @@
         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)
 
@@ -223,7 +231,9 @@
 
         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

=== modified file 'nova/tests/scheduler_unittest.py'
--- nova/tests/scheduler_unittest.py	2010-10-04 09:53:27 +0000
+++ nova/tests/scheduler_unittest.py	2010-10-13 07:50:58 +0000
@@ -75,6 +75,7 @@
         self.flags(connection_type='fake',
                    max_cores=4,
                    max_gigabytes=4,
+                   network_manager='nova.network.manager.FlatManager',
                    volume_driver='nova.volume.driver.FakeAOEDriver',
                    scheduler_driver='nova.scheduler.simple.SimpleScheduler')
         self.scheduler = manager.SchedulerManager()

=== modified file 'nova/tests/virt_unittest.py'
--- nova/tests/virt_unittest.py	2010-10-12 20:18:29 +0000
+++ nova/tests/virt_unittest.py	2010-10-13 07:50:58 +0000
@@ -20,21 +20,22 @@
 from nova import db
 from nova import flags
 from nova import test
+from nova import utils
 from nova.api import context
 from nova.api.ec2 import cloud
 from nova.auth import manager
-
-# Needed to get FLAGS.instances_path defined:
-from nova.compute import manager as compute_manager
 from nova.virt import libvirt_conn
 
 FLAGS = flags.FLAGS
+flags.DECLARE('instances_path', 'nova.compute.manager')
 
 class LibvirtConnTestCase(test.TrialTestCase):
     def setUp(self):
+        super(LibvirtConnTestCase, self).setUp()
         self.manager = manager.AuthManager()
         self.user = self.manager.create_user('fake', 'fake', 'fake', admin=True)
         self.project = self.manager.create_project('fake', 'fake', 'fake')
+        self.network = utils.import_object(FLAGS.network_manager)
         FLAGS.instances_path = ''
 
     def test_get_uri_and_template(self):
@@ -51,11 +52,15 @@
                      'instance_type'  : 'm1.small'}
 
         instance_ref = db.instance_create(None, instance)
-        network_ref = db.project_get_network(None, self.project.id)
+        user_context = context.APIRequestContext(project=self.project,
+                                                 user=self.user)
+        network_ref = self.network.get_network(user_context)
+        self.network.set_network_host(context.get_admin_context(),
+                                      network_ref['id'])
 
         fixed_ip = { 'address'    : ip,
                      'network_id' : network_ref['id'] }
-                     
+
         fixed_ip_ref = db.fixed_ip_create(None, fixed_ip)
         db.fixed_ip_update(None, ip, { 'allocated'   : True,
                                           'instance_id' : instance_ref['id'] })
@@ -113,6 +118,7 @@
 
 
     def tearDown(self):
+        super(LibvirtConnTestCase, self).tearDown()
         self.manager.delete_project(self.project)
         self.manager.delete_user(self.user)
 

=== modified file 'nova/virt/libvirt_conn.py'
--- nova/virt/libvirt_conn.py	2010-10-04 19:58:22 +0000
+++ nova/virt/libvirt_conn.py	2010-10-13 07:50:58 +0000
@@ -287,7 +287,7 @@
 
         key = str(inst['key_data'])
         net = None
-        network_ref = db.project_get_network(None, project.id)
+        network_ref = db.network_get_by_instance(None, inst['id'])
         if network_ref['injected']:
             address = db.instance_get_fixed_address(None, inst['id'])
             with open(FLAGS.injected_network_template) as f:
@@ -320,7 +320,8 @@
     def to_xml(self, instance):
         # TODO(termie): cache?
         logging.debug('instance %s: starting toXML method', instance['name'])
-        network = db.project_get_network(None, instance['project_id'])
+        network = db.project_get_network(None,
+                                         instance['project_id'])
         # FIXME(vish): stick this in db
         instance_type = instance_types.INSTANCE_TYPES[instance['instance_type']]
         ip_address = db.instance_get_fixed_address({}, instance['id'])