diff --git a/repos/system_upgrade/common/actors/targetuserspacecreator/libraries/repoinfo.py b/repos/system_upgrade/common/actors/targetuserspacecreator/libraries/repoinfo.py new file mode 100644 index 0000000000..e90a0e9d21 --- /dev/null +++ b/repos/system_upgrade/common/actors/targetuserspacecreator/libraries/repoinfo.py @@ -0,0 +1,97 @@ +from collections import defaultdict + +from leapp.libraries.common import repofileutils +from leapp.libraries.stdlib import api +from leapp.models import TargetRepositories + +DEFAULT_RHSM_REPOFILE = '/etc/yum.repos.d/redhat.repo' + +REPO_KIND_RHUI = 'rhui' +REPO_KIND_RHSM = 'rhsm' +REPO_KIND_CUSTOM = 'custom' + + +class _RequestedRepoIDs(object): + def __init__(self): + self.rhel_repos = set() + self.custom_repos = set() + + @property + def combined(self): + return self.rhel_repos | self.custom_repos + + +def _get_requested_repo_ids(): + """ + Get all requested target repositories. + """ + result = _RequestedRepoIDs() + for msg in api.consume(TargetRepositories): + result.rhel_repos.update({r.repoid for r in msg.rhel_repos}) + result.custom_repos.update({r.repoid for r in msg.custom_repos}) + return result + + +class RepositoryInformation(object): + def __init__(self, context, cloud_repo=None): + self.repos = [] + self.rfiles = [] + self.mapped = defaultdict(list) + self.repo_type_map = defaultdict(set) + self._target_repo_ids = _get_requested_repo_ids() + self._load_repofiles(context=context, cloud_repo=cloud_repo) + + def _load_repofiles(self, context, cloud_repo=None): + for rfile in repofileutils.get_parsed_repofiles( + context=context, + kind_resolve=resolve_repo_file_kind(cloud_repo)): + self.add_file(rfile) + + @property + def rhsm_repoids(self): + return self.repo_type_map[REPO_KIND_RHSM] + + @property + def rhui_repoids(self): + return self.repo_type_map[REPO_KIND_RHUI] + + @property + def rhel_repoids(self): + return self.rhsm_repoids | self.rhui_repoids + + @property + def custom_repoids(self): + return self.repo_type_map[REPO_KIND_CUSTOM] + + @property + def missing_custom_repoids(self): + return self._target_repo_ids.custom_repos - self.custom_repoids + + @property + def target_repoids(self): + return (self._target_repo_ids.custom_repos & self.custom_repoids) | ( + self._target_repo_ids.rhel_repos & self.rhel_repoids) + + def add_file(self, rfile): + self.rfiles.append(rfile) + for repo in rfile.data: + self.add(repo) + + def add(self, repo): + self.repos.append(repo) + self.mapped[repo.repoid].append(repo) + self.repo_type_map[repo.kind].add(repo.repoid) + + @property + def duplicated_repoids(self): + return {k: v for k, v in self.mapped.items() if len(v) > 1} + + +def resolve_repo_file_kind(cloud_repo): + def resolver(path): + if path == DEFAULT_RHSM_REPOFILE: + return REPO_KIND_RHSM + if cloud_repo and path == cloud_repo: + return REPO_KIND_RHUI + return REPO_KIND_CUSTOM + return resolver diff --git a/repos/system_upgrade/common/actors/targetuserspacecreator/libraries/repos.py b/repos/system_upgrade/common/actors/targetuserspacecreator/libraries/repos.py new file mode 100644 index 0000000000..56d5835f85 --- /dev/null +++ b/repos/system_upgrade/common/actors/targetuserspacecreator/libraries/repos.py @@ -0,0 +1,34 @@ +import os + +from leapp.libraries.actor import repoinfo +from leapp.libraries.common import rhsm, rhui + + +def _install_custom_repofiles(context, custom_repofiles): + """ + Install the required custom repository files into the container. + + The repository files are copied from the host into the /etc/yum.repos.d + directory into the container. + + :param context: the container where the repofiles should be copied + :type context: mounting.IsolatedActions class + :param custom_repofiles: list of custom repo files + :type custom_repofiles: List(CustomTargetRepositoryFile) + """ + for rfile in custom_repofiles: + _dst_path = os.path.join('/etc/yum.repos.d', os.path.basename(rfile.file)) + context.copy_to(rfile.file, _dst_path) + + +def prepare_repository_collection(context, indata, prod_cert_path): + rhsm.set_container_mode(context) + rhsm.switch_certificate(context, indata.rhsm_info, prod_cert_path) + if indata.rhui_info: + rhui.copy_rhui_data(context, indata.rhui_info.provider) + _install_custom_repofiles(context, indata.custom_repofiles) + + +def collect_repositories(context, cloud_repo=None): + info = repoinfo.RepositoryInformation(context, cloud_repo) + return info diff --git a/repos/system_upgrade/common/actors/targetuserspacecreator/libraries/userspacegen.py b/repos/system_upgrade/common/actors/targetuserspacecreator/libraries/userspacegen.py index f2391ee83c..c4ff3b1a4a 100644 --- a/repos/system_upgrade/common/actors/targetuserspacecreator/libraries/userspacegen.py +++ b/repos/system_upgrade/common/actors/targetuserspacecreator/libraries/userspacegen.py @@ -3,7 +3,7 @@ from leapp import reporting from leapp.exceptions import StopActorExecution, StopActorExecutionError -from leapp.libraries.actor import constants +from leapp.libraries.actor import constants, repos from leapp.libraries.common import dnfplugin, mounting, overlaygen, repofileutils, rhsm, rhui, utils from leapp.libraries.common.config import get_env, get_product_type from leapp.libraries.common.config.version import get_target_major_version @@ -18,7 +18,6 @@ RHUIInfo, StorageInfo, TargetOSInstallationImage, - TargetRepositories, TargetUserSpaceInfo, TargetUserSpacePreupgradeTasks, UsedTargetRepositories, @@ -431,23 +430,19 @@ def _create_target_userspace_directories(target_userspace): ) -def _inhibit_on_duplicate_repos(repofiles): +def _inhibit_on_duplicate_repos(repoinfo): """ Inhibit the upgrade if any repoid is defined multiple times. When that happens, it not only shows misconfigured system, but then we can't get details of all the available repos as well. """ - # TODO: this is is duplicate of rhsm._inhibit_on_duplicate_repos - # Issue: #486 - duplicates = repofileutils.get_duplicate_repositories(repofiles).keys() - - if not duplicates: + if not repoinfo.duplicated_repoids: return list_separator_fmt = '\n - ' api.current_logger().warning( 'The following repoids are defined multiple times:{0}{1}' - .format(list_separator_fmt, list_separator_fmt.join(duplicates)) + .format(list_separator_fmt, list_separator_fmt.join(repoinfo.duplicated_repoids.keys())) ) reporting.create_report([ @@ -455,7 +450,7 @@ def _inhibit_on_duplicate_repos(repofiles): reporting.Summary( 'The following repositories are defined multiple times inside the' ' "upgrade" container:{0}{1}' - .format(list_separator_fmt, list_separator_fmt.join(duplicates)) + .format(list_separator_fmt, list_separator_fmt.join(repoinfo.duplicated_repoids.keys())) ), reporting.Severity(reporting.Severity.MEDIUM), reporting.Groups([reporting.Groups.REPOSITORY]), @@ -469,22 +464,7 @@ def _inhibit_on_duplicate_repos(repofiles): ]) -def _get_all_available_repoids(context): - repofiles = repofileutils.get_parsed_repofiles(context) - # TODO: this is not good solution, but keep it as it is now - # Issue: #486 - if rhsm.skip_rhsm(): - # only if rhsm is skipped, the duplicate repos are not detected - # automatically and we need to do it extra - _inhibit_on_duplicate_repos(repofiles) - repoids = [] - for rfile in repofiles: - if rfile.data: - repoids += [repo.repoid for repo in rfile.data] - return set(repoids) - - -def _get_rhsm_available_repoids(context): +def _check_available_rhel_repos(repoinfo): target_major_version = get_target_major_version() # FIXME: check that required repo IDs (baseos, appstream) # + or check that all required RHEL repo IDs are available. @@ -493,8 +473,7 @@ def _get_rhsm_available_repoids(context): # Get the RHSM repos available in the target RHEL container # TODO: very similar thing should happens for all other repofiles in container # - repoids = rhsm.get_available_repo_ids(context) - if not repoids or len(repoids) < 2: + if len(repoinfo.rhel_repoids) < 2: reporting.create_report([ reporting.Title('Cannot find required basic RHEL target repositories.'), reporting.Summary( @@ -520,94 +499,22 @@ def _get_rhsm_available_repoids(context): url='https://red.ht/preparing-for-upgrade-to-rhel8', title='Preparing for the upgrade') ]) + api.current_actor().show_message( + 'Cannot find required basic RHEL target repositories. Available: {}'.format( + ', '.join(repoinfo.rhel_repoids) + )) raise StopActorExecution() - return set(repoids) - - -def _get_rhui_available_repoids(context, cloud_repo): - repofiles = repofileutils.get_parsed_repofiles(context) - - # TODO: same refactoring as Issue #486? - _inhibit_on_duplicate_repos(repofiles) - repoids = [] - for rfile in repofiles: - if rfile.file == cloud_repo and rfile.data: - repoids = [repo.repoid for repo in rfile.data] - repoids.sort() - break - return set(repoids) - - -def _get_rh_available_repoids(context, indata): - """ - RH repositories are provided either by RHSM or are stored in the expected repo file provided by - RHUI special packages (every cloud provider has itw own rpm). - """ - - upg_path = rhui.get_upg_path() - - rh_repoids = _get_rhsm_available_repoids(context) - - if indata and indata.rhui_info: - cloud_repo = os.path.join( - '/etc/yum.repos.d/', rhui.RHUI_CLOUD_MAP[upg_path][indata.rhui_info.provider]['leapp_pkg_repo'] - ) - rhui_repoids = _get_rhui_available_repoids(context, cloud_repo) - rh_repoids.update(rhui_repoids) - - return rh_repoids - -def gather_target_repositories(context, indata): - """ - Get available required target repositories and inhibit or raise error if basic checks do not pass. - In case of repositories provided by Red Hat, it's checked whether the basic - required repositories are available (or at least defined) in the given - context. If not, raise StopActorExecutionError. - - For the custom target repositories we expect all of them have to be defined. - If any custom target repository is missing, raise StopActorExecutionError. - - If any repository is defined multiple times, produce the inhibitor Report - msg. - - :param context: An instance of a mounting.IsolatedActions class - :type context: mounting.IsolatedActions class - :return: List of target system repoids - :rtype: List(string) - """ - rh_available_repoids = _get_rh_available_repoids(context, indata) - all_available_repoids = _get_all_available_repoids(context) - - target_repoids = [] - missing_custom_repoids = [] - for target_repo in api.consume(TargetRepositories): - for rhel_repo in target_repo.rhel_repos: - if rhel_repo.repoid in rh_available_repoids: - target_repoids.append(rhel_repo.repoid) - else: - # TODO: We shall report that the RHEL repos that we deem necessary for - # the upgrade are not available; but currently it would just print bunch of - # data every time as we maps EUS and other repositories as well. But these - # do not have to be necessary available on the target system in the time - # of the upgrade. Let's skip it for now until it's clear how we will deal - # with it. - pass - for custom_repo in target_repo.custom_repos: - if custom_repo.repoid in all_available_repoids: - target_repoids.append(custom_repo.repoid) - else: - missing_custom_repoids.append(custom_repo.repoid) - api.current_logger().debug("Gathered target repositories: {}".format(', '.join(target_repoids))) - if not target_repoids: +def _check_available_rhsm_repos(repoinfo): + if not repoinfo.target_repoids: reporting.create_report([ reporting.Title('There are no enabled target repositories'), reporting.Summary( 'This can happen when a system is not correctly registered with the subscription manager' ' or, when the leapp --no-rhsm option has been used, no custom repositories have been' ' passed on the command line.' - ), + ), reporting.Groups([reporting.Groups.REPOSITORY]), reporting.Groups([reporting.Groups.INHIBITOR]), reporting.Severity(reporting.Severity.HIGH), @@ -620,16 +527,19 @@ def gather_target_repositories(context, indata): ' repositories are defined in any repofiles under the /etc/yum.repos.d/ directory.' ' For more information on custom repository files, see the documentation.' ' Finally, verify that the "/etc/leapp/files/repomap.json" file is up-to-date.' - ).format(version=api.current_actor().configuration.version.target)), + ).format(version=api.current_actor().configuration.version.target)), reporting.ExternalLink( # TODO: How to handle different documentation links for each version? url='https://red.ht/preparing-for-upgrade-to-rhel8', title='Preparing for the upgrade'), reporting.RelatedResource("file", "/etc/leapp/files/repomap.json"), reporting.RelatedResource("file", "/etc/yum.repos.d/") - ]) + ]) raise StopActorExecution() - if missing_custom_repoids: + + +def _check_available_custom_repos(repoinfo): + if repoinfo.missing_custom_repoids: reporting.create_report([ reporting.Title('Some required custom target repositories have not been found'), reporting.Summary( @@ -637,7 +547,7 @@ def gather_target_repositories(context, indata): ' while using the --enablerepo option of leapp, or in a third party actor that produces a' ' CustomTargetRepositoryMessage.\n' 'The following repositories IDs could not be found in the target configuration:\n' - '- {}\n'.format('\n- '.join(missing_custom_repoids)) + '- {}\n'.format('\n- '.join(repoinfo.missing_custom_repoids)) ), reporting.Groups([reporting.Groups.REPOSITORY]), reporting.Groups([reporting.Groups.INHIBITOR]), @@ -654,47 +564,70 @@ def gather_target_repositories(context, indata): )) ]) raise StopActorExecution() - return set(target_repoids) -def _install_custom_repofiles(context, custom_repofiles): +def target_repo_checks(repoinfo): """ - Install the required custom repository files into the container. + Check if the target repositories are available. - The repository files are copied from the host into the /etc/yum.repos.d - directory into the container. + :param repoinfo: a named tuple with the following attributes: + rhel_repoids: a set of RHEL repository IDs + rhsm_repoids: a set of RHSM repository IDs + duplicated_repoids: a dict of duplicated repoids and their repofiles + """ + _inhibit_on_duplicate_repos(repoinfo) + _check_available_rhel_repos(repoinfo) + _check_available_rhsm_repos(repoinfo) + _check_available_custom_repos(repoinfo) - :param context: the container where the repofiles should be copied - :type context: mounting.IsolatedActions class - :param custom_repofiles: list of custom repo files - :type custom_repofiles: List(CustomTargetRepositoryFile) + # TODO: We shall report that the RHEL repos that we deem necessary for + # the upgrade are not available; but currently it would just print bunch of + # data every time as we maps EUS and other repositories as well. But these + # do not have to be necessary available on the target system in the time + # of the upgrade. Let's skip it for now until it's clear how we will deal + # with it. + + +def _get_cloud_repo(indata): + """ + RH repositories are provided either by RHSM or are stored in the expected repo file provided by + RHUI special packages (every cloud provider has itw own rpm). """ - for rfile in custom_repofiles: - _dst_path = os.path.join('/etc/yum.repos.d', os.path.basename(rfile.file)) - context.copy_to(rfile.file, _dst_path) + if indata and indata.rhui_info: + upg_path = rhui.get_upg_path() + return os.path.join( + '/etc/yum.repos.d/', rhui.RHUI_CLOUD_MAP[upg_path][indata.rhui_info.provider]['leapp_pkg_repo'] + ) + return None -def _gather_target_repositories(context, indata, prod_cert_path): + +def gather_target_repositories(context, indata): """ - This is wrapper function to gather the target repoids. + Get available required target repositories and inhibit or raise error if basic checks do not pass. + + In case of repositories provided by Red Hat, it's checked whether the basic + required repositories are available (or at least defined) in the given + context. If not, raise StopActorExecutionError. + + For the custom target repositories we expect all of them have to be defined. + If any custom target repository is missing, raise StopActorExecutionError. - Probably the function could be partially merged into gather_target_repositories - and this could be really just wrapper with the switch of certificates. - I am keeping that for now as it is as interim step. + If any repository is defined multiple times, produce the inhibitor Report + msg. - :param context: the container where the repofiles should be copied + :param context: An instance of a mounting.IsolatedActions class :type context: mounting.IsolatedActions class - :param indata: majority of input data for the actor - :type indata: class _InputData - :param prod_cert_path: path where the target product cert is stored - :type prod_cert_path: string + :return: List of target system repoids + :rtype: List(string) """ - rhsm.set_container_mode(context) - rhsm.switch_certificate(context, indata.rhsm_info, prod_cert_path) - if indata.rhui_info: - rhui.copy_rhui_data(context, indata.rhui_info.provider) - _install_custom_repofiles(context, indata.custom_repofiles) - return gather_target_repositories(context, indata) + + repoinfo = repos.collect_repositories(context, cloud_repo=_get_cloud_repo(indata)) + target_repo_checks(repoinfo) + + api.current_logger().debug("Gathered target repositories: {}".format(', '.join(repoinfo.target_repoids))) + + return repoinfo.target_repoids def _copy_files(context, files): @@ -752,7 +685,8 @@ def perform(): # Mount the ISO into the scratch container target_iso = next(api.consume(TargetOSInstallationImage), None) with mounting.mount_upgrade_iso_to_root_dir(overlay.target, target_iso): - target_repoids = _gather_target_repositories(context, indata, prod_cert_path) + repos.prepare_repository_collection(context, indata, prod_cert_path) + target_repoids = gather_target_repositories(context, indata) _create_target_userspace(context, indata.packages, indata.files, target_repoids) # TODO: this is tmp solution as proper one needs significant refactoring target_repo_facts = repofileutils.get_parsed_repofiles(context) diff --git a/repos/system_upgrade/common/actors/targetuserspacecreator/tests/unit_test_targetuserspacecreator.py b/repos/system_upgrade/common/actors/targetuserspacecreator/tests/unit_test_targetuserspacecreator.py index 5f544471ee..d83d6c77eb 100644 --- a/repos/system_upgrade/common/actors/targetuserspacecreator/tests/unit_test_targetuserspacecreator.py +++ b/repos/system_upgrade/common/actors/targetuserspacecreator/tests/unit_test_targetuserspacecreator.py @@ -5,7 +5,7 @@ from leapp import models, reporting from leapp.exceptions import StopActorExecution, StopActorExecutionError -from leapp.libraries.actor import userspacegen +from leapp.libraries.actor import repoinfo, userspacegen from leapp.libraries.common import overlaygen, repofileutils, rhsm from leapp.libraries.common.config import architecture from leapp.libraries.common.testutils import CurrentActorMocked, logger_mocked, produce_mocked @@ -38,6 +38,15 @@ def __call__(self, **dummy_kwarg): def call(self, *args, **kwargs): return {'stdout': ''} + def is_isolated(self): + return False + + def full_path(self, path): + return path + + def copy_to(self, src, dst): + pass + def nspawn(self): return self @@ -260,12 +269,24 @@ def test_gather_target_repositories(monkeypatch): assert target_repoids == ['repoidX', 'repoidY', 'repoidCustom'] +def _mocked_repo_info(rhel_repos=(), custom_repos=(), rhui_repos=()): + class RepositoryInformationMocked(repoinfo.RepositoryInformation): + def __init__(self, context, cloud_repo=None): + super(RepositoryInformationMocked, self).__init__(context, cloud_repo=cloud_repo) + + def _load_repofiles(self, context, cloud_repo=None): + self.repo_type_map[repoinfo.REPO_KIND_RHSM] = set(rhel_repos) + self.repo_type_map[repoinfo.REPO_KIND_CUSTOM] = set(custom_repos) + self.repo_type_map[repoinfo.REPO_KIND_RHUI] = set(rhui_repos) + return RepositoryInformationMocked + + def test_gather_target_repositories_none_available(monkeypatch): mocked_produce = produce_mocked() monkeypatch.setattr(userspacegen.api, 'current_actor', CurrentActorMocked()) + monkeypatch.setattr(repoinfo, 'RepositoryInformation', _mocked_repo_info()) monkeypatch.setattr(userspacegen.api.current_actor(), 'produce', mocked_produce) - monkeypatch.setattr(rhsm, 'get_available_repo_ids', lambda x: []) monkeypatch.setattr(rhsm, 'skip_rhsm', lambda: False) with pytest.raises(StopActorExecution): userspacegen.gather_target_repositories(None, None) @@ -283,9 +304,10 @@ def test_gather_target_repositories_rhui(monkeypatch): ) monkeypatch.setattr(userspacegen.api, 'current_actor', CurrentActorMocked()) - monkeypatch.setattr(userspacegen, '_get_all_available_repoids', lambda x: []) monkeypatch.setattr( - userspacegen, '_get_rh_available_repoids', lambda x, y: ['rhui-1', 'rhui-2', 'rhui-3'] + repoinfo, + 'RepositoryInformation', + _mocked_repo_info(rhui_repos=['rhui-1', 'rhui-2', 'rhui-3']) ) monkeypatch.setattr(rhsm, 'skip_rhsm', lambda: True) monkeypatch.setattr( @@ -311,7 +333,11 @@ def test_gather_target_repositories_required_not_available(monkeypatch): monkeypatch.setattr(userspacegen.api, 'current_actor', CurrentActorMocked()) monkeypatch.setattr(userspacegen.api.current_actor(), 'produce', mocked_produce) # The available RHSM repos - monkeypatch.setattr(rhsm, 'get_available_repo_ids', lambda x: ['repoidA', 'repoidB', 'repoidC']) + monkeypatch.setattr( + repoinfo, + 'RepositoryInformation', + _mocked_repo_info(custom_repos=['repoidA', 'repoidB', 'repoidC']) + ) monkeypatch.setattr(rhsm, 'skip_rhsm', lambda: False) # The required RHEL repos based on the repo mapping and PES data + custom repos required by third party actors monkeypatch.setattr(userspacegen.api, 'consume', lambda x: iter([models.TargetRepositories( @@ -362,17 +388,25 @@ def test_perform_ok(monkeypatch): repoids = ['repoidX', 'repoidY'] monkeypatch.setattr(userspacegen, '_InputData', mocked_consume_data) monkeypatch.setattr(userspacegen, '_get_product_certificate_path', lambda: _DEFAULT_CERT_PATH) + monkeypatch.setattr(userspacegen.rhsm, 'skip_rhsm', lambda: True) monkeypatch.setattr(overlaygen, 'create_source_overlay', MockedMountingBase) - monkeypatch.setattr(userspacegen, '_gather_target_repositories', lambda *x: repoids) + monkeypatch.setattr( + repoinfo, + 'RepositoryInformation', + _mocked_repo_info(rhel_repos=repoids) + ) + monkeypatch.setattr(userspacegen.api, 'consume', lambda x: iter([models.TargetRepositories( + rhel_repos=[models.RHELTargetRepository(repoid=repoid) for repoid in repoids])] + if x == models.TargetRepositories else ())) monkeypatch.setattr(userspacegen, '_create_target_userspace', lambda *x: None) monkeypatch.setattr(userspacegen.api, 'current_actor', CurrentActorMocked()) monkeypatch.setattr(userspacegen.api, 'produce', produce_mocked()) + monkeypatch.setattr(userspacegen.reporting, 'produce', produce_mocked()) monkeypatch.setattr(repofileutils, 'get_repodirs', lambda: ['/etc/yum.repos.d']) userspacegen.perform() - msg_target_repos = models.UsedTargetRepositories( - repos=[models.UsedTargetRepository(repoid=repo) for repo in repoids]) assert userspacegen.api.produce.called == 3 assert isinstance(userspacegen.api.produce.model_instances[0], models.TMPTargetRepositoriesFacts) - assert userspacegen.api.produce.model_instances[1] == msg_target_repos + reported_used_target_repositories = userspacegen.api.produce.model_instances[1] + assert {r.repoid for r in reported_used_target_repositories.repos} == set(repoids) # this one is full of constants, so it's safe to check just the instance assert isinstance(userspacegen.api.produce.model_instances[2], models.TargetUserSpaceInfo) diff --git a/repos/system_upgrade/common/libraries/repofileutils.py b/repos/system_upgrade/common/libraries/repofileutils.py index a563be5204..6da3cf05ea 100644 --- a/repos/system_upgrade/common/libraries/repofileutils.py +++ b/repos/system_upgrade/common/libraries/repofileutils.py @@ -11,10 +11,10 @@ api.current_logger().warning('repofileutils.py: failed to import dnf') -def _parse_repository(repoid, repo_data): +def _parse_repository(repoid, repo_data, path, kind='custom'): def asbool(x): return x == '1' - prepared = {'repoid': repoid, 'additional_fields': {}} + prepared = {'repoid': repoid, 'additional_fields': {}, 'file': path, 'kind': kind} for key in repo_data.keys(): if key in RepositoryData.fields: if isinstance(RepositoryData.fields[key], fields.Boolean): @@ -26,7 +26,7 @@ def asbool(x): return RepositoryData(**prepared) -def parse_repofile(repofile): +def parse_repofile(repofile, rfile=None, kind='custom'): """ Parse the given repo file. @@ -38,7 +38,7 @@ def parse_repofile(repofile): with open(repofile, mode='r') as fp: cp = utils.parse_config(fp, strict=False) for repoid in cp.sections(): - data.append(_parse_repository(repoid, dict(cp.items(repoid)))) + data.append(_parse_repository(repoid, dict(cp.items(repoid)), kind=kind, path=rfile or repofile)) return RepositoryFile(file=repofile, data=data) @@ -56,7 +56,7 @@ def get_repodirs(): return list({os.path.realpath(d) for d in base.conf.reposdir if os.path.isdir(d)}) -def get_parsed_repofiles(context=mounting.NotIsolatedActions(base_dir='/')): +def get_parsed_repofiles(context=None, kind_resolve=lambda rpath: 'custom'): """ Scan all repositories on the system. @@ -70,14 +70,16 @@ def get_parsed_repofiles(context=mounting.NotIsolatedActions(base_dir='/')): :type context: mounting.IsolatedActions class :rtype: List(RepositoryFile) """ + context = context or mounting.NotIsolatedActions(base_dir='/') repofiles = [] cmd = ['find', '-L'] + get_repodirs() + ['-maxdepth', '1', '-type', 'f', '-name', '*.repo'] repofiles_paths = context.call(cmd, split=True)['stdout'] for repofile_path in repofiles_paths: - repofile = parse_repofile(context.full_path(repofile_path)) - # we want full path in cotext, not the real full path - repofile.file = repofile_path - repofiles.append(repofile) + repofiles.append( + parse_repofile( + repofile=context.full_path(repofile_path), + rfile=repofile_path, + kind=kind_resolve(repofile_path))) return repofiles diff --git a/repos/system_upgrade/common/libraries/rhsm.py b/repos/system_upgrade/common/libraries/rhsm.py index 4a5b0eb0eb..360df9ec40 100644 --- a/repos/system_upgrade/common/libraries/rhsm.py +++ b/repos/system_upgrade/common/libraries/rhsm.py @@ -4,7 +4,6 @@ import re import time -from leapp import reporting from leapp.exceptions import StopActorExecutionError from leapp.libraries.common import repofileutils from leapp.libraries.common.config import get_env @@ -169,13 +168,6 @@ def get_available_repo_ids(context): ) repofiles = repofileutils.get_parsed_repofiles(context) - - # TODO: move this functionality out! Create check actor that will do - # the inhibit. The functionality is really not good here in the current - # shape of the leapp-repository. See the targetuserspacecreator and - # systemfacts actor if this is moved out. - # Issue: #486 - _inhibit_on_duplicate_repos(repofiles) rhsm_repos = [] for rfile in repofiles: if rfile.file == _DEFAULT_RHSM_REPOFILE and rfile.data: @@ -192,36 +184,6 @@ def get_available_repo_ids(context): return rhsm_repos -def _inhibit_on_duplicate_repos(repofiles): - """ - Inhibit the upgrade if any repoid is defined multiple times. - - When that happens, it not only shows misconfigured system, but then - we can't get details of all the available repos as well. - """ - duplicates = repofileutils.get_duplicate_repositories(repofiles).keys() - - if not duplicates: - return - list_separator_fmt = '\n - ' - api.current_logger().warning( - 'The following repoids are defined multiple times:{0}{1}' - .format(list_separator_fmt, list_separator_fmt.join(duplicates)) - ) - - reporting.create_report([ - reporting.Title('A YUM/DNF repository defined multiple times'), - reporting.Summary( - 'The following repositories are defined multiple times:{0}{1}' - .format(list_separator_fmt, list_separator_fmt.join(duplicates)) - ), - reporting.Severity(reporting.Severity.MEDIUM), - reporting.Groups([reporting.Groups.REPOSITORY]), - reporting.Groups([reporting.Groups.INHIBITOR]), - reporting.Remediation(hint='Remove the duplicate repository definitions.') - ]) - - @with_rhsm def get_enabled_repo_ids(context): """ diff --git a/repos/system_upgrade/common/libraries/tests/test_rhsm.py b/repos/system_upgrade/common/libraries/tests/test_rhsm.py index a6dbea968d..4972f146a7 100644 --- a/repos/system_upgrade/common/libraries/tests/test_rhsm.py +++ b/repos/system_upgrade/common/libraries/tests/test_rhsm.py @@ -3,13 +3,11 @@ import pytest -from leapp import reporting from leapp.exceptions import StopActorExecutionError from leapp.libraries.common import repofileutils, rhsm -from leapp.libraries.common.testutils import create_report_mocked, CurrentActorMocked, logger_mocked +from leapp.libraries.common.testutils import CurrentActorMocked, logger_mocked from leapp.libraries.stdlib import api, CalledProcessError from leapp.models import RepositoryData, RepositoryFile -from leapp.utils.report import is_inhibitor Repository = namedtuple('Repository', ['repoid', 'file']) LIST_SEPARATOR = '\n - ' @@ -143,7 +141,6 @@ def test_get_available_repo_ids(monkeypatch, other_repofiles, rhsm_repofile): rhsm_repos = [repo.repoid for repo in rhsm_repofile.data] if rhsm_repofile else [] monkeypatch.setattr(api, 'current_logger', logger_mocked()) - monkeypatch.setattr(rhsm, '_inhibit_on_duplicate_repos', lambda x: None) monkeypatch.setattr(repofileutils, 'get_parsed_repofiles', lambda x: repos) result = rhsm.get_available_repo_ids(context_mocked) @@ -170,37 +167,6 @@ def test_get_available_repo_ids_error(): assert 'Unable to use yum' in str(err) -def test_inhibit_on_duplicate_repos(monkeypatch): - monkeypatch.setattr(reporting, 'create_report', create_report_mocked()) - monkeypatch.setattr(api, 'current_logger', logger_mocked()) - repofiles = [ - _gen_repofile("foo", [_gen_repo('repoX'), _gen_repo('repoY')]), - _gen_repofile("bar", [_gen_repo('repoX')]), - ] - - rhsm._inhibit_on_duplicate_repos(repofiles) - - dups = ['repoX'] - assert ('The following repoids are defined multiple times:{0}{1}' - .format(LIST_SEPARATOR, LIST_SEPARATOR.join(dups))) in api.current_logger.warnmsg - assert reporting.create_report.called == 1 - assert is_inhibitor(reporting.create_report.report_fields) - assert reporting.create_report.report_fields['title'] == 'A YUM/DNF repository defined multiple times' - summary = ('The following repositories are defined multiple times:{0}{1}' - .format(LIST_SEPARATOR, LIST_SEPARATOR.join(dups))) - assert summary in reporting.create_report.report_fields['summary'] - - -def test_inhibit_on_duplicate_repos_no_dups(monkeypatch): - monkeypatch.setattr(reporting, 'create_report', create_report_mocked()) - monkeypatch.setattr(api, 'current_logger', logger_mocked()) - - rhsm._inhibit_on_duplicate_repos([_gen_repofile("foo")]) - - assert not api.current_logger.warnmsg - assert reporting.create_report.called == 0 - - def test_sku_listing(monkeypatch, actor_mocked, context_mocked): """Tests whether the rhsm library can obtain used SKUs correctly.""" context_mocked.add_mocked_command_call_with_stdout(CMD_RHSM_LIST_CONSUMED, 'SKU: 598339696910') diff --git a/repos/system_upgrade/common/libraries/testutils.py b/repos/system_upgrade/common/libraries/testutils.py index c538af1a13..4d0740bc5a 100644 --- a/repos/system_upgrade/common/libraries/testutils.py +++ b/repos/system_upgrade/common/libraries/testutils.py @@ -1,3 +1,5 @@ +from __future__ import print_function + import json import logging import os @@ -113,7 +115,7 @@ def get_answers(self, dialog): raise NotImplementedError def show_message(self, message): - raise NotImplementedError + print('SHOW_MESSAGE({})'.format(message)) @property def actor_files_paths(self): diff --git a/repos/system_upgrade/common/models/repositoriesfacts.py b/repos/system_upgrade/common/models/repositoriesfacts.py index cd2124fc77..e34271d7a3 100644 --- a/repos/system_upgrade/common/models/repositoriesfacts.py +++ b/repos/system_upgrade/common/models/repositoriesfacts.py @@ -7,13 +7,47 @@ class RepositoryData(Model): topic = SystemFactsTopic repoid = fields.String() + """ + Unique identifier for the repository. + """ name = fields.String() + """ + Display name of the repository. + """ baseurl = fields.Nullable(fields.String()) + """ + URL where the repository data is located, possibly missing if there's a meta link or mirror list. + """ metalink = fields.Nullable(fields.String()) + """ + See documentation for repository files. + """ mirrorlist = fields.Nullable(fields.String()) + """ + See documentation for repository files. + """ enabled = fields.Boolean(default=True) + """ + Is this repository enabled? + """ additional_fields = fields.Nullable(fields.String()) + """ + See documentation for repository files. + """ proxy = fields.Nullable(fields.String()) + """ + Proxy URL necessary for this repository + """ + # TODO: Remove default + file = fields.String(default='') + """ + The repository file where this repo was defined + """ + # TODO: Remove default + kind = fields.StringEnum(choices=['custom', 'rhui', 'rhsm'], default='custom') + """ + Declares if this comes through RHSM, RHUI or from a custom repo file. + """ class RepositoryFile(Model):