From c873cfe646340e3304f1bcb407a5296f1b42cbec Mon Sep 17 00:00:00 2001 From: Alexei Znamensky Date: Thu, 27 Apr 2023 11:55:50 +1200 Subject: [PATCH 1/9] [WIP] snap: aware of channel in installed snaps --- plugins/modules/snap.py | 8 +- tests/integration/targets/snap/tasks/main.yml | 239 +--------------- tests/integration/targets/snap/tasks/test.yml | 262 ++++++++++++++++++ 3 files changed, 270 insertions(+), 239 deletions(-) create mode 100644 tests/integration/targets/snap/tasks/test.yml diff --git a/plugins/modules/snap.py b/plugins/modules/snap.py index 359b7fa323f..779fa61d12c 100644 --- a/plugins/modules/snap.py +++ b/plugins/modules/snap.py @@ -52,9 +52,9 @@ description: - Define which release of a snap is installed and tracked for updates. This option can only be specified if there is a single snap in the task. + - If not passed, the C(snap) command will default to I(stable). type: str required: false - default: stable options: description: - Set options with pattern C(key=value) or C(snap:key=value). If a snap name is given, the option will be applied @@ -167,7 +167,7 @@ class Snap(StateModuleHelper): 'state': dict(type='str', default='present', choices=['absent', 'present', 'enabled', 'disabled']), 'classic': dict(type='bool', default=False), - 'channel': dict(type='str', default='stable'), + 'channel': dict(type='str'), 'options': dict(type='list', elements='str'), }, supports_check_mode=True, @@ -212,6 +212,10 @@ def _run_multiple_commands(self, commands, actionable_names, bundle=True): '\n'.join(results_err), ] + def __quit_module__(self): + if self.vars.channel is None: + self.vars.channel = "stable" + def convert_json_subtree_to_map(self, json_subtree, prefix=None): option_map = {} diff --git a/tests/integration/targets/snap/tasks/main.yml b/tests/integration/targets/snap/tasks/main.yml index 0f24e69f3db..aa1f8f2f32a 100644 --- a/tests/integration/targets/snap/tasks/main.yml +++ b/tests/integration/targets/snap/tasks/main.yml @@ -1,243 +1,8 @@ --- -#################################################################### -# WARNING: These are designed specifically for Ansible tests # -# and should not be used as examples of how to write Ansible roles # -#################################################################### - # Copyright (c) Ansible Project # GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) # SPDX-License-Identifier: GPL-3.0-or-later -- name: Has-snap block +- name: Has-snap include + ansible.builtin.include_tasks: test.yml when: has_snap - block: - - name: Make sure package is not installed (hello-world) - community.general.snap: - name: hello-world - state: absent - - - name: Install package (hello-world) (check mode) - community.general.snap: - name: hello-world - state: present - register: install_check - check_mode: true - - - name: Install package (hello-world) - community.general.snap: - name: hello-world - state: present - register: install - - - name: Install package again (hello-world) (check mode) - community.general.snap: - name: hello-world - state: present - register: install_again_check - check_mode: true - - - name: Install package again (hello-world) - community.general.snap: - name: hello-world - state: present - register: install_again - - - name: Assert package has been installed just once (hello-world) - 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 (hello-world) - command: hello-world - environment: - PATH: /snap/bin/ - - - name: Remove package (hello-world) (check mode) - community.general.snap: - name: hello-world - state: absent - register: remove_check - check_mode: true - - - name: Remove package (hello-world) - community.general.snap: - name: hello-world - state: absent - register: remove - - - name: Remove package again (hello-world) (check mode) - community.general.snap: - name: hello-world - state: absent - register: remove_again_check - check_mode: true - - - name: Remove package again (hello-world) - community.general.snap: - name: hello-world - state: absent - register: remove_again - - - name: Assert package has been removed just once (hello-world) - assert: - that: - - remove is changed - - remove_check is changed - - remove_again is not changed - - remove_again_check is not changed - - - name: Make sure package from classic snap is not installed (nvim) - community.general.snap: - name: nvim - state: absent - - - name: Install package from classic snap (nvim) - community.general.snap: - name: nvim - state: present - classic: true - register: classic_install - - # testing classic idempotency - - name: Install package from classic snap again (nvim) - community.general.snap: - name: nvim - state: present - classic: true - register: classic_install_again - - - name: Assert package has been installed just once (nvim) - assert: - that: - - classic_install is changed - - classic_install_again is not changed - - # this is just testing if a package which has been installed - # with true classic can be removed without setting classic to true - - name: Remove package from classic snap without setting classic to true (nvim) - community.general.snap: - name: nvim - state: absent - register: classic_remove_without_true_classic - - - name: Remove package from classic snap with setting classic to true (nvim) - community.general.snap: - name: nvim - state: absent - classic: true - register: classic_remove_with_true_classic - - - name: Assert package has been removed without setting classic to true (nvim) - assert: - that: - - classic_remove_without_true_classic is changed - - classic_remove_with_true_classic is not changed - - - - name: Make sure package is not installed (uhttpd) - community.general.snap: - name: uhttpd - state: absent - - - name: Install package (uhttpd) - community.general.snap: - name: uhttpd - state: present - register: install - - - name: Install package (uhttpd) - community.general.snap: - name: uhttpd - state: present - options: - - "listening-port=8080" - register: install_with_option - - - name: Install package again with option (uhttpd) - community.general.snap: - name: uhttpd - state: present - options: - - "listening-port=8080" - register: install_with_option_again - - - name: Install package again with different options (uhttpd) - community.general.snap: - name: uhttpd - state: present - options: - - "listening-port=8088" - - "document-root-dir=/tmp" - register: install_with_option_changed - - - name: Remove package (uhttpd) - community.general.snap: - name: uhttpd - state: absent - register: remove - - - name: Assert package has been installed with options just once and only changed options trigger a change (uhttpd) - assert: - that: - - install is changed - - install_with_option is changed - - "install_with_option.options_changed[0] == 'uhttpd:listening-port=8080'" - - install_with_option_again is not changed - - install_with_option_changed is changed - - "'uhttpd:listening-port=8088' in install_with_option_changed.options_changed" - - "'uhttpd:document-root-dir=/tmp' in install_with_option_changed.options_changed" - - "'uhttpd:listening-port=8080' not in install_with_option_changed.options_changed" - - remove is changed - - - name: Install two packages at the same time - community.general.snap: - name: - - hello-world - - uhttpd - state: present - register: install_two - - - name: Install two packages at the same time (again) - community.general.snap: - name: - - hello-world - - uhttpd - state: present - register: install_two_again - - - name: Remove packages (hello-world & uhttpd) - community.general.snap: - name: - - hello-world - - uhttpd - state: absent - register: install_two_remove - - - name: Remove packages again (hello-world & uhttpd) - community.general.snap: - name: - - hello-world - - uhttpd - state: absent - register: install_two_remove_again - - - name: Assert installation of two packages - assert: - that: - - install_two is changed - - "'hello-world' in install_two.snaps_installed" - - "'uhttpd' in install_two.snaps_installed" - - install_two.snaps_removed is not defined - - install_two_again is not changed - - install_two_again.snaps_installed is not defined - - install_two_again.snaps_removed is not defined - - install_two_remove is changed - - install_two_again.snaps_installed is not defined - - "'hello-world' in install_two_remove.snaps_removed" - - "'uhttpd' in install_two_remove.snaps_removed" - - install_two_remove_again is not changed - - install_two_remove_again.snaps_installed is not defined - - install_two_remove_again.snaps_removed is not defined diff --git a/tests/integration/targets/snap/tasks/test.yml b/tests/integration/targets/snap/tasks/test.yml new file mode 100644 index 00000000000..bce48ebd803 --- /dev/null +++ b/tests/integration/targets/snap/tasks/test.yml @@ -0,0 +1,262 @@ +--- +# Copyright (c) Ansible Project +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +- name: Make sure package is not installed (hello-world) + community.general.snap: + name: hello-world + state: absent + +- name: Install package (hello-world) (check mode) + community.general.snap: + name: hello-world + state: present + register: install_check + check_mode: true + +- name: Install package (hello-world) + community.general.snap: + name: hello-world + state: present + register: install + +- name: Install package again (hello-world) (check mode) + community.general.snap: + name: hello-world + state: present + register: install_again_check + check_mode: true + +- name: Install package again (hello-world) + community.general.snap: + name: hello-world + state: present + register: install_again + +- name: Assert package has been installed just once (hello-world) + 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 (hello-world) + command: hello-world + environment: + PATH: /snap/bin/ + +- name: Remove package (hello-world) (check mode) + community.general.snap: + name: hello-world + state: absent + register: remove_check + check_mode: true + +- name: Remove package (hello-world) + community.general.snap: + name: hello-world + state: absent + register: remove + +- name: Remove package again (hello-world) (check mode) + community.general.snap: + name: hello-world + state: absent + register: remove_again_check + check_mode: true + +- name: Remove package again (hello-world) + community.general.snap: + name: hello-world + state: absent + register: remove_again + +- name: Assert package has been removed just once (hello-world) + assert: + that: + - remove is changed + - remove_check is changed + - remove_again is not changed + - remove_again_check is not changed + +- name: Make sure package from classic snap is not installed (nvim) + community.general.snap: + name: nvim + state: absent + +- name: Install package from classic snap (nvim) + community.general.snap: + name: nvim + state: present + classic: true + register: classic_install + +# testing classic idempotency +- name: Install package from classic snap again (nvim) + community.general.snap: + name: nvim + state: present + classic: true + register: classic_install_again + +- name: Assert package has been installed just once (nvim) + assert: + that: + - classic_install is changed + - classic_install_again is not changed + +# this is just testing if a package which has been installed +# with true classic can be removed without setting classic to true +- name: Remove package from classic snap without setting classic to true (nvim) + community.general.snap: + name: nvim + state: absent + register: classic_remove_without_true_classic + +- name: Remove package from classic snap with setting classic to true (nvim) + community.general.snap: + name: nvim + state: absent + classic: true + register: classic_remove_with_true_classic + +- name: Assert package has been removed without setting classic to true (nvim) + assert: + that: + - classic_remove_without_true_classic is changed + - classic_remove_with_true_classic is not changed + + +- name: Make sure package is not installed (uhttpd) + community.general.snap: + name: uhttpd + state: absent + +- name: Install package (uhttpd) + community.general.snap: + name: uhttpd + state: present + register: install + +- name: Install package (uhttpd) + community.general.snap: + name: uhttpd + state: present + options: + - "listening-port=8080" + register: install_with_option + +- name: Install package again with option (uhttpd) + community.general.snap: + name: uhttpd + state: present + options: + - "listening-port=8080" + register: install_with_option_again + +- name: Install package again with different options (uhttpd) + community.general.snap: + name: uhttpd + state: present + options: + - "listening-port=8088" + - "document-root-dir=/tmp" + register: install_with_option_changed + +- name: Remove package (uhttpd) + community.general.snap: + name: uhttpd + state: absent + register: remove + +- name: Assert package has been installed with options just once and only changed options trigger a change (uhttpd) + assert: + that: + - install is changed + - install_with_option is changed + - "install_with_option.options_changed[0] == 'uhttpd:listening-port=8080'" + - install_with_option_again is not changed + - install_with_option_changed is changed + - "'uhttpd:listening-port=8088' in install_with_option_changed.options_changed" + - "'uhttpd:document-root-dir=/tmp' in install_with_option_changed.options_changed" + - "'uhttpd:listening-port=8080' not in install_with_option_changed.options_changed" + - remove is changed + +- name: Install two packages at the same time + community.general.snap: + name: + - hello-world + - uhttpd + state: present + register: install_two + +- name: Install two packages at the same time (again) + community.general.snap: + name: + - hello-world + - uhttpd + state: present + register: install_two_again + +- name: Remove packages (hello-world & uhttpd) + community.general.snap: + name: + - hello-world + - uhttpd + state: absent + register: install_two_remove + +- name: Remove packages again (hello-world & uhttpd) + community.general.snap: + name: + - hello-world + - uhttpd + state: absent + register: install_two_remove_again + +- name: Assert installation of two packages + assert: + that: + - install_two is changed + - "'hello-world' in install_two.snaps_installed" + - "'uhttpd' in install_two.snaps_installed" + - install_two.snaps_removed is not defined + - install_two_again is not changed + - install_two_again.snaps_installed is not defined + - install_two_again.snaps_removed is not defined + - install_two_remove is changed + - install_two_again.snaps_installed is not defined + - "'hello-world' in install_two_remove.snaps_removed" + - "'uhttpd' in install_two_remove.snaps_removed" + - install_two_remove_again is not changed + - install_two_remove_again.snaps_installed is not defined + - install_two_remove_again.snaps_removed is not defined + +- name: Install package (microk8s) + community.general.snap: + name: microk8s + classic: true + state: present + register: install_microk8s + +- name: Install package with channel (microk8s) + community.general.snap: + name: microk8s + classic: true + channel: 1.20/stable + state: present + register: install_microk8s_chan + +- name: Remove package (microk8s) + community.general.snap: + name: microk8s + state: absent + register: remove_microk8s + +- assert: + that: + - install_microk8s is changed + - install_microk8s_chan is changed + - remove_microk8s is changed From c37e3089706cd943fe0569f479d58631a2777ab4 Mon Sep 17 00:00:00 2001 From: Alexei Znamensky Date: Thu, 27 Apr 2023 14:13:59 +1200 Subject: [PATCH 2/9] parse snap list output and assert whether channel matches --- plugins/modules/snap.py | 27 ++++++++++++------- tests/integration/targets/snap/tasks/test.yml | 1 + 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/plugins/modules/snap.py b/plugins/modules/snap.py index 779fa61d12c..2ee4d8484ec 100644 --- a/plugins/modules/snap.py +++ b/plugins/modules/snap.py @@ -161,11 +161,11 @@ class Snap(StateModuleHelper): __disable_re = re.compile(r'(?:\S+\s+){5}(?P\S+)') __set_param_re = re.compile(r'(?P\S+:)?(?P\S+)\s*=\s*(?P.+)') + __list_re = re.compile(r'^(?P\S+)\s+\S+\s+\S+\s+(?P\S+)') module = dict( argument_spec={ 'name': dict(type='list', elements='str', required=True), - 'state': dict(type='str', default='present', - choices=['absent', 'present', 'enabled', 'disabled']), + 'state': dict(type='str', default='present', choices=['absent', 'present', 'enabled', 'disabled']), 'classic': dict(type='bool', default=False), 'channel': dict(type='str'), 'options': dict(type='list', elements='str'), @@ -259,9 +259,18 @@ def retrieve_option_map(self, snap_name): return option_map - def is_snap_installed(self, snap_name): - rc, dummy, dummy = self.runner("_list name").run(name=snap_name) - return rc == 0 + def is_snap_installed(self, snap_name, channel): + with self.runner("_list name") as ctx: + rc, out, err = ctx.run(name=snap_name) + if rc != 0: + return False + out = out.split('\n')[1:] + out = [self.__list_re.match(x) for x in out] + out = [(m.group('name'), m.group('channel')) for m in out if m] + if channel is None: + return any(snap_name == m[0] for m in out) + else: + return any((snap_name, channel) == m for m in out) def is_snap_enabled(self, snap_name): with self.runner("_list name") as ctx: @@ -276,7 +285,7 @@ def is_snap_enabled(self, snap_name): return "disabled" not in notes.split(',') def process_actionable_snaps(self, actionable_snaps): - self.changed = True + self.changed = False self.vars.snaps_installed = actionable_snaps if self.check_mode: @@ -309,7 +318,7 @@ def state_present(self): self.vars.meta('classic').set(output=True) self.vars.meta('channel').set(output=True) - 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 not self.is_snap_installed(s, self.vars.channel)] if actionable_snaps: self.process_actionable_snaps(actionable_snaps) @@ -320,7 +329,7 @@ def set_options(self): if self.vars.options is None: return - 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 self.is_snap_installed(s, self.vars.channel)] overall_options_changed = [] for snap_name in actionable_snaps: @@ -388,7 +397,7 @@ def _generic_state_action(self, actionable_func, actionable_var, params=None): self.do_raise(msg=msg) def state_absent(self): - self._generic_state_action(self.is_snap_installed, "snaps_removed", ['classic', 'channel', 'state']) + self._generic_state_action(lambda s: self.is_snap_installed(s, self.vars.channel), "snaps_removed", ['classic', 'channel', 'state']) def state_enabled(self): self._generic_state_action(lambda s: not self.is_snap_enabled(s), "snaps_enabled", ['classic', 'channel', 'state']) diff --git a/tests/integration/targets/snap/tasks/test.yml b/tests/integration/targets/snap/tasks/test.yml index bce48ebd803..c643200da64 100644 --- a/tests/integration/targets/snap/tasks/test.yml +++ b/tests/integration/targets/snap/tasks/test.yml @@ -234,6 +234,7 @@ - install_two_remove_again.snaps_installed is not defined - install_two_remove_again.snaps_removed is not defined +# Test for https://github.com/ansible-collections/community.general/issues/1606 - name: Install package (microk8s) community.general.snap: name: microk8s From 393403ed27069c16314ca08aad14fb25450eae1e Mon Sep 17 00:00:00 2001 From: Alexei Znamensky Date: Thu, 27 Apr 2023 14:14:45 +1200 Subject: [PATCH 3/9] undo test --- plugins/modules/snap.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/modules/snap.py b/plugins/modules/snap.py index 2ee4d8484ec..b47b20065df 100644 --- a/plugins/modules/snap.py +++ b/plugins/modules/snap.py @@ -285,7 +285,7 @@ def is_snap_enabled(self, snap_name): return "disabled" not in notes.split(',') def process_actionable_snaps(self, actionable_snaps): - self.changed = False + self.changed = True self.vars.snaps_installed = actionable_snaps if self.check_mode: From 3ae95a351dd4d64fe509b965c0a1f05650c15308 Mon Sep 17 00:00:00 2001 From: Alexei Znamensky Date: Thu, 27 Apr 2023 15:53:49 +1200 Subject: [PATCH 4/9] fail rightfully when install with different channel does not work --- plugins/modules/snap.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/plugins/modules/snap.py b/plugins/modules/snap.py index b47b20065df..77880a4d73a 100644 --- a/plugins/modules/snap.py +++ b/plugins/modules/snap.py @@ -162,6 +162,7 @@ class Snap(StateModuleHelper): __disable_re = re.compile(r'(?:\S+\s+){5}(?P\S+)') __set_param_re = re.compile(r'(?P\S+:)?(?P\S+)\s*=\s*(?P.+)') __list_re = re.compile(r'^(?P\S+)\s+\S+\s+\S+\s+(?P\S+)') + __install_re = re.compile(r'(?P\S+)\s.+\sinstalled') module = dict( argument_spec={ 'name': dict(type='list', elements='str', required=True), @@ -301,7 +302,10 @@ def process_actionable_snaps(self, actionable_snaps): self.vars.cmd, rc, out, err = self._run_multiple_commands(params, actionable_snaps) if rc == 0: - return + match_install = [self.__install_re.match(line) for line in out.split('\n')] + match_install = [m.group('name') in actionable_snaps for m in match_install if m] + if len(match_install) == len(commands): + return classic_snap_pattern = re.compile(r'^error: This revision of snap "(?P\w+)"' r' was published using classic confinement') From 09465db0fe47855f305315687789ad6cb6760323 Mon Sep 17 00:00:00 2001 From: Alexei Znamensky Date: Mon, 5 Jun 2023 11:17:42 +1200 Subject: [PATCH 5/9] transparetent refresh --- plugins/module_utils/snap.py | 1 + plugins/modules/snap.py | 83 ++++++++++++------- tests/integration/targets/snap/tasks/main.yml | 6 +- tests/integration/targets/snap/tasks/test.yml | 28 ------- .../targets/snap/tasks/test_channel.yml | 46 ++++++++++ 5 files changed, 106 insertions(+), 58 deletions(-) create mode 100644 tests/integration/targets/snap/tasks/test_channel.yml diff --git a/plugins/module_utils/snap.py b/plugins/module_utils/snap.py index 3ae126dbfd4..1b3bdf2fe51 100644 --- a/plugins/module_utils/snap.py +++ b/plugins/module_utils/snap.py @@ -20,6 +20,7 @@ absent='remove', enabled='enable', disabled='disable', + refresh='refresh', ) diff --git a/plugins/modules/snap.py b/plugins/modules/snap.py index 77880a4d73a..4dc245874e2 100644 --- a/plugins/modules/snap.py +++ b/plugins/modules/snap.py @@ -35,7 +35,9 @@ state: description: - Desired state of the package. - required: false + - > + When I(state=present) the module will use C(snap install) if the snap is not installed, + and C(snap refresh) if it is installed but from a different channel. default: present choices: [ absent, present, enabled, disabled ] type: str @@ -159,10 +161,14 @@ class Snap(StateModuleHelper): + NOT_INSTALLED = 0 + CHANNEL_MISMATCH = 1 + INSTALLED = 2 + __disable_re = re.compile(r'(?:\S+\s+){5}(?P\S+)') __set_param_re = re.compile(r'(?P\S+:)?(?P\S+)\s*=\s*(?P.+)') __list_re = re.compile(r'^(?P\S+)\s+\S+\s+\S+\s+(?P\S+)') - __install_re = re.compile(r'(?P\S+)\s.+\sinstalled') + __install_re = re.compile(r'(?P\S+)\s.+\s(installed|refreshed)') module = dict( argument_spec={ 'name': dict(type='list', elements='str', required=True), @@ -184,33 +190,41 @@ def _first_non_zero(a): def __init_module__(self): self.runner = snap_runner(self.module) + self.vars.set("snap_status", self.snap_status(self.vars.name, self.vars.channel), output=False) + self.vars.set("snap_status_map", dict(zip(self.vars.name, self.vars.snap_status)), output=False) - def _run_multiple_commands(self, commands, actionable_names, bundle=True): + def _run_multiple_commands(self, commands, actionable_names, bundle=True, refresh=False): results_cmd = [] results_rc = [] results_out = [] results_err = [] + results_run_info = [] + + state = "refresh" if refresh else self.vars.state with self.runner(commands + ["name"]) as ctx: if bundle: - rc, out, err = ctx.run(name=actionable_names) + rc, out, err = ctx.run(state=state, name=actionable_names) results_cmd.append(commands + actionable_names) results_rc.append(rc) results_out.append(out) results_err.append(err) + results_run_info.append(ctx.run_info) else: for name in actionable_names: - rc, out, err = ctx.run(name=name) + rc, out, err = ctx.run(state=state, name=name) results_cmd.append(commands + [name]) results_rc.append(rc) results_out.append(out) results_err.append(err) + results_run_info.append(ctx.run_info) return [ '; '.join([to_native(x) for x in results_cmd]), self._first_non_zero(results_rc), '\n'.join(results_out), '\n'.join(results_err), + results_run_info, ] def __quit_module__(self): @@ -229,7 +243,6 @@ def convert_json_subtree_to_map(self, json_subtree, prefix=None): if isinstance(value, (str, float, bool, numbers.Integral)): option_map[full_key] = str(value) - else: option_map.update(self.convert_json_subtree_to_map(json_subtree=value, prefix=full_key)) @@ -253,25 +266,32 @@ def retrieve_option_map(self, snap_name): try: option_map = self.convert_json_to_map(out) - except Exception as e: self.do_raise( msg="Parsing option map returned by 'snap get {0}' triggers exception '{1}', output:\n'{2}'".format(snap_name, str(e), out)) return option_map - def is_snap_installed(self, snap_name, channel): - with self.runner("_list name") as ctx: - rc, out, err = ctx.run(name=snap_name) - if rc != 0: - return False + def snap_status(self, snap_name, channel): + def _status_check(name, channel, installed): + match = [c for n, c in installed if n == name] + if not match: + return Snap.NOT_INSTALLED + if channel and channel != match[0]: + return Snap.CHANNEL_MISMATCH + else: + return Snap.INSTALLED + + with self.runner("_list") as ctx: + rc, out, err = ctx.run(check_rc=True) out = out.split('\n')[1:] out = [self.__list_re.match(x) for x in out] out = [(m.group('name'), m.group('channel')) for m in out if m] - if channel is None: - return any(snap_name == m[0] for m in out) - else: - return any((snap_name, channel) == m for m in out) + if self.verbosity >= 4: + self.vars.status_out = out + self.vars.status_run_info = ctx.run_info + + return [_status_check(n, channel, out) for n in snap_name] def is_snap_enabled(self, snap_name): with self.runner("_list name") as ctx: @@ -285,7 +305,7 @@ def is_snap_enabled(self, snap_name): notes = match.group('notes') return "disabled" not in notes.split(',') - def process_actionable_snaps(self, actionable_snaps): + def _present(self, actionable_snaps, refresh=False): self.changed = True self.vars.snaps_installed = actionable_snaps @@ -297,14 +317,16 @@ def process_actionable_snaps(self, actionable_snaps): has_multiple_snaps = len(actionable_snaps) > 1 if has_one_pkg_params and has_multiple_snaps: - self.vars.cmd, rc, out, err = self._run_multiple_commands(params, actionable_snaps, bundle=False) + self.vars.cmd, rc, out, err, run_info = self._run_multiple_commands(params, actionable_snaps, bundle=False, refresh=refresh) else: - self.vars.cmd, rc, out, err = self._run_multiple_commands(params, actionable_snaps) + self.vars.cmd, rc, out, err, run_info = self._run_multiple_commands(params, actionable_snaps, refresh=refresh) + if self.verbosity >= 4: + self.vars.run_info = run_info if rc == 0: match_install = [self.__install_re.match(line) for line in out.split('\n')] match_install = [m.group('name') in actionable_snaps for m in match_install if m] - if len(match_install) == len(commands): + if len(match_install) == len(actionable_snaps): return classic_snap_pattern = re.compile(r'^error: This revision of snap "(?P\w+)"' @@ -322,10 +344,13 @@ def state_present(self): self.vars.meta('classic').set(output=True) self.vars.meta('channel').set(output=True) - actionable_snaps = [s for s in self.vars.name if not self.is_snap_installed(s, self.vars.channel)] - if actionable_snaps: - self.process_actionable_snaps(actionable_snaps) + actionable_refresh = [snap for snap in self.vars.name if self.vars.snap_status_map[snap] == Snap.CHANNEL_MISMATCH] + if actionable_refresh: + self._present(actionable_refresh, refresh=True) + actionable_install = [snap for snap in self.vars.name if self.vars.snap_status_map[snap] == Snap.NOT_INSTALLED] + if actionable_install: + self._present(actionable_install) self.set_options() @@ -333,7 +358,7 @@ def set_options(self): if self.vars.options is None: return - actionable_snaps = [s for s in self.vars.name if self.is_snap_installed(s, self.vars.channel)] + actionable_snaps = [s for s in self.vars.name if self.vars.snap_status_map[s] != Snap.NOT_INSTALLED] overall_options_changed = [] for snap_name in actionable_snaps: @@ -383,7 +408,7 @@ def set_options(self): if overall_options_changed: self.vars.options_changed = overall_options_changed - def _generic_state_action(self, actionable_func, actionable_var, params=None): + def _generic_state_action(self, actionable_func, actionable_var, params): actionable_snaps = [s for s in self.vars.name if actionable_func(s)] if not actionable_snaps: return @@ -391,9 +416,9 @@ def _generic_state_action(self, actionable_func, actionable_var, params=None): self.vars[actionable_var] = actionable_snaps if self.check_mode: return - if params is None: - params = ['state'] - self.vars.cmd, rc, out, err = self._run_multiple_commands(params, actionable_snaps) + self.vars.cmd, rc, out, err, run_info = self._run_multiple_commands(params, actionable_snaps) + if self.verbosity >= 4: + self.vars.run_info = run_info if rc == 0: return msg = "Ooops! Snap operation failed while executing '{cmd}', please examine logs and " \ @@ -401,7 +426,7 @@ def _generic_state_action(self, actionable_func, actionable_var, params=None): self.do_raise(msg=msg) def state_absent(self): - self._generic_state_action(lambda s: self.is_snap_installed(s, self.vars.channel), "snaps_removed", ['classic', 'channel', 'state']) + self._generic_state_action(lambda s: self.vars.snap_status_map[s] != Snap.NOT_INSTALLED, "snaps_removed", ['classic', 'channel', 'state']) def state_enabled(self): self._generic_state_action(lambda s: not self.is_snap_enabled(s), "snaps_enabled", ['classic', 'channel', 'state']) diff --git a/tests/integration/targets/snap/tasks/main.yml b/tests/integration/targets/snap/tasks/main.yml index aa1f8f2f32a..8873d381dc9 100644 --- a/tests/integration/targets/snap/tasks/main.yml +++ b/tests/integration/targets/snap/tasks/main.yml @@ -4,5 +4,9 @@ # SPDX-License-Identifier: GPL-3.0-or-later - name: Has-snap include - ansible.builtin.include_tasks: test.yml when: has_snap + block: + - name: Include test + ansible.builtin.include_tasks: test.yml + - name: Include test_channel + ansible.builtin.include_tasks: test_channel.yml diff --git a/tests/integration/targets/snap/tasks/test.yml b/tests/integration/targets/snap/tasks/test.yml index c643200da64..3a77704b3e8 100644 --- a/tests/integration/targets/snap/tasks/test.yml +++ b/tests/integration/targets/snap/tasks/test.yml @@ -233,31 +233,3 @@ - install_two_remove_again is not changed - install_two_remove_again.snaps_installed is not defined - install_two_remove_again.snaps_removed is not defined - -# Test for https://github.com/ansible-collections/community.general/issues/1606 -- name: Install package (microk8s) - community.general.snap: - name: microk8s - classic: true - state: present - register: install_microk8s - -- name: Install package with channel (microk8s) - community.general.snap: - name: microk8s - classic: true - channel: 1.20/stable - state: present - register: install_microk8s_chan - -- name: Remove package (microk8s) - community.general.snap: - name: microk8s - state: absent - register: remove_microk8s - -- assert: - that: - - install_microk8s is changed - - install_microk8s_chan is changed - - remove_microk8s is changed diff --git a/tests/integration/targets/snap/tasks/test_channel.yml b/tests/integration/targets/snap/tasks/test_channel.yml new file mode 100644 index 00000000000..135ee1825e1 --- /dev/null +++ b/tests/integration/targets/snap/tasks/test_channel.yml @@ -0,0 +1,46 @@ +--- +# Copyright (c) Ansible Project +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +- name: Make sure package is not installed (microk8s) + community.general.snap: + name: microk8s + state: absent + +# Test for https://github.com/ansible-collections/community.general/issues/1606 +- name: Install package (microk8s) + community.general.snap: + name: microk8s + classic: true + state: present + register: install_microk8s + +- name: Install package with channel (microk8s) + community.general.snap: + name: microk8s + classic: true + channel: 1.20/stable + state: present + register: install_microk8s_chan + +- name: Install package with channel (microk8s) again + community.general.snap: + name: microk8s + classic: true + channel: 1.20/stable + state: present + register: install_microk8s_chan_again + +- name: Remove package (microk8s) + community.general.snap: + name: microk8s + state: absent + register: remove_microk8s + +- assert: + that: + - install_microk8s is changed + - install_microk8s_chan is changed + - install_microk8s_chan_again is not changed + - remove_microk8s is changed From 5b51b31654657e2e6483c30e7d31df28836a9024 Mon Sep 17 00:00:00 2001 From: Alexei Znamensky Date: Mon, 5 Jun 2023 11:34:10 +1200 Subject: [PATCH 6/9] rollback comment in integration test --- tests/integration/targets/setup_snap/handlers/main.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/integration/targets/setup_snap/handlers/main.yml b/tests/integration/targets/setup_snap/handlers/main.yml index 08c1a23f1d7..785854244d7 100644 --- a/tests/integration/targets/setup_snap/handlers/main.yml +++ b/tests/integration/targets/setup_snap/handlers/main.yml @@ -1,4 +1,9 @@ --- +#################################################################### +# WARNING: These are designed specifically for Ansible tests # +# and should not be used as examples of how to write Ansible roles # +#################################################################### + # Copyright (c) Ansible Project # GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) # SPDX-License-Identifier: GPL-3.0-or-later From 9fd439f3c9d87da386ae1e659c1f8629933c513c Mon Sep 17 00:00:00 2001 From: Alexei Znamensky Date: Mon, 5 Jun 2023 11:35:00 +1200 Subject: [PATCH 7/9] rollback comment in integration test --- tests/integration/targets/setup_snap/handlers/main.yml | 5 ----- tests/integration/targets/snap/tasks/main.yml | 5 +++++ 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/integration/targets/setup_snap/handlers/main.yml b/tests/integration/targets/setup_snap/handlers/main.yml index 785854244d7..08c1a23f1d7 100644 --- a/tests/integration/targets/setup_snap/handlers/main.yml +++ b/tests/integration/targets/setup_snap/handlers/main.yml @@ -1,9 +1,4 @@ --- -#################################################################### -# WARNING: These are designed specifically for Ansible tests # -# and should not be used as examples of how to write Ansible roles # -#################################################################### - # Copyright (c) Ansible Project # GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) # SPDX-License-Identifier: GPL-3.0-or-later diff --git a/tests/integration/targets/snap/tasks/main.yml b/tests/integration/targets/snap/tasks/main.yml index 8873d381dc9..126c2cbbd53 100644 --- a/tests/integration/targets/snap/tasks/main.yml +++ b/tests/integration/targets/snap/tasks/main.yml @@ -1,4 +1,9 @@ --- +#################################################################### +# WARNING: These are designed specifically for Ansible tests # +# and should not be used as examples of how to write Ansible roles # +#################################################################### + # Copyright (c) Ansible Project # GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) # SPDX-License-Identifier: GPL-3.0-or-later From 0511fddba8719ddca0b797a4fc25aead205faea3 Mon Sep 17 00:00:00 2001 From: Alexei Znamensky Date: Tue, 6 Jun 2023 15:16:20 +1200 Subject: [PATCH 8/9] add changelog frag --- changelogs/fragments/6435-snap-channel-aware.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelogs/fragments/6435-snap-channel-aware.yml diff --git a/changelogs/fragments/6435-snap-channel-aware.yml b/changelogs/fragments/6435-snap-channel-aware.yml new file mode 100644 index 00000000000..5787de3502d --- /dev/null +++ b/changelogs/fragments/6435-snap-channel-aware.yml @@ -0,0 +1,2 @@ +minor_changes: + - snap - module is now aware of channel when deciding whether to install or refresh the snap (https://github.com/ansible-collections/community.general/pull/6435, https://github.com/ansible-collections/community.general/issues/1606). From 124b64d2d27bc5a06e94e764968bad84ecdbf7c6 Mon Sep 17 00:00:00 2001 From: Alexei Znamensky <103110+russoz@users.noreply.github.com> Date: Tue, 6 Jun 2023 18:07:38 +1200 Subject: [PATCH 9/9] Update plugins/modules/snap.py Co-authored-by: Felix Fontein --- plugins/modules/snap.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/modules/snap.py b/plugins/modules/snap.py index 4dc245874e2..43abe4e9131 100644 --- a/plugins/modules/snap.py +++ b/plugins/modules/snap.py @@ -54,7 +54,7 @@ description: - Define which release of a snap is installed and tracked for updates. This option can only be specified if there is a single snap in the task. - - If not passed, the C(snap) command will default to I(stable). + - If not passed, the C(snap) command will default to V(stable). type: str required: false options: