Remove conductor usage from compute.utils.EventReporter

This makes compute.utils.EventReporter use the InstanceActionEvent
object instead of making raw conductor calls. Callers are changed to
stop passing conductor API to it.

In addition, moves helper methods pack_action_event*() from
compute.utils to objects.instance_action like previously was done
with pack_action_[start|finish]().

Related to blueprint compute-manager-objects-juno

Change-Id: I308959a1b206899369a10d391991124ef014c89f

nova/compute/manager.py

@@ -331,9 +331,7 @@ def wrap_instance_event(function):
         instance_uuid = keyed_args['instance']['uuid']
 
         event_name = 'compute_{0}'.format(function.func_name)
-        with compute_utils.EventReporter(context, self.conductor_api,
+        with compute_utils.EventReporter(context, event_name, instance_uuid):
-                                         event_name, instance_uuid):
-
             function(self, context, *args, **kwargs)
 
     return decorated_function

nova/compute/utils.py

@@ -29,10 +29,10 @@ from nova import exception
 from nova.network import model as network_model
 from nova import notifications
 from nova.objects import instance as instance_obj
+from nova.objects import instance_action as instance_action_obj
 from nova.objects import instance_fault as instance_fault_obj
 from nova.openstack.common.gettextutils import _
 from nova.openstack.common import log
-from nova.openstack.common import timeutils
 from nova import rpc
 from nova import utils
 from nova.virt import driver
@@ -95,47 +95,6 @@ def add_instance_fault_from_exc(context, instance, fault, exc_info=None):
     fault_obj.create()
 
 
