Refactor instance state change event handler

We ended up having too much logic in the event handler. This change
trims it down, making it easier to add callbacks.

The actions taken when receiving events are now moved out of the
event handler, being registered as callbacks.

Related-Bug: #1799163
Change-Id: Ie93aa0e184c5ff9daef4e857e212bb54e9473a78

compute_hyperv/nova/driver.py

@@ -126,6 +126,7 @@ class HyperVDriver(driver.ComputeDriver):
         self._imagecache = imagecache.ImageCache()
         self._image_api = image.API()
         self._pathutils = pathutils.PathUtils()
+        self._event_handler = eventhandler.InstanceEventHandler()
 
     def _check_minimum_windows_version(self):
         hostutils = utilsfactory.get_hostutils()
@@ -151,13 +152,19 @@ class HyperVDriver(driver.ComputeDriver):
 
     def init_host(self, host):
         self._serialconsoleops.start_console_handlers()
-        event_handler = eventhandler.InstanceEventHandler(
+
-            state_change_callback=self.emit_event)
+        self._set_event_handler_callbacks()
-        event_handler.start_listener()
+        self._event_handler.start_listener()
 
         instance_path = self._pathutils.get_instances_dir()
         self._pathutils.check_create_dir(instance_path)
 
+    def _set_event_handler_callbacks(self):
+        # Subclasses may override this.
+        self._event_handler.add_callback(self.emit_event)
+        self._event_handler.add_callback(
+            self._vmops.instance_state_change_callback)
+
     def list_instance_uuids(self):
         return self._vmops.list_instance_uuids()
 

compute_hyperv/nova/eventhandler.py

@@ -20,7 +20,6 @@ from os_win import utilsfactory
 from oslo_log import log as logging
 
 import compute_hyperv.nova.conf
-from compute_hyperv.nova import serialconsoleops
 from compute_hyperv.nova import vmops
 
 LOG = logging.getLogger(__name__)
@@ -28,6 +27,13 @@ LOG = logging.getLogger(__name__)
 CONF = compute_hyperv.nova.conf.CONF
 
 
+class HyperVLifecycleEvent(virtevent.LifecycleEvent):
+    def __init__(self, uuid, name, transition, timestamp=None):
+        super(HyperVLifecycleEvent, self).__init__(uuid, transition, timestamp)
+
+        self.name = name
+
+
 class InstanceEventHandler(object):
     _TRANSITION_MAP = {
         constants.HYPERV_VM_STATE_ENABLED: virtevent.EVENT_LIFECYCLE_STARTED,
@@ -37,7 +43,7 @@ class InstanceEventHandler(object):
             virtevent.EVENT_LIFECYCLE_SUSPENDED
     }
 
-    def __init__(self, state_change_callback=None):
+    def __init__(self):
         self._vmutils = utilsfactory.get_vmutils()
         self._listener = self._vmutils.get_vm_power_state_change_listener(
             timeframe=CONF.hyperv.power_state_check_timeframe,
@@ -46,13 +52,16 @@ class InstanceEventHandler(object):
             get_handler=True)
 
         self._vmops = vmops.VMOps()
-        self._serial_console_ops = serialconsoleops.SerialConsoleOps()
+
-        self._state_change_callback = state_change_callback
+        self._callbacks = []
+
+    def add_callback(self, callback):
+        self._callbacks.append(callback)
 
     def start_listener(self):
-        utils.spawn_n(self._listener, self._event_callback)
+        utils.spawn_n(self._listener, self._handle_event)
 
-    def _event_callback(self, instance_name, instance_power_state):
+    def _handle_event(self, instance_name, instance_power_state):
         # Instance uuid set by Nova. If this is missing, we assume that
         # the instance was not created by Nova and ignore the event.
         instance_uuid = self._vmops.get_instance_uuid(instance_name)
@@ -69,27 +78,15 @@ class InstanceEventHandler(object):
 
     def _emit_event(self, instance_name, instance_uuid, instance_state):
         virt_event = self._get_virt_event(instance_uuid,
+                                          instance_name,
                                           instance_state)
