Merge trunk and resolve conflicts

bin/nova-api

@@ -26,11 +26,8 @@ from tornado import httpserver
 from tornado import ioloop
 
 from nova import flags
-from nova import rpc
 from nova import server
 from nova import utils
-from nova.auth import manager
-from nova.compute import model
 from nova.endpoint import admin
 from nova.endpoint import api
 from nova.endpoint import cloud
@@ -39,20 +36,13 @@ FLAGS = flags.FLAGS
 
 
 def main(_argv):
+    """Load the controllers and start the tornado I/O loop."""
     controllers = {
         'Cloud': cloud.CloudController(),
-        'Admin': admin.AdminController()
+        'Admin': admin.AdminController()}
-    }
     _app = api.APIServerApplication(controllers)
 
-    conn = rpc.Connection.instance()
-    consumer = rpc.AdapterConsumer(connection=conn,
-                                       topic=FLAGS.cloud_topic,
-                                       proxy=controllers['Cloud'])
-
     io_inst = ioloop.IOLoop.instance()
-    _injected = consumer.attach_to_tornado(io_inst)
-
     http_server = httpserver.HTTPServer(_app)
     http_server.listen(FLAGS.cc_port)
     logging.debug('Started HTTP server on %s', FLAGS.cc_port)

bin/nova-api-new

@@ -0,0 +1,34 @@
+#!/usr/bin/env python
+# pylint: disable-msg=C0103
+# 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.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License");
+#    you may not use this file except in compliance with the License.
+#    You may obtain a copy of the License at
+#
+#        http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS,
+#    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#    See the License for the specific language governing permissions and
+#    limitations under the License.
+"""
+Nova API daemon.
+"""
+
+from nova import api
+from nova import flags
+from nova import utils
+from nova import wsgi
+
+FLAGS = flags.FLAGS
+flags.DEFINE_integer('api_port', 8773, 'API port')
+
+if __name__ == '__main__':
+    utils.default_flagfile()
+    wsgi.run_server(api.API(), FLAGS.api_port)

bin/nova-compute

@@ -29,4 +29,4 @@ if __name__ == '__main__':
     twistd.serve(__file__)
 
 if __name__ == '__builtin__':
-    application = service.ComputeService.create()
+    application = service.ComputeService.create() # pylint: disable-msg=C0103

bin/nova-dhcpbridge

@@ -18,8 +18,6 @@
 #    under the License.
 
 """
-nova-dhcpbridge
-
 Handle lease database updates from DHCP servers.
 """
 
@@ -42,34 +40,42 @@ from nova.network import service
 FLAGS = flags.FLAGS
 
 
-def add_lease(mac, ip, hostname, interface):
+def add_lease(_mac, ip_address, _hostname, _interface):
+    """Set the IP that was assigned by the DHCP server."""
     if FLAGS.fake_rabbit:
-        service.VlanNetworkService().lease_ip(ip)
+        service.VlanNetworkService().lease_ip(ip_address)
     else:
-        rpc.cast("%s.%s" (FLAGS.network_topic, FLAGS.node_name),
+        rpc.cast("%s.%s" % (FLAGS.network_topic, FLAGS.node_name),
                  {"method": "lease_ip",
-                  "args" : {"fixed_ip": ip}})
+                  "args": {"fixed_ip": ip_address}})
 
-def old_lease(mac, ip, hostname, interface):
+
+def old_lease(_mac, _ip_address, _hostname, _interface):
+    """Do nothing, just an old lease update."""
     logging.debug("Adopted old lease or got a change of mac/hostname")
 
-def del_lease(mac, ip, hostname, interface):
+
+def del_lease(_mac, ip_address, _hostname, _interface):
+    """Called when a lease expires."""
     if FLAGS.fake_rabbit:
-        service.VlanNetworkService().release_ip(ip)
+        service.VlanNetworkService().release_ip(ip_address)
     else:
-        rpc.cast("%s.%s" (FLAGS.network_topic, FLAGS.node_name),
+        rpc.cast("%s.%s" % (FLAGS.network_topic, FLAGS.node_name),
                  {"method": "release_ip",
-                  "args" : {"fixed_ip": ip}})
+                  "args": {"fixed_ip": ip_address}})
+
 
 def init_leases(interface):
+    """Get the list of hosts for an interface."""
     net = model.get_network_by_interface(interface)
     res = ""
-    for host_name in net.hosts:
+    for address in net.assigned_objs:
-        res += "%s\n" % linux_net.hostDHCP(net, host_name, net.hosts[host_name])
+        res += "%s\n" % linux_net.host_dhcp(address)
     return res
 
 
 def main():
+    """Parse environment and arguments and call the approproate action."""
     flagfile = os.environ.get('FLAGFILE', FLAGS.dhcpbridge_flagfile)
     utils.default_flagfile(flagfile)
     argv = FLAGS(sys.argv)
@@ -79,18 +85,19 @@ def main():
         FLAGS.redis_db = 8
         FLAGS.network_size = 32
         FLAGS.connection_type = 'fake'
-        FLAGS.fake_network=True
+        FLAGS.fake_network = True
-        FLAGS.auth_driver='nova.auth.ldapdriver.FakeLdapDriver'
+        FLAGS.auth_driver = 'nova.auth.ldapdriver.FakeLdapDriver'
     action = argv[1]
-    if action in ['add','del','old']:
+    if action in ['add', 'del', 'old']:
         mac = argv[2]
         ip = argv[3]
         hostname = argv[4]
-        logging.debug("Called %s for mac %s with ip %s and hostname %s on interface %s" % (action, mac, ip, hostname, interface))
+        logging.debug("Called %s for mac %s with ip %s and "
-        globals()[action+'_lease'](mac, ip, hostname, interface)
+                      "hostname %s on interface %s",
+                      action, mac, ip, hostname, interface)
+        globals()[action + '_lease'](mac, ip, hostname, interface)
     else:
         print init_leases(interface)
-    exit(0)
 
 if __name__ == "__main__":
-    sys.exit(main())
+    main()

bin/nova-import-canonical-imagestore

@@ -35,22 +35,19 @@ from nova.objectstore import image
 
 FLAGS = flags.FLAGS
 
-api_url = 'https://imagestore.canonical.com/api/dashboard'
+API_URL = 'https://imagestore.canonical.com/api/dashboard'
+
+
+def get_images():
+    """Get a list of the images from the imagestore URL."""
+    images = json.load(urllib2.urlopen(API_URL))['images']
+    images = [img for img in images if img['title'].find('amd64') > -1]
+    return images
 
-image_cache = None
-def images():
-    global image_cache
-    if not image_cache:
-        try:
-            images = json.load(urllib2.urlopen(api_url))['images']
-            image_cache = [i for i in images if i['title'].find('amd64') > -1]
-        except Exception:
-            print 'unable to download canonical image list'
-            sys.exit(1)
-    return image_cache
 
-# FIXME(ja): add checksum/signature checks
 def download(img):
+    """Download an image to the local filesystem."""
+    # FIXME(ja): add checksum/signature checks
     tempdir = tempfile.mkdtemp(prefix='cis-')
 
     kernel_id = None
@@ -59,40 +56,42 @@ def download(img):
     for f in img['files']:
         if f['kind'] == 'kernel':
             dest = os.path.join(tempdir, 'kernel')
-            subprocess.call(['curl', f['url'], '-o', dest])
+            subprocess.call(['curl', '--fail', f['url'], '-o', dest])
             kernel_id = image.Image.add(dest,
                 description='kernel/' + img['title'], kernel=True)
 
     for f in img['files']:
         if f['kind'] == 'ramdisk':
             dest = os.path.join(tempdir, 'ramdisk')
-            subprocess.call(['curl', f['url'], '-o', dest])
+            subprocess.call(['curl', '--fail', f['url'], '-o', dest])
             ramdisk_id = image.Image.add(dest,
                 description='ramdisk/' + img['title'], ramdisk=True)
 
     for f in img['files']:
         if f['kind'] == 'image':
             dest = os.path.join(tempdir, 'image')
-            subprocess.call(['curl', f['url'], '-o', dest])
+            subprocess.call(['curl', '--fail', f['url'], '-o', dest])
             ramdisk_id = image.Image.add(dest,
                 description=img['title'], kernel=kernel_id, ramdisk=ramdisk_id)
 
     shutil.rmtree(tempdir)
 
+
 def main():
+    """Main entry point."""
     utils.default_flagfile()
     argv = FLAGS(sys.argv)
+    images = get_images()
 
     if len(argv) == 2:
-        for img in images():
+        for img in images:
             if argv[1] == 'all' or argv[1] == img['title']:
                 download(img)
     else:
         print 'usage: %s (title|all)'
         print 'available images:'
-        for image in images():
+        for img in images:
-            print image['title']
+            print img['title']
 
 if __name__ == '__main__':
     main()
-

bin/nova-instancemonitor

@@ -22,7 +22,6 @@
 """
 
 import logging
-from twisted.internet import task
 from twisted.application import service
 
 from nova import twistd
@@ -30,22 +29,16 @@ from nova.compute import monitor
 
 logging.getLogger('boto').setLevel(logging.WARN)
 
-def main():
-    logging.warn('Starting instance monitor')
-    m = monitor.InstanceMonitor()
-
-    # This is the parent service that twistd will be looking for when it
-    # parses this file, return it so that we can get it into globals below
-    application = service.Application('nova-instancemonitor')
-    m.setServiceParent(application)
-    return application
 
 if __name__ == '__main__':
     twistd.serve(__file__)
 
 if __name__ == '__builtin__':
-    application = main()
+    logging.warn('Starting instance monitor')
-
+    # pylint: disable-msg=C0103
-
+    monitor = monitor.InstanceMonitor()
-
 
+    # This is the parent service that twistd will be looking for when it
+    # parses this file, return it so that we can get it into globals below
+    application = service.Application('nova-instancemonitor')
+    monitor.setServiceParent(application)

bin/nova-manage

@@ -37,12 +37,15 @@ FLAGS = flags.FLAGS
 
 
 class VpnCommands(object):
+    """Class for managing VPNs."""
+
     def __init__(self):
         self.manager = manager.AuthManager()
         self.instdir = model.InstanceDirectory()
         self.pipe = pipelib.CloudPipe(cloud.CloudController())
 
     def list(self):
+        """Print a listing of the VPNs for all projects."""
         print "%-12s\t" % 'project',
         print "%-12s\t" % 'ip:port',
         print "%s" % 'state'
@@ -50,9 +53,11 @@ class VpnCommands(object):
             print "%-12s\t" % project.name,
             print "%s:%s\t" % (project.vpn_ip, project.vpn_port),
 
-            vpn = self.__vpn_for(project.id)
+            vpn = self._vpn_for(project.id)
             if vpn:
-                out, err = utils.execute("ping -c1 -w1 %s > /dev/null; echo $?" % vpn['private_dns_name'])
+                command = "ping -c1 -w1 %s > /dev/null; echo $?"
+                out, _err = utils.execute(  command % vpn['private_dns_name'],
+                                            check_exit_code=False)
                 if out.strip() == '0':
                     net = 'up'
                 else:
@@ -66,25 +71,32 @@ class VpnCommands(object):
             else:
                 print None
 
-    def __vpn_for(self, project_id):
+    def _vpn_for(self, project_id):
+        """Get the VPN instance for a project ID."""
         for instance in self.instdir.all:
-            if (instance.state.has_key('image_id')
+            if ('image_id' in instance.state
                 and instance['image_id'] == FLAGS.vpn_image_id
-                and not instance['state_description'] in ['shutting_down', 'shutdown']
+                and not instance['state_description'] in
+                    ['shutting_down', 'shutdown']
                 and instance['project_id'] == project_id):
                 return instance
 
     def spawn(self):
+        """Run all VPNs."""
         for p in reversed(self.manager.get_projects()):
-             if not self.__vpn_for(p.id):
+            if not self._vpn_for(p.id):
-                 print 'spawning %s' % p.id
+                print 'spawning %s' % p.id
-                 self.pipe.launch_vpn_instance(p.id)
+                self.pipe.launch_vpn_instance(p.id)
-                 time.sleep(10)
+                time.sleep(10)
 
     def run(self, project_id):
+        """Start the VPN for a given project."""
         self.pipe.launch_vpn_instance(project_id)
 
+
 class RoleCommands(object):
+    """Class for managing roles."""
+
     def __init__(self):
         self.manager = manager.AuthManager()
 
@@ -107,25 +119,24 @@ class RoleCommands(object):
         arguments: user, role [project]"""
         self.manager.remove_role(user, role, project)
 
+
 class UserCommands(object):
+    """Class for managing users."""
+
     def __init__(self):
         self.manager = manager.AuthManager()
 
-    def __print_export(self, user):
-        print 'export EC2_ACCESS_KEY=%s' % user.access
-        print 'export EC2_SECRET_KEY=%s' % user.secret
-
     def admin(self, name, access=None, secret=None):
         """creates a new admin and prints exports
         arguments: name [access] [secret]"""
         user = self.manager.create_user(name, access, secret, True)
-        self.__print_export(user)
+        print_export(user)
 
     def create(self, name, access=None, secret=None):
         """creates a new user and prints exports
         arguments: name [access] [secret]"""
         user = self.manager.create_user(name, access, secret, False)
-        self.__print_export(user)
+        print_export(user)
 
     def delete(self, name):
         """deletes an existing user
@@ -137,7 +148,7 @@ class UserCommands(object):
         arguments: name"""
         user = self.manager.get_user(name)
         if user:
-            self.__print_export(user)
+            print_export(user)
         else:
             print "User %s doesn't exist" % name
 
@@ -147,56 +158,61 @@ class UserCommands(object):
         for user in self.manager.get_users():
             print user.name
 
+
+def print_export(user):
+    """Print export variables to use with API."""
+    print 'export EC2_ACCESS_KEY=%s' % user.access
+    print 'export EC2_SECRET_KEY=%s' % user.secret
+
+
 class ProjectCommands(object):
+    """Class for managing projects."""
+
     def __init__(self):
         self.manager = manager.AuthManager()
 
     def add(self, project, user):
-        """adds user to project
+        """Adds user to project
         arguments: project user"""
         self.manager.add_to_project(user, project)
 
     def create(self, name, project_manager, description=None):
-        """creates a new project
+        """Creates a new project
         arguments: name project_manager [description]"""
-        user = self.manager.create_project(name, project_manager, description)
+        self.manager.create_project(name, project_manager, description)
 
     def delete(self, name):
-        """deletes an existing project
+        """Deletes an existing project
         arguments: name"""
         self.manager.delete_project(name)
 
     def environment(self, project_id, user_id, filename='novarc'):
-        """exports environment variables to an sourcable file
+        """Exports environment variables to an sourcable file
         arguments: project_id user_id [filename='novarc]"""
         rc = self.manager.get_environment_rc(project_id, user_id)
         with open(filename, 'w') as f:
             f.write(rc)
 
     def list(self):
-        """lists all projects
+        """Lists all projects
         arguments: <none>"""
         for project in self.manager.get_projects():
             print project.name
 
     def remove(self, project, user):
-        """removes user from project
+        """Removes user from project
         arguments: project user"""
         self.manager.remove_from_project(user, project)
 
