fixed tests, moved compute network config call, added notes, made inject option into a boolean

5 files changed, 81 insertions, 40 deletions

diff --git a/bin/nova-network b/bin/nova-network
index 1d620b525..ba9063f56 100755
--- a/bin/nova-network
+++ b/bin/nova-network
@@ -23,7 +23,6 @@
 
 from nova import flags
 from nova import twistd
-from nova import utils
 
 from nova.network import service
 
@@ -34,8 +33,4 @@ if __name__ == '__main__':
     twistd.serve(__file__)
 
 if __name__ == '__builtin__':
-    t = FLAGS.network_type
-    if t == 'flat':
-        application = service.FlatNetworkService.create()
-    elif t == 'vlan':
-        application = service.VlanNetworkService.create()
+    application = service.type_to_class(FLAGS.network_type).create()
diff --git a/nova/compute/service.py b/nova/compute/service.py
index a22240b05..820116453 100644
--- a/nova/compute/service.py
+++ b/nova/compute/service.py
@@ -41,7 +41,7 @@ from nova.compute import disk
 from nova.compute import model
 from nova.compute import power_state
 from nova.compute.instance_types import INSTANCE_TYPES
-from nova.network import model as net_model
+from nova.network import service as network_service
 from nova.objectstore import image # for image_path flag
 from nova.virt import connection as virt_connection
 from nova.volume import service as volume_service
@@ -117,12 +117,17 @@ class ComputeService(service.Service):
         """ launch a new instance with specified options """
         logging.debug("Starting instance %s..." % (instance_id))
         inst = self.instdir.get(instance_id)
-        if inst.get('network_type', 'dhcp') == 'dhcp':
-            # TODO: Get the real security group of launch in here
-            security_group = "default"
-            net = net_model.BridgedNetwork.get_network_for_project(inst['user_id'],
-                                                             inst['project_id'],
-                                            security_group).express()
+        # TODO: Get the real security group of launch in here
+        security_group = "default"
+        # NOTE(vish): passing network type allows us to express the
+        #             network without making a call to network to find
+        #             out which type of network to setup
+        network_service.setup_compute_network(
+                                           inst.get('network_type', 'vlan'),
+                                           inst['user_id'],
+                                           inst['project_id'],
+                                           security_group)
+
         inst['node_name'] = FLAGS.node_name
         inst.save()
         # TODO(vish) check to make sure the availability zone matches
diff --git a/nova/network/service.py b/nova/network/service.py
index bf4aa0073..57b8bbb78 100644
--- a/nova/network/service.py
+++ b/nova/network/service.py
@@ -20,8 +20,6 @@
 Network Nodes are responsible for allocating ips and setting up network
 """
 
-import logging
-
 from twisted.internet import defer
 
 from nova import datastore
@@ -29,6 +27,7 @@ from nova import flags
 from nova import service
 from nova import utils
 from nova.auth import manager
+from nova.exception import NotFound
 from nova.network import exception
 from nova.network import model
 
@@ -53,10 +52,24 @@ flags.DEFINE_string('flat_network_broadcast', '192.168.0.255',
 flags.DEFINE_string('flat_network_dns', '8.8.4.4',
                        'Dns for simple network')
 
+def type_to_class(network_type):
+    if network_type == 'flat':
+        return FlatNetworkService
+    elif network_type  == 'vlan':
+        return VlanNetworkService
+    raise NotFound("Couldn't find %s network type" % network_type)
+
+
+def setup_compute_network(network_type, user_id, project_id, security_group):
+    srv = type_to_class(network_type)
+    srv.setup_compute_network(network_type, user_id, project_id, security_group)
+
+
 def get_host_for_project(project_id):
     redis = datastore.Redis.instance()
     return redis.get(_host_key(project_id))
 
+
 def _host_key(project_id):
     return "network_host:%s" % project_id
 
@@ -88,6 +101,12 @@ class BaseNetworkService(service.Service):
         """Subclass implements return of ip to the pool"""
         raise NotImplementedError()
 
+    @classmethod
+    def setup_compute_network(self, user_id, project_id, security_group,
+                              *args, **kwargs):
+        """Sets up matching network for compute hosts"""
+        raise NotImplementedError()
+
     def allocate_elastic_ip(self, user_id, project_id):
         """Gets a elastic ip from the pool"""
         # NOTE(vish): Replicating earlier decision to use 'public' as
@@ -111,6 +130,11 @@ class BaseNetworkService(service.Service):
 class FlatNetworkService(BaseNetworkService):
     """Basic network where no vlans are used"""
 
+    @classmethod
+    def setup_compute_network(self, fixed_ip, *args, **kwargs):
+        """Network is created manually"""
+        pass
+
     def allocate_fixed_ip(self, user_id, project_id,
                           security_group='default',
                           *args, **kwargs):
@@ -118,6 +142,9 @@ class FlatNetworkService(BaseNetworkService):
 
         Flat network just grabs the next available ip from the pool
         """
