openstack
/
nova
Code Issues Proposed changes

Store instance action event exc_val fault details 

The InstanceActionEvent.pack_action_event_finish method was
not storing the exc_val in a field that was actually part
of the data model so it was never stored ("message" isn't a
column in the instance_actions_events table, "details" is).

This formats and stores the exc_val, if provided, using the
same utility code that is used to record the message for
an instance fault record.

Eventually we can build on this in the os-instance-actions API
by exposing the fault details to the user without needing to
expose the traceback (like the server fault "details" traceback).

Co-Authored-By: Brin Zhang <zhangbailin@inspur.com>

Part of blueprint action-event-fault-details

Change-Id: Ie3e11b38aac251c3f8911ed57dc5e7503aa91301
This commit is contained in:
Matt Riedemann 2019-11-14 18:27:01 -05:00 committed by Brin Zhang
parent a9346e6033
commit a81d27beba
5 changed files with 66 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
nova/objects/instance_action.py
View File

@ -14,7 +14,9 @@
from oslo_utils import timeutils
from oslo_utils import versionutils
import six
from nova.compute import utils as compute_utils
from nova.db import api as db
from nova import exception
from nova import objects
@ -138,7 +140,8 @@ class InstanceActionEvent(base.NovaPersistentObject, base.NovaObject,
# Version 1.1: event_finish_with_failure decorated with serialize_args
# Version 1.2: Add 'host' field
# Version 1.3: Add create() method.
VERSION = '1.3'
# Version 1.4: Added 'details' field.
VERSION = '1.4'
fields = {
'id': fields.IntegerField(),
'event': fields.StringField(nullable=True),
@ -148,10 +151,13 @@ class InstanceActionEvent(base.NovaPersistentObject, base.NovaObject,
'result': fields.StringField(nullable=True),
'traceback': fields.StringField(nullable=True),
'host': fields.StringField(nullable=True),
'details': fields.StringField(nullable=True)
}
def obj_make_compatible(self, primitive, target_version):
target_version = versionutils.convert_version_to_tuple(target_version)
if target_version < (1, 4) and 'details' in primitive:
del primitive['details']
if target_version < (1, 2) and 'host' in primitive:
del primitive['host']
@ -184,18 +190,18 @@ class InstanceActionEvent(base.NovaPersistentObject, base.NovaObject,
values['result'] = 'Success'
else:
values['result'] = 'Error'
# FIXME(mriedem): message is not used. The instance_actions_events
# table has a "details" column but not a "message" column which
# means the exc_val is never stored in the record. So far it does
# not matter because the exc_val is not exposed out of the API,
# but we should consider storing at least the exception type so
# we could expose that to non-admin owners of a server in the API
# since then they could see something like NoValidHost to know why
# the operation failed. Note by default policy non-admins are not
# allowed to see the traceback field. If we expose exc_val somehow
# we might consider re-using logic from exception_to_dict which
# is used to store an instance fault message.
values['message'] = exc_val
# Store the details using the same logic as storing an instance
# fault message.
if exc_val:
# If we got a string for exc_val it's probably because of
# the serialize_args decorator on event_finish_with_failure
# so pass that as the message to exception_to_dict otherwise
# the details will just the exception class name since it
# cannot format the message as a NovaException.
message = (
exc_val if isinstance(exc_val, six.string_types) else None)
values['details'] = compute_utils.exception_to_dict(
exc_val, message=message)['message']
values['traceback'] = exc_tb
return values

2
nova/tests/unit/api/openstack/compute/test_instance_actions.py
View File

@ -65,7 +65,7 @@ def format_event(event, project_id, expect_traceback=True, expect_host=False,
expect_hostId=False):
'''Remove keys that aren't serialized.'''
to_delete = ['id', 'created_at', 'updated_at', 'deleted_at', 'deleted',
'action_id']
'action_id', 'details']
if not expect_traceback:
to_delete.append('traceback')
if not expect_host:

9
nova/tests/unit/fake_server_actions.py
View File

@ -73,7 +73,8 @@ FAKE_EVENTS = {
'deleted_at': None,
'deleted': False,
'host': 'host1',
'hostId': FAKE_HOST_ID1
'hostId': FAKE_HOST_ID1,
'details': None
},
{'id': 2,
'action_id': FAKE_ACTION_ID1,
@ -89,7 +90,8 @@ FAKE_EVENTS = {
'deleted_at': None,
'deleted': False,
'host': 'host1',
'hostId': FAKE_HOST_ID1
'hostId': FAKE_HOST_ID1,
'details': None
}
],
FAKE_ACTION_ID2: [{'id': 3,
@ -106,7 +108,8 @@ FAKE_EVENTS = {
'deleted_at': None,
'deleted': False,
'host': 'host2',
'hostId': FAKE_HOST_ID2
'hostId': FAKE_HOST_ID2,
'details': None
}
]
}

47
nova/tests/unit/objects/test_instance_action.py
View File

@ -12,6 +12,7 @@
# License for the specific language governing permissions and limitations
# under the License.
import copy
import traceback
import mock
@ -22,6 +23,7 @@ import six
from nova.db import api as db
from nova import exception
from nova import objects
from nova.objects import instance_action
from nova import test
from nova.tests.unit.objects import test_objects
@ -56,6 +58,7 @@ fake_event = {
'result': 'fake-result',
'traceback': 'fake-tb',
'host': 'fake-host',
'details': None
}
@ -250,6 +253,7 @@ class _TestInstanceActionEventObject(object):
expected_packed_values = test_class.pack_action_event_finish(
self.context, 'fake-uuid', 'fake-event')
expected_packed_values['finish_time'] = NOW
self.assertNotIn('details', expected_packed_values)
mock_finish.return_value = fake_event
event = instance_action.InstanceActionEvent.event_finish(
self.context, 'fake-uuid', 'fake-event', want_result=True)
@ -276,17 +280,24 @@ class _TestInstanceActionEventObject(object):
def test_event_finish_with_failure(self, mock_finish, mock_tb):
self.useFixture(utils_fixture.TimeFixture(NOW))
test_class = instance_action.InstanceActionEvent
# The NovaException message will get formatted for the 'details' field.
exc_val = exception.NoValidHost(reason='some error')
expected_packed_values = test_class.pack_action_event_finish(
self.context, 'fake-uuid', 'fake-event', 'val', 'fake-tb')
self.context, 'fake-uuid', 'fake-event', exc_val, 'fake-tb')
expected_packed_values['finish_time'] = NOW
self.assertEqual(exc_val.format_message(),
expected_packed_values['details'])
mock_finish.return_value = fake_event
fake_event_with_details = copy.deepcopy(fake_event)
fake_event_with_details['details'] = expected_packed_values['details']
mock_finish.return_value = fake_event_with_details
event = test_class.event_finish_with_failure(
self.context, 'fake-uuid', 'fake-event', 'val', 'fake-tb',
self.context, 'fake-uuid', 'fake-event', exc_val, 'fake-tb',
want_result=True)
mock_finish.assert_called_once_with(self.context,
expected_packed_values)
self.compare_obj(event, fake_event)
self.compare_obj(event, fake_event_with_details)
mock_tb.assert_not_called()
@mock.patch.object(traceback, 'format_tb')
@mock.patch.object(db, 'action_event_finish')
@ -295,18 +306,27 @@ class _TestInstanceActionEventObject(object):
mock_tb.return_value = 'fake-tb'
self.useFixture(utils_fixture.TimeFixture(NOW))
test_class = instance_action.InstanceActionEvent
# A non-NovaException will use the exception class name for
# the 'details' field.
exc_val = test.TestingException('non-nova-error')
expected_packed_values = test_class.pack_action_event_finish(
self.context, 'fake-uuid', 'fake-event', 'val', 'fake-tb')
self.context, 'fake-uuid', 'fake-event', exc_val, 'fake-tb')
expected_packed_values['finish_time'] = NOW
self.assertEqual('TestingException', expected_packed_values['details'])
mock_finish.return_value = fake_event
fake_event_with_details = copy.deepcopy(fake_event)
fake_event_with_details['details'] = expected_packed_values['details']
mock_finish.return_value = fake_event_with_details
fake_tb = mock.sentinel.fake_tb
event = test_class.event_finish_with_failure(
self.context, 'fake-uuid', 'fake-event', exc_val='val',
self.context, 'fake-uuid', 'fake-event', exc_val=exc_val,
exc_tb=fake_tb, want_result=True)
# When calling event_finish_with_failure and using exc_val as a kwarg
# serialize_args will convert exc_val to non-nova exception class name
# form before it reaches event_finish_with_failure.
mock_finish.assert_called_once_with(self.context,
expected_packed_values)
self.compare_obj(event, fake_event)
self.compare_obj(event, fake_event_with_details)
mock_tb.assert_called_once_with(fake_tb)
@mock.patch.object(db, 'action_event_finish')
@ -398,6 +418,17 @@ class _TestInstanceActionEventObject(object):
self.context, expected_updates)
self.compare_obj(event, fake_event)
def test_obj_make_compatible(self):
action_event_obj = objects.InstanceActionEvent(
details=None, # added in 1.4
host='fake-host' # added in 1.2
)
data = lambda x: x['nova_object.data']
primitive = data(action_event_obj.obj_to_primitive(
target_version='1.3'))
self.assertIn('host', primitive)
self.assertNotIn('details', primitive)
class TestInstanceActionEventObject(test_objects._LocalTest,
_TestInstanceActionEventObject):

2
nova/tests/unit/objects/test_objects.py
View File

@ -1080,7 +1080,7 @@ object_data = {
'ImageMetaProps': '1.25-66fc973af215eb5701ed4034bb6f0685',
'Instance': '2.7-d187aec68cad2e4d8b8a03a68e4739ce',
'InstanceAction': '1.2-9a5abc87fdd3af46f45731960651efb5',
'InstanceActionEvent': '1.3-c749e1b3589e7117c81cb2aa6ac438d5',
'InstanceActionEvent': '1.4-5b1f361bd81989f8bb2c20bb7e8a4cb4',
'InstanceActionEventList': '1.1-13d92fb953030cdbfee56481756e02be',
'InstanceActionList': '1.1-a2b2fb6006b47c27076d3a1d48baa759',
'InstanceDeviceMetadata': '1.0-74d78dd36aa32d26d2769a1b57caf186',