-    def zip(self, project_id, user_id, filename='nova.zip'):
+    def zipfile(self, project_id, user_id, filename='nova.zip'):
-        """exports credentials for project to a zip file
+        """Exports credentials for project to a zip file
         arguments: project_id user_id [filename='nova.zip]"""
-        zip = self.manager.get_credentials(project_id, user_id)
+        zip_file = self.manager.get_credentials(user_id, project_id)
         with open(filename, 'w') as f:
-            f.write(zip)
+            f.write(zip_file)
 
 
-def usage(script_name):
+CATEGORIES = [
-    print script_name + " category action [<args>]"
-
-
-categories = [
     ('user', UserCommands),
     ('project', ProjectCommands),
     ('role', RoleCommands),
@@ -205,62 +221,61 @@ categories = [
 
 
 def lazy_match(name, key_value_tuples):
-    """finds all objects that have a key that case insensitively contains [name]
+    """Finds all objects that have a key that case insensitively contains
-    key_value_tuples is a list of tuples of the form (key, value)
+    [name] key_value_tuples is a list of tuples of the form (key, value)
     returns a list of tuples of the form (key, value)"""
-    return [(k, v) for (k, v) in key_value_tuples if k.lower().find(name.lower()) == 0]
+    result = []
+    for (k, v) in key_value_tuples:
+        if k.lower().find(name.lower()) == 0:
+            result.append((k, v))
+    if len(result) == 0:
+        print "%s does not match any options:" % name
+        for k, _v in key_value_tuples:
+            print "\t%s" % k
+        sys.exit(2)
+    if len(result) > 1:
+        print "%s matched multiple options:" % name
+        for k, _v in result:
+            print "\t%s" % k
+        sys.exit(2)
+    return result
 
 
 def methods_of(obj):
-    """get all callable methods of an object that don't start with underscore
+    """Get all callable methods of an object that don't start with underscore
     returns a list of tuples of the form (method_name, method)"""
-    return [(i, getattr(obj, i)) for i in dir(obj) if callable(getattr(obj, i)) and not i.startswith('_')]
+    result = []
+    for i in dir(obj):
+        if callable(getattr(obj, i)) and not i.startswith('_'):
+            result.append((i, getattr(obj, i)))
+    return result
 
 
-if __name__ == '__main__':
+def main():
+    """Parse options and call the appropriate class/method."""
     utils.default_flagfile('/etc/nova/nova-manage.conf')
     argv = FLAGS(sys.argv)
     script_name = argv.pop(0)
     if len(argv) < 1:
-        usage(script_name)
+        print script_name + " category action [<args>]"
         print "Available categories:"
-        for k, v in categories:
+        for k, _ in CATEGORIES:
             print "\t%s" % k
         sys.exit(2)
     category = argv.pop(0)
-    matches = lazy_match(category, categories)
+    matches = lazy_match(category, CATEGORIES)
-    if len(matches) == 0:
-        print "%s does not match any categories:" % category
-        for k, v in categories:
-            print "\t%s" % k
-        sys.exit(2)
-    if len(matches) > 1:
-        print "%s matched multiple categories:" % category
-        for k, v in matches:
-            print "\t%s" % k
-        sys.exit(2)
     # instantiate the command group object
     category, fn = matches[0]
     command_object = fn()
     actions = methods_of(command_object)
     if len(argv) < 1:
-        usage(script_name)
+        print script_name + " category action [<args>]"
         print "Available actions for %s category:" % category
-        for k, v in actions:
+        for k, _v in actions:
             print "\t%s" % k
         sys.exit(2)
     action = argv.pop(0)
     matches = lazy_match(action, actions)
-    if len(matches) == 0:
-        print "%s does not match any actions" % action
-        for k, v in actions:
-            print "\t%s" % k
-        sys.exit(2)
-    if len(matches) > 1:
-        print "%s matched multiple actions:" % action
-        for k, v in matches:
-            print "\t%s" % k
-        sys.exit(2)
     action, fn = matches[0]
     # call the action with the remaining arguments
     try:
@@ -271,3 +286,5 @@ if __name__ == '__main__':
         print "%s %s: %s" % (category, action, fn.__doc__)
         sys.exit(2)
 
+if __name__ == '__main__':
+    main()

bin/nova-network

@@ -33,4 +33,5 @@ if __name__ == '__main__':
     twistd.serve(__file__)
 
 if __name__ == '__builtin__':
+    # pylint: disable-msg=C0103
     application = service.type_to_class(FLAGS.network_type).create()

bin/nova-objectstore

@@ -30,15 +30,9 @@ from nova.objectstore import handler
 FLAGS = flags.FLAGS
 
 
-def main():
-    app = handler.get_application()
-    print app
-    return app
-
-# NOTE(soren): Stolen from nova-compute
 if __name__ == '__main__':
     twistd.serve(__file__)
 
 if __name__ == '__builtin__':
     utils.default_flagfile()
-    application = main()
+    application = handler.get_application() # pylint: disable-msg=C0103

bin/nova-rsapi

@@ -1,58 +0,0 @@
-#!/usr/bin/env python
-# 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.
-#
-#    Licensed under the Apache License, Version 2.0 (the "License");
-#    you may not use this file except in compliance with the License.
-#    You may obtain a copy of the License at
-#
-#        http://www.apache.org/licenses/LICENSE-2.0
-#
-#    Unless required by applicable law or agreed to in writing, software
-#    distributed under the License is distributed on an "AS IS" BASIS,
-#    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-#    See the License for the specific language governing permissions and
-#    limitations under the License.
-"""
-  WSGI daemon for the main API endpoint.
-"""
-
-import logging
-from tornado import ioloop
-from wsgiref import simple_server
-
-from nova import flags
-from nova import rpc
-from nova import server
-from nova import utils
-from nova.auth import manager
-from nova.endpoint import rackspace
-
-FLAGS = flags.FLAGS
-flags.DEFINE_integer('cc_port', 8773, 'cloud controller port')
-
-def main(_argv):
-    user_manager = manager.AuthManager()
-    api_instance = rackspace.Api(user_manager)
-    conn = rpc.Connection.instance()
-    rpc_consumer = rpc.AdapterConsumer(connection=conn,
-                                       topic=FLAGS.cloud_topic,
-                                       proxy=api_instance)
-
-#   TODO: fire rpc response listener (without attach to tornado)
-#    io_inst = ioloop.IOLoop.instance()
-#    _injected = consumer.attach_to_tornado(io_inst)
-
-    http_server = simple_server.WSGIServer(('0.0.0.0', FLAGS.cc_port), simple_server.WSGIRequestHandler)
-    http_server.set_app(api_instance.handler)
-    logging.debug('Started HTTP server on port %i' % FLAGS.cc_port)
-    while True:
-        http_server.handle_request()
-#    io_inst.start()
-
-if __name__ == '__main__':
-    utils.default_flagfile()
-    server.serve('nova-rsapi', main)

bin/nova-volume

@@ -29,4 +29,4 @@ if __name__ == '__main__':
     twistd.serve(__file__)
 
 if __name__ == '__builtin__':
-    application = service.VolumeService.create()
+    application = service.VolumeService.create() # pylint: disable-msg=C0103

nova/adminclient.py

@@ -20,6 +20,7 @@ Nova User API client library.
 """
 
 import base64
+
 import boto
 from boto.ec2.regioninfo import RegionInfo
 
@@ -57,6 +58,30 @@ class UserInfo(object):
         elif name == 'secretkey':
             self.secretkey = str(value)
 
+
+class UserRole(object):
+    """
+    Information about a Nova user's role, as parsed through SAX.
+    Fields include:
+        role
+    """
+    def __init__(self, connection=None):
+        self.connection = connection
+        self.role = None
+    
+    def __repr__(self):
+        return 'UserRole:%s' % self.role
+
+    def startElement(self, name, attrs, connection):
+        return None
+        
+    def endElement(self, name, value, connection):
+        if name == 'role':
+            self.role = value
+        else:
+            setattr(self, name, str(value))
+
+
 class ProjectInfo(object):
     """
     Information about a Nova project, as parsed through SAX
@@ -92,12 +117,14 @@ class ProjectInfo(object):
         else:
             setattr(self, name, str(value))
 
+
 class ProjectMember(object):
     """
     Information about a Nova project member, as parsed through SAX.
     Fields include:
         memberId
     """
+
     def __init__(self, connection=None):
         self.connection = connection
         self.memberId = None
@@ -113,8 +140,8 @@ class ProjectMember(object):
             self.memberId = value
         else:
             setattr(self, name, str(value))
-            
 
+            
 class HostInfo(object):
     """
     Information about a Nova Host, as parsed through SAX:
@@ -142,6 +169,7 @@ class HostInfo(object):
     def endElement(self, name, value, connection):
         setattr(self, name, value)
 
+
 class NovaAdminClient(object):
     def __init__(self, clc_ip='127.0.0.1', region='nova', access_key='admin',
                  secret_key='admin', **kwargs):
@@ -196,6 +224,24 @@ class NovaAdminClient(object):
         """ deletes a user """
         return self.apiconn.get_object('DeregisterUser', {'Name': username}, UserInfo)
 
+    def get_roles(self, project_roles=True):
+        """Returns a list of available roles."""
+        return self.apiconn.get_list('DescribeRoles',
+                                     {'ProjectRoles': project_roles},
+                                     [('item', UserRole)])
+
+    def get_user_roles(self, user, project=None):
+        """Returns a list of roles for the given user.
+           Omitting project will return any global roles that the user has.
+           Specifying project will return only project specific roles.
+        """
+        params = {'User':user}
+        if project:
+            params['Project'] = project
+        return self.apiconn.get_list('DescribeUserRoles',
+                                     params,
+                                     [('item', UserRole)])
+
     def add_user_role(self, user, role, project=None):
         """
         Add a role to a user either globally or for a specific project.

nova/auth/fakeldap.py

@@ -30,20 +30,23 @@ from nova import datastore
 
 SCOPE_BASE = 0
 SCOPE_ONELEVEL = 1 # not implemented
-SCOPE_SUBTREE  = 2
+SCOPE_SUBTREE = 2
 MOD_ADD = 0
 MOD_DELETE = 1
 
 
-class NO_SUCH_OBJECT(Exception):
+class NO_SUCH_OBJECT(Exception): # pylint: disable-msg=C0103
+    """Duplicate exception class from real LDAP module."""
     pass
 
 
-class OBJECT_CLASS_VIOLATION(Exception):
+class OBJECT_CLASS_VIOLATION(Exception): # pylint: disable-msg=C0103
+    """Duplicate exception class from real LDAP module."""
     pass
 
 
-def initialize(uri):
+def initialize(_uri):
+    """Opens a fake connection with an LDAP server."""
     return FakeLDAP()
 
 
@@ -68,7 +71,7 @@ def _match_query(query, attrs):
         # cut off the ! and the nested parentheses
         return not _match_query(query[2:-1], attrs)
 
-    (k, sep, v) = inner.partition('=')
+    (k, _sep, v) = inner.partition('=')
     return _match(k, v, attrs)
 
 
@@ -85,20 +88,20 @@ def _paren_groups(source):
         if source[pos] == ')':
             count -= 1
             if count == 0:
-                result.append(source[start:pos+1])
+                result.append(source[start:pos + 1])
     return result
 
 
-def _match(k, v, attrs):
+def _match(key, value, attrs):
     """Match a given key and value against an attribute list."""
-    if k not in attrs:
+    if key not in attrs:
         return False
-    if k != "objectclass":
+    if key != "objectclass":
-        return v in attrs[k]
+        return value in attrs[key]
     # it is an objectclass check, so check subclasses
-    values = _subs(v)
+    values = _subs(value)
-    for value in values:
+    for v in values:
-        if value in attrs[k]:
+        if v in attrs[key]:
             return True
     return False
 
@@ -145,6 +148,7 @@ def _to_json(unencoded):
 class FakeLDAP(object):
     #TODO(vish): refactor this class to use a wrapper instead of accessing
     #            redis directly
+    """Fake LDAP connection."""
 
     def simple_bind_s(self, dn, password):
         """This method is ignored, but provided for compatibility."""
@@ -207,6 +211,7 @@ class FakeLDAP(object):
             # get the attributes from redis
             attrs = redis.hgetall(key)
             # turn the values from redis into lists
+            # pylint: disable-msg=E1103
             attrs = dict([(k, _from_json(v))
                           for k, v in attrs.iteritems()])
             # filter the objects by query
@@ -215,13 +220,12 @@ class FakeLDAP(object):
                 attrs = dict([(k, v) for k, v in attrs.iteritems()
                               if not fields or k in fields])
                 objects.append((key[len(self.__redis_prefix):], attrs))
+            # pylint: enable-msg=E1103
         if objects == []:
             raise NO_SUCH_OBJECT()
         return objects
 
-
     @property
-    def __redis_prefix(self):
+    def __redis_prefix(self): # pylint: disable-msg=R0201
+        """Get the prefix to use for all redis keys."""
         return 'ldap:'
-
-

nova/auth/ldapdriver.py

@@ -30,10 +30,11 @@ import sys
 from nova import exception
 from nova import flags
 
+
 FLAGS = flags.FLAGS
 flags.DEFINE_string('ldap_url', 'ldap://localhost',
                     'Point this at your ldap server')
-flags.DEFINE_string('ldap_password',  'changeme', 'LDAP password')
+flags.DEFINE_string('ldap_password', 'changeme', 'LDAP password')
 flags.DEFINE_string('ldap_user_dn', 'cn=Manager,dc=example,dc=com',
                     'DN of admin user')
 flags.DEFINE_string('ldap_user_unit', 'Users', 'OID for Users')
@@ -62,14 +63,18 @@ flags.DEFINE_string('ldap_developer',
 #             to define a set interface for AuthDrivers. I'm delaying
 #             creating this now because I'm expecting an auth refactor
 #             in which we may want to change the interface a bit more.
+
+
 class LdapDriver(object):
     """Ldap Auth driver
 
     Defines enter and exit and therefore supports the with/as syntax.
     """
+
     def __init__(self):
         """Imports the LDAP module"""
         self.ldap = __import__('ldap')
+        self.conn = None
 
     def __enter__(self):
         """Creates the connection to LDAP"""
@@ -77,7 +82,7 @@ class LdapDriver(object):
         self.conn.simple_bind_s(FLAGS.ldap_user_dn, FLAGS.ldap_password)
         return self
 
-    def __exit__(self, type, value, traceback):
+    def __exit__(self, exc_type, exc_value, traceback):
         """Destroys the connection to LDAP"""
         self.conn.unbind_s()
         return False
@@ -122,11 +127,11 @@ class LdapDriver(object):
 
     def get_projects(self, uid=None):
         """Retrieve list of projects"""
-        filter = '(objectclass=novaProject)'
+        pattern = '(objectclass=novaProject)'
         if uid:
-            filter = "(&%s(member=%s))" % (filter, self.__uid_to_dn(uid))
+            pattern = "(&%s(member=%s))" % (pattern, self.__uid_to_dn(uid))
         attrs = self.__find_objects(FLAGS.ldap_project_subtree,
-                                  filter)
+                                    pattern)
         return [self.__to_project(attr) for attr in attrs]
 
     def create_user(self, name, access_key, secret_key, is_admin):
@@ -182,7 +187,8 @@ class LdapDriver(object):
             for member_uid in member_uids:
                 if not self.__user_exists(member_uid):
                     raise exception.NotFound("Project can't be created "
-                            "because user %s doesn't exist" % member_uid)
+                                             "because user %s doesn't exist"
+                                             % member_uid)
                 members.append(self.__uid_to_dn(member_uid))
         # always add the manager as a member because members is required
         if not manager_dn in members:
@@ -192,8 +198,7 @@ class LdapDriver(object):
             ('cn', [name]),
             ('description', [description]),
             ('projectManager', [manager_dn]),
-            ('member', members)
+            ('member', members)]
-        ]
         self.conn.add_s('cn=%s,%s' % (name, FLAGS.ldap_project_subtree), attr)
         return self.__to_project(dict(attr))
 
@@ -236,6 +241,26 @@ class LdapDriver(object):
         role_dn = self.__role_to_dn(role, project_id)
         return self.__remove_from_group(uid, role_dn)
 
+    def get_user_roles(self, uid, project_id=None):
+        """Retrieve list of roles for user (or user and project)"""
+        if project_id is None:
+            # NOTE(vish): This is unneccesarily slow, but since we can't
+            #             guarantee that the global roles are located
+            #             together in the ldap tree, we're doing this version.
+            roles = []
+            for role in FLAGS.allowed_roles:
+                role_dn = self.__role_to_dn(role)
+                if self.__is_in_group(uid, role_dn):
+                    roles.append(role)
+            return roles
+        else:
+            project_dn = 'cn=%s,%s' % (project_id, FLAGS.ldap_project_subtree)
+            roles = self.__find_objects(project_dn,
+                                        '(&(&(objectclass=groupOfNames)'
+                                        '(!(objectclass=novaProject)))'
+                                        '(member=%s))' % self.__uid_to_dn(uid))
+            return [role['cn'][0] for role in roles]
+
     def delete_user(self, uid):
         """Delete a user"""
         if not self.__user_exists(uid):
@@ -253,24 +278,23 @@ class LdapDriver(object):
         self.conn.delete_s('cn=%s,uid=%s,%s' % (key_name, uid,
                                           FLAGS.ldap_user_subtree))
 
-    def delete_project(self, name):
+    def delete_project(self, project_id):
         """Delete a project"""
-        project_dn = 'cn=%s,%s' % (name, FLAGS.ldap_project_subtree)
+        project_dn = 'cn=%s,%s' % (project_id, FLAGS.ldap_project_subtree)
         self.__delete_roles(project_dn)
         self.__delete_group(project_dn)
 
-    def __user_exists(self, name):
+    def __user_exists(self, uid):
         """Check if user exists"""
-        return self.get_user(name) != None
+        return self.get_user(uid) != None
 
     def __key_pair_exists(self, uid, key_name):
         """Check if key pair exists"""
-        return self.get_user(uid) != None
         return self.get_key_pair(uid, key_name) != None
 
-    def __project_exists(self, name):
+    def __project_exists(self, project_id):
         """Check if project exists"""
-        return self.get_project(name) != None
+        return self.get_project(project_id) != None
 
     def __find_object(self, dn, query=None, scope=None):
         """Find an object by dn and query"""
@@ -288,7 +312,7 @@ class LdapDriver(object):
         except self.ldap.NO_SUCH_OBJECT:
             return []
         # just return the DNs
-        return [dn for dn, attributes in res]
+        return [dn for dn, _attributes in res]
 
     def __find_objects(self, dn, query=None, scope=None):
         """Find objects by query"""
@@ -324,7 +348,8 @@ class LdapDriver(object):
             for key in keys:
                 self.delete_key_pair(uid, key['name'])
 
-    def __role_to_dn(self, role, project_id=None):
+    @staticmethod
+    def __role_to_dn(role, project_id=None):
         """Convert role to corresponding dn"""
         if project_id == None:
             return FLAGS.__getitem__("ldap_%s" % role).value
@@ -334,7 +359,7 @@ class LdapDriver(object):
                                        FLAGS.ldap_project_subtree)
 
     def __create_group(self, group_dn, name, uid,
-                       description, member_uids = None):
+                       description, member_uids=None):
         """Create a group"""
         if self.__group_exists(group_dn):
             raise exception.Duplicate("Group can't be created because "
@@ -353,8 +378,7 @@ class LdapDriver(object):
             ('objectclass', ['groupOfNames']),
             ('cn', [name]),
             ('description', [description]),
-            ('member', members)
+            ('member', members)]
-        ]
         self.conn.add_s(group_dn, attr)
 
     def __is_in_group(self, uid, group_dn):
@@ -380,9 +404,7 @@ class LdapDriver(object):
         if self.__is_in_group(uid, group_dn):
             raise exception.Duplicate("User %s is already a member of "
                                       "the group %s" % (uid, group_dn))
-        attr = [
+        attr = [(self.ldap.MOD_ADD, 'member', self.__uid_to_dn(uid))]
-            (self.ldap.MOD_ADD, 'member', self.__uid_to_dn(uid))
-        ]
         self.conn.modify_s(group_dn, attr)
 
     def __remove_from_group(self, uid, group_dn):
@@ -410,7 +432,7 @@ class LdapDriver(object):
             self.conn.modify_s(group_dn, attr)
         except self.ldap.OBJECT_CLASS_VIOLATION:
             logging.debug("Attempted to remove the last member of a group. "
-                          "Deleting the group at %s instead." % group_dn )
+                          "Deleting the group at %s instead.", group_dn)
             self.__delete_group(group_dn)
 
     def __remove_from_all(self, uid):
@@ -418,7 +440,6 @@ class LdapDriver(object):
         if not self.__user_exists(uid):
             raise exception.NotFound("User %s can't be removed from all "
                     "because the user doesn't exist" % (uid,))
-        dn = self.__uid_to_dn(uid)
         role_dns = self.__find_group_dns_with_member(
                 FLAGS.role_project_subtree, uid)
         for role_dn in role_dns:
@@ -426,7 +447,7 @@ class LdapDriver(object):
         project_dns = self.__find_group_dns_with_member(
                 FLAGS.ldap_project_subtree, uid)
         for project_dn in project_dns:
-            self.__safe_remove_from_group(uid, role_dn)
+            self.__safe_remove_from_group(uid, project_dn)
 
     def __delete_group(self, group_dn):
         """Delete Group"""
@@ -439,7 +460,8 @@ class LdapDriver(object):
         for role_dn in self.__find_role_dns(project_dn):
             self.__delete_group(role_dn)
 
-    def __to_user(self, attr):
+    @staticmethod
+    def __to_user(attr):
         """Convert ldap attributes to User object"""
         if attr == None:
             return None
@@ -448,10 +470,10 @@ class LdapDriver(object):
             'name': attr['cn'][0],
             'access': attr['accessKey'][0],
             'secret': attr['secretKey'][0],
-            'admin': (attr['isAdmin'][0] == 'TRUE')
+            'admin': (attr['isAdmin'][0] == 'TRUE')}
-        }
 
-    def __to_key_pair(self, owner, attr):
+    @staticmethod
+    def __to_key_pair(owner, attr):
         """Convert ldap attributes to KeyPair object"""
         if attr == None:
             return None
@@ -460,8 +482,7 @@ class LdapDriver(object):
             'name': attr['cn'][0],
             'owner_id': owner,
             'public_key': attr['sshPublicKey'][0],
-            'fingerprint': attr['keyFingerprint'][0],
+            'fingerprint': attr['keyFingerprint'][0]}
-        }
 
     def __to_project(self, attr):
         """Convert ldap attributes to Project object"""
@@ -473,21 +494,22 @@ class LdapDriver(object):
             'name': attr['cn'][0],
             'project_manager_id': self.__dn_to_uid(attr['projectManager'][0]),
             'description': attr.get('description', [None])[0],
-            'member_ids': [self.__dn_to_uid(x) for x in member_dns]
+            'member_ids': [self.__dn_to_uid(x) for x in member_dns]}
-        }
 
-    def __dn_to_uid(self, dn):
+    @staticmethod
+    def __dn_to_uid(dn):
         """Convert user dn to uid"""
         return dn.split(',')[0].split('=')[1]
 
