openstack/nova
Code Issues Proposed changes

Fix wrap_exception to get all arguments for payload

Browse Source 
The wrap_exception decorator that attempts to send a notification when
exceptions occur was not sending all the arguments it was intending to.
It relies on getcallargs to get the arguments and argument names for the
called method but if the method has another decorator on it getcallargs
pulls information for the decorator rather than the decorated method.
This pulls the decorated method with get_wrapped_function and then calls
getcallargs.

get_wrapped_function was moved to safeutils because utils.py can't be
imported by exception.py without a circular import.

A few tests were updated to include the id on the instance object used.
This was done because serialization of the object includes the
instance.name field which assumes that id is set to populate the
CONF.instance_name_template.  When id is not set it triggers a lazy load
which fails in the test environment.

Change-Id: I87d8691a2aae6f3555177364f3c40a490a6f7591
Closes-bug: 1525282
This commit is contained in:
Andrew Laski 2015-12-10 17:34:09 -05:00
parent 2910d75b28
commit 1b757c328d
8 changed files with 90 additions and 84 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
nova/compute/manager.py
View File

@ -287,7 +287,7 @@ def errors_out_migration(function):
return function(self, context, *args, **kwargs)
except Exception as ex:
with excutils.save_and_reraise_exception():
wrapped_func = utils.get_wrapped_function(function)
wrapped_func = safe_utils.get_wrapped_function(function)
keyed_args = safe_utils.getcallargs(wrapped_func, self,
context, *args, **kwargs)
migration = keyed_args['migration']
@ -329,7 +329,7 @@ def reverts_task_state(function):
e.format_message())
except Exception:
with excutils.save_and_reraise_exception():
wrapped_func = utils.get_wrapped_function(function)
wrapped_func = safe_utils.get_wrapped_function(function)
keyed_args = safe_utils.getcallargs(wrapped_func, self,
context, *args, **kwargs)
# NOTE(mriedem): 'instance' must be in keyed_args because we
@ -389,7 +389,7 @@ def wrap_instance_event(function):
@functools.wraps(function)
def decorated_function(self, context, *args, **kwargs):
wrapped_func = utils.get_wrapped_function(function)
wrapped_func = safe_utils.get_wrapped_function(function)
keyed_args = safe_utils.getcallargs(wrapped_func, self, context, *args,
**kwargs)
instance_uuid = keyed_args['instance']['uuid']

6
nova/exception.py
View File

@ -90,8 +90,10 @@ def wrap_exception(notifier=None, get_notifier=None):
with excutils.save_and_reraise_exception():
if notifier or get_notifier:
payload = dict(exception=e)
call_dict = safe_utils.getcallargs(f, self, context,
*args, **kw)
wrapped_func = safe_utils.get_wrapped_function(f)
call_dict = safe_utils.getcallargs(wrapped_func, self,
context, *args,
**kw)
# self can't be serialized and shouldn't be in the
# payload
call_dict.pop('self', None)

21
nova/safe_utils.py
View File

@ -49,3 +49,24 @@ def getcallargs(function, *args, **kwargs):
keyed_args[argname] = value
return keyed_args
def get_wrapped_function(function):
"""Get the method at the bottom of a stack of decorators."""
if not hasattr(function, '__closure__') or not function.__closure__:
return function
def _get_wrapped_function(function):
if not hasattr(function, '__closure__') or not function.__closure__:
return None
for closure in function.__closure__:
func = closure.cell_contents
deeper_func = _get_wrapped_function(func)
if deeper_func:
return deeper_func
elif hasattr(closure.cell_contents, '__call__'):
return closure.cell_contents
return _get_wrapped_function(function)

6
nova/tests/unit/compute/test_compute.py
View File

