From 0055d384a6e9fce614b7585705e58ed5d5cc74d0 Mon Sep 17 00:00:00 2001 From: Mark Goddard Date: Wed, 20 Dec 2023 17:09:50 +0000 Subject: [PATCH] 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 --- doc/source/multiple-environments.rst | 56 +++++-- kayobe/cli/commands.py | 41 ++++-- kayobe/tests/unit/cli/test_commands.py | 139 +++++++++++++++--- .../env-aware-hooks-2faf451050a06287.yaml | 4 + 4 files changed, 200 insertions(+), 40 deletions(-) create mode 100644 releasenotes/notes/env-aware-hooks-2faf451050a06287.yaml diff --git a/doc/source/multiple-environments.rst b/doc/source/multiple-environments.rst index b3b655295..e211fc6db 100644 --- a/doc/source/multiple-environments.rst +++ b/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 +------------------------ -The following files and directories are currently shared across all -environments: +:doc:`Custom Ansible playbooks `, roles and +requirements file under ``$KAYOBE_CONFIG_PATH/ansible`` are currently shared +across all environments. -* 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`` +Hooks +----- + +Prior to the Caracal 16.0.0 release, :ref:`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`` + +Ansible Configuration +--------------------- + +Ansible configuration at ``$KAYOBE_CONFIG_PATH/ansible.cfg`` or +``$KAYOBE_CONFIG_PATH/kolla/ansible.cfg`` is currently shared across all +environments. Dynamic Variable Definitions ---------------------------- diff --git a/kayobe/cli/commands.py b/kayobe/cli/commands.py index 464d83b0e..5b1a7ffae 100644 --- a/kayobe/cli/commands.py +++ b/kayobe/cli/commands.py @@ -153,6 +153,7 @@ class KollaAnsibleMixin(object): 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 @@ class HookDispatcher(CommandHook): 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 @@ class HookDispatcher(CommandHook): 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) diff --git a/kayobe/tests/unit/cli/test_commands.py b/kayobe/tests/unit/cli/test_commands.py index 38d6f9664..d21227f11 100644 --- a/kayobe/tests/unit/cli/test_commands.py +++ b/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 @@ -2393,29 +2395,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", + "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_path.realpath.side_effect = lambda x: x - actual = dispatcher.hooks("config/path", "pre", None) + 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') @@ -2432,7 +2436,7 @@ class TestHookDispatcher(unittest.TestCase): "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') @@ -2456,6 +2460,105 @@ class TestHookDispatcher(unittest.TestCase): "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() diff --git a/releasenotes/notes/env-aware-hooks-2faf451050a06287.yaml b/releasenotes/notes/env-aware-hooks-2faf451050a06287.yaml new file mode 100644 index 000000000..a6df97c94 --- /dev/null +++ b/releasenotes/notes/env-aware-hooks-2faf451050a06287.yaml @@ -0,0 +1,4 @@ +--- +features: + - | + Adds support for defining custom playbook hooks in Kayobe environments.