+        # 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
         redis = datastore.Redis.instance()
         if not redis.exists('ips') and not len(redis.keys('instances:*')):
             for fixed_ip in FLAGS.flat_network_ips:
@@ -125,15 +152,16 @@ class FlatNetworkService(BaseNetworkService):
         fixed_ip = redis.spop('ips')
         if not fixed_ip:
             raise exception.NoMoreAddresses()
-        return defer.succeed({'network_type': 'injected',
-                'mac_address': utils.generate_mac(),
-                'private_dns_name': str(fixed_ip),
-                'bridge_name': FLAGS.flat_network_bridge,
-                'network_network': FLAGS.flat_network_network,
-                'network_netmask': FLAGS.flat_network_netmask,
-                'network_gateway': FLAGS.flat_network_gateway,
-                'network_broadcast': FLAGS.flat_network_broadcast,
-                'network_dns': FLAGS.flat_network_dns})
+        return defer.succeed({'inject_network': True,
+                            'network_type': FLAGS.network_type,
+                            'mac_address': utils.generate_mac(),
+                            'private_dns_name': str(fixed_ip),
+                            'bridge_name': FLAGS.flat_network_bridge,
+                            'network_network': FLAGS.flat_network_network,
+                            'network_netmask': FLAGS.flat_network_netmask,
+                            'network_gateway': FLAGS.flat_network_gateway,
+                            'network_broadcast': FLAGS.flat_network_broadcast,
+                            'network_dns': FLAGS.flat_network_dns})
 
     def deallocate_fixed_ip(self, fixed_ip, *args, **kwargs):
         """Returns an ip to the pool"""
@@ -141,7 +169,10 @@ class FlatNetworkService(BaseNetworkService):
 
 class VlanNetworkService(BaseNetworkService):
     """Vlan network with dhcp"""
-
+    # NOTE(vish): A lot of the interactions with network/model.py can be
+    #             simplified and improved.  Also there it may be useful
+    #             to support vlans separately from dhcp, instead of having
+    #             both of them together in this class.
     def allocate_fixed_ip(self, user_id, project_id,
                           security_group='default',
                           vpn=False, *args, **kwargs):
@@ -152,10 +183,10 @@ class VlanNetworkService(BaseNetworkService):
             fixed_ip = net.allocate_vpn_ip(user_id, project_id, mac)
         else:
             fixed_ip = net.allocate_ip(user_id, project_id, mac)
-        return defer.succeed({'network_type': 'dhcp',
-                'bridge_name': net['bridge_name'],
-                'mac_address': mac,
-                'private_dns_name' : fixed_ip})
+        return defer.succeed({'network_type': FLAGS.network_type,
+                              'bridge_name': net['bridge_name'],
+                              'mac_address': mac,
+                              'private_dns_name' : fixed_ip})
 
     def deallocate_fixed_ip(self, fixed_ip,
                             *args, **kwargs):
@@ -173,4 +204,14 @@ class VlanNetworkService(BaseNetworkService):
         for project in manager.AuthManager().get_projects():
             model.get_project_network(project.id).express()
 
