[RHELC-1753] Fix reaching unaccessible repos from enablerepo (#1425)

* [RHELC-1753] Fix reaching unaccessible repos from enablerepo

Avoid reaching unaccessible repositories provided by user using
--enablerepo argument. Those repos are manually disabled during
analysis, but when user provides unreachable repo the analysis would
fail.

This change adds check if provided repos are accessible - if not, those
unaccessible ones are removed from the list for manual disable during
analysis.

* Disable repo code cleanup

---------

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

convert2rhel/actions/pre_ponr_changes/handle_packages.py

@@ -43,9 +43,9 @@ def run(self):
         if third_party_pkgs:
             # RHELC-884 disable the RHEL repos to avoid reaching them when checking original system.
             # There is needed for avoid reaching out RHEL repositories while requesting info about pkgs.
-            disable_repos = repo.get_rhel_repos_to_disable()
+            repos_to_disable = repo.DisableReposDuringAnalysis().get_rhel_repos_to_disable()
             pkg_list = pkghandler.format_pkg_info(
-                sorted(third_party_pkgs, key=self.extract_packages), disable_repos=disable_repos
+                sorted(third_party_pkgs, key=self.extract_packages), disable_repos=repos_to_disable
             )
             warning_message = (
                 "Only packages signed by {} are to be"
@@ -130,8 +130,8 @@ def run(self):
 
             # RHELC-884 disable the RHEL repos to avoid reaching them when checking original system.
             # There is needed for avoid reaching out RHEL repositories while requesting info about pkgs.
-            disable_repos = repo.get_rhel_repos_to_disable()
-            pkgs_removed = _remove_packages_unless_from_redhat(pkgs_list=all_pkgs, disable_repos=disable_repos)
+            repos_to_disable = repo.DisableReposDuringAnalysis().get_rhel_repos_to_disable()
+            pkgs_removed = _remove_packages_unless_from_redhat(pkgs_list=all_pkgs, disable_repos=repos_to_disable)
 
             # https://issues.redhat.com/browse/RHELC-1677
             # In some cases the {system}-release package takes ownership of the /etc/yum.repos.d/ directory,

convert2rhel/actions/system_checks/is_loaded_kernel_latest.py

@@ -45,7 +45,8 @@ def run(self):
             return
 
         # RHELC-884 disable the RHEL repos to avoid reaching them when checking original system.
-        disable_repo_command = repo.get_rhel_disable_repos_command(repo.get_rhel_repos_to_disable())
+        repos_to_disable = repo.DisableReposDuringAnalysis().get_rhel_repos_to_disable()
+        disable_repo_command = repo.get_rhel_disable_repos_command(repos_to_disable)
 
         cmd = [
             "repoquery",

convert2rhel/backup/packages.py

@@ -60,7 +60,7 @@ def __init__(
         self.varsdir = varsdir
 
         # RHELC-884 disable the RHEL repos to avoid downloading pkg from them.
-        self.disable_repos = disable_repos or repo.get_rhel_repos_to_disable()
+        self.disable_repos = disable_repos or repo.DisableReposDuringAnalysis().get_rhel_repos_to_disable()
 
         self._backedup_pkgs_paths = []
 

convert2rhel/pkghandler.py

@@ -734,12 +734,12 @@ def get_total_packages_to_update():
     # RHEL repositories needs to be disabled when checking if the packages are up to date.
     # When user specifies rhel repos to use during conversion, disabling them is also needed.
     # Note: Do not depend on the --no-rhsm option, enable repos can be specified without it.
-    disable_repos = repo.get_rhel_repos_to_disable()
+    repos_to_disable = repo.DisableReposDuringAnalysis().get_rhel_repos_to_disable()
 
     if pkgmanager.TYPE == "yum":
-        packages = _get_packages_to_update_yum(disable_repos=disable_repos)
+        packages = _get_packages_to_update_yum(disable_repos=repos_to_disable)
     elif pkgmanager.TYPE == "dnf":
-        packages = _get_packages_to_update_dnf(disable_repos=disable_repos)
+        packages = _get_packages_to_update_dnf(disable_repos=repos_to_disable)
 
     return set(packages)
 

convert2rhel/pkgmanager/__init__.py

@@ -243,6 +243,7 @@ def call_yum_cmd(
         # When using subscription-manager for the conversion, use those repos for the yum call that have been enabled
         # through subscription-manager
         repos_to_enable = system_info.get_enabled_rhel_repos()
+        logger.debug("Custom epos in yum cmd: {repos_to_enable}".format(repos_to_enable=repos_to_enable))
 
     for repo in repos_to_enable:
         cmd.append("--enablerepo={}".format(repo))

convert2rhel/repo.py

@@ -19,6 +19,7 @@
 
 
 import tempfile
+import re
 
 from contextlib import closing
 
@@ -29,6 +30,7 @@
 from convert2rhel.systeminfo import system_info
 from convert2rhel.toolopts import tool_opts
 from convert2rhel.utils import TMP_DIR, store_content_to_file
+from convert2rhel.pkgmanager import TYPE, call_yum_cmd
 
 
 DEFAULT_YUM_REPOFILE_DIR = "/etc/yum.repos.d"
@@ -58,34 +60,110 @@ def get_rhel_repoids():
     return repos_needed
 
 
-def get_rhel_repos_to_disable():
-    """Get the list of repositories which should be disabled when performing pre-conversion checks.
+class DisableReposDuringAnalysis(object):
+    _instance = None
+    _repos_to_disable = None
 
-    Avoid downloading backup and up-to-date checks from them. The output list can looks like: ["rhel*"]
+    def __new__(cls):
+        """Singleton pattern"""
+        if cls._instance is None:
+            cls._instance = super(DisableReposDuringAnalysis, cls).__new__(cls)
+            # Cannot call the _set_rhel_repos_to_disable() directly due Python 2 support
+            cls._instance._initialized = False
 
-    .. note::
-        If --enablerepo switch is used together with the --no-rhsm, we will return a combination of repositories to disable as following:
+        return cls._instance
 
-        >>> # tool_opts.enablerepo comes from the CLI option `--enablerepo`.
-        >>> repos_to_disable = ["rhel*"]
-        >>> repos_to_disable.extend(tool_opts.enablerepo) # returns: ["rhel*", "my-rhel-repo-mirror"]
+    def __init__(self):
+        if self._initialized:
+            return
 
-    :return: List of repositories to disable when performing checks.
-    :rtype: list[str]
+        self._set_rhel_repos_to_disable()
+        self._instance._initialized = True
+
+    def _set_rhel_repos_to_disable(self):
+        """Set the list of repositories which should be disabled when performing pre-conversion checks.
+
+        Avoid using RHEL repos for certain re-conversion analysis phase operations such as:
+         - downloading a package backup
+         - the package up-to-date check
+         - querying what repository local packages have been installed from
+         - the latest available kernel check
+        Only the original system vendor repos should be used for these pre-conversion analysis phase operations.
+
+        .. note::
+            If --enablerepo switch is used together with the --no-rhsm, we will return a combination of repositories to
+            disable as following:
+
+            >>> # tool_opts.enablerepo comes from the CLI option `--enablerepo`.
+            >>> self.repos_to_disable = ["rhel*"]
+            >>> self.repos_to_disable.extend(tool_opts.enablerepo) # returns: ["rhel*", "my-rhel-repo-mirror"]
+
+        :return: List of repoids to disable, such as ["rhel*", "my-optional-repo"]
+        :rtype: List
+        """
+        # RHELC-884 disable the RHEL repos to avoid reaching them when checking original system.
+        self._repos_to_disable = ["rhel*"]
+
+        # this is for the case where the user configures e.g. [my-rhel-repo-mirror] on the system and leaves it enabled
+        # before running convert2rhel - we want to prevent the checks from accessing it as it contains packages for RHEL
+        if tool_opts.no_rhsm and tool_opts.enablerepo:
+            logger.debug("Preparing a list of RHEL repositories to be disabled during analysis.")
+            self._set_custom_repos()
+
+        return self._repos_to_disable
+
+    def _set_custom_repos(self):
+        """If we are using the YUM pkg manager, we need to check if all custom repositories provided by the user through
+        --enablerepo are accessible. DNF package manager can handle situation of unreachable repo by skipping it."""
+        if TYPE == "dnf":
+            self._repos_to_disable.extend(tool_opts.enablerepo)
+            return
+
+        # pkg manager is yum
+        # copy the enablerepo list to avoid changing the original one
+        repos_to_check = list(tool_opts.enablerepo)
+        self._repos_to_disable.extend(_get_valid_custom_repos(repos_to_check))
+
+    def get_rhel_repos_to_disable(self):
+        """See the docstring of _set_rhel_repos_to_disable for details about the repos to disable."""
+        return self._repos_to_disable
+
+
+def _get_valid_custom_repos(repos_to_check):
+    """Check if provided repo IDs are accessible. The function is recursive.
+
+    :arg repos_to_check: Repo IDs to be checked
+    :arg type: List
+    :return: Accessible repositories
+    :rtype: List
     """
-    # RHELC-884 disable the RHEL repos to avoid reaching them when checking original system.
-    repos_to_disable = ["rhel*"]
+    if not repos_to_check:
+        return []
 
-    # this is for the case where the user configures e.g. [my-rhel-repo-mirror] on the system and leaves it enabled
-    # before running convert2rhel - we want to prevent the checks from accessing it as it contains packages for RHEL
-    if tool_opts.no_rhsm:
-        repos_to_disable.extend(tool_opts.enablerepo)
+    args = ["-v", "--setopt=*.skip_if_unavailable=False"]
+    output, ret_code = call_yum_cmd(
+        command="makecache", args=args, print_output=False, disable_repos=repos_to_check, enable_repos=[]
+    )
+
+    if ret_code:
+        reponame_regex = r"Error getting repository data for ([^,]+),"
+        problematic_reponame_line = re.search(reponame_regex, output)
+        if problematic_reponame_line:
+            reponame = problematic_reponame_line.group(1)
+            logger.debug(
+                "Removed the {reponame} repository from the list of repositories to disable in certain"
+                " pre-conversion analysis checks as it is inaccessible at the moment and yum fails when trying to"
+                " disable an inaccessible repository.".format(reponame=reponame)
+            )
+            repos_to_check.remove(reponame)
+            return _get_valid_custom_repos(repos_to_check)
 
-    return repos_to_disable
+    # The list of repositories passed to the function sans the inaccessible ones
+    return repos_to_check
 
 
 def get_rhel_disable_repos_command(disable_repos):
-    """Build command containing all the repos for disable. The result looks like
+    """Build command containing all the repos to disable. The result looks like
     '--disablerepo repo --disablerepo repo1 --disablerepo repo2'
     If provided list is empty, empty string is returned.
 

convert2rhel/unit_tests/repo_test.py

@@ -77,24 +77,6 @@ def test_get_rhel_repoids_el7(pretend_os, is_els_release, expected, monkeypatch)
     assert repos == expected
 
 
-@pytest.mark.parametrize(
-    ("no_rhsm", "enablerepo", "disablerepos"),
-    (
-        (False, [], ["rhel*"]),
-        (True, ["test-repo"], ["rhel*", "test-repo"]),
-        (True, [], ["rhel*"]),
-        (False, ["test-repo"], ["rhel*"]),
-    ),
-)
-def test_get_rhel_repos_to_disable(monkeypatch, global_tool_opts, no_rhsm, enablerepo, disablerepos):
-    monkeypatch.setattr(repo, "tool_opts", global_tool_opts)
-    global_tool_opts.enablerepo = enablerepo
-    global_tool_opts.no_rhsm = no_rhsm
-
-    repos = repo.get_rhel_repos_to_disable()
-    assert repos == disablerepos
-
-
 @pytest.mark.parametrize(
     ("disable_repos", "command"),
     (
@@ -189,3 +171,113 @@ def test_write_temporary_repofile_store_failure(tmpdir, monkeypatch):
     assert "STORE_REPOFILE_FAILED" in execinfo._excinfo[1].id
     assert "Failed to store a repository file" in execinfo._excinfo[1].title
     assert "test" in execinfo._excinfo[1].description
+
+
+class TestDisableReposDuringAnalysis:
+    def test_singleton(self, monkeypatch):
+        """Test if the singleton works properly and only 1 instance is created."""
+        # Force remove the singleton instance
+        repo.DisableReposDuringAnalysis._instance = None
+
+        set_rhel_repos_to_disable = mock.Mock()
+        monkeypatch.setattr(repo.DisableReposDuringAnalysis, "_set_rhel_repos_to_disable", set_rhel_repos_to_disable)
+
+        singleton1 = repo.DisableReposDuringAnalysis()
+        singleton2 = repo.DisableReposDuringAnalysis()
+
+        assert singleton1 is singleton2
+        assert set_rhel_repos_to_disable.call_count == 1
+
+        # Force remove the singleton instance
+        repo.DisableReposDuringAnalysis._instance = None
+
+    @pytest.mark.parametrize(
+        ("disable_repos", "yum_output", "result"),
+        (
+            (
+                ["repo1", "repo2", "repo3"],
+                [
+                    (
+                        """Loading "fastestmirror" plugin
+                        Config time: 0.003
+
+
+                        Error getting repository data for repo1, repository not found""",
+                        1,
+                    ),
+                    ("", 0),
+                ],
+                ["repo2", "repo3"],
+            ),
+            ([], "Some testing output, won't be called", []),
+            (["repo1", "repo2"], [("Message causing exit code 1", 1)], ["repo1", "repo2"]),
+            (
+                ["repo1"],
+                [("All fine", 0)],
+                ["repo1"],
+            ),
+        ),
+    )
+    def test_get_valid_custom_repos(self, yum_output, disable_repos, result, monkeypatch):
+        """Test parsing of the yum output and getting the unavailable repositories."""
+        monkeypatch.setattr(repo, "call_yum_cmd", mock.Mock(side_effect=yum_output))
+
+        output = repo._get_valid_custom_repos(disable_repos)
+
+        assert output == result
+
+    @pytest.mark.parametrize(
+        ("no_rhsm", "enablerepo", "disablerepos"),
+        (
+            (False, [], ["rhel*"]),
+            (True, ["test-repo"], ["rhel*", "test-repo"]),
+            (True, [], ["rhel*"]),
+            (False, ["test-repo"], ["rhel*"]),
+        ),
+    )
+    def test_get_rhel_repos_to_disable_dnf(self, monkeypatch, global_tool_opts, no_rhsm, enablerepo, disablerepos):
+        """Test getting the repositories to be disabled on systems with DNF. On DNF there is no need to check if
+        repositories for disabling are accessible. The package manager handles it well.
+        """
+        # Force remove the singleton instance
+        repo.DisableReposDuringAnalysis._instance = None
+
+        monkeypatch.setattr(repo, "tool_opts", global_tool_opts)
+        monkeypatch.setattr(repo, "TYPE", "dnf")
+        global_tool_opts.enablerepo = enablerepo
+        global_tool_opts.no_rhsm = no_rhsm
+
+        repos = repo.DisableReposDuringAnalysis().get_rhel_repos_to_disable()
+        assert repos == disablerepos
+
+        # Force remove the singleton instance
+        repo.DisableReposDuringAnalysis._instance = None
+
+    @pytest.mark.parametrize(
+        ("no_rhsm", "enablerepo", "disablerepos"),
+        (
+            (False, [], ["rhel*"]),
+            (True, ["test-repo"], ["rhel*", "test-repo"]),
+            (True, [], ["rhel*"]),
+            (False, ["test-repo"], ["rhel*"]),
+        ),
+    )
+    def test_get_rhel_repos_to_disable_yum(self, monkeypatch, global_tool_opts, no_rhsm, enablerepo, disablerepos):
+        """Test getting the repositories to be disabled on systems with YUM. With YUM all the repositories needs
+        to be accessible.
+        """
+        # Force remove the singleton instance
+        repo.DisableReposDuringAnalysis._instance = None
+
+        monkeypatch.setattr(repo, "tool_opts", global_tool_opts)
+        monkeypatch.setattr(repo, "TYPE", "yum")
+        # Set the output of yum makecache empty (no problem, simulating the inaccessible repo is in different test)
+        monkeypatch.setattr(repo, "call_yum_cmd", mock.Mock(return_value=("", 0)))
+        global_tool_opts.enablerepo = enablerepo
+        global_tool_opts.no_rhsm = no_rhsm
+
+        repos = repo.DisableReposDuringAnalysis().get_rhel_repos_to_disable()
+        assert repos == disablerepos
+
+        # Force remove the singleton instance
+        repo.DisableReposDuringAnalysis._instance = None

tests/integration/tier0/non-destructive/custom-repository/test_custom_repository.py

@@ -44,7 +44,9 @@ def test_custom_invalid_repo_without_rhsm(shell, convert2rhel):
     with invalid repository passed.
     Make sure that after failed repo check there is a kernel installed.
     """
-    with convert2rhel("-y --no-rhsm --enablerepo fake-rhel-8-for-x86_64-baseos-rpms --debug", unregister=True) as c2r:
+    with convert2rhel(
+        "-y --no-rhsm --enablerepo fake-rhel-repo-1 --enablerepo fake-rhel-repo-2 --debug", unregister=True
+    ) as c2r:
         c2r.expect("CUSTOM_REPOSITORIES_ARE_VALID::UNABLE_TO_ACCESS_REPOSITORIES")
 
     assert c2r.exitstatus == 2