Merge "Avoid leaking security group quota reservations"

2 files changed, 38 insertions, 5 deletions

diff --git a/nova/compute/api.py b/nova/compute/api.py
index 6ce5b5526..58044ed7f 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -2110,13 +2110,14 @@ class SecurityGroupAPI(base.Base):
 
         LOG.audit(_("Create Security Group %s"), name, context=context)
 
-        self.ensure_default(context)
+        try:
+            self.ensure_default(context)
 
-        if self.db.security_group_exists(context, context.project_id, name):
-            msg = _('Security group %s already exists') % name
-            self.raise_group_already_exists(msg)
+            if self.db.security_group_exists(context,
+                                             context.project_id, name):
+                msg = _('Security group %s already exists') % name
+                self.raise_group_already_exists(msg)
 
-        try:
             group = {'user_id': context.user_id,
                      'project_id': context.project_id,
                      'name': name,
diff --git a/nova/tests/api/openstack/compute/contrib/test_security_groups.py b/nova/tests/api/openstack/compute/contrib/test_security_groups.py
index f977fc04e..f32b688d9 100644
--- a/nova/tests/api/openstack/compute/contrib/test_security_groups.py
+++ b/nova/tests/api/openstack/compute/contrib/test_security_groups.py
@@ -29,6 +29,7 @@ import nova.db
 from nova import exception
 from nova import flags
 from nova.openstack.common import jsonutils
+from nova import quota
 from nova import test
 from nova.tests.api.openstack import fakes
 
@@ -121,6 +122,11 @@ class TestSecurityGroups(test.TestCase):
             security_groups.ServerSecurityGroupController())
         self.manager = security_groups.SecurityGroupActionController()
 
+    def _assert_no_security_groups_reserved(self, context):
+        """Check that no reservations are leaked during tests."""
+        result = quota.QUOTAS.get_project_quotas(context, context.project_id)
+        self.assertEqual(result['security_groups']['reserved'], 0)
+
     def test_create_security_group(self):
         sg = security_group_template()
 
@@ -138,6 +144,8 @@ class TestSecurityGroups(test.TestCase):
         self.assertRaises(webob.exc.HTTPUnprocessableEntity,
                           self.controller.create, req, sg)
 
+        self._assert_no_security_groups_reserved(req.environ['nova.context'])
+
     def test_create_security_group_with_no_description(self):
         sg = security_group_template()
         del sg['description']
@@ -146,6 +154,8 @@ class TestSecurityGroups(test.TestCase):
         self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create,
                           req, {'security_group': sg})
 
+        self._assert_no_security_groups_reserved(req.environ['nova.context'])
+
     def test_create_security_group_with_blank_name(self):
         sg = security_group_template(name='')
 
@@ -153,6 +163,8 @@ class TestSecurityGroups(test.TestCase):
         self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create,
                           req, {'security_group': sg})
 
+        self._assert_no_security_groups_reserved(req.environ['nova.context'])
+
     def test_create_security_group_with_whitespace_name(self):
         sg = security_group_template(name=' ')
 
@@ -160,6 +172,8 @@ class TestSecurityGroups(test.TestCase):
         self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create,
                           req, {'security_group': sg})
 
+        self._assert_no_security_groups_reserved(req.environ['nova.context'])
+
     def test_create_security_group_with_blank_description(self):
         sg = security_group_template(description='')
 
@@ -167,6 +181,8 @@ class TestSecurityGroups(test.TestCase):
         self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create,
                           req, {'security_group': sg})
 
+        self._assert_no_security_groups_reserved(req.environ['nova.context'])
+
     def test_create_security_group_with_whitespace_description(self):
         sg = security_group_template(description=' ')
 
@@ -174,6 +190,8 @@ class TestSecurityGroups(test.TestCase):
         self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create,
                           req, {'security_group': sg})
 
+        self._assert_no_security_groups_reserved(req.environ['nova.context'])
+
     def test_create_security_group_with_duplicate_name(self):
         sg = security_group_template()
 
@@ -185,11 +203,15 @@ class TestSecurityGroups(test.TestCase):
         self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create,
                           req, {'security_group': sg})
 
+        self._assert_no_security_groups_reserved(req.environ['nova.context'])
+
     def test_create_security_group_with_no_body(self):
         req = fakes.HTTPRequest.blank('/v2/fake/os-security-groups')
         self.assertRaises(webob.exc.HTTPUnprocessableEntity,
                           self.controller.create, req, None)
 
+        self._assert_no_security_groups_reserved(req.environ['nova.context'])
+
     def test_create_security_group_with_no_security_group(self):
         body = {'no-securityGroup': None}
 
@@ -197,6 +219,8 @@ class TestSecurityGroups(test.TestCase):
         self.assertRaises(webob.exc.HTTPUnprocessableEntity,
                           self.controller.create, req, body)
 
+        self._assert_no_security_groups_reserved(req.environ['nova.context'])
+
     def test_create_security_group_above_255_characters_name(self):
         sg = security_group_template(name='1234567890' * 26)
 
@@ -204,6 +228,8 @@ class TestSecurityGroups(test.TestCase):
         self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create,
                           req, {'security_group': sg})
 
+        self._assert_no_security_groups_reserved(req.environ['nova.context'])
+
     def test_create_security_group_above_255_characters_description(self):
         sg = security_group_template(description='1234567890' * 26)
 
@@ -211,6 +237,8 @@ class TestSecurityGroups(test.TestCase):
         self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create,
                           req, {'security_group': sg})
 
+        self._assert_no_security_groups_reserved(req.environ['nova.context'])
+
     def test_create_security_group_non_string_name(self):
         sg = security_group_template(name=12)
 
@@ -218,6 +246,8 @@ class TestSecurityGroups(test.TestCase):
         self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create,
                           req, {'security_group': sg})
 
+        self._assert_no_security_groups_reserved(req.environ['nova.context'])
+
     def test_create_security_group_non_string_description(self):
         sg = security_group_template(description=12)
 
@@ -225,6 +255,8 @@ class TestSecurityGroups(test.TestCase):
         self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create,
                           req, {'security_group': sg})
 
+        self._assert_no_security_groups_reserved(req.environ['nova.context'])
+
     def test_create_security_group_quota_limit(self):
         req = fakes.HTTPRequest.blank('/v2/fake/os-security-groups')
         for num in range(1, FLAGS.quota_security_groups + 1):