diff --git a/nova/objects/instance_action.py b/nova/objects/instance_action.py index 0e798839259e..a3f504607256 100644 --- a/nova/objects/instance_action.py +++ b/nova/objects/instance_action.py @@ -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 diff --git a/nova/tests/unit/api/openstack/compute/test_instance_actions.py b/nova/tests/unit/api/openstack/compute/test_instance_actions.py index 144a78f5e0e1..1dcabdf61be9 100644 --- a/nova/tests/unit/api/openstack/compute/test_instance_actions.py +++ b/nova/tests/unit/api/openstack/compute/test_instance_actions.py @@ -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: diff --git a/nova/tests/unit/fake_server_actions.py b/nova/tests/unit/fake_server_actions.py index 707f4e50d99d..8f7299a36a71 100644 --- a/nova/tests/unit/fake_server_actions.py +++ b/nova/tests/unit/fake_server_actions.py @@ -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 } ] } diff --git a/nova/tests/unit/objects/test_instance_action.py b/nova/tests/unit/objects/test_instance_action.py index bcc5c001881c..6576f60b9963 100644 --- a/nova/tests/unit/objects/test_instance_action.py +++ b/nova/tests/unit/objects/test_instance_action.py @@ -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): diff --git a/nova/tests/unit/objects/test_objects.py b/nova/tests/unit/objects/test_objects.py index e97809ca8392..b5bd0f32414e 100644 --- a/nova/tests/unit/objects/test_objects.py +++ b/nova/tests/unit/objects/test_objects.py @@ -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',