Fix inconsistent quota usage for security group

The quota usage for security group do not decrease when the security
group is deleted by admin. Fixes the issue by using the new quotas
object.

Change-Id: If82ee4edeca934c7be360e4129baa12b1c9a4da1
Closes-bug: 1260575

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,