[RHELC-297] Clean up kmods functionality (#469)

* Clean up kmods functionality

These changes are cleanup of most of the things that were left behind from
PR#193.

Some of the functions were changed to make it easier to read and debug if
necessary, some of them were moved around to the files that made sense to place
they and some of them just needed a docstring to tell what the function was all
about.

This PR it is introduced a unit test for the function `get_enabled_rhel_repos`
since that was introduced in the past and never had a unit test tied to it.

Jira reference (If any): https://issues.redhat.com/browse/OAMG-4668
Bugzilla reference (If any):

Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>

* Add unit tests for the code refactored

Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>

* Apply integration tests from a59c4bd

Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>

* Apply patch from 3b61461

Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>

* Apply suggestions from Michal's review

Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>

* Edit force loaded kmod test

As discussed with developers, force loaded kmod should inhibit the c2r
run, as it is denoted with (FE) for F = module was force loaded E = unsigned module was loaded.

Signed-off-by: Daniel Diblik <ddiblik@redhat.com>

* Incorporate review suggestions

fix typos

Signed-off-by: Daniel Diblik <ddiblik@redhat.com>

* Fix formatting issue

Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>

* Fix unit_tests

Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>

* Add cmp_to_key for python 2.6 compatibility

Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>

* Update convert2rhel/checks.py

Co-authored-by: Michal Bocek <mbocek@redhat.com>

Signed-off-by: Rodolfo Olivieri <rolivier@redhat.com>
Signed-off-by: Daniel Diblik <ddiblik@redhat.com>
Co-authored-by: Daniel Diblik <ddiblik@redhat.com>
Co-authored-by: Michal Bocek <mbocek@redhat.com>

convert2rhel/checks.py

@@ -15,7 +15,6 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <https://www.gnu.org/licenses/>.
 
-
 import itertools
 import logging
 import os
@@ -29,6 +28,7 @@
 from convert2rhel import __version__ as installed_convert2rhel_version
 from convert2rhel import grub, pkgmanager, utils
 from convert2rhel.pkghandler import (
+    _package_version_cmp,
     call_yum_cmd,
     compare_package_versions,
     get_installed_pkg_objects,
@@ -67,15 +67,54 @@
 PKG_NEVR = r"\b(\S+)-(?:([0-9]+):)?(\S+)-(\S+)\b"
 
 LINK_KMODS_RH_POLICY = "https://access.redhat.com/third-party-software-support"
+LINK_PREVENT_KMODS_FROM_LOADING = "https://access.redhat.com/solutions/41278"
 # The kernel version stays the same throughout a RHEL major version
 COMPATIBLE_KERNELS_VERS = {
     6: "2.6.32",
     7: "3.10.0",
     8: "4.18.0",
 }
 
+# Python 2.6 compatibility.
+# This code is copied from Pthon-3.10's functools module,
+# licensed under the Python Software Foundation License, version 2
+try:
+    from functools import cmp_to_key
+except ImportError:
+
+    def cmp_to_key(mycmp):
+        """Convert a cmp= function into a key= function"""
+
+        class K(object):
+            __slots__ = ["obj"]
+
+            def __init__(self, obj):
+                self.obj = obj
+
+            def __lt__(self, other):
+                return mycmp(self.obj, other.obj) < 0
+
+            def __gt__(self, other):
+                return mycmp(self.obj, other.obj) > 0
+
+            def __eq__(self, other):
+                return mycmp(self.obj, other.obj) == 0
+
+            def __le__(self, other):
+                return mycmp(self.obj, other.obj) <= 0
+
+            def __ge__(self, other):
+                return mycmp(self.obj, other.obj) >= 0
+
+            __hash__ = None
+
+        return K
 
-def perform_pre_checks():
+
+# End of PSF Licensed code
+
+
+def perform_system_checks():
     """Early checks after system facts should be added here."""
 
     check_custom_repos_are_valid()
@@ -232,18 +271,20 @@ def check_tainted_kmods():
         multipath 20480 0 - Live 0x0000000000000000
         linear 20480 0 - Live 0x0000000000000000
         system76_io 16384 0 - Live 0x0000000000000000 (OE)  <<<<<< Tainted
-        system76_acpi 16384 0 - Live 0x0000000000000000 (OE) <<<<< Tainted
+        system76_acpi 16384 0 - Live 0x0000000000000000 (OE) <<<<<< Tainted
     """
     logger.task("Prepare: Checking if loaded kernel modules are not tainted")
     unsigned_modules, _ = run_subprocess(["grep", "(", "/proc/modules"])
     module_names = "\n  ".join([mod.split(" ")[0] for mod in unsigned_modules.splitlines()])
     if unsigned_modules:
         logger.critical(
-            "Tainted kernel module(s) detected. "
+            "Tainted kernel modules detected:\n  {0}\n"
             "Third-party components are not supported per our "
-            "software support policy\n{0}\n\n"
-            "Uninstall or disable the following module(s) and run convert2rhel "
-            "again to continue with the conversion:\n  {1}".format(LINK_KMODS_RH_POLICY, module_names)
+            "software support policy:\n {1}\n"
+            "Prevent the modules from loading by following {2}"
+            " and run convert2rhel again to continue with the conversion.".format(
+                module_names, LINK_KMODS_RH_POLICY, LINK_PREVENT_KMODS_FROM_LOADING
+            )
         )
     logger.info("No tainted kernel module is loaded.")
 
@@ -291,7 +332,9 @@ def check_custom_repos_are_valid():
         return
 
     output, ret_code = call_yum_cmd(
-        command="makecache", args=["-v", "--setopt=*.skip_if_unavailable=False"], print_output=False
+        command="makecache",
+        args=["-v", "--setopt=*.skip_if_unavailable=False"],
+        print_output=False,
     )
     if ret_code != 0:
         logger.critical(
@@ -304,26 +347,33 @@ def check_custom_repos_are_valid():
 
 
 def ensure_compatibility_of_kmods():
-    """Ensure if the host kernel modules are compatible with RHEL."""
+    """Ensure that the host kernel modules are compatible with RHEL.
+
+    :raises SystemExit: Interrupts the conversion because some kernel modules are not supported in RHEL.
+    """
     host_kmods = get_loaded_kmods()
     rhel_supported_kmods = get_rhel_supported_kmods()
     unsupported_kmods = get_unsupported_kmods(host_kmods, rhel_supported_kmods)
-    if unsupported_kmods:
+
+    # Validate the best case first. If we don't have any unsupported_kmods, this means
+    # that everything is compatible and good to go.
+    if not unsupported_kmods:
+        logger.debug("All loaded kernel modules are available in RHEL.")
+    else:
         not_supported_kmods = "\n".join(
             map(
                 lambda kmod: "/lib/modules/{kver}/{kmod}".format(kver=system_info.booted_kernel, kmod=kmod),
                 unsupported_kmods,
             )
         )
         logger.critical(
-            (
-                "The following kernel modules are not supported in RHEL:\n{kmods}\n"
-                "Make sure you have updated the kernel to the latest available version and rebooted the system. "
-                "Remove the unsupported modules and run convert2rhel again to continue with the conversion."
-            ).format(kmods=not_supported_kmods, system=system_info.name)
+            "The following loaded kernel modules are not available in RHEL:\n{0}\n"
+            "First, make sure you have updated the kernel to the latest available version and rebooted the system.\n"
+            "If this message appears again after doing the above, prevent the modules from loading by following {1}"
+            " and run convert2rhel again to continue with the conversion.".format(
+                "\n".join(not_supported_kmods), LINK_PREVENT_KMODS_FROM_LOADING
+            )
         )
-    else:
-        logger.info("Kernel modules are compatible.")
 
 
 def validate_package_manager_transaction():
@@ -421,64 +471,46 @@ def get_most_recent_unique_kernel_pkgs(pkgs):
     kernel pkg do not deprecate kernel modules we only select
     the most recent ones.
 
-    All RHEL kmods packages starts with kernel* or kmod*
-
-    For example, we have the following packages list:
-        kernel-core-0:4.18.0-240.10.1.el8_3.x86_64
-        kernel-core-0:4.19.0-240.10.1.el8_3.x86_64
-        kmod-debug-core-0:4.18.0-240.10.1.el8_3.x86_64
-        kmod-debug-core-0:4.18.0-245.10.1.el8_3.x86_64
-    ==> (output of this function will be)
-        kernel-core-0:4.19.0-240.10.1.el8_3.x86_64
-        kmod-debug-core-0:4.18.0-245.10.1.el8_3.x86_64
-
-    _repos_version_key extract the version of a package
-        into the tuple, i.e.
-        kernel-core-0:4.18.0-240.10.1.el8_3.x86_64 ==>
-        (4, 15, 0, 240, 10, 1)
-
-
-    :type pkgs: Iterable[str]
-    :type pkgs_groups:
-        Iterator[
-            Tuple[
-                package_name_without_version,
-                Iterator[package_name, ...],
-                ...,
-            ]
+    .. note::
+        All RHEL kmods packages starts with kernel* or kmod*
+
+    For example, consider the following list of packages::
+
+        list_of_pkgs = [
+            'kernel-core-0:4.18.0-240.10.1.el8_3.x86_64',
+            'kernel-core-0:4.19.0-240.10.1.el8_3.x86_64',
+            'kmod-debug-core-0:4.18.0-240.10.1.el8_3.x86_64',
+            'kmod-debug-core-0:4.18.0-245.10.1.el8_3.x86_64
         ]
-    """
 
-    pkgs_groups = itertools.groupby(sorted(pkgs), lambda pkg_name: pkg_name.split(":")[0])
-    return tuple(
-        max(distinct_kernel_pkgs[1], key=_repos_version_key)
-        for distinct_kernel_pkgs in pkgs_groups
-        if distinct_kernel_pkgs[0].startswith(("kernel", "kmod"))
-    )
+    And when this function gets called with that same list of packages,
+    we have the following output::
 
+        result = get_most_recent_unique_kernel_pkgs(pkgs=list_of_pkgs)
+        print(result)
+        # (
+        #   'kernel-core-0:4.19.0-240.10.1.el8_3.x86_64',
+        #   'kmod-debug-core-0:4.18.0-245.10.1.el8_3.x86_64'
+        # )
 
-def _repos_version_key(pkg_name):
-    try:
-        rpm_version = KERNEL_REPO_RE.search(pkg_name).group("version")
-    except AttributeError:
-        logger.critical(
-            "Unexpected package:\n%s\n is a source of kernel modules.",
-            pkg_name,
-        )
-    else:
-        return tuple(
-            map(
-                _convert_to_int_or_zero,
-                KERNEL_REPO_VER_SPLIT_RE.split(rpm_version),
-            )
-        )
+    :param pkgs: A list of package names to be analyzed.
+    :type pkgs: list[str]
+    :return: A tuple of packages name sorted and normalized
+    :rtype: tuple[str]
+    """
 
+    pkgs_groups = itertools.groupby(sorted(pkgs), lambda pkg_name: pkg_name.split(":")[0])
+    list_of_sorted_pkgs = []
+    for distinct_kernel_pkgs in pkgs_groups:
+        if distinct_kernel_pkgs[0].startswith(("kernel", "kmod")):
+            list_of_sorted_pkgs.append(
+                max(
+                    distinct_kernel_pkgs[1],
+                    key=cmp_to_key(_package_version_cmp),
+                )
+            )
 
-def _convert_to_int_or_zero(s):
-    try:
-        return int(s)
-    except ValueError:
-        return 0
+    return tuple(list_of_sorted_pkgs)
 
 
 def get_rhel_kmods_keys(rhel_kmods_str):
@@ -492,11 +524,19 @@ def get_rhel_kmods_keys(rhel_kmods_str):
 
 
 def get_unsupported_kmods(host_kmods, rhel_supported_kmods):
-    """Return a set of those installed kernel modules that are not available in RHEL repositories.
+    """Return a set of full paths to those installed kernel modules that are
+    not available in RHEL repositories.
 
-    Ignore certain kmods mentioned in the system configs. These kernel modules moved to kernel core, meaning that the
-    functionality is retained and we would be incorrectly saying that the modules are not supported in RHEL."""
-    return host_kmods - rhel_supported_kmods - set(system_info.kmods_to_ignore)
+    Ignore certain kmods mentioned in the system configs. These kernel modules
+    moved to kernel core, meaning that the functionality is retained and we
+    would be incorrectly saying that the modules are not supported in RHEL.
+    """
+    unsupported_kmods_subpaths = host_kmods - rhel_supported_kmods - set(system_info.kmods_to_ignore)
+    unsupported_kmods_full_paths = [
+        "/lib/modules/{kver}/{kmod}".format(kver=system_info.booted_kernel, kmod=kmod)
+        for kmod in unsupported_kmods_subpaths
+    ]
+    return unsupported_kmods_full_paths
 
 
 def check_rhel_compatible_kernel_is_used():
@@ -557,7 +597,10 @@ def _bad_kernel_version(kernel_release):
 def _bad_kernel_package_signature(kernel_release):
     """Return True if the booted kernel is not signed by the original OS vendor, i.e. it's a custom kernel."""
     vmlinuz_path = "/boot/vmlinuz-%s" % kernel_release
-    kernel_pkg, return_code = run_subprocess(["rpm", "-qf", "--qf", "%{NAME}", vmlinuz_path], print_output=False)
+    kernel_pkg, return_code = run_subprocess(
+        ["rpm", "-qf", "--qf", "%{NAME}", vmlinuz_path],
+        print_output=False,
+    )
     logger.debug("Booted kernel package name: {0}".format(kernel_pkg))
 
     os_vendor = system_info.name.split()[0]

convert2rhel/main.py

@@ -97,7 +97,7 @@ def main():
         pkghandler.clean_yum_metadata()
 
         # check the system prior the conversion (possible inhibit)
-        checks.perform_pre_checks()
+        checks.perform_system_checks()
 
         # backup system release file before starting conversion process
         loggerinst.task("Prepare: Backup System")

convert2rhel/pkghandler.py

@@ -1111,3 +1111,54 @@ def clean_yum_metadata():
         return
 
     loggerinst.info("Cached yum metadata cleaned successfully.")
+
+
+def _package_version_cmp(pkg_1, pkg_2):
+    """Compare the version key in a given package name.
+
+    Consider the following variables that will be passed to this function::
+
+        pkg_1 = 'kernel-core-0:4.18.0-240.10.1.el8_3.x86_64'
+        pkg_2 = 'kernel-core-0:4.18.0-239.0.0.el8_3.x86_64'
+
+    The output of this will be a tuple containing the package version in a
+    tuple::
+
+        result = _package_version_cmp(pkg_1, pkg_2)
+        print("Result is: %s" % result)
+        # Result is: -1
+
+    The function will ignore the package name as it is not an important
+    information here and will only care about the version that is tied to it's
+    name.
+
+    :param pkg_1: The first package to extract the version
+    :type pkg_1: str
+    :param pkg_2: The second package to extract the version
+    :type pkg_2: str
+    :return: An integer resulting in the package comparision
+    :rtype: int
+    """
+
+    # TODO(r0x0d): This function still needs some enhancements code-wise, it
+    # workes perfectly, but the way we are handling the versions is not 100%
+    # complete yet.  will be done in a future work. Right now, all the list of
+    # changes are listed in this comment:
+    # https://github.com/oamg/convert2rhel/pull/469#discussion_r873971400
+    pkg_ver_components = []
+    for pkg in pkg_1, pkg_2:
+        # Remove the package name and split the rest between epoch + version
+        # and release + arch
+        epoch_version, release_arch = pkg.rsplit("-", 2)[-2:]
+        # Separate the (optional) epoch from the version
+        epoch_version = epoch_version.split(":", 1)
+        if len(epoch_version) == 1:
+            epoch = "0"
+            version = epoch_version[0]
+        else:
+            epoch, version = epoch_version
+        # Discard the arch
+        release = release_arch.rsplit(".", 1)[0]
+        pkg_ver_components.append((epoch, version, release))
+
+    return rpm.labelCompare(pkg_ver_components[0], pkg_ver_components[1])

convert2rhel/systeminfo.py

@@ -332,12 +332,18 @@ def is_rpm_installed(name):
         _, return_code = run_subprocess(["rpm", "-q", name], print_cmd=False, print_output=False)
         return return_code == 0
 
-    # TODO write unit tests
     def get_enabled_rhel_repos(self):
-        """Return a tuple of repoids containing RHEL packages.
+        """Get a list of enabled repositories containing RHEL packages.
 
-        These are either the repos enabled through RHSM or the custom
-        repositories passed though CLI.
+        This function can return either the repositories enabled throught the RHSM tool during the conversion or, if
+        the user manually specified the repositories throught the CLI, it will return them based on the
+        `tool_opts.enablerepo` option.
+
+        .. note::
+            The repositories passed through the CLI have more priority than the ones get get from RHSM.
+
+        :return: A list of enabled repos to use during the conversion
+        :rtype: list[str]
         """
         # TODO:
         # if not self.submgr_enabled_repos: