Make hooks environment-aware

Previously it was only possible to define custom playbook hooks in the
base configuration, and not in environments. This could be limiting in
cases where different environments require different hooks.

With this change it is now possible to define hooks both in the base
configuration and in environments.

Change-Id: Ic003c18402177318ac1aa4c2d851263893bd4e9f
(cherry picked from commit 0055d38)

doc/source/multiple-environments.rst

@@ -29,15 +29,19 @@ configuration.
 Supporting multiple environments is done through a
 ``$KAYOBE_CONFIG_PATH/environments`` directory, under which each directory
 represents a different environment.  Each environment contains its own Ansible
-inventory, extra variable files, and Kolla configuration. The following layout
-shows two environments called ``staging`` and ``production`` within a single
-Kayobe configuration.
+inventory, extra variable files, hooks, and Kolla configuration. The following
+layout shows two environments called ``staging`` and ``production`` within a
+single Kayobe configuration.
 
 .. code-block:: text
 
    $KAYOBE_CONFIG_PATH/
    └── environments/
        ├── production/
+       │   ├── hooks/
+       │   │   └── overcloud-service-deploy/
+       │   │       └── pre.d/
+       │   │           └── 1-prep-stuff.yml
        │   ├── inventory/
        │   │   ├── groups
        │   │   ├── group_vars/
@@ -349,17 +353,45 @@ For example, symbolic links can be used to share common variable definitions.
 It is advised to avoid sharing credentials between environments by making each
 Kolla ``passwords.yml`` file unique.
 
-Custom Ansible Playbooks and Hooks
-----------------------------------
+Custom Ansible Playbooks
+------------------------
+
+:doc:`Custom Ansible playbooks <custom-ansible-playbooks>`, roles and
+requirements file under ``$KAYOBE_CONFIG_PATH/ansible`` are currently shared
+across all environments.
+
+Hooks
+-----
+
+Prior to the Caracal 16.0.0 release, :ref:`hooks <custom-playbooks-hooks>` were
+shared across all environments.  Since Caracal it is possible to define hooks
+on a per-environment basis. Hooks are collected from all environments and the
+base configuration. Where multiple hooks exist with the same name, the
+environment's hook takes precedence and *replaces* the other hooks. Execution
+order follows the normal rules, regardless of where each hook is defined.
+
+For example, the base configuration defines the following hooks:
+
+* ``$KAYOBE_CONFIG_PATH/hooks/overcloud-service-deploy/pre.d/1-base.yml``
+* ``$KAYOBE_CONFIG_PATH/hooks/overcloud-service-deploy/pre.d/2-both.yml``
+
+The environment defines the following hooks:
+
+* ``$KAYOBE_CONFIG_PATH/environments/env/hooks/overcloud-service-deploy/pre.d/2-both.yml``
+* ``$KAYOBE_CONFIG_PATH/environments/env/hooks/overcloud-service-deploy/pre.d/3-env.yml``
+
+The following hooks will execute in the order shown:
+
+* ``$KAYOBE_CONFIG_PATH/hooks/overcloud-service-deploy/pre.d/1-base.yml``
+* ``$KAYOBE_CONFIG_PATH/environments/env/hooks/overcloud-service-deploy/pre.d/2-both.yml``
+* ``$KAYOBE_CONFIG_PATH/environments/env/hooks/overcloud-service-deploy/pre.d/3-env.yml``
 
-The following files and directories are currently shared across all
-environments:
+Ansible Configuration
+---------------------
 
-* Ansible playbooks, roles and requirements file under
-  ``$KAYOBE_CONFIG_PATH/ansible``
-* Ansible configuration at ``$KAYOBE_CONFIG_PATH/ansible.cfg`` and
-  ``$KAYOBE_CONFIG_PATH/kolla/ansible.cfg``
-* Hooks under ``$KAYOBE_CONFIG_PATH/hooks``
+Ansible configuration at ``$KAYOBE_CONFIG_PATH/ansible.cfg`` or
+``$KAYOBE_CONFIG_PATH/kolla/ansible.cfg`` is currently shared across all
+environments.
 
 Dynamic Variable Definitions
 ----------------------------

kayobe/cli/commands.py

@@ -153,6 +153,7 @@ def run_kolla_ansible_seed(self, *args, **kwargs):
 
 
 def _split_hook_sequence_number(hook):
+    hook = os.path.basename(hook)
     parts = hook.split("-", 1)
     if len(parts) < 2:
         return (DEFAULT_SEQUENCE_NUMBER, hook)
@@ -181,22 +182,38 @@ def get_epilog(self):
     def get_parser(self, prog_name):
         pass
 
-    def _find_hooks(self, config_path, target):
+    def _find_hooks(self, env_paths, target):
         name = self.name
