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

@@ -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,

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

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."""

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)

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)

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)

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)