Merge pull request #298 from stackhpc/backport/2023.1/environment-hooks

Make hooks environment-aware

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.