Explicit user messages

Use 'action', 'resource', 'detail' to replace 'event'
in user messages.

APIImpact
DocImpact

Partial-Implements: blueprint better-user-message
Change-Id: I8a635a07ed6ff93ccb71df8c404c927d1ecef005

cinder/api/v3/messages.py

@@ -21,6 +21,7 @@ from cinder.api.openstack import wsgi
 from cinder.api.v3.views import messages as messages_view
 from cinder.message import api as message_api
 from cinder.message import defined_messages
+from cinder.message import message_field
 import cinder.policy
 
 
@@ -48,6 +49,19 @@ class MessagesController(wsgi.Controller):
         self.ext_mgr = ext_mgr
         super(MessagesController, self).__init__()
 
+    def _build_user_message(self, message):
+        # NOTE(tommylikehu): if the `action_id` is empty, we use 'event_id'
+        # to translate the user message.
+        if message is None:
+            return
+        if message['action_id'] is None and message['event_id'] is not None:
+            message['user_message'] = defined_messages.get_message_text(
+                message['event_id'])
+        else:
+            message['user_message'] = "%s:%s" % (
+                message_field.translate_action(message['action_id']),
+                message_field.translate_detail(message['detail_id']))
+
     @wsgi.Controller.api_version(MESSAGES_BASE_MICRO_VERSION)
     def show(self, req, id):
         """Return the given message."""
@@ -58,10 +72,7 @@ class MessagesController(wsgi.Controller):
 
         check_policy(context, 'get', message)
 
-        # Fetches message text based on event id passed to it.
+        self._build_user_message(message)
-        message['user_message'] = defined_messages.get_message_text(
-            message['event_id'])
-
         return self._view_builder.detail(req, message)
 
     @wsgi.Controller.api_version(MESSAGES_BASE_MICRO_VERSION)
@@ -107,11 +118,7 @@ class MessagesController(wsgi.Controller):
                                             sort_dirs=sort_dirs)
 
         for message in messages:
-            # Fetches message text based on event id passed to it.
+            self._build_user_message(message)
-            user_message = defined_messages.get_message_text(
-                message['event_id'])
-            message['user_message'] = user_message
-
         messages = self._view_builder.index(req, messages)
         return messages
 

cinder/db/sqlalchemy/api.py

@@ -6194,6 +6194,8 @@ def _translate_message(message):
         'resource_type': message['resource_type'],
         'resource_uuid': message.get('resource_uuid'),
         'event_id': message['event_id'],
+        'detail_id': message['detail_id'],
+        'action_id': message['action_id'],
         'message_level': message['message_level'],
         'created_at': message['created_at'],
         'expires_at': message.get('expires_at'),

cinder/db/sqlalchemy/migrate_repo/versions/103_message_action_detail_column.py

@@ -10,6 +10,14 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-"""Resource type constants."""
+from sqlalchemy import Column, String, MetaData, Table
 
-VOLUME = 'VOLUME'
+
+def upgrade(migrate_engine):
+    meta = MetaData(migrate_engine)
+
+    messages = Table('messages', meta, autoload=True)
+    detail_id = Column('detail_id', String(10), nullable=True)
+    action_id = Column('action_id', String(10), nullable=True)
+    messages.create_column(detail_id)
+    messages.create_column(action_id)

cinder/db/sqlalchemy/models.py

@@ -822,6 +822,10 @@ class Message(BASE, CinderBase):
     resource_uuid = Column(String(36), nullable=True)
     # Operation specific event ID.
     event_id = Column(String(255), nullable=False)
+    # Message detail ID.
+    detail_id = Column(String(10), nullable=True)
+    # Operation specific action.
+    action_id = Column(String(10), nullable=True)
     # After this time the message may no longer exist
     expires_at = Column(DateTime, nullable=True)
 

cinder/message/api.py

@@ -19,7 +19,7 @@ from oslo_log import log as logging
 from oslo_utils import timeutils
 
 from cinder.db import base
