diff --git a/nova/compute/api.py b/nova/compute/api.py index 9f35dea42f84..1fa848e5540a 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -4797,12 +4797,8 @@ class KeypairAPI(base.Base): raise exception.InvalidKeypair( reason=_('Keypair name must be string and between ' '1 and 255 characters long')) - - count = objects.Quotas.count_as_dict(context, 'key_pairs', user_id) - count_value = count['user']['key_pairs'] - try: - objects.Quotas.limit_check(context, key_pairs=count_value + 1) + objects.Quotas.check_deltas(context, {'key_pairs': 1}, user_id) except exception.OverQuota: raise exception.KeypairLimitExceeded() @@ -4854,6 +4850,18 @@ class KeypairAPI(base.Base): keypair.fingerprint = fingerprint keypair.public_key = public_key keypair.create() + + # NOTE(melwitt): We recheck the quota after creating the object to + # prevent users from allocating more resources than their allowed quota + # in the event of a race. This is configurable because it can be + # expensive if strict quota limits are not required in a deployment. + if CONF.quota.recheck_quota: + try: + objects.Quotas.check_deltas(context, {'key_pairs': 0}, user_id) + except exception.OverQuota: + keypair.destroy() + raise exception.KeypairLimitExceeded() + compute_utils.notify_about_keypair_action( context=context, keypair=keypair, diff --git a/nova/tests/unit/api/openstack/compute/test_keypairs.py b/nova/tests/unit/api/openstack/compute/test_keypairs.py index 01dd19967021..63b5343d467f 100644 --- a/nova/tests/unit/api/openstack/compute/test_keypairs.py +++ b/nova/tests/unit/api/openstack/compute/test_keypairs.py @@ -181,13 +181,10 @@ class KeypairsTestV21(test.TestCase): self.assertNotIn('private_key', res_dict['keypair']) self._assert_keypair_type(res_dict) - def test_keypair_import_quota_limit(self): - - def fake_quotas_count(self, context, resource, *args, **kwargs): - return {'user': {'key_pairs': 100}} - - self.stubs.Set(QUOTAS, "count_as_dict", fake_quotas_count) - + @mock.patch('nova.objects.Quotas.check_deltas') + def test_keypair_import_quota_limit(self, mock_check): + mock_check.side_effect = exception.OverQuota(overs='key_pairs', + usages={'key_pairs': 100}) body = { 'keypair': { 'name': 'create_test', @@ -207,13 +204,10 @@ class KeypairsTestV21(test.TestCase): self.controller.create, self.req, body=body) self.assertIn('Quota exceeded, too many key pairs.', ex.explanation) - def test_keypair_create_quota_limit(self): - - def fake_quotas_count(self, context, resource, *args, **kwargs): - return {'user': {'key_pairs': 100}} - - self.stubs.Set(QUOTAS, "count_as_dict", fake_quotas_count) - + @mock.patch('nova.objects.Quotas.check_deltas') + def test_keypair_create_quota_limit(self, mock_check): + mock_check.side_effect = exception.OverQuota(overs='key_pairs', + usages={'key_pairs': 100}) body = { 'keypair': { 'name': 'create_test', @@ -224,6 +218,50 @@ class KeypairsTestV21(test.TestCase): self.controller.create, self.req, body=body) self.assertIn('Quota exceeded, too many key pairs.', ex.explanation) + @mock.patch('nova.objects.Quotas.check_deltas') + def test_keypair_create_over_quota_during_recheck(self, mock_check): + # Simulate a race where the first check passes and the recheck fails. + # First check occurs in compute/api. + exc = exception.OverQuota(overs='key_pairs', usages={'key_pairs': 100}) + mock_check.side_effect = [None, exc] + body = { + 'keypair': { + 'name': 'create_test', + }, + } + + self.assertRaises(webob.exc.HTTPForbidden, + self.controller.create, self.req, body=body) + + ctxt = self.req.environ['nova.context'] + self.assertEqual(2, mock_check.call_count) + call1 = mock.call(ctxt, {'key_pairs': 1}, ctxt.user_id) + call2 = mock.call(ctxt, {'key_pairs': 0}, ctxt.user_id) + mock_check.assert_has_calls([call1, call2]) + + # Verify we removed the key pair that was added after the first + # quota check passed. + key_pairs = objects.KeyPairList.get_by_user(ctxt, ctxt.user_id) + names = [key_pair.name for key_pair in key_pairs] + self.assertNotIn('create_test', names) + + @mock.patch('nova.objects.Quotas.check_deltas') + def test_keypair_create_no_quota_recheck(self, mock_check): + # Disable recheck_quota. + self.flags(recheck_quota=False, group='quota') + + body = { + 'keypair': { + 'name': 'create_test', + }, + } + self.controller.create(self.req, body=body) + + ctxt = self.req.environ['nova.context'] + # check_deltas should have been called only once. + mock_check.assert_called_once_with(ctxt, {'key_pairs': 1}, + ctxt.user_id) + def test_keypair_create_duplicate(self): self.stub_out("nova.objects.KeyPair.create", db_key_pair_create_duplicate)