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
This commit is contained in:
parent
8c44c7a960
commit
f6c6dbdf9d
|
@ -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)
|
||||
event_handler.start_listener()
|
||||
|
||||
self._set_event_handler_callbacks()
|
||||
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()
|
||||
|
||||
|
|
|
@ -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 = (
|
||||
CONF.hyperv.enable_instance_metrics_collection and
|
||||
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)
|
||||
for callback in self._callbacks:
|
||||
utils.spawn_n(callback, virt_event)
|
||||
|
||||
utils.spawn_n(self._handle_serial_console_workers,
|
||||
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):
|
||||
def _get_virt_event(self, instance_uuid, instance_name, instance_state):
|
||||
transition = self._TRANSITION_MAP[instance_state]
|
||||
return virtevent.LifecycleEvent(uuid=instance_uuid,
|
||||
transition=transition)
|
||||
return HyperVLifecycleEvent(
|
||||
uuid=instance_uuid,
|
||||
name=instance_name,
|
||||
transition=transition)
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -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, mock_InstanceEventHandler):
|
||||
|
||||
def test_init_host(self):
|
||||
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(
|
||||
state_change_callback=self.driver.emit_event)
|
||||
fake_event_handler = mock_InstanceEventHandler.return_value
|
||||
fake_event_handler.start_listener.assert_called_once_with()
|
||||
self.driver._event_handler.add_callback.assert_has_calls(
|
||||
[mock.call(self.driver.emit_event),
|
||||
mock.call(self.driver._vmops.instance_state_change_callback)])
|
||||
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(
|
||||
|
|
|
@ -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._state_change_callback)
|
||||
self._event_handler = eventhandler.InstanceEventHandler()
|
||||
|
||||
@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,
|
||||
missing_uuid=False):
|
||||
def test_handle_event(self, missing_uuid, mock_emit_event, mock_get_uuid):
|
||||
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,
|
||||
mock.sentinel.power_state)
|
||||
self._event_handler._handle_event(mock.sentinel.instance_name,
|
||||
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')
|
||||
def test_emit_event(self, mock_spawn, mock_get_event):
|
||||
@mock.patch.object(utils, 'spawn_n',
|
||||
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,
|
||||
group="hyperv")
|
||||
callbacks = [mock.Mock(), mock.Mock()]
|
||||
|
||||
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
|
||||
mock_spawn.assert_has_calls(
|
||||
[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)])
|
||||
for cbk in callbacks:
|
||||
cbk.assert_called_once_with(mock_get_event.return_value)
|
||||
|
||||
def test_handle_serial_console_instance_running(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):
|
||||
def test_get_virt_event(self):
|
||||
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,
|
||||
virt_event)
|
||||
mock_lifecycle_event.assert_called_once_with(
|
||||
uuid=mock.sentinel.instance_uuid,
|
||||
transition=expected_transition)
|
||||
self.assertEqual(mock.sentinel.instance_name, virt_event.name)
|
||||
self.assertEqual(mock.sentinel.instance_uuid, virt_event.uuid)
|
||||
self.assertEqual(expected_transition, virt_event.transition)
|
||||
|
|
|
@ -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)
|
||||
|
|
Loading…
Reference in New Issue