-from cinder.message import defined_messages
+from cinder.message import message_field
 
 
 messages_opts = [
@@ -40,23 +40,27 @@ LOG = logging.getLogger(__name__)
 class API(base.Base):
     """API for handling user messages."""
 
-    def create(self, context, event_id, project_id, resource_type=None,
+    def create(self, context, action,
-               resource_uuid=None, level="ERROR"):
+               resource_type=message_field.Resource.VOLUME,
+               resource_uuid=None, exception=None, detail=None, level="ERROR"):
         """Create a message with the specified information."""
         LOG.info("Creating message record for request_id = %s",
                  context.request_id)
-        # Ensure valid event_id
-        defined_messages.get_message_text(event_id)
         # Updates expiry time for message as per message_ttl config.
         expires_at = (timeutils.utcnow() + datetime.timedelta(
                       seconds=CONF.message_ttl))
 
-        message_record = {'project_id': project_id,
+        detail_id = message_field.translate_detail_id(exception, detail)
+        message_record = {'project_id': context.project_id,
                           'request_id': context.request_id,
                           'resource_type': resource_type,
                           'resource_uuid': resource_uuid,
-                          'event_id': event_id,
+                          'action_id': action[0] if action else '',
                           'message_level': level,
+                          'event_id': "VOLUME_%s_%s_%s" % (resource_type,
+                                                           action[0],
+                                                           detail_id),
+                          'detail_id': detail_id,
                           'expires_at': expires_at}
         try:
             self.db.message_create(context, message_record)

cinder/message/message_field.py

@@ -0,0 +1,98 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+"""Message Resource, Action, Detail and user visible message.
+
+Use Resource, Action and Detail's combination to indicate the Event
+in the format of:
+    EVENT: VOLUME_RESOURCE_ACTION_DETAIL
+Also, use exception-to-detail mapping to decrease the workload of
+classifying event in cinder's task code.
+"""
+
+from oslo_versionedobjects import fields
+
+from cinder.i18n import _
+
+
+class Resource(fields.Enum):
+
+    VOLUME = 'VOLUME'
+
+
+class Action(fields.Enum):
+
+    SCHEDULE_ALLOCATE_VOLUME = ('001', _('schedule allocate volume'))
+    ATTACH_VOLUME = ('002', _('attach volume'))
+    COPY_VOLUME_TO_IMAGE = ('003', _('copy volume to image'))
+    UPDATE_ATTACHMENT = ('004', _('update attachment'))
+
+    ALL = (SCHEDULE_ALLOCATE_VOLUME,
+           ATTACH_VOLUME,
+           COPY_VOLUME_TO_IMAGE,
+           UPDATE_ATTACHMENT)
+
+
+class Detail(fields.Enum):
+
+    UNKNOWN_ERROR = ('001', _('An unknown error occurred.'))
+    DRIVER_NOT_INITIALIZED = ('002',
+                              _('Driver is not initialized at present.'))
+    NO_BACKEND_AVAILABLE = ('003',
+                            _('Could not found any available '
+                              'weighted backend.'))
+    FAILED_TO_UPLOAD_VOLUME = ('004',
+                               _("Failed to upload volume to image "
+                                 "at backend."))
+    VOLUME_ATTACH_MODE_INVALID = ('005',
+                                  _("Volume's attach mode is invalid."))
+    QUOTA_EXCEED = ('006',
+                    _("Not enough quota resource for operation."))
+
+    ALL = (UNKNOWN_ERROR,
+           DRIVER_NOT_INITIALIZED,
+           NO_BACKEND_AVAILABLE,
+           FAILED_TO_UPLOAD_VOLUME,
+           VOLUME_ATTACH_MODE_INVALID,
+           QUOTA_EXCEED)
+
+    # Exception and detail mappings
+    EXCEPTION_DETAIL_MAPPINGS = {
+        DRIVER_NOT_INITIALIZED: ['DriverNotInitialized'],
+        NO_BACKEND_AVAILABLE: ['NoValidBackend'],
+        VOLUME_ATTACH_MODE_INVALID: ['InvalidVolumeAttachMode'],
+        QUOTA_EXCEED: ['ImageLimitExceeded',
+                       'BackupLimitExceeded',
+                       'SnapshotLimitExceeded']
+    }
+
+
+def translate_action(action_id):
+    action_message = next((action[1] for action in Action.ALL
+                           if action[0] == action_id), None)
+    return action_message or 'unknown action'
+
+
+def translate_detail(detail_id):
+    detail_message = next((action[1] for action in Detail.ALL
+                           if action[0] == detail_id), None)
+    return detail_message or Detail.UNKNOWN_ERROR[1]
+
+
+def translate_detail_id(exception, detail):
+    if exception is not None and isinstance(exception, Exception):
+        for key, value in Detail.EXCEPTION_DETAIL_MAPPINGS.items():
+            if exception.__class__.__name__ in value:
+                return key[0]
+    if detail in Detail.ALL:
+        return detail[0]
+    return Detail.UNKNOWN_ERROR[0]

cinder/scheduler/flows/create_volume.py

@@ -18,8 +18,7 @@ from taskflow.patterns import linear_flow
 from cinder import exception
 from cinder import flow_utils
 from cinder.message import api as message_api
-from cinder.message import defined_messages
+from cinder.message import message_field
-from cinder.message import resource_types
 from cinder import rpc
 from cinder import utils
 from cinder.volume.flows import common
@@ -122,19 +121,17 @@ class ScheduleCreateVolumeTask(flow_utils.CinderTask):
             self.driver_api.schedule_create_volume(context, request_spec,
                                                    filter_properties)
         except Exception as e:
+            self.message_api.create(
+                context,
+                message_field.Action.SCHEDULE_ALLOCATE_VOLUME,
+                resource_uuid=request_spec['volume_id'],
+                exception=e)
             # An error happened, notify on the scheduler queue and log that
             # this happened and set the volume to errored out and reraise the
             # error *if* exception caught isn't NoValidBackend. Otherwise *do
             # not* reraise (since what's the point?)
             with excutils.save_and_reraise_exception(
                     reraise=not isinstance(e, exception.NoValidBackend)):
-                if isinstance(e, exception.NoValidBackend):
-                    self.message_api.create(
-                        context,
-                        defined_messages.EventIds.UNABLE_TO_ALLOCATE,
-                        context.project_id,
-                        resource_type=resource_types.VOLUME,
-                        resource_uuid=request_spec['volume_id'])
                 try:
                     self._handle_failure(context, request_spec, e)
                 finally:

cinder/tests/unit/api/v3/fakes.py

@@ -13,7 +13,6 @@
 import datetime
 import iso8601
 
-from cinder.message import defined_messages
 from cinder.tests.unit import fake_constants as fake
 from cinder.tests.unit import fake_volume
 from cinder import utils
@@ -31,7 +30,9 @@ DEFAULT_AZ = "fakeaz"
 def fake_message(id, **kwargs):
     message = {
         'id': id,
-        'event_id': defined_messages.EventIds.UNABLE_TO_ALLOCATE,
+        'action_id': "002",
+        'detail_id': "001",
+        'event_id': "VOLUME_VOLUME_002_001",
         'message_level': "ERROR",
         'request_id': FAKE_UUID,
         'updated_at': datetime.datetime(1900, 1, 1, 1, 1, 1,

cinder/tests/unit/api/v3/stubs.py

@@ -13,7 +13,6 @@
 import datetime
 import iso8601
 
-from cinder.message import defined_messages
 from cinder.tests.unit import fake_constants as fake
 
 
@@ -23,7 +22,9 @@ FAKE_UUID = fake.OBJECT_ID
 def stub_message(id, **kwargs):
     message = {
         'id': id,
-        'event_id': defined_messages.EventIds.UNABLE_TO_ALLOCATE,
+        'action_id': "002",
+        'detail_id': "001",
+        'event_id': "VOLUME_VOLUME_002_001",
         'message_level': "ERROR",
         'request_id': FAKE_UUID,
         'updated_at': datetime.datetime(1900, 1, 1, 1, 1, 1,

cinder/tests/unit/api/v3/test_messages.py

@@ -19,7 +19,7 @@ from cinder.api.v3 import messages
 from cinder import context
 from cinder import exception
 from cinder.message import api as message_api
-from cinder.message import defined_messages
+from cinder.message import message_field
 from cinder import test
 from cinder.tests.unit.api import fakes
 from cinder.tests.unit.api.v3 import fakes as v3_fakes
@@ -50,8 +50,9 @@ class MessageApiTest(test.TestCase):
         return {
             'message': {
                 'id': message.get('id'),
-                'user_message': defined_messages.get_message_text(
+                'user_message': "%s:%s" % (
-                    message.get('event_id')),
+                    message_field.translate_action(message.get('action_id')),
+                    message_field.translate_detail(message.get('detail_id'))),
                 'request_id': message.get('request_id'),
                 'event_id': message.get('event_id'),
                 'created_at': message.get('created_at'),

cinder/tests/unit/message/test_api.py

@@ -20,7 +20,7 @@ from cinder.api.openstack import api_version_request as api_version
 from cinder.api.v3 import messages
 from cinder import context
 from cinder.message import api as message_api
-from cinder.message import defined_messages
+from cinder.message import message_field
 from cinder import test
 from cinder.tests.unit.api import fakes
 import cinder.tests.unit.fake_constants as fake_constants
@@ -53,13 +53,16 @@ class MessageApiTest(test.TestCase):
             'request_id': 'fakerequestid',
             'resource_type': 'fake_resource_type',
             'resource_uuid': None,
-            'event_id': defined_messages.EventIds.UNABLE_TO_ALLOCATE,
+            'action_id':
+                message_field.Action.SCHEDULE_ALLOCATE_VOLUME[0],
+            'detail_id': message_field.Detail.UNKNOWN_ERROR[0],
             'message_level': 'ERROR',
             'expires_at': expected_expires_at,
+            'event_id': "VOLUME_fake_resource_type_001_001",
         }
         self.message_api.create(self.ctxt,
-                                defined_messages.EventIds.UNABLE_TO_ALLOCATE,
+                                message_field.Action.SCHEDULE_ALLOCATE_VOLUME,
-                                "fakeproject",
+                                detail=message_field.Detail.UNKNOWN_ERROR,
                                 resource_type="fake_resource_type")
 
         self.message_api.db.message_create.assert_called_once_with(
@@ -70,20 +73,12 @@ class MessageApiTest(test.TestCase):
         self.mock_object(self.message_api.db, 'create',
                          side_effect=Exception())
         self.message_api.create(self.ctxt,
-                                defined_messages.EventIds.UNABLE_TO_ALLOCATE,
+                                message_field.Action.ATTACH_VOLUME,
-                                "fakeproject",
                                 "fake_resource")
 
         self.message_api.db.message_create.assert_called_once_with(
             self.ctxt, mock.ANY)
 
-    def test_create_does_not_allow_undefined_messages(self):
-        self.assertRaises(KeyError, self.message_api.create,
-                          self.ctxt,
-                          "FAKE_EVENT_ID",
-                          "fakeproject",
-                          "fake_resource")
-
     def test_get(self):
         self.message_api.get(self.ctxt, 'fake_id')
 
@@ -116,15 +111,15 @@ class MessageApiTest(test.TestCase):
     def create_message_for_tests(self):
         """Create messages to test pagination functionality"""
         utils.create_message(
-            self.ctxt, event_id=defined_messages.EventIds.UNKNOWN_ERROR)
+            self.ctxt, action=message_field.Action.ATTACH_VOLUME)
         utils.create_message(
-            self.ctxt, event_id=defined_messages.EventIds.UNABLE_TO_ALLOCATE)
+            self.ctxt, action=message_field.Action.SCHEDULE_ALLOCATE_VOLUME)
         utils.create_message(
             self.ctxt,
-            event_id=defined_messages.EventIds.ATTACH_READONLY_VOLUME)
+            action=message_field.Action.COPY_VOLUME_TO_IMAGE)
         utils.create_message(
             self.ctxt,
-            event_id=defined_messages.EventIds.IMAGE_FROM_VOLUME_OVER_QUOTA)
+            action=message_field.Action.COPY_VOLUME_TO_IMAGE)
 
     def test_get_all_messages_with_limit(self):
         self.create_message_for_tests()
@@ -196,8 +191,8 @@ class MessageApiTest(test.TestCase):
     def test_get_all_messages_with_filter(self):
         self.create_message_for_tests()
 
-        url = '/v3/messages?event_id=%s' % (
+        url = '/v3/messages?action_id=%s' % (
-            defined_messages.EventIds.UNKNOWN_ERROR)
+            message_field.Action.ATTACH_VOLUME[0])
         req = fakes.HTTPRequest.blank(url)
         req.method = 'GET'
         req.content_type = 'application/json'
@@ -222,10 +217,10 @@ class MessageApiTest(test.TestCase):
         res = self.controller.index(req)
 
         expect_result = [
-            defined_messages.EventIds.UNKNOWN_ERROR,
+            "VOLUME_VOLUME_001_002",
-            defined_messages.EventIds.UNABLE_TO_ALLOCATE,
+            "VOLUME_VOLUME_002_002",
-            defined_messages.EventIds.IMAGE_FROM_VOLUME_OVER_QUOTA,
+            "VOLUME_VOLUME_003_002",
-            defined_messages.EventIds.ATTACH_READONLY_VOLUME
+            "VOLUME_VOLUME_003_002",
         ]
         expect_result.sort()
 

cinder/tests/unit/message/test_message_field.py

@@ -0,0 +1,63 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import ddt
+
+from oslo_config import cfg
+
+from cinder import exception
+from cinder.message import message_field
+from cinder import test
+
+CONF = cfg.CONF
+
+
+@ddt.ddt
+class MessageFieldTest(test.TestCase):
+
+    @ddt.data({'id': '001', 'content': 'schedule allocate volume'},
+              {'id': '002', 'content': 'attach volume'},
+              {'id': 'invalid', 'content': None})
+    @ddt.unpack
+    def test_translate_action(self, id, content):
+        result = message_field.translate_action(id)
+        if content is None:
+            content = 'unknown action'
+        self.assertEqual(content, result)
+
+    @ddt.data({'id': '001',
+               'content': 'An unknown error occurred.'},
+              {'id': '002',
+               'content': 'Driver is not initialized at present.'},
+              {'id': 'invalid', 'content': None})
+    @ddt.unpack
+    def test_translate_detail(self, id, content):
+        result = message_field.translate_detail(id)
+        if content is None:
+            content = 'An unknown error occurred.'
+        self.assertEqual(content, result)
+
+    @ddt.data({'exception': exception.DriverNotInitialized(),
+               'detail': '',
+               'expected': '002'},
+              {'exception': exception.CinderException(),
+               'detail': '',
+               'expected': '001'},
+              {'exception': exception.CinderException(),
+               'detail': message_field.Detail.QUOTA_EXCEED,
+               'expected': '007'},
+              {'exception': '', 'detail': message_field.Detail.QUOTA_EXCEED,
+               'expected': '007'})
+    @ddt.unpack
+    def translate_detail_id(self, exception, detail, expected):
+        result = message_field.translate_detail_id(exception, detail)
+        self.assertEqual(expected, result)

cinder/tests/unit/scheduler/test_scheduler.py

@@ -25,7 +25,7 @@ from oslo_config import cfg
 
 from cinder import context
 from cinder import exception
-from cinder.message import defined_messages
+from cinder.message import message_field
 from cinder import objects
 from cinder.scheduler import driver
 from cinder.scheduler import manager
@@ -194,9 +194,9 @@ class SchedulerManagerTestCase(test.TestCase):
                                                    request_spec_obj, {})
 
         _mock_message_create.assert_called_once_with(
-            self.context, defined_messages.EventIds.UNABLE_TO_ALLOCATE,
+            self.context, message_field.Action.SCHEDULE_ALLOCATE_VOLUME,
-            self.context.project_id, resource_type='VOLUME',
+            resource_uuid=volume.id,
-            resource_uuid=volume.id)
+            exception=mock.ANY)
 
     @mock.patch('cinder.scheduler.driver.Scheduler.schedule_create_volume')
     @mock.patch('eventlet.sleep')

cinder/tests/unit/test_migrations.py

@@ -1244,6 +1244,16 @@ class MigrationsMixin(test_migrations.WalkVersionsMixin):
         self.assertIsInstance(groups.c.replication_status.type,
                               self.VARCHAR_TYPE)
 
+    def _check_103(self, engine, data):
+        self.assertTrue(engine.dialect.has_table(engine.connect(),
+                                                 "messages"))
+        attachment = db_utils.get_table(engine, 'messages')
+
+        self.assertIsInstance(attachment.c.detail_id.type,
+                              self.VARCHAR_TYPE)
+        self.assertIsInstance(attachment.c.action_id.type,
+                              self.VARCHAR_TYPE)
+
     def test_walk_versions(self):
         self.walk_versions(False, False)
         self.assert_each_foreign_key_is_part_of_an_index()

cinder/tests/unit/utils.py

@@ -371,7 +371,7 @@ def create_message(ctxt,
                    request_id='test_backup',
                    resource_type='This is a test backup',
                    resource_uuid='3asf434-3s433df43-434adf3-343df443',
-                   event_id=None,
+                   action=None,
                    message_level='Error'):
     """Create a message in the DB."""
     expires_at = (timeutils.utcnow() + datetime.timedelta(
@@ -380,7 +380,8 @@ def create_message(ctxt,
                       'request_id': request_id,
                       'resource_type': resource_type,
                       'resource_uuid': resource_uuid,
-                      'event_id': event_id,
+                      'action_id': action[0] if action else '',
+                      'event_id': "VOLUME_VOLUME_%s_002" % action[0],
                       'message_level': message_level,
                       'expires_at': expires_at}
     return db.message_create(ctxt, message_record)

cinder/tests/unit/volume/test_connection.py

@@ -21,8 +21,7 @@ import mock
 from cinder import context
 from cinder import db
 from cinder import exception
-from cinder.message import defined_messages
+from cinder.message import message_field
-from cinder.message import resource_types
 from cinder import objects
 from cinder.objects import fields
 from cinder.tests import fake_driver
@@ -1033,9 +1032,9 @@ class VolumeAttachDetachTestCase(base.BaseVolumeTestCase):
 
         # Assert a user message was created
         self.volume.message_api.create.assert_called_once_with(
-            self.context, defined_messages.EventIds.ATTACH_READONLY_VOLUME,
+            self.context, message_field.Action.ATTACH_VOLUME,
-            self.context.project_id, resource_type=resource_types.VOLUME,
+            resource_uuid=volume['id'],
-            resource_uuid=volume['id'])
+            exception=mock.ANY)
 
         attachment = objects.VolumeAttachmentList.get_all_by_volume_id(
             context.get_admin_context(), volume_id)[0]

cinder/tests/unit/volume/test_image.py

@@ -25,8 +25,7 @@ from oslo_utils import units
 
 from cinder import db
 from cinder import exception
-from cinder.message import defined_messages
+from cinder.message import message_field
-from cinder.message import resource_types
 from cinder import objects
 from cinder.objects import fields
 from cinder import quota
@@ -115,9 +114,10 @@ class CopyVolumeToImageTestCase(base.BaseVolumeTestCase):
             # Assert a user message was created
             self.volume.message_api.create.assert_called_once_with(
                 self.context,
-                defined_messages.EventIds.IMAGE_FROM_VOLUME_OVER_QUOTA,
+                message_field.Action.COPY_VOLUME_TO_IMAGE,
-                self.context.project_id, resource_type=resource_types.VOLUME,
+                resource_uuid=volume['id'],
-                resource_uuid=volume['id'])
+                exception=mock.ANY,
+                detail=message_field.Detail.FAILED_TO_UPLOAD_VOLUME)
 
     def test_copy_volume_to_image_instance_deleted(self):
         # During uploading volume to image if instance is deleted,

cinder/volume/manager.py

@@ -67,8 +67,7 @@ from cinder.image import image_utils
 from cinder import keymgr as key_manager
 from cinder import manager
 from cinder.message import api as message_api
-from cinder.message import defined_messages
+from cinder.message import message_field
-from cinder.message import resource_types
 from cinder import objects
 from cinder.objects import cgsnapshot
 from cinder.objects import consistencygroup
@@ -1051,10 +1050,6 @@ class VolumeManager(manager.CleanableManager,
 
         try:
             if volume_metadata.get('readonly') == 'True' and mode != 'ro':
-                self.message_api.create(
-                    context, defined_messages.EventIds.ATTACH_READONLY_VOLUME,
-                    context.project_id, resource_type=resource_types.VOLUME,
-                    resource_uuid=volume.id)
                 raise exception.InvalidVolumeAttachMode(mode=mode,
                                                         volume_id=volume.id)
             # NOTE(flaper87): Verify the driver is enabled
@@ -1073,8 +1068,13 @@ class VolumeManager(manager.CleanableManager,
                                       instance_uuid,
                                       host_name_sanitized,
                                       mountpoint)
-        except Exception:
+        except Exception as excep:
             with excutils.save_and_reraise_exception():
+                self.message_api.create(
+                    context,
+                    message_field.Action.ATTACH_VOLUME,
+                    resource_uuid=volume_id,
+                    exception=excep)
                 attachment.attach_status = (
                     fields.VolumeAttachStatus.ERROR_ATTACHING)
                 attachment.save()
@@ -1368,19 +1368,17 @@ class VolumeManager(manager.CleanableManager,
                       "(image-id: %(image_id)s).",
                       {'image_id': image_meta['id']},
                       resource=volume)
+            self.message_api.create(
+                context,
+                message_field.Action.COPY_VOLUME_TO_IMAGE,
+                resource_uuid=volume_id,
+                exception=error,
+                detail=message_field.Detail.FAILED_TO_UPLOAD_VOLUME)
             if image_service is not None:
                 # Deletes the image if it is in queued or saving state
                 self._delete_image(context, image_meta['id'], image_service)
-
             with excutils.save_and_reraise_exception():
                 payload['message'] = six.text_type(error)
-                if isinstance(error, exception.ImageLimitExceeded):
-                    self.message_api.create(
-                        context,
-                        defined_messages.EventIds.IMAGE_FROM_VOLUME_OVER_QUOTA,
-                        context.project_id,
-                        resource_type=resource_types.VOLUME,
-                        resource_uuid=volume_id)
         finally:
             self.db.volume_update_status_based_on_attachment(context,
                                                              volume_id)
@@ -4108,10 +4106,6 @@ class VolumeManager(manager.CleanableManager,
 
         try:
             if volume_metadata.get('readonly') == 'True' and mode != 'ro':
-                self.message_api.create(
-                    context, defined_messages.EventIds.ATTACH_READONLY_VOLUME,
-                    context.project_id, resource_type=resource_types.VOLUME,
-                    resource_uuid=vref.id)
                 raise exception.InvalidVolumeAttachMode(mode=mode,
                                                         volume_id=vref.id)
             utils.require_driver_initialized(self.driver)
@@ -4120,7 +4114,11 @@ class VolumeManager(manager.CleanableManager,
                                       attachment_ref.instance_uuid,
                                       connector.get('host', ''),
                                       connector.get('mountpoint', 'na'))
-        except Exception:
+        except Exception as err:
+            self.message_api.create(
+                context, message_field.Action.UPDATE_ATTACHMENT,
+                resource_uuid=vref.id,
+                exception=err)
             with excutils.save_and_reraise_exception():
                 self.db.volume_attachment_update(
                     context, attachment_ref.id,