-def pack_action_start(context, instance_uuid, action_name):
-    values = {'action': action_name,
-              'instance_uuid': instance_uuid,
-              'request_id': context.request_id,
-              'user_id': context.user_id,
-              'project_id': context.project_id,
-              'start_time': context.timestamp}
-    return values
-
-
-def pack_action_finish(context, instance_uuid):
-    values = {'instance_uuid': instance_uuid,
-              'request_id': context.request_id,
-              'finish_time': timeutils.utcnow()}
-    return values
-
-
-def pack_action_event_start(context, instance_uuid, event_name):
-    values = {'event': event_name,
-              'instance_uuid': instance_uuid,
-              'request_id': context.request_id,
-              'start_time': timeutils.utcnow()}
-    return values
-
-
-def pack_action_event_finish(context, instance_uuid, event_name, exc_val=None,
-                             exc_tb=None):
-    values = {'event': event_name,
-              'instance_uuid': instance_uuid,
-              'request_id': context.request_id,
-              'finish_time': timeutils.utcnow()}
-    if exc_tb is None:
-        values['result'] = 'Success'
-    else:
-        values['result'] = 'Error'
-        values['message'] = str(exc_val)
-        values['traceback'] = ''.join(traceback.format_tb(exc_tb))
-
-    return values
-
-
 def get_device_name_for_instance(context, instance, bdms, device):
     """Validates (or generates) a device name for instance.
 
@@ -458,25 +417,23 @@ def get_reboot_type(task_state, current_power_state):
 class EventReporter(object):
     """Context manager to report instance action events."""
 
-    def __init__(self, context, conductor, event_name, *instance_uuids):
+    def __init__(self, context, event_name, *instance_uuids):
         self.context = context
-        self.conductor = conductor
         self.event_name = event_name
         self.instance_uuids = instance_uuids
 
     def __enter__(self):
         for uuid in self.instance_uuids:
-            event = pack_action_event_start(self.context, uuid,
+            instance_action_obj.InstanceActionEvent.event_start(
-                                            self.event_name)
+                self.context, uuid, self.event_name)
-            self.conductor.action_event_start(self.context, event)
 
         return self
 
     def __exit__(self, exc_type, exc_val, exc_tb):
         for uuid in self.instance_uuids:
-            event = pack_action_event_finish(self.context, uuid,
+            instance_action_obj.InstanceActionEvent.event_finish_with_failure(
-                                             self.event_name, exc_val, exc_tb)
+                self.context, uuid, self.event_name, exc_val=exc_val,
-            self.conductor.action_event_finish(self.context, event)
+                exc_tb=exc_tb)
         return False
 
 

nova/conductor/manager.py

@@ -681,8 +681,8 @@ class ComputeTaskManager(base.Base):
                                block_migration, disk_over_commit)
         elif not live and not rebuild and flavor:
             instance_uuid = instance['uuid']
-            with compute_utils.EventReporter(context, self.db,
+            with compute_utils.EventReporter(context, 'cold_migrate',
-                                         'cold_migrate', instance_uuid):
+                                             instance_uuid):
                 self._cold_migrate(context, instance, flavor,
                                    scheduler_hint['filter_properties'],
                                    reservations)
@@ -828,7 +828,7 @@ class ComputeTaskManager(base.Base):
         elif instance.vm_state == vm_states.SHELVED_OFFLOADED:
             image_id = sys_meta.get('shelved_image_id')
             with compute_utils.EventReporter(
-                context, self.db, 'get_image_info', instance.uuid):
+                context, 'get_image_info', instance.uuid):
                 try:
                     image = safe_image_show(context, image_id)
                 except exception.ImageNotFound:
@@ -841,8 +841,7 @@ class ComputeTaskManager(base.Base):
                         instance_id=instance.uuid, reason=reason)
 
             try:
-                with compute_utils.EventReporter(context, self.db,
+                with compute_utils.EventReporter(context, 'schedule_instances',
-                                                 'schedule_instances',
                                                  instance.uuid):
                     filter_properties = {}
                     hosts = self._schedule_instances(context, image,

nova/objects/instance_action.py

@@ -12,7 +12,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-from nova.compute import utils as compute_utils
+import traceback
+
 from nova import db
 from nova.objects import base
 from nova.objects import fields
@@ -127,6 +128,29 @@ class InstanceActionEvent(base.NovaPersistentObject, base.NovaObject):
         event.obj_reset_changes()
         return event
 
+    @staticmethod
+    def pack_action_event_start(context, instance_uuid, event_name):
+        values = {'event': event_name,
+                  'instance_uuid': instance_uuid,
+                  'request_id': context.request_id,
+                  'start_time': timeutils.utcnow()}
+        return values
+
+    @staticmethod
+    def pack_action_event_finish(context, instance_uuid, event_name,
+                                 exc_val=None, exc_tb=None):
+        values = {'event': event_name,
+                  'instance_uuid': instance_uuid,
+                  'request_id': context.request_id,
+                  'finish_time': timeutils.utcnow()}
+        if exc_tb is None:
+            values['result'] = 'Success'
+        else:
+            values['result'] = 'Error'
+            values['message'] = str(exc_val)
+            values['traceback'] = ''.join(traceback.format_tb(exc_tb))
+        return values
+
     @base.remotable_classmethod
     def get_by_id(cls, context, action_id, event_id):
         db_event = db.action_event_get_by_id(context, action_id, event_id)
@@ -134,7 +158,7 @@ class InstanceActionEvent(base.NovaPersistentObject, base.NovaObject):
 
     @base.remotable_classmethod
     def event_start(cls, context, instance_uuid, event_name, want_result=True):
-        values = compute_utils.pack_action_event_start(context, instance_uuid,
+        values = cls.pack_action_event_start(context, instance_uuid,
                                              event_name)
         db_event = db.action_event_start(context, values)
         if want_result:
@@ -143,9 +167,8 @@ class InstanceActionEvent(base.NovaPersistentObject, base.NovaObject):
     @base.remotable_classmethod
     def event_finish_with_failure(cls, context, instance_uuid, event_name,
                                   exc_val=None, exc_tb=None, want_result=None):
-        values = compute_utils.pack_action_event_finish(context, instance_uuid,
+        values = cls.pack_action_event_finish(context, instance_uuid,
-                                                        event_name,
+                                              event_name, exc_val=exc_val,
-                                                        exc_val=exc_val,
                                               exc_tb=exc_tb)
         db_event = db.action_event_finish(context, values)
         if want_result:
@@ -161,10 +184,8 @@ class InstanceActionEvent(base.NovaPersistentObject, base.NovaObject):
 
     @base.remotable
     def finish_with_failure(self, context, exc_val, exc_tb):
-        values = compute_utils.pack_action_event_finish(context,
+        values = self.pack_action_event_finish(context, self.instance_uuid,
-                                                        self.instance_uuid,
+                                               self.event, exc_val=exc_val,
-                                                        self.event,
-                                                        exc_val=exc_val,
                                                exc_tb=exc_tb)
         db_event = db.action_event_finish(context, values)
         self._from_db_object(context, self, db_event)

nova/scheduler/manager.py

@@ -26,7 +26,6 @@ from nova.compute import rpcapi as compute_rpcapi
 from nova.compute import task_states
 from nova.compute import utils as compute_utils
 from nova.compute import vm_states
-from nova.conductor import api as conductor_api
 from nova.conductor.tasks import live_migrate
 from nova import exception
 from nova import manager
@@ -131,8 +130,7 @@ class SchedulerManager(manager.Manager):
         Sets instance vm_state to ERROR on exceptions
         """
         instance_uuids = request_spec['instance_uuids']