-    def __uid_to_dn(self, dn):
+    @staticmethod
+    def __uid_to_dn(dn):
         """Convert uid to dn"""
         return 'uid=%s,%s' % (dn, FLAGS.ldap_user_subtree)
 
 
 class FakeLdapDriver(LdapDriver):
     """Fake Ldap Auth driver"""
-    def __init__(self):
+
+    def __init__(self): # pylint: disable-msg=W0231
         __import__('nova.auth.fakeldap')
         self.ldap = sys.modules['nova.auth.fakeldap']
-

nova/auth/manager.py

@@ -23,22 +23,23 @@ Nova authentication management
 import logging
 import os
 import shutil
-import string
+import string # pylint: disable-msg=W0402
 import tempfile
 import uuid
 import zipfile
 
 from nova import crypto
-from nova import datastore
 from nova import exception
 from nova import flags
-from nova import objectstore # for flags
 from nova import utils
-from nova.auth import ldapdriver # for flags
 from nova.auth import signer
 from nova.network import vpn
 
+
 FLAGS = flags.FLAGS
+flags.DEFINE_list('allowed_roles',
+                  ['cloudadmin', 'itsec', 'sysadmin', 'netadmin', 'developer'],
+                  'Allowed roles for project')
 
 # NOTE(vish): a user with one of these roles will be a superuser and
 #             have access to all api commands