-        path = os.path.join(config_path, "hooks", name, "%s.d" % target)
-        self.logger.debug("Discovering hooks in: %s" % path)
-        if not os.path.exists:
-            return []
-        hooks = glob.glob(os.path.join(path, "*.yml"))
+        # Map from hook directory path to a set of hook basenames in that path.
+        hooks: {str: {str}} = {}
+        for env_path in env_paths:
+            path = os.path.join(env_path, "hooks", name, "%s.d" % target)
+            self.logger.debug("Discovering hooks in: %s" % path)
+            if not os.path.exists(path):
+                continue
+
+            hook_paths = glob.glob(os.path.join(path, "*.yml"))
+            hook_basenames = {os.path.basename(hook) for hook in hook_paths}
+
+            # Override any earlier hooks with the same basename.
+            for other_hooks in hooks.values():
+                other_hooks -= hook_basenames
+
+            hooks[path] = hook_basenames
+
+        # Return a flat list of hook paths (including directory).
+        hooks = [os.path.join(path, basename)
+                 for path, basenames in hooks.items()
+                 for basename in basenames]
         self.logger.debug("Discovered the following hooks: %s" % hooks)
         return hooks
 
-    def hooks(self, config_path, target, filter):
+    def hooks(self, env_paths, target, filter):
         hooks_out = []
         if filter == "all":
             self.logger.debug("Skipping all hooks")
             return hooks_out
-        hooks_in = self._find_hooks(config_path, target)
+        hooks_in = self._find_hooks(env_paths, target)
         # Hooks can be prefixed with a sequence number to adjust running order,
         # e.g 10-my-custom-playbook.yml. Sort by sequence number.
         hooks_in = sorted(hooks_in, key=_split_hook_sequence_number)
@@ -210,8 +227,12 @@ def hooks(self, config_path, target, filter):
         return hooks_out
 
     def run_hooks(self, parsed_args, target):
-        config_path = parsed_args.config_path
-        hooks = self.hooks(config_path, target, parsed_args.skip_hooks)
+        env_paths = [parsed_args.config_path]
+        environment_finder = utils.EnvironmentFinder(
+            parsed_args.config_path, parsed_args.environment)
+        env_paths.extend(environment_finder.ordered_paths())
+
+        hooks = self.hooks(env_paths, target, parsed_args.skip_hooks)
         if hooks:
             self.logger.debug("Running hooks: %s" % hooks)
             self.command.run_kayobe_playbooks(parsed_args, hooks)

kayobe/tests/unit/cli/test_commands.py

@@ -12,6 +12,8 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+import glob
+import os
 import unittest
 from unittest import mock
 
@@ -2412,29 +2414,31 @@ class TestHookDispatcher(unittest.TestCase):
 
     maxDiff = None
 
-    @mock.patch('kayobe.cli.commands.os.path')
-    def test_hook_ordering(self, mock_path):
+    @mock.patch.object(os.path, 'realpath')
+    def test_hook_ordering(self, mock_realpath):
         mock_command = mock.MagicMock()
         dispatcher = commands.HookDispatcher(command=mock_command)
         dispatcher._find_hooks = mock.MagicMock()
+        # Include multiple hook directories to show that they don't influence
+        # the order.
         dispatcher._find_hooks.return_value = [
-            "10-hook.yml",
-            "5-hook.yml",
-            "z-test-alphabetical.yml",
-            "10-before-hook.yml",
-            "5-multiple-dashes-in-name.yml",
-            "no-prefix.yml"
+            "config/path/10-hook.yml",
+            "config/path/5-hook.yml",
+            "config/path/z-test-alphabetical.yml",
+            "env/path/10-before-hook.yml",
+            "env/path/5-multiple-dashes-in-name.yml",
+            "env/path/no-prefix.yml"
         ]
         expected_result = [
-            "5-hook.yml",
-            "5-multiple-dashes-in-name.yml",
-            "10-before-hook.yml",
-            "10-hook.yml",
-            "no-prefix.yml",
-            "z-test-alphabetical.yml",
-        ]
-        mock_path.realpath.side_effect = lambda x: x
-        actual = dispatcher.hooks("config/path", "pre", None)
+            "config/path/5-hook.yml",
+            "env/path/5-multiple-dashes-in-name.yml",
+            "env/path/10-before-hook.yml",
+            "config/path/10-hook.yml",
+            "env/path/no-prefix.yml",
+            "config/path/z-test-alphabetical.yml",
+        ]
+        mock_realpath.side_effect = lambda x: x
+        actual = dispatcher.hooks(["config/path", "env/path"], "pre", None)
         self.assertListEqual(actual, expected_result)
 
     @mock.patch('kayobe.cli.commands.os.path')
@@ -2451,7 +2455,7 @@ def test_hook_filter_all(self, mock_path):
             "z-test-alphabetical.yml",
         ]
         mock_path.realpath.side_effect = lambda x: x
-        actual = dispatcher.hooks("config/path", "pre", "all")
+        actual = dispatcher.hooks(["config/path"], "pre", "all")
         self.assertListEqual(actual, [])
 
     @mock.patch('kayobe.cli.commands.os.path')
@@ -2475,6 +2479,105 @@ def test_hook_filter_one(self, mock_path):
             "z-test-alphabetical.yml",
         ]
         mock_path.realpath.side_effect = lambda x: x