@ -9458,6 +9458,7 @@ class ComputeAPITestCase(BaseTestCase):
def test_attach_interface_failed(self):
new_type = flavors.get_flavor_by_flavor_id('4')
instance = objects.Instance(
id=42,
uuid='f0000000-0000-0000-0000-000000000000',
image_ref='foo',
system_metadata={},
@ -9500,7 +9501,7 @@ class ComputeAPITestCase(BaseTestCase):
def test_detach_interface_failed(self):
nwinfo, port_id = self.test_attach_interface()
instance = objects.Instance()
instance = objects.Instance(id=42)
instance['uuid'] = 'f6000000-0000-0000-0000-000000000000'
instance.info_cache = objects.InstanceInfoCache.new(
self.context, 'f6000000-0000-0000-0000-000000000000')
@ -9524,7 +9525,7 @@ class ComputeAPITestCase(BaseTestCase):
# Tests that when deallocate_port_for_instance fails we log the failure
# before exiting compute.detach_interface.
nwinfo, port_id = self.test_attach_interface()
instance = objects.Instance(uuid=uuidutils.generate_uuid())
instance = objects.Instance(id=42, uuid=uuidutils.generate_uuid())
instance.info_cache = objects.InstanceInfoCache.new(
self.context, 'f6000000-0000-0000-0000-000000000000')
instance.info_cache.network_info = network_model.NetworkInfo.hydrate(
@ -9558,6 +9559,7 @@ class ComputeAPITestCase(BaseTestCase):
block_device_obj.BlockDeviceMapping(),
fake_bdm)
instance = self._create_fake_instance_obj()
instance.id = 42
fake_volume = {'id': 'fake-volume-id'}
with test.nested(

54
nova/tests/unit/test_safeutils.py
View File

@ -12,6 +12,8 @@
# License for the specific language governing permissions and limitations
# under the License.
import functools
from nova import safe_utils
from nova import test
@ -108,3 +110,55 @@ class GetCallArgsTestCase(test.NoDBTestCase):
self.assertEqual(1, len(callargs))
self.assertIn('instance', callargs)
self.assertEqual({'uuid': 1}, callargs['instance'])
class WrappedCodeTestCase(test.NoDBTestCase):
"""Test the get_wrapped_function utility method."""
def _wrapper(self, function):
@functools.wraps(function)
def decorated_function(self, *args, **kwargs):
function(self, *args, **kwargs)
return decorated_function
def test_single_wrapped(self):
@self._wrapper
def wrapped(self, instance, red=None, blue=None):
pass
func = safe_utils.get_wrapped_function(wrapped)
func_code = func.__code__
self.assertEqual(4, len(func_code.co_varnames))
self.assertIn('self', func_code.co_varnames)
self.assertIn('instance', func_code.co_varnames)
self.assertIn('red', func_code.co_varnames)
self.assertIn('blue', func_code.co_varnames)
def test_double_wrapped(self):
@self._wrapper
@self._wrapper
def wrapped(self, instance, red=None, blue=None):
pass
func = safe_utils.get_wrapped_function(wrapped)
func_code = func.__code__
self.assertEqual(4, len(func_code.co_varnames))
self.assertIn('self', func_code.co_varnames)
self.assertIn('instance', func_code.co_varnames)
self.assertIn('red', func_code.co_varnames)
self.assertIn('blue', func_code.co_varnames)
def test_triple_wrapped(self):
@self._wrapper
@self._wrapper
@self._wrapper
def wrapped(self, instance, red=None, blue=None):
pass
func = safe_utils.get_wrapped_function(wrapped)
func_code = func.__code__
self.assertEqual(4, len(func_code.co_varnames))
self.assertIn('self', func_code.co_varnames)
self.assertIn('instance', func_code.co_varnames)
self.assertIn('red', func_code.co_varnames)
self.assertIn('blue', func_code.co_varnames)

53
nova/tests/unit/test_utils.py
View File

@ -13,7 +13,6 @@
# under the License.
import datetime
import functools
import hashlib
import importlib
import logging
@ -847,58 +846,6 @@ class MetadataToDictTestCase(test.NoDBTestCase):
self.assertEqual(utils.dict_to_metadata({}), [])
class WrappedCodeTestCase(test.NoDBTestCase):
"""Test the get_wrapped_function utility method."""
def _wrapper(self, function):
@functools.wraps(function)
def decorated_function(self, *args, **kwargs):
function(self, *args, **kwargs)
return decorated_function
def test_single_wrapped(self):
@self._wrapper
def wrapped(self, instance, red=None, blue=None):
pass
func = utils.get_wrapped_function(wrapped)
func_code = func.__code__
self.assertEqual(4, len(func_code.co_varnames))
self.assertIn('self', func_code.co_varnames)
self.assertIn('instance', func_code.co_varnames)
self.assertIn('red', func_code.co_varnames)
self.assertIn('blue', func_code.co_varnames)
def test_double_wrapped(self):
@self._wrapper
@self._wrapper
def wrapped(self, instance, red=None, blue=None):
pass
func = utils.get_wrapped_function(wrapped)
func_code = func.__code__
self.assertEqual(4, len(func_code.co_varnames))
self.assertIn('self', func_code.co_varnames)
self.assertIn('instance', func_code.co_varnames)
self.assertIn('red', func_code.co_varnames)
self.assertIn('blue', func_code.co_varnames)
def test_triple_wrapped(self):
@self._wrapper
@self._wrapper
@self._wrapper
def wrapped(self, instance, red=None, blue=None):
pass
func = utils.get_wrapped_function(wrapped)
func_code = func.__code__
self.assertEqual(4, len(func_code.co_varnames))
self.assertIn('self', func_code.co_varnames)
self.assertIn('instance', func_code.co_varnames)
self.assertIn('red', func_code.co_varnames)
self.assertIn('blue', func_code.co_varnames)
class ExpectedArgsTestCase(test.NoDBTestCase):
def test_passes(self):
@utils.expects_func_args('foo', 'baz')

4
nova/tests/unit/virt/hyperv/test_driver.py
View File

@ -23,9 +23,9 @@ import mock
from os_win import exceptions as os_win_exc
from nova import exception
from nova import safe_utils
from nova.tests.unit import fake_instance
from nova.tests.unit.virt.hyperv import test_base
from nova import utils
from nova.virt import driver as base_driver
from nova.virt.hyperv import driver
@ -66,7 +66,7 @@ class HyperVDriverTestCase(test_base.HyperVBaseTestCase):
if callable(class_member):
mocked_method = mock.patch.object(
driver.HyperVDriver, attr,
utils.get_wrapped_function(class_member))
safe_utils.get_wrapped_function(class_member))
mocked_method.start()
self.addCleanup(mocked_method.stop)

24
nova/utils.py
View File

@ -57,6 +57,7 @@ from six.moves import range
from nova import exception
from nova.i18n import _, _LE, _LI, _LW
from nova import safe_utils
notify_decorator = 'nova.notifications.notify_decorator'
@ -1041,32 +1042,11 @@ def instance_sys_meta(instance):
include_deleted=True)
def get_wrapped_function(function):
"""Get the method at the bottom of a stack of decorators."""
if not hasattr(function, '__closure__') or not function.__closure__:
return function
def _get_wrapped_function(function):
if not hasattr(function, '__closure__') or not function.__closure__:
return None
for closure in function.__closure__:
func = closure.cell_contents
deeper_func = _get_wrapped_function(func)
if deeper_func:
return deeper_func
elif hasattr(closure.cell_contents, '__call__'):
return closure.cell_contents
return _get_wrapped_function(function)
def expects_func_args(*args):
def _decorator_checker(dec):
@functools.wraps(dec)
def _decorator(f):
base_f = get_wrapped_function(f)
base_f = safe_utils.get_wrapped_function(f)
arg_names, a, kw, _default = inspect.getargspec(base_f)
if a or kw or set(args) <= set(arg_names):
# NOTE (ndipanov): We can't really tell if correct stuff will