Skip to content

Commit

Permalink
Remove the yum module, redirect it to dnf (ansible#81895)
Browse files Browse the repository at this point in the history
  • Loading branch information
mkrizek authored Jan 23, 2024
1 parent c7334ea commit f024cf3
Show file tree
Hide file tree
Showing 45 changed files with 70 additions and 4,791 deletions.
2 changes: 1 addition & 1 deletion .github/ISSUE_TEMPLATE/bug_report.yml
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ body:
[collections org]: /ansible-collections
placeholder: dnf, apt, yum, pip, user etc.
placeholder: dnf, apt, pip, user etc.
validations:
required: true

Expand Down
2 changes: 1 addition & 1 deletion .github/ISSUE_TEMPLATE/feature_request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ body:
[collections org]: /ansible-collections
placeholder: dnf, apt, yum, pip, user etc.
placeholder: dnf, apt, pip, user etc.
validations:
required: true

Expand Down
2 changes: 2 additions & 0 deletions changelogs/fragments/yum-removal.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
removed_features:
- "With the removal of Python 2 support, the yum module and yum action plugin are removed and redirected to ``dnf``."
2 changes: 1 addition & 1 deletion hacking/azp/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ This directory contains the following scripts:

Incidental testing and code coverage occurs when a test covers one or more portions of code as an unintentional side-effect of testing another portion of code.

For example, the ``yum`` integration test intentionally tests the ``yum`` Ansible module.
For example, the ``dnf`` integration test intentionally tests the ``dnf`` Ansible module.
However, in doing so it also uses, and unintentionally tests the ``file`` module as well.

As part of the process of migrating modules and plugins into collections, integration tests were identified that provided exclusive incidental code coverage.
Expand Down
2 changes: 2 additions & 0 deletions lib/ansible/config/ansible_builtin_runtime.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9088,6 +9088,8 @@ plugin_routing:
tombstone:
removal_date: "2023-05-16"
warning_text: Use include_tasks or import_tasks instead.
yum:
redirect: ansible.builtin.dnf
become:
doas:
redirect: community.general.doas
Expand Down
2 changes: 1 addition & 1 deletion lib/ansible/executor/task_result.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def is_skipped(self):
if 'results' in self._result:
results = self._result['results']
# Loop tasks are only considered skipped if all items were skipped.
# some squashed results (eg, yum) are not dicts and can't be skipped individually
# some squashed results (eg, dnf) are not dicts and can't be skipped individually
if results and all(isinstance(res, dict) and res.get('skipped', False) for res in results):
return True

Expand Down
2 changes: 1 addition & 1 deletion lib/ansible/module_utils/common/respawn.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def respawn_module(interpreter_path):
Respawn the currently-running Ansible Python module under the specified Python interpreter.
Ansible modules that require libraries that are typically available only under well-known interpreters
(eg, ``yum``, ``apt``, ``dnf``) can use bespoke logic to determine the libraries they need are not
(eg, ``apt``, ``dnf``) can use bespoke logic to determine the libraries they need are not
available, then call `respawn_module` to re-execute the current module under a different interpreter
and exit the current process when the new subprocess has completed. The respawned process inherits only
stdout/stderr from the current process.
Expand Down
34 changes: 6 additions & 28 deletions lib/ansible/module_utils/facts/system/pkg_mgr.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@
# package manager, put the preferred one last. If there is an
# ansible module, use that as the value for the 'name' key.
PKG_MGRS = [{'path': '/usr/bin/rpm-ostree', 'name': 'atomic_container'},
{'path': '/usr/bin/yum', 'name': 'yum'},

# NOTE the `path` key for dnf/dnf5 is effectively discarded when matched for Red Hat OS family,
# special logic to infer the default `pkg_mgr` is used in `PkgMgrFactCollector._check_rh_versions()`
# leaving them here so a list of package modules can be constructed by iterating over `name` keys
{'path': '/usr/bin/yum', 'name': 'dnf'},
{'path': '/usr/bin/dnf-3', 'name': 'dnf'},
{'path': '/usr/bin/dnf5', 'name': 'dnf5'},

Expand All @@ -45,7 +45,6 @@
{'path': '/usr/bin/swupd', 'name': 'swupd'},
{'path': '/usr/sbin/sorcery', 'name': 'sorcery'},
{'path': '/usr/bin/installp', 'name': 'installp'},
{'path': '/QOpenSys/pkgs/bin/yum', 'name': 'yum'},
]


Expand All @@ -69,39 +68,18 @@ def __init__(self, *args, **kwargs):
super(PkgMgrFactCollector, self).__init__(*args, **kwargs)
self._default_unknown_pkg_mgr = 'unknown'

def _check_rh_versions(self, pkg_mgr_name, collected_facts):
def _check_rh_versions(self):
if os.path.exists('/run/ostree-booted'):
return "atomic_container"

# Reset whatever was matched from PKG_MGRS, infer the default pkg_mgr below
pkg_mgr_name = self._default_unknown_pkg_mgr
# Since /usr/bin/dnf and /usr/bin/microdnf can point to different versions of dnf in different distributions
# the only way to infer the default package manager is to look at the binary they are pointing to.
# /usr/bin/microdnf is likely used only in fedora minimal container so /usr/bin/dnf takes precedence
for bin_path in ('/usr/bin/dnf', '/usr/bin/microdnf'):
if os.path.exists(bin_path):
pkg_mgr_name = 'dnf5' if os.path.realpath(bin_path) == '/usr/bin/dnf5' else 'dnf'
break

try:
major_version = collected_facts['ansible_distribution_major_version']
if collected_facts['ansible_distribution'] == 'Kylin Linux Advanced Server':
major_version = major_version.lstrip('V')
distro_major_ver = int(major_version)
except ValueError:
# a non integer magical future version
return self._default_unknown_pkg_mgr

if (
(collected_facts['ansible_distribution'] == 'Fedora' and distro_major_ver < 23)
or (collected_facts['ansible_distribution'] == 'Kylin Linux Advanced Server' and distro_major_ver < 10)
or (collected_facts['ansible_distribution'] == 'Amazon' and distro_major_ver < 2022)
or (collected_facts['ansible_distribution'] == 'TencentOS' and distro_major_ver < 3)
or distro_major_ver < 8 # assume RHEL or a clone
) and any(pm for pm in PKG_MGRS if pm['name'] == 'yum' and os.path.exists(pm['path'])):
pkg_mgr_name = 'yum'
return 'dnf5' if os.path.realpath(bin_path) == '/usr/bin/dnf5' else 'dnf'

return pkg_mgr_name
return self._default_unknown_pkg_mgr

def _check_apt_flavor(self, pkg_mgr_name):
# Check if '/usr/bin/apt' is APT-RPM or an ordinary (dpkg-based) APT.
Expand Down Expand Up @@ -142,9 +120,9 @@ def collect(self, module=None, collected_facts=None):
# installed or available to the distro, the ansible_fact entry should be
# the default package manager officially supported by the distro.
if collected_facts['ansible_os_family'] == "RedHat":
pkg_mgr_name = self._check_rh_versions(pkg_mgr_name, collected_facts)
pkg_mgr_name = self._check_rh_versions()
elif collected_facts['ansible_os_family'] == 'Debian' and pkg_mgr_name != 'apt':
# It's possible to install yum, dnf, zypper, rpm, etc inside of
# It's possible to install dnf, zypper, rpm, etc inside of
# Debian. Doing so does not mean the system wants to use them.
pkg_mgr_name = 'apt'
elif collected_facts['ansible_os_family'] == 'Altlinux':
Expand Down
41 changes: 9 additions & 32 deletions lib/ansible/module_utils/yumdnf.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,12 @@

from __future__ import annotations

import os
import time
import glob
from abc import ABCMeta, abstractmethod

from ansible.module_utils.six import with_metaclass

yumdnf_argument_spec = dict(
argument_spec=dict(
allow_downgrade=dict(type='bool', default=False),
allowerasing=dict(default=False, type="bool"),
autoremove=dict(type='bool', default=False),
bugfix=dict(required=False, type='bool', default=False),
cacheonly=dict(type='bool', default=False),
Expand All @@ -35,10 +31,14 @@
enablerepo=dict(type='list', elements='str', default=[]),
exclude=dict(type='list', elements='str', default=[]),
installroot=dict(type='str', default="/"),
install_repoquery=dict(type='bool', default=True),
install_repoquery=dict(
type='bool', default=True,
removed_in_version='2.20', removed_from_collection='ansible.builtin',
),
install_weak_deps=dict(type='bool', default=True),
list=dict(type='str'),
name=dict(type='list', elements='str', aliases=['pkg'], default=[]),
nobest=dict(default=False, type="bool"),
releasever=dict(default=None),
security=dict(type='bool', default=False),
skip_broken=dict(type='bool', default=False),
Expand All @@ -56,7 +56,7 @@
)


class YumDnf(with_metaclass(ABCMeta, object)): # type: ignore[misc]
class YumDnf(metaclass=ABCMeta):
"""
Abstract class that handles the population of instance variables that should
be identical between both YUM and DNF modules because of the feature parity
Expand All @@ -68,6 +68,7 @@ def __init__(self, module):
self.module = module

self.allow_downgrade = self.module.params['allow_downgrade']
self.allowerasing = self.module.params['allowerasing']
self.autoremove = self.module.params['autoremove']
self.bugfix = self.module.params['bugfix']
self.cacheonly = self.module.params['cacheonly']
Expand All @@ -86,6 +87,7 @@ def __init__(self, module):
self.install_weak_deps = self.module.params['install_weak_deps']
self.list = self.module.params['list']
self.names = [p.strip() for p in self.module.params['name']]
self.nobest = self.module.params['nobest']
self.releasever = self.module.params['releasever']
self.security = self.module.params['security']
self.skip_broken = self.module.params['skip_broken']
Expand Down Expand Up @@ -126,31 +128,6 @@ def __init__(self, module):
results=[],
)

# This should really be redefined by both the yum and dnf module but a
# default isn't a bad idea
self.lockfile = '/var/run/yum.pid'

@abstractmethod
def is_lockfile_pid_valid(self):
return

def _is_lockfile_present(self):
return (os.path.isfile(self.lockfile) or glob.glob(self.lockfile)) and self.is_lockfile_pid_valid()

def wait_for_lock(self):
'''Poll until the lock is removed if timeout is a positive number'''

if not self._is_lockfile_present():
return

if self.lock_timeout > 0:
for iteration in range(0, self.lock_timeout):
time.sleep(1)
if not self._is_lockfile_present():
return

self.module.fail_json(msg='{0} lockfile is held by another process'.format(self.pkg_mgr_name))

def listify_comma_sep_strings_in_list(self, some_list):
"""
method to accept a list of strings as the parameter, find any strings
Expand Down
10 changes: 5 additions & 5 deletions lib/ansible/modules/async_status.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,25 +54,25 @@

EXAMPLES = r'''
---
- name: Asynchronous yum task
ansible.builtin.yum:
- name: Asynchronous dnf task
ansible.builtin.dnf:
name: docker-io
state: present
async: 1000
poll: 0
register: yum_sleeper
register: dnf_sleeper
- name: Wait for asynchronous job to end
ansible.builtin.async_status:
jid: '{{ yum_sleeper.ansible_job_id }}'
jid: '{{ dnf_sleeper.ansible_job_id }}'
register: job_result
until: job_result.finished
retries: 100
delay: 10
- name: Clean up async file
ansible.builtin.async_status:
jid: '{{ yum_sleeper.ansible_job_id }}'
jid: '{{ dnf_sleeper.ansible_job_id }}'
mode: cleanup
'''

Expand Down
26 changes: 6 additions & 20 deletions lib/ansible/modules/dnf.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
description:
- By default, this module will select the backend based on the C(ansible_pkg_mgr) fact.
default: "auto"
choices: [ auto, dnf4, dnf5 ]
choices: [ auto, yum, yum4, dnf4, dnf5 ]
type: str
version_added: 2.15
name:
Expand Down Expand Up @@ -206,8 +206,8 @@
version_added: "2.7"
install_repoquery:
description:
- This is effectively a no-op in DNF as it is not needed with DNF, but is an accepted parameter for feature
parity/compatibility with the M(ansible.builtin.yum) module.
- This is effectively a no-op in DNF as it is not needed with DNF.
- This option is deprecated and will be removed in ansible-core 2.20.
type: bool
default: "yes"
version_added: "2.7"
Expand Down Expand Up @@ -261,7 +261,7 @@
- action_common_attributes.flow
attributes:
action:
details: In the case of dnf, it has 2 action plugins that use it under the hood, M(ansible.builtin.yum) and M(ansible.builtin.package).
details: dnf has 2 action plugins that use it under the hood, M(ansible.builtin.dnf) and M(ansible.builtin.package).
support: partial
async:
support: none
Expand Down Expand Up @@ -409,23 +409,13 @@ def __init__(self, module):
super(DnfModule, self).__init__(module)

self._ensure_dnf()
self.lockfile = "/var/cache/dnf/*_lock.pid"
self.pkg_mgr_name = "dnf"

try:
self.with_modules = dnf.base.WITH_MODULES
except AttributeError:
self.with_modules = False

# DNF specific args that are not part of YumDnf
self.allowerasing = self.module.params['allowerasing']
self.nobest = self.module.params['nobest']

def is_lockfile_pid_valid(self):
# FIXME? it looks like DNF takes care of invalid lock files itself?
# https://github.com/ansible/ansible/issues/57189
return True

def _sanitize_dnf_error_msg_install(self, spec, error):
"""
For unhandled dnf.exceptions.Error scenarios, there are certain error
Expand Down Expand Up @@ -467,7 +457,7 @@ def _package_dict(self, package):
'version': package.version,
'repo': package.repoid}

# envra format for alignment with the yum module
# envra format for backwards compat
result['envra'] = '{epoch}:{name}-{version}-{release}.{arch}'.format(**result)

# keep nevra key for backwards compat as it was previously
Expand Down Expand Up @@ -1461,11 +1451,7 @@ def main():
# list=repos
# list=pkgspec

# Extend yumdnf_argument_spec with dnf-specific features that will never be
# backported to yum because yum is now in "maintenance mode" upstream
yumdnf_argument_spec['argument_spec']['allowerasing'] = dict(default=False, type='bool')
yumdnf_argument_spec['argument_spec']['nobest'] = dict(default=False, type='bool')
yumdnf_argument_spec['argument_spec']['use_backend'] = dict(default='auto', choices=['auto', 'dnf4', 'dnf5'])
yumdnf_argument_spec['argument_spec']['use_backend'] = dict(default='auto', choices=['auto', 'yum', 'yum4', 'dnf4', 'dnf5'])

module = AnsibleModule(
**yumdnf_argument_spec
Expand Down
22 changes: 4 additions & 18 deletions lib/ansible/modules/dnf5.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@
validate_certs:
description:
- This is effectively a no-op in the dnf5 module as dnf5 itself handles downloading a https url as the source of the rpm,
but is an accepted parameter for feature parity/compatibility with the M(ansible.builtin.yum) module.
but is an accepted parameter for feature parity/compatibility with the M(ansible.builtin.dnf) module.
type: bool
default: "yes"
sslverify:
Expand All @@ -174,8 +174,8 @@
default: "no"
install_repoquery:
description:
- This is effectively a no-op in DNF as it is not needed with DNF, but is an accepted parameter for feature
parity/compatibility with the M(ansible.builtin.yum) module.
- This is effectively a no-op in DNF as it is not needed with DNF.
- This option is deprecated and will be removed in ansible-core 2.20.
type: bool
default: "yes"
download_only:
Expand Down Expand Up @@ -222,7 +222,7 @@
- action_common_attributes.flow
attributes:
action:
details: In the case of dnf, it has 2 action plugins that use it under the hood, M(ansible.builtin.yum) and M(ansible.builtin.package).
details: dnf5 has 2 action plugins that use it under the hood, M(ansible.builtin.dnf) and M(ansible.builtin.package).
support: partial
async:
support: none
Expand Down Expand Up @@ -408,14 +408,8 @@ def __init__(self, module):
super(Dnf5Module, self).__init__(module)
self._ensure_dnf()

# FIXME https://github.com/rpm-software-management/dnf5/issues/402
self.lockfile = ""
self.pkg_mgr_name = "dnf5"

# DNF specific args that are not part of YumDnf
self.allowerasing = self.module.params["allowerasing"]
self.nobest = self.module.params["nobest"]

def _ensure_dnf(self):
locale = get_best_parsable_locale(self.module)
os.environ["LC_ALL"] = os.environ["LC_MESSAGES"] = locale
Expand Down Expand Up @@ -457,10 +451,6 @@ def _ensure_dnf(self):
failures=[],
)

def is_lockfile_pid_valid(self):
# FIXME https://github.com/rpm-software-management/dnf5/issues/402
return True

def run(self):
if sys.version_info.major < 3:
self.module.fail_json(
Expand Down Expand Up @@ -702,10 +692,6 @@ def run(self):


def main():
# Extend yumdnf_argument_spec with dnf-specific features that will never be
# backported to yum because yum is now in "maintenance mode" upstream
yumdnf_argument_spec["argument_spec"]["allowerasing"] = dict(default=False, type="bool")
yumdnf_argument_spec["argument_spec"]["nobest"] = dict(default=False, type="bool")
Dnf5Module(AnsibleModule(**yumdnf_argument_spec)).run()


Expand Down
Loading

0 comments on commit f024cf3

Please sign in to comment.