diff --git a/changelogs/fragments/4337-pacman-update_cache.yml b/changelogs/fragments/4337-pacman-update_cache.yml new file mode 100644 index 0000000000..01a9cb11ac --- /dev/null +++ b/changelogs/fragments/4337-pacman-update_cache.yml @@ -0,0 +1,4 @@ +minor_changes: + - "pacman - now implements proper change detection for ``update_cache=true``. Adds ``cache_updated`` return value to when ``update_cache=true`` + to report this result independently of the module's overall changed return value + (https://github.com/ansible-collections/community.general/pull/4337)." diff --git a/plugins/modules/packaging/os/pacman.py b/plugins/modules/packaging/os/pacman.py index 8d534ed8d6..3b0fc5c599 100644 --- a/plugins/modules/packaging/os/pacman.py +++ b/plugins/modules/packaging/os/pacman.py @@ -125,6 +125,15 @@ packages: elements: str sample: [ package, other-package ] +cache_updated: + description: + - The changed status of C(pacman -Sy). + - Useful when I(name) or I(upgrade=true) are specified next to I(update_cache=true). + returned: success, when I(update_cache=true) + type: bool + sample: false + version_added: 4.6.0 + stdout: description: - Output from pacman. @@ -251,6 +260,8 @@ class Pacman(object): self.pacman_path = self.m.get_bin_path(p["executable"], True) + self._cached_database = None + # Normalize for old configs if p["state"] == "installed": self.target_state = "present" @@ -413,6 +424,7 @@ class Pacman(object): if rc != 0: self.fail("Failed to install package(s)", cmd=cmd, stdout=stdout, stderr=stderr) self.add_exit_infos(stdout=stdout, stderr=stderr) + self._invalidate_database() if pkgs_to_install: _install_packages_for_real("--sync", pkgs_to_install) @@ -467,6 +479,7 @@ class Pacman(object): rc, stdout, stderr = self.m.run_command(cmd, check_rc=False) if rc != 0: self.fail("failed to remove package(s)", cmd=cmd, stdout=stdout, stderr=stderr) + self._invalidate_database() self.exit_params["packages"] = removed_pkgs self.add_exit_infos("Removed %d package(s)" % len(removed_pkgs), stdout=stdout, stderr=stderr) @@ -502,16 +515,29 @@ class Pacman(object): if self.m.params["upgrade_extra_args"]: cmd += self.m.params["upgrade_extra_args"] rc, stdout, stderr = self.m.run_command(cmd, check_rc=False) + self._invalidate_database() if rc == 0: self.add_exit_infos("System upgraded", stdout=stdout, stderr=stderr) else: self.fail("Could not upgrade", cmd=cmd, stdout=stdout, stderr=stderr) + def _list_database(self): + """runs pacman --sync --list with some caching""" + if self._cached_database is None: + dummy, packages, dummy = self.m.run_command([self.pacman_path, '--sync', '--list'], check_rc=True) + self._cached_database = packages.splitlines() + return self._cached_database + + def _invalidate_database(self): + """invalidates the pacman --sync --list cache""" + self._cached_database = None + def update_package_db(self): """runs pacman --sync --refresh""" if self.m.check_mode: self.add_exit_infos("Would have updated the package db") self.changed = True + self.exit_params["cache_updated"] = True return cmd = [ @@ -523,10 +549,23 @@ class Pacman(object): cmd += self.m.params["update_cache_extra_args"] if self.m.params["force"]: cmd += ["--refresh"] + else: + # Dump package database to get contents before update + pre_state = sorted(self._list_database()) rc, stdout, stderr = self.m.run_command(cmd, check_rc=False) + self._invalidate_database() - self.changed = True + if self.m.params["force"]: + # Always changed when force=true + self.exit_params["cache_updated"] = True + else: + # Dump package database to get contents after update + post_state = sorted(self._list_database()) + # If contents changed, set changed=true + self.exit_params["cache_updated"] = pre_state != post_state + if self.exit_params["cache_updated"]: + self.changed = True if rc == 0: self.add_exit_infos("Updated package db", stdout=stdout, stderr=stderr) @@ -622,9 +661,9 @@ class Pacman(object): installed_groups[group].add(pkgname) available_pkgs = {} - dummy, stdout, dummy = self.m.run_command([self.pacman_path, "--sync", "--list"], check_rc=True) + database = self._list_database() # Format of a line: "core pacman 6.0.1-2" - for l in stdout.splitlines(): + for l in database: l = l.strip() if not l: continue diff --git a/tests/integration/targets/pacman/tasks/main.yml b/tests/integration/targets/pacman/tasks/main.yml index 968f620873..7014651804 100644 --- a/tests/integration/targets/pacman/tasks/main.yml +++ b/tests/integration/targets/pacman/tasks/main.yml @@ -10,3 +10,4 @@ - include: 'basic.yml' - include: 'package_urls.yml' - include: 'remove_nosave.yml' + - include: 'update_cache.yml' diff --git a/tests/integration/targets/pacman/tasks/update_cache.yml b/tests/integration/targets/pacman/tasks/update_cache.yml new file mode 100644 index 0000000000..c9b33e65ff --- /dev/null +++ b/tests/integration/targets/pacman/tasks/update_cache.yml @@ -0,0 +1,23 @@ +--- +- name: Make sure package cache is updated + pacman: + update_cache: true + +- name: Update package cache again (should not be changed) + pacman: + update_cache: true + register: update_cache_idem + +- name: Update package cache again with force=true (should be changed) + pacman: + update_cache: true + force: true + register: update_cache_force + +- name: Check conditions + assert: + that: + - update_cache_idem is not changed + - update_cache_idem.cache_updated == false + - update_cache_force is changed + - update_cache_force.cache_updated == true diff --git a/tests/unit/plugins/modules/packaging/os/test_pacman.py b/tests/unit/plugins/modules/packaging/os/test_pacman.py index 9b906d2a30..bbc8f3435c 100644 --- a/tests/unit/plugins/modules/packaging/os/test_pacman.py +++ b/tests/unit/plugins/modules/packaging/os/test_pacman.py @@ -329,33 +329,75 @@ class TestPacman: assert out["changed"] @pytest.mark.parametrize( - "module_args,expected_call", + "module_args,expected_calls,changed", [ - ({}, ["pacman", "--sync", "--refresh"]), - ({"force": True}, ["pacman", "--sync", "--refresh", "--refresh"]), ( - {"update_cache_extra_args": "--some-extra args"}, - ["pacman", "--sync", "--refresh", "--some-extra", "args"], # shlex test + {}, + [ + (["pacman", "--sync", "--list"], {'check_rc': True}, 0, 'a\nb\nc', ''), + (["pacman", "--sync", "--refresh"], {'check_rc': False}, 0, 'stdout', 'stderr'), + (["pacman", "--sync", "--list"], {'check_rc': True}, 0, 'b\na\nc', ''), + ], + False, + ), + ( + {"force": True}, + [ + (["pacman", "--sync", "--refresh", "--refresh"], {'check_rc': False}, 0, 'stdout', 'stderr'), + ], + True, + ), + ( + {"update_cache_extra_args": "--some-extra args"}, # shlex test + [ + (["pacman", "--sync", "--list"], {'check_rc': True}, 0, 'a\nb\nc', ''), + (["pacman", "--sync", "--refresh", "--some-extra", "args"], {'check_rc': False}, 0, 'stdout', 'stderr'), + (["pacman", "--sync", "--list"], {'check_rc': True}, 0, 'a changed\nb\nc', ''), + ], + True, ), ( {"force": True, "update_cache_extra_args": "--some-extra args"}, - ["pacman", "--sync", "--refresh", "--some-extra", "args", "--refresh"], + [ + (["pacman", "--sync", "--refresh", "--some-extra", "args", "--refresh"], {'check_rc': False}, 0, 'stdout', 'stderr'), + ], + True, + ), + ( + # Test whether pacman --sync --list is not called more than twice + {"upgrade": True}, + [ + (["pacman", "--sync", "--list"], {'check_rc': True}, 0, 'core foo 1.0.0-1 [installed]', ''), + (["pacman", "--sync", "--refresh"], {'check_rc': False}, 0, 'stdout', 'stderr'), + (["pacman", "--sync", "--list"], {'check_rc': True}, 0, 'core foo 1.0.0-1 [installed]', ''), + # The following is _build_inventory: + (["pacman", "--query"], {'check_rc': True}, 0, 'foo 1.0.0-1', ''), + (["pacman", "--query", "--groups"], {'check_rc': True}, 0, '', ''), + (["pacman", "--sync", "--groups", "--groups"], {'check_rc': True}, 0, '', ''), + (["pacman", "--query", "--upgrades"], {'check_rc': False}, 0, '', ''), + ], + False, ), ], ) - def test_update_db(self, mock_empty_inventory, module_args, expected_call): + def test_update_db(self, module_args, expected_calls, changed): args = {"update_cache": True} args.update(module_args) set_module_args(args) - self.mock_run_command.return_value = [0, "stdout", "stderr"] + self.mock_run_command.side_effect = [ + (rc, stdout, stderr) for expected_call, kwargs, rc, stdout, stderr in expected_calls + ] with pytest.raises(AnsibleExitJson) as e: P = pacman.Pacman(pacman.setup_module()) P.run() - self.mock_run_command.assert_called_with(mock.ANY, expected_call, check_rc=False) + self.mock_run_command.assert_has_calls([ + mock.call(mock.ANY, expected_call, **kwargs) for expected_call, kwargs, rc, stdout, stderr in expected_calls + ]) out = e.value.args[0] - assert out["changed"] + assert out["cache_updated"] == changed + assert out["changed"] == changed @pytest.mark.parametrize( "check_mode_value, run_command_data, upgrade_extra_args",