diff --git a/changelogs/fragments/2912-snap-module-helper.yml b/changelogs/fragments/2912-snap-module-helper.yml new file mode 100644 index 0000000000..cb9935a5e4 --- /dev/null +++ b/changelogs/fragments/2912-snap-module-helper.yml @@ -0,0 +1,3 @@ +bugfixes: + - module_helper module utils - avoid failing when non-zero ``rc`` is present on regular exit (https://github.com/ansible-collections/community.general/pull/2912). + - snap - fix various bugs which prevented the module from working at all, and which resulted in ``state=absent`` fail on absent snaps (https://github.com/ansible-collections/community.general/issues/2835, https://github.com/ansible-collections/community.general/issues/2906, https://github.com/ansible-collections/community.general/pull/2912). diff --git a/plugins/module_utils/mh/base.py b/plugins/module_utils/mh/base.py index e0de7f2fdd..a120c2556e 100644 --- a/plugins/module_utils/mh/base.py +++ b/plugins/module_utils/mh/base.py @@ -59,4 +59,7 @@ class ModuleHelperBase(object): self.__init_module__() self.__run__() self.__quit_module__() - self.module.exit_json(changed=self.has_changed(), **self.output) + output = self.output + if 'failed' not in output: + output['failed'] = False + self.module.exit_json(changed=self.has_changed(), **output) diff --git a/plugins/modules/packaging/os/snap.py b/plugins/modules/packaging/os/snap.py index fab2558ccf..8051b90445 100644 --- a/plugins/modules/packaging/os/snap.py +++ b/plugins/modules/packaging/os/snap.py @@ -107,6 +107,8 @@ snaps_removed: import re +from ansible.module_utils.common.text.converters import to_native + from ansible_collections.community.general.plugins.module_utils.module_helper import ( CmdStateModuleHelper, ArgFormat, ModuleHelperException ) @@ -123,7 +125,7 @@ __state_map = dict( def _state_map(value): - return __state_map[value] + return [__state_map[value]] class Snap(CmdStateModuleHelper): @@ -163,20 +165,20 @@ class Snap(CmdStateModuleHelper): results[i].append(output[i]) return [ - '; '.join(results[0]), + '; '.join([to_native(x) for x in results[0]]), self._first_non_zero(results[1]), '\n'.join(results[2]), '\n'.join(results[3]), ] def snap_exists(self, snap_name): - return 0 == self.run_command(params=[{'state': 'info'}, {'name': [snap_name]}])[0] + return 0 == self.run_command(params=[{'state': 'info'}, {'name': snap_name}])[0] def is_snap_installed(self, snap_name): - return 0 == self.run_command(params=[{'state': 'list'}, {'name': [snap_name]}])[0] + return 0 == self.run_command(params=[{'state': 'list'}, {'name': snap_name}])[0] def is_snap_enabled(self, snap_name): - rc, out, err = self.run_command(params=[{'state': 'list'}, {'name': [snap_name]}]) + rc, out, err = self.run_command(params=[{'state': 'list'}, {'name': snap_name}]) if rc != 0: return None result = out.splitlines()[1] @@ -196,7 +198,7 @@ class Snap(CmdStateModuleHelper): self.validate_input_snaps() # if snap doesnt exist, it will explode when trying to install self.vars.meta('classic').set(output=True) self.vars.meta('channel').set(output=True) - actionable_snaps = [s for s in self.vars.name if self.is_snap_installed(s)] + actionable_snaps = [s for s in self.vars.name if not self.is_snap_installed(s)] if not actionable_snaps: return self.changed = True @@ -207,9 +209,9 @@ class Snap(CmdStateModuleHelper): has_one_pkg_params = bool(self.vars.classic) or self.vars.channel != 'stable' has_multiple_snaps = len(actionable_snaps) > 1 if has_one_pkg_params and has_multiple_snaps: - commands = [params + [s] for s in actionable_snaps] + commands = [params + [{'actionable_snaps': [s]}] for s in actionable_snaps] else: - commands = [params + actionable_snaps] + commands = [params + [{'actionable_snaps': actionable_snaps}]] self.vars.cmd, rc, out, err = self._run_multiple_commands(commands) if rc == 0: return @@ -227,7 +229,7 @@ class Snap(CmdStateModuleHelper): def state_absent(self): self.validate_input_snaps() # if snap doesnt exist, it will be absent by definition - actionable_snaps = [s for s in self.vars.name if not self.is_snap_installed(s)] + actionable_snaps = [s for s in self.vars.name if self.is_snap_installed(s)] if not actionable_snaps: return self.changed = True @@ -235,7 +237,7 @@ class Snap(CmdStateModuleHelper): if self.module.check_mode: return params = ['classic', 'channel', 'state'] # get base cmd parts - commands = [params + actionable_snaps] + commands = [params + [{'actionable_snaps': actionable_snaps}]] self.vars.cmd, rc, out, err = self._run_multiple_commands(commands) if rc == 0: return @@ -253,7 +255,7 @@ class Snap(CmdStateModuleHelper): if self.module.check_mode: return params = ['classic', 'channel', 'state'] # get base cmd parts - commands = [params + actionable_snaps] + commands = [params + [{'actionable_snaps': actionable_snaps}]] self.vars.cmd, rc, out, err = self._run_multiple_commands(commands) if rc == 0: return @@ -271,7 +273,7 @@ class Snap(CmdStateModuleHelper): if self.module.check_mode: return params = ['classic', 'channel', 'state'] # get base cmd parts - commands = [params + actionable_snaps] + commands = [params + [{'actionable_snaps': actionable_snaps}]] self.vars.cmd, rc, out, err = self._run_multiple_commands(commands) if rc == 0: return diff --git a/tests/integration/targets/snap/aliases b/tests/integration/targets/snap/aliases index d7f5ce60c5..ee303bf346 100644 --- a/tests/integration/targets/snap/aliases +++ b/tests/integration/targets/snap/aliases @@ -3,4 +3,4 @@ skip/aix skip/freebsd skip/osx skip/macos -disabled #FIXME 2609 +skip/docker diff --git a/tests/integration/targets/snap/defaults/main.yml b/tests/integration/targets/snap/defaults/main.yml new file mode 100644 index 0000000000..2290001f7e --- /dev/null +++ b/tests/integration/targets/snap/defaults/main.yml @@ -0,0 +1,4 @@ +has_snap: false + +snap_packages: + - snapd diff --git a/tests/integration/targets/snap/handlers/main.yml b/tests/integration/targets/snap/handlers/main.yml new file mode 100644 index 0000000000..a80cc98e49 --- /dev/null +++ b/tests/integration/targets/snap/handlers/main.yml @@ -0,0 +1,5 @@ +--- +- name: Remove snapd + package: + name: "{{ snap_packages }}" + state: absent diff --git a/tests/integration/targets/snap/meta/main.yml b/tests/integration/targets/snap/meta/main.yml new file mode 100644 index 0000000000..0e51c36ebd --- /dev/null +++ b/tests/integration/targets/snap/meta/main.yml @@ -0,0 +1,3 @@ +dependencies: + - setup_pkg_mgr + - setup_epel diff --git a/tests/integration/targets/snap/tasks/Debian.yml b/tests/integration/targets/snap/tasks/Debian.yml new file mode 120000 index 0000000000..0abaec1677 --- /dev/null +++ b/tests/integration/targets/snap/tasks/Debian.yml @@ -0,0 +1 @@ +default.yml \ No newline at end of file diff --git a/tests/integration/targets/snap/tasks/Fedora.yml b/tests/integration/targets/snap/tasks/Fedora.yml new file mode 120000 index 0000000000..0abaec1677 --- /dev/null +++ b/tests/integration/targets/snap/tasks/Fedora.yml @@ -0,0 +1 @@ +default.yml \ No newline at end of file diff --git a/tests/integration/targets/snap/tasks/RedHat.yml b/tests/integration/targets/snap/tasks/RedHat.yml new file mode 120000 index 0000000000..0abaec1677 --- /dev/null +++ b/tests/integration/targets/snap/tasks/RedHat.yml @@ -0,0 +1 @@ +default.yml \ No newline at end of file diff --git a/tests/integration/targets/snap/tasks/default.yml b/tests/integration/targets/snap/tasks/default.yml new file mode 100644 index 0000000000..4cc38f7bf2 --- /dev/null +++ b/tests/integration/targets/snap/tasks/default.yml @@ -0,0 +1,15 @@ +--- +- name: Install snapd + package: + name: "{{ snap_packages }}" + state: present + notify: Remove snapd + +- name: Make sure that snapd is running + service: + name: snapd + state: started + +- name: Inform that snap is installed + set_fact: + has_snap: true diff --git a/tests/integration/targets/snap/tasks/main.yml b/tests/integration/targets/snap/tasks/main.yml index e015122ff2..73604d3895 100644 --- a/tests/integration/targets/snap/tasks/main.yml +++ b/tests/integration/targets/snap/tasks/main.yml @@ -4,28 +4,46 @@ # and should not be used as examples of how to write Ansible roles # #################################################################### -- name: install snapd - apt: - name: snapd - state: present - register: snapd_install_ubuntu - when: ansible_distribution == 'Ubuntu' - -- name: install snapd - dnf: - name: snapd - state: present - register: snapd_install_fedora - when: ansible_distribution == 'Fedora' +- name: Include distribution specific tasks + include_tasks: "{{ lookup('first_found', params) }}" + vars: + params: + files: + - "{{ ansible_facts.distribution }}-{{ ansible_facts.distribution_major_version }}.yml" + - "{{ ansible_facts.os_family }}-{{ ansible_facts.distribution_major_version }}.yml" + - "{{ ansible_facts.distribution }}.yml" + - "{{ ansible_facts.os_family }}.yml" + - "nothing.yml" + paths: + - "{{ role_path }}/tasks" - block: - - name: install package + - name: Make sure package is not installed + community.general.snap: + name: hello-world + state: absent + + - name: Install package (check mode) + community.general.snap: + name: hello-world + state: present + register: install_check + check_mode: true + + - name: Install package community.general.snap: name: hello-world state: present register: install - - name: install package again + - name: Install package again (check mode) + community.general.snap: + name: hello-world + state: present + register: install_again_check + check_mode: true + + - name: Install package again community.general.snap: name: hello-world state: present @@ -35,18 +53,36 @@ assert: that: - install is changed + - install_check is changed - install_again is not changed + - install_again_check is not changed - - name: check package has been installed correctly + - name: Check package has been installed correctly command: hello-world + environment: + PATH: /var/lib/snapd/snap/bin/ - - name: remove package + - name: Remove package (check mode) + community.general.snap: + name: hello-world + state: absent + register: remove_check + check_mode: true + + - name: Remove package community.general.snap: name: hello-world state: absent register: remove - - name: remove package again + - name: Remove package again (check mode) + community.general.snap: + name: hello-world + state: absent + register: remove_again_check + check_mode: true + + - name: Remove package again community.general.snap: name: hello-world state: absent @@ -56,17 +92,7 @@ assert: that: - remove is changed + - remove_check is changed - remove_again is not changed - when: ansible_distribution in ['Ubuntu','Fedora'] - -- name: Remove snapd in case it was not installed - apt: - name: snapd - state: absent - when: snapd_install_ubuntu is changed and ansible_distribution == 'Ubuntu' - -- name: Remove snapd in case it was not installed - dnf: - name: snapd - state: absent - when: snapd_install_fedora is changed and ansible_distribution == 'Fedora' + - remove_again_check is not changed + when: has_snap diff --git a/tests/integration/targets/snap/tasks/nothing.yml b/tests/integration/targets/snap/tasks/nothing.yml new file mode 100644 index 0000000000..11642d1fcd --- /dev/null +++ b/tests/integration/targets/snap/tasks/nothing.yml @@ -0,0 +1,2 @@ +--- +# Do nothing