From fad8c095bfb408c79b89272eef05c822e2d9983e Mon Sep 17 00:00:00 2001 From: Marius Rieder Date: Mon, 9 May 2022 07:58:17 +0200 Subject: [PATCH 01/28] Add slaves parameter for module alternatives. --- plugins/modules/system/alternatives.py | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/plugins/modules/system/alternatives.py b/plugins/modules/system/alternatives.py index ca075d69b4a..f3c28b27ef9 100644 --- a/plugins/modules/system/alternatives.py +++ b/plugins/modules/system/alternatives.py @@ -51,6 +51,12 @@ default: selected type: str version_added: 4.8.0 + slaves: + description: + - A list of slaves + - Each slave needs a name, a link and a path parameter + type: list + version_added: 2.5.0 requirements: [ update-alternatives ] ''' @@ -78,6 +84,16 @@ path: /usr/bin/python3.5 link: /usr/bin/python state: present + +- name: keytool is a slave of java + alternatives: + name: java + link: /usr/bin/java + path: /usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java + slaves: + - name: keytool + link: /usr/bin/keytool + path: /usr/lib/jvm/java-7-openjdk-amd64/jre/bin/keytool ''' import os @@ -109,6 +125,7 @@ def main(): choices=AlternativeState.to_list(), default=AlternativeState.SELECTED, ), + slaves=dict(type='list'), ), supports_check_mode=True, ) @@ -180,8 +197,13 @@ def main(): if not link: module.fail_json(msg="Needed to install the alternative, but unable to do so as we are missing the link") + cmd = [UPDATE_ALTERNATIVES, '--install', link, name, path, str(priority)] + if params['slaves']: + slaves = map(lambda slave: ['--slave', slave['link'], slave['name'], slave['path']], params['slaves']) + cmd += [item for sublist in slaves for item in sublist] + module.run_command( - [UPDATE_ALTERNATIVES, '--install', link, name, path, str(priority)], + cmd, check_rc=True ) changed = True From b7606a543ef7d91a77b289bc626d724362fa6e03 Mon Sep 17 00:00:00 2001 From: Marius Rieder Date: Mon, 9 May 2022 08:31:05 +0200 Subject: [PATCH 02/28] alternatives: Improve documentation abous slaves parameter --- plugins/modules/system/alternatives.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/plugins/modules/system/alternatives.py b/plugins/modules/system/alternatives.py index f3c28b27ef9..0a5c956acb7 100644 --- a/plugins/modules/system/alternatives.py +++ b/plugins/modules/system/alternatives.py @@ -56,6 +56,22 @@ - A list of slaves - Each slave needs a name, a link and a path parameter type: list + elements: dict + suboptions: + name: + description: + - The generic name of the slave. + type: str + required: true + path: + description: + - The path to the real executable that the slave should point to. + type: path + required: true + link: + description: + - The path to the symbolic link that should point to the real slave executable. + type: path version_added: 2.5.0 requirements: [ update-alternatives ] ''' @@ -125,7 +141,7 @@ def main(): choices=AlternativeState.to_list(), default=AlternativeState.SELECTED, ), - slaves=dict(type='list'), + slaves=dict(type='list', elements='dict'), ), supports_check_mode=True, ) From 88a00aeea09aca71466eb35adfbb8910de855f40 Mon Sep 17 00:00:00 2001 From: Marius Rieder Date: Tue, 10 May 2022 07:12:08 +0200 Subject: [PATCH 03/28] alternatives: Apply suggestions from code review Co-authored-by: Felix Fontein --- plugins/modules/system/alternatives.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/plugins/modules/system/alternatives.py b/plugins/modules/system/alternatives.py index 0a5c956acb7..57895b53434 100644 --- a/plugins/modules/system/alternatives.py +++ b/plugins/modules/system/alternatives.py @@ -53,8 +53,8 @@ version_added: 4.8.0 slaves: description: - - A list of slaves - - Each slave needs a name, a link and a path parameter + - A list of slaves. + - Each slave needs a name, a link and a path parameter. type: list elements: dict suboptions: @@ -72,7 +72,7 @@ description: - The path to the symbolic link that should point to the real slave executable. type: path - version_added: 2.5.0 + version_added: 5.0.0 requirements: [ update-alternatives ] ''' @@ -102,7 +102,7 @@ state: present - name: keytool is a slave of java - alternatives: + community.general.alternatives: name: java link: /usr/bin/java path: /usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java @@ -141,7 +141,11 @@ def main(): choices=AlternativeState.to_list(), default=AlternativeState.SELECTED, ), - slaves=dict(type='list', elements='dict'), + slaves=dict(type='list', elements='dict', options=dict( + name=dict(type='str', required=True), + path=dict(type='path', required=True), + link=dict(type='path'), + )), ), supports_check_mode=True, ) @@ -215,7 +219,7 @@ def main(): cmd = [UPDATE_ALTERNATIVES, '--install', link, name, path, str(priority)] if params['slaves']: - slaves = map(lambda slave: ['--slave', slave['link'], slave['name'], slave['path']], params['slaves']) + slaves = [['--slave', slave['link'], slave['name'], slave['path']] for slave in params['slaves']] cmd += [item for sublist in slaves for item in sublist] module.run_command( From fc2b694bb3805a0756b931c56b8b03d11d350298 Mon Sep 17 00:00:00 2001 From: Marius Rieder Date: Tue, 10 May 2022 07:15:40 +0200 Subject: [PATCH 04/28] alternatives: Add schangelog for slaves parameter --- changelogs/fragments/4654-alternatives-add-slaves.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelogs/fragments/4654-alternatives-add-slaves.yml diff --git a/changelogs/fragments/4654-alternatives-add-slaves.yml b/changelogs/fragments/4654-alternatives-add-slaves.yml new file mode 100644 index 00000000000..29d276c0516 --- /dev/null +++ b/changelogs/fragments/4654-alternatives-add-slaves.yml @@ -0,0 +1,2 @@ +minor_changes: + - alternatives - add ``slaves`` parameter (https://github.com/ansible-collections/community.general/pull/4654). From 910e7fe02d10698f781b808d0a5c12b3753bdbaf Mon Sep 17 00:00:00 2001 From: Marius Rieder Date: Tue, 10 May 2022 09:20:28 +0200 Subject: [PATCH 05/28] alernatives: Add integration tests --- .../targets/alternatives/tasks/main.yml | 3 ++ .../targets/alternatives/tasks/slaves.yml | 35 +++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 tests/integration/targets/alternatives/tasks/slaves.yml diff --git a/tests/integration/targets/alternatives/tasks/main.yml b/tests/integration/targets/alternatives/tasks/main.yml index 1120cfd37de..70f75fe1619 100644 --- a/tests/integration/targets/alternatives/tasks/main.yml +++ b/tests/integration/targets/alternatives/tasks/main.yml @@ -49,6 +49,9 @@ # Test that path is checked: alternatives must fail when path is nonexistent - import_tasks: path_is_checked.yml + # Test that slave commands work + - import_tasks: slaves.yml + # Test operation of the 'state' parameter - block: - include_tasks: remove_links.yml diff --git a/tests/integration/targets/alternatives/tasks/slaves.yml b/tests/integration/targets/alternatives/tasks/slaves.yml new file mode 100644 index 00000000000..485918c21ec --- /dev/null +++ b/tests/integration/targets/alternatives/tasks/slaves.yml @@ -0,0 +1,35 @@ +- name: Try with slaves + alternatives: + name: dummymain + path: '/usr/bin/dummy1' + link: '/usr/bin/dummymain' + slaves: + - name: dummyslave + path: '/usr/bin/dummy2' + link: '/usr/bin/dummyslave' + ignore_errors: True + register: alternative + +- name: check expected command was executed + assert: + that: + - 'alternative is successful' + - 'alternative is changed' + +- name: Execute the current dummymain command + shell: dummymain + register: cmd + +- name: Ensure that the expected command was executed + assert: + that: + - cmd.stdout == "dummy1" + +- name: Execute the current dummyslave command + shell: dummyslave + register: cmd + +- name: Ensure that the expected command was executed + assert: + that: + - cmd.stdout == "dummy2" \ No newline at end of file From b3658edc22191c6ec7b70763d101f3d9dfb25c3d Mon Sep 17 00:00:00 2001 From: Marius Rieder Date: Wed, 11 May 2022 08:49:16 +0200 Subject: [PATCH 06/28] alternatives: Improv tests --- tests/integration/targets/alternatives/tasks/slaves.yml | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/integration/targets/alternatives/tasks/slaves.yml b/tests/integration/targets/alternatives/tasks/slaves.yml index 485918c21ec..b878802a3e7 100644 --- a/tests/integration/targets/alternatives/tasks/slaves.yml +++ b/tests/integration/targets/alternatives/tasks/slaves.yml @@ -7,17 +7,16 @@ - name: dummyslave path: '/usr/bin/dummy2' link: '/usr/bin/dummyslave' - ignore_errors: True register: alternative -- name: check expected command was executed +- name: Check expected command was executed assert: that: - 'alternative is successful' - 'alternative is changed' - name: Execute the current dummymain command - shell: dummymain + command: dummymain register: cmd - name: Ensure that the expected command was executed @@ -26,7 +25,7 @@ - cmd.stdout == "dummy1" - name: Execute the current dummyslave command - shell: dummyslave + command: dummyslave register: cmd - name: Ensure that the expected command was executed From 197e98cfbb6b890e2989a9e0e6da6ede81132250 Mon Sep 17 00:00:00 2001 From: Marius Rieder Date: Wed, 11 May 2022 14:01:17 +0200 Subject: [PATCH 07/28] alternatives: Update tests/integration/targets/alternatives/tasks/slaves.yml Co-authored-by: Felix Fontein --- tests/integration/targets/alternatives/tasks/slaves.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration/targets/alternatives/tasks/slaves.yml b/tests/integration/targets/alternatives/tasks/slaves.yml index b878802a3e7..744cd11a3ab 100644 --- a/tests/integration/targets/alternatives/tasks/slaves.yml +++ b/tests/integration/targets/alternatives/tasks/slaves.yml @@ -12,7 +12,6 @@ - name: Check expected command was executed assert: that: - - 'alternative is successful' - 'alternative is changed' - name: Execute the current dummymain command From 81cf47d5cf6b7909ea0595f497a5099a09ea3379 Mon Sep 17 00:00:00 2001 From: Marius Rieder Date: Thu, 12 May 2022 14:35:06 +0200 Subject: [PATCH 08/28] alternatives: Rework logic to support updating priority and subcommands --- plugins/modules/system/alternatives.py | 312 ++++++++++++++++--------- 1 file changed, 203 insertions(+), 109 deletions(-) diff --git a/plugins/modules/system/alternatives.py b/plugins/modules/system/alternatives.py index 57895b53434..9411483bb70 100644 --- a/plugins/modules/system/alternatives.py +++ b/plugins/modules/system/alternatives.py @@ -3,6 +3,7 @@ # Copyright: (c) 2014, Gabe Mulley # Copyright: (c) 2015, David Wittman +# Copyright: (c) 2022, Marius Rieder # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) from __future__ import absolute_import, division, print_function @@ -17,6 +18,7 @@ - Manages symbolic links using the 'update-alternatives' tool. - Useful when multiple programs are installed but provide similar functionality (e.g. different editors). author: + - Marius Rieder (@Jiuka) - David Wittman (@DavidWittman) - Gabe Mulley (@mulby) options: @@ -47,16 +49,19 @@ not set it as the currently selected alternative for the group. - C(selected) - install the alternative (if not already installed), and set it as the currently selected alternative for the group. - choices: [ present, selected ] + - C(absent) - removes the alternative. + version_added: 5.0.0 + choices: [ present, selected, absent ] default: selected type: str version_added: 4.8.0 - slaves: + subcommands: description: - A list of slaves. - Each slave needs a name, a link and a path parameter. type: list elements: dict + aliases: ['slaves'] suboptions: name: description: @@ -122,10 +127,203 @@ class AlternativeState: PRESENT = "present" SELECTED = "selected" + ABSENT = "absent" @classmethod def to_list(cls): - return [cls.PRESENT, cls.SELECTED] + return [cls.PRESENT, cls.SELECTED, cls.ABSENT] + + +class AlternativesModule(object): + _UPDATE_ALTERNATIVES = None + + def __init__(self, module): + self.module = module + self.result = dict(changed=False, diff=dict(before=dict(), after=dict())) + self.run() + + @property + def present(self): + return self.module.params.get('state') in [AlternativeState.PRESENT, AlternativeState.SELECTED] + + @property + def selected(self): + return self.module.params.get('state') == AlternativeState.SELECTED + + def run(self): + self.parse() + + if self.module._diff and self.path in self.current_alternatives: + self.result['diff']['before'].update(dict( + path = self.path, + priority = self.current_alternatives[self.path].get('priority'), + link = self.current_link, + subcommands = self.current_alternatives[self.path].get('subcommands') + )) + + if self.present: + # Check if we need to (re)install + if ( + self.path not in self.current_alternatives or + self.current_alternatives[self.path].get('priority') != self.priority or + not all([s in self.subcommands for s in self.current_alternatives[self.path].get('subcommands') ]) or + not all([s in self.current_alternatives[self.path].get('subcommands') for s in self.subcommands ]) + ): + self.install() + + if self.selected and self.current_path != self.path: + self.set() + + if not self.selected and self.current_path == self.path and self.current_mode == 'manual': + self.auto() + + else: + # Check if we need to uninstall + if self.path in self.current_alternatives: + self.remove() + + self.module.exit_json(**self.result) + + + def install(self): + if not self.link: + self.module.fail_json(msg='Needed to install the alternative, but unable to do so as we are missing the link') + + cmd = [self.UPDATE_ALTERNATIVES, '--install', self.link, self.name, self.path, str(self.priority)] + + if self.subcommands: + subcommands = [['--slave', subcmd['link'], subcmd['name'], subcmd['path']] for subcmd in self.subcommands] + cmd += [item for sublist in subcommands for item in sublist] + + self.result['changed'] = True + self.result['msg'] = "Install alternative '%s' for '%s'" % (self.path, self.name) + + if not self.module.check_mode: + self.module.run_command(cmd, check_rc=True) + + if self.module._diff: + self.result['diff']['after'] = dict( + path = self.path, + priority = self.priority, + link = self.link, + subcommands = self.subcommands + ) + + def remove(self): + cmd = [self.UPDATE_ALTERNATIVES, '--remove', self.name, self.path] + self.result['changed'] = True + self.result['msg'] = "Remove alternative '%s' from '%s'" % (self.path, self.name) + + if not self.module.check_mode: + self.module.run_command(cmd, check_rc=True) + + if self.module._diff: + self.result['diff']['after'] = dict(state = AlternativeState.ABSENT) + + def set(self): + cmd = [self.UPDATE_ALTERNATIVES, '--set', self.name, self.path] + self.result['changed'] = True + self.result['msg'] = "Set alternative '%s' for '%s'" % (self.path, self.name) + + if not self.module.check_mode: + self.module.run_command(cmd, check_rc=True) + + if self.module._diff: + self.result['diff']['before']['state'] = AlternativeState.PRESENT + self.result['diff']['after']['state'] = AlternativeState.SELECTED + + def auto(self): + cmd = [self.UPDATE_ALTERNATIVES, '--auto', self.name] + self.result['msg'] = "Set alternative to auto for '%s'" % (self.name) + self.result['changed'] = True + + if not self.module.check_mode: + self.module.run_command(cmd, check_rc=True) + + if self.module._diff: + self.result['diff']['before']['state'] = AlternativeState.SELECTED + self.result['diff']['after']['state'] = AlternativeState.PRESENT + + @property + def name(self): + return self.module.params.get('name') + + @property + def path(self): + return self.module.params.get('path') + + @property + def link(self): + return self.module.params.get('link') or self.current_link + + @property + def priority(self): + return self.module.params.get('priority') + + @property + def subcommands(self): + return self.module.params.get('subcommands') + + @property + def UPDATE_ALTERNATIVES(self): + if self._UPDATE_ALTERNATIVES is None: + self._UPDATE_ALTERNATIVES = self.module.get_bin_path('update-alternatives', True) + return self._UPDATE_ALTERNATIVES + + def parse(self): + self.current_mode = None + self.current_path = None + self.current_link = None + self.current_alternatives = {} + + # Run `update-alternatives --display ` to find existing alternatives + (rc, display_output, dummy) = self.module.run_command( + ['env', 'LC_ALL=C', self.UPDATE_ALTERNATIVES, '--display', self.name] + ) + + if rc != 0: + self.module.debug("No current alternative found. '%s' exited with %s" % (self.UPDATE_ALTERNATIVES, rc)) + return + + current_mode_regex = re.compile(r'\s-\s(?:status\sis\s)?(\w*)(?:\smode|.)$', re.MULTILINE) + current_path_regex = re.compile(r'^\s*link currently points to (.*)$', re.MULTILINE) + current_link_regex = re.compile(r'^\s*link \w+ is (.*)$', re.MULTILINE) + subcmd_path_link_regex = re.compile(r'^\s*slave (\S+) is (.*)$', re.MULTILINE) + + alternative_regex = re.compile(r'^(\/.*)\s-\s(?:family\s\S+\s)?priority\s(\d+)((?:\s+slave.*)*)', re.MULTILINE) + subcmd_regex = re.compile('^\s+slave (.*): (.*)$', re.MULTILINE) + + match = current_mode_regex.search(display_output) + if not match: + self.module.debug("No current path found in output") + return + self.current_mode = match.group(1) + + match = current_path_regex.search(display_output) + if not match: + self.module.debug("No current path found in output") + return + self.current_path = match.group(1) + + match = current_link_regex.search(display_output) + if not match: + self.module.debug("No current link found in output") + else: + self.current_link = match.group(1) + + subcmd_path_map = dict(subcmd_path_link_regex.findall(display_output)) + if not subcmd_path_map: + subcmd_path_map = {s['name']: s['link'] for s in self.subcommands} + + for path, prio, subcmd in alternative_regex.findall(display_output): + self.current_alternatives[path] = dict( + priority = int(prio), + subcommands = [dict( + name = name, + path = path, + link = subcmd_path_map.get(name) + ) for name, path in subcmd_regex.findall(subcmd) if path != '(null)'] + ) def main(): @@ -141,7 +339,7 @@ def main(): choices=AlternativeState.to_list(), default=AlternativeState.SELECTED, ), - slaves=dict(type='list', elements='dict', options=dict( + subcommands=dict(type='list', elements='dict', aliases=['slaves'], default=[], options=dict( name=dict(type='str', required=True), path=dict(type='path', required=True), link=dict(type='path'), @@ -150,111 +348,7 @@ def main(): supports_check_mode=True, ) - params = module.params - name = params['name'] - path = params['path'] - link = params['link'] - priority = params['priority'] - state = params['state'] - - UPDATE_ALTERNATIVES = module.get_bin_path('update-alternatives', True) - - current_path = None - all_alternatives = [] - - # Run `update-alternatives --display ` to find existing alternatives - (rc, display_output, dummy) = module.run_command( - ['env', 'LC_ALL=C', UPDATE_ALTERNATIVES, '--display', name] - ) - - if rc == 0: - # Alternatives already exist for this link group - # Parse the output to determine the current path of the symlink and - # available alternatives - current_path_regex = re.compile(r'^\s*link currently points to (.*)$', - re.MULTILINE) - alternative_regex = re.compile(r'^(\/.*)\s-\s(?:family\s\S+\s)?priority', re.MULTILINE) - - match = current_path_regex.search(display_output) - if match: - current_path = match.group(1) - all_alternatives = alternative_regex.findall(display_output) - - if not link: - # Read the current symlink target from `update-alternatives --query` - # in case we need to install the new alternative before setting it. - # - # This is only compatible on Debian-based systems, as the other - # alternatives don't have --query available - rc, query_output, dummy = module.run_command( - ['env', 'LC_ALL=C', UPDATE_ALTERNATIVES, '--query', name] - ) - if rc == 0: - for line in query_output.splitlines(): - if line.startswith('Link:'): - link = line.split()[1] - break - - changed = False - if current_path != path: - - # Check mode: expect a change if this alternative is not already - # installed, or if it is to be set as the current selection. - if module.check_mode: - module.exit_json( - changed=( - path not in all_alternatives or - state == AlternativeState.SELECTED - ), - current_path=current_path, - ) - - try: - # install the requested path if necessary - if path not in all_alternatives: - if not os.path.exists(path): - module.fail_json(msg="Specified path %s does not exist" % path) - if not link: - module.fail_json(msg="Needed to install the alternative, but unable to do so as we are missing the link") - - cmd = [UPDATE_ALTERNATIVES, '--install', link, name, path, str(priority)] - if params['slaves']: - slaves = [['--slave', slave['link'], slave['name'], slave['path']] for slave in params['slaves']] - cmd += [item for sublist in slaves for item in sublist] - - module.run_command( - cmd, - check_rc=True - ) - changed = True - - # set the current selection to this path (if requested) - if state == AlternativeState.SELECTED: - module.run_command( - [UPDATE_ALTERNATIVES, '--set', name, path], - check_rc=True - ) - changed = True - - except subprocess.CalledProcessError as cpe: - module.fail_json(msg=str(dir(cpe))) - elif current_path == path and state == AlternativeState.PRESENT: - # Case where alternative is currently selected, but state is set - # to 'present'. In this case, we set to auto mode. - if module.check_mode: - module.exit_json(changed=True, current_path=current_path) - - changed = True - try: - module.run_command( - [UPDATE_ALTERNATIVES, '--auto', name], - check_rc=True, - ) - except subprocess.CalledProcessError as cpe: - module.fail_json(msg=str(dir(cpe))) - - module.exit_json(changed=changed) - + AlternativesModule(module) if __name__ == '__main__': main() From 4516c8878953436f6aa3cf85f0248886d1fbbdcf Mon Sep 17 00:00:00 2001 From: Marius Rieder Date: Thu, 12 May 2022 14:38:06 +0200 Subject: [PATCH 09/28] alternatives: Use more inclusive naming --- .../integration/targets/alternatives/tasks/main.yml | 4 ++-- .../tasks/{slaves.yml => subcommands.yml} | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) rename tests/integration/targets/alternatives/tasks/{slaves.yml => subcommands.yml} (75%) diff --git a/tests/integration/targets/alternatives/tasks/main.yml b/tests/integration/targets/alternatives/tasks/main.yml index 70f75fe1619..70853e80054 100644 --- a/tests/integration/targets/alternatives/tasks/main.yml +++ b/tests/integration/targets/alternatives/tasks/main.yml @@ -49,8 +49,8 @@ # Test that path is checked: alternatives must fail when path is nonexistent - import_tasks: path_is_checked.yml - # Test that slave commands work - - import_tasks: slaves.yml + # Test that subcommands commands work + - import_tasks: subcommands.yml # Test operation of the 'state' parameter - block: diff --git a/tests/integration/targets/alternatives/tasks/slaves.yml b/tests/integration/targets/alternatives/tasks/subcommands.yml similarity index 75% rename from tests/integration/targets/alternatives/tasks/slaves.yml rename to tests/integration/targets/alternatives/tasks/subcommands.yml index 744cd11a3ab..62b5c6c3451 100644 --- a/tests/integration/targets/alternatives/tasks/slaves.yml +++ b/tests/integration/targets/alternatives/tasks/subcommands.yml @@ -1,12 +1,12 @@ -- name: Try with slaves +- name: Try with subcommands alternatives: name: dummymain path: '/usr/bin/dummy1' link: '/usr/bin/dummymain' - slaves: - - name: dummyslave + subcommands: + - name: dummysubcmd path: '/usr/bin/dummy2' - link: '/usr/bin/dummyslave' + link: '/usr/bin/dummysubcmd' register: alternative - name: Check expected command was executed @@ -23,8 +23,8 @@ that: - cmd.stdout == "dummy1" -- name: Execute the current dummyslave command - command: dummyslave +- name: Execute the current dummysubcmd command + command: dummysubcmd register: cmd - name: Ensure that the expected command was executed From 8ee5cfdc05102d99057204f67265cfd86ec47059 Mon Sep 17 00:00:00 2001 From: Marius Rieder Date: Thu, 12 May 2022 14:54:36 +0200 Subject: [PATCH 10/28] alternatives: Fix linter warnings --- .github/BOTMETA.yml | 2 +- plugins/modules/system/alternatives.py | 89 +++++++++++++------------- 2 files changed, 46 insertions(+), 45 deletions(-) diff --git a/.github/BOTMETA.yml b/.github/BOTMETA.yml index ecedff1e81b..d829d2a89f2 100644 --- a/.github/BOTMETA.yml +++ b/.github/BOTMETA.yml @@ -1024,7 +1024,7 @@ files: $modules/system/alternatives.py: maintainers: mulby labels: alternatives - ignore: DavidWittman + ignore: DavidWittman jiuka $modules/system/aix_lvol.py: maintainers: adejoux $modules/system/awall.py: diff --git a/plugins/modules/system/alternatives.py b/plugins/modules/system/alternatives.py index 9411483bb70..bc4133a8aac 100644 --- a/plugins/modules/system/alternatives.py +++ b/plugins/modules/system/alternatives.py @@ -18,7 +18,7 @@ - Manages symbolic links using the 'update-alternatives' tool. - Useful when multiple programs are installed but provide similar functionality (e.g. different editors). author: - - Marius Rieder (@Jiuka) + - Marius Rieder (@jiuka) - David Wittman (@DavidWittman) - Gabe Mulley (@mulby) options: @@ -49,33 +49,32 @@ not set it as the currently selected alternative for the group. - C(selected) - install the alternative (if not already installed), and set it as the currently selected alternative for the group. - - C(absent) - removes the alternative. - version_added: 5.0.0 + - C(absent) - removes the alternative. (Added in version 5.0.0) choices: [ present, selected, absent ] default: selected type: str version_added: 4.8.0 subcommands: description: - - A list of slaves. - - Each slave needs a name, a link and a path parameter. + - A list of subcommands. + - Each subcommand needs a name, a link and a path parameter. type: list elements: dict aliases: ['slaves'] suboptions: name: description: - - The generic name of the slave. + - The generic name of the subcommand. type: str required: true path: description: - - The path to the real executable that the slave should point to. + - The path to the real executable that the subcommand should point to. type: path required: true link: description: - - The path to the symbolic link that should point to the real slave executable. + - The path to the symbolic link that should point to the real subcommand executable. type: path version_added: 5.0.0 requirements: [ update-alternatives ] @@ -106,12 +105,12 @@ link: /usr/bin/python state: present -- name: keytool is a slave of java +- name: keytool is a subcommand of java community.general.alternatives: name: java link: /usr/bin/java path: /usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java - slaves: + subcommands: - name: keytool link: /usr/bin/keytool path: /usr/lib/jvm/java-7-openjdk-amd64/jre/bin/keytool @@ -155,10 +154,10 @@ def run(self): if self.module._diff and self.path in self.current_alternatives: self.result['diff']['before'].update(dict( - path = self.path, - priority = self.current_alternatives[self.path].get('priority'), - link = self.current_link, - subcommands = self.current_alternatives[self.path].get('subcommands') + path=self.path, + priority=self.current_alternatives[self.path].get('priority'), + link=self.current_link, + subcommands=self.current_alternatives[self.path].get('subcommands') )) if self.present: @@ -166,8 +165,8 @@ def run(self): if ( self.path not in self.current_alternatives or self.current_alternatives[self.path].get('priority') != self.priority or - not all([s in self.subcommands for s in self.current_alternatives[self.path].get('subcommands') ]) or - not all([s in self.current_alternatives[self.path].get('subcommands') for s in self.subcommands ]) + not all(s in self.subcommands for s in self.current_alternatives[self.path].get('subcommands')) or + not all(s in self.current_alternatives[self.path].get('subcommands') for s in self.subcommands) ): self.install() @@ -184,8 +183,9 @@ def run(self): self.module.exit_json(**self.result) - def install(self): + if not os.path.exists(self.path): + self.module.fail_json(msg="Specified path %s does not exist" % self.path) if not self.link: self.module.fail_json(msg='Needed to install the alternative, but unable to do so as we are missing the link') @@ -197,46 +197,46 @@ def install(self): self.result['changed'] = True self.result['msg'] = "Install alternative '%s' for '%s'" % (self.path, self.name) - + if not self.module.check_mode: self.module.run_command(cmd, check_rc=True) if self.module._diff: self.result['diff']['after'] = dict( - path = self.path, - priority = self.priority, - link = self.link, - subcommands = self.subcommands + path=self.path, + priority=self.priority, + link=self.link, + subcommands=self.subcommands ) - + def remove(self): cmd = [self.UPDATE_ALTERNATIVES, '--remove', self.name, self.path] self.result['changed'] = True self.result['msg'] = "Remove alternative '%s' from '%s'" % (self.path, self.name) - + if not self.module.check_mode: self.module.run_command(cmd, check_rc=True) if self.module._diff: - self.result['diff']['after'] = dict(state = AlternativeState.ABSENT) - + self.result['diff']['after'] = dict(state=AlternativeState.ABSENT) + def set(self): cmd = [self.UPDATE_ALTERNATIVES, '--set', self.name, self.path] self.result['changed'] = True self.result['msg'] = "Set alternative '%s' for '%s'" % (self.path, self.name) - + if not self.module.check_mode: self.module.run_command(cmd, check_rc=True) if self.module._diff: self.result['diff']['before']['state'] = AlternativeState.PRESENT self.result['diff']['after']['state'] = AlternativeState.SELECTED - + def auto(self): cmd = [self.UPDATE_ALTERNATIVES, '--auto', self.name] self.result['msg'] = "Set alternative to auto for '%s'" % (self.name) self.result['changed'] = True - + if not self.module.check_mode: self.module.run_command(cmd, check_rc=True) @@ -246,29 +246,29 @@ def auto(self): @property def name(self): - return self.module.params.get('name') + return self.module.params.get('name') @property def path(self): - return self.module.params.get('path') + return self.module.params.get('path') @property def link(self): - return self.module.params.get('link') or self.current_link + return self.module.params.get('link') or self.current_link @property def priority(self): - return self.module.params.get('priority') + return self.module.params.get('priority') @property def subcommands(self): - return self.module.params.get('subcommands') + return self.module.params.get('subcommands') @property def UPDATE_ALTERNATIVES(self): - if self._UPDATE_ALTERNATIVES is None: - self._UPDATE_ALTERNATIVES = self.module.get_bin_path('update-alternatives', True) - return self._UPDATE_ALTERNATIVES + if self._UPDATE_ALTERNATIVES is None: + self._UPDATE_ALTERNATIVES = self.module.get_bin_path('update-alternatives', True) + return self._UPDATE_ALTERNATIVES def parse(self): self.current_mode = None @@ -291,7 +291,7 @@ def parse(self): subcmd_path_link_regex = re.compile(r'^\s*slave (\S+) is (.*)$', re.MULTILINE) alternative_regex = re.compile(r'^(\/.*)\s-\s(?:family\s\S+\s)?priority\s(\d+)((?:\s+slave.*)*)', re.MULTILINE) - subcmd_regex = re.compile('^\s+slave (.*): (.*)$', re.MULTILINE) + subcmd_regex = re.compile(r'^\s+slave (.*): (.*)$', re.MULTILINE) match = current_mode_regex.search(display_output) if not match: @@ -313,15 +313,15 @@ def parse(self): subcmd_path_map = dict(subcmd_path_link_regex.findall(display_output)) if not subcmd_path_map: - subcmd_path_map = {s['name']: s['link'] for s in self.subcommands} + subcmd_path_map = dict((s['name'], s['link']) for s in self.subcommands) for path, prio, subcmd in alternative_regex.findall(display_output): self.current_alternatives[path] = dict( - priority = int(prio), - subcommands = [dict( - name = name, - path = path, - link = subcmd_path_map.get(name) + priority=int(prio), + subcommands=[dict( + name=name, + path=path, + link=subcmd_path_map.get(name) ) for name, path in subcmd_regex.findall(subcmd) if path != '(null)'] ) @@ -350,5 +350,6 @@ def main(): AlternativesModule(module) + if __name__ == '__main__': main() From 7ff3983932488c4bcf49743a6fa0be6bdfb2324c Mon Sep 17 00:00:00 2001 From: Marius Rieder Date: Thu, 12 May 2022 18:24:39 +0200 Subject: [PATCH 11/28] alternatives: Dont fail if link is absent --- plugins/modules/system/alternatives.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/modules/system/alternatives.py b/plugins/modules/system/alternatives.py index bc4133a8aac..0e1488d3148 100644 --- a/plugins/modules/system/alternatives.py +++ b/plugins/modules/system/alternatives.py @@ -295,15 +295,15 @@ def parse(self): match = current_mode_regex.search(display_output) if not match: - self.module.debug("No current path found in output") + self.module.debug("No current mode found in output") return self.current_mode = match.group(1) match = current_path_regex.search(display_output) if not match: self.module.debug("No current path found in output") - return - self.current_path = match.group(1) + else: + self.current_path = match.group(1) match = current_link_regex.search(display_output) if not match: From e9a128fc6d56d95bff951e9bd0f3f1f7287e1f56 Mon Sep 17 00:00:00 2001 From: Marius Rieder Date: Thu, 12 May 2022 19:45:33 +0200 Subject: [PATCH 12/28] alternatives: Update changelog fragment --- changelogs/fragments/4654-alternatives-add-slaves.yml | 2 -- changelogs/fragments/4654-alternatives-add-subcommands.yml | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) delete mode 100644 changelogs/fragments/4654-alternatives-add-slaves.yml create mode 100644 changelogs/fragments/4654-alternatives-add-subcommands.yml diff --git a/changelogs/fragments/4654-alternatives-add-slaves.yml b/changelogs/fragments/4654-alternatives-add-slaves.yml deleted file mode 100644 index 29d276c0516..00000000000 --- a/changelogs/fragments/4654-alternatives-add-slaves.yml +++ /dev/null @@ -1,2 +0,0 @@ -minor_changes: - - alternatives - add ``slaves`` parameter (https://github.com/ansible-collections/community.general/pull/4654). diff --git a/changelogs/fragments/4654-alternatives-add-subcommands.yml b/changelogs/fragments/4654-alternatives-add-subcommands.yml new file mode 100644 index 00000000000..a25c9ae19e8 --- /dev/null +++ b/changelogs/fragments/4654-alternatives-add-subcommands.yml @@ -0,0 +1,2 @@ +minor_changes: + - alternatives - add ``subcommands`` parameter (https://github.com/ansible-collections/community.general/pull/4654). From c2423825eb5a9cfb96326220c72029f2e561fc7b Mon Sep 17 00:00:00 2001 From: Marius Rieder Date: Fri, 13 May 2022 16:22:14 +0200 Subject: [PATCH 13/28] alternatives: Add tests for prio change and removing --- .../targets/alternatives/tasks/test.yml | 4 +--- .../alternatives/tasks/tests_set_priority.yml | 11 ++++++++++ .../alternatives/tasks/tests_state.yml | 22 +++++++++++++++++++ 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/tests/integration/targets/alternatives/tasks/test.yml b/tests/integration/targets/alternatives/tasks/test.yml index 92721a995d3..a5b36ce9224 100644 --- a/tests/integration/targets/alternatives/tasks/test.yml +++ b/tests/integration/targets/alternatives/tasks/test.yml @@ -48,6 +48,4 @@ when: ansible_os_family == 'RedHat' and not with_alternatives and item == 1 - name: check that alternative has been updated - command: "grep -Pzq '/bin/dummy{{ item }}\\n' '{{ alternatives_dir }}/dummy'" - # priority doesn't seem updated - #command: "grep -Pzq '/bin/dummy{{ item }}\\n50' '{{ alternatives_dir }}/dummy'" + command: "grep -Pzq '/bin/dummy{{ item }}\\n50' '{{ alternatives_dir }}/dummy'" diff --git a/tests/integration/targets/alternatives/tasks/tests_set_priority.yml b/tests/integration/targets/alternatives/tasks/tests_set_priority.yml index ab79f62a3c9..a629e3f3682 100644 --- a/tests/integration/targets/alternatives/tasks/tests_set_priority.yml +++ b/tests/integration/targets/alternatives/tasks/tests_set_priority.yml @@ -21,3 +21,14 @@ - name: check that alternative has been updated command: "grep -Pzq '/bin/dummy{{ item }}\\n{{ 60 + item|int }}' '{{ alternatives_dir }}/dummy'" + +- name: update dummy priority + alternatives: + name: dummy + path: '/usr/bin/dummy{{ item }}' + link: /usr/bin/dummy + priority: '{{ 70 + item|int }}' + register: alternative + +- name: check that alternative priority has been updated + command: "grep -Pzq '/bin/dummy{{ item }}\\n{{ 70 + item|int }}' '{{ alternatives_dir }}/dummy'" \ No newline at end of file diff --git a/tests/integration/targets/alternatives/tasks/tests_state.yml b/tests/integration/targets/alternatives/tasks/tests_state.yml index 357da315ed4..abeb09ea185 100644 --- a/tests/integration/targets/alternatives/tasks/tests_state.yml +++ b/tests/integration/targets/alternatives/tasks/tests_state.yml @@ -69,3 +69,25 @@ assert: that: - cmd.stdout == "dummy2" + +# Remove an alternative with state = 'absent' and make sure that +# this change results in the alternative being removed. +- name: Remove best dummy alternative with state = absent + alternatives: + name: dummy + path: /usr/bin/dummy2 + state: absent + +- name: Ensure that the link group is in auto mode + shell: 'grep "/usr/bin/dummy2" {{ alternatives_dir }}/dummy' + register: cmd + failed_when: cmd.rc == 0 + +- name: Execute the current dummy command + shell: dummy + register: cmd + +- name: Ensure that the expected command was executed + assert: + that: + - cmd.stdout == "dummy1" From 63e1d2c8a1a89379fb94eef66070e468d1089d5f Mon Sep 17 00:00:00 2001 From: Marius Rieder Date: Fri, 13 May 2022 16:25:06 +0200 Subject: [PATCH 14/28] alternatives: Apply suggestions from code review Co-authored-by: Felix Fontein --- changelogs/fragments/4654-alternatives-add-subcommands.yml | 1 + plugins/modules/system/alternatives.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/changelogs/fragments/4654-alternatives-add-subcommands.yml b/changelogs/fragments/4654-alternatives-add-subcommands.yml index a25c9ae19e8..f771e9b51c8 100644 --- a/changelogs/fragments/4654-alternatives-add-subcommands.yml +++ b/changelogs/fragments/4654-alternatives-add-subcommands.yml @@ -1,2 +1,3 @@ minor_changes: - alternatives - add ``subcommands`` parameter (https://github.com/ansible-collections/community.general/pull/4654). + - alternatives - add ``state=absent`` to be able to remove an alternative (https://github.com/ansible-collections/community.general/pull/4654). diff --git a/plugins/modules/system/alternatives.py b/plugins/modules/system/alternatives.py index 0e1488d3148..b26f7850fdb 100644 --- a/plugins/modules/system/alternatives.py +++ b/plugins/modules/system/alternatives.py @@ -49,7 +49,7 @@ not set it as the currently selected alternative for the group. - C(selected) - install the alternative (if not already installed), and set it as the currently selected alternative for the group. - - C(absent) - removes the alternative. (Added in version 5.0.0) + - C(absent) - remove the alternative. Added in community.general 5.0.0. choices: [ present, selected, absent ] default: selected type: str From aaf2a8378435bd44e5c3c4c2b842d424c3cdb83d Mon Sep 17 00:00:00 2001 From: Marius Rieder Date: Fri, 13 May 2022 21:28:24 +0200 Subject: [PATCH 15/28] alternatives: Add `state=auto`to reset mode to auto --- plugins/modules/system/alternatives.py | 33 ++++++++++++++----- .../alternatives/tasks/tests_state.yml | 24 +++++++++++++- 2 files changed, 47 insertions(+), 10 deletions(-) diff --git a/plugins/modules/system/alternatives.py b/plugins/modules/system/alternatives.py index b26f7850fdb..4eb4cde3b74 100644 --- a/plugins/modules/system/alternatives.py +++ b/plugins/modules/system/alternatives.py @@ -49,7 +49,9 @@ not set it as the currently selected alternative for the group. - C(selected) - install the alternative (if not already installed), and set it as the currently selected alternative for the group. - - C(absent) - remove the alternative. Added in community.general 5.0.0. + - C(auto) - install the alternative (if not already installed), and + set the group to auto mode. (Added in version 5.0.0) + - C(absent) - removes the alternative. (Added in version 5.0.0) choices: [ present, selected, absent ] default: selected type: str @@ -105,6 +107,13 @@ link: /usr/bin/python state: present +- name: Install Python 3.5 and reset selection to auto + community.general.alternatives: + name: python + path: /usr/bin/python3.5 + link: /usr/bin/python + state: auto + - name: keytool is a subcommand of java community.general.alternatives: name: java @@ -127,10 +136,11 @@ class AlternativeState: PRESENT = "present" SELECTED = "selected" ABSENT = "absent" + AUTO = "auto" @classmethod def to_list(cls): - return [cls.PRESENT, cls.SELECTED, cls.ABSENT] + return [cls.PRESENT, cls.SELECTED, cls.ABSENT, cls.AUTO] class AlternativesModule(object): @@ -142,13 +152,17 @@ def __init__(self, module): self.run() @property - def present(self): - return self.module.params.get('state') in [AlternativeState.PRESENT, AlternativeState.SELECTED] + def mode_present(self): + return self.module.params.get('state') in [AlternativeState.PRESENT, AlternativeState.SELECTED, AlternativeState.AUTO] @property - def selected(self): + def mode_selected(self): return self.module.params.get('state') == AlternativeState.SELECTED + @property + def mode_auto(self): + return self.module.params.get('state') == AlternativeState.AUTO + def run(self): self.parse() @@ -160,7 +174,7 @@ def run(self): subcommands=self.current_alternatives[self.path].get('subcommands') )) - if self.present: + if self.mode_present: # Check if we need to (re)install if ( self.path not in self.current_alternatives or @@ -170,12 +184,13 @@ def run(self): ): self.install() - if self.selected and self.current_path != self.path: + # Check if we need to set the preference + if self.mode_selected and self.current_path != self.path: self.set() - if not self.selected and self.current_path == self.path and self.current_mode == 'manual': + #Check if we need to reset to auto + if self.mode_auto and self.current_mode == 'manual': self.auto() - else: # Check if we need to uninstall if self.path in self.current_alternatives: diff --git a/tests/integration/targets/alternatives/tasks/tests_state.yml b/tests/integration/targets/alternatives/tasks/tests_state.yml index abeb09ea185..dd3be565baa 100644 --- a/tests/integration/targets/alternatives/tasks/tests_state.yml +++ b/tests/integration/targets/alternatives/tasks/tests_state.yml @@ -49,6 +49,28 @@ - cmd.stdout == "dummy4" # Set the currently selected alternative to state = 'present' (was previously +# selected), and ensure that this results in the group not being set to 'auto' +# mode, and the alternative is still selected. +- name: Set current selected dummy to state = present + alternatives: + name: dummy + path: /usr/bin/dummy4 + link: /usr/bin/dummy + state: present + +- name: Ensure that the link group is in auto mode + shell: 'head -n1 {{ alternatives_dir }}/dummy | grep "^manual$"' + +- name: Execute the current dummy command + shell: dummy + register: cmd + +- name: Ensure that the expected command was executed + assert: + that: + - cmd.stdout == "dummy4" + +# Set the currently selected alternative to state = 'auto' (was previously # selected), and ensure that this results in the group being set to 'auto' # mode, and the highest priority alternative is selected. - name: Set current selected dummy to state = present @@ -56,7 +78,7 @@ name: dummy path: /usr/bin/dummy4 link: /usr/bin/dummy - state: present + state: auto - name: Ensure that the link group is in auto mode shell: 'head -n1 {{ alternatives_dir }}/dummy | grep "^auto$"' From 87b9f66595628ce44cbe5f4644c0427364dd2131 Mon Sep 17 00:00:00 2001 From: Marius Rieder Date: Fri, 13 May 2022 21:42:33 +0200 Subject: [PATCH 16/28] alternatives: Fix linter warnings --- plugins/modules/system/alternatives.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/modules/system/alternatives.py b/plugins/modules/system/alternatives.py index 4eb4cde3b74..3925f5ad6fc 100644 --- a/plugins/modules/system/alternatives.py +++ b/plugins/modules/system/alternatives.py @@ -188,7 +188,7 @@ def run(self): if self.mode_selected and self.current_path != self.path: self.set() - #Check if we need to reset to auto + # Check if we need to reset to auto if self.mode_auto and self.current_mode == 'manual': self.auto() else: From 708ef3eb70753b42e6fadd8879bb0e238cb43ca4 Mon Sep 17 00:00:00 2001 From: Marius Rieder Date: Sat, 14 May 2022 20:38:09 +0200 Subject: [PATCH 17/28] alternatives: Fix documentation. --- plugins/modules/system/alternatives.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/modules/system/alternatives.py b/plugins/modules/system/alternatives.py index 3925f5ad6fc..3dc240d7099 100644 --- a/plugins/modules/system/alternatives.py +++ b/plugins/modules/system/alternatives.py @@ -50,9 +50,9 @@ - C(selected) - install the alternative (if not already installed), and set it as the currently selected alternative for the group. - C(auto) - install the alternative (if not already installed), and - set the group to auto mode. (Added in version 5.0.0) - - C(absent) - removes the alternative. (Added in version 5.0.0) - choices: [ present, selected, absent ] + set the group to auto mode. Added in community.general 5.0.0. + - C(absent) - removes the alternative. Added in community.general 5.0.0. + choices: [ present, selected, auto, absent ] default: selected type: str version_added: 4.8.0 From 851be5e2307bbd9b0a199ff40cdb7b7cb12d2d6a Mon Sep 17 00:00:00 2001 From: Marius Rieder Date: Sat, 14 May 2022 20:41:38 +0200 Subject: [PATCH 18/28] alternatives: Combine multiple messages. --- plugins/modules/system/alternatives.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/plugins/modules/system/alternatives.py b/plugins/modules/system/alternatives.py index 3dc240d7099..eb55cc8786e 100644 --- a/plugins/modules/system/alternatives.py +++ b/plugins/modules/system/alternatives.py @@ -149,6 +149,7 @@ class AlternativesModule(object): def __init__(self, module): self.module = module self.result = dict(changed=False, diff=dict(before=dict(), after=dict())) + self.messages = [] self.run() @property @@ -196,6 +197,7 @@ def run(self): if self.path in self.current_alternatives: self.remove() + self.result['msg'] = ' '.join(self.messages) self.module.exit_json(**self.result) def install(self): @@ -211,7 +213,7 @@ def install(self): cmd += [item for sublist in subcommands for item in sublist] self.result['changed'] = True - self.result['msg'] = "Install alternative '%s' for '%s'" % (self.path, self.name) + self.messages.append("Install alternative '%s' for '%s'." % (self.path, self.name)) if not self.module.check_mode: self.module.run_command(cmd, check_rc=True) @@ -227,7 +229,7 @@ def install(self): def remove(self): cmd = [self.UPDATE_ALTERNATIVES, '--remove', self.name, self.path] self.result['changed'] = True - self.result['msg'] = "Remove alternative '%s' from '%s'" % (self.path, self.name) + self.messages.append("Remove alternative '%s' from '%s'." % (self.path, self.name)) if not self.module.check_mode: self.module.run_command(cmd, check_rc=True) @@ -238,7 +240,7 @@ def remove(self): def set(self): cmd = [self.UPDATE_ALTERNATIVES, '--set', self.name, self.path] self.result['changed'] = True - self.result['msg'] = "Set alternative '%s' for '%s'" % (self.path, self.name) + self.messages.append("Set alternative '%s' for '%s'." % (self.path, self.name)) if not self.module.check_mode: self.module.run_command(cmd, check_rc=True) @@ -249,7 +251,7 @@ def set(self): def auto(self): cmd = [self.UPDATE_ALTERNATIVES, '--auto', self.name] - self.result['msg'] = "Set alternative to auto for '%s'" % (self.name) + self.messages.append("Set alternative to auto for '%s'." % (self.name)) self.result['changed'] = True if not self.module.check_mode: From 56a09b7ea27daa88a58ae8e06c614a9b676a4d06 Mon Sep 17 00:00:00 2001 From: Marius Rieder Date: Sat, 14 May 2022 20:42:54 +0200 Subject: [PATCH 19/28] alternatives: Set command env for all commands. --- plugins/modules/system/alternatives.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/modules/system/alternatives.py b/plugins/modules/system/alternatives.py index eb55cc8786e..3d3389aa6ad 100644 --- a/plugins/modules/system/alternatives.py +++ b/plugins/modules/system/alternatives.py @@ -149,6 +149,7 @@ class AlternativesModule(object): def __init__(self, module): self.module = module self.result = dict(changed=False, diff=dict(before=dict(), after=dict())) + self.module.run_command_environ_update = {'LC_ALL': 'C'} self.messages = [] self.run() @@ -295,7 +296,7 @@ def parse(self): # Run `update-alternatives --display ` to find existing alternatives (rc, display_output, dummy) = self.module.run_command( - ['env', 'LC_ALL=C', self.UPDATE_ALTERNATIVES, '--display', self.name] + [self.UPDATE_ALTERNATIVES, '--display', self.name] ) if rc != 0: From 6ef4b5f2757dfee5c9c6ba1730da47f2d3c5f250 Mon Sep 17 00:00:00 2001 From: Marius Rieder Date: Sat, 14 May 2022 20:45:20 +0200 Subject: [PATCH 20/28] alternatives: Do not update subcommands if parameter is omited --- plugins/modules/system/alternatives.py | 15 ++++-- .../alternatives/tasks/subcommands.yml | 47 ++++++++++++++++++- 2 files changed, 56 insertions(+), 6 deletions(-) diff --git a/plugins/modules/system/alternatives.py b/plugins/modules/system/alternatives.py index 3d3389aa6ad..899717b3cf2 100644 --- a/plugins/modules/system/alternatives.py +++ b/plugins/modules/system/alternatives.py @@ -181,8 +181,10 @@ def run(self): if ( self.path not in self.current_alternatives or self.current_alternatives[self.path].get('priority') != self.priority or - not all(s in self.subcommands for s in self.current_alternatives[self.path].get('subcommands')) or - not all(s in self.current_alternatives[self.path].get('subcommands') for s in self.subcommands) + (self.subcommands is not None and ( + not all(s in self.subcommands for s in self.current_alternatives[self.path].get('subcommands')) or + not all(s in self.current_alternatives[self.path].get('subcommands') for s in self.subcommands) + )) ): self.install() @@ -209,9 +211,12 @@ def install(self): cmd = [self.UPDATE_ALTERNATIVES, '--install', self.link, self.name, self.path, str(self.priority)] - if self.subcommands: + if self.subcommands is not None: subcommands = [['--slave', subcmd['link'], subcmd['name'], subcmd['path']] for subcmd in self.subcommands] cmd += [item for sublist in subcommands for item in sublist] + elif self.path in self.current_alternatives and self.current_alternatives[self.path].get('subcommands'): + subcommands = [['--slave', subcmd['link'], subcmd['name'], subcmd['path']] for subcmd in self.current_alternatives[self.path].get('subcommands')] + cmd += [item for sublist in subcommands for item in sublist] self.result['changed'] = True self.messages.append("Install alternative '%s' for '%s'." % (self.path, self.name)) @@ -330,7 +335,7 @@ def parse(self): self.current_link = match.group(1) subcmd_path_map = dict(subcmd_path_link_regex.findall(display_output)) - if not subcmd_path_map: + if not subcmd_path_map and self.subcommands: subcmd_path_map = dict((s['name'], s['link']) for s in self.subcommands) for path, prio, subcmd in alternative_regex.findall(display_output): @@ -357,7 +362,7 @@ def main(): choices=AlternativeState.to_list(), default=AlternativeState.SELECTED, ), - subcommands=dict(type='list', elements='dict', aliases=['slaves'], default=[], options=dict( + subcommands=dict(type='list', elements='dict', aliases=['slaves'], options=dict( name=dict(type='str', required=True), path=dict(type='path', required=True), link=dict(type='path'), diff --git a/tests/integration/targets/alternatives/tasks/subcommands.yml b/tests/integration/targets/alternatives/tasks/subcommands.yml index 62b5c6c3451..b196319366b 100644 --- a/tests/integration/targets/alternatives/tasks/subcommands.yml +++ b/tests/integration/targets/alternatives/tasks/subcommands.yml @@ -30,4 +30,49 @@ - name: Ensure that the expected command was executed assert: that: - - cmd.stdout == "dummy2" \ No newline at end of file + - cmd.stdout == "dummy2" + +- name: Subcommands ar not removed if not specified + alternatives: + name: dummymain + path: '/usr/bin/dummy1' + link: '/usr/bin/dummymain' + register: alternative + +- name: Check expected command was executed + assert: + that: + - 'alternative is not changed' + +- name: Execute the current dummysubcmd command + command: dummysubcmd + register: cmd + +- name: Ensure that the expected command was executed + assert: + that: + - cmd.stdout == "dummy2" + +- name: Subcommands ar not removed if not specified + alternatives: + name: dummymain + path: '/usr/bin/dummy1' + link: '/usr/bin/dummymain' + subcommands: [] + register: alternative + +- name: Check expected command was executed + assert: + that: + - 'alternative is changed' + +- name: Execute the current dummysubcmd command + command: dummysubcmd + register: cmd + ignore_errors: True + +- name: Ensure that the subcommand is gone + assert: + that: + - cmd.rc == 2 + - '"No such file" in cmd.msg' \ No newline at end of file From 66a7b3db71ce0bb361789ca0b14b7ac69c177b2d Mon Sep 17 00:00:00 2001 From: Marius Rieder Date: Sun, 15 May 2022 10:24:47 +0200 Subject: [PATCH 21/28] alternatives: Fix a bug with python 2.7 var scoping --- plugins/modules/system/alternatives.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/modules/system/alternatives.py b/plugins/modules/system/alternatives.py index 899717b3cf2..42f11689ba5 100644 --- a/plugins/modules/system/alternatives.py +++ b/plugins/modules/system/alternatives.py @@ -343,9 +343,9 @@ def parse(self): priority=int(prio), subcommands=[dict( name=name, - path=path, + path=spath, link=subcmd_path_map.get(name) - ) for name, path in subcmd_regex.findall(subcmd) if path != '(null)'] + ) for name, spath in subcmd_regex.findall(subcmd) if spath != '(null)'] ) From e2751ce32e47e5adbe87c7c9a3d2e8985c3768ed Mon Sep 17 00:00:00 2001 From: Marius Rieder Date: Sun, 15 May 2022 10:34:28 +0200 Subject: [PATCH 22/28] alternatives: Improce diff before generation --- plugins/modules/system/alternatives.py | 28 +++++++++++++++++--------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/plugins/modules/system/alternatives.py b/plugins/modules/system/alternatives.py index 42f11689ba5..933f712d07f 100644 --- a/plugins/modules/system/alternatives.py +++ b/plugins/modules/system/alternatives.py @@ -168,14 +168,6 @@ def mode_auto(self): def run(self): self.parse() - if self.module._diff and self.path in self.current_alternatives: - self.result['diff']['before'].update(dict( - path=self.path, - priority=self.current_alternatives[self.path].get('priority'), - link=self.current_link, - subcommands=self.current_alternatives[self.path].get('subcommands') - )) - if self.mode_present: # Check if we need to (re)install if ( @@ -252,7 +244,6 @@ def set(self): self.module.run_command(cmd, check_rc=True) if self.module._diff: - self.result['diff']['before']['state'] = AlternativeState.PRESENT self.result['diff']['after']['state'] = AlternativeState.SELECTED def auto(self): @@ -264,7 +255,6 @@ def auto(self): self.module.run_command(cmd, check_rc=True) if self.module._diff: - self.result['diff']['before']['state'] = AlternativeState.SELECTED self.result['diff']['after']['state'] = AlternativeState.PRESENT @property @@ -348,6 +338,24 @@ def parse(self): ) for name, spath in subcmd_regex.findall(subcmd) if spath != '(null)'] ) + if self.module._diff: + if self.path in self.current_alternatives: + self.result['diff']['before'].update(dict( + state=AlternativeState.PRESENT, + path=self.path, + priority=self.current_alternatives[self.path].get('priority'), + link=self.current_link, + subcommands=self.current_alternatives[self.path].get('subcommands') + )) + if self.current_mode == 'manual' and self.current_path != self.path: + self.result['diff']['before'].update(dict( + state=AlternativeState.SELECTED + )) + else: + self.result['diff']['before'].update(dict( + state=AlternativeState.ABSENT + )) + def main(): From 19350161e3316fa5b378c056d5829e5ddaa6ee27 Mon Sep 17 00:00:00 2001 From: Marius Rieder Date: Sun, 15 May 2022 10:47:36 +0200 Subject: [PATCH 23/28] alternatives: Fix linter warnings --- plugins/modules/system/alternatives.py | 29 +++++++++++++------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/plugins/modules/system/alternatives.py b/plugins/modules/system/alternatives.py index 933f712d07f..fed6a19a4e6 100644 --- a/plugins/modules/system/alternatives.py +++ b/plugins/modules/system/alternatives.py @@ -218,6 +218,7 @@ def install(self): if self.module._diff: self.result['diff']['after'] = dict( + state=AlternativeState.PRESENT, path=self.path, priority=self.priority, link=self.link, @@ -340,21 +341,21 @@ def parse(self): if self.module._diff: if self.path in self.current_alternatives: - self.result['diff']['before'].update(dict( - state=AlternativeState.PRESENT, - path=self.path, - priority=self.current_alternatives[self.path].get('priority'), - link=self.current_link, - subcommands=self.current_alternatives[self.path].get('subcommands') - )) - if self.current_mode == 'manual' and self.current_path != self.path: - self.result['diff']['before'].update(dict( - state=AlternativeState.SELECTED - )) + self.result['diff']['before'].update(dict( + state=AlternativeState.PRESENT, + path=self.path, + priority=self.current_alternatives[self.path].get('priority'), + link=self.current_link, + subcommands=self.current_alternatives[self.path].get('subcommands') + )) + if self.current_mode == 'manual' and self.current_path != self.path: + self.result['diff']['before'].update(dict( + state=AlternativeState.SELECTED + )) else: - self.result['diff']['before'].update(dict( - state=AlternativeState.ABSENT - )) + self.result['diff']['before'].update(dict( + state=AlternativeState.ABSENT + )) def main(): From b30160b315275a828487bec59dfab6ca722f76bc Mon Sep 17 00:00:00 2001 From: Marius Rieder Date: Mon, 16 May 2022 16:50:39 +0200 Subject: [PATCH 24/28] alternatives: Fix test names --- tests/integration/targets/alternatives/tasks/subcommands.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/targets/alternatives/tasks/subcommands.yml b/tests/integration/targets/alternatives/tasks/subcommands.yml index b196319366b..ba4ecbbafe7 100644 --- a/tests/integration/targets/alternatives/tasks/subcommands.yml +++ b/tests/integration/targets/alternatives/tasks/subcommands.yml @@ -32,7 +32,7 @@ that: - cmd.stdout == "dummy2" -- name: Subcommands ar not removed if not specified +- name: Subcommands are not removed if not specified alternatives: name: dummymain path: '/usr/bin/dummy1' @@ -53,7 +53,7 @@ that: - cmd.stdout == "dummy2" -- name: Subcommands ar not removed if not specified +- name: Subcommands are removed if set to an empty list alternatives: name: dummymain path: '/usr/bin/dummy1' From 8ed183678f3df23660c21706e582cc933e010191 Mon Sep 17 00:00:00 2001 From: Marius Rieder Date: Mon, 16 May 2022 16:51:41 +0200 Subject: [PATCH 25/28] alternatives: Simplify subcommands handling and improve diffs --- plugins/modules/system/alternatives.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/plugins/modules/system/alternatives.py b/plugins/modules/system/alternatives.py index fed6a19a4e6..ef825c887b1 100644 --- a/plugins/modules/system/alternatives.py +++ b/plugins/modules/system/alternatives.py @@ -206,9 +206,6 @@ def install(self): if self.subcommands is not None: subcommands = [['--slave', subcmd['link'], subcmd['name'], subcmd['path']] for subcmd in self.subcommands] cmd += [item for sublist in subcommands for item in sublist] - elif self.path in self.current_alternatives and self.current_alternatives[self.path].get('subcommands'): - subcommands = [['--slave', subcmd['link'], subcmd['name'], subcmd['path']] for subcmd in self.current_alternatives[self.path].get('subcommands')] - cmd += [item for sublist in subcommands for item in sublist] self.result['changed'] = True self.messages.append("Install alternative '%s' for '%s'." % (self.path, self.name)) @@ -222,8 +219,11 @@ def install(self): path=self.path, priority=self.priority, link=self.link, - subcommands=self.subcommands ) + if self.subcommands: + self.result['diff']['after'].update(dict( + subcommands=self.subcommands + )) def remove(self): cmd = [self.UPDATE_ALTERNATIVES, '--remove', self.name, self.path] @@ -276,7 +276,11 @@ def priority(self): @property def subcommands(self): - return self.module.params.get('subcommands') + if self.module.params.get('subcommands') is not None: + return self.module.params.get('subcommands') + elif self.path in self.current_alternatives and self.current_alternatives[self.path].get('subcommands'): + return self.current_alternatives[self.path].get('subcommands') + return None @property def UPDATE_ALTERNATIVES(self): @@ -346,8 +350,11 @@ def parse(self): path=self.path, priority=self.current_alternatives[self.path].get('priority'), link=self.current_link, - subcommands=self.current_alternatives[self.path].get('subcommands') )) + if self.current_alternatives[self.path].get('subcommands'): + self.result['diff']['before'].update(dict( + subcommands=self.current_alternatives[self.path].get('subcommands') + )) if self.current_mode == 'manual' and self.current_path != self.path: self.result['diff']['before'].update(dict( state=AlternativeState.SELECTED From 8c2fa3313de9a8ea8e356a61db150bd414b6a9bb Mon Sep 17 00:00:00 2001 From: Marius Rieder Date: Mon, 16 May 2022 23:52:38 +0200 Subject: [PATCH 26/28] aliases: Only test for subcommand changes if subcommands parameter is set. --- plugins/modules/system/alternatives.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/modules/system/alternatives.py b/plugins/modules/system/alternatives.py index ef825c887b1..3c2acb99ae0 100644 --- a/plugins/modules/system/alternatives.py +++ b/plugins/modules/system/alternatives.py @@ -173,7 +173,7 @@ def run(self): if ( self.path not in self.current_alternatives or self.current_alternatives[self.path].get('priority') != self.priority or - (self.subcommands is not None and ( + (self.module.params.get('subcommands') is not None and ( not all(s in self.subcommands for s in self.current_alternatives[self.path].get('subcommands')) or not all(s in self.current_alternatives[self.path].get('subcommands') for s in self.subcommands) )) From 069eed73c70a100f3db6d9ed7431fa5c503d5d19 Mon Sep 17 00:00:00 2001 From: Marius Rieder Date: Tue, 17 May 2022 17:59:07 +0200 Subject: [PATCH 27/28] Update plugins/modules/system/alternatives.py Co-authored-by: Felix Fontein --- plugins/modules/system/alternatives.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/plugins/modules/system/alternatives.py b/plugins/modules/system/alternatives.py index 3c2acb99ae0..0d88b1b92ea 100644 --- a/plugins/modules/system/alternatives.py +++ b/plugins/modules/system/alternatives.py @@ -170,12 +170,13 @@ def run(self): if self.mode_present: # Check if we need to (re)install + subcommands_parameter = self.module.params['subcommands'] if ( self.path not in self.current_alternatives or self.current_alternatives[self.path].get('priority') != self.priority or - (self.module.params.get('subcommands') is not None and ( - not all(s in self.subcommands for s in self.current_alternatives[self.path].get('subcommands')) or - not all(s in self.current_alternatives[self.path].get('subcommands') for s in self.subcommands) + (subcommands_parameter is not None and ( + not all(s in subcommands_parameter for s in self.current_alternatives[self.path].get('subcommands')) or + not all(s in self.current_alternatives[self.path].get('subcommands') for s in subcommands_parameter) )) ): self.install() From abd45ff08f0d990408fc7257b0fb181581b72d41 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Mon, 30 May 2022 07:25:46 +0200 Subject: [PATCH 28/28] Apply suggestions from code review --- plugins/modules/system/alternatives.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/modules/system/alternatives.py b/plugins/modules/system/alternatives.py index 0d88b1b92ea..1fbc5ccddc2 100644 --- a/plugins/modules/system/alternatives.py +++ b/plugins/modules/system/alternatives.py @@ -50,8 +50,8 @@ - C(selected) - install the alternative (if not already installed), and set it as the currently selected alternative for the group. - C(auto) - install the alternative (if not already installed), and - set the group to auto mode. Added in community.general 5.0.0. - - C(absent) - removes the alternative. Added in community.general 5.0.0. + set the group to auto mode. Added in community.general 5.1.0. + - C(absent) - removes the alternative. Added in community.general 5.1.0. choices: [ present, selected, auto, absent ] default: selected type: str @@ -78,7 +78,7 @@ description: - The path to the symbolic link that should point to the real subcommand executable. type: path - version_added: 5.0.0 + version_added: 5.1.0 requirements: [ update-alternatives ] '''