From 3e9a6acff7b960b85d28ee7038c8fd15e706823e Mon Sep 17 00:00:00 2001 From: Alexei Znamensky <103110+russoz@users.noreply.github.com> Date: Sat, 16 Jan 2021 09:29:23 +1300 Subject: [PATCH] Modhelper improvements (#1480) * Improvements in module_helper - added `ModuleHelperException` to handle problems specific to ModuleHelper - updated `module_fails_on_exception` for `ModuleHelperException` - `StateMixin`: composed names of state methods are now calculated instead of fixed. - `CmdMixin`: added `run_command_fixed_options` to pass some parameters on every call * Improvements in module_helper - Named deprecations: ability to declare a `dict` of deprecations indexed by names, allowing module maintainer to trigger them by those names, and also allowing the module user to acknowledge them in a similar way. - Adding `ack_named_deprecations` to module's `argument_spec` when they exist. - Providing doc fragment for `ack_named_deprecations`. - Added method `__quit_module__` providing a hook for code that needs to be run when quitting the module. - Created convenience classes combining `ModuleHelper`, `StateMixin`, `CmdMixin`. * fixed validation * fixed validation * changelog fragment * Apply suggestions from code review Co-authored-by: Felix Fontein * Improvement on Named Deprecations Per the comments in PR, we want to expose a call to a ``deprecate`` method on the module code, so that pylint can properly perform its static analysis on deprecations. This prompted a revamp of the named deprecation feature. * Use .get() instead of [] for the param to ack named deprecations. * Changes from suggestions in the PR * removed named deprecations * Update changelogs/fragments/1480-module-helper-improvements.yml Co-authored-by: Felix Fontein * Update plugins/module_utils/module_helper.py Co-authored-by: Felix Fontein --- .../1480-module-helper-improvements.yml | 2 + plugins/module_utils/module_helper.py | 69 +++++++++++++++---- .../module_utils/test_module_helper.py | 35 ++++++---- 3 files changed, 78 insertions(+), 28 deletions(-) create mode 100644 changelogs/fragments/1480-module-helper-improvements.yml diff --git a/changelogs/fragments/1480-module-helper-improvements.yml b/changelogs/fragments/1480-module-helper-improvements.yml new file mode 100644 index 0000000000..655cc34069 --- /dev/null +++ b/changelogs/fragments/1480-module-helper-improvements.yml @@ -0,0 +1,2 @@ +minor_changes: + - module_helper module utils - multiple convenience features added (https://github.com/ansible-collections/community.general/pull/1480). diff --git a/plugins/module_utils/module_helper.py b/plugins/module_utils/module_helper.py index 0e52db7b3d..42fb17d210 100644 --- a/plugins/module_utils/module_helper.py +++ b/plugins/module_utils/module_helper.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- -# Copyright: (c) 2018, Ansible Project +# (c) 2020, Alexei Znamensky +# Copyright: (c) 2020, Ansible Project # Simplified BSD License (see licenses/simplified_bsd.txt or https://opensource.org/licenses/BSD-2-Clause) from __future__ import absolute_import, division, print_function @@ -11,6 +12,21 @@ import traceback from ansible.module_utils.basic import AnsibleModule +class ModuleHelperException(Exception): + @staticmethod + def _get_remove(key, kwargs): + if key in kwargs: + result = kwargs[key] + del kwargs[key] + return result + return None + + def __init__(self, *args, **kwargs): + self.msg = self._get_remove('msg', kwargs) or "Module failed with exception: {0}".format(self) + self.update_output = self._get_remove('update_output', kwargs) or {} + super(ModuleHelperException, self).__init__(*args, **kwargs) + + class ArgFormat(object): """ Argument formatter @@ -102,6 +118,9 @@ def module_fails_on_exception(func): func(self, *args, **kwargs) except SystemExit: raise + except ModuleHelperException as e: + if e.update_output: + self.update_output(e.update_output) except Exception as e: self.vars.msg = "Module failed with exception: {0}".format(str(e).strip()) self.vars.exception = traceback.format_exc() @@ -151,16 +170,14 @@ class ModuleHelper(object): if module: self.module = module - if not isinstance(module, AnsibleModule): + if isinstance(self.module, dict): self.module = AnsibleModule(**self.module) def update_output(self, **kwargs): - if kwargs: - self.output_dict.update(kwargs) + self.output_dict.update(kwargs) def update_facts(self, **kwargs): - if kwargs: - self.facts_dict.update(kwargs) + self.facts_dict.update(kwargs) def __init_module__(self): pass @@ -168,6 +185,9 @@ class ModuleHelper(object): def __run__(self): raise NotImplementedError() + def __quit_module__(self): + pass + @property def changed(self): return self._changed @@ -189,6 +209,7 @@ class ModuleHelper(object): self.fail_on_missing_deps() self.__init_module__() self.__run__() + self.__quit_module__() self.module.exit_json(changed=self.changed, **self.output_dict) @classmethod @@ -213,6 +234,9 @@ class StateMixin(object): state = self.module.params.get(self.state_param) return self.default_state if state is None else state + def _method(self, state): + return "{0}_{1}".format(self.state_param, state) + def __run__(self): state = self._state() self.vars.state = state @@ -224,14 +248,14 @@ class StateMixin(object): state = aliased[0] self.vars.effective_state = state - method = "state_{0}".format(state) + method = self._method(state) if not hasattr(self, method): return self.__state_fallback__() func = getattr(self, method) return func() def __state_fallback__(self): - raise ValueError("Cannot find method for state: {0}".format(self._state())) + raise ValueError("Cannot find method: {0}".format(self._method(self._state()))) class CmdMixin(object): @@ -239,7 +263,8 @@ class CmdMixin(object): Mixin for mapping module options to running a CLI command with its arguments. """ command = None - command_args_formats = dict() + command_args_formats = {} + run_command_fixed_options = {} check_rc = False force_lang = "C" @@ -266,7 +291,8 @@ class CmdMixin(object): return self.custom_formats.get(_param, self.module_formats.get(_param)) extra_params = extra_params or dict() - cmd_args = [self.module.get_bin_path(self.command)] + cmd_args = list([self.command]) if isinstance(self.command, str) else list(self.command) + cmd_args[0] = self.module.get_bin_path(cmd_args[0]) param_list = params if params else self.module.params.keys() for param in param_list: @@ -290,13 +316,26 @@ class CmdMixin(object): def run_command(self, extra_params=None, params=None, *args, **kwargs): self.vars['cmd_args'] = self._calculate_args(extra_params, params) - env_update = kwargs.get('environ_update', {}) - check_rc = kwargs.get('check_rc', self.check_rc) + options = dict(self.run_command_fixed_options) + env_update = dict(options.get('environ_update', {})) + options['check_rc'] = options.get('check_rc', self.check_rc) if self.force_lang: env_update.update({'LANGUAGE': self.force_lang}) self.update_output(force_lang=self.force_lang) - rc, out, err = self.module.run_command(self.vars['cmd_args'], - environ_update=env_update, - check_rc=check_rc, *args, **kwargs) + options['environ_update'] = env_update + options.update(kwargs) + rc, out, err = self.module.run_command(self.vars['cmd_args'], *args, **options) self.update_output(rc=rc, stdout=out, stderr=err) return self.process_command_output(rc, out, err) + + +class StateModuleHelper(StateMixin, ModuleHelper): + pass + + +class CmdModuleHelper(CmdMixin, ModuleHelper): + pass + + +class CmdStateModuleHelper(CmdMixin, StateMixin, ModuleHelper): + pass diff --git a/tests/unit/plugins/module_utils/test_module_helper.py b/tests/unit/plugins/module_utils/test_module_helper.py index f20594bbf0..fb7746a91a 100644 --- a/tests/unit/plugins/module_utils/test_module_helper.py +++ b/tests/unit/plugins/module_utils/test_module_helper.py @@ -18,19 +18,28 @@ def single_lambda_2star(x, y, z): ARG_FORMATS = dict( - simple_boolean_true=("--superflag", ArgFormat.BOOLEAN, 0, True, ["--superflag"]), - simple_boolean_false=("--superflag", ArgFormat.BOOLEAN, 0, False, []), - single_printf=("--param=%s", ArgFormat.PRINTF, 0, "potatoes", ["--param=potatoes"]), - single_printf_no_substitution=("--param", ArgFormat.PRINTF, 0, "potatoes", ["--param"]), - multiple_printf=(["--param", "free-%s"], ArgFormat.PRINTF, 0, "potatoes", ["--param", "free-potatoes"]), - single_format=("--param={0}", ArgFormat.FORMAT, 0, "potatoes", ["--param=potatoes"]), - single_format_no_substitution=("--param", ArgFormat.FORMAT, 0, "potatoes", ["--param"]), - multiple_format=(["--param", "free-{0}"], ArgFormat.FORMAT, 0, "potatoes", ["--param", "free-potatoes"]), - single_lambda_0star=((lambda v: ["piggies=[{0},{1},{2}]".format(v[0], v[1], v[2])]), - None, 0, ['a', 'b', 'c'], ["piggies=[a,b,c]"]), - single_lambda_1star=((lambda a, b, c: ["piggies=[{0},{1},{2}]".format(a, b, c)]), - None, 1, ['a', 'b', 'c'], ["piggies=[a,b,c]"]), - single_lambda_2star=(single_lambda_2star, None, 2, dict(z='c', x='a', y='b'), ["piggies=[a,b,c]"]) + simple_boolean_true=("--superflag", ArgFormat.BOOLEAN, 0, + True, ["--superflag"]), + simple_boolean_false=("--superflag", ArgFormat.BOOLEAN, 0, + False, []), + single_printf=("--param=%s", ArgFormat.PRINTF, 0, + "potatoes", ["--param=potatoes"]), + single_printf_no_substitution=("--param", ArgFormat.PRINTF, 0, + "potatoes", ["--param"]), + multiple_printf=(["--param", "free-%s"], ArgFormat.PRINTF, 0, + "potatoes", ["--param", "free-potatoes"]), + single_format=("--param={0}", ArgFormat.FORMAT, 0, + "potatoes", ["--param=potatoes"]), + single_format_no_substitution=("--param", ArgFormat.FORMAT, 0, + "potatoes", ["--param"]), + multiple_format=(["--param", "free-{0}"], ArgFormat.FORMAT, 0, + "potatoes", ["--param", "free-potatoes"]), + single_lambda_0star=((lambda v: ["piggies=[{0},{1},{2}]".format(v[0], v[1], v[2])]), None, 0, + ['a', 'b', 'c'], ["piggies=[a,b,c]"]), + single_lambda_1star=((lambda a, b, c: ["piggies=[{0},{1},{2}]".format(a, b, c)]), None, 1, + ['a', 'b', 'c'], ["piggies=[a,b,c]"]), + single_lambda_2star=(single_lambda_2star, None, 2, + dict(z='c', x='a', y='b'), ["piggies=[a,b,c]"]) ) ARG_FORMATS_IDS = sorted(ARG_FORMATS.keys())