Merge lp:~vishvananda/nova/quotas into lp:~hudson-openstack/nova/trunk

[Vish Ishaya]

Implements quotas with overrides for instances, volumes, and floating ips.

Implements quotas with overrides for instances, volumes, and floating ips.

[Jesse Andrews (anotherjesse)]

I'd like to see nova-manage commands to alter the project's quotas before we merge this

[Vish Ishaya (vishvananda)]

Working on nova-manage commands

[Vish Ishaya (vishvananda)]

Nova manage command is in:
nova-manage project quota [key] [value]

[Jay Pipes (jaypipes)]

Hi Vish!

Nice stuff!

Some small stuff I noticed...

87 + project_quota = quota._get_quota(None, project_id)

You might as well make quota._get_quota() into a public function since it's used outside of the quote.py file, so a rename to quota.get_quota() would be good (also a docstring on that function would be super-cool that indicates the function returns a set of default quota configuration options, overridden with any non-null value stored in the db for that project...)

379 + size = int(size)
380 + if quota.allowed_volumes(context, 1, size) < 1:
381 + logging.warn("Quota exceeeded for %s, tried to create %sG volume",
382 + context.project.id, size)
383 + raise QuotaError("Volume quota exceeded. You cannot "
384 + "create a volume of size %s" %
385 + size)

The above interface for determining whether a quote has been exceeded could be refactored slightly to reduce misuse. Instead of having the caller remember to cast size to an int, the quota.allowed_volumes() would be a better place to do that cast since it means one less assumption on the part of the interface.

Also, you may consider changing the interface to return a boolean instead of a number of allowed volumes. This may improve readability.

For example, you could have the following code instead:

if quota.volumes_exceeded(context, size):
    ... raise error, log, etc..

Hope that makes sense...I think that would make the interface a bit more intuitive. You could even keep the quota.allowed_volumes() function and make a small convenience wrapper around it to remove some of the awkwardness in the current interface.

Anyway, just some thoughts. Great work overall, Vish :)

-jay

[Vish Ishaya (vishvananda)]

I went ahead and made the first couple of changes. The idea of passing in requested volumes and returning allowed is to allow for the future possibility for requesting more than one volume (or address at a time). The current api allows for launching n instances, so it makes sense there to pass in num_requested and get a num_allowed back.

I agree that it is a bit awkward given the current api, but i think keeping the same logic that we use for instances makes sense for the future idea of creating multiple volume and multiple ips in a single call.

[Jay Pipes (jaypipes)]

OK, no worries, like I mentioned, that last comment was recently just a suggestion, nothing more. :)

I would approve, but I'm seeing merge conflicts showing up. Vish, could you remerge trunk one more time? I think the scheduler branch may have introduced a conflict?

Cheers!
jay

review: Needs Fixing

[lp:~vishvananda/nova/quotas]

375. By Vish Ishaya on 2010-09-21

merged trunk

[Jesse Andrews (anotherjesse)]

lgtm

review: Approve

[Jay Pipes (jaypipes)]

yep, cool with me.

review: Approve

=== modified file 'bin/nova-manage'
--- bin/nova-manage	2010-09-11 07:21:58 +0000
+++ bin/nova-manage	2010-09-21 04:22:40 +0000
@@ -50,7 +50,6 @@
 
 """
   CLI interface for nova management.
-  Connects to the running ADMIN api in the api daemon.
 """
 
 import os
@@ -68,7 +67,9 @@
     sys.path.insert(0, possible_topdir)
 
 from nova import db
+from nova import exception
 from nova import flags
+from nova import quota
 from nova import utils
 from nova.auth import manager
 from nova.cloudpipe import pipelib
@@ -186,6 +187,13 @@
 class UserCommands(object):
     """Class for managing users."""
 
+    @staticmethod
+    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
+
+
     def __init__(self):
         self.manager = manager.AuthManager()
 
@@ -193,13 +201,13 @@
         """creates a new admin and prints exports
         arguments: name [access] [secret]"""
         user = self.manager.create_user(name, access, secret, True)
