From f1ded0f41759235eb15a7d13dbc3c95dce5d5acd Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Fri, 8 Mar 2024 09:43:42 +0100 Subject: [PATCH] dnf: utilize the API for the installed checks (#82725) Fixes #71808 Fixes #76463 Fixes #81018 --- .../fragments/dnf-installed-checks-api.yml | 3 + lib/ansible/modules/dnf.py | 139 ++---------------- lib/ansible/modules/dnf5.py | 46 +++--- test/integration/targets/dnf/tasks/repo.yml | 53 +++++++ .../setup_rpm_repo/library/create_repo.py | 4 + 5 files changed, 103 insertions(+), 142 deletions(-) create mode 100644 changelogs/fragments/dnf-installed-checks-api.yml diff --git a/changelogs/fragments/dnf-installed-checks-api.yml b/changelogs/fragments/dnf-installed-checks-api.yml new file mode 100644 index 00000000000..291385ae2f6 --- /dev/null +++ b/changelogs/fragments/dnf-installed-checks-api.yml @@ -0,0 +1,3 @@ +bugfixes: + - Mirror the behavior of dnf on the command line when handling NEVRAs with omitted epoch (https://github.com/ansible/ansible/issues/71808) + - Fix NEVRA parsing of package names that include digit(s) in them (https://github.com/ansible/ansible/issues/76463, https://github.com/ansible/ansible/issues/81018) diff --git a/lib/ansible/modules/dnf.py b/lib/ansible/modules/dnf.py index 1780c2f8650..593f006eb86 100644 --- a/lib/ansible/modules/dnf.py +++ b/lib/ansible/modules/dnf.py @@ -387,7 +387,6 @@ EXAMPLES = ''' ''' import os -import re import sys from ansible.module_utils.common.text.converters import to_native, to_text @@ -479,94 +478,6 @@ class DnfModule(YumDnf): return result - def _split_package_arch(self, packagename): - # This list was auto generated on a Fedora 28 system with the following one-liner - # printf '[ '; for arch in $(ls /usr/lib/rpm/platform); do printf '"%s", ' ${arch%-linux}; done; printf ']\n' - redhat_rpm_arches = [ - "aarch64", "alphaev56", "alphaev5", "alphaev67", "alphaev6", "alpha", - "alphapca56", "amd64", "armv3l", "armv4b", "armv4l", "armv5tejl", "armv5tel", - "armv5tl", "armv6hl", "armv6l", "armv7hl", "armv7hnl", "armv7l", "athlon", - "geode", "i386", "i486", "i586", "i686", "ia32e", "ia64", "m68k", "mips64el", - "mips64", "mips64r6el", "mips64r6", "mipsel", "mips", "mipsr6el", "mipsr6", - "noarch", "pentium3", "pentium4", "ppc32dy4", "ppc64iseries", "ppc64le", "ppc64", - "ppc64p7", "ppc64pseries", "ppc8260", "ppc8560", "ppciseries", "ppc", "ppcpseries", - "riscv64", "s390", "s390x", "sh3", "sh4a", "sh4", "sh", "sparc64", "sparc64v", - "sparc", "sparcv8", "sparcv9", "sparcv9v", "x86_64" - ] - - name, delimiter, arch = packagename.rpartition('.') - if name and arch and arch in redhat_rpm_arches: - return name, arch - return packagename, None - - def _packagename_dict(self, packagename): - """ - Return a dictionary of information for a package name string or None - if the package name doesn't contain at least all NVR elements - """ - - if packagename[-4:] == '.rpm': - packagename = packagename[:-4] - - rpm_nevr_re = re.compile(r'(\S+)-(?:(\d*):)?(.*)-(~?\w+[\w.+]*)') - try: - arch = None - nevr, arch = self._split_package_arch(packagename) - if arch: - packagename = nevr - rpm_nevr_match = rpm_nevr_re.match(packagename) - if rpm_nevr_match: - name, epoch, version, release = rpm_nevr_re.match(packagename).groups() - if not version or not version.split('.')[0].isdigit(): - return None - else: - return None - except AttributeError as e: - self.module.fail_json( - msg='Error attempting to parse package: %s, %s' % (packagename, to_native(e)), - rc=1, - results=[] - ) - - if not epoch: - epoch = "0" - - if ':' in name: - epoch_name = name.split(":") - - epoch = epoch_name[0] - name = ''.join(epoch_name[1:]) - - result = { - 'name': name, - 'epoch': epoch, - 'release': release, - 'version': version, - } - - return result - - # Original implementation from yum.rpmUtils.miscutils (GPLv2+) - # http://yum.baseurl.org/gitweb?p=yum.git;a=blob;f=rpmUtils/miscutils.py - def _compare_evr(self, e1, v1, r1, e2, v2, r2): - # return 1: a is newer than b - # 0: a and b are the same version - # -1: b is newer than a - if e1 is None: - e1 = '0' - else: - e1 = str(e1) - v1 = str(v1) - r1 = str(r1) - if e2 is None: - e2 = '0' - else: - e2 = str(e2) - v2 = str(v2) - r2 = str(r2) - rc = dnf.rpm.rpm.labelCompare((e1, v1, r1), (e2, v2, r2)) - return rc - def _ensure_dnf(self): locale = get_best_parsable_locale(self.module) os.environ['LC_ALL'] = os.environ['LC_MESSAGES'] = locale @@ -575,7 +486,6 @@ class DnfModule(YumDnf): global dnf try: import dnf - import dnf.cli import dnf.const import dnf.exceptions import dnf.package @@ -813,43 +723,22 @@ class DnfModule(YumDnf): self.module.exit_json(msg="", results=results) def _is_installed(self, pkg): - installed = self.base.sack.query().installed() - - package_spec = {} - name, arch = self._split_package_arch(pkg) - if arch: - package_spec['arch'] = arch - - package_details = self._packagename_dict(pkg) - if package_details: - package_details['epoch'] = int(package_details['epoch']) - package_spec.update(package_details) - else: - package_spec['name'] = name - - return bool(installed.filter(**package_spec)) + return bool( + dnf.subject.Subject(pkg).get_best_query(sack=self.base.sack).installed().run() + ) def _is_newer_version_installed(self, pkg_name): - candidate_pkg = self._packagename_dict(pkg_name) - if not candidate_pkg: - # The user didn't provide a versioned rpm, so version checking is - # not required - return False - - installed = self.base.sack.query().installed() - installed_pkg = installed.filter(name=candidate_pkg['name']).run() - if installed_pkg: - installed_pkg = installed_pkg[0] - - # this looks weird but one is a dict and the other is a dnf.Package - evr_cmp = self._compare_evr( - installed_pkg.epoch, installed_pkg.version, installed_pkg.release, - candidate_pkg['epoch'], candidate_pkg['version'], candidate_pkg['release'], - ) - - return evr_cmp == 1 - else: + try: + if isinstance(pkg_name, dnf.package.Package): + available = pkg_name + else: + available = sorted( + dnf.subject.Subject(pkg_name).get_best_query(sack=self.base.sack).available().run() + )[-1] + installed = sorted(self.base.sack.query().installed().filter(name=available.name).run())[-1] + except IndexError: return False + return installed > available def _mark_package_install(self, pkg_spec, upgrade=False): """Mark the package for install.""" @@ -1000,7 +889,7 @@ class DnfModule(YumDnf): else: for pkg in pkgs: try: - if self._is_newer_version_installed(self._package_dict(pkg)['nevra']): + if self._is_newer_version_installed(pkg): if self.allow_downgrade: self.base.package_install(pkg, strict=self.base.conf.strict) else: diff --git a/lib/ansible/modules/dnf5.py b/lib/ansible/modules/dnf5.py index e88c0a6ae05..8d122aef7cf 100644 --- a/lib/ansible/modules/dnf5.py +++ b/lib/ansible/modules/dnf5.py @@ -374,19 +374,37 @@ def is_newer_version_installed(base, spec): spec_nevra = next(iter(libdnf5.rpm.Nevra.parse(spec))) except (RuntimeError, StopIteration): return False - spec_name = spec_nevra.get_name() - v = spec_nevra.get_version() - r = spec_nevra.get_release() - if not v or not r: + + spec_version = spec_nevra.get_version() + if not spec_version: return False - spec_evr = "{}:{}-{}".format(spec_nevra.get_epoch() or "0", v, r) - query = libdnf5.rpm.PackageQuery(base) - query.filter_installed() - query.filter_name([spec_name]) - query.filter_evr([spec_evr], libdnf5.common.QueryCmp_GT) + installed = libdnf5.rpm.PackageQuery(base) + installed.filter_installed() + installed.filter_name([spec_nevra.get_name()]) + installed.filter_latest_evr() + try: + installed_package = list(installed)[-1] + except IndexError: + return False - return query.size() > 0 + target = libdnf5.rpm.PackageQuery(base) + target.filter_name([spec_nevra.get_name()]) + target.filter_version([spec_version]) + spec_release = spec_nevra.get_release() + if spec_release: + target.filter_release([spec_release]) + spec_epoch = spec_nevra.get_epoch() + if spec_epoch: + target.filter_epoch([spec_epoch]) + target.filter_latest_evr() + try: + target_package = list(target)[-1] + except IndexError: + return False + + # FIXME https://github.com/rpm-software-management/dnf5/issues/1104 + return libdnf5.rpm.rpmvercmp(installed_package.get_evr(), target_package.get_evr()) == 1 def package_to_dict(package): @@ -604,13 +622,7 @@ class Dnf5Module(YumDnf): for spec in self.names: if is_newer_version_installed(base, spec): if self.allow_downgrade: - if upgrade: - if is_installed(base, spec): - goal.add_upgrade(spec, settings) - else: - goal.add_install(spec, settings) - else: - goal.add_install(spec, settings) + goal.add_install(spec, settings) elif is_installed(base, spec): if upgrade: goal.add_upgrade(spec, settings) diff --git a/test/integration/targets/dnf/tasks/repo.yml b/test/integration/targets/dnf/tasks/repo.yml index 529bc905835..3eed448955b 100644 --- a/test/integration/targets/dnf/tasks/repo.yml +++ b/test/integration/targets/dnf/tasks/repo.yml @@ -467,3 +467,56 @@ state: absent loop: - provides_foo_b + +- name: ensure that a package named "$str-$number-$str" is parsed correctly + block: + - dnf: + name: number-11-name-11.0 + state: "{{ item }}" + loop: + - absent + - present + + - dnf: + name: number-11-name + state: present + register: dnf_result + + - assert: + that: + - dnf_result is not changed + + - dnf: + name: number-11-name + state: latest + update_only: true + register: dnf_result + + - assert: + that: + - dnf_result is changed + always: + - name: Clean up + dnf: + name: number-11-name + state: absent + +- name: test that epochs are handled the same way as via DNF on the command line + block: + - dnf: + name: "{{ item }}" + state: present + loop: + - "epochone-1.0-1.noarch" + - "epochone-1.1-1.noarch" + register: dnf_results + + - assert: + that: + - dnf_results["results"][0] is changed + - dnf_results["results"][1] is changed + always: + - name: Clean up + dnf: + name: epochone + state: absent diff --git a/test/integration/targets/setup_rpm_repo/library/create_repo.py b/test/integration/targets/setup_rpm_repo/library/create_repo.py index 8ca4a82e4cc..a07d8df657d 100644 --- a/test/integration/targets/setup_rpm_repo/library/create_repo.py +++ b/test/integration/targets/setup_rpm_repo/library/create_repo.py @@ -49,6 +49,10 @@ SPECS = [ RPM('noarchfake', '1.0', '1', None, None, None, 'noarch'), RPM('provides_foo_a', '1.0', '1', None, None, 'foo.so', 'noarch'), RPM('provides_foo_b', '1.0', '1', None, None, 'foo.so', 'noarch'), + RPM('number-11-name', '11.0', '1', None, None, None, None), + RPM('number-11-name', '11.1', '1', None, None, None, None), + RPM('epochone', '1.0', '1', '1', None, None, "noarch"), + RPM('epochone', '1.1', '1', '1', None, None, "noarch"), ]