From 4275bfe87bb73bfbe470c32c39c6fbe3f96c592f Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Mon, 13 Jun 2022 21:51:08 +0200 Subject: [PATCH] alternatives: Fix bug with priority default (#4810) (#4835) * alternatives: Fix bug with priority default If neigther the priority nor the subcommands where specified the module decided to update the priority with the default value anyway. This resulted in bug #4803 and #4804 * Add changelog fragment. * Distinguish None from 0. * Address review comments. * Update plugins/modules/system/alternatives.py Co-authored-by: Pilou * Remove unrelated issues from changelog. Co-authored-by: Felix Fontein Co-authored-by: Pilou (cherry picked from commit 57e83ac80b727c472d0c3911d9189ae667feb110) Co-authored-by: Marius Rieder --- changelogs/fragments/4810-alternatives-bug.yml | 2 ++ plugins/modules/system/alternatives.py | 12 +++++++----- .../targets/alternatives/tasks/test.yml | 2 +- .../alternatives/tasks/tests_set_priority.yml | 17 ++++++++++++++++- 4 files changed, 26 insertions(+), 7 deletions(-) create mode 100644 changelogs/fragments/4810-alternatives-bug.yml diff --git a/changelogs/fragments/4810-alternatives-bug.yml b/changelogs/fragments/4810-alternatives-bug.yml new file mode 100644 index 0000000000..d4c1ea2742 --- /dev/null +++ b/changelogs/fragments/4810-alternatives-bug.yml @@ -0,0 +1,2 @@ +bugfixes: + - "alternatives - do not set the priority if the priority was not set by the user (https://github.com/ansible-collections/community.general/pull/4810)." diff --git a/plugins/modules/system/alternatives.py b/plugins/modules/system/alternatives.py index 1fbc5ccddc..56ae57fe8b 100644 --- a/plugins/modules/system/alternatives.py +++ b/plugins/modules/system/alternatives.py @@ -40,9 +40,8 @@ options: type: path priority: description: - - The priority of the alternative. + - The priority of the alternative. If no priority is given for creation C(50) is used as a fallback. type: int - default: 50 state: description: - C(present) - install the alternative (if not already installed), but do @@ -171,9 +170,10 @@ class AlternativesModule(object): if self.mode_present: # Check if we need to (re)install subcommands_parameter = self.module.params['subcommands'] + priority_parameter = self.module.params['priority'] if ( self.path not in self.current_alternatives or - self.current_alternatives[self.path].get('priority') != self.priority or + (priority_parameter is not None and self.current_alternatives[self.path].get('priority') != priority_parameter) or (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) @@ -273,7 +273,9 @@ class AlternativesModule(object): @property def priority(self): - return self.module.params.get('priority') + if self.module.params.get('priority') is not None: + return self.module.params.get('priority') + return self.current_alternatives.get(self.path, {}).get('priority', 50) @property def subcommands(self): @@ -373,7 +375,7 @@ def main(): name=dict(type='str', required=True), path=dict(type='path', required=True), link=dict(type='path'), - priority=dict(type='int', default=50), + priority=dict(type='int'), state=dict( type='str', choices=AlternativeState.to_list(), diff --git a/tests/integration/targets/alternatives/tasks/test.yml b/tests/integration/targets/alternatives/tasks/test.yml index a5b36ce922..a4d2f95502 100644 --- a/tests/integration/targets/alternatives/tasks/test.yml +++ b/tests/integration/targets/alternatives/tasks/test.yml @@ -48,4 +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 }}\\n50' '{{ alternatives_dir }}/dummy'" + command: "grep -Pzq '/bin/dummy{{ item }}\\n' '{{ 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 a629e3f368..12dcc1cbf6 100644 --- a/tests/integration/targets/alternatives/tasks/tests_set_priority.yml +++ b/tests/integration/targets/alternatives/tasks/tests_set_priority.yml @@ -31,4 +31,19 @@ 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 + command: "grep -Pzq '/bin/dummy{{ item }}\\n{{ 70 + item|int }}' '{{ alternatives_dir }}/dummy'" + +- name: no change without priority + alternatives: + name: dummy + path: '/usr/bin/dummy{{ item }}' + link: /usr/bin/dummy + register: alternative + +- name: check no change was triggered without priority + assert: + that: + - 'alternative is not changed' + +- name: check that alternative priority has not been changed + command: "grep -Pzq '/bin/dummy{{ item }}\\n{{ 70 + item|int }}' '{{ alternatives_dir }}/dummy'"