From 4a1650eea23c3b8a064721a2985d481218f04500 Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Thu, 26 Jul 2012 17:32:03 -0400 Subject: Make instance_uuid backwards compat actually work. In some refactoring of the decorator modifications I made for eebc64f949ccb2acb7462efc18f538f1827985af, I ended up breaking the case where an instance_uuid was passed to one of the methods that is supposed to be able to accept either an instance or instance_uuid. Oops. :-( This patch makes them work again, and adds one unit test that verifies that it works. These decorators are really ugly right now, but keep in mind that the 2-version stuff is temporary. As soon as this patch series is finished (changing methods to take an instance), these decorators will get simplified. Part of blueprint no-db-messaging. Change-Id: Ic1b1269d90e147035c4eb7b4bd47de81eff3ffdc --- nova/compute/manager.py | 43 ++++++++++++++++++-------------------- nova/tests/compute/test_compute.py | 7 +++++++ 2 files changed, 27 insertions(+), 23 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 7e555b010..d25405ce3 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -172,8 +172,8 @@ def checks_instance_lock(function): # as a transition from when every function expected a context # and instance_uuid as positional arguments to where everything is a kwarg, # and the function may get either an instance_uuid or an instance. - def _checks_instance_lock_core(self, cb, context, instance_uuid, - *args, **kwargs): + def _checks_instance_lock_core(self, cb, context, *args, **kwargs): + instance_uuid = kwargs['instance_uuid'] locked = self._get_lock(context, instance_uuid) admin = context.is_admin @@ -193,23 +193,22 @@ def checks_instance_lock(function): def decorated_function(self, context, instance_uuid, *args, **kwargs): def _cb(self, context, *args, **kwargs): + instance_uuid = kwargs.pop('instance_uuid') function(self, context, instance_uuid, *args, **kwargs) - return _checks_instance_lock_core(self, _cb, context, instance_uuid, - *args, **kwargs) + kwargs['instance_uuid'] = instance_uuid + + return _checks_instance_lock_core(self, _cb, context, *args, **kwargs) @functools.wraps(function) def decorated_function_new(self, context, *args, **kwargs): - try: - instance_uuid = kwargs['instance_uuid'] - except KeyError: - instance_uuid = kwargs['instance']['uuid'] + if 'instance_uuid' not in kwargs: + kwargs['instance_uuid'] = kwargs['instance']['uuid'] def _cb(self, context, *args, **kwargs): function(self, context, *args, **kwargs) - return _checks_instance_lock_core(self, _cb, context, instance_uuid, - *args, **kwargs) + return _checks_instance_lock_core(self, _cb, context, *args, **kwargs) expected_args = ['context', 'instance_uuid'] argspec = inspect.getargspec(function) @@ -232,38 +231,36 @@ def wrap_instance_fault(function): # as a transition from when every function expected a context # and instance_uuid as positional arguments to where everything is a kwarg, # and the function may get either an instance_uuid or an instance. - def _wrap_instance_fault_core(self, cb, context, instance_uuid, - *args, **kwargs): + def _wrap_instance_fault_core(self, cb, context, *args, **kwargs): try: return cb(self, context, *args, **kwargs) except exception.InstanceNotFound: raise except Exception, e: with excutils.save_and_reraise_exception(): - self.add_instance_fault_from_exc(context, instance_uuid, e, - sys.exc_info()) + self.add_instance_fault_from_exc(context, + kwargs['instance_uuid'], e, sys.exc_info()) @functools.wraps(function) def decorated_function(self, context, instance_uuid, *args, **kwargs): - def _cb(self, context, *args, **_kwargs): + def _cb(self, context, *args, **kwargs): + instance_uuid = kwargs.pop('instance_uuid') return function(self, context, instance_uuid, *args, **kwargs) - return _wrap_instance_fault_core(self, _cb, context, instance_uuid, - *args, **kwargs) + kwargs['instance_uuid'] = instance_uuid + + return _wrap_instance_fault_core(self, _cb, context, *args, **kwargs) @functools.wraps(function) def decorated_function_new(self, context, *args, **kwargs): - if 'instance_uuid' in kwargs: - instance_uuid = kwargs['instance_uuid'] - else: - instance_uuid = kwargs['instance']['uuid'] + if 'instance_uuid' not in kwargs: + kwargs['instance_uuid'] = kwargs['instance']['uuid'] def _cb(self, context, *args, **kwargs): return function(self, context, *args, **kwargs) - return _wrap_instance_fault_core(self, _cb, context, instance_uuid, - *args, **kwargs) + return _wrap_instance_fault_core(self, _cb, context, *args, **kwargs) expected_args = ['context', 'instance_uuid'] argspec = inspect.getargspec(function) diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py index 9fcb004a4..75df22f8a 100644 --- a/nova/tests/compute/test_compute.py +++ b/nova/tests/compute/test_compute.py @@ -805,9 +805,16 @@ class ComputeTestCase(BaseTestCase): instance = jsonutils.to_primitive(self._create_fake_instance()) self.compute.run_instance(self.context, instance['uuid']) + # Try with the full instance console = self.compute.get_vnc_console(self.context, 'novnc', instance=instance) self.assert_(console) + + # Also make sure it still works with just the instance UUID + console = self.compute.get_vnc_console(self.context, 'novnc', + instance_uuid=instance['uuid']) + self.assert_(console) + self.compute.terminate_instance(self.context, instance['uuid']) def test_xvpvnc_vnc_console(self): -- cgit