-        utils.spawn_n(self._state_change_callback, virt_event)
 
-        should_enable_metrics = (
+        for callback in self._callbacks:
-            CONF.hyperv.enable_instance_metrics_collection and
+            utils.spawn_n(callback, virt_event)
-            instance_state == constants.HYPERV_VM_STATE_ENABLED)
-        if should_enable_metrics:
-            utils.spawn_n(self._vmops.configure_instance_metrics,
-                          instance_name,
-                          enable_network_metrics=True)
 
-        utils.spawn_n(self._handle_serial_console_workers,
+    def _get_virt_event(self, instance_uuid, instance_name, instance_state):
-                      instance_name, instance_state)
-
-    def _handle_serial_console_workers(self, instance_name, instance_state):
-        if instance_state == constants.HYPERV_VM_STATE_ENABLED:
-            self._serial_console_ops.start_console_handler(instance_name)
-        else:
-            self._serial_console_ops.stop_console_handler(instance_name)
-
-    def _get_virt_event(self, instance_uuid, instance_state):
         transition = self._TRANSITION_MAP[instance_state]
-        return virtevent.LifecycleEvent(uuid=instance_uuid,
+        return HyperVLifecycleEvent(
-                                        transition=transition)
+            uuid=instance_uuid,
+            name=instance_name,
+            transition=transition)

compute_hyperv/nova/vmops.py

@@ -30,6 +30,7 @@ from nova import objects
 from nova.objects import fields
 from nova import utils
 from nova.virt import configdrive
+from nova.virt import event as virtevent
 from nova.virt import hardware
 from os_win import constants as os_win_const
 from os_win import exceptions as os_win_exc
@@ -1466,3 +1467,15 @@ class VMOps(object):
                           "its uuid. It may have been deleted meanwhile.",
                           instance_name)
                 ctxt.reraise = expect_existing
+
+    def instance_state_change_callback(self, event):
+        if event.transition in (virtevent.EVENT_LIFECYCLE_STARTED,
+                                virtevent.EVENT_LIFECYCLE_RESUMED):
+            # We can handle the following operations concurrently.
+            utils.spawn_n(self._serial_console_ops.start_console_handler,
+                          event.name)
+            utils.spawn_n(self.configure_instance_metrics,
+                          event.name,
+                          enable_network_metrics=True)
+        else:
+            self._serial_console_ops.stop_console_handler(event.name)

compute_hyperv/tests/unit/test_driver.py

