From 3a48c1ad3cd39283ed9e26ff0ca2b7986a2aabea Mon Sep 17 00:00:00 2001 From: Dan Smith Date: Thu, 14 Mar 2013 13:34:30 -0400 Subject: [PATCH] Make conductor's quota methods pass project_id properly Commit 652a487ed9daba9ae97f7df77ae35720322d1af3 added a workaround for project_id on conductor's quota methods. This fully plumbs project_id through to the manager (and thus the quota implementation) and removes the tests that verify the workaround. Fixes bug 1153795 Change-Id: I7cd5e3de39acfaba7831892aff6c754569c8e1bf --- nova/conductor/api.py | 34 +++++---------------- nova/conductor/manager.py | 10 +++--- nova/conductor/rpcapi.py | 15 +++++---- nova/tests/compute/test_compute.py | 22 +++++++++----- nova/tests/conductor/test_conductor.py | 42 ++++---------------------- 5 files changed, 42 insertions(+), 81 deletions(-) diff --git a/nova/conductor/api.py b/nova/conductor/api.py index a8a6e9f53852..1825a9e7e621 100644 --- a/nova/conductor/api.py +++ b/nova/conductor/api.py @@ -324,22 +324,12 @@ class LocalAPI(object): migration) def quota_commit(self, context, reservations, project_id=None): - # FIXME(comstud): bug 1153795: Conductor manager should accept - # a project_id kwarg to be able to pass to the quota commit call. - if project_id is None: - project_id = context.project_id - with utils.temporary_mutation(context, project_id=project_id): - return self._manager.quota_commit(context, - reservations=reservations) + return self._manager.quota_commit(context, reservations, + project_id=project_id) def quota_rollback(self, context, reservations, project_id=None): - # FIXME(comstud): bug 1153795: Conductor manager should accept - # a project_id kwarg to be able to pass to the quota rollback call. - if project_id is None: - project_id = context.project_id - with utils.temporary_mutation(context, project_id=project_id): - return self._manager.quota_rollback(context, - reservations=reservations) + return self._manager.quota_rollback(context, reservations, + project_id=project_id) def get_ec2_ids(self, context, instance): return self._manager.get_ec2_ids(context, instance) @@ -669,20 +659,12 @@ class API(object): migration) def quota_commit(self, context, reservations, project_id=None): - # FIXME(comstud): bug 1153795: Conductor manager should accept - # a project_id kwarg to be able to pass to the quota commit call. - if project_id is None: - project_id = context.project_id - with utils.temporary_mutation(context, project_id=project_id): - return self.conductor_rpcapi.quota_commit(context, reservations) + return self.conductor_rpcapi.quota_commit(context, reservations, + project_id=project_id) def quota_rollback(self, context, reservations, project_id=None): - # FIXME(comstud): bug 1153795: Conductor manager should accept - # a project_id kwarg to be able to pass to the quota rollback call. - if project_id is None: - project_id = context.project_id - with utils.temporary_mutation(context, project_id=project_id): - return self.conductor_rpcapi.quota_rollback(context, reservations) + return self.conductor_rpcapi.quota_rollback(context, reservations, + project_id=project_id) def get_ec2_ids(self, context, instance): return self.conductor_rpcapi.get_ec2_ids(context, instance) diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index 5acd7b678664..71b08bad4f0c 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -49,7 +49,7 @@ datetime_fields = ['launched_at', 'terminated_at', 'updated_at'] class ConductorManager(manager.Manager): """Mission: TBD.""" - RPC_API_VERSION = '1.44' + RPC_API_VERSION = '1.45' def __init__(self, *args, **kwargs): super(ConductorManager, self).__init__(*args, **kwargs) @@ -398,11 +398,11 @@ class ConductorManager(manager.Manager): def network_migrate_instance_finish(self, context, instance, migration): self.network_api.migrate_instance_finish(context, instance, migration) - def quota_commit(self, context, reservations): - quota.QUOTAS.commit(context, reservations) + def quota_commit(self, context, reservations, project_id=None): + quota.QUOTAS.commit(context, reservations, project_id=project_id) - def quota_rollback(self, context, reservations): - quota.QUOTAS.rollback(context, reservations) + def quota_rollback(self, context, reservations, project_id=None): + quota.QUOTAS.rollback(context, reservations, project_id=project_id) def get_ec2_ids(self, context, instance): ec2_ids = {} diff --git a/nova/conductor/rpcapi.py b/nova/conductor/rpcapi.py index 6c0705e8b305..22b1b039fb35 100644 --- a/nova/conductor/rpcapi.py +++ b/nova/conductor/rpcapi.py @@ -82,6 +82,7 @@ class ConductorAPI(nova.openstack.common.rpc.proxy.RpcProxy): 1.42 - Added get_ec2_ids, aggregate_metadata_get_by_host 1.43 - Added compute_stop 1.44 - Added compute_node_delete + 1.45 - Added project_id to quota_commit and quota_rollback """ BASE_RPC_API_VERSION = '1.0' @@ -414,15 +415,17 @@ class ConductorAPI(nova.openstack.common.rpc.proxy.RpcProxy): instance=instance_p, migration=migration_p) return self.call(context, msg, version='1.41') - def quota_commit(self, context, reservations): + def quota_commit(self, context, reservations, project_id=None): reservations_p = jsonutils.to_primitive(reservations) - msg = self.make_msg('quota_commit', reservations=reservations_p) - return self.call(context, msg, version='1.41') + msg = self.make_msg('quota_commit', reservations=reservations_p, + project_id=project_id) + return self.call(context, msg, version='1.45') - def quota_rollback(self, context, reservations): + def quota_rollback(self, context, reservations, project_id=None): reservations_p = jsonutils.to_primitive(reservations) - msg = self.make_msg('quota_rollback', reservations=reservations_p) - return self.call(context, msg, version='1.41') + msg = self.make_msg('quota_rollback', reservations=reservations_p, + project_id=project_id) + return self.call(context, msg, version='1.45') def get_ec2_ids(self, context, instance): instance_p = jsonutils.to_primitive(instance) diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py index 3fdd79731022..aa58e799900f 100644 --- a/nova/tests/compute/test_compute.py +++ b/nova/tests/compute/test_compute.py @@ -2065,25 +2065,31 @@ class ComputeTestCase(BaseTestCase): for operation in actions: self._test_state_revert(instance, *operation) - def _ensure_quota_reservations_committed(self): + def _ensure_quota_reservations_committed(self, expect_project=False): """Mock up commit of quota reservations.""" reservations = list('fake_res') self.mox.StubOutWithMock(nova.quota.QUOTAS, 'commit') - nova.quota.QUOTAS.commit(mox.IgnoreArg(), reservations) + nova.quota.QUOTAS.commit(mox.IgnoreArg(), reservations, + project_id=(expect_project and + self.context.project_id or + None)) self.mox.ReplayAll() return reservations - def _ensure_quota_reservations_rolledback(self): + def _ensure_quota_reservations_rolledback(self, expect_project=False): """Mock up rollback of quota reservations.""" reservations = list('fake_res') self.mox.StubOutWithMock(nova.quota.QUOTAS, 'rollback') - nova.quota.QUOTAS.rollback(mox.IgnoreArg(), reservations) + nova.quota.QUOTAS.rollback(mox.IgnoreArg(), reservations, + project_id=(expect_project and + self.context.project_id or + None)) self.mox.ReplayAll() return reservations def test_quotas_succesful_delete(self): instance = jsonutils.to_primitive(self._create_fake_instance()) - resvs = self._ensure_quota_reservations_committed() + resvs = self._ensure_quota_reservations_committed(True) self.compute.terminate_instance(self.context, instance, bdms=None, reservations=resvs) @@ -2096,7 +2102,7 @@ class ComputeTestCase(BaseTestCase): self.stubs.Set(self.compute, '_shutdown_instance', fake_shutdown_instance) - resvs = self._ensure_quota_reservations_rolledback() + resvs = self._ensure_quota_reservations_rolledback(True) self.assertRaises(test.TestingException, self.compute.terminate_instance, self.context, instance, @@ -2105,7 +2111,7 @@ class ComputeTestCase(BaseTestCase): def test_quotas_succesful_soft_delete(self): instance = jsonutils.to_primitive(self._create_fake_instance( params=dict(task_state=task_states.SOFT_DELETING))) - resvs = self._ensure_quota_reservations_committed() + resvs = self._ensure_quota_reservations_committed(True) self.compute.soft_delete_instance(self.context, instance, reservations=resvs) @@ -2119,7 +2125,7 @@ class ComputeTestCase(BaseTestCase): self.stubs.Set(self.compute.driver, 'soft_delete', fake_soft_delete) - resvs = self._ensure_quota_reservations_rolledback() + resvs = self._ensure_quota_reservations_rolledback(True) self.assertRaises(test.TestingException, self.compute.soft_delete_instance, self.context, instance, diff --git a/nova/tests/conductor/test_conductor.py b/nova/tests/conductor/test_conductor.py index dd779c778a14..df0f5e73b5c4 100644 --- a/nova/tests/conductor/test_conductor.py +++ b/nova/tests/conductor/test_conductor.py @@ -546,15 +546,19 @@ class _BaseTestCase(object): def test_quota_commit(self): self.mox.StubOutWithMock(quota.QUOTAS, 'commit') - quota.QUOTAS.commit(self.context, 'reservations') + quota.QUOTAS.commit(self.context, 'reservations', project_id=None) + quota.QUOTAS.commit(self.context, 'reservations', project_id='proj') self.mox.ReplayAll() self.conductor.quota_commit(self.context, 'reservations') + self.conductor.quota_commit(self.context, 'reservations', 'proj') def test_quota_rollback(self): self.mox.StubOutWithMock(quota.QUOTAS, 'rollback') - quota.QUOTAS.rollback(self.context, 'reservations') + quota.QUOTAS.rollback(self.context, 'reservations', project_id=None) + quota.QUOTAS.rollback(self.context, 'reservations', project_id='proj') self.mox.ReplayAll() self.conductor.quota_rollback(self.context, 'reservations') + self.conductor.quota_rollback(self.context, 'reservations', 'proj') def test_get_ec2_ids(self): expected = { @@ -1067,40 +1071,6 @@ class ConductorAPITestCase(_BaseTestCase, test.TestCase): self.conductor.security_groups_trigger_handler(self.context, 'event', 'arg') - def test_quota_commit_with_project_id(self): - diff_proj_id = 'diff_fake_proj_id' - self.assertNotEqual(self.context.project_id, diff_proj_id) - call_info = {} - - def mgr_quota_commit(ctxt, reservations): - call_info['resvs'] = reservations - call_info['project_id'] = ctxt.project_id - - self.stubs.Set(self.conductor_manager, 'quota_commit', - mgr_quota_commit) - - self.conductor.quota_commit(self.context, 'fake_resvs', - project_id=diff_proj_id) - self.assertEqual(diff_proj_id, call_info['project_id']) - self.assertEqual('fake_resvs', call_info['resvs']) - - def test_quota_rollback_with_project_id(self): - diff_proj_id = 'diff_fake_proj_id' - self.assertNotEqual(self.context.project_id, diff_proj_id) - call_info = {} - - def mgr_quota_rollback(ctxt, reservations): - call_info['resvs'] = reservations - call_info['project_id'] = ctxt.project_id - - self.stubs.Set(self.conductor_manager, 'quota_rollback', - mgr_quota_rollback) - - self.conductor.quota_rollback(self.context, 'fake_resvs', - project_id=diff_proj_id) - self.assertEqual(diff_proj_id, call_info['project_id']) - self.assertEqual('fake_resvs', call_info['resvs']) - class ConductorLocalAPITestCase(ConductorAPITestCase): """Conductor LocalAPI Tests."""