-        actual = dispatcher.hooks("config/path", "pre",
+        actual = dispatcher.hooks(["config/path"], "pre",
                                   "5-multiple-dashes-in-name.yml")
         self.assertListEqual(actual, expected_result)
+
+    @mock.patch.object(glob, 'glob')
+    @mock.patch.object(os.path, 'exists')
+    def test__find_hooks(self, mock_exists, mock_glob):
+        mock_exists.return_value = True
+        mock_command = mock.MagicMock()
+        dispatcher = commands.HookDispatcher(command=mock_command)
+        mock_glob.return_value = [
+            "config/path/hooks/pre.d/1-hook.yml",
+            "config/path/hooks/pre.d/5-hook.yml",
+            "config/path/hooks/pre.d/10-hook.yml",
+        ]
+        expected_result = [
+            "config/path/hooks/pre.d/1-hook.yml",
+            "config/path/hooks/pre.d/10-hook.yml",
+            "config/path/hooks/pre.d/5-hook.yml",
+        ]
+        actual = dispatcher._find_hooks(["config/path"], "pre")
+        # Sort the result - it is not ordered at this stage.
+        actual.sort()
+        self.assertListEqual(actual, expected_result)
+
+    @mock.patch.object(glob, 'glob')
+    @mock.patch.object(os.path, 'exists')
+    def test__find_hooks_with_env(self, mock_exists, mock_glob):
+        mock_exists.return_value = True
+        mock_command = mock.MagicMock()
+        dispatcher = commands.HookDispatcher(command=mock_command)
+        mock_glob.side_effect = [
+            [
+                "config/path/hooks/pre.d/all.yml",
+                "config/path/hooks/pre.d/base-only.yml",
+            ],
+            [
+                "env/path/hooks/pre.d/all.yml",
+                "env/path/hooks/pre.d/env-only.yml",
+            ]
+        ]
+        expected_result = [
+            "config/path/hooks/pre.d/base-only.yml",
+            "env/path/hooks/pre.d/all.yml",
+            "env/path/hooks/pre.d/env-only.yml",
+        ]
+        actual = dispatcher._find_hooks(["config/path", "env/path"], "pre")
+        # Sort the result - it is not ordered at this stage.
+        actual.sort()
+        self.assertListEqual(actual, expected_result)
+
+    @mock.patch.object(glob, 'glob')
+    @mock.patch.object(os.path, 'exists')
+    def test__find_hooks_with_nested_envs(self, mock_exists, mock_glob):
+        mock_exists.return_value = True
+        mock_command = mock.MagicMock()
+        dispatcher = commands.HookDispatcher(command=mock_command)
+        mock_glob.side_effect = [
+            [
+                "config/path/hooks/pre.d/all.yml",
+                "config/path/hooks/pre.d/base-only.yml",
+                "config/path/hooks/pre.d/base-env1.yml",
+                "config/path/hooks/pre.d/base-env2.yml",
+            ],
+            [
+                "env1/path/hooks/pre.d/all.yml",
+                "env1/path/hooks/pre.d/env1-only.yml",
+                "env1/path/hooks/pre.d/base-env1.yml",
+                "env1/path/hooks/pre.d/env1-env2.yml",
+            ],
+            [
+                "env2/path/hooks/pre.d/all.yml",
+                "env2/path/hooks/pre.d/env2-only.yml",
+                "env2/path/hooks/pre.d/base-env2.yml",
+                "env2/path/hooks/pre.d/env1-env2.yml",
+            ]
+        ]
+        expected_result = [
+            "config/path/hooks/pre.d/base-only.yml",
+            "env1/path/hooks/pre.d/base-env1.yml",
+            "env1/path/hooks/pre.d/env1-only.yml",
+            "env2/path/hooks/pre.d/all.yml",
+            "env2/path/hooks/pre.d/base-env2.yml",
+            "env2/path/hooks/pre.d/env1-env2.yml",
+            "env2/path/hooks/pre.d/env2-only.yml",
+        ]
+        actual = dispatcher._find_hooks(["config/path", "env1/path",
+                                         "env2/path"], "pre")
+        # Sort the result - it is not ordered at this stage.
+        actual.sort()
+        self.assertListEqual(actual, expected_result)
+
+    @mock.patch.object(glob, 'glob')
+    @mock.patch.object(os.path, 'exists')
+    def test__find_hooks_non_existent(self, mock_exists, mock_glob):
+        mock_exists.return_value = False
+        mock_command = mock.MagicMock()
+        dispatcher = commands.HookDispatcher(command=mock_command)
+        expected_result = []
+        actual = dispatcher._find_hooks(["config/path"], "pre")
+        self.assertListEqual(actual, expected_result)
+        mock_glob.assert_not_called()

releasenotes/notes/env-aware-hooks-2faf451050a06287.yaml

@@ -0,0 +1,4 @@
+---
+features:
+  - |
+    Adds support for defining custom playbook hooks in Kayobe environments.