From 79fe4d8e076c9c7bb76f0afb1b2787d51b2c5037 Mon Sep 17 00:00:00 2001 From: Rui Chen Date: Tue, 30 Jun 2015 20:34:32 +0800 Subject: [PATCH] Fix incomplete error message of quota exceeded When we boot and resize instance, if multiple requested resource(core, ram and instances) exceeded quota, only the detail of core resource will been outputed to user in the exception, the loss of core and instances number will make end user have no idea which flavor can be used to boot instance successfully. Fix this issue and update related test cases. Change-Id: I969d73e2f222278ea8a2bb4c21474c13ab213d81 Closes-Bug: #1469942 --- nova/compute/api.py | 48 +++++++++------- nova/exception.py | 2 +- .../openstack/compute/test_migrate_server.py | 2 +- nova/tests/unit/api/openstack/fakes.py | 5 +- nova/tests/unit/compute/test_compute_api.py | 55 +++++++++++++++++++ nova/tests/unit/test_quota.py | 18 ++++-- 6 files changed, 98 insertions(+), 32 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 32e8065eb4cf..8c9e002e65ea 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -429,10 +429,12 @@ class API(base.Base): msg = (_("Can only run %s more instances of this type.") % allowed) - resource = overs[0] - used = quotas[resource] - headroom[resource] - total_allowed = quotas[resource] - overs = ','.join(overs) + num_instances = (str(min_count) if min_count == max_count else + "%s-%s" % (min_count, max_count)) + requested = dict(instances=num_instances, cores=req_cores, + ram=req_ram) + (overs, reqs, total_alloweds, useds) = self._get_over_quota_detail( + headroom, overs, quotas, requested) params = {'overs': overs, 'pid': context.project_id, 'min_count': min_count, 'max_count': max_count, 'msg': msg} @@ -446,18 +448,25 @@ class API(base.Base): " tried to run between %(min_count)d and" " %(max_count)d instances. %(msg)s"), params) - - num_instances = (str(min_count) if min_count == max_count else - "%s-%s" % (min_count, max_count)) - requested = dict(instances=num_instances, cores=req_cores, - ram=req_ram) raise exception.TooManyInstances(overs=overs, - req=requested[resource], - used=used, allowed=total_allowed, - resource=resource) + req=reqs, + used=useds, + allowed=total_alloweds) return max_count, quotas + def _get_over_quota_detail(self, headroom, overs, quotas, requested): + reqs = [] + useds = [] + total_alloweds = [] + for resource in overs: + reqs.append(str(requested[resource])) + useds.append(str(quotas[resource] - headroom[resource])) + total_alloweds.append(str(quotas[resource])) + (overs, reqs, useds, total_alloweds) = map(', '.join, ( + overs, reqs, useds, total_alloweds)) + return overs, reqs, total_alloweds, useds + def _check_metadata_properties_quota(self, context, metadata=None): """Enforce quota limits on metadata properties.""" if not metadata: @@ -2636,19 +2645,16 @@ class API(base.Base): overs = exc.kwargs['overs'] usages = exc.kwargs['usages'] headroom = self._get_headroom(quotas, usages, deltas) - - resource = overs[0] - used = quotas[resource] - headroom[resource] - total_allowed = used + headroom[resource] - overs = ','.join(overs) + (overs, reqs, total_alloweds, + useds) = self._get_over_quota_detail(headroom, overs, quotas, + deltas) LOG.warning(_LW("%(overs)s quota exceeded for %(pid)s," " tried to resize instance."), {'overs': overs, 'pid': context.project_id}) raise exception.TooManyInstances(overs=overs, - req=deltas[resource], - used=used, - allowed=total_allowed, - resource=resource) + req=reqs, + used=useds, + allowed=total_alloweds) else: quotas = objects.Quotas(context=context) diff --git a/nova/exception.py b/nova/exception.py index 855bfe8ee2d9..f6da014149d6 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -1305,7 +1305,7 @@ class QuotaError(NovaException): class TooManyInstances(QuotaError): msg_fmt = _("Quota exceeded for %(overs)s: Requested %(req)s," - " but already used %(used)d of %(allowed)d %(resource)s") + " but already used %(used)s of %(allowed)s %(overs)s") class FloatingIpLimitExceeded(QuotaError): diff --git a/nova/tests/unit/api/openstack/compute/test_migrate_server.py b/nova/tests/unit/api/openstack/compute/test_migrate_server.py index 34027e861180..f56dfd0f2af1 100644 --- a/nova/tests/unit/api/openstack/compute/test_migrate_server.py +++ b/nova/tests/unit/api/openstack/compute/test_migrate_server.py @@ -111,7 +111,7 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests): def test_migrate_too_many_instances(self): exc_info = exception.TooManyInstances(overs='', req='', used=0, - allowed=0, resource='') + allowed=0) self._test_migrate_exception(exc_info, webob.exc.HTTPForbidden) def _test_migrate_live_succeeded(self, param): diff --git a/nova/tests/unit/api/openstack/fakes.py b/nova/tests/unit/api/openstack/fakes.py index 456df189b5eb..ac3f79f033ed 100644 --- a/nova/tests/unit/api/openstack/fakes.py +++ b/nova/tests/unit/api/openstack/fakes.py @@ -171,9 +171,8 @@ def stub_out_instance_quota(stubs, allowed, quota, resource='instances'): usages = dict(instances=dict(in_use=0, reserved=0), cores=dict(in_use=0, reserved=0), ram=dict(in_use=0, reserved=0)) - usages[resource]['in_use'] = (quotas[resource] * 0.9 - - allowed) - usages[resource]['reserved'] = quotas[resource] * 0.1 + usages[resource]['in_use'] = (quotas[resource] * 9 // 10 - allowed) + usages[resource]['reserved'] = quotas[resource] // 10 raise exc.OverQuota(overs=[resource], quotas=quotas, usages=usages) stubs.Set(QUOTAS, 'reserve', fake_reserve) diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index 381c19ed57f9..fc3e461b0e9d 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -1574,6 +1574,61 @@ class _ComputeAPIUnitTestMixIn(object): fake_inst, flavor_id='flavor-id') self.assertFalse(mock_save.called) + def test_check_instance_quota_exceeds_with_multiple_resources(self): + quotas = {'cores': 1, 'instances': 1, 'ram': 512} + usages = {'cores': dict(in_use=1, reserved=0), + 'instances': dict(in_use=1, reserved=0), + 'ram': dict(in_use=512, reserved=0)} + overs = ['cores', 'instances', 'ram'] + over_quota_args = dict(quotas=quotas, + usages=usages, + overs=overs) + e = exception.OverQuota(**over_quota_args) + fake_flavor = self._create_flavor() + instance_num = 1 + with mock.patch.object(objects.Quotas, 'reserve', side_effect=e): + try: + self.compute_api._check_num_instances_quota(self.context, + fake_flavor, + instance_num, + instance_num) + except exception.TooManyInstances as e: + self.assertEqual('cores, instances, ram', e.kwargs['overs']) + self.assertEqual('1, 1, 512', e.kwargs['req']) + self.assertEqual('1, 1, 512', e.kwargs['used']) + self.assertEqual('1, 1, 512', e.kwargs['allowed']) + else: + self.fail("Exception not raised") + + @mock.patch.object(flavors, 'get_flavor_by_flavor_id') + @mock.patch.object(objects.Quotas, 'reserve') + def test_resize_instance_quota_exceeds_with_multiple_resources( + self, mock_reserve, mock_get_flavor): + quotas = {'cores': 1, 'ram': 512} + usages = {'cores': dict(in_use=1, reserved=0), + 'ram': dict(in_use=512, reserved=0)} + overs = ['cores', 'ram'] + over_quota_args = dict(quotas=quotas, + usages=usages, + overs=overs) + + mock_reserve.side_effect = exception.OverQuota(**over_quota_args) + mock_get_flavor.return_value = self._create_flavor(id=333, + vcpus=3, + memory_mb=1536) + try: + self.compute_api.resize(self.context, self._create_instance_obj(), + 'fake_flavor_id') + except exception.TooManyInstances as e: + self.assertEqual('cores, ram', e.kwargs['overs']) + self.assertEqual('2, 1024', e.kwargs['req']) + self.assertEqual('1, 512', e.kwargs['used']) + self.assertEqual('1, 512', e.kwargs['allowed']) + mock_get_flavor.assert_called_once_with('fake_flavor_id', + read_deleted="no") + else: + self.fail("Exception not raised") + def test_pause(self): # Ensure instance can be paused. instance = self._create_instance_obj() diff --git a/nova/tests/unit/test_quota.py b/nova/tests/unit/test_quota.py index 4195adb8acc0..6534cf260475 100644 --- a/nova/tests/unit/test_quota.py +++ b/nova/tests/unit/test_quota.py @@ -87,9 +87,12 @@ class QuotaIntegrationTestCase(test.TestCase): instance_type=inst_type, image_href=image_uuid) except exception.QuotaError as e: - expected_kwargs = {'code': 413, 'resource': 'cores', 'req': 1, - 'used': 4, 'allowed': 4, 'overs': 'cores,instances'} - self.assertEqual(e.kwargs, expected_kwargs) + expected_kwargs = {'code': 413, + 'req': '1, 1', + 'used': '4, 2', + 'allowed': '4, 2', + 'overs': 'cores, instances'} + self.assertEqual(expected_kwargs, e.kwargs) else: self.fail('Expected QuotaError exception') for instance_uuid in instance_uuids: @@ -104,9 +107,12 @@ class QuotaIntegrationTestCase(test.TestCase): instance_type=inst_type, image_href=image_uuid) except exception.QuotaError as e: - expected_kwargs = {'code': 413, 'resource': 'cores', 'req': 1, - 'used': 4, 'allowed': 4, 'overs': 'cores'} - self.assertEqual(e.kwargs, expected_kwargs) + expected_kwargs = {'code': 413, + 'req': '1', + 'used': '4', + 'allowed': '4', + 'overs': 'cores'} + self.assertEqual(expected_kwargs, e.kwargs) else: self.fail('Expected QuotaError exception') db.instance_destroy(self.context, instance['uuid'])