-        with compute_utils.EventReporter(context, conductor_api.LocalAPI(),
+        with compute_utils.EventReporter(context, 'schedule', *instance_uuids):
-                                         'schedule', *instance_uuids):
             try:
                 return self.driver.schedule_run_instance(context,
                         request_spec, admin_password, injected_files,
@@ -159,8 +157,7 @@ class SchedulerManager(manager.Manager):
         Sets vm_state to ERROR on other exceptions
         """
         instance_uuid = instance['uuid']
-        with compute_utils.EventReporter(context, conductor_api.LocalAPI(),
+        with compute_utils.EventReporter(context, 'schedule', instance_uuid):
-                                         'schedule', instance_uuid):
             try:
                 request_spec['num_instances'] = len(
                         request_spec['instance_uuids'])

nova/tests/compute/test_compute.py

@@ -58,6 +58,7 @@ from nova.objects import base as obj_base
 from nova.objects import block_device as block_device_obj
 from nova.objects import compute_node as compute_node_obj
 from nova.objects import instance as instance_obj
+from nova.objects import instance_action as instance_action_obj
 from nova.objects import instance_group as instance_group_obj
 from nova.objects import migration as migration_obj
 from nova.objects import quotas as quotas_obj
@@ -1117,53 +1118,27 @@ class ComputeTestCase(BaseTestCase):
 
         self.assertFalse(called['fault_added'])
 
-    def test_wrap_instance_event(self):
+    @mock.patch.object(instance_action_obj.InstanceActionEvent, 'event_start')
+    @mock.patch.object(instance_action_obj.InstanceActionEvent,
+                       'event_finish_with_failure')
+    def test_wrap_instance_event(self, mock_finish, mock_start):
         inst = {"uuid": "fake_uuid"}
 
-        called = {'started': False,
-                  'finished': False}
-
-        def did_it_update_start(self2, context, values):
-            called['started'] = True
-
-        def did_it_update_finish(self2, context, values):
-            called['finished'] = True
-
-        self.stubs.Set(conductor_manager.ConductorManager,
-                       'action_event_start', did_it_update_start)
-
-        self.stubs.Set(conductor_manager.ConductorManager,
-                       'action_event_finish', did_it_update_finish)
-
         @compute_manager.wrap_instance_event
         def fake_event(self, context, instance):
             pass
 
         fake_event(self.compute, self.context, instance=inst)
 
-        self.assertTrue(called['started'])
+        self.assertTrue(mock_start.called)
-        self.assertTrue(called['finished'])
+        self.assertTrue(mock_finish.called)
 
-    def test_wrap_instance_event_log_exception(self):
+    @mock.patch.object(instance_action_obj.InstanceActionEvent, 'event_start')
+    @mock.patch.object(instance_action_obj.InstanceActionEvent,
+                       'event_finish_with_failure')
+    def test_wrap_instance_event_log_exception(self, mock_finish, mock_start):
         inst = {"uuid": "fake_uuid"}
 
-        called = {'started': False,
-                  'finished': False,
-                  'message': ''}
-
-        def did_it_update_start(self2, context, values):
-            called['started'] = True
-
-        def did_it_update_finish(self2, context, values):
-            called['finished'] = True
-            called['message'] = values['message']
-
-        self.stubs.Set(conductor_manager.ConductorManager,
-                       'action_event_start', did_it_update_start)
-
-        self.stubs.Set(conductor_manager.ConductorManager,
-                       'action_event_finish', did_it_update_finish)
-
         @compute_manager.wrap_instance_event
         def fake_event(self2, context, instance):
             raise exception.NovaException()
@@ -1171,9 +1146,10 @@ class ComputeTestCase(BaseTestCase):
         self.assertRaises(exception.NovaException, fake_event,
                           self.compute, self.context, instance=inst)
 
-        self.assertTrue(called['started'])
+        self.assertTrue(mock_start.called)
-        self.assertTrue(called['finished'])
+        self.assertTrue(mock_finish.called)
-        self.assertEqual('An unknown exception occurred.', called['message'])
+        args, kwargs = mock_finish.call_args
+        self.assertIsInstance(kwargs['exc_val'], exception.NovaException)
 
     def test_object_compat(self):
         db_inst = fake_instance.fake_db_instance()

nova/tests/compute/test_compute_mgr.py

@@ -33,6 +33,7 @@ from nova.objects import base as obj_base
 from nova.objects import block_device as block_device_obj
 from nova.objects import external_event as external_event_obj
 from nova.objects import instance as instance_obj
+from nova.objects import instance_action as instance_action_obj
 from nova.objects import migration as migration_obj
 from nova.openstack.common import importutils
 from nova.openstack.common import uuidutils
@@ -1267,21 +1268,25 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
         self.compute._notify_about_instance_usage(self.context, self.instance,
                 event, **kwargs)
 
+    def _instance_action_events(self):
+        self.mox.StubOutWithMock(instance_action_obj.InstanceActionEvent,
+                                 'event_start')
+        self.mox.StubOutWithMock(instance_action_obj.InstanceActionEvent,
+                                 'event_finish_with_failure')
+        instance_action_obj.InstanceActionEvent.event_start(
+                self.context, self.instance['uuid'], mox.IgnoreArg())
+        instance_action_obj.InstanceActionEvent.event_finish_with_failure(
+                self.context, self.instance['uuid'], mox.IgnoreArg(),
+                exc_val=mox.IgnoreArg(), exc_tb=mox.IgnoreArg())
+
     def test_build_and_run_instance_called_with_proper_args(self):
         self.mox.StubOutWithMock(self.compute, '_build_and_run_instance')
-        self.mox.StubOutWithMock(self.compute.conductor_api,
-                                 'action_event_start')
-        self.mox.StubOutWithMock(self.compute.conductor_api,
-                                 'action_event_finish')
         self._do_build_instance_update()
         self.compute._build_and_run_instance(self.context, self.instance,
                 self.image, self.injected_files, self.admin_pass,
                 self.requested_networks, self.security_groups,
                 self.block_device_mapping, self.node, self.limits)
-        self.compute.conductor_api.action_event_start(self.context,
+        self._instance_action_events()
-                                                      mox.IgnoreArg())
-        self.compute.conductor_api.action_event_finish(self.context,
-                                                       mox.IgnoreArg())
         self.mox.ReplayAll()
 
         self.compute.build_and_run_instance(self.context, self.instance,
@@ -1299,10 +1304,6 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
         self.mox.StubOutWithMock(self.compute, '_set_instance_error_state')
         self.mox.StubOutWithMock(self.compute.compute_task_api,
                                  'build_instances')
-        self.mox.StubOutWithMock(self.compute.conductor_api,
-                                 'action_event_start')
-        self.mox.StubOutWithMock(self.compute.conductor_api,
-                                 'action_event_finish')
         self._do_build_instance_update()
         self.compute._build_and_run_instance(self.context, self.instance,
                 self.image, self.injected_files, self.admin_pass,
@@ -1314,10 +1315,7 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
                 self.requested_networks)
         self.compute._set_instance_error_state(self.context,
                 self.instance['uuid'])
-        self.compute.conductor_api.action_event_start(self.context,
+        self._instance_action_events()
-                                                      mox.IgnoreArg())
-        self.compute.conductor_api.action_event_finish(self.context,
-                                                       mox.IgnoreArg())
         self.mox.ReplayAll()
 
         self.compute.build_and_run_instance(self.context, self.instance,
@@ -1334,10 +1332,6 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
         self.mox.StubOutWithMock(self.compute, '_set_instance_error_state')
         self.mox.StubOutWithMock(self.compute.compute_task_api,
                                  'build_instances')
-        self.mox.StubOutWithMock(self.compute.conductor_api,
-                                 'action_event_start')
-        self.mox.StubOutWithMock(self.compute.conductor_api,
-                                 'action_event_finish')
         self._do_build_instance_update(reschedule_update=True)
         self.compute._build_and_run_instance(self.context, self.instance,
                 self.image, self.injected_files, self.admin_pass,
@@ -1349,10 +1343,7 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
                 [self.instance], self.image, [], self.admin_pass,
                 self.injected_files, self.requested_networks,
                 self.security_groups, self.block_device_mapping)
-        self.compute.conductor_api.action_event_start(self.context,
+        self._instance_action_events()
-                                                      mox.IgnoreArg())
-        self.compute.conductor_api.action_event_finish(self.context,
-                                                       mox.IgnoreArg())
         self.mox.ReplayAll()
 
         self.compute.build_and_run_instance(self.context, self.instance,
@@ -1369,10 +1360,6 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
         self.mox.StubOutWithMock(self.compute, '_cleanup_allocated_networks')
         self.mox.StubOutWithMock(self.compute.compute_task_api,
                 'build_instances')
-        self.mox.StubOutWithMock(self.compute.conductor_api,
-                                 'action_event_start')
-        self.mox.StubOutWithMock(self.compute.conductor_api,
-                                 'action_event_finish')
         self._do_build_instance_update(reschedule_update=True)
         self.compute._build_and_run_instance(self.context, self.instance,
                 self.image, self.injected_files, self.admin_pass,
@@ -1384,10 +1371,7 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
                 [self.instance], self.image, [], self.admin_pass,
                 self.injected_files, self.requested_networks,
                 self.security_groups, self.block_device_mapping)
-        self.compute.conductor_api.action_event_start(self.context,
+        self._instance_action_events()
-                                                      mox.IgnoreArg())
-        self.compute.conductor_api.action_event_finish(self.context,
-                                                       mox.IgnoreArg())
         self.mox.ReplayAll()
 
         self.compute.build_and_run_instance(self.context, self.instance,
@@ -1406,10 +1390,6 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
         self.mox.StubOutWithMock(self.compute, '_cleanup_allocated_networks')
         self.mox.StubOutWithMock(self.compute.compute_task_api,
                 'build_instances')
-        self.mox.StubOutWithMock(self.compute.conductor_api,
-                                 'action_event_start')
-        self.mox.StubOutWithMock(self.compute.conductor_api,
-                                 'action_event_finish')
         self._do_build_instance_update(reschedule_update=True)
         self.compute._build_and_run_instance(self.context, self.instance,
                 self.image, self.injected_files, self.admin_pass,
@@ -1425,10 +1405,7 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
                 [self.instance], self.image, [], self.admin_pass,
                 self.injected_files, self.requested_networks,
                 self.security_groups, self.block_device_mapping)
-        self.compute.conductor_api.action_event_start(self.context,
+        self._instance_action_events()
-                                                      mox.IgnoreArg())
-        self.compute.conductor_api.action_event_finish(self.context,
-                                                       mox.IgnoreArg())
         self.mox.ReplayAll()
 
         self.compute.build_and_run_instance(self.context, self.instance,
@@ -1446,10 +1423,6 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
         self.mox.StubOutWithMock(self.compute, '_set_instance_error_state')
         self.mox.StubOutWithMock(self.compute.compute_task_api,
                 'build_instances')
-        self.mox.StubOutWithMock(self.compute.conductor_api,
-                                 'action_event_start')
-        self.mox.StubOutWithMock(self.compute.conductor_api,
-                                 'action_event_finish')
         self._do_build_instance_update()
         self.compute._build_and_run_instance(self.context, self.instance,
                 self.image, self.injected_files, self.admin_pass,
@@ -1460,10 +1433,7 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
                 self.requested_networks)
         self.compute._set_instance_error_state(self.context,
                 self.instance['uuid'])
-        self.compute.conductor_api.action_event_start(self.context,
+        self._instance_action_events()
-                                                      mox.IgnoreArg())
-        self.compute.conductor_api.action_event_finish(self.context,
-                                                       mox.IgnoreArg())
         self.mox.ReplayAll()
 
         self.compute.build_and_run_instance(self.context, self.instance,
@@ -1604,10 +1574,6 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
         self.mox.StubOutWithMock(self.compute, '_get_resource_tracker')
         self.mox.StubOutWithMock(self.compute.compute_task_api,
                 'build_instances')
-        self.mox.StubOutWithMock(self.compute.conductor_api,
-                                 'action_event_start')
-        self.mox.StubOutWithMock(self.compute.conductor_api,
-                                 'action_event_finish')
         self.compute._get_resource_tracker(self.node).AndReturn(
             FakeResourceTracker())
         self._do_build_instance_update(reschedule_update=True)
@@ -1619,10 +1585,7 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
                 [self.instance], self.image, [], self.admin_pass,
                 self.injected_files, self.requested_networks,
                 self.security_groups, self.block_device_mapping)
-        self.compute.conductor_api.action_event_start(self.context,
+        self._instance_action_events()
-                                                      mox.IgnoreArg())
-        self.compute.conductor_api.action_event_finish(self.context,
-                                                       mox.IgnoreArg())
         self.mox.ReplayAll()
 
         self.compute.build_and_run_instance(self.context, self.instance,
@@ -1864,10 +1827,10 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase):
         with contextlib.nested(
             mock.patch.object(self.compute, '_finish_resize',
                               side_effect=exception.ResizeError(reason='')),
-            mock.patch.object(self.compute.conductor_api,
+            mock.patch.object(instance_action_obj.InstanceActionEvent,
-                              'action_event_start'),
+                              'event_start'),
-            mock.patch.object(self.compute.conductor_api,
+            mock.patch.object(instance_action_obj.InstanceActionEvent,
-                              'action_event_finish'),
+                              'event_finish_with_failure'),
             mock.patch.object(db, 'instance_fault_create'),
             mock.patch.object(self.compute, '_instance_update'),
             mock.patch.object(self.migration, 'save'),
@@ -1893,10 +1856,10 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase):
             mock.patch.object(self.compute.driver,
                               'migrate_disk_and_power_off',
                               side_effect=exception.ResizeError(reason='')),
-            mock.patch.object(self.compute.conductor_api,
+            mock.patch.object(instance_action_obj.InstanceActionEvent,
-                              'action_event_start'),
+                              'event_start'),
-            mock.patch.object(self.compute.conductor_api,
+            mock.patch.object(instance_action_obj.InstanceActionEvent,
-                              'action_event_finish'),
+                              'event_finish_with_failure'),
             mock.patch.object(db, 'instance_fault_create'),
             mock.patch.object(self.compute, '_instance_update'),
             mock.patch.object(self.migration, 'save'),

nova/tests/fake_server_actions.py

@@ -107,11 +107,11 @@ FAKE_EVENTS = {
 
 
 def fake_action_event_start(*args):
-    pass
+    return FAKE_EVENTS[FAKE_ACTION_ID1][0]
 
 
 def fake_action_event_finish(*args):
-    pass
+    return FAKE_EVENTS[FAKE_ACTION_ID1][0]
 
 
 def stub_out_action_events(stubs):