@@ -50,7 +51,6 @@ flags.DEFINE_list('superuser_roles', ['cloudadmin'],
 flags.DEFINE_list('global_roles', ['cloudadmin', 'itsec'],
                   'Roles that apply to all projects')
 
-
 flags.DEFINE_string('credentials_template',
                     utils.abspath('auth/novarc.template'),
                     'Template for creating users rc file')
@@ -65,15 +65,14 @@ flags.DEFINE_string('credential_cert_file', 'cert.pem',
                     'Filename of certificate in credentials zip')
 flags.DEFINE_string('credential_rc_file', 'novarc',
                     'Filename of rc in credentials zip')
-
 flags.DEFINE_string('credential_cert_subject',
                     '/C=US/ST=California/L=MountainView/O=AnsoLabs/'
                     'OU=NovaDev/CN=%s-%s',
                     'Subject for certificate for users')
-
 flags.DEFINE_string('auth_driver', 'nova.auth.ldapdriver.FakeLdapDriver',
                     'Driver that auth manager uses')
 
+
 class AuthBase(object):
     """Base class for objects relating to auth
 
@@ -81,6 +80,7 @@ class AuthBase(object):
     an id member. They may optionally contain methods that delegate to
     AuthManager, but should not implement logic themselves.
     """
+
     @classmethod
     def safe_id(cls, obj):
         """Safe get object id
@@ -98,7 +98,9 @@ class AuthBase(object):
 
 class User(AuthBase):
     """Object representing a user"""
+
     def __init__(self, id, name, access, secret, admin):
+        AuthBase.__init__(self)
         self.id = id
         self.name = name
         self.access = access
@@ -158,7 +160,9 @@ class KeyPair(AuthBase):
     Even though this object is named KeyPair, only the public key and
     fingerprint is stored. The user's private key is not saved.
     """
+
     def __init__(self, id, name, owner_id, public_key, fingerprint):
+        AuthBase.__init__(self)
         self.id = id
         self.name = name
         self.owner_id = owner_id
@@ -175,7 +179,9 @@ class KeyPair(AuthBase):
 
 class Project(AuthBase):
     """Represents a Project returned from the datastore"""
+
     def __init__(self, id, name, project_manager_id, description, member_ids):
+        AuthBase.__init__(self)
         self.id = id
         self.name = name
         self.project_manager_id = project_manager_id
@@ -188,12 +194,12 @@ class Project(AuthBase):
 
     @property
     def vpn_ip(self):
-        ip, port = AuthManager().get_project_vpn_data(self)
+        ip, _port = AuthManager().get_project_vpn_data(self)
         return ip
 
     @property
     def vpn_port(self):
-        ip, port = AuthManager().get_project_vpn_data(self)
+        _ip, port = AuthManager().get_project_vpn_data(self)
         return port
 
     def has_manager(self, user):
@@ -215,12 +221,9 @@ class Project(AuthBase):
         return AuthManager().get_credentials(user, self)
 
     def __repr__(self):
-        return "Project('%s', '%s', '%s', '%s', %s)" % (self.id,
+        return "Project('%s', '%s', '%s', '%s', %s)" % \
-                                                        self.name,
+            (self.id, self.name, self.project_manager_id, self.description,
-                                                        self.project_manager_id,
+             self.member_ids)
-                                                        self.description,
-                                                        self.member_ids)
-
 
 
 class AuthManager(object):
@@ -234,7 +237,9 @@ class AuthManager(object):
     AuthManager also manages associated data related to Auth objects that
     need to be more accessible, such as vpn ips and ports.
     """
-    _instance=None
+
+    _instance = None
+
     def __new__(cls, *args, **kwargs):
         """Returns the AuthManager singleton"""
         if not cls._instance:
@@ -248,7 +253,7 @@ class AuthManager(object):
         reset the driver if it is not set or a new driver is specified.
         """
         if driver or not getattr(self, 'driver', None):
-           self.driver = utils.import_class(driver or FLAGS.auth_driver)
+            self.driver = utils.import_class(driver or FLAGS.auth_driver)
 
     def authenticate(self, access, signature, params, verb='GET',
                      server_string='127.0.0.1:8773', path='/',
@@ -290,7 +295,7 @@ class AuthManager(object):
         @return: User and project that the request represents.
         """
         # TODO(vish): check for valid timestamp
-        (access_key, sep, project_id) = access.partition(':')
+        (access_key, _sep, project_id) = access.partition(':')
 
         logging.info('Looking up user: %r', access_key)
         user = self.get_user_from_access_key(access_key)
@@ -313,7 +318,8 @@ class AuthManager(object):
             raise exception.NotFound('User %s is not a member of project %s' %
                                      (user.id, project.id))
         if check_type == 's3':
-            expected_signature = signer.Signer(user.secret.encode()).s3_authorization(headers, verb, path)
+            sign = signer.Signer(user.secret.encode())
+            expected_signature = sign.s3_authorization(headers, verb, path)
             logging.debug('user.secret: %s', user.secret)
             logging.debug('expected_signature: %s', expected_signature)
             logging.debug('signature: %s', signature)
@@ -431,6 +437,10 @@ class AuthManager(object):
         @type project: Project or project_id
         @param project: Project in which to add local role.
         """
+        if role not in FLAGS.allowed_roles:
+            raise exception.NotFound("The %s role can not be found" % role)
+        if project is not None and role in FLAGS.global_roles:
+            raise exception.NotFound("The %s role is global only" % role)
         with self.driver() as drv:
             drv.add_role(User.safe_id(user), role, Project.safe_id(project))
 
@@ -454,6 +464,20 @@ class AuthManager(object):
         with self.driver() as drv:
             drv.remove_role(User.safe_id(user), role, Project.safe_id(project))
 
+    @staticmethod
+    def get_roles(project_roles=True):
+        """Get list of allowed roles"""
+        if project_roles:
+            return list(set(FLAGS.allowed_roles) - set(FLAGS.global_roles))
+        else:
+            return FLAGS.allowed_roles
+
+    def get_user_roles(self, user, project=None):
+        """Get user global or per-project roles"""
+        with self.driver() as drv:
+            return drv.get_user_roles(User.safe_id(user),
+                                      Project.safe_id(project))
+
     def get_project(self, pid):
         """Get project object by id"""
         with self.driver() as drv:
@@ -494,10 +518,10 @@ class AuthManager(object):
         if member_users:
             member_users = [User.safe_id(u) for u in member_users]
         with self.driver() as drv:
-            project_dict =  drv.create_project(name,
+            project_dict = drv.create_project(name,
-                                               User.safe_id(manager_user),
+                                              User.safe_id(manager_user),
-                                               description,
+                                              description,
-                                               member_users)
+                                              member_users)
             if project_dict:
                 return Project(**project_dict)
 
@@ -525,7 +549,8 @@ class AuthManager(object):
             return drv.remove_from_project(User.safe_id(user),
                                             Project.safe_id(project))
 
-    def get_project_vpn_data(self, project):
+    @staticmethod
+    def get_project_vpn_data(project):
         """Gets vpn ip and port for project
 
         @type project: Project or project_id
@@ -589,8 +614,10 @@ class AuthManager(object):
         @rtype: User
         @return: The new user.
         """
-        if access == None: access = str(uuid.uuid4())
+        if access == None:
-        if secret == None: secret = str(uuid.uuid4())
+            access = str(uuid.uuid4())
+        if secret == None:
+            secret = str(uuid.uuid4())
         with self.driver() as drv:
             user_dict = drv.create_user(name, access, secret, admin)
             if user_dict:
@@ -632,10 +659,10 @@ class AuthManager(object):
     def create_key_pair(self, user, key_name, public_key, fingerprint):
         """Creates a key pair for user"""
         with self.driver() as drv:
-            kp_dict =  drv.create_key_pair(User.safe_id(user),
+            kp_dict = drv.create_key_pair(User.safe_id(user),
-                                           key_name,
+                                          key_name,
-                                           public_key,
+                                          public_key,
-                                           fingerprint)
+                                          fingerprint)
             if kp_dict:
                 return KeyPair(**kp_dict)
 
@@ -678,7 +705,7 @@ class AuthManager(object):
 
         network_data = vpn.NetworkData.lookup(pid)
         if network_data:
-            configfile = open(FLAGS.vpn_client_template,"r")
+            configfile = open(FLAGS.vpn_client_template, "r")
             s = string.Template(configfile.read())
             configfile.close()
             config = s.substitute(keyfile=FLAGS.credential_key_file,
@@ -693,10 +720,10 @@ class AuthManager(object):
         zippy.writestr(FLAGS.ca_file, crypto.fetch_ca(user.id))
         zippy.close()
         with open(zf, 'rb') as f:
-            buffer = f.read()
+            read_buffer = f.read()
 
         shutil.rmtree(tmpdir)
-        return buffer
+        return read_buffer
 
     def get_environment_rc(self, user, project=None):
         """Get credential zip for user in project"""
@@ -707,18 +734,18 @@ class AuthManager(object):
         pid = Project.safe_id(project)
         return self.__generate_rc(user.access, user.secret, pid)
 
-    def __generate_rc(self, access, secret, pid):
+    @staticmethod
+    def __generate_rc(access, secret, pid):
         """Generate rc file for user"""
         rc = open(FLAGS.credentials_template).read()
-        rc = rc % { 'access': access,
+        rc = rc % {'access': access,
-                    'project': pid,
+                   'project': pid,
-                    'secret': secret,
+                   'secret': secret,
-                    'ec2': FLAGS.ec2_url,
+                   'ec2': FLAGS.ec2_url,
-                    's3': 'http://%s:%s' % (FLAGS.s3_host, FLAGS.s3_port),
+                   's3': 'http://%s:%s' % (FLAGS.s3_host, FLAGS.s3_port),
-                    'nova': FLAGS.ca_file,
+                   'nova': FLAGS.ca_file,
-                    'cert': FLAGS.credential_cert_file,
+                   'cert': FLAGS.credential_cert_file,
-                    'key': FLAGS.credential_key_file,
+                   'key': FLAGS.credential_key_file}
-            }
         return rc
 
     def _generate_x509_cert(self, uid, pid):
@@ -729,6 +756,7 @@ class AuthManager(object):
         signed_cert = crypto.sign_csr(csr, pid)
         return (private_key, signed_cert)
 
-    def __cert_subject(self, uid):
+    @staticmethod
+    def __cert_subject(uid):
         """Helper to generate cert subject"""
         return FLAGS.credential_cert_subject % (uid, utils.isotime())

nova/auth/rbac.py

@@ -16,38 +16,54 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+"""Role-based access control decorators to use fpr wrapping other
+methods with."""
+
 from nova import exception
-from nova.auth import manager
 
 
 def allow(*roles):
-    def wrap(f):
+    """Allow the given roles access the wrapped function."""
-        def wrapped_f(self, context, *args, **kwargs):
+
+    def wrap(func): # pylint: disable-msg=C0111
+
+        def wrapped_func(self, context, *args,
+                         **kwargs): # pylint: disable-msg=C0111
             if context.user.is_superuser():
-                return f(self, context, *args, **kwargs)
+                return func(self, context, *args, **kwargs)
             for role in roles:
                 if __matches_role(context, role):
-                    return f(self, context, *args, **kwargs)
+                    return func(self, context, *args, **kwargs)
             raise exception.NotAuthorized()
-        return wrapped_f
+
+        return wrapped_func
+
     return wrap
 
+
 def deny(*roles):
-    def wrap(f):
+    """Deny the given roles access the wrapped function."""
-        def wrapped_f(self, context, *args, **kwargs):
+
+    def wrap(func): # pylint: disable-msg=C0111
+
+        def wrapped_func(self, context, *args,
+                         **kwargs): # pylint: disable-msg=C0111
             if context.user.is_superuser():
-                return f(self, context, *args, **kwargs)
+                return func(self, context, *args, **kwargs)
             for role in roles:
                 if __matches_role(context, role):
                     raise exception.NotAuthorized()
-            return f(self, context, *args, **kwargs)
+            return func(self, context, *args, **kwargs)
-        return wrapped_f
+
+        return wrapped_func
+
     return wrap
 
+
 def __matches_role(context, role):
+    """Check if a role is allowed."""
     if role == 'all':
         return True
     if role == 'none':
         return False
     return context.project.has_role(context.user.id, role)
-

nova/auth/signer.py

@@ -48,13 +48,17 @@ import hashlib
 import hmac
 import logging
 import urllib
-import boto       # NOTE(vish): for new boto
+
-import boto.utils # NOTE(vish): for old boto
+# NOTE(vish): for new boto
+import boto
+# NOTE(vish): for old boto
+import boto.utils
 
 from nova.exception import Error
 
+
 class Signer(object):
-    """ hacked up code from boto/connection.py """
+    """Hacked up code from boto/connection.py"""
 
     def __init__(self, secret_key):
         self.hmac = hmac.new(secret_key, digestmod=hashlib.sha1)
@@ -62,23 +66,27 @@ class Signer(object):
             self.hmac_256 = hmac.new(secret_key, digestmod=hashlib.sha256)
 
     def s3_authorization(self, headers, verb, path):
+        """Generate S3 authorization string."""
         c_string = boto.utils.canonical_string(verb, path, headers)
-        hmac = self.hmac.copy()
+        hmac_copy = self.hmac.copy()
-        hmac.update(c_string)
+        hmac_copy.update(c_string)
-        b64_hmac = base64.encodestring(hmac.digest()).strip()
+        b64_hmac = base64.encodestring(hmac_copy.digest()).strip()
         return b64_hmac
 
     def generate(self, params, verb, server_string, path):
+        """Generate auth string according to what SignatureVersion is given."""
         if params['SignatureVersion'] == '0':
             return self._calc_signature_0(params)
         if params['SignatureVersion'] == '1':
             return self._calc_signature_1(params)
         if params['SignatureVersion'] == '2':
             return self._calc_signature_2(params, verb, server_string, path)
-        raise Error('Unknown Signature Version: %s' % self.SignatureVersion)
+        raise Error('Unknown Signature Version: %s' %
+                    params['SignatureVersion'])
 
-
+    @staticmethod
-    def _get_utf8_value(self, value):
+    def _get_utf8_value(value):
+        """Get the UTF8-encoded version of a value."""
         if not isinstance(value, str) and not isinstance(value, unicode):
             value = str(value)
         if isinstance(value, unicode):
@@ -87,10 +95,11 @@ class Signer(object):
             return value
 
     def _calc_signature_0(self, params):
+        """Generate AWS signature version 0 string."""
         s = params['Action'] + params['Timestamp']
         self.hmac.update(s)
         keys = params.keys()
-        keys.sort(cmp = lambda x, y: cmp(x.lower(), y.lower()))
+        keys.sort(cmp=lambda x, y: cmp(x.lower(), y.lower()))
         pairs = []
         for key in keys:
             val = self._get_utf8_value(params[key])
@@ -98,8 +107,9 @@ class Signer(object):
         return base64.b64encode(self.hmac.digest())
 
     def _calc_signature_1(self, params):
+        """Generate AWS signature version 1 string."""
         keys = params.keys()
-        keys.sort(cmp = lambda x, y: cmp(x.lower(), y.lower()))
+        keys.sort(cmp=lambda x, y: cmp(x.lower(), y.lower()))
         pairs = []
         for key in keys:
             self.hmac.update(key)
@@ -109,29 +119,34 @@ class Signer(object):
         return base64.b64encode(self.hmac.digest())
 
     def _calc_signature_2(self, params, verb, server_string, path):
+        """Generate AWS signature version 2 string."""
         logging.debug('using _calc_signature_2')
         string_to_sign = '%s\n%s\n%s\n' % (verb, server_string, path)
         if self.hmac_256:
-            hmac = self.hmac_256
+            current_hmac = self.hmac_256
             params['SignatureMethod'] = 'HmacSHA256'
         else:
-            hmac = self.hmac
+            current_hmac = self.hmac
             params['SignatureMethod'] = 'HmacSHA1'
         keys = params.keys()
         keys.sort()
         pairs = []
         for key in keys:
             val = self._get_utf8_value(params[key])
-            pairs.append(urllib.quote(key, safe='') + '=' + urllib.quote(val, safe='-_~'))
+            val = urllib.quote(val, safe='-_~')
+            pairs.append(urllib.quote(key, safe='') + '=' + val)
         qs = '&'.join(pairs)
-        logging.debug('query string: %s' % qs)
+        logging.debug('query string: %s', qs)
         string_to_sign += qs
-        logging.debug('string_to_sign: %s' % string_to_sign)
+        logging.debug('string_to_sign: %s', string_to_sign)
-        hmac.update(string_to_sign)
+        current_hmac.update(string_to_sign)
-        b64 = base64.b64encode(hmac.digest())
+        b64 = base64.b64encode(current_hmac.digest())
-        logging.debug('len(b64)=%d' % len(b64))
+        logging.debug('len(b64)=%d', len(b64))
-        logging.debug('base64 encoded digest: %s' % b64)
+        logging.debug('base64 encoded digest: %s', b64)
         return b64
 
+
 if __name__ == '__main__':
-    print Signer('foo').generate({"SignatureMethod": 'HmacSHA256', 'SignatureVersion': '2'}, "get", "server", "/foo")
+    print Signer('foo').generate({'SignatureMethod': 'HmacSHA256',
+                                  'SignatureVersion': '2'},
+                                 'get', 'server', '/foo')

nova/datastore.py

@@ -124,12 +124,16 @@ 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 cls.associated_keys(foreign_type, foreign_id):
-        for identifier in Redis.instance().smembers(redis_set):
             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)
@@ -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):
@@ -170,6 +174,9 @@ class BasicModel(object):
     def setdefault(self, item, default):
         return self.state.setdefault(item, default)
 
+    def __contains__(self, item):
+        return item in self.state
+
     def __getitem__(self, item):
         return self.state[item]
 

nova/endpoint/__init__.py

@@ -1,32 +0,0 @@
-# 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.
-#
-#    Licensed under the Apache License, Version 2.0 (the "License"); you may
-#    not use this file except in compliance with the License. You may obtain
-#    a copy of the License at
-#
-#         http://www.apache.org/licenses/LICENSE-2.0
-#
-#    Unless required by applicable law or agreed to in writing, software
-#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
-#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
-#    License for the specific language governing permissions and limitations
-#    under the License.
-
-"""
-:mod:`nova.endpoint` -- Main NOVA Api endpoints
-=====================================================
-
-.. automodule:: nova.endpoint
-   :platform: Unix
-   :synopsis: REST APIs for all nova functions
-.. moduleauthor:: Jesse Andrews <jesse@ansolabs.com>
-.. moduleauthor:: Devin Carlen <devin.carlen@gmail.com>
-.. moduleauthor:: Vishvananda Ishaya <vishvananda@yahoo.com>
-.. moduleauthor:: Joshua McKenty <joshua@cognition.ca>
-.. moduleauthor:: Manish Singh <yosh@gimp.org>
-.. moduleauthor:: Andy Smith <andy@anarkystic.com>
-"""

nova/endpoint/admin.py

@@ -37,6 +37,7 @@ def user_dict(user, base64_file=None):
     else:
         return {}
 
+
 def project_dict(project):
     """Convert the project object to a result dict"""
     if project:
@@ -47,6 +48,7 @@ def project_dict(project):
     else:
         return {}
 
+
 def host_dict(host):
     """Convert a host model object to a result dict"""
     if host:
@@ -54,6 +56,7 @@ def host_dict(host):
     else:
         return {}
 
+
 def admin_only(target):
     """Decorator for admin-only API calls"""
     def wrapper(*args, **kwargs):
@@ -66,6 +69,7 @@ def admin_only(target):
 
     return wrapper
 
+
 class AdminController(object):
     """
     API Controller for users, hosts, nodes, and workers.
@@ -102,6 +106,21 @@ class AdminController(object):
 
         return True
 
+    @admin_only
+    def describe_roles(self, context, project_roles=True, **kwargs):
+        """Returns a list of allowed roles."""
+        roles = manager.AuthManager().get_roles(project_roles)
+        return { 'roles': [{'role': r} for r in roles]}
+
+    @admin_only
+    def describe_user_roles(self, context, user, project=None, **kwargs):
+        """Returns a list of roles for the given user.
+           Omitting project will return any global roles that the user has.
+           Specifying project will return only project specific roles.
+        """
+        roles = manager.AuthManager().get_user_roles(user, project=project)
+        return { 'roles': [{'role': r} for r in roles]}
+
     @admin_only
     def modify_user_role(self, context, user, role, project=None,
                          operation='add', **kwargs):

nova/endpoint/api.py

@@ -25,12 +25,13 @@ import logging
 import multiprocessing
 import random
 import re
-import tornado.web
-from twisted.internet import defer
 import urllib
 # TODO(termie): replace minidom with etree
 from xml.dom import minidom
 
+import tornado.web
+from twisted.internet import defer
+
 from nova import crypto
 from nova import exception
 from nova import flags
@@ -43,6 +44,7 @@ from nova.endpoint import cloud
 FLAGS = flags.FLAGS
 flags.DEFINE_integer('cc_port', 8773, 'cloud controller port')
 
+
 _log = logging.getLogger("api")
 _log.setLevel(logging.DEBUG)
 
@@ -227,6 +229,7 @@ class MetadataRequestHandler(tornado.web.RequestHandler):
         self.print_data(data)
         self.finish()
 
+
 class APIRequestHandler(tornado.web.RequestHandler):
     def get(self, controller_name):
         self.execute(controller_name)

nova/endpoint/cloud.py

@@ -26,6 +26,7 @@ import base64
 import logging
 import os
 import time
+
 from twisted.internet import defer
 
 from nova import datastore
@@ -45,7 +46,6 @@ from nova.volume import service
 
 FLAGS = flags.FLAGS
 
-flags.DEFINE_string('cloud_topic', 'cloud', 'the topic clouds listen on')
 
 def _gen_key(user_id, key_name):
     """ Tuck this into AuthManager """
@@ -85,7 +85,7 @@ class CloudController(object):
         """ Ensure the keychains and folders exist. """
         # Create keys folder, if it doesn't exist
         if not os.path.exists(FLAGS.keys_path):
-            os.makedirs(os.path.abspath(FLAGS.keys_path))
+            os.makedirs(FLAGS.keys_path)
         # Gen root CA, if we don't have one
         root_ca_path = os.path.join(FLAGS.ca_path, FLAGS.ca_file)
         if not os.path.exists(root_ca_path):
@@ -102,15 +102,16 @@ class CloudController(object):
         result = {}
         for instance in self.instdir.all:
             if instance['project_id'] == project_id:
-                line = '%s slots=%d' % (instance['private_dns_name'], INSTANCE_TYPES[instance['instance_type']]['vcpus'])
+                line = '%s slots=%d' % (instance['private_dns_name'],
+                    INSTANCE_TYPES[instance['instance_type']]['vcpus'])
                 if instance['key_name'] in result:
                     result[instance['key_name']].append(line)
                 else:
                     result[instance['key_name']] = [line]
         return result
 
-    def get_metadata(self, ip):
+    def get_metadata(self, ipaddress):
-        i = self.get_instance_by_ip(ip)
+        i = self.get_instance_by_ip(ipaddress)
         if i is None:
             return None
         mpi = self._get_mpi_data(i['project_id'])
@@ -123,6 +124,12 @@ class CloudController(object):
             }
         else:
             keys = ''
+
+        address_record = network_model.FixedIp(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': {
@@ -135,19 +142,19 @@ class CloudController(object):
                     'root': '/dev/sda1',
                     'swap': 'sda3'
                 },
-                'hostname': i['private_dns_name'], # is this public sometimes?
+                'hostname': hostname,
                 'instance-action': 'none',
                 'instance-id': i['instance_id'],
                 'instance-type': i.get('instance_type', ''),
-                'local-hostname': i['private_dns_name'],
+                '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.get('dns_name', ''),
+                'public-hostname': hostname,
                 'public-ipv4': i.get('dns_name', ''), # TODO: switch to IP
-                'public-keys' : keys,
+                'public-keys': keys,
                 'ramdisk-id': i.get('ramdisk_id', ''),
                 'reservation-id': i['reservation_id'],
                 'security-groups': i.get('groups', ''),
@@ -203,26 +210,22 @@ class CloudController(object):
                     'keyFingerprint': key_pair.fingerprint,
                 })
 
-        return { 'keypairsSet': result }
+        return {'keypairsSet': result}
 
     @rbac.allow('all')
     def create_key_pair(self, context, key_name, **kwargs):
-        try:
+        dcall = defer.Deferred()
-            d = defer.Deferred()
+        pool = context.handler.application.settings.get('pool')
-            p = context.handler.application.settings.get('pool')
+        def _complete(kwargs):
-            def _complete(kwargs):
+            if 'exception' in kwargs:
-                if 'exception' in kwargs:
+                dcall.errback(kwargs['exception'])
-                    d.errback(kwargs['exception'])
+                return
-                    return
+            dcall.callback({'keyName': key_name,
-                d.callback({'keyName': key_name,
+                'keyFingerprint': kwargs['fingerprint'],
-                    'keyFingerprint': kwargs['fingerprint'],
+                'keyMaterial': kwargs['private_key']})
-                    'keyMaterial': kwargs['private_key']})
+        pool.apply_async(_gen_key, [context.user.id, key_name],
-            p.apply_async(_gen_key, [context.user.id, key_name],
+            callback=_complete)
-                callback=_complete)
+        return dcall
-            return d
-
-        except manager.UserError as e:
-            raise
 
     @rbac.allow('all')
     def delete_key_pair(self, context, key_name, **kwargs):
@@ -232,7 +235,7 @@ class CloudController(object):
 
     @rbac.allow('all')
     def describe_security_groups(self, context, group_names, **kwargs):
-        groups = { 'securityGroupSet': [] }
+        groups = {'securityGroupSet': []}
 
         # Stubbed for now to unblock other things.
         return groups
@@ -251,7 +254,7 @@ class CloudController(object):
         instance = self._get_instance(context, instance_id[0])
         return rpc.call('%s.%s' % (FLAGS.compute_topic, instance['node_name']),
             {"method": "get_console_output",
-             "args" : {"instance_id": instance_id[0]}})
+             "args": {"instance_id": instance_id[0]}})
 
     def _get_user_id(self, context):
         if context and context.user:
@@ -285,10 +288,10 @@ class CloudController(object):
         if volume['attach_status'] == 'attached':
             v['attachmentSet'] = [{'attachTime': volume['attach_time'],
                                    'deleteOnTermination': volume['delete_on_termination'],
-                                   'device' : volume['mountpoint'],
+                                   'device': volume['mountpoint'],
-                                   'instanceId' : volume['instance_id'],
+                                   'instanceId': volume['instance_id'],
-                                   'status' : 'attached',
+                                   'status': 'attached',
-                                   'volume_id' : volume['volume_id']}]
+                                   'volume_id': volume['volume_id']}]
         else:
             v['attachmentSet'] = [{}]
         return v
@@ -298,16 +301,16 @@ class CloudController(object):
     def create_volume(self, context, size, **kwargs):
         # TODO(vish): refactor this to create the volume object here and tell service to create it
         result = yield rpc.call(FLAGS.volume_topic, {"method": "create_volume",
-                                 "args" : {"size": size,
+                                 "args": {"size": size,
                                            "user_id": context.user.id,
                                            "project_id": context.project.id}})
         # NOTE(vish): rpc returned value is in the result key in the dictionary
-        volume = self._get_volume(context, result['result'])
+        volume = self._get_volume(context, result)
         defer.returnValue({'volumeSet': [self.format_volume(context, volume)]})
 
     def _get_address(self, context, public_ip):
         # FIXME(vish) this should move into network.py
-        address = network_model.PublicAddress.lookup(public_ip)
+        address = network_model.ElasticIp.lookup(public_ip)
         if address and (context.user.is_admin() or address['project_id'] == context.project.id):
             return address
         raise exception.NotFound("Address at ip %s not found" % public_ip)
@@ -348,16 +351,15 @@ class CloudController(object):
         compute_node = instance['node_name']
         rpc.cast('%s.%s' % (FLAGS.compute_topic, compute_node),
                                 {"method": "attach_volume",
-                                 "args" : {"volume_id": volume_id,
+                                 "args": {"volume_id": volume_id,
-                                           "instance_id" : instance_id,
+                                           "instance_id": instance_id,
-                                           "mountpoint" : device}})
+                                           "mountpoint": device}})
-        return defer.succeed({'attachTime' : volume['attach_time'],
+        return defer.succeed({'attachTime': volume['attach_time'],
-                              'device' : volume['mountpoint'],
+                              'device': volume['mountpoint'],
-                              'instanceId' : instance_id,
+                              'instanceId': instance_id,
-                              'requestId' : context.request_id,
+                              'requestId': context.request_id,
-                              'status' : volume['attach_status'],
+                              'status': volume['attach_status'],
-                              'volumeId' : volume_id})
+                              'volumeId': volume_id})
-
 
     @rbac.allow('projectmanager', 'sysadmin')
     def detach_volume(self, context, volume_id, **kwargs):
@@ -372,18 +374,18 @@ class CloudController(object):
             instance = self._get_instance(context, instance_id)
             rpc.cast('%s.%s' % (FLAGS.compute_topic, instance['node_name']),
                                 {"method": "detach_volume",
-                                 "args" : {"instance_id": instance_id,
+                                 "args": {"instance_id": instance_id,
                                            "volume_id": volume_id}})
         except exception.NotFound:
             # If the instance doesn't exist anymore,
             # then we need to call detach blind
             volume.finish_detach()
-        return defer.succeed({'attachTime' : volume['attach_time'],
+        return defer.succeed({'attachTime': volume['attach_time'],
-                              'device' : volume['mountpoint'],
+                              'device': volume['mountpoint'],
-                              'instanceId' : instance_id,
+                              'instanceId': instance_id,
-                              'requestId' : context.request_id,
+                              'requestId': context.request_id,
-                              'status' : volume['attach_status'],
+                              'status': volume['attach_status'],
-                              'volumeId' : volume_id})
+                              'volumeId': volume_id})
 
     def _convert_to_set(self, lst, label):
         if lst == None or lst == []:
@@ -394,7 +396,15 @@ class CloudController(object):
 
     @rbac.allow('all')
     def describe_instances(self, context, **kwargs):
-        return defer.succeed(self._format_instances(context))
+        return defer.succeed(self._format_describe_instances(context))
+
+    def _format_describe_instances(self, context):
+        return { 'reservationSet': self._format_instances(context) }
+
+    def _format_run_instances(self, context, reservation_id):
+        i = self._format_instances(context, reservation_id)
+        assert len(i) == 1
+        return i[0]
 
     def _format_instances(self, context, reservation_id = None):
         reservations = {}
@@ -425,7 +435,8 @@ class CloudController(object):
             i['key_name'] = instance.get('key_name', None)
             if context.user.is_admin():
                 i['key_name'] = '%s (%s, %s)' % (i['key_name'],
-                    instance.get('project_id', None), instance.get('node_name',''))
+                    instance.get('project_id', None),
+                    instance.get('node_name', ''))
             i['product_codes_set'] = self._convert_to_set(
                 instance.get('product_codes', None), 'product_code')
             i['instance_type'] = instance.get('instance_type', None)
@@ -442,8 +453,7 @@ class CloudController(object):
                 reservations[res_id] = r
             reservations[res_id]['instances_set'].append(i)
 
-        instance_response = {'reservationSet' : list(reservations.values()) }
+        return list(reservations.values())
-        return instance_response
 
     @rbac.allow('all')
     def describe_addresses(self, context, **kwargs):
@@ -451,13 +461,13 @@ class CloudController(object):
 
     def format_addresses(self, context):
         addresses = []
-        for address in network_model.PublicAddress.all():
+        for address in network_model.ElasticIp.all():
             # TODO(vish): implement a by_project iterator for addresses
             if (context.user.is_admin() or
                 address['project_id'] == context.project.id):
                 address_rv = {
                     'public_ip': address['address'],
-                    'instance_id' : address.get('instance_id', 'free')
+                    'instance_id': address.get('instance_id', 'free')
                 }
                 if context.user.is_admin():
                     address_rv['instance_id'] = "%s (%s, %s)" % (
@@ -472,12 +482,11 @@ class CloudController(object):
     @defer.inlineCallbacks
     def allocate_address(self, context, **kwargs):
         network_topic = yield self._get_network_topic(context)
-        alloc_result = yield rpc.call(network_topic,
+        public_ip = yield rpc.call(network_topic,
                          {"method": "allocate_elastic_ip",
                           "args": {"user_id": context.user.id,
                                    "project_id": context.project.id}})
-        public_ip = alloc_result['result']
+        defer.returnValue({'addressSet': [{'publicIp': public_ip}]})
-        defer.returnValue({'addressSet': [{'publicIp' : public_ip}]})
 
     @rbac.allow('netadmin')
     @defer.inlineCallbacks
@@ -517,11 +526,10 @@ class CloudController(object):
         """Retrieves the network host for a project"""
         host = network_service.get_host_for_project(context.project.id)
         if not host:
-            result = yield rpc.call(FLAGS.network_topic,
+            host = yield rpc.call(FLAGS.network_topic,
                                     {"method": "set_network_host",
                                      "args": {"user_id": context.user.id,
                                               "project_id": context.project.id}})
-            host = result['result']
         defer.returnValue('%s.%s' %(FLAGS.network_topic, host))
 
     @rbac.allow('projectmanager', 'sysadmin')
@@ -561,17 +569,17 @@ class CloudController(object):
         # TODO: Get the real security group of launch in here
         security_group = "default"
         for num in range(int(kwargs['max_count'])):
-            vpn = False
+            is_vpn = False
             if image_id  == FLAGS.vpn_image_id:
-                vpn = True
+                is_vpn = True
-            allocate_result = yield rpc.call(network_topic,
+            inst = self.instdir.new()
+            allocate_data = yield rpc.call(network_topic,
                      {"method": "allocate_fixed_ip",
                       "args": {"user_id": context.user.id,
                                "project_id": context.project.id,
                                "security_group": security_group,
-                               "vpn": vpn}})
+                               "is_vpn": is_vpn,
-            allocate_data = allocate_result['result']
+                               "hostname": inst.instance_id}})
-            inst = self.instdir.new()
             inst['image_id'] = image_id
             inst['kernel_id'] = kernel_id
             inst['ramdisk_id'] = ramdisk_id
@@ -585,17 +593,18 @@ 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
 
             inst.save()
             rpc.cast(FLAGS.compute_topic,
                  {"method": "run_instance",
-                  "args": {"instance_id" : inst.instance_id}})
+                  "args": {"instance_id": inst.instance_id}})
             logging.debug("Casting to node for %s's instance with IP of %s" %
                       (context.user.name, inst['private_dns_name']))
         # TODO: Make Network figure out the network name from ip.
-        defer.returnValue(self._format_instances(context, reservation_id))
+        defer.returnValue(self._format_run_instances(context, reservation_id))
 
     @rbac.allow('projectmanager', 'sysadmin')
     @defer.inlineCallbacks
@@ -646,7 +655,7 @@ class CloudController(object):
             instance = self._get_instance(context, i)
             rpc.cast('%s.%s' % (FLAGS.compute_topic, instance['node_name']),
                              {"method": "reboot_instance",
-                              "args" : {"instance_id": i}})
+                              "args": {"instance_id": i}})
         return defer.succeed(True)
 
     @rbac.allow('projectmanager', 'sysadmin')
@@ -656,7 +665,7 @@ class CloudController(object):
         volume_node = volume['node_name']
         rpc.cast('%s.%s' % (FLAGS.volume_topic, volume_node),
                             {"method": "delete_volume",
-                             "args" : {"volume_id": volume_id}})
+                             "args": {"volume_id": volume_id}})
         return defer.succeed(True)
 
     @rbac.allow('all')
@@ -689,9 +698,9 @@ class CloudController(object):
             image = images.list(context, image_id)[0]
         except IndexError:
             raise exception.ApiError('invalid id: %s' % image_id)
-        result = { 'image_id': image_id, 'launchPermission': [] }
+        result = {'image_id': image_id, 'launchPermission': []}
         if image['isPublic']:
-            result['launchPermission'].append({ 'group': 'all' })
+            result['launchPermission'].append({'group': 'all'})
         return defer.succeed(result)
 
     @rbac.allow('projectmanager', 'sysadmin')

nova/endpoint/images.py

@@ -21,10 +21,11 @@ Proxy AMI-related calls from the cloud controller, to the running
 objectstore daemon.
 """
 
-import boto.s3.connection
 import json
 import urllib
 
+import boto.s3.connection
+
 from nova import flags
 from nova import utils
 from nova.auth import manager
@@ -32,6 +33,7 @@ from nova.auth import manager
 
 FLAGS = flags.FLAGS
 
+
 def modify(context, image_id, operation):
     conn(context).make_request(
         method='POST',
@@ -53,6 +55,7 @@ def register(context, image_location):
 
     return image_id
 
+
 def list(context, filter_list=[]):
     """ return a list of all images that a user can see
 
@@ -68,6 +71,7 @@ def list(context, filter_list=[]):
         return [i for i in result if i['imageId'] in filter_list]
     return result
 
+
 def deregister(context, image_id):
     """ unregister an image """
     conn(context).make_request(
@@ -75,6 +79,7 @@ def deregister(context, image_id):
             bucket='_images',
             query_args=qs({'image_id': image_id}))
 
+
 def conn(context):
     access = manager.AuthManager().get_access_key(context.user,
                                                   context.project)

nova/endpoint/rackspace.py

@@ -1,226 +0,0 @@
-# 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.
-#
-#    Licensed under the Apache License, Version 2.0 (the "License"); you may
-#    not use this file except in compliance with the License. You may obtain
-#    a copy of the License at
-#
-#         http://www.apache.org/licenses/LICENSE-2.0
-#
-#    Unless required by applicable law or agreed to in writing, software
-#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
-#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
-#    License for the specific language governing permissions and limitations
-#    under the License.
-
-"""
-Rackspace API
-"""
-
-import base64
-import json
-import logging
-import multiprocessing
-import os
-import time
-import tornado.web
-from twisted.internet import defer
-
-from nova import datastore
-from nova import exception
-from nova import flags
-from nova import rpc
-from nova import utils
-from nova.auth import manager
-from nova.compute import model
-from nova.compute import network
-from nova.endpoint import images
-from nova.endpoint import wsgi
-
-
-FLAGS = flags.FLAGS
-flags.DEFINE_string('cloud_topic', 'cloud', 'the topic clouds listen on')
-
-
-# TODO(todd): subclass Exception so we can bubble meaningful errors
-
-
-class Api(object):
-
-    def __init__(self, rpc_mechanism):
-        self.controllers = {
-            "v1.0":   RackspaceAuthenticationApi(),
-            "servers": RackspaceCloudServerApi()
-        }
-        self.rpc_mechanism = rpc_mechanism
-
-    def handler(self, environ, responder):
-        environ['nova.context'] = self.build_context(environ)
-        controller, path = wsgi.Util.route(
-                             environ['PATH_INFO'],
-                             self.controllers
-                           )
-        if not controller:
-            # TODO(todd): Exception (404)
-            raise Exception("Missing Controller")
-        rv = controller.process(path, environ)
-        if type(rv) is tuple:
-            responder(rv[0], rv[1])
-            rv = rv[2]
-        else:
-            responder("200 OK", [])
-        return rv
-
-    def build_context(self, env):
-        rv = {}
-        if env.has_key("HTTP_X_AUTH_TOKEN"):
-            rv['user'] = manager.AuthManager().get_user_from_access_key(
-                           env['HTTP_X_AUTH_TOKEN']
-                         )
-            if rv['user']:
-                rv['project'] = manager.AuthManager().get_project(
-                                  rv['user'].name
-                                )
-        return rv
-
-
-class RackspaceApiEndpoint(object):
-    def process(self, path, env):
-        if not self.check_authentication(env):
-            # TODO(todd): Exception (Unauthorized)
-            raise Exception("Unable to authenticate")
-
-        if len(path) == 0:
-            return self.index(env)
-
-        action = path.pop(0)
-        if hasattr(self, action):
-            method = getattr(self, action)
-            return method(path, env)
-        else:
-            # TODO(todd): Exception (404)
-            raise Exception("Missing method %s" % path[0])
-
-    def check_authentication(self, env):
-        if hasattr(self, "process_without_authentication") \
-        and getattr(self, "process_without_authentication"):
-            return True
-        if not env['nova.context']['user']:
-            return False
-        return True
-
-
-class RackspaceAuthenticationApi(RackspaceApiEndpoint):
-
-    def __init__(self):
-        self.process_without_authentication = True
-
-    # TODO(todd): make a actual session with a unique token
-    # just pass the auth key back through for now
-    def index(self, env):
-        response = '204 No Content'
-        headers = [
-            ('X-Server-Management-Url', 'http://%s' % env['HTTP_HOST']),
-            ('X-Storage-Url', 'http://%s' % env['HTTP_HOST']),
-            ('X-CDN-Managment-Url', 'http://%s' % env['HTTP_HOST']),
-            ('X-Auth-Token', env['HTTP_X_AUTH_KEY'])
-        ]
-        body = ""
-        return (response, headers, body)
-
-
-class RackspaceCloudServerApi(RackspaceApiEndpoint):
-
-    def __init__(self):
-        self.instdir = model.InstanceDirectory()
-        self.network = network.PublicNetworkController()
-
-    def index(self, env):
-        if env['REQUEST_METHOD'] == 'GET':
-            return self.detail(env)
-        elif env['REQUEST_METHOD'] == 'POST':
-            return self.launch_server(env)
-
-    def detail(self, args, env):
-        value = {
-            "servers":
-                []
-        }
-        for inst in self.instdir.all:
-            value["servers"].append(self.instance_details(inst))
-
-        return json.dumps(value)
-
-    ##
-    ##
-
-    def launch_server(self, env):
-        data = json.loads(env['wsgi.input'].read(int(env['CONTENT_LENGTH'])))
-        inst = self.build_server_instance(data, env['nova.context'])
-        self.schedule_launch_of_instance(inst)
-        return json.dumps({"server": self.instance_details(inst)})
-
-    def instance_details(self, inst):
-        return {
-            "id": inst.get("instance_id", None),
-            "imageId": inst.get("image_id", None),
-            "flavorId": inst.get("instacne_type", None),
-            "hostId": inst.get("node_name", None),
-            "status": inst.get("state", "pending"),
-            "addresses": {
-                "public": [self.network.get_public_ip_for_instance(
-                            inst.get("instance_id", None)
-                          )],
-                "private": [inst.get("private_dns_name", None)]
-            },
-
-            # implemented only by Rackspace, not AWS
-            "name": inst.get("name", "Not-Specified"),
-
-            # not supported
-            "progress": "Not-Supported",
-            "metadata": {
-                "Server Label": "Not-Supported",
-                "Image Version": "Not-Supported"
-            }
-        }
-
-    def build_server_instance(self, env, context):
-        reservation = utils.generate_uid('r')
-        ltime = time.strftime('%Y-%m-%dT%H:%M:%SZ', time.gmtime())
-        inst = self.instdir.new()
-        inst['name'] = env['server']['name']
-        inst['image_id'] = env['server']['imageId']
-        inst['instance_type'] = env['server']['flavorId']
-        inst['user_id'] = context['user'].id
-        inst['project_id'] = context['project'].id
-        inst['reservation_id'] = reservation
-        inst['launch_time'] = ltime
-        inst['mac_address'] = utils.generate_mac()
-        address = network.allocate_ip(
-                    inst['user_id'],
-                    inst['project_id'],
-                    mac=inst['mac_address']
-                  )
-        inst['private_dns_name'] = str(address)
-        inst['bridge_name'] = network.BridgedNetwork.get_network_for_project(
-                                inst['user_id'],
-                                inst['project_id'],
-                                'default' # security group
-                              )['bridge_name']
-        # key_data, key_name, ami_launch_index
-        # TODO(todd): key data or root password
-        inst.save()
-        return inst
-
-    def schedule_launch_of_instance(self, inst):
-        rpc.cast(
-            FLAGS.compute_topic,
-            {
-                "method": "run_instance",
-                "args": {"instance_id": inst.instance_id}
-            }
-        )

nova/fakerabbit.py

@@ -16,12 +16,13 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-""" Based a bit on the carrot.backeds.queue backend... but a lot better """
+"""Based a bit on the carrot.backeds.queue backend... but a lot better."""
 
-from carrot.backends import base
 import logging
 import Queue as queue
 
+from carrot.backends import base
+
 
 class Message(base.BaseMessage):
     pass

nova/flags.py

@@ -21,16 +21,146 @@ Package-level global flags are defined here, the rest are defined
 where they're used.
 """
 
+import getopt
 import socket
+import sys
+
+import gflags
 
 
-from gflags import *
+class FlagValues(gflags.FlagValues):
+    """Extension of gflags.FlagValues that allows undefined and runtime flags.
+
+    Unknown flags will be ignored when parsing the command line, but the
+    command line will be kept so that it can be replayed if new flags are
+    defined after the initial parsing.
+    
+    """
+
+    def __init__(self):
+        gflags.FlagValues.__init__(self)
+        self.__dict__['__dirty'] = []
+        self.__dict__['__was_already_parsed'] = False
+        self.__dict__['__stored_argv'] = []
+
+    def __call__(self, argv):
+        # We're doing some hacky stuff here so that we don't have to copy
+        # out all the code of the original verbatim and then tweak a few lines.
+        # We're hijacking the output of getopt so we can still return the
+        # leftover args at the end
+        sneaky_unparsed_args = {"value": None}
+        original_argv = list(argv)
+        
+        if self.IsGnuGetOpt():
+            orig_getopt = getattr(getopt, 'gnu_getopt')
+            orig_name = 'gnu_getopt'
+        else:
+            orig_getopt = getattr(getopt, 'getopt')
+            orig_name = 'getopt'
+
+        def _sneaky(*args, **kw):
+            optlist, unparsed_args = orig_getopt(*args, **kw)
+            sneaky_unparsed_args['value'] = unparsed_args
+            return optlist, unparsed_args
+
+        try:
+            setattr(getopt, orig_name, _sneaky)
+            args = gflags.FlagValues.__call__(self, argv)
+        except gflags.UnrecognizedFlagError:
+            # Undefined args were found, for now we don't care so just
+            # act like everything went well
+            # (these three lines are copied pretty much verbatim from the end
+            # of the __call__ function we are wrapping)
+            unparsed_args = sneaky_unparsed_args['value']
+            if unparsed_args:
+                if self.IsGnuGetOpt():
+                    args = argv[:1] + unparsed
+                else:
+                    args = argv[:1] + original_argv[-len(unparsed_args):]
+            else:
+                args = argv[:1]
+        finally:
+            setattr(getopt, orig_name, orig_getopt)
+        
+        # Store the arguments for later, we'll need them for new flags
+        # added at runtime
+        self.__dict__['__stored_argv'] = original_argv
+        self.__dict__['__was_already_parsed'] = True
+        self.ClearDirty()
+        return args
+
+    def SetDirty(self, name):
+        """Mark a flag as dirty so that accessing it will case a reparse."""
+        self.__dict__['__dirty'].append(name)
+    
+    def IsDirty(self, name):
+        return name in self.__dict__['__dirty']
+
+    def ClearDirty(self):
+        self.__dict__['__is_dirty'] = []
+
+    def WasAlreadyParsed(self):
+        return self.__dict__['__was_already_parsed']
+
+    def ParseNewFlags(self):
+        if '__stored_argv' not in self.__dict__:
+            return
+        new_flags = FlagValues()
+        for k in self.__dict__['__dirty']:
+            new_flags[k] = gflags.FlagValues.__getitem__(self, k)
+
+        new_flags(self.__dict__['__stored_argv'])
+        for k in self.__dict__['__dirty']:
+            setattr(self, k, getattr(new_flags, k))
+        self.ClearDirty()
+        
+    def __setitem__(self, name, flag):
+        gflags.FlagValues.__setitem__(self, name, flag)
+        if self.WasAlreadyParsed():
+            self.SetDirty(name)
+    
+    def __getitem__(self, name):
+        if self.IsDirty(name):
+            self.ParseNewFlags()
+        return gflags.FlagValues.__getitem__(self, name)
+
+    def __getattr__(self, name):
+        if self.IsDirty(name):
+            self.ParseNewFlags()
+        return gflags.FlagValues.__getattr__(self, name)
+
+
+FLAGS = FlagValues()
+
+
+def _wrapper(func):
+    def _wrapped(*args, **kw):
+        kw.setdefault('flag_values', FLAGS)
+        func(*args, **kw)
+    _wrapped.func_name = func.func_name
+    return _wrapped
+
+
+DEFINE = _wrapper(gflags.DEFINE)
+DEFINE_string = _wrapper(gflags.DEFINE_string)
+DEFINE_integer = _wrapper(gflags.DEFINE_integer)
+DEFINE_bool = _wrapper(gflags.DEFINE_bool)
+DEFINE_boolean = _wrapper(gflags.DEFINE_boolean)
+DEFINE_float = _wrapper(gflags.DEFINE_float)
+DEFINE_enum = _wrapper(gflags.DEFINE_enum)
+DEFINE_list = _wrapper(gflags.DEFINE_list)
+DEFINE_spaceseplist = _wrapper(gflags.DEFINE_spaceseplist)
+DEFINE_multistring = _wrapper(gflags.DEFINE_multistring)
+DEFINE_multi_int = _wrapper(gflags.DEFINE_multi_int)
+
+
+def DECLARE(name, module_string, flag_values=FLAGS):
+    if module_string not in sys.modules:
+        __import__(module_string, globals(), locals())
+    if name not in flag_values:
+        raise gflags.UnrecognizedFlag(
+                "%s not defined by %s" % (name, module_string))
 
-# This keeps pylint from barfing on the imports
-FLAGS = FLAGS
-DEFINE_string = DEFINE_string
-DEFINE_integer = DEFINE_integer
-DEFINE_bool = DEFINE_bool
 
 # __GLOBAL FLAGS ONLY__
 # Define any app-specific flags in their own files, docs at:
@@ -39,36 +169,31 @@ DEFINE_bool = DEFINE_bool
 DEFINE_string('connection_type', 'libvirt', 'libvirt, xenapi or fake')
 DEFINE_integer('s3_port', 3333, 's3 port')
 DEFINE_string('s3_host', '127.0.0.1', 's3 host')
-#DEFINE_string('cloud_topic', 'cloud', 'the topic clouds listen on')
 DEFINE_string('compute_topic', 'compute', 'the topic compute nodes listen on')
 DEFINE_string('volume_topic', 'volume', 'the topic volume nodes listen on')
 DEFINE_string('network_topic', 'network', 'the topic network nodes listen on')
 
 DEFINE_bool('verbose', False, 'show debug output')
 DEFINE_boolean('fake_rabbit', False, 'use a fake rabbit')
-DEFINE_bool('fake_network', False, 'should we use fake network devices and addresses')
+DEFINE_bool('fake_network', False,
+            'should we use fake network devices and addresses')
 DEFINE_string('rabbit_host', 'localhost', 'rabbit host')
 DEFINE_integer('rabbit_port', 5672, 'rabbit port')
 DEFINE_string('rabbit_userid', 'guest', 'rabbit userid')
 DEFINE_string('rabbit_password', 'guest', 'rabbit password')
 DEFINE_string('rabbit_virtual_host', '/', 'rabbit virtual host')
 DEFINE_string('control_exchange', 'nova', 'the main exchange to connect to')
-DEFINE_string('ec2_url',
+DEFINE_string('ec2_url', 'http://127.0.0.1:8773/services/Cloud',
-                'http://127.0.0.1:8773/services/Cloud',
+              'Url to ec2 api server')
-                'Url to ec2 api server')
 
-DEFINE_string('default_image',
+DEFINE_string('default_image', 'ami-11111',
-                    'ami-11111',
+              'default image to use, testing only')
-                    'default image to use, testing only')
+DEFINE_string('default_kernel', 'aki-11111',
-DEFINE_string('default_kernel',
+              'default kernel to use, testing only')
-                    'aki-11111',
+DEFINE_string('default_ramdisk', 'ari-11111',
-                    'default kernel to use, testing only')
+              'default ramdisk to use, testing only')
-DEFINE_string('default_ramdisk',
+DEFINE_string('default_instance_type', 'm1.small',
-                    'ari-11111',
+              'default instance type to use, testing only')
-                    'default ramdisk to use, testing only')
-DEFINE_string('default_instance_type',
-                    'm1.small',
-                    'default instance type to use, testing only')
 
 DEFINE_string('vpn_image_id', 'ami-CLOUDPIPE', 'AMI for cloudpipe vpn server')
 DEFINE_string('vpn_key_suffix',
@@ -78,10 +203,8 @@ DEFINE_string('vpn_key_suffix',
 DEFINE_integer('auth_token_ttl', 3600, 'Seconds for auth tokens to linger')
 
 # UNUSED
-DEFINE_string('node_availability_zone',
+DEFINE_string('node_availability_zone', 'nova',
-                    'nova',
+              'availability zone of this node')
-                    'availability zone of this node')
+DEFINE_string('node_name', socket.gethostname(),
-DEFINE_string('node_name',
+              'name of this node')
-                    socket.gethostname(),
-                    'name of this node')
 

nova/process.py

@@ -2,6 +2,7 @@
 
 # Copyright 2010 United States Government as represented by the
 # Administrator of the National Aeronautics and Space Administration.
+# Copyright 2010 FathomDB Inc.
 # All Rights Reserved.
 #
 #    Licensed under the Apache License, Version 2.0 (the "License"); you may
@@ -20,16 +21,12 @@
 Process pool, still buggy right now.
 """
 
-import logging
-import multiprocessing
 import StringIO
+
 from twisted.internet import defer
 from twisted.internet import error
-from twisted.internet import process
 from twisted.internet import protocol
 from twisted.internet import reactor
-from twisted.internet import threads
-from twisted.python import failure
 
 from nova import flags
 
@@ -54,111 +51,100 @@ class UnexpectedErrorOutput(IOError):
         IOError.__init__(self, "got stdout: %r\nstderr: %r" % (stdout, stderr))
 
 
-# NOTE(termie): this too
+# This is based on _BackRelay from twister.internal.utils, but modified to 
-class _BackRelay(protocol.ProcessProtocol):
+#  capture both stdout and stderr, without odd stderr handling, and also to 
+#  handle stdin
+class BackRelayWithInput(protocol.ProcessProtocol):
     """
     Trivial protocol for communicating with a process and turning its output
     into the result of a L{Deferred}.
 
     @ivar deferred: A L{Deferred} which will be called back with all of stdout
-        and, if C{errortoo} is true, all of stderr as well (mixed together in
+        and all of stderr as well (as a tuple).  C{terminate_on_stderr} is true
-        one string).  If C{errortoo} is false and any bytes are received over
+        and any bytes are received over stderr, this will fire with an
-        stderr, this will fire with an L{_UnexpectedErrorOutput} instance and
+        L{_UnexpectedErrorOutput} instance and the attribute will be set to 
-        the attribute will be set to C{None}.
+        C{None}.
 
-    @ivar onProcessEnded: If C{errortoo} is false and bytes are received over
+    @ivar onProcessEnded: If C{terminate_on_stderr} is false and bytes are 
-        stderr, this attribute will refer to a L{Deferred} which will be called
+        received over stderr, this attribute will refer to a L{Deferred} which 
-        back when the process ends.  This C{Deferred} is also associated with
+        will be called back when the process ends.  This C{Deferred} is also 
-        the L{_UnexpectedErrorOutput} which C{deferred} fires with earlier in
+        associated with the L{_UnexpectedErrorOutput} which C{deferred} fires 
-        this case so that users can determine when the process has actually
+        with earlier in this case so that users can determine when the process 
-        ended, in addition to knowing when bytes have been received via stderr.
+        has actually ended, in addition to knowing when bytes have been received
+        via stderr.
     """
 
-    def __init__(self, deferred, errortoo=0):
+    def __init__(self, deferred, started_deferred=None, 
+                    terminate_on_stderr=False, check_exit_code=True, 
+                    process_input=None):
         self.deferred = deferred
-        self.s = StringIO.StringIO()
+        self.stdout = StringIO.StringIO()
-        if errortoo:
+        self.stderr = StringIO.StringIO()
-            self.errReceived = self.errReceivedIsGood
+        self.started_deferred = started_deferred
-        else:
+        self.terminate_on_stderr = terminate_on_stderr
-            self.errReceived = self.errReceivedIsBad
+        self.check_exit_code = check_exit_code
-
+        self.process_input = process_input
-    def errReceivedIsBad(self, text):
+        self.on_process_ended = None
-        if self.deferred is not None:
+    
-            self.onProcessEnded = defer.Deferred()
+    def errReceived(self, text):
-            err = UnexpectedErrorOutput(text, self.onProcessEnded)
+        self.stderr.write(text)
-            self.deferred.errback(failure.Failure(err))
+        if self.terminate_on_stderr and (self.deferred is not None):
+            self.on_process_ended = defer.Deferred()
+            self.deferred.errback(UnexpectedErrorOutput(
+                                        stdout=self.stdout.getvalue(),
+                                        stderr=self.stderr.getvalue()))
             self.deferred = None
             self.transport.loseConnection()
 
-    def errReceivedIsGood(self, text):
-        self.s.write(text)
-
     def outReceived(self, text):
-        self.s.write(text)
+        self.stdout.write(text)
 
     def processEnded(self, reason):
         if self.deferred is not None:
-            self.deferred.callback(self.s.getvalue())
+            stdout, stderr = self.stdout.getvalue(), self.stderr.getvalue()
-        elif self.onProcessEnded is not None:
-            self.onProcessEnded.errback(reason)
-
-
-class BackRelayWithInput(_BackRelay):
-    def __init__(self, deferred, startedDeferred=None, error_ok=0,
-                 input=None):
-        # Twisted doesn't use new-style classes in most places :(
-        _BackRelay.__init__(self, deferred, errortoo=error_ok)
-        self.error_ok = error_ok
-        self.input = input
-        self.stderr = StringIO.StringIO()
-        self.startedDeferred = startedDeferred
-
-    def errReceivedIsBad(self, text):
-        self.stderr.write(text)
-        self.transport.loseConnection()
-
-    def errReceivedIsGood(self, text):
-        self.stderr.write(text)
-
-    def connectionMade(self):
-        if self.startedDeferred:
-            self.startedDeferred.callback(self)
-        if self.input:
-            self.transport.write(self.input)
-        self.transport.closeStdin()
-
-    def processEnded(self, reason):
-        if self.deferred is not None:
-            stdout, stderr = self.s.getvalue(), self.stderr.getvalue()
             try:
-                # NOTE(termie): current behavior means if error_ok is True
+                if self.check_exit_code:
-                #               we won't throw an error even if the process
-                #               exited with a non-0 status, so you can't be
-                #               okay with stderr output and not with bad exit
-                #               codes.
-                if not self.error_ok:
                     reason.trap(error.ProcessDone)
                 self.deferred.callback((stdout, stderr))
             except:
+                # NOTE(justinsb): This logic is a little suspicious to me...
+                # If the callback throws an exception, then errback will be 
+                # called also. However, this is what the unit tests test for...
                 self.deferred.errback(UnexpectedErrorOutput(stdout, stderr))
+        elif self.on_process_ended is not None:
+            self.on_process_ended.errback(reason)
 
 
-def getProcessOutput(executable, args=None, env=None, path=None, reactor=None,
+    def connectionMade(self):
-                     error_ok=0, input=None, startedDeferred=None):
+        if self.started_deferred:
-    if reactor is None:
+            self.started_deferred.callback(self)
-        from twisted.internet import reactor
+        if self.process_input:
+            self.transport.write(self.process_input)
+        self.transport.closeStdin()
+
+def get_process_output(executable, args=None, env=None, path=None, 
+                       process_reactor=None, check_exit_code=True, 
+                       process_input=None, started_deferred=None,
+                       terminate_on_stderr=False):
+    if process_reactor is None:
+        process_reactor = reactor
     args = args and args or ()
     env = env and env and {}
-    d = defer.Deferred()
+    deferred = defer.Deferred()
-    p = BackRelayWithInput(
+    process_handler = BackRelayWithInput(
-            d, startedDeferred=startedDeferred, error_ok=error_ok, input=input)
+            deferred, 
+            started_deferred=started_deferred, 
+            check_exit_code=check_exit_code, 
+            process_input=process_input,
+            terminate_on_stderr=terminate_on_stderr)
     # NOTE(vish): commands come in as unicode, but self.executes needs
     #             strings or process.spawn raises a deprecation warning
     executable = str(executable)
     if not args is None:
         args = [str(x) for x in args]
-    reactor.spawnProcess(p, executable, (executable,)+tuple(args), env, path)
+    process_reactor.spawnProcess(   process_handler, executable, 
-    return d
+                                    (executable,)+tuple(args), env, path)
+    return deferred
 
 
 class ProcessPool(object):
@@ -184,26 +170,27 @@ class ProcessPool(object):
         return self.execute(executable, args, **kw)
 
     def execute(self, *args, **kw):
-        d = self._pool.acquire()
+        deferred = self._pool.acquire()
 
-        def _associateProcess(proto):
+        def _associate_process(proto):
-            d.process = proto.transport
+            deferred.process = proto.transport
             return proto.transport
 
         started = defer.Deferred()
-        started.addCallback(_associateProcess)
+        started.addCallback(_associate_process)
-        kw.setdefault('startedDeferred', started)
+        kw.setdefault('started_deferred', started)
 
-        d.process = None
+        deferred.process = None
-        d.started = started
+        deferred.started = started
 
-        d.addCallback(lambda _: getProcessOutput(*args, **kw))
+        deferred.addCallback(lambda _: get_process_output(*args, **kw))
-        d.addBoth(self._release)
+        deferred.addBoth(self._release)
-        return d
+        return deferred
 
-    def _release(self, rv=None):
+    def _release(self, retval=None):
         self._pool.release()
-        return rv
+        return retval
+
 
 class SharedPool(object):
     _instance = None
@@ -213,5 +200,6 @@ class SharedPool(object):
     def __getattr__(self, key):
         return getattr(self._instance, key)
 
+
 def simple_execute(cmd, **kwargs):
     return SharedPool().simple_execute(cmd, **kwargs)

nova/rpc.py

@@ -21,14 +21,14 @@ AMQP-based RPC. Queues have consumers and publishers.
 No fan-out support yet.
 """
 
-from carrot import connection
-from carrot import messaging
 import json
 import logging
 import sys
 import uuid
+
+from carrot import connection as carrot_connection
+from carrot import messaging
 from twisted.internet import defer
-from twisted.internet import reactor
 from twisted.internet import task
 
 from nova import exception
@@ -39,13 +39,15 @@ from nova import flags
 FLAGS = flags.FLAGS
 
 
-_log = logging.getLogger('amqplib')
+LOG = logging.getLogger('amqplib')
-_log.setLevel(logging.WARN)
+LOG.setLevel(logging.DEBUG)
 
 
-class Connection(connection.BrokerConnection):
+class Connection(carrot_connection.BrokerConnection):
+    """Connection instance object"""
     @classmethod
     def instance(cls):
+        """Returns the instance"""
         if not hasattr(cls, '_instance'):
             params = dict(hostname=FLAGS.rabbit_host,
                           port=FLAGS.rabbit_port,
@@ -56,18 +58,33 @@ class Connection(connection.BrokerConnection):
             if FLAGS.fake_rabbit:
                 params['backend_cls'] = fakerabbit.Backend
 
+            # NOTE(vish): magic is fun!
+            # pylint: disable-msg=W0142
             cls._instance = cls(**params)
         return cls._instance
 
     @classmethod
     def recreate(cls):
+        """Recreates the connection instance
+
+        This is necessary to recover from some network errors/disconnects"""
         del cls._instance
         return cls.instance()
 
+
 class Consumer(messaging.Consumer):
+    """Consumer base class
+
+    Contains methods for connecting the fetch method to async loops
+    """
+    def __init__(self, *args, **kwargs):
+        self.failed_connection = False
+        super(Consumer, self).__init__(*args, **kwargs)
+
     # TODO(termie): it would be nice to give these some way of automatically
     #               cleaning up after themselves
     def attach_to_tornado(self, io_inst=None):
+        """Attach a callback to tornado that fires 10 times a second"""
         from tornado import ioloop
         if io_inst is None:
             io_inst = ioloop.IOLoop.instance()
@@ -79,33 +96,44 @@ class Consumer(messaging.Consumer):
 
     attachToTornado = attach_to_tornado
 
-    def fetch(self, *args, **kwargs):
+    def fetch(self, no_ack=None, auto_ack=None, enable_callbacks=False):
+        """Wraps the parent fetch with some logic for failed connections"""
         # TODO(vish): the logic for failed connections and logging should be
         #             refactored into some sort of connection manager object
         try:
-            if getattr(self, 'failed_connection', False):
+            if self.failed_connection:
-                # attempt to reconnect
+                # NOTE(vish): conn is defined in the parent class, we can
+                #             recreate it as long as we create the backend too
+                # pylint: disable-msg=W0201
                 self.conn = Connection.recreate()
                 self.backend = self.conn.create_backend()
-            super(Consumer, self).fetch(*args, **kwargs)
+            super(Consumer, self).fetch(no_ack, auto_ack, enable_callbacks)
-            if getattr(self, 'failed_connection', False):
+            if self.failed_connection:
                 logging.error("Reconnected to queue")
                 self.failed_connection = False
-        except Exception, ex:
+        # NOTE(vish): This is catching all errors because we really don't
-            if not getattr(self, 'failed_connection', False):
+        #             exceptions to be logged 10 times a second if some
+        #             persistent failure occurs.
+        except Exception:  # pylint: disable-msg=W0703
+            if not self.failed_connection:
                 logging.exception("Failed to fetch message from queue")
                 self.failed_connection = True
 
     def attach_to_twisted(self):
+        """Attach a callback to twisted that fires 10 times a second"""
         loop = task.LoopingCall(self.fetch, enable_callbacks=True)
         loop.start(interval=0.1)
 
+
 class Publisher(messaging.Publisher):
+    """Publisher base class"""
     pass
 
 
 class TopicConsumer(Consumer):
+    """Consumes messages on a specific topic"""
     exchange_type = "topic"
+
     def __init__(self, connection=None, topic="broadcast"):
         self.queue = topic
         self.routing_key = topic
@@ -115,14 +143,24 @@ class TopicConsumer(Consumer):
 
 
 class AdapterConsumer(TopicConsumer):
+    """Calls methods on a proxy object based on method and args"""
     def __init__(self, connection=None, topic="broadcast", proxy=None):
-        _log.debug('Initing the Adapter Consumer for %s' % (topic))
+        LOG.debug('Initing the Adapter Consumer for %s' % (topic))
         self.proxy = proxy
-        super(AdapterConsumer, self).__init__(connection=connection, topic=topic)
+        super(AdapterConsumer, self).__init__(connection=connection,
+                                              topic=topic)
 
     @exception.wrap_exception
     def receive(self, message_data, message):
-        _log.debug('received %s' % (message_data))
+        """Magically looks for a method on the proxy object and calls it
+
+        Message data should be a dictionary with two keys:
+            method: string representing the method to call
+            args: dictionary of arg: value
+
+        Example: {'method': 'echo', 'args': {'value': 42}}
+        """
+        LOG.debug('received %s' % (message_data))
         msg_id = message_data.pop('_msg_id', None)
 
         method = message_data.get('method')
@@ -133,21 +171,25 @@ class AdapterConsumer(TopicConsumer):
             #             messages stay in the queue indefinitely, so for now
             #             we just log the message and send an error string
             #             back to the caller
-            _log.warn('no method for message: %s' % (message_data))
+            LOG.warn('no method for message: %s' % (message_data))
             msg_reply(msg_id, 'No method for message: %s' % message_data)
             return
 
         node_func = getattr(self.proxy, str(method))
         node_args = dict((str(k), v) for k, v in args.iteritems())
+        # NOTE(vish): magic is fun!
+        # pylint: disable-msg=W0142
         d = defer.maybeDeferred(node_func, **node_args)
         if msg_id:
-            d.addCallback(lambda rval: msg_reply(msg_id, rval))
+            d.addCallback(lambda rval: msg_reply(msg_id, rval, None))
-            d.addErrback(lambda e: msg_reply(msg_id, str(e)))
+            d.addErrback(lambda e: msg_reply(msg_id, None, e))
         return
 
 
 class TopicPublisher(Publisher):
+    """Publishes messages on a specific topic"""
     exchange_type = "topic"
+
     def __init__(self, connection=None, topic="broadcast"):
         self.routing_key = topic
         self.exchange = FLAGS.control_exchange
@@ -156,7 +198,9 @@ class TopicPublisher(Publisher):
 
 
 class DirectConsumer(Consumer):
+    """Consumes messages directly on a channel specified by msg_id"""
     exchange_type = "direct"
+
     def __init__(self, connection=None, msg_id=None):
         self.queue = msg_id
         self.routing_key = msg_id
@@ -166,7 +210,9 @@ class DirectConsumer(Consumer):
 
 
 class DirectPublisher(Publisher):
+    """Publishes messages directly on a channel specified by msg_id"""
     exchange_type = "direct"
+
     def __init__(self, connection=None, msg_id=None):
         self.routing_key = msg_id
         self.exchange = msg_id
@@ -174,32 +220,63 @@ class DirectPublisher(Publisher):
         super(DirectPublisher, self).__init__(connection=connection)
 
 
-def msg_reply(msg_id, reply):
+def msg_reply(msg_id, reply=None, failure=None):
+    """Sends a reply or an error on the channel signified by msg_id
+
+    failure should be a twisted failure object"""
+    if failure:
+        message = failure.getErrorMessage()
+        traceback = failure.getTraceback()
+        logging.error("Returning exception %s to caller", message)
+        logging.error(traceback)
+        failure = (failure.type.__name__, str(failure.value), traceback)
     conn = Connection.instance()
     publisher = DirectPublisher(connection=conn, msg_id=msg_id)
-
     try:
-        publisher.send({'result': reply})
+        publisher.send({'result': reply, 'failure': failure})
     except TypeError:
         publisher.send(
                 {'result': dict((k, repr(v))
-                                for k, v in reply.__dict__.iteritems())
+                                for k, v in reply.__dict__.iteritems()),
-                 })
+                 'failure': failure})
     publisher.close()
 
 
+class RemoteError(exception.Error):
+    """Signifies that a remote class has raised an exception
+
+    Containes a string representation of the type of the original exception,
+    the value of the original exception, and the traceback.  These are
+    sent to the parent as a joined string so printing the exception
+    contains all of the relevent info."""
+    def __init__(self, exc_type, value, traceback):
+        self.exc_type = exc_type
+        self.value = value
+        self.traceback = traceback
+        super(RemoteError, self).__init__("%s %s\n%s" % (exc_type,
+                                                         value,
+                                                         traceback))
+
+
 def call(topic, msg):
-    _log.debug("Making asynchronous call...")
+    """Sends a message on a topic and wait for a response"""
+    LOG.debug("Making asynchronous call...")
     msg_id = uuid.uuid4().hex
     msg.update({'_msg_id': msg_id})
-    _log.debug("MSG_ID is %s" % (msg_id))
+    LOG.debug("MSG_ID is %s" % (msg_id))
 
     conn = Connection.instance()
     d = defer.Deferred()
     consumer = DirectConsumer(connection=conn, msg_id=msg_id)
+
     def deferred_receive(data, message):
+        """Acks message and callbacks or errbacks"""
         message.ack()
-        d.callback(data)
+        if data['failure']:
+            return d.errback(RemoteError(*data['failure']))
+        else:
+            return d.callback(data['result'])
+
     consumer.register_callback(deferred_receive)
     injected = consumer.attach_to_tornado()
 
@@ -213,7 +290,8 @@ def call(topic, msg):
 
 
 def cast(topic, msg):
-    _log.debug("Making asynchronous cast...")
+    """Sends a message on a topic without waiting for a response"""
+    LOG.debug("Making asynchronous cast...")
     conn = Connection.instance()
     publisher = TopicPublisher(connection=conn, topic=topic)
     publisher.send(msg)
@@ -221,16 +299,18 @@ def cast(topic, msg):
 
 
 def generic_response(message_data, message):
-    _log.debug('response %s', message_data)
+    """Logs a result and exits"""
+    LOG.debug('response %s', message_data)
     message.ack()
     sys.exit(0)
 
 
 def send_message(topic, message, wait=True):
+    """Sends a message for testing"""
     msg_id = uuid.uuid4().hex
     message.update({'_msg_id': msg_id})
-    _log.debug('topic is %s', topic)
+    LOG.debug('topic is %s', topic)
-    _log.debug('message %s', message)
+    LOG.debug('message %s', message)
 
     if wait:
         consumer = messaging.Consumer(connection=Connection.instance(),
@@ -253,6 +333,8 @@ def send_message(topic, message, wait=True):
         consumer.wait()
 
 
-# TODO: Replace with a docstring test
 if __name__ == "__main__":
+    # NOTE(vish): you can send messages from the command line using
+    #             topic and a json sting representing a dictionary
+    #             for the method
     send_message(sys.argv[1], json.loads(sys.argv[2]))

nova/server.py

@@ -44,6 +44,8 @@ flags.DEFINE_bool('use_syslog', True, 'output to syslog when daemonizing')
 flags.DEFINE_string('logfile', None, 'log file to output to')
 flags.DEFINE_string('pidfile', None, 'pid file to output to')
 flags.DEFINE_string('working_directory', './', 'working directory...')
+flags.DEFINE_integer('uid', os.getuid(), 'uid under which to run')
+flags.DEFINE_integer('gid', os.getgid(), 'gid under which to run')
 
 
 def stop(pidfile):
@@ -135,6 +137,8 @@ def daemonize(args, name, main):
                                                    threaded=False),
             stdin=stdin,
             stdout=stdout,
-            stderr=stderr
+            stderr=stderr,
+            uid=FLAGS.uid,
+            gid=FLAGS.gid
             ):
         main(args)

nova/tests/auth_unittest.py

@@ -179,7 +179,21 @@ class AuthTestCase(test.BaseTestCase):
         project.add_role('test1', 'sysadmin')
         self.assertTrue(project.has_role('test1', 'sysadmin'))
 
-    def test_211_can_remove_project_role(self):
+    def test_211_can_list_project_roles(self):
+        project = self.manager.get_project('testproj')
+        user = self.manager.get_user('test1')
+        self.manager.add_role(user, 'netadmin', project)
+        roles = self.manager.get_user_roles(user)
+        self.assertTrue('sysadmin' in roles)
+        self.assertFalse('netadmin' in roles)
+        project_roles = self.manager.get_user_roles(user, project)
+        self.assertTrue('sysadmin' in project_roles)
+        self.assertTrue('netadmin' in project_roles)
+        # has role should be false because global role is missing
+        self.assertFalse(self.manager.has_role(user, 'netadmin', project))
+
+
+    def test_212_can_remove_project_role(self):
         project = self.manager.get_project('testproj')
         self.assertTrue(project.has_role('test1', 'sysadmin'))
         project.remove_role('test1', 'sysadmin')

nova/tests/cloud_unittest.py

@@ -47,10 +47,6 @@ class CloudTestCase(test.BaseTestCase):
 
         # set up our cloud
         self.cloud = cloud.CloudController()
-        self.cloud_consumer = rpc.AdapterConsumer(connection=self.conn,
-                                                      topic=FLAGS.cloud_topic,
-                                                      proxy=self.cloud)
-        self.injected.append(self.cloud_consumer.attach_to_tornado(self.ioloop))
 
         # set up a service
         self.compute = service.ComputeService()
@@ -132,7 +128,7 @@ class CloudTestCase(test.BaseTestCase):
                 'state': 0x01,
                 'user_data': ''
             }