-
+    @classmethod
+    def setup_compute_network(self, user_id, project_id, security_group,
+                              *args, **kwargs):
+        """Sets up matching network for compute hosts"""
+        # NOTE(vish): Use BridgedNetwork instead of DHCPNetwork because
+        #             we don't want to run dnsmasq on the client machines
+        net = model.BridgedNetwork.get_network_for_project(
+                                            user_id,
+                                            project_id,
+                                            security_group)
+        net.express()
diff --git a/nova/tests/network_unittest.py b/nova/tests/network_unittest.py
index a9695f818..42cae327f 100644
--- a/nova/tests/network_unittest.py
+++ b/nova/tests/network_unittest.py
@@ -69,7 +69,7 @@ class NetworkTestCase(test.TrialTestCase):
         self.assertTrue(IPy.IP(address) in self.network.network)
 
     def test_allocate_deallocate_fixed_ip(self):
-        result  = self.service.allocate_fixed_ip(
+        result  = yield self.service.allocate_fixed_ip(
                 self.user.id, self.projects[0].id)
         address = result['private_dns_name']
         mac = result['mac_address']
@@ -88,11 +88,11 @@ class NetworkTestCase(test.TrialTestCase):
 
     def test_range_allocation(self):
         hostname = "test-host"
-        result = self.service.allocate_fixed_ip(
+        result = yield self.service.allocate_fixed_ip(
                     self.user.id, self.projects[0].id)
         mac = result['mac_address']
         address = result['private_dns_name']
-        result = self.service.allocate_fixed_ip(
+        result = yield self.service.allocate_fixed_ip(
                 self.user, self.projects[1].id)
         secondmac = result['mac_address']
         secondaddress = result['private_dns_name']
@@ -122,21 +122,21 @@ class NetworkTestCase(test.TrialTestCase):
         self.assertEqual(False, is_in_project(secondaddress, self.projects[1].id))
 
     def test_subnet_edge(self):
-        result = self.service.allocate_fixed_ip(self.user.id,
+        result = yield self.service.allocate_fixed_ip(self.user.id,
                                                        self.projects[0].id)
         firstaddress = result['private_dns_name']
         hostname = "toomany-hosts"
         for i in range(1,5):
             project_id = self.projects[i].id
-            result = self.service.allocate_fixed_ip(
+            result = yield self.service.allocate_fixed_ip(
                     self.user, project_id)
             mac = result['mac_address']
             address = result['private_dns_name']
-            result = self.service.allocate_fixed_ip(
+            result = yield self.service.allocate_fixed_ip(
                     self.user, project_id)
             mac2 = result['mac_address']
             address2 = result['private_dns_name']
-            result = self.service.allocate_fixed_ip(
+            result = yield self.service.allocate_fixed_ip(
                    self.user, project_id)
             mac3 = result['mac_address']
             address3 = result['private_dns_name']
@@ -193,12 +193,12 @@ class NetworkTestCase(test.TrialTestCase):
         macs = {}
         addresses = {}
         for i in range(0, (num_available_ips - 1)):
-            result = self.service.allocate_fixed_ip(self.user.id, self.projects[0].id)
+            result = yield self.service.allocate_fixed_ip(self.user.id, self.projects[0].id)
             macs[i] = result['mac_address']
             addresses[i] = result['private_dns_name']
             self.dnsmasq.issue_ip(macs[i], addresses[i], hostname, net.bridge_name)
 
-        self.assertRaises(NoMoreAddresses, self.service.allocate_fixed_ip, self.user.id, self.projects[0].id)
+        self.assertFailure(self.service.allocate_fixed_ip(self.user.id, self.projects[0].id), NoMoreAddresses)
 
         for i in range(0, (num_available_ips - 1)):
             rv = self.service.deallocate_fixed_ip(addresses[i])
diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py
index 8133daf0b..7c3f7a6e1 100644
--- a/nova/virt/libvirt_conn.py
+++ b/nova/virt/libvirt_conn.py
@@ -204,7 +204,7 @@ class LibvirtConnection(object):
 
         key = data['key_data']
         net = None
-        if data.get('network_type', None) == 'injected':
+        if data.get('inject_network', False):
             with open(FLAGS.injected_network_template) as f:
                 net = f.read() % {'address': data['private_dns_name'],
                                   'network': data['network_network'],