Merge "Fix inconsistent quota usage for security group"

nova/compute/api.py

@@ -3664,11 +3664,13 @@ def destroy(self, context, security_group):
             msg = _("Security group is still in use")
             self.raise_invalid_group(msg)
 
-        # Get reservations
+        quotas = quotas_obj.Quotas()
+        quota_project, quota_user = quotas_obj.ids_from_security_group(
+                                context, security_group)
         try:
-            reservations = QUOTAS.reserve(context, security_groups=-1)
+            quotas.reserve(context, project_id=quota_project,
+                           user_id=quota_user, security_groups=-1)
         except Exception:
-            reservations = None
             LOG.exception(_("Failed to update usages deallocating "
                             "security group"))
 
@@ -3677,8 +3679,7 @@ def destroy(self, context, security_group):
         self.db.security_group_destroy(context, security_group['id'])
 
         # Commit the reservations
-        if reservations:
-            QUOTAS.commit(context, reservations)
+        quotas.commit()
 
     def is_associated_with_server(self, security_group, instance_uuid):
         """Check if the security group is already associated

nova/objects/quotas.py

@@ -31,6 +31,13 @@ def ids_from_instance(context, instance):
     return project_id, user_id
 
 
+# TODO(lyj): This method needs to be cleaned up once the
+# ids_from_instance helper method is renamed or some common
+# method is added for objects.quotas.
+def ids_from_security_group(context, security_group):
+    return ids_from_instance(context, security_group)
+
+
 class Quotas(base.NovaObject):
     fields = {
         'reservations': fields.ListOfStringsField(nullable=True),

nova/tests/api/openstack/compute/contrib/test_neutron_security_groups.py

@@ -165,6 +165,12 @@ def test_delete_security_group_by_id(self):
                                       sg['id'])
         self.controller.delete(req, sg['id'])
 
+    def test_delete_security_group_by_admin(self):
+        sg = self._create_sg_template().get('security_group')
+        req = fakes.HTTPRequest.blank('/v2/fake/os-security-groups/%s' %
+                                      sg['id'], use_admin_context=True)
+        self.controller.delete(req, sg['id'])
+
     def test_delete_security_group_in_use(self):
         sg = self._create_sg_template().get('security_group')
         self._create_network()

nova/tests/api/openstack/compute/contrib/test_security_groups.py

@@ -23,6 +23,7 @@
 from nova.api.openstack import xmlutil
 from nova import compute
 from nova.compute import power_state
+from nova import context as context_maker
 import nova.db
 from nova import exception
 from nova.objects import instance as instance_obj
@@ -140,6 +141,11 @@ def _assert_no_security_groups_reserved(self, context):
         result = quota.QUOTAS.get_project_quotas(context, context.project_id)
         self.assertEqual(result['security_groups']['reserved'], 0)
 
+    def _assert_security_groups_in_use(self, project_id, user_id, in_use):
+        context = context_maker.get_admin_context()
+        result = quota.QUOTAS.get_user_quotas(context, project_id, user_id)
+        self.assertEqual(result['security_groups']['in_use'], in_use)
+
     def test_create_security_group(self):
         sg = security_group_template()
 
@@ -465,7 +471,8 @@ def test_update_default_security_group_fail(self):
                           req, '1', {'security_group': sg})
 
     def test_delete_security_group_by_id(self):
-        sg = security_group_template(id=1, rules=[])
+        sg = security_group_template(id=1, project_id='fake_project',
+                                     user_id='fake_user', rules=[])
 
         self.called = False
 
@@ -486,6 +493,26 @@ def return_security_group(context, group_id):
 
         self.assertTrue(self.called)
 
+    def test_delete_security_group_by_admin(self):
+        sg = security_group_template(id=2, rules=[])
+
+        req = fakes.HTTPRequest.blank('/v2/fake/os-security-groups')
+        self.controller.create(req, {'security_group': sg})
+        context = req.environ['nova.context']
+
+        # Ensure quota usage for security group is correct.
+        self._assert_security_groups_in_use(context.project_id,
+                                            context.user_id, 2)
+
+        # Delete the security group by admin.
+        req = fakes.HTTPRequest.blank('/v2/fake/os-security-groups/2',
+                                      use_admin_context=True)
+        self.controller.delete(req, '2')
+
+        # Ensure quota for security group in use is released.
+        self._assert_security_groups_in_use(context.project_id,
+                                            context.user_id, 1)
+
     def test_delete_security_group_by_invalid_id(self):
         req = fakes.HTTPRequest.blank('/v2/fake/os-security-groups/invalid')
         self.assertRaises(webob.exc.HTTPBadRequest, self.controller.delete,