diff --git a/lib/ansible/modules/dnf.py b/lib/ansible/modules/dnf.py index 1922ba85e79..9d14a90dc15 100644 --- a/lib/ansible/modules/dnf.py +++ b/lib/ansible/modules/dnf.py @@ -208,6 +208,8 @@ options: packages to install (because dependencies between the downgraded package and others can cause changes to the packages which were in the earlier transaction). + - Since this feature is not provided by C(dnf) itself but by M(ansible.builtin.dnf) module, + using this in combination with wildcard characters in O(name) may result in an unexpected results. type: bool default: "no" version_added: "2.7" @@ -701,72 +703,56 @@ class DnfModule(YumDnf): self.module.exit_json(msg="", results=results) def _is_installed(self, pkg): - installed_query = dnf.subject.Subject(pkg).get_best_query(sack=self.base.sack).installed() - if dnf.util.is_glob_pattern(pkg): - available_query = dnf.subject.Subject(pkg).get_best_query(sack=self.base.sack).available() - return not ( - {p.name for p in available_query} - {p.name for p in installed_query} - ) - else: - return bool(installed_query) + return bool(dnf.subject.Subject(pkg).get_best_query(sack=self.base.sack).installed()) def _is_newer_version_installed(self, pkg_spec): + # expects a versioned package spec try: if isinstance(pkg_spec, dnf.package.Package): installed = sorted(self.base.sack.query().installed().filter(name=pkg_spec.name, arch=pkg_spec.arch))[-1] return installed.evr_gt(pkg_spec) else: - available = dnf.subject.Subject(pkg_spec).get_best_query(sack=self.base.sack).available() - installed = self.base.sack.query().installed().filter(name=available[0].name) - for arch in sorted(set(p.arch for p in installed)): # select only from already-installed arches for this case - installed_pkg = sorted(installed.filter(arch=arch))[-1] - try: - available_pkg = sorted(available.filter(arch=arch))[-1] - except IndexError: - continue # nothing currently available for this arch; keep going - if installed_pkg.evr_gt(available_pkg): - return True - return False + solution = dnf.subject.Subject(pkg_spec).get_best_solution(self.base.sack) + q = solution["query"] + if not q or not solution['nevra'] or solution['nevra'].has_just_name(): + return False + installed = self.base.sack.query().installed().filter(name=solution['nevra'].name) + if not installed: + return False + return installed[0].evr_gt(q[0]) except IndexError: return False def _mark_package_install(self, pkg_spec, upgrade=False): """Mark the package for install.""" - is_newer_version_installed = self._is_newer_version_installed(pkg_spec) - is_installed = self._is_installed(pkg_spec) msg = '' try: - if is_newer_version_installed: + if dnf.util.is_glob_pattern(pkg_spec): + # Special case for package specs that contain glob characters. + # For these we skip `is_installed` and `is_newer_version_installed` tests that allow for the + # allow_downgrade feature and pass the package specs to dnf. + # Since allow_downgrade is not available in dnf and while it is relatively easy to implement it for + # package specs that evaluate to a single package, trying to mimic what would the dnf machinery do + # for glob package specs and then filtering those for allow_downgrade appears to always + # result in naive/inferior solution. + # NOTE this has historically never worked even before https://github.com/ansible/ansible/pull/82725 + # where our (buggy) custom code ignored wildcards for the installed checks. + # TODO reasearch how feasible it is to implement the above + if upgrade: + # for upgrade we pass the spec to both upgrade and install, to satisfy both available and installed + # packages evaluated from the glob spec + try: + self.base.upgrade(pkg_spec) + except dnf.exceptions.PackagesNotInstalledError: + pass + self.base.install(pkg_spec, strict=self.base.conf.strict) + elif self._is_newer_version_installed(pkg_spec): if self.allow_downgrade: - # dnf only does allow_downgrade, we have to handle this ourselves - # because it allows a possibility for non-idempotent transactions - # on a system's package set (pending the yum repo has many old - # NVRs indexed) - if upgrade: - if is_installed: # Case 1 - # TODO: Is this case reachable? - # - # _is_installed() demands a name (*not* NVR) or else is always False - # (wildcards are treated literally). - # - # Meanwhile, _is_newer_version_installed() demands something versioned - # or else is always false. - # - # I fail to see how they can both be true at the same time for any - # given pkg_spec. -re - self.base.upgrade(pkg_spec) - else: # Case 2 - self.base.install(pkg_spec, strict=self.base.conf.strict) - else: # Case 3 - self.base.install(pkg_spec, strict=self.base.conf.strict) - else: # Case 4, Nothing to do, report back - pass - elif is_installed: # A potentially older (or same) version is installed - if upgrade: # Case 5 + self.base.install(pkg_spec, strict=self.base.conf.strict) + elif self._is_installed(pkg_spec): + if upgrade: self.base.upgrade(pkg_spec) - else: # Case 6, Nothing to do, report back - pass - else: # Case 7, The package is not installed, simply install it + else: self.base.install(pkg_spec, strict=self.base.conf.strict) except dnf.exceptions.MarkingError as e: msg = "No package {0} available.".format(pkg_spec) diff --git a/lib/ansible/modules/dnf5.py b/lib/ansible/modules/dnf5.py index cd9bf6e3f2e..dc2d0eb5e13 100644 --- a/lib/ansible/modules/dnf5.py +++ b/lib/ansible/modules/dnf5.py @@ -178,6 +178,8 @@ options: packages to install (because dependencies between the downgraded package and others can cause changes to the packages which were in the earlier transaction). + - Since this feature is not provided by C(dnf5) itself but by M(ansible.builtin.dnf5) module, + using this in combination with wildcard characters in O(name) may result in an unexpected results. type: bool default: "no" download_only: @@ -362,7 +364,7 @@ libdnf5 = None LIBDNF5_ERRORS = RuntimeError -def is_installed(base, spec): +def get_resolve_spec_settings(): settings = libdnf5.base.ResolveSpecSettings() try: settings.set_group_with_name(True) @@ -388,47 +390,34 @@ def is_installed(base, spec): settings.group_with_name = True settings.with_binaries = False settings.with_provides = False + return settings + + +def is_installed(base, spec): + settings = get_resolve_spec_settings() installed_query = libdnf5.rpm.PackageQuery(base) installed_query.filter_installed() match, nevra = installed_query.resolve_pkg_spec(spec, settings, True) - - # FIXME use `is_glob_pattern` function when available: - # https://github.com/rpm-software-management/dnf5/issues/1563 - glob_patterns = set("*[?") - if any(set(char) & glob_patterns for char in spec): - available_query = libdnf5.rpm.PackageQuery(base) - available_query.filter_available() - available_query.resolve_pkg_spec(spec, settings, True) - - return not ( - {p.get_name() for p in available_query} - {p.get_name() for p in installed_query} - ) - else: - return match + return match def is_newer_version_installed(base, spec): - # FIXME investigate whether this function can be replaced by dnf5's allow_downgrade option + # expects a versioned package spec if "/" in spec: spec = spec.split("/")[-1] if spec.endswith(".rpm"): spec = spec[:-4] - try: - spec_nevra = next(iter(libdnf5.rpm.Nevra.parse(spec))) - except LIBDNF5_ERRORS: - return False - except StopIteration: - return False - - spec_version = spec_nevra.get_version() - if not spec_version: + settings = get_resolve_spec_settings() + match, spec_nevra = libdnf5.rpm.PackageQuery(base).resolve_pkg_spec(spec, settings, True) + if not match or spec_nevra.has_just_name(): return False + spec_name = spec_nevra.get_name() installed = libdnf5.rpm.PackageQuery(base) installed.filter_installed() - installed.filter_name([spec_nevra.get_name()]) + installed.filter_name([spec_name]) installed.filter_latest_evr() try: installed_package = list(installed)[-1] @@ -436,8 +425,8 @@ def is_newer_version_installed(base, spec): return False target = libdnf5.rpm.PackageQuery(base) - target.filter_name([spec_nevra.get_name()]) - target.filter_version([spec_version]) + target.filter_name([spec_name]) + target.filter_version([spec_nevra.get_version()]) spec_release = spec_nevra.get_release() if spec_release: target.filter_release([spec_release]) @@ -719,8 +708,26 @@ class Dnf5Module(YumDnf): goal.add_rpm_upgrade(settings) elif self.state in {"installed", "present", "latest"}: upgrade = self.state == "latest" + # FIXME use `is_glob_pattern` function when available: + # https://github.com/rpm-software-management/dnf5/issues/1563 + glob_patterns = set("*[?") for spec in self.names: - if is_newer_version_installed(base, spec): + if any(set(char) & glob_patterns for char in spec): + # Special case for package specs that contain glob characters. + # For these we skip `is_installed` and `is_newer_version_installed` tests that allow for the + # allow_downgrade feature and pass the package specs to dnf. + # Since allow_downgrade is not available in dnf and while it is relatively easy to implement it for + # package specs that evaluate to a single package, trying to mimic what would the dnf machinery do + # for glob package specs and then filtering those for allow_downgrade appears to always + # result in naive/inferior solution. + # TODO reasearch how feasible it is to implement the above + if upgrade: + # for upgrade we pass the spec to both upgrade and install, to satisfy both available and installed + # packages evaluated from the glob spec + goal.add_upgrade(spec, settings) + if not self.update_only: + goal.add_install(spec, settings) + elif is_newer_version_installed(base, spec): if self.allow_downgrade: goal.add_install(spec, settings) elif is_installed(base, spec): diff --git a/test/integration/targets/dnf/tasks/repo.yml b/test/integration/targets/dnf/tasks/repo.yml index 8240b580f38..00034169b09 100644 --- a/test/integration/targets/dnf/tasks/repo.yml +++ b/test/integration/targets/dnf/tasks/repo.yml @@ -630,6 +630,87 @@ - provides-package - provided-package +# https://github.com/ansible/ansible/issues/45250 +- block: + - name: Install dinginessentail-1.0, dinginessentail-olive-1.0, landsidescalping-1.0 + dnf: + name: "dinginessentail-1.0,dinginessentail-olive-1.0,landsidescalping-1.0" + state: present + + - name: Upgrade dinginessentail* + dnf: + name: dinginessentail* + state: latest + register: dnf_result + + - name: Check dinginessentail with rpm + shell: rpm -q dinginessentail + register: rpm_result + + - name: Verify update of dinginessentail + assert: + that: + - "rpm_result.stdout.startswith('dinginessentail-1.1-1')" + + - name: Check dinginessentail-olive with rpm + shell: rpm -q dinginessentail-olive + register: rpm_result + + - name: Verify update of dinginessentail-olive + assert: + that: + - "rpm_result.stdout.startswith('dinginessentail-olive-1.1-1')" + + - name: Check landsidescalping with rpm + shell: rpm -q landsidescalping + register: rpm_result + + - name: Verify landsidescalping did NOT get updated + assert: + that: + - "rpm_result.stdout.startswith('landsidescalping-1.0-1')" + + - name: Verify yum module outputs + assert: + that: + - "dnf_result is changed" + - "'msg' in dnf_result" + - "'rc' in dnf_result" + - "'results' in dnf_result" + always: + - name: Clean up + dnf: + name: dinginessentail,dinginessentail-olive,landsidescalping + state: absent + +- name: test allow_downgrade + block: + - dnf: + name: dinginessentail-1.1 + state: present + + - dnf: + name: dinginessentail-1.0 + state: present + allow_downgrade: true + - dnf: + name: dinginessentail-1.1 + state: present + + - dnf: + name: dinginessentail-1.0 + state: present + allow_downgrade: false + register: r + + - assert: + that: + - r is not changed + always: + - dnf: + name: dinginessentail + state: absent + - name: Test failures occured during loading repositories are properly handled vars: repo_name: test-non-existing-gpgkey-file @@ -657,3 +738,15 @@ - file: name: /etc/yum.repos.d/{{ repo_name }}.repo state: absent + + +- name: Attempt to install a package with invalid name + dnf: + name: invalid[package_name] + register: r + ignore_errors: true + +- assert: + that: + - r is failed + - r.msg is contains("Failed to install some of the specified packages")