-        rv = self.cloud._format_instances(self.context)
+        rv = self.cloud._format_describe_instances(self.context)
         self.assert_(len(rv['reservationSet']) == 0)
 
         # simulate launch of 5 instances

nova/tests/declare_flags.py

@@ -16,25 +16,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-'''
+from nova import flags
-Utility methods for working with WSGI servers
-'''
 
-class Util(object):
+FLAGS = flags.FLAGS
-
-    @staticmethod
-    def route(reqstr, controllers):
-        if len(reqstr) == 0:
-            return Util.select_root_controller(controllers), []
-        parts = [x for x in reqstr.split("/") if len(x) > 0]
-        if len(parts) == 0:
-            return Util.select_root_controller(controllers), []
-        return controllers[parts[0]], parts[1:]
-
-    @staticmethod
-    def select_root_controller(controllers):
-        if '' in controllers:
-            return controllers['']
-        else:
-            return None
 
+flags.DEFINE_integer('answer', 42, 'test flag')

nova/tests/flags_unittest.py

@@ -0,0 +1,87 @@
+# 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.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from nova import exception
+from nova import flags
+from nova import test
+
+
+class FlagsTestCase(test.TrialTestCase):
+    def setUp(self):
+        super(FlagsTestCase, self).setUp()
+        self.FLAGS = flags.FlagValues()
+        self.global_FLAGS = flags.FLAGS
+
+    def test_define(self):
+        self.assert_('string' not in self.FLAGS)
+        self.assert_('int' not in self.FLAGS)
+        self.assert_('false' not in self.FLAGS)
+        self.assert_('true' not in self.FLAGS)
+
+        flags.DEFINE_string('string', 'default', 'desc', flag_values=self.FLAGS)
+        flags.DEFINE_integer('int', 1, 'desc', flag_values=self.FLAGS)
+        flags.DEFINE_bool('false', False, 'desc', flag_values=self.FLAGS)
+        flags.DEFINE_bool('true', True, 'desc', flag_values=self.FLAGS)
+
+        self.assert_(self.FLAGS['string'])
+        self.assert_(self.FLAGS['int'])
+        self.assert_(self.FLAGS['false'])
+        self.assert_(self.FLAGS['true'])
+        self.assertEqual(self.FLAGS.string, 'default')
+        self.assertEqual(self.FLAGS.int, 1)
+        self.assertEqual(self.FLAGS.false, False)
+        self.assertEqual(self.FLAGS.true, True)
+
+        argv = ['flags_test',
+                '--string', 'foo',
+                '--int', '2',
+                '--false',
+                '--notrue']
+
+        self.FLAGS(argv)
+        self.assertEqual(self.FLAGS.string, 'foo')
+        self.assertEqual(self.FLAGS.int, 2)
+        self.assertEqual(self.FLAGS.false, True)
+        self.assertEqual(self.FLAGS.true, False)
+
+    def test_declare(self):
+        self.assert_('answer' not in self.global_FLAGS)
+        flags.DECLARE('answer', 'nova.tests.declare_flags')
+        self.assert_('answer' in self.global_FLAGS)
+        self.assertEqual(self.global_FLAGS.answer, 42)
+
+        # Make sure we don't overwrite anything
+        self.global_FLAGS.answer = 256
+        self.assertEqual(self.global_FLAGS.answer, 256)
+        flags.DECLARE('answer', 'nova.tests.declare_flags')
+        self.assertEqual(self.global_FLAGS.answer, 256)
+
+    def test_runtime_and_unknown_flags(self):
+        self.assert_('runtime_answer' not in self.global_FLAGS)
+
+        argv = ['flags_test', '--runtime_answer=60', 'extra_arg']
+        args = self.global_FLAGS(argv)
+        self.assertEqual(len(args), 2)
+        self.assertEqual(args[1], 'extra_arg')
+
+        self.assert_('runtime_answer' not in self.global_FLAGS)
+
+        import nova.tests.runtime_flags
+
+        self.assert_('runtime_answer' in self.global_FLAGS)
+        self.assertEqual(self.global_FLAGS.runtime_answer, 60)

