Added instance actions for conductor

to track result of conductor actions wrap_instance_event decorator is moved to compute_utils and is added to migrate_server method on conductor.manager bp: async-live-migration-rest-check Change-Id: I01e6ba9c9d8df8690d2cecea2cc48e45cf7a53c4
2016-04-27 19:48:02 +03:00 · 2016-04-27 19:48:02 +03:00 · 51fdeaf5e0
commit 51fdeaf5e0
parent 4fee0f0dc6
6 changed files with 126 additions and 105 deletions
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@ -63,6 +63,7 @@ from nova.compute import resource_tracker
 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.utils import wrap_instance_event
 from nova.compute import vm_states
 from nova import conductor
 import nova.conf
@ -213,28 +214,6 @@ def wrap_instance_fault(function):
    return decorated_function


-@utils.expects_func_args('instance')
-def wrap_instance_event(function):
-    """Wraps a method to log the event taken on the instance, and result.
-
-    This decorator wraps a method to log the start and result of an event, as
-    part of an action taken on an instance.
-    """
-
-    @functools.wraps(function)
-    def decorated_function(self, context, *args, **kwargs):
-        wrapped_func = safe_utils.get_wrapped_function(function)
-        keyed_args = inspect.getcallargs(wrapped_func, self, context, *args,
-                                         **kwargs)
-        instance_uuid = keyed_args['instance']['uuid']
-
-        event_name = 'compute_{0}'.format(function.__name__)
-        with compute_utils.EventReporter(context, event_name, instance_uuid):
-            return function(self, context, *args, **kwargs)
-
-    return decorated_function
-
-
@utils.expects_func_args('image_id', 'instance')
 def delete_image_on_error(function):
    """Used for snapshot related method to ensure the image created in
@ -1710,7 +1689,7 @@ class ComputeManager(manager.Manager):
    @hooks.add_hook('build_instance')
    @wrap_exception()
    @reverts_task_state
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @wrap_instance_fault
    def _do_build_and_run_instance(self, context, instance, image,
            request_spec, filter_properties, admin_password, injected_files,
@ -2305,7 +2284,7 @@ class ComputeManager(manager.Manager):

    @wrap_exception()
    @reverts_task_state
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @wrap_instance_fault
    def terminate_instance(self, context, instance, bdms, reservations):
        """Terminate an instance on this host."""
@ -2351,7 +2330,7 @@ class ComputeManager(manager.Manager):
    # can't use that name in grizzly.
    @wrap_exception()
    @reverts_task_state
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @wrap_instance_fault
    def stop_instance(self, context, instance, clean_shutdown):
        """Stopping an instance on this host."""
@ -2423,7 +2402,7 @@ class ComputeManager(manager.Manager):
    # can't use that name in grizzly.
    @wrap_exception()
    @reverts_task_state
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @wrap_instance_fault
    def start_instance(self, context, instance):
        """Starting an instance on this host."""
@ -2449,7 +2428,7 @@ class ComputeManager(manager.Manager):
                                   exception.TriggerCrashDumpNotSupported,
                                   exception.InstanceNotRunning)
    @wrap_exception()
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @wrap_instance_fault
    def trigger_crash_dump(self, context, instance):
        """Trigger crash dump in an instance."""
@ -2466,7 +2445,7 @@ class ComputeManager(manager.Manager):

    @wrap_exception()
    @reverts_task_state
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @wrap_instance_fault
    def soft_delete_instance(self, context, instance, reservations):
        """Soft delete an instance on this host."""
@ -2495,7 +2474,7 @@ class ComputeManager(manager.Manager):

    @wrap_exception()
    @reverts_task_state
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @wrap_instance_fault
    def restore_instance(self, context, instance):
        """Restore a soft-deleted instance on this host."""
@ -2561,7 +2540,7 @@ class ComputeManager(manager.Manager):
    @messaging.expected_exceptions(exception.PreserveEphemeralNotSupported)
    @wrap_exception()
    @reverts_task_state
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @wrap_instance_fault
    def rebuild_instance(self, context, instance, orig_image_ref, image_ref,
                         injected_files, new_pass, orig_sys_metadata,
@ -2837,7 +2816,7 @@ class ComputeManager(manager.Manager):

    @wrap_exception()
    @reverts_task_state
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @wrap_instance_fault
    def reboot_instance(self, context, instance, block_device_info,
                        reboot_type):
@ -3094,7 +3073,7 @@ class ComputeManager(manager.Manager):

    @wrap_exception()
    @reverts_task_state
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @wrap_instance_fault
    def set_admin_password(self, context, instance, new_pass):
        """Set the root/admin password for an instance on this host.
