Skip to content

Commit

Permalink
alternatives: Fix bug with priority default (#4810) (#4835)
Browse files Browse the repository at this point in the history
* 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 <pierre-louis@libregerbil.fr>

* Remove unrelated issues from changelog.

Co-authored-by: Felix Fontein <felix@fontein.de>
Co-authored-by: Pilou <pierre-louis@libregerbil.fr>
(cherry picked from commit 57e83ac)

Co-authored-by: Marius Rieder <marius.rieder@durchmesser.ch>
  • Loading branch information
patchback[bot] and jiuka authored Jun 13, 2022
1 parent 2f87b8c commit 4275bfe
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 7 deletions.
2 changes: 2 additions & 0 deletions changelogs/fragments/4810-alternatives-bug.yml
Original file line number Diff line number Diff line change
@@ -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)."
12 changes: 7 additions & 5 deletions plugins/modules/system/alternatives.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,8 @@
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
Expand Down Expand Up @@ -171,9 +170,10 @@ def run(self):
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)
Expand Down Expand Up @@ -273,7 +273,9 @@ def link(self):

@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):
Expand Down Expand Up @@ -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(),
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/targets/alternatives/tasks/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'"
Original file line number Diff line number Diff line change
Expand Up @@ -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'"
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'"

0 comments on commit 4275bfe

Please sign in to comment.