From b78d1999e16075b15e4dd036fa432d24026b363b Mon Sep 17 00:00:00 2001 From: Alexei Znamensky <103110+russoz@users.noreply.github.com> Date: Wed, 7 Jun 2023 06:49:12 +1200 Subject: [PATCH] snap: aware of channel in installed snaps (#6435) * [WIP] snap: aware of channel in installed snaps * parse snap list output and assert whether channel matches * undo test * fail rightfully when install with different channel does not work * transparetent refresh * rollback comment in integration test * rollback comment in integration test * add changelog frag * Update plugins/modules/snap.py Co-authored-by: Felix Fontein --------- Co-authored-by: Felix Fontein --- .../fragments/6435-snap-channel-aware.yml | 2 + plugins/module_utils/snap.py | 1 + plugins/modules/snap.py | 94 +++++-- tests/integration/targets/snap/tasks/main.yml | 236 +----------------- tests/integration/targets/snap/tasks/test.yml | 235 +++++++++++++++++ .../targets/snap/tasks/test_channel.yml | 46 ++++ 6 files changed, 357 insertions(+), 257 deletions(-) create mode 100644 changelogs/fragments/6435-snap-channel-aware.yml create mode 100644 tests/integration/targets/snap/tasks/test.yml create mode 100644 tests/integration/targets/snap/tasks/test_channel.yml diff --git a/changelogs/fragments/6435-snap-channel-aware.yml b/changelogs/fragments/6435-snap-channel-aware.yml new file mode 100644 index 0000000000..5787de3502 --- /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). diff --git a/plugins/module_utils/snap.py b/plugins/module_utils/snap.py index 3ae126dbfd..1b3bdf2fe5 100644 --- a/plugins/module_utils/snap.py +++ b/plugins/module_utils/snap.py @@ -20,6 +20,7 @@ _state_map = dict( absent='remove', enabled='enable', disabled='disable', + refresh='refresh', ) diff --git a/plugins/modules/snap.py b/plugins/modules/snap.py index 359b7fa323..43abe4e913 100644 --- a/plugins/modules/snap.py +++ b/plugins/modules/snap.py @@ -35,7 +35,9 @@ options: 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 @@ -52,9 +54,9 @@ options: 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 V(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 @@ -159,15 +161,20 @@ from ansible_collections.community.general.plugins.module_utils.snap import snap 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.+\s(installed|refreshed)') 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', default='stable'), + 'channel': dict(type='str'), 'options': dict(type='list', elements='str'), }, supports_check_mode=True, @@ -183,35 +190,47 @@ class Snap(StateModuleHelper): 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): + if self.vars.channel is None: + self.vars.channel = "stable" + def convert_json_subtree_to_map(self, json_subtree, prefix=None): option_map = {} @@ -224,7 +243,6 @@ class Snap(StateModuleHelper): 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)) @@ -248,16 +266,32 @@ class Snap(StateModuleHelper): 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): - rc, dummy, dummy = self.runner("_list name").run(name=snap_name) - return rc == 0 + 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 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: @@ -271,7 +305,7 @@ class Snap(StateModuleHelper): 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 @@ -283,12 +317,17 @@ class Snap(StateModuleHelper): 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: - 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(actionable_snaps): + return classic_snap_pattern = re.compile(r'^error: This revision of snap "(?P\w+)"' r' was published using classic confinement') @@ -305,10 +344,13 @@ class Snap(StateModuleHelper): 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)] - 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() @@ -316,7 +358,7 @@ class Snap(StateModuleHelper): 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.vars.snap_status_map[s] != Snap.NOT_INSTALLED] overall_options_changed = [] for snap_name in actionable_snaps: @@ -366,7 +408,7 @@ class Snap(StateModuleHelper): 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 @@ -374,9 +416,9 @@ class Snap(StateModuleHelper): 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 " \ @@ -384,7 +426,7 @@ class Snap(StateModuleHelper): 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.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 0f24e69f3d..126c2cbbd5 100644 --- a/tests/integration/targets/snap/tasks/main.yml +++ b/tests/integration/targets/snap/tasks/main.yml @@ -8,236 +8,10 @@ # 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 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 + - 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 new file mode 100644 index 0000000000..3a77704b3e --- /dev/null +++ b/tests/integration/targets/snap/tasks/test.yml @@ -0,0 +1,235 @@ +--- +# 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 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 0000000000..135ee1825e --- /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