@@ -35,6 +35,7 @@ from compute_hyperv.tests.unit import test_base
 class HyperVDriverTestCase(test_base.HyperVBaseTestCase):
 
     _autospec_classes = [
+        driver.eventhandler.InstanceEventHandler,
         driver.hostops.HostOps,
         driver.volumeops.VolumeOps,
         driver.vmops.VMOps,
@@ -124,9 +125,7 @@ class HyperVDriverTestCase(test_base.HyperVBaseTestCase):
             # original frame will contain the 'foo' variable.
             self.assertEqual('foofoo', trace.tb_frame.f_locals['foo'])
 
-    @mock.patch.object(driver.eventhandler, 'InstanceEventHandler')
+    def test_init_host(self):
-    def test_init_host(self, mock_InstanceEventHandler):
-
         mock_get_inst_dir = self.driver._pathutils.get_instances_dir
         mock_get_inst_dir.return_value = mock.sentinel.FAKE_DIR
 
@@ -135,10 +134,10 @@ class HyperVDriverTestCase(test_base.HyperVBaseTestCase):
         mock_start_console_handlers = (
             self.driver._serialconsoleops.start_console_handlers)
         mock_start_console_handlers.assert_called_once_with()
-        mock_InstanceEventHandler.assert_called_once_with(
+        self.driver._event_handler.add_callback.assert_has_calls(
-            state_change_callback=self.driver.emit_event)
+            [mock.call(self.driver.emit_event),
-        fake_event_handler = mock_InstanceEventHandler.return_value
+             mock.call(self.driver._vmops.instance_state_change_callback)])
-        fake_event_handler.start_listener.assert_called_once_with()
+        self.driver._event_handler.start_listener.assert_called_once_with()
 
         mock_get_inst_dir.assert_called_once_with()
         self.driver._pathutils.check_create_dir.assert_called_once_with(

compute_hyperv/tests/unit/test_eventhandler.py

@@ -13,9 +13,9 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import ddt
 import mock
 from nova import utils
-from nova.virt import driver
 from os_win import constants
 
 from compute_hyperv.nova import eventhandler
@@ -23,20 +23,14 @@ from compute_hyperv.nova import vmops
 from compute_hyperv.tests.unit import test_base
 
 
+@ddt.ddt
 class EventHandlerTestCase(test_base.HyperVBaseTestCase):
-
-    _autospec_classes = [
-        eventhandler.serialconsoleops.SerialConsoleOps,
-    ]
-
     _FAKE_POLLING_INTERVAL = 3
     _FAKE_EVENT_CHECK_TIMEFRAME = 15
 
     def setUp(self):
         super(EventHandlerTestCase, self).setUp()
 
-        self._state_change_callback = mock.Mock(
-            autospec=driver.ComputeDriver.emit_event)
         self.flags(
             power_state_check_timeframe=self._FAKE_EVENT_CHECK_TIMEFRAME,
             group='hyperv')
@@ -44,20 +38,19 @@ class EventHandlerTestCase(test_base.HyperVBaseTestCase):
             power_state_event_polling_interval=self._FAKE_POLLING_INTERVAL,
             group='hyperv')
 
-        self._event_handler = eventhandler.InstanceEventHandler(
+        self._event_handler = eventhandler.InstanceEventHandler()
-            self._state_change_callback)
 
+    @ddt.data(True, False)
     @mock.patch.object(vmops.VMOps, 'get_instance_uuid')
     @mock.patch.object(eventhandler.InstanceEventHandler, '_emit_event')
-    def _test_event_callback(self, mock_emit_event, mock_get_uuid,
+    def test_handle_event(self, missing_uuid, mock_emit_event, mock_get_uuid):
-                             missing_uuid=False):
         mock_get_uuid.return_value = (
             mock.sentinel.instance_uuid if not missing_uuid else None)
         self._event_handler._vmutils.get_vm_power_state.return_value = (
             mock.sentinel.power_state)
 
-        self._event_handler._event_callback(mock.sentinel.instance_name,
+        self._event_handler._handle_event(mock.sentinel.instance_name,
-                                            mock.sentinel.power_state)
+                                          mock.sentinel.power_state)
 
         if not missing_uuid:
             mock_emit_event.assert_called_once_with(
@@ -67,60 +60,33 @@ class EventHandlerTestCase(test_base.HyperVBaseTestCase):
         else:
             self.assertFalse(mock_emit_event.called)
 
-    def test_event_callback_uuid_present(self):
-        self._test_event_callback()
-
-    def test_event_callback_missing_uuid(self):
-        self._test_event_callback(missing_uuid=True)
-
     @mock.patch.object(eventhandler.InstanceEventHandler, '_get_virt_event')
-    @mock.patch.object(utils, 'spawn_n')
+    @mock.patch.object(utils, 'spawn_n',
-    def test_emit_event(self, mock_spawn, mock_get_event):
+                       lambda f, *args, **kwargs: f(*args, **kwargs))
+    def test_emit_event(self, mock_get_event):
         state = constants.HYPERV_VM_STATE_ENABLED
-        self.flags(enable_instance_metrics_collection=True,
+        callbacks = [mock.Mock(), mock.Mock()]
-                   group="hyperv")
+
+        for cbk in callbacks:
+            self._event_handler.add_callback(cbk)
 
         self._event_handler._emit_event(mock.sentinel.instance_name,
                                         mock.sentinel.instance_uuid,
                                         state)
 
-        virt_event = mock_get_event.return_value
+        for cbk in callbacks:
-        mock_spawn.assert_has_calls(
+            cbk.assert_called_once_with(mock_get_event.return_value)
-            [mock.call(self._state_change_callback, virt_event),
-             mock.call(self._event_handler._vmops.configure_instance_metrics,
-                       mock.sentinel.instance_name,
-                       enable_network_metrics=True),
-             mock.call(self._event_handler._handle_serial_console_workers,
-                       mock.sentinel.instance_name,
-                       state)])
 
-    def test_handle_serial_console_instance_running(self):
+    def test_get_virt_event(self):
-        self._event_handler._handle_serial_console_workers(
-            mock.sentinel.instance_name,
-            constants.HYPERV_VM_STATE_ENABLED)
-        serialops = self._event_handler._serial_console_ops
-        serialops.start_console_handler.assert_called_once_with(
-            mock.sentinel.instance_name)
-
-    def test_handle_serial_console_instance_stopped(self):
-        self._event_handler._handle_serial_console_workers(
-            mock.sentinel.instance_name,
-            constants.HYPERV_VM_STATE_DISABLED)
-        serialops = self._event_handler._serial_console_ops
-        serialops.stop_console_handler.assert_called_once_with(
-            mock.sentinel.instance_name)
-
-    @mock.patch('nova.virt.event.LifecycleEvent')
-    def test_get_virt_event(self, mock_lifecycle_event):
         instance_state = constants.HYPERV_VM_STATE_ENABLED
         expected_transition = self._event_handler._TRANSITION_MAP[
             instance_state]
 
         virt_event = self._event_handler._get_virt_event(
-            mock.sentinel.instance_uuid, instance_state)
+            mock.sentinel.instance_uuid,
+            mock.sentinel.instance_name,
+            instance_state)
 
-        self.assertEqual(mock_lifecycle_event.return_value,
+        self.assertEqual(mock.sentinel.instance_name, virt_event.name)
-                         virt_event)
+        self.assertEqual(mock.sentinel.instance_uuid, virt_event.uuid)
-        mock_lifecycle_event.assert_called_once_with(
+        self.assertEqual(expected_transition, virt_event.transition)
-            uuid=mock.sentinel.instance_uuid,
-            transition=expected_transition)

compute_hyperv/tests/unit/test_vmops.py

@@ -22,6 +22,8 @@ from nova import exception
 from nova import objects
 from nova.objects import fields
 from nova.tests.unit.objects import test_virtual_interface
+from nova import utils
+from nova.virt import event as virtevent
 from nova.virt import hardware
 from os_win import constants as os_win_const
 from os_win import exceptions as os_win_exc
@@ -31,6 +33,7 @@ from oslo_utils import units
 
 import compute_hyperv.nova.conf
 from compute_hyperv.nova import constants
+from compute_hyperv.nova import eventhandler
 from compute_hyperv.nova import vmops
 from compute_hyperv.tests import fake_instance
 from compute_hyperv.tests.unit import test_base
@@ -2420,3 +2423,25 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
                           self._vmops.get_instance_uuid,
                           mock.sentinel.instance_name,
                           expect_existing=True)
+
+    @ddt.data(virtevent.EVENT_LIFECYCLE_STARTED,
+              virtevent.EVENT_LIFECYCLE_STOPPED)
+    @mock.patch.object(vmops.VMOps, 'configure_instance_metrics')
+    @mock.patch.object(utils, 'spawn_n',
+                       lambda f, *args, **kwargs: f(*args, **kwargs))
+    def test_instance_state_change_callback(self, transition,
+                                            mock_configure_metrics):
+        event = eventhandler.HyperVLifecycleEvent(
+            mock.sentinel.uuid,
+            mock.sentinel.name,
+            transition)
+
+        self._vmops.instance_state_change_callback(event)
+
+        serialops = self._vmops._serial_console_ops
+        if transition == virtevent.EVENT_LIFECYCLE_STARTED:
+            serialops.start_console_handler.assert_called_once_with(event.name)
+            mock_configure_metrics.assert_called_once_with(
+                event.name, enable_network_metrics=True)
+        else:
+            serialops.stop_console_handler.assert_called_once_with(event.name)