@ -3215,7 +3194,7 @@ class ComputeManager(manager.Manager):

    @wrap_exception()
    @reverts_task_state
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @wrap_instance_fault
    def rescue_instance(self, context, instance, rescue_password,
                        rescue_image_ref, clean_shutdown):
@ -3265,7 +3244,7 @@ class ComputeManager(manager.Manager):

    @wrap_exception()
    @reverts_task_state
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @wrap_instance_fault
    def unrescue_instance(self, context, instance):
        context = context.elevated()
@ -3297,7 +3276,7 @@ class ComputeManager(manager.Manager):
        self.driver.change_instance_metadata(context, instance, diff)

    @wrap_exception()
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @wrap_instance_fault
    def confirm_resize(self, context, instance, reservations, migration):

@ -3414,7 +3393,7 @@ class ComputeManager(manager.Manager):

    @wrap_exception()
    @reverts_task_state
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @errors_out_migration
    @wrap_instance_fault
    def revert_resize(self, context, instance, migration, reservations):
@ -3482,7 +3461,7 @@ class ComputeManager(manager.Manager):

    @wrap_exception()
    @reverts_task_state
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @errors_out_migration
    @wrap_instance_fault
    def finish_revert_resize(self, context, instance, reservations, migration):
@ -3600,7 +3579,7 @@ class ComputeManager(manager.Manager):

    @wrap_exception()
    @reverts_task_state
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @wrap_instance_fault
    def prep_resize(self, context, image, instance, instance_type,
                    reservations, request_spec, filter_properties, node,
@ -3701,7 +3680,7 @@ class ComputeManager(manager.Manager):

    @wrap_exception()
    @reverts_task_state
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @errors_out_migration
    @wrap_instance_fault
    def resize_instance(self, context, instance, image,
@ -3863,7 +3842,7 @@ class ComputeManager(manager.Manager):

    @wrap_exception()
    @reverts_task_state
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @errors_out_migration
    @wrap_instance_fault
    def finish_resize(self, context, disk_info, image, instance,
@ -3942,7 +3921,7 @@ class ComputeManager(manager.Manager):

    @wrap_exception()
    @reverts_task_state
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @wrap_instance_fault
    def pause_instance(self, context, instance):
        """Pause an instance on this host."""
@ -3958,7 +3937,7 @@ class ComputeManager(manager.Manager):

    @wrap_exception()
    @reverts_task_state
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @wrap_instance_fault
    def unpause_instance(self, context, instance):
        """Unpause a paused instance on this host."""
@ -4030,7 +4009,7 @@ class ComputeManager(manager.Manager):

    @wrap_exception()
    @reverts_task_state
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @wrap_instance_fault
    def suspend_instance(self, context, instance):
        """Suspend the given instance."""
@ -4051,7 +4030,7 @@ class ComputeManager(manager.Manager):

    @wrap_exception()
    @reverts_task_state
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @wrap_instance_fault
    def resume_instance(self, context, instance):
        """Resume the given suspended instance."""
@ -4080,7 +4059,7 @@ class ComputeManager(manager.Manager):

    @wrap_exception()
    @reverts_task_state
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @wrap_instance_fault
    def shelve_instance(self, context, instance, image_id,
                        clean_shutdown):
@ -4177,7 +4156,7 @@ class ComputeManager(manager.Manager):

    @wrap_exception()
    @reverts_task_state
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @wrap_instance_fault
    def unshelve_instance(self, context, instance, image,
                          filter_properties, node):
@ -4957,7 +4936,7 @@ class ComputeManager(manager.Manager):
        return self.driver.check_instance_shared_storage_remote(ctxt, data)

    @wrap_exception()
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @wrap_instance_fault
    def check_can_live_migrate_destination(self, ctxt, instance,
                                           block_migration, disk_over_commit):
@ -4999,7 +4978,7 @@ class ComputeManager(manager.Manager):
        return migrate_data

    @wrap_exception()
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @wrap_instance_fault
    def check_can_live_migrate_source(self, ctxt, instance, dest_check_data):
        """Check if it is possible to execute live migration.
@ -5037,7 +5016,7 @@ class ComputeManager(manager.Manager):
        return result

    @wrap_exception()
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @wrap_instance_fault
    def pre_live_migration(self, context, instance, block_migration, disk,
                           migrate_data):
@ -5153,7 +5132,7 @@ class ComputeManager(manager.Manager):
                self._set_migration_status(migration, 'error')

    @wrap_exception()
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @wrap_instance_fault
    def live_migration(self, context, dest, instance, block_migration,
                       migration, migrate_data):
@ -5185,7 +5164,7 @@ class ComputeManager(manager.Manager):
    # remove migration_id parameter when the compute RPC version
    # is bumped to 5.x.
    @wrap_exception()
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @wrap_instance_fault
    def live_migration_force_complete(self, context, instance,
                                      migration_id=None):
@ -5204,7 +5183,7 @@ class ComputeManager(manager.Manager):
            context, instance, 'live.migration.force.complete.end')

    @wrap_exception()
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @wrap_instance_fault
    def live_migration_abort(self, context, instance, migration_id):
        """Abort an in-progress live migration.
@ -5390,7 +5369,7 @@ class ComputeManager(manager.Manager):
                    ctxt, instance.uuid)

    @wrap_exception()
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @wrap_instance_fault
    def post_live_migration_at_destination(self, context, instance,
                                           block_migration):
@ -5521,7 +5500,7 @@ class ComputeManager(manager.Manager):
        self._set_migration_status(migration, migration_status)

    @wrap_exception()
-    @wrap_instance_event
+    @wrap_instance_event(prefix='compute')
    @wrap_instance_fault
    def rollback_live_migration_at_destination(self, context, instance,
                                               destroy_disks,
--- a/nova/compute/utils.py
+++ b/nova/compute/utils.py
@ -14,6 +14,8 @@

 """Compute-related Utilities and helpers."""

+import functools
+import inspect
 import itertools
 import string
 import traceback
@ -32,6 +34,7 @@ from nova.network import model as network_model
 from nova import notifications
 from nova import objects
 from nova import rpc
+from nova import safe_utils
 from nova import utils
 from nova.virt import driver

@ -543,6 +546,29 @@ class EventReporter(object):
        return False


+def wrap_instance_event(prefix):
+    """Wraps a method to log the event taken on the instance, and result.
+
+    This decorator wraps a method to log the start and result of an event, as
+    part of an action taken on an instance.
+    """
+    @utils.expects_func_args('instance')
+    def helper(function):
+
+        @functools.wraps(function)
+        def decorated_function(self, context, *args, **kwargs):
+            wrapped_func = safe_utils.get_wrapped_function(function)
+            keyed_args = inspect.getcallargs(wrapped_func, self, context,
+                                             *args, **kwargs)
+            instance_uuid = keyed_args['instance']['uuid']
+
+            event_name = '{0}_{1}'.format(prefix, function.__name__)
+            with EventReporter(context, event_name, instance_uuid):
+                return function(self, context, *args, **kwargs)
+        return decorated_function
+    return helper
+
+
 class UnlimitedSemaphore(object):
    def __enter__(self):
        pass
--- a/nova/conductor/manager.py
+++ b/nova/conductor/manager.py
@ -25,6 +25,7 @@ import six
 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.utils import wrap_instance_event
 from nova.compute import vm_states
 from nova.conductor.tasks import live_migrate
 from nova.conductor.tasks import migrate
@ -175,6 +176,7 @@ class ComputeTaskManager(base.Base):
        exception.MigrationPreCheckClientException,
        exception.LiveMigrationWithOldNovaNotSupported,
        exception.UnsupportedPolicyException)
+    @wrap_instance_event(prefix='conductor')
    def migrate_server(self, context, instance, scheduler_hint, live, rebuild,
            flavor, block_migration, disk_over_commit, reservations=None,
            clean_shutdown=True, request_spec=None):
--- a/nova/tests/unit/compute/test_compute.py
+++ b/nova/tests/unit/compute/test_compute.py
@ -1309,55 +1309,6 @@ class ComputeTestCase(BaseTestCase):

        self.assertFalse(called['fault_added'])

-    @mock.patch.object(objects.InstanceActionEvent, 'event_start')
-    @mock.patch.object(objects.InstanceActionEvent,
-                       'event_finish_with_failure')
-    def test_wrap_instance_event(self, mock_finish, mock_start):
-        inst = {"uuid": uuids.instance}
-
-        @compute_manager.wrap_instance_event
-        def fake_event(self, context, instance):
-            pass
-
-        fake_event(self.compute, self.context, instance=inst)
-
-        self.assertTrue(mock_start.called)
-        self.assertTrue(mock_finish.called)
-
-    @mock.patch.object(objects.InstanceActionEvent, 'event_start')
-    @mock.patch.object(objects.InstanceActionEvent,
-                       'event_finish_with_failure')
-    def test_wrap_instance_event_return(self, mock_finish, mock_start):
-        inst = {"uuid": uuids.instance}
-
-        @compute_manager.wrap_instance_event
-        def fake_event(self, context, instance):
-            return True
-
-        retval = fake_event(self.compute, self.context, instance=inst)
-
-        self.assertTrue(retval)
-        self.assertTrue(mock_start.called)
-        self.assertTrue(mock_finish.called)
-
-    @mock.patch.object(objects.InstanceActionEvent, 'event_start')
-    @mock.patch.object(objects.InstanceActionEvent,
-                       'event_finish_with_failure')
-    def test_wrap_instance_event_log_exception(self, mock_finish, mock_start):
-        inst = {"uuid": uuids.instance}
-
-        @compute_manager.wrap_instance_event
-        def fake_event(self2, context, instance):
-            raise exception.NovaException()
-
-        self.assertRaises(exception.NovaException, fake_event,
-                          self.compute, self.context, instance=inst)
-
-        self.assertTrue(mock_start.called)
-        self.assertTrue(mock_finish.called)
-        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()

--- a/nova/tests/unit/compute/test_compute_utils.py
+++ b/nova/tests/unit/compute/test_compute_utils.py
@ -642,6 +642,64 @@ class ComputeUtilsGetRebootTypes(test.NoDBTestCase):


 class ComputeUtilsTestCase(test.NoDBTestCase):
+
+    def setUp(self):
+        super(ComputeUtilsTestCase, self).setUp()
+        self.compute = 'compute'
+        self.user_id = 'fake'
+        self.project_id = 'fake'
+        self.context = context.RequestContext(self.user_id,
+                                              self.project_id)
+
+    @mock.patch.object(objects.InstanceActionEvent, 'event_start')
+    @mock.patch.object(objects.InstanceActionEvent,
+                       'event_finish_with_failure')
+    def test_wrap_instance_event(self, mock_finish, mock_start):
+        inst = {"uuid": uuids.instance}
+
+        @compute_utils.wrap_instance_event(prefix='compute')
+        def fake_event(self, context, instance):
+            pass
+
+        fake_event(self.compute, self.context, instance=inst)
+
+        self.assertTrue(mock_start.called)
+        self.assertTrue(mock_finish.called)
+
+    @mock.patch.object(objects.InstanceActionEvent, 'event_start')
+    @mock.patch.object(objects.InstanceActionEvent,
+                       'event_finish_with_failure')
+    def test_wrap_instance_event_return(self, mock_finish, mock_start):
+        inst = {"uuid": uuids.instance}
+
+        @compute_utils.wrap_instance_event(prefix='compute')
+        def fake_event(self, context, instance):
+            return True
+
+        retval = fake_event(self.compute, self.context, instance=inst)
+
+        self.assertTrue(retval)
+        self.assertTrue(mock_start.called)
+        self.assertTrue(mock_finish.called)
+
+    @mock.patch.object(objects.InstanceActionEvent, 'event_start')
+    @mock.patch.object(objects.InstanceActionEvent,
+                       'event_finish_with_failure')
+    def test_wrap_instance_event_log_exception(self, mock_finish, mock_start):
+        inst = {"uuid": uuids.instance}
+
+        @compute_utils.wrap_instance_event(prefix='compute')
+        def fake_event(self2, context, instance):
+            raise exception.NovaException()
+
+        self.assertRaises(exception.NovaException, fake_event,
+                          self.compute, self.context, instance=inst)
+
+        self.assertTrue(mock_start.called)
+        self.assertTrue(mock_finish.called)
+        args, kwargs = mock_finish.call_args
+        self.assertIsInstance(kwargs['exc_val'], exception.NovaException)
+
    @mock.patch('netifaces.interfaces')
    def test_get_machine_ips_value_error(self, mock_interfaces):
        # Tests that the utility method does not explode if netifaces raises
--- a/nova/tests/unit/conductor/test_conductor.py
+++ b/nova/tests/unit/conductor/test_conductor.py
@ -1309,13 +1309,18 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase):
                                self.conductor_manager.compute_rpcapi)

    def test_migrate_server_fails_with_rebuild(self):
+        instance = fake_instance.fake_instance_obj(self.context,
+                                                   vm_state=vm_states.ACTIVE)
        self.assertRaises(NotImplementedError, self.conductor.migrate_server,
-            self.context, None, None, True, True, None, None, None)
+            self.context, instance, None, True, True, None, None, None)

    def test_migrate_server_fails_with_flavor(self):
        flavor = flavors.get_flavor_by_name('m1.tiny')
+        instance = fake_instance.fake_instance_obj(self.context,
+                                                   vm_state=vm_states.ACTIVE,
+                                                   flavor=flavor)
        self.assertRaises(NotImplementedError, self.conductor.migrate_server,
-            self.context, None, None, True, False, flavor, None, None)
+            self.context, instance, None, True, False, flavor, None, None)

    def _build_request_spec(self, instance):
        return {