-        print_export(user)
+        self._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)
-        print_export(user)
+        self._print_export(user)
 
     def delete(self, name):
         """deletes an existing user
@@ -211,7 +219,7 @@
         arguments: name"""
         user = self.manager.get_user(name)
         if user:
-            print_export(user)
+            self._print_export(user)
         else:
             print "User %s doesn't exist" % name
 
@@ -222,12 +230,6 @@
             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."""
 
@@ -262,6 +264,19 @@
         for project in self.manager.get_projects():
             print project.name
 
+    def quota(self, project_id, key=None, value=None):
+        """Set or display quotas for project
+        arguments: project_id [key] [value]"""
+        if key:
+            quo = {'project_id': project_id, key: value}
+            try:
+                db.quota_update(None, project_id, quo)
+            except exception.NotFound:
+                db.quota_create(None, quo)
+        project_quota = quota.get_quota(None, project_id)
+        for key, value in project_quota.iteritems():
+            print '%s: %s' % (key, value)
+
     def remove(self, project, user):
         """Removes user from project
         arguments: project user"""
@@ -274,6 +289,7 @@
         with open(filename, 'w') as f:
             f.write(zip_file)
 
+
 class FloatingIpCommands(object):
     """Class for managing floating ip."""
 
@@ -306,6 +322,7 @@
                                   floating_ip['address'],
                                   instance)
 
+
 CATEGORIES = [
     ('user', UserCommands),
     ('project', ProjectCommands),

=== modified file 'nova/endpoint/cloud.py'
--- nova/endpoint/cloud.py	2010-09-21 04:08:25 +0000
+++ nova/endpoint/cloud.py	2010-09-21 04:22:40 +0000
@@ -294,7 +294,6 @@
     @rbac.allow('projectmanager', 'sysadmin')
     def create_volume(self, context, size, **kwargs):
         # check quota
-        size = int(size)
         if quota.allowed_volumes(context, 1, size) < 1:
             logging.warn("Quota exceeeded for %s, tried to create %sG volume",
                          context.project.id, size)

=== modified file 'nova/quota.py'
--- nova/quota.py	2010-09-10 04:42:18 +0000
+++ nova/quota.py	2010-09-21 04:22:40 +0000
@@ -37,7 +37,7 @@
 flags.DEFINE_integer('quota_floating_ips', 10,
                      'number of floating ips allowed per project')
 
-def _get_quota(context, project_id):
+def get_quota(context, project_id):
     rval = {'instances': FLAGS.quota_instances,
             'cores': FLAGS.quota_cores,
             'volumes': FLAGS.quota_volumes,
@@ -57,7 +57,7 @@
     project_id = context.project.id
     used_instances, used_cores = db.instance_data_get_for_project(context,
                                                                   project_id)
-    quota = _get_quota(context, project_id)
+    quota = get_quota(context, project_id)
     allowed_instances = quota['instances'] - used_instances
     allowed_cores = quota['cores'] - used_cores
     type_cores = instance_types.INSTANCE_TYPES[instance_type]['vcpus']
@@ -72,9 +72,10 @@
     project_id = context.project.id
     used_volumes, used_gigabytes = db.volume_data_get_for_project(context,
                                                                   project_id)
-    quota = _get_quota(context, project_id)
+    quota = get_quota(context, project_id)
     allowed_volumes = quota['volumes'] - used_volumes
     allowed_gigabytes = quota['gigabytes'] - used_gigabytes
+    size = int(size)
     num_gigabytes = num_volumes * size
     allowed_volumes = min(allowed_volumes,
                           int(allowed_gigabytes // size))
@@ -85,7 +86,7 @@
     """Check quota and return min(num_floating_ips, allowed_floating_ips)"""
     project_id = context.project.id
     used_floating_ips = db.floating_ip_count_by_project(context, project_id)
-    quota = _get_quota(context, project_id)
+    quota = get_quota(context, project_id)
     allowed_floating_ips = quota['floating_ips'] - used_floating_ips
     return min(num_floating_ips, allowed_floating_ips)