From a0e07a77d0242ef0ceb1b0f496db3bde081b6b8d Mon Sep 17 00:00:00 2001 From: Valeriy Ponomaryov Date: Tue, 1 Sep 2015 19:13:51 +0300 Subject: [PATCH] Add share hooks Add new feature called 'hooks', that allows to: - Perform actions before some share driver methods calls. - Perform actions after some share driver methods calls with results of driver call and preceding hook call. - Call additional 'periodic' hook each 'N' ticks. - Possibility to update results of driver's action by post-running hook. Features of hooks: - Errors can be suppressed. - Any of hooks can be disabled. - Any amount of hook instances can be run. Known limitations: - Hooks are not asynchronous. It means, if we run hooks, and especially, more than one instance, then all of them will be executed in one thread. Implements bp mount-automation-framework Change-Id: I7f496ac49e828f361c18ff89c5a308d698f2a4aa --- manila/opts.py | 2 + manila/share/driver.py | 15 ++ manila/share/hook.py | 166 +++++++++++++++ manila/share/hooks/__init__.py | 0 manila/share/manager.py | 75 +++++++ manila/tests/share/test_driver.py | 9 + manila/tests/share/test_hook.py | 321 +++++++++++++++++++++++++++++ manila/tests/share/test_manager.py | 94 +++++++++ 8 files changed, 682 insertions(+) create mode 100644 manila/share/hook.py create mode 100644 manila/share/hooks/__init__.py create mode 100644 manila/tests/share/test_hook.py diff --git a/manila/opts.py b/manila/opts.py index 2b0d066ce6..b2ccb14985 100644 --- a/manila/opts.py +++ b/manila/opts.py @@ -67,6 +67,7 @@ import manila.share.drivers.service_instance import manila.share.drivers.windows.winrm_helper import manila.share.drivers.zfssa.zfssashare import manila.share.drivers_private_data +import manila.share.hook import manila.share.manager import manila.volume import manila.volume.cinder @@ -129,6 +130,7 @@ _global_opt_lists = [ manila.share.drivers.service_instance.share_servers_handling_mode_opts, manila.share.drivers.windows.winrm_helper.winrm_opts, manila.share.drivers.zfssa.zfssashare.ZFSSA_OPTS, + manila.share.hook.hook_options, manila.share.manager.share_manager_opts, manila.volume._volume_opts, manila.volume.cinder.cinder_opts, diff --git a/manila/share/driver.py b/manila/share/driver.py index 8b1185aa05..91ecca13aa 100644 --- a/manila/share/driver.py +++ b/manila/share/driver.py @@ -511,3 +511,18 @@ class ShareDriver(object): :param share_server: ShareServer class instance. """ return [] + + def get_periodic_hook_data(self, context, share_instances): + """Dedicated for update/extend of data for existing share instances. + + Redefine this method in share driver to be able to update/change/extend + share instances data that will be used by periodic hook action. + One of possible updates is add-on of "automount" CLI commands for each + share instance for case of notification is enabled using 'hook' + approach. + + :param context: Current context + :param share_instances: share instances list provided by share manager + :return: list of share instances. + """ + return share_instances diff --git a/manila/share/hook.py b/manila/share/hook.py new file mode 100644 index 0000000000..2d762747d0 --- /dev/null +++ b/manila/share/hook.py @@ -0,0 +1,166 @@ +# Copyright (c) 2015 Mirantis, Inc. +# All Rights Reserved. +# +# 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. + +""" +Module with hook interface for actions performed by share driver. + +All available hooks are placed in manila/share/hooks dir. + +Hooks are used by share services and can serve several use cases such as +any kind of notification and performing additional backend-specific actions. +""" + +import abc + +from oslo_config import cfg +from oslo_log import log +import six + +from manila import context as ctxt +from manila.i18n import _LW + + +hook_options = [ + cfg.BoolOpt( + "enable_pre_hooks", + default=False, + help="Whether to enable pre hooks or not.", + deprecated_group='DEFAULT'), + cfg.BoolOpt( + "enable_post_hooks", + default=False, + help="Whether to enable post hooks or not.", + deprecated_group='DEFAULT'), + cfg.BoolOpt( + "enable_periodic_hooks", + default=False, + help="Whether to enable periodic hooks or not.", + deprecated_group='DEFAULT'), + cfg.BoolOpt( + "suppress_pre_hooks_errors", + default=False, + help="Whether to suppress pre hook errors (allow driver perform " + "actions) or not.", + deprecated_group='DEFAULT'), + cfg.BoolOpt( + "suppress_post_hooks_errors", + default=False, + help="Whether to suppress post hook errors (allow driver's results " + "to pass through) or not.", + deprecated_group='DEFAULT'), + cfg.FloatOpt( + "periodic_hooks_interval", + default=300.0, + help="Interval in seconds between execution of periodic hooks. " + "Used when option 'enable_periodic_hooks' is set to True. " + "Default is 300.", + deprecated_group='DEFAULT'), +] + +CONF = cfg.CONF +CONF.register_opts(hook_options) +LOG = log.getLogger(__name__) + + +@six.add_metaclass(abc.ABCMeta) +class HookBase(object): + + def get_config_option(self, key): + if self.configuration: + return self.configuration.safe_get(key) + return CONF.get(key) + + def __init__(self, configuration, host): + self.host = host + self.configuration = configuration + if self.configuration: + self.configuration.append_config_values(hook_options) + + self.pre_hooks_enabled = self.get_config_option("enable_pre_hooks") + self.post_hooks_enabled = self.get_config_option("enable_post_hooks") + self.periodic_hooks_enabled = self.get_config_option( + "enable_periodic_hooks") + self.suppress_pre_hooks_errors = self.get_config_option( + "suppress_pre_hooks_errors") + self.suppress_post_hooks_errors = self.get_config_option( + "suppress_post_hooks_errors") + + def execute_pre_hook(self, context=None, func_name=None, *args, **kwargs): + """Hook called before driver's action.""" + if not self.pre_hooks_enabled: + return + LOG.debug("Running 'pre hook'.") + context = context or ctxt.get_admin_context() + try: + pre_data = self._execute_pre_hook( + context=context, + func_name=func_name, + *args, **kwargs) + except Exception as e: + if self.suppress_pre_hooks_errors: + LOG.warning(_LW("\nSuppressed exception in pre hook. %s\n"), e) + pre_data = e + else: + raise + return pre_data + + def execute_post_hook(self, context=None, func_name=None, + pre_hook_data=None, driver_action_results=None, + *args, **kwargs): + """Hook called after driver's action.""" + if not self.post_hooks_enabled: + return + LOG.debug("Running 'post hook'.") + context = context or ctxt.get_admin_context() + + try: + post_data = self._execute_post_hook( + context=context, + func_name=func_name, + pre_hook_data=pre_hook_data, + driver_action_results=driver_action_results, + *args, **kwargs) + except Exception as e: + if self.suppress_post_hooks_errors: + LOG.warning( + _LW("\nSuppressed exception in post hook. %s\n"), e) + post_data = e + else: + raise + return post_data + + def execute_periodic_hook(self, context, periodic_hook_data, + *args, **kwargs): + """Hook called on periodic basis.""" + if not self.periodic_hooks_enabled: + return + LOG.debug("Running 'periodic hook'.") + context = context or ctxt.get_admin_context() + return self._execute_periodic_hook( + context, periodic_hook_data, *args, **kwargs) + + @abc.abstractmethod + def _execute_pre_hook(self, context, func_name, *args, **kwargs): + """Redefine this method for pre hook action.""" + + @abc.abstractmethod + def _execute_post_hook(self, context, func_name, pre_hook_data, + driver_action_results, *args, **kwargs): + """Redefine this method for post hook action.""" + + @abc.abstractmethod + def _execute_periodic_hook(self, context, periodic_hook_data, + *args, **kwargs): + """Redefine this method for periodic hook action.""" diff --git a/manila/share/hooks/__init__.py b/manila/share/hooks/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/manila/share/manager.py b/manila/share/manager.py index e568c3b8ca..c372e9c33e 100644 --- a/manila/share/manager.py +++ b/manila/share/manager.py @@ -50,6 +50,12 @@ share_manager_opts = [ cfg.StrOpt('share_driver', default='manila.share.drivers.generic.GenericShareDriver', help='Driver to use for share creation.'), + cfg.ListOpt('hook_drivers', + default=[], + help='Driver(s) to perform some additional actions before and ' + 'after share driver actions and on a periodic basis. ' + 'Default is [].', + deprecated_group='DEFAULT'), cfg.BoolOpt('delete_share_server_with_last_share', default=False, help='Whether share servers will ' @@ -81,6 +87,7 @@ share_manager_opts = [ CONF = cfg.CONF CONF.register_opts(share_manager_opts) +CONF.import_opt('periodic_hooks_interval', 'manila.share.hook') # Drivers that need to change module paths or class names can add their # old/new path here to maintain backward compatibility. @@ -91,6 +98,33 @@ MAPPING = { QUOTAS = quota.QUOTAS +def add_hooks(f): + + def wrapped(self, *args, **kwargs): + if not self.hooks: + return f(self, *args, **kwargs) + + pre_hook_results = [] + for hook in self.hooks: + pre_hook_results.append( + hook.execute_pre_hook( + func_name=f.__name__, + *args, **kwargs)) + + wrapped_func_results = f(self, *args, **kwargs) + + for i, hook in enumerate(self.hooks): + hook.execute_post_hook( + func_name=f.__name__, + driver_action_results=wrapped_func_results, + pre_hook_data=pre_hook_results[i], + *args, **kwargs) + + return wrapped_func_results + + return wrapped + + class ShareManager(manager.SchedulerDependentManager): """Manages NAS storages.""" @@ -125,6 +159,21 @@ class ShareManager(manager.SchedulerDependentManager): configuration=self.configuration ) + self.hooks = [] + self._init_hook_drivers() + + def _init_hook_drivers(self): + # Try to initialize hook driver(s). + hook_drivers = self.configuration.safe_get("hook_drivers") + for hook_driver in hook_drivers: + self.hooks.append( + importutils.import_object( + hook_driver, + configuration=self.configuration, + host=self.host, + ) + ) + def _ensure_share_instance_has_pool(self, ctxt, share_instance): pool = share_utils.extract_host(share_instance['host'], 'pool') if pool is None: @@ -148,6 +197,7 @@ class ShareManager(manager.SchedulerDependentManager): return pool + @add_hooks def init_host(self): """Initialization for a standalone service.""" @@ -357,6 +407,7 @@ class ShareManager(manager.SchedulerDependentManager): id = share.instance['id'] return self.db.share_instance_get(context, id, with_share_data=True) + @add_hooks def create_share_instance(self, context, share_instance_id, request_spec=None, filter_properties=None, snapshot_id=None): @@ -452,6 +503,7 @@ class ShareManager(manager.SchedulerDependentManager): 'launched_at': timeutils.utcnow()} ) + @add_hooks def manage_share(self, context, share_id, driver_options): context = context.elevated() share_ref = self.db.share_get(context, share_id) @@ -516,6 +568,7 @@ class ShareManager(manager.SchedulerDependentManager): self.db.quota_usage_create(context, project_id, user_id, resource, usage) + @add_hooks def unmanage_share(self, context, share_id): context = context.elevated() share_ref = self.db.share_get(context, share_id) @@ -573,6 +626,7 @@ class ShareManager(manager.SchedulerDependentManager): {'status': constants.STATUS_UNMANAGED, 'deleted': True}) + @add_hooks def delete_share_instance(self, context, share_instance_id): """Delete a share instance.""" context = context.elevated() @@ -627,6 +681,7 @@ class ShareManager(manager.SchedulerDependentManager): self._deny_access(context, access_ref, share_instance, share_server) + @add_hooks def create_snapshot(self, context, share_id, snapshot_id): """Create snapshot for share.""" snapshot_ref = self.db.share_snapshot_get(context, snapshot_id) @@ -661,6 +716,7 @@ class ShareManager(manager.SchedulerDependentManager): ) return snapshot_id + @add_hooks def delete_snapshot(self, context, snapshot_id): """Delete share snapshot.""" context = context.elevated() @@ -705,6 +761,7 @@ class ShareManager(manager.SchedulerDependentManager): if reservations: QUOTAS.commit(context, reservations, project_id=project_id) + @add_hooks def allow_access(self, context, share_instance_id, access_id): """Allow access to some share instance.""" access_mapping = self.db.share_instance_access_get(context, access_id, @@ -727,6 +784,7 @@ class ShareManager(manager.SchedulerDependentManager): self.db.share_instance_access_update_state( context, access_mapping['id'], access_mapping.STATE_ERROR) + @add_hooks def deny_access(self, context, share_instance_id, access_id): """Deny access to some share.""" access_ref = self.db.share_access_get(context, access_id) @@ -761,6 +819,19 @@ class ShareManager(manager.SchedulerDependentManager): self.update_service_capabilities(share_stats) + @periodic_task.periodic_task(spacing=CONF.periodic_hooks_interval) + def _execute_periodic_hook(self, context): + """Executes periodic-based hooks.""" + # TODO(vponomaryov): add also access rules and share servers + share_instances = ( + self.db.share_instances_get_all_by_host( + context=context, host=self.host)) + periodic_hook_data = self.driver.get_periodic_hook_data( + context=context, share_instances=share_instances) + for hook in self.hooks: + hook.execute_periodic_hook( + context=context, periodic_hook_data=periodic_hook_data) + def _get_servers_pool_mapping(self, context): """Get info about relationships between pools and share_servers.""" share_servers = self.db.share_server_get_all_by_host(context, @@ -768,6 +839,7 @@ class ShareManager(manager.SchedulerDependentManager): return dict((server['id'], self.driver.get_share_server_pools(server)) for server in share_servers) + @add_hooks def publish_service_capabilities(self, context): """Collect driver status and then publish it.""" self._report_driver_status(context) @@ -914,6 +986,7 @@ class ShareManager(manager.SchedulerDependentManager): raise exception.NetworkBadConfigurationException( reason=msg % network_info['segmentation_id']) + @add_hooks def delete_share_server(self, context, share_server): @utils.synchronized( @@ -971,6 +1044,7 @@ class ShareManager(manager.SchedulerDependentManager): "Option unused_share_server_cleanup_interval should be " "between 10 minutes and 1 hour.") + @add_hooks def extend_share(self, context, share_id, new_size, reservations): context = context.elevated() share = self.db.share_get(context, share_id) @@ -1006,6 +1080,7 @@ class ShareManager(manager.SchedulerDependentManager): LOG.info(_LI("Extend share completed successfully."), resource=share) + @add_hooks def shrink_share(self, context, share_id, new_size): context = context.elevated() share = self.db.share_get(context, share_id) diff --git a/manila/tests/share/test_driver.py b/manila/tests/share/test_driver.py index 742f079b59..b1f65e3d19 100644 --- a/manila/tests/share/test_driver.py +++ b/manila/tests/share/test_driver.py @@ -305,3 +305,12 @@ class ShareDriverTestCase(test.TestCase): snapshots_are_supported, child_class_instance._stats["snapshot_support"]) self.assertTrue(child_class_instance.configuration.safe_get.called) + + def test_get_periodic_hook_data(self): + share_driver = self._instantiate_share_driver(None, False) + share_instances = ["list", "of", "share", "instances"] + + result = share_driver.get_periodic_hook_data( + "fake_context", share_instances) + + self.assertEqual(share_instances, result) diff --git a/manila/tests/share/test_hook.py b/manila/tests/share/test_hook.py new file mode 100644 index 0000000000..1d3dceadeb --- /dev/null +++ b/manila/tests/share/test_hook.py @@ -0,0 +1,321 @@ +# Copyright 2015 Mirantis Inc. +# All Rights Reserved. +# +# 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 +import mock + +from manila import context +from manila.share import hook +from manila import test + + +class FakeHookImplementation(hook.HookBase): + def _execute_pre_hook(self, context, func_name, *args, **kwargs): + """Fake implementation of a pre hook action.""" + + def _execute_post_hook(self, context, func_name, pre_hook_data, + driver_action_results, *args, **kwargs): + """Fake implementation of a post hook action.""" + + def _execute_periodic_hook(self, context, periodic_hook_data, + *args, **kwargs): + """Fake implementation of a periodic hook action.""" + + +@ddt.ddt +class HookBaseTestCase(test.TestCase): + + def setUp(self): + super(HookBaseTestCase, self).setUp() + self.context = context.get_admin_context() + self.default_config = { + "enable_pre_hooks": "fake_enable_pre_hooks", + "enable_post_hooks": "fake_enable_post_hooks", + "enable_periodic_hooks": "fake_enable_periodic_hooks", + "suppress_pre_hooks_errors": "fake_suppress_pre_hook_errors", + "suppress_post_hooks_errors": "fake_suppress_post_hook_errors", + } + for k, v in self.default_config.items(): + hook.CONF.set_default(k, v) + + def _fake_safe_get(self, key): + return self.default_config.get(key) + "_safe_get" + + def _get_hook_instance(self, set_configuration=True, host="fake_host"): + if set_configuration: + configuration = mock.Mock() + configuration.safe_get.side_effect = self._fake_safe_get + else: + configuration = None + instance = FakeHookImplementation( + configuration=configuration, host=host) + return instance + + def test_instantiate_hook_fail(self): + self.assertRaises(TypeError, hook.HookBase) + + @ddt.data(True, False) + def test_instantiate_hook_successfully_and_set_configuration( + self, set_configuration): + instance = self._get_hook_instance(set_configuration) + + self.assertTrue(hasattr(instance, 'host')) + self.assertEqual("fake_host", instance.host) + self.assertTrue(hasattr(instance, 'configuration')) + if not set_configuration: + self.assertIsNone(instance.configuration) + for attr_name in ("pre_hooks_enabled", + "post_hooks_enabled", + "periodic_hooks_enabled", + "suppress_pre_hooks_errors", + "suppress_post_hooks_errors"): + self.assertTrue(hasattr(instance, attr_name)) + if set_configuration: + instance.configuration.append_config_values.assert_has_calls([ + mock.call(hook.hook_options)]) + conf_func = self._fake_safe_get + else: + conf_func = self.default_config.get + self.assertEqual( + conf_func("enable_pre_hooks"), instance.pre_hooks_enabled) + self.assertEqual( + conf_func("enable_post_hooks"), instance.post_hooks_enabled) + self.assertEqual( + conf_func("enable_periodic_hooks"), + instance.periodic_hooks_enabled) + self.assertEqual( + conf_func("suppress_pre_hooks_errors"), + instance.suppress_pre_hooks_errors) + self.assertEqual( + conf_func("suppress_post_hooks_errors"), + instance.suppress_post_hooks_errors) + + def test_execute_pre_hook_disabled(self): + instance = self._get_hook_instance() + instance.pre_hooks_enabled = False + self.mock_object( + instance, "_execute_pre_hook", + mock.Mock(side_effect=Exception("I should not be raised."))) + + result = instance.execute_pre_hook( + self.context, "fake_func_name", "some_arg", some_kwarg="foo") + + self.assertIsNone(result) + + @ddt.data(True, False) + def test_execute_pre_hook_success(self, provide_context): + instance = self._get_hook_instance() + instance.pre_hooks_enabled = True + instance.suppress_pre_hooks_errors = True + expected = "fake_expected_result" + some_arg = "some_arg" + func_name = "fake_func_name" + self.mock_object(hook.LOG, 'error') + self.mock_object( + instance, "_execute_pre_hook", mock.Mock(return_value=expected)) + mock_ctxt = self.mock_object(context, 'get_admin_context') + ctxt = self.context if provide_context else mock_ctxt + + result = instance.execute_pre_hook( + ctxt, func_name, some_arg, some_kwarg="foo") + + self.assertEqual(expected, result) + instance._execute_pre_hook.assert_called_once_with( + some_arg, + context=self.context if provide_context else mock_ctxt, + func_name=func_name, + some_kwarg="foo") + self.assertFalse(hook.LOG.error.called) + + def test_execute_pre_hook_exception_with_suppression(self): + instance = self._get_hook_instance() + instance.pre_hooks_enabled = True + instance.suppress_pre_hooks_errors = True + some_arg = "some_arg" + func_name = "fake_func_name" + FakeException = type("FakeException", (Exception, ), {}) + self.mock_object(hook.LOG, 'warning') + self.mock_object( + instance, "_execute_pre_hook", mock.Mock(side_effect=( + FakeException("Some exception that should be suppressed.")))) + + result = instance.execute_pre_hook( + self.context, func_name, some_arg, some_kwarg="foo") + + self.assertIsInstance(result, FakeException) + instance._execute_pre_hook.assert_called_once_with( + some_arg, + context=self.context, + func_name=func_name, + some_kwarg="foo") + self.assertTrue(hook.LOG.warning.called) + + def test_execute_pre_hook_exception_without_suppression(self): + instance = self._get_hook_instance() + instance.pre_hooks_enabled = True + instance.suppress_pre_hooks_errors = False + some_arg = "some_arg" + func_name = "fake_func_name" + FakeException = type("FakeException", (Exception, ), {}) + self.mock_object(hook.LOG, 'warning') + self.mock_object( + instance, "_execute_pre_hook", mock.Mock(side_effect=( + FakeException( + "Some exception that should NOT be suppressed.")))) + + self.assertRaises( + FakeException, + instance.execute_pre_hook, + self.context, func_name, some_arg, some_kwarg="foo") + + instance._execute_pre_hook.assert_called_once_with( + some_arg, + context=self.context, + func_name=func_name, + some_kwarg="foo") + self.assertFalse(hook.LOG.warning.called) + + def test_execute_post_hook_disabled(self): + instance = self._get_hook_instance() + instance.post_hooks_enabled = False + self.mock_object( + instance, "_execute_post_hook", + mock.Mock(side_effect=Exception("I should not be raised."))) + + result = instance.execute_post_hook( + self.context, "fake_func_name", "some_pre_hook_data", + "some_driver_action_results", "some_arg", some_kwarg="foo") + + self.assertIsNone(result) + + @ddt.data(True, False) + def test_execute_post_hook_success(self, provide_context): + instance = self._get_hook_instance() + instance.post_hooks_enabled = True + instance.suppress_post_hooks_errors = True + expected = "fake_expected_result" + some_arg = "some_arg" + func_name = "fake_func_name" + pre_hook_data = "some_pre_hook_data" + driver_action_results = "some_driver_action_results" + self.mock_object(hook.LOG, 'warning') + self.mock_object( + instance, "_execute_post_hook", mock.Mock(return_value=expected)) + mock_ctxt = self.mock_object(context, 'get_admin_context') + ctxt = self.context if provide_context else mock_ctxt + + result = instance.execute_post_hook( + ctxt, func_name, pre_hook_data, driver_action_results, + some_arg, some_kwarg="foo") + + self.assertEqual(expected, result) + instance._execute_post_hook.assert_called_once_with( + some_arg, + context=self.context if provide_context else mock_ctxt, + func_name=func_name, + pre_hook_data=pre_hook_data, + driver_action_results=driver_action_results, + some_kwarg="foo") + self.assertFalse(hook.LOG.warning.called) + + def test_execute_post_hook_exception_with_suppression(self): + instance = self._get_hook_instance() + instance.post_hooks_enabled = True + instance.suppress_post_hooks_errors = True + some_arg = "some_arg" + func_name = "fake_func_name" + pre_hook_data = "some_pre_hook_data" + driver_action_results = "some_driver_action_results" + FakeException = type("FakeException", (Exception, ), {}) + self.mock_object(hook.LOG, 'warning') + self.mock_object( + instance, "_execute_post_hook", mock.Mock(side_effect=( + FakeException("Some exception that should be suppressed.")))) + + result = instance.execute_post_hook( + self.context, func_name, pre_hook_data, driver_action_results, + some_arg, some_kwarg="foo") + + self.assertIsInstance(result, FakeException) + instance._execute_post_hook.assert_called_once_with( + some_arg, + context=self.context, + func_name=func_name, + pre_hook_data=pre_hook_data, + driver_action_results=driver_action_results, + some_kwarg="foo") + self.assertTrue(hook.LOG.warning.called) + + def test_execute_post_hook_exception_without_suppression(self): + instance = self._get_hook_instance() + instance.post_hooks_enabled = True + instance.suppress_post_hooks_errors = False + some_arg = "some_arg" + func_name = "fake_func_name" + pre_hook_data = "some_pre_hook_data" + driver_action_results = "some_driver_action_results" + FakeException = type("FakeException", (Exception, ), {}) + self.mock_object(hook.LOG, 'error') + self.mock_object( + instance, "_execute_post_hook", mock.Mock(side_effect=( + FakeException( + "Some exception that should NOT be suppressed.")))) + + self.assertRaises( + FakeException, + instance.execute_post_hook, + self.context, func_name, pre_hook_data, driver_action_results, + some_arg, some_kwarg="foo") + + instance._execute_post_hook.assert_called_once_with( + some_arg, + context=self.context, + func_name=func_name, + pre_hook_data=pre_hook_data, + driver_action_results=driver_action_results, + some_kwarg="foo") + self.assertFalse(hook.LOG.error.called) + + def test_execute_periodic_hook_disabled(self): + instance = self._get_hook_instance() + instance.periodic_hooks_enabled = False + self.mock_object(instance, "_execute_periodic_hook") + + instance.execute_periodic_hook( + self.context, "fake_periodic_hook_data", + "some_arg", some_kwarg="foo") + + self.assertFalse(instance._execute_periodic_hook.called) + + @ddt.data(True, False) + def test_execute_periodic_hook_enabled(self, provide_context): + instance = self._get_hook_instance() + instance.periodic_hooks_enabled = True + expected = "some_expected_result" + self.mock_object( + instance, + "_execute_periodic_hook", + mock.Mock(return_value=expected)) + mock_ctxt = self.mock_object(context, 'get_admin_context') + ctxt = self.context if provide_context else mock_ctxt + + result = instance.execute_periodic_hook( + ctxt, "fake_periodic_hook_data", + "some_arg", some_kwarg="foo") + + instance._execute_periodic_hook.assert_called_once_with( + ctxt, "fake_periodic_hook_data", + "some_arg", some_kwarg="foo") + self.assertEqual(expected, result) diff --git a/manila/tests/share/test_manager.py b/manila/tests/share/test_manager.py index 7494af2586..c3e55e2e1e 100644 --- a/manila/tests/share/test_manager.py +++ b/manila/tests/share/test_manager.py @@ -58,6 +58,7 @@ class ShareManagerTestCase(test.TestCase): private_data_mock = mock.Mock() self.mock_object(drivers_private_data, "DriverPrivateData", private_data_mock) + self.mock_object(manager.ShareManager, '_init_hook_drivers') share_manager = manager.ShareManager(service_name=fake_service_name) @@ -67,6 +68,56 @@ class ShareManagerTestCase(test.TestCase): config_group=fake_service_name ) self.assertTrue(import_mock.called) + self.assertTrue(manager.ShareManager._init_hook_drivers.called) + + def test__init_hook_drivers(self): + fake_service_name = "fake_service" + import_mock = mock.Mock() + self.mock_object(importutils, "import_object", import_mock) + self.mock_object(drivers_private_data, "DriverPrivateData") + share_manager = manager.ShareManager(service_name=fake_service_name) + share_manager.configuration.safe_get = mock.Mock( + return_value=["Foo", "Bar"]) + self.assertEqual(0, len(share_manager.hooks)) + import_mock.reset() + + share_manager._init_hook_drivers() + + self.assertEqual( + len(share_manager.configuration.safe_get.return_value), + len(share_manager.hooks)) + import_mock.assert_has_calls([ + mock.call( + hook, + configuration=share_manager.configuration, + host=share_manager.host + ) for hook in share_manager.configuration.safe_get.return_value + ], any_order=True) + + def test__execute_periodic_hook(self): + share_instances_mock = mock.Mock() + hook_data_mock = mock.Mock() + self.mock_object( + self.share_manager.db, + "share_instances_get_all_by_host", + share_instances_mock) + self.mock_object( + self.share_manager.driver, + "get_periodic_hook_data", + hook_data_mock) + self.share_manager.hooks = [mock.Mock(return_value=i) for i in (0, 1)] + + self.share_manager._execute_periodic_hook(self.context) + + share_instances_mock.assert_called_once_with( + context=self.context, host=self.share_manager.host) + hook_data_mock.assert_called_once_with( + context=self.context, + share_instances=share_instances_mock.return_value) + for mock_hook in self.share_manager.hooks: + mock_hook.execute_periodic_hook.assert_called_once_with( + context=self.context, + periodic_hook_data=hook_data_mock.return_value) def test_init_host_with_no_shares(self): self.mock_object(self.share_manager.db, @@ -1768,3 +1819,46 @@ class ShareManagerTestCase(test.TestCase): self.assertFalse(driver.get_share_server_pools.called) self.assertEqual(old_capabilities, self.share_manager.last_capabilities) + + +@ddt.ddt +class HookWrapperTestCase(test.TestCase): + + def setUp(self): + super(HookWrapperTestCase, self).setUp() + self.configuration = mock.Mock() + self.configuration.safe_get.return_value = True + + @manager.add_hooks + def _fake_wrapped_method(self, some_arg, some_kwarg): + return "foo" + + def test_hooks_enabled(self): + self.hooks = [mock.Mock(return_value=i) for i in range(2)] + + result = self._fake_wrapped_method( + "some_arg", some_kwarg="some_kwarg_value") + + self.assertEqual("foo", result) + for i, mock_hook in enumerate(self.hooks): + mock_hook.execute_pre_hook.assert_called_once_with( + "some_arg", + func_name="_fake_wrapped_method", + some_kwarg="some_kwarg_value") + mock_hook.execute_post_hook.assert_called_once_with( + "some_arg", + func_name="_fake_wrapped_method", + driver_action_results="foo", + pre_hook_data=self.hooks[i].execute_pre_hook.return_value, + some_kwarg="some_kwarg_value") + + def test_hooks_disabled(self): + self.hooks = [] + + result = self._fake_wrapped_method( + "some_arg", some_kwarg="some_kwarg_value") + + self.assertEqual("foo", result) + for mock_hook in self.hooks: + self.assertFalse(mock_hook.execute_pre_hook.called) + self.assertFalse(mock_hook.execute_post_hook.called)