nova/tests/network_unittest.py

@@ -15,7 +15,9 @@
 #    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 #    License for the specific language governing permissions and limitations
 #    under the License.
-
+"""
+Unit Tests for network code
+"""
 import IPy
 import os
 import logging
@@ -31,8 +33,10 @@ from nova.network.exception import NoMoreAddresses
 
 FLAGS = flags.FLAGS
 
+
 class NetworkTestCase(test.TrialTestCase):
-    def setUp(self):
+    """Test cases for network code"""
+    def setUp(self):  # pylint: disable-msg=C0103
         super(NetworkTestCase, self).setUp()
         # NOTE(vish): if you change these flags, make sure to change the
         #             flags in the corresponding section in nova-dhcpbridge
@@ -43,7 +47,6 @@ class NetworkTestCase(test.TrialTestCase):
                    network_size=32)
         logging.getLogger().setLevel(logging.DEBUG)
         self.manager = manager.AuthManager()
-        self.dnsmasq = FakeDNSMasq()
         self.user = self.manager.create_user('netuser', 'netuser', 'netuser')
         self.projects = []
         self.projects.append(self.manager.create_project('netuser',
@@ -54,47 +57,49 @@ class NetworkTestCase(test.TrialTestCase):
             self.projects.append(self.manager.create_project(name,
                                                              'netuser',
                                                              name))
-        self.network = model.PublicNetworkController()
+            vpn.NetworkData.create(self.projects[i].id)
         self.service = service.VlanNetworkService()
 
-    def tearDown(self):
+    def tearDown(self):  # pylint: disable-msg=C0103
         super(NetworkTestCase, self).tearDown()
         for project in self.projects:
             self.manager.delete_project(project)
         self.manager.delete_user(self.user)
 
     def test_public_network_allocation(self):
+        """Makes sure that we can allocaate a public ip"""
         pubnet = IPy.IP(flags.FLAGS.public_range)
-        address = self.network.allocate_ip(self.user.id, self.projects[0].id, "public")
+        address = self.service.allocate_elastic_ip(self.user.id,
+                                           self.projects[0].id)
         self.assertTrue(IPy.IP(address) in pubnet)
-        self.assertTrue(IPy.IP(address) in self.network.network)
 
     def test_allocate_deallocate_fixed_ip(self):
-        result  = yield self.service.allocate_fixed_ip(
+        """Makes sure that we can allocate and deallocate a fixed ip"""
+        result = self.service.allocate_fixed_ip(
                 self.user.id, self.projects[0].id)
         address = result['private_dns_name']
         mac = result['mac_address']
-        logging.debug("Was allocated %s" % (address))
         net = model.get_project_network(self.projects[0].id, "default")
         self.assertEqual(True, is_in_project(address, self.projects[0].id))
         hostname = "test-host"
-        self.dnsmasq.issue_ip(mac, address, hostname, net.bridge_name)
+        issue_ip(mac, address, hostname, net.bridge_name)
-        rv = self.service.deallocate_fixed_ip(address)
+        self.service.deallocate_fixed_ip(address)
 
         # Doesn't go away until it's dhcp released
         self.assertEqual(True, is_in_project(address, self.projects[0].id))
 
-        self.dnsmasq.release_ip(mac, address, hostname, net.bridge_name)
+        release_ip(mac, address, hostname, net.bridge_name)
         self.assertEqual(False, is_in_project(address, self.projects[0].id))
 
-    def test_range_allocation(self):
+    def test_side_effects(self):
-        hostname = "test-host"
+        """Ensures allocating and releasing has no side effects"""
-        result = yield self.service.allocate_fixed_ip(
+        hostname = "side-effect-host"
-                    self.user.id, self.projects[0].id)
+        result = self.service.allocate_fixed_ip(self.user.id,
+                                                self.projects[0].id)
         mac = result['mac_address']
         address = result['private_dns_name']
-        result = yield self.service.allocate_fixed_ip(
+        result = self.service.allocate_fixed_ip(self.user,
-                self.user, self.projects[1].id)
+                                                self.projects[1].id)
         secondmac = result['mac_address']
         secondaddress = result['private_dns_name']
 
@@ -102,66 +107,74 @@ class NetworkTestCase(test.TrialTestCase):
         secondnet = model.get_project_network(self.projects[1].id, "default")
 
         self.assertEqual(True, is_in_project(address, self.projects[0].id))
-        self.assertEqual(True, is_in_project(secondaddress, self.projects[1].id))
+        self.assertEqual(True, is_in_project(secondaddress,
+                                             self.projects[1].id))
         self.assertEqual(False, is_in_project(address, self.projects[1].id))
 
         # Addresses are allocated before they're issued
-        self.dnsmasq.issue_ip(mac, address, hostname, net.bridge_name)
+        issue_ip(mac, address, hostname, net.bridge_name)
-        self.dnsmasq.issue_ip(secondmac, secondaddress,
+        issue_ip(secondmac, secondaddress, hostname, secondnet.bridge_name)
-                                hostname, secondnet.bridge_name)
 
-        rv = self.service.deallocate_fixed_ip(address)
+        self.service.deallocate_fixed_ip(address)
-        self.dnsmasq.release_ip(mac, address, hostname, net.bridge_name)
+        release_ip(mac, address, hostname, net.bridge_name)
         self.assertEqual(False, is_in_project(address, self.projects[0].id))
 
         # First address release shouldn't affect the second
-        self.assertEqual(True, is_in_project(secondaddress, self.projects[1].id))
+        self.assertEqual(True, is_in_project(secondaddress,
+                                             self.projects[1].id))
 
-        rv = self.service.deallocate_fixed_ip(secondaddress)
+        self.service.deallocate_fixed_ip(secondaddress)
-        self.dnsmasq.release_ip(secondmac, secondaddress,
+        release_ip(secondmac, secondaddress, hostname, secondnet.bridge_name)
-                                hostname, secondnet.bridge_name)
+        self.assertEqual(False, is_in_project(secondaddress,
-        self.assertEqual(False, is_in_project(secondaddress, self.projects[1].id))
+                                              self.projects[1].id))
 
     def test_subnet_edge(self):
-        result = yield self.service.allocate_fixed_ip(self.user.id,
+        """Makes sure that private ips don't overlap"""
+        result = 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):
+        for i in range(1, 5):
             project_id = self.projects[i].id
-            result = yield self.service.allocate_fixed_ip(
+            result = self.service.allocate_fixed_ip(
                     self.user, project_id)
             mac = result['mac_address']
             address = result['private_dns_name']
-            result = yield self.service.allocate_fixed_ip(
+            result = self.service.allocate_fixed_ip(
                     self.user, project_id)
             mac2 = result['mac_address']
             address2 = result['private_dns_name']
-            result = yield self.service.allocate_fixed_ip(
+            result = self.service.allocate_fixed_ip(
                    self.user, project_id)
             mac3 = result['mac_address']
             address3 = result['private_dns_name']
-            self.assertEqual(False, is_in_project(address, self.projects[0].id))
-            self.assertEqual(False, is_in_project(address2, self.projects[0].id))
-            self.assertEqual(False, is_in_project(address3, self.projects[0].id))
-            rv = self.service.deallocate_fixed_ip(address)
-            rv = self.service.deallocate_fixed_ip(address2)
-            rv = self.service.deallocate_fixed_ip(address3)
             net = model.get_project_network(project_id, "default")
-            self.dnsmasq.release_ip(mac, address, hostname, net.bridge_name)
+            issue_ip(mac, address, hostname, net.bridge_name)
-            self.dnsmasq.release_ip(mac2, address2, hostname, net.bridge_name)
+            issue_ip(mac2, address2, hostname, net.bridge_name)
-            self.dnsmasq.release_ip(mac3, address3, hostname, net.bridge_name)
+            issue_ip(mac3, address3, hostname, net.bridge_name)
+            self.assertEqual(False, is_in_project(address,
+                                                  self.projects[0].id))
+            self.assertEqual(False, is_in_project(address2,
+                                                  self.projects[0].id))
+            self.assertEqual(False, is_in_project(address3,
+                                                  self.projects[0].id))
+            self.service.deallocate_fixed_ip(address)
+            self.service.deallocate_fixed_ip(address2)
+            self.service.deallocate_fixed_ip(address3)
+            release_ip(mac, address, hostname, net.bridge_name)
+            release_ip(mac2, address2, hostname, net.bridge_name)
+            release_ip(mac3, address3, hostname, net.bridge_name)
         net = model.get_project_network(self.projects[0].id, "default")
-        rv = self.service.deallocate_fixed_ip(firstaddress)
+        self.service.deallocate_fixed_ip(firstaddress)
-        self.dnsmasq.release_ip(mac, firstaddress, hostname, net.bridge_name)
 
-    def test_212_vpn_ip_and_port_looks_valid(self):
+    def test_vpn_ip_and_port_looks_valid(self):
-        vpn.NetworkData.create(self.projects[0].id)
+        """Ensure the vpn ip and port are reasonable"""
         self.assert_(self.projects[0].vpn_ip)
         self.assert_(self.projects[0].vpn_port >= FLAGS.vpn_start_port)
         self.assert_(self.projects[0].vpn_port <= FLAGS.vpn_end_port)
 
     def test_too_many_vpns(self):
+        """Ensure error is raised if we run out of vpn ports"""
         vpns = []
         for i in xrange(vpn.NetworkData.num_ports_for_ip(FLAGS.vpn_ip)):
             vpns.append(vpn.NetworkData.create("vpnuser%s" % i))
@@ -169,84 +182,115 @@ class NetworkTestCase(test.TrialTestCase):
         for network_datum in vpns:
             network_datum.destroy()
 
-    def test_release_before_deallocate(self):
+    def test_ips_are_reused(self):
-        pass
+        """Makes sure that ip addresses that are deallocated get reused"""
+        net = model.get_project_network(self.projects[0].id, "default")
 
-    def test_deallocate_before_issued(self):
+        hostname = "reuse-host"
-        pass
+        macs = {}
+        addresses = {}
+        num_available_ips = net.num_available_ips
+        for i in range(num_available_ips - 1):
+            result = self.service.allocate_fixed_ip(self.user.id,
+                                                    self.projects[0].id)
+            macs[i] = result['mac_address']
+            addresses[i] = result['private_dns_name']
+            issue_ip(macs[i], addresses[i], hostname, net.bridge_name)
 
-    def test_too_many_addresses(self):
+        result = self.service.allocate_fixed_ip(self.user.id,
-        """
+                                                self.projects[0].id)
-        Here, we test that a proper NoMoreAddresses exception is raised.
+        mac = result['mac_address']
+        address = result['private_dns_name']
 
-        However, the number of available IP addresses depends on the test
+        issue_ip(mac, address, hostname, net.bridge_name)
+        self.service.deallocate_fixed_ip(address)
+        release_ip(mac, address, hostname, net.bridge_name)
+
+        result = self.service.allocate_fixed_ip(
+                self.user, self.projects[0].id)
+        secondmac = result['mac_address']
+        secondaddress = result['private_dns_name']
+        self.assertEqual(address, secondaddress)
+        issue_ip(secondmac, secondaddress, hostname, net.bridge_name)
+        self.service.deallocate_fixed_ip(secondaddress)
+        release_ip(secondmac, secondaddress, hostname, net.bridge_name)
+
+        for i in range(len(addresses)):
+            self.service.deallocate_fixed_ip(addresses[i])
+            release_ip(macs[i], addresses[i], hostname, net.bridge_name)
+
+    def test_available_ips(self):
+        """Make sure the number of available ips for the network is correct
+
+        The number of available IP addresses depends on the test
         environment's setup.
 
         Network size is set in test fixture's setUp method.
 
-        There are FLAGS.cnt_vpn_clients addresses reserved for VPN (NUM_RESERVED_VPN_IPS)
+        There are ips reserved at the bottom and top of the range.
-
+        services (network, gateway, CloudPipe, broadcast)
-        And there are NUM_STATIC_IPS that are always reserved by Nova for the necessary
-        services (gateway, CloudPipe, etc)
-
-        So we should get flags.network_size - (NUM_STATIC_IPS +
-                                               NUM_PREALLOCATED_IPS +
-                                               NUM_RESERVED_VPN_IPS)
-        usable addresses
         """
         net = model.get_project_network(self.projects[0].id, "default")
+        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 +
+                                        net.num_top_reserved_ips)
+        self.assertEqual(num_available_ips, net.num_available_ips)
 
-        # Determine expected number of available IP addresses
+    def test_too_many_addresses(self):
-        num_static_ips = net.num_static_ips
+        """Test for a NoMoreAddresses exception when all fixed ips are used.
-        num_preallocated_ips = len(net.hosts.keys())
+        """
-        num_reserved_vpn_ips = flags.FLAGS.cnt_vpn_clients
+        net = model.get_project_network(self.projects[0].id, "default")
-        num_available_ips = flags.FLAGS.network_size - (num_static_ips +
-                                                        num_preallocated_ips +
-                                                        num_reserved_vpn_ips)
 
         hostname = "toomany-hosts"
         macs = {}
         addresses = {}
-        for i in range(0, (num_available_ips - 1)):
+        num_available_ips = net.num_available_ips
-            result = yield self.service.allocate_fixed_ip(self.user.id, self.projects[0].id)
+        for i in range(num_available_ips):
+            result = 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)
+            issue_ip(macs[i], addresses[i], hostname, net.bridge_name)
 
-        self.assertFailure(self.service.allocate_fixed_ip(self.user.id, self.projects[0].id), NoMoreAddresses)
+        self.assertEqual(net.num_available_ips, 0)
+        self.assertRaises(NoMoreAddresses, self.service.allocate_fixed_ip,
+                          self.user.id, self.projects[0].id)
+
+        for i in range(len(addresses)):
+            self.service.deallocate_fixed_ip(addresses[i])
+            release_ip(macs[i], addresses[i], hostname, net.bridge_name)
+        self.assertEqual(net.num_available_ips, num_available_ips)
 
-        for i in range(0, (num_available_ips - 1)):
-            rv = self.service.deallocate_fixed_ip(addresses[i])
-            self.dnsmasq.release_ip(macs[i], addresses[i], hostname, net.bridge_name)
 
 def is_in_project(address, project_id):
-    return address in model.get_project_network(project_id).list_addresses()
+    """Returns true if address is in specified project"""
+    return address in model.get_project_network(project_id).assigned
 
-def _get_project_addresses(project_id):
-    project_addresses = []
-    for addr in model.get_project_network(project_id).list_addresses():
-        project_addresses.append(addr)
-    return project_addresses
 
 def binpath(script):
+    """Returns the absolute path to a script in bin"""
     return os.path.abspath(os.path.join(__file__, "../../../bin", script))
 
-class FakeDNSMasq(object):
-    def issue_ip(self, mac, ip, hostname, interface):
-        cmd = "%s add %s %s %s" % (binpath('nova-dhcpbridge'),
-                                   mac, ip, hostname)
-        env = {'DNSMASQ_INTERFACE': interface,
-               'TESTING' : '1',
-               'FLAGFILE' : FLAGS.dhcpbridge_flagfile}
-        (out, err) = utils.execute(cmd, addl_env=env)
-        logging.debug("ISSUE_IP: %s, %s " % (out, err))
 
-    def release_ip(self, mac, ip, hostname, interface):
+def issue_ip(mac, private_ip, hostname, interface):
-        cmd = "%s del %s %s %s" % (binpath('nova-dhcpbridge'),
+    """Run add command on dhcpbridge"""
-                                   mac, ip, hostname)
+    cmd = "%s add %s %s %s" % (binpath('nova-dhcpbridge'),
-        env = {'DNSMASQ_INTERFACE': interface,
+                               mac, private_ip, hostname)
-               'TESTING' : '1',
+    env = {'DNSMASQ_INTERFACE': interface,
-               'FLAGFILE' : FLAGS.dhcpbridge_flagfile}
+           'TESTING': '1',
-        (out, err) = utils.execute(cmd, addl_env=env)
+           'FLAGFILE': FLAGS.dhcpbridge_flagfile}
-        logging.debug("RELEASE_IP: %s, %s " % (out, err))
+    (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'),
+                               mac, private_ip, hostname)
+    env = {'DNSMASQ_INTERFACE': interface,
+           'TESTING': '1',
+           'FLAGFILE': FLAGS.dhcpbridge_flagfile}
+    (out, err) = utils.execute(cmd, addl_env=env)
+    logging.debug("RELEASE_IP: %s, %s ", out, err)

nova/tests/process_unittest.py

@@ -48,7 +48,7 @@ class ProcessTestCase(test.TrialTestCase):
 
     def test_execute_stderr(self):
         pool = process.ProcessPool(2)
-        d = pool.simple_execute('cat BAD_FILE', error_ok=1)
+        d = pool.simple_execute('cat BAD_FILE', check_exit_code=False)
         def _check(rv):
             self.assertEqual(rv[0], '')
             self.assert_('No such file' in rv[1])

nova/tests/rpc_unittest.py

@@ -0,0 +1,85 @@
+# 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.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+"""
+Unit Tests for remote procedure calls using queue
+"""
+import logging
+
+from twisted.internet import defer
+
+from nova import flags
+from nova import rpc
+from nova import test
+
+
+FLAGS = flags.FLAGS
+
+
+class RpcTestCase(test.BaseTestCase):
+    """Test cases for rpc"""
+    def setUp(self):  # pylint: disable-msg=C0103
+        super(RpcTestCase, self).setUp()
+        self.conn = rpc.Connection.instance()
+        self.receiver = TestReceiver()
+        self.consumer = rpc.AdapterConsumer(connection=self.conn,
+                                            topic='test',
+                                            proxy=self.receiver)
+
+        self.injected.append(self.consumer.attach_to_tornado(self.ioloop))
+
+    def test_call_succeed(self):
+        """Get a value through rpc call"""
+        value = 42
+        result = yield rpc.call('test', {"method": "echo",
+                                         "args": {"value": value}})
+        self.assertEqual(value, result)
+
+    def test_call_exception(self):
+        """Test that exception gets passed back properly
+
+        rpc.call returns a RemoteError object.  The value of the
+        exception is converted to a string, so we convert it back
+        to an int in the test.
+        """
+        value = 42
+        self.assertFailure(rpc.call('test', {"method": "fail",
+                                             "args": {"value": value}}),
+                           rpc.RemoteError)
+        try:
+            yield rpc.call('test', {"method": "fail",
+                                    "args": {"value": value}})
+            self.fail("should have thrown rpc.RemoteError")
+        except rpc.RemoteError as exc:
+            self.assertEqual(int(exc.value), value)
+
+
+class TestReceiver(object):
+    """Simple Proxy class so the consumer has methods to call
+
+    Uses static methods because we aren't actually storing any state"""
+
+    @staticmethod
+    def echo(value):
+        """Simply returns whatever value is sent in"""
+        logging.debug("Received %s", value)
+        return defer.succeed(value)
+
+    @staticmethod
+    def fail(value):
+        """Raises an exception with the value sent in"""
+        raise Exception(value)

nova/tests/runtime_flags.py

@@ -16,36 +16,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import cloudservers
+from nova import flags
 
-class IdFake:
+FLAGS = flags.FLAGS
-    def __init__(self, id):
-        self.id = id
 
-# to get your access key:
+flags.DEFINE_integer('runtime_answer', 54, 'test flag')
-# from nova.auth import users
-# users.UserManger.instance().get_users()[0].access
-rscloud = cloudservers.CloudServers(
-            'admin',
-            '6cca875e-5ab3-4c60-9852-abf5c5c60cc6'
-          )
-rscloud.client.AUTH_URL = 'http://localhost:8773/v1.0'
-
-
-rv = rscloud.servers.list()
-print "SERVERS: %s" % rv
-
-if len(rv) == 0:
-    server = rscloud.servers.create(
-               "test-server",
-               IdFake("ami-tiny"),
-               IdFake("m1.tiny")
-             )
-    print "LAUNCH: %s" % server
-else:
-    server = rv[0]
-    print "Server to kill: %s" % server
-
-raw_input("press enter key to kill the server")
-
-server.delete()

nova/tests/virt_unittest.py

@@ -0,0 +1,69 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+#
+#    Copyright 2010 OpenStack LLC
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from nova import flags
+from nova import test
+from nova.virt import libvirt_conn
+
+FLAGS = flags.FLAGS
+
+
+class LibvirtConnTestCase(test.TrialTestCase):
+    def test_get_uri_and_template(self):
+        class MockDataModel(object):
+            def __init__(self):
+                self.datamodel = { 'name' : 'i-cafebabe',
+                                   'memory_kb' : '1024000',
+                                   'basepath' : '/some/path',
+                                   'bridge_name' : 'br100',
+                                   'mac_address' : '02:12:34:46:56:67',
+                                   'vcpus' : 2 }
+
+        type_uri_map = { 'qemu' : ('qemu:///system',
+                                [lambda s: '<domain type=\'qemu\'>' in s,
+                                 lambda s: 'type>hvm</type' in s,
+                                 lambda s: 'emulator>/usr/bin/kvm' not in s]),
+                         'kvm' : ('qemu:///system',
+                                [lambda s: '<domain type=\'kvm\'>' in s,
+                                 lambda s: 'type>hvm</type' in s,
+                                 lambda s: 'emulator>/usr/bin/qemu<' not in s]),
+                         'uml' : ('uml:///system',
+                                [lambda s: '<domain type=\'uml\'>' in s,
+                                 lambda s: 'type>uml</type' in s]),
+                          }
+
+        for (libvirt_type,(expected_uri, checks)) in type_uri_map.iteritems():
+            FLAGS.libvirt_type = libvirt_type
+            conn = libvirt_conn.LibvirtConnection(True)
+
+            uri, template = conn.get_uri_and_template()
+            self.assertEquals(uri, expected_uri)
+
+            for i, check in enumerate(checks):
+                xml = conn.toXml(MockDataModel())
+                self.assertTrue(check(xml), '%s failed check %d' % (xml, i))
+
+        # Deliberately not just assigning this string to FLAGS.libvirt_uri and
+        # checking against that later on. This way we make sure the
+        # implementation doesn't fiddle around with the FLAGS.
+        testuri = 'something completely different'
+        FLAGS.libvirt_uri = testuri
+        for (libvirt_type,(expected_uri, checks)) in type_uri_map.iteritems():
+            FLAGS.libvirt_type = libvirt_type
+            conn = libvirt_conn.LibvirtConnection(True)
+            uri, template = conn.get_uri_and_template()
+            self.assertEquals(uri, testuri)
+

nova/tests/volume_unittest.py

@@ -17,6 +17,10 @@
 #    under the License.
 
 import logging
+import shutil
+import tempfile
+
+from twisted.internet import defer
 
 from nova import compute
 from nova import exception
@@ -34,10 +38,16 @@ class VolumeTestCase(test.TrialTestCase):
         super(VolumeTestCase, self).setUp()
         self.compute = compute.service.ComputeService()
         self.volume = None
+        self.tempdir = tempfile.mkdtemp()
         self.flags(connection_type='fake',
-                   fake_storage=True)
+                   fake_storage=True,
+                   aoe_export_dir=self.tempdir)
         self.volume = volume_service.VolumeService()
 
+    def tearDown(self):
+        shutil.rmtree(self.tempdir)
+
+    @defer.inlineCallbacks
     def test_run_create_volume(self):
         vol_size = '0'
         user_id = 'fake'
@@ -48,34 +58,40 @@ class VolumeTestCase(test.TrialTestCase):
                          volume_service.get_volume(volume_id)['volume_id'])
 
         rv = self.volume.delete_volume(volume_id)
-        self.assertFailure(volume_service.get_volume(volume_id),
+        self.assertRaises(exception.Error, volume_service.get_volume, volume_id)
-                           exception.Error)
 
+    @defer.inlineCallbacks
     def test_too_big_volume(self):
         vol_size = '1001'
         user_id = 'fake'
         project_id = 'fake'
-        self.assertRaises(TypeError,
+        try:
-                          self.volume.create_volume,
+            yield self.volume.create_volume(vol_size, user_id, project_id)
-                          vol_size, user_id, project_id)
+            self.fail("Should have thrown TypeError")
+        except TypeError:
+            pass
 
+    @defer.inlineCallbacks
     def test_too_many_volumes(self):
         vol_size = '1'
         user_id = 'fake'
         project_id = 'fake'
         num_shelves = FLAGS.last_shelf_id - FLAGS.first_shelf_id + 1
-        total_slots = FLAGS.slots_per_shelf * num_shelves
+        total_slots = FLAGS.blades_per_shelf * num_shelves
         vols = []
+        from nova import datastore
+        redis = datastore.Redis.instance()
         for i in xrange(total_slots):
             vid = yield self.volume.create_volume(vol_size, user_id, project_id)
             vols.append(vid)
         self.assertFailure(self.volume.create_volume(vol_size,
                                                      user_id,
                                                      project_id),
-                           volume_service.NoMoreVolumes)
+                           volume_service.NoMoreBlades)
         for id in vols:
             yield self.volume.delete_volume(id)
 
+    @defer.inlineCallbacks
     def test_run_attach_detach_volume(self):
         # Create one volume and one compute to test with
         instance_id = "storage-test"
@@ -84,22 +100,26 @@ class VolumeTestCase(test.TrialTestCase):
         project_id = 'fake'
         mountpoint = "/dev/sdf"
         volume_id = yield self.volume.create_volume(vol_size, user_id, project_id)
-
         volume_obj = volume_service.get_volume(volume_id)
         volume_obj.start_attach(instance_id, mountpoint)
-        rv = yield self.compute.attach_volume(volume_id,
+        if FLAGS.fake_tests:
-                                          instance_id,
+            volume_obj.finish_attach()
-                                          mountpoint)
+        else:
+            rv = yield self.compute.attach_volume(instance_id,
+                                                  volume_id,
+                                                  mountpoint)
         self.assertEqual(volume_obj['status'], "in-use")
-        self.assertEqual(volume_obj['attachStatus'], "attached")
+        self.assertEqual(volume_obj['attach_status'], "attached")
         self.assertEqual(volume_obj['instance_id'], instance_id)
         self.assertEqual(volume_obj['mountpoint'], mountpoint)
 
-        self.assertRaises(exception.Error,
+        self.assertFailure(self.volume.delete_volume(volume_id), exception.Error)
-                          self.volume.delete_volume,
+        volume_obj.start_detach()
-                          volume_id)
+        if FLAGS.fake_tests:
-
+            volume_obj.finish_detach()
-        rv = yield self.volume.detach_volume(volume_id)
+        else:
+            rv = yield self.volume.detach_volume(instance_id,
+                                                 volume_id)
         volume_obj = volume_service.get_volume(volume_id)
         self.assertEqual(volume_obj['status'], "available")
 
@@ -108,6 +128,27 @@ class VolumeTestCase(test.TrialTestCase):
                           volume_service.get_volume,
                           volume_id)
 
+    @defer.inlineCallbacks
+    def test_multiple_volume_race_condition(self):
+        vol_size = "5"
+        user_id = "fake"
+        project_id = 'fake'
+        shelf_blades = []
+        def _check(volume_id):
+            vol = volume_service.get_volume(volume_id)
+            shelf_blade = '%s.%s' % (vol['shelf_id'], vol['blade_id'])
+            self.assert_(shelf_blade not in shelf_blades)
+            shelf_blades.append(shelf_blade)
+            logging.debug("got %s" % shelf_blade)
+            vol.destroy()
+        deferreds = []
+        for i in range(5):
+            d = self.volume.create_volume(vol_size, user_id, project_id)
+            d.addCallback(_check)
+            d.addErrback(self.fail)
+            deferreds.append(d)
+        yield defer.DeferredList(deferreds)
+
     def test_multi_node(self):
         # TODO(termie): Figure out how to test with two nodes,
         # each of them having a different FLAG for storage_node

nova/twistd.py

@@ -21,6 +21,7 @@ Twisted daemon helpers, specifically to parse out gFlags from twisted flags,
 manage pid files and support syslogging.
 """
 
+import gflags
 import logging
 import os
 import signal
@@ -49,6 +50,14 @@ class TwistdServerOptions(ServerOptions):
         return
 
 
+class FlagParser(object):
+    def __init__(self, parser):
+        self.parser = parser
+
+    def Parse(self, s):
+        return self.parser(s)
+
+
 def WrapTwistedOptions(wrapped):
     class TwistedOptionsToFlags(wrapped):
         subCommands = None
@@ -79,7 +88,12 @@ def WrapTwistedOptions(wrapped):
             reflect.accumulateClassList(self.__class__, 'optParameters', twistd_params)
             for param in twistd_params:
                 key = param[0].replace('-', '_')
-                flags.DEFINE_string(key, param[2], str(param[-1]))
+                if len(param) > 4:
+                    flags.DEFINE(FlagParser(param[4]),
+                                 key, param[2], str(param[3]),
+                                 serializer=gflags.ArgumentSerializer())
+                else:
+                    flags.DEFINE_string(key, param[2], str(param[3]))
 
         def _absorbHandlers(self):
             twistd_handlers = {}
@@ -241,15 +255,7 @@ def serve(filename):
         print 'usage: %s [options] [start|stop|restart]' % argv[0]
         sys.exit(1)
 
-    class NoNewlineFormatter(logging.Formatter):
+    formatter = logging.Formatter(
-        """Strips newlines from default formatter"""
-        def format(self, record):
-            """Grabs default formatter's output and strips newlines"""
-            data = logging.Formatter.format(self, record)
-            return data.replace("\n", "--")
-
-    # NOTE(vish): syslog-ng doesn't handle newlines from trackbacks very well
-    formatter = NoNewlineFormatter(
         '(%(name)s): %(levelname)s %(message)s')
     handler = logging.StreamHandler(log.StdioOnnaStick())
     handler.setFormatter(formatter)

nova/validate.py

@@ -57,6 +57,7 @@ def rangetest(**argchecks):                 # validate ranges for both+defaults
         return onCall
     return onDecorator
 
+
 def typetest(**argchecks):
     def onDecorator(func):
         import sys

nova/wsgi_test.py

@@ -0,0 +1,96 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2010 United States Government as represented by the
+# Administrator of the National Aeronautics and Space Administration.
+# Copyright 2010 OpenStack LLC.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+"""
+Test WSGI basics and provide some helper functions for other WSGI tests.
+"""
+
+import unittest
+
+import routes
+import webob
+
+from nova import wsgi
+
+
+class Test(unittest.TestCase):
+
+    def test_debug(self):
+
+        class Application(wsgi.Application):
+            """Dummy application to test debug."""
+
+            def __call__(self, environ, start_response):
+                start_response("200", [("X-Test", "checking")])
+                return ['Test result']
+
+        application = wsgi.Debug(Application())
+        result = webob.Request.blank('/').get_response(application)
+        self.assertEqual(result.body, "Test result")
+
+    def test_router(self):
+
+        class Application(wsgi.Application):
+            """Test application to call from router."""
+
+            def __call__(self, environ, start_response):
+                start_response("200", [])
+                return ['Router result']
+
+        class Router(wsgi.Router):
+            """Test router."""
+
+            def __init__(self):
+                mapper = routes.Mapper()
+                mapper.connect("/test", controller=Application())
+                super(Router, self).__init__(mapper)
+
+        result = webob.Request.blank('/test').get_response(Router())
+        self.assertEqual(result.body, "Router result")
+        result = webob.Request.blank('/bad').get_response(Router())
+        self.assertNotEqual(result.body, "Router result")
+
+    def test_controller(self):
+
+        class Controller(wsgi.Controller):
+            """Test controller to call from router."""
+            test = self
+
+            def show(self, req, id): # pylint: disable-msg=W0622,C0103
+                """Default action called for requests with an ID."""
+                self.test.assertEqual(req.path_info, '/tests/123')
+                self.test.assertEqual(id, '123')
+                return id
+
+        class Router(wsgi.Router):
+            """Test router."""
+
+            def __init__(self):
+                mapper = routes.Mapper()
+                mapper.resource("test", "tests", controller=Controller())
+                super(Router, self).__init__(mapper)
+
+        result = webob.Request.blank('/tests/123').get_response(Router())
+        self.assertEqual(result.body, "123")
+        result = webob.Request.blank('/test/123').get_response(Router())
+        self.assertNotEqual(result.body, "123")
+
+    def test_serializer(self):
+        # TODO(eday): Placeholder for serializer testing.
+        pass

run_tests.py

@@ -38,11 +38,11 @@ Due to our use of multiprocessing it we frequently get some ignorable
 'Interrupted system call' exceptions after test completion.
 
 """
+
 import __main__
 import os
 import sys
 
-
 from twisted.scripts import trial as trial_script
 
 from nova import datastore
@@ -54,21 +54,23 @@ from nova.tests.auth_unittest import *
 from nova.tests.api_unittest import *
 from nova.tests.cloud_unittest import *
 from nova.tests.compute_unittest import *
+from nova.tests.flags_unittest import *
 from nova.tests.model_unittest import *
 from nova.tests.network_unittest import *
 from nova.tests.objectstore_unittest import *
 from nova.tests.process_unittest import *
+from nova.tests.rpc_unittest import *
 from nova.tests.validator_unittest import *
 from nova.tests.volume_unittest import *
 
 
 FLAGS = flags.FLAGS
-
 flags.DEFINE_bool('flush_db', True,
                   'Flush the database before running fake tests')
-
 flags.DEFINE_string('tests_stderr', 'run_tests.err.log',
-                    'Path to where to pipe STDERR during test runs. Default = "run_tests.err.log"')
+                    'Path to where to pipe STDERR during test runs.'
+                    ' Default = "run_tests.err.log"')
+
 
 if __name__ == '__main__':
     OptionsClass = twistd.WrapTwistedOptions(trial_script.Options)