From 353cd03d5339a6f3905f8bc4f067e0758f6e1d78 Mon Sep 17 00:00:00 2001 From: Petr Stodulka Date: Wed, 17 Jan 2024 21:46:19 +0100 Subject: [PATCH 66/66] upgrade data files loading: update error msgs and repors + minor changes Updated number of error messages and reports to be sure that users know what files are actually problematic. Original errors and reports usually didn't contain the full path to an upgrade data file due to possibility to download the file from a server. However, the posibility to download fresh data files from a requested server is not expected to support in the current state as the data files are part of provided packages. I've been thinking quite long time whether to actually drop or deprecate bigger part of the code to simplify the whole solution, as currently it's not planned to have a possibility to download some data files from servers in future. However, thinking about upcoming challenges, I am not totally persuaded that we will not revive that functionality in future, or that we will not want to use it for something little bit different. From that POV (and late phase of development prior the planned release) I think it will be better to preserve it for now and raise a discussion about it later. Other changes in this PR: * drop hardcoded name of the leapp-upgrade-elXtoelY rpm and use the leapp.libraries.common.rpms.get_leapp_packages() function * replace REPOSITORY group by SANITY; it was originally a mixture of both and SANITY fits better to me from this point * the check of consumed data sets could produce report with empty links, as the original article(s) we referred to have been obsoleted; so added filter for missing URLs Co-authored-by: Toshio Kuratomi --- .../libraries/check_consumed_assets.py | 31 ++++++++++++++++--- .../deviceanddriverdeprecationdataload.py | 3 ++ .../libraries/pes_event_parsing.py | 24 +++++++++----- .../libraries/repositoriesmapping.py | 21 +++++++------ .../system_upgrade/common/libraries/fetch.py | 28 ++++++++++------- 5 files changed, 75 insertions(+), 32 deletions(-) diff --git a/repos/system_upgrade/common/actors/checkconsumedassets/libraries/check_consumed_assets.py b/repos/system_upgrade/common/actors/checkconsumedassets/libraries/check_consumed_assets.py index f5998de0..1558c2fc 100644 --- a/repos/system_upgrade/common/actors/checkconsumedassets/libraries/check_consumed_assets.py +++ b/repos/system_upgrade/common/actors/checkconsumedassets/libraries/check_consumed_assets.py @@ -4,10 +4,27 @@ from collections import defaultdict, namedtuple from leapp import reporting from leapp.libraries.common.config import get_consumed_data_stream_id from leapp.libraries.common.fetch import ASSET_PROVIDED_DATA_STREAMS_FIELD +from leapp.libraries.common.rpms import get_leapp_packages, LeappComponents from leapp.libraries.stdlib import api from leapp.models import ConsumedDataAsset +def _get_hint(): + hint = ( + 'All official assets (data files) are part of the installed rpms these days.' + ' This issue is usually encountered when the data files are incorrectly' + ' customized, replaced, or removed. ' + ' In case you want to recover the original files, remove them (if they still exist)' + ' and reinstall the following rpms: {rpms}.\n' + 'The listed assets (data files) are usually inside the /etc/leapp/files/' + ' directory.' + .format( + rpms=', '.join(get_leapp_packages(component=LeappComponents.REPOSITORY)) + ) + ) + return hint + + def compose_summary_for_incompatible_assets(assets, incompatibility_reason): if not assets: return [] @@ -69,13 +86,16 @@ def report_incompatible_assets(assets): summary_lines += compose_summary_for_incompatible_assets(incompatible_assets, reason) for asset in incompatible_assets: - doc_url_to_title[asset.docs_url].append(asset.docs_title) + if asset.docs_url: + # Add URLs only when they are specified. docs_url could be empty string + doc_url_to_title[asset.docs_url].append(asset.docs_title) report_parts = [ reporting.Title(title), reporting.Summary('\n'.join(summary_lines)), reporting.Severity(reporting.Severity.HIGH), - reporting.Groups([reporting.Groups.INHIBITOR, reporting.Groups.REPOSITORY]), + reporting.Remediation(hint=_get_hint()), + reporting.Groups([reporting.Groups.INHIBITOR, reporting.Groups.SANITY]), ] report_parts += make_report_entries_with_unique_urls(docs_url_to_title_map) @@ -101,13 +121,16 @@ def report_malformed_assets(malformed_assets): details = ' - The asset file {filename} contains invalid value in its "{data_streams_field}"' details = details.format(filename=asset.filename, data_streams_field=ASSET_PROVIDED_DATA_STREAMS_FIELD) summary_lines.append(details) - docs_url_to_title_map[asset.docs_url].append(asset.docs_title) + if asset.docs_url: + # Add URLs only when they are specified. docs_url could be empty string + docs_url_to_title_map[asset.docs_url].append(asset.docs_title) report_parts = [ reporting.Title(title), reporting.Summary('\n'.join(summary_lines)), + reporting.Remediation(hint=_get_hint()), reporting.Severity(reporting.Severity.HIGH), - reporting.Groups([reporting.Groups.INHIBITOR, reporting.Groups.REPOSITORY]), + reporting.Groups([reporting.Groups.INHIBITOR, reporting.Groups.SANITY]), ] report_parts += make_report_entries_with_unique_urls(docs_url_to_title_map) diff --git a/repos/system_upgrade/common/actors/loaddevicedriverdeprecationdata/libraries/deviceanddriverdeprecationdataload.py b/repos/system_upgrade/common/actors/loaddevicedriverdeprecationdata/libraries/deviceanddriverdeprecationdataload.py index 3caa4e0a..f422c2c3 100644 --- a/repos/system_upgrade/common/actors/loaddevicedriverdeprecationdata/libraries/deviceanddriverdeprecationdataload.py +++ b/repos/system_upgrade/common/actors/loaddevicedriverdeprecationdata/libraries/deviceanddriverdeprecationdataload.py @@ -13,6 +13,9 @@ def process(): supported_device_types = set(DeviceDriverDeprecationEntry.device_type.serialize()['choices']) data_file_name = 'device_driver_deprecation_data.json' + # NOTE(pstodulk): load_data_assert raises StopActorExecutionError, see + # the code for more info. Keeping the handling on the framework in such + # a case as we have no work to do in such a case here. deprecation_data = fetch.load_data_asset(api.current_actor(), data_file_name, asset_fulltext_name='Device driver deprecation data', diff --git a/repos/system_upgrade/common/actors/peseventsscanner/libraries/pes_event_parsing.py b/repos/system_upgrade/common/actors/peseventsscanner/libraries/pes_event_parsing.py index 35bcec73..f24dda68 100644 --- a/repos/system_upgrade/common/actors/peseventsscanner/libraries/pes_event_parsing.py +++ b/repos/system_upgrade/common/actors/peseventsscanner/libraries/pes_event_parsing.py @@ -8,7 +8,7 @@ from leapp import reporting from leapp.exceptions import StopActorExecution from leapp.libraries.common import fetch from leapp.libraries.common.config import architecture -from leapp.libraries.common.config.version import get_source_major_version, get_target_major_version +from leapp.libraries.common.rpms import get_leapp_packages, LeappComponents from leapp.libraries.stdlib import api # NOTE(mhecko): The modulestream field contains a set of modulestreams until the very end when we generate a Package @@ -67,6 +67,9 @@ def get_pes_events(pes_json_directory, pes_json_filename): :return: List of Event tuples, where each event contains event type and input/output pkgs """ try: + # NOTE(pstodulk): load_data_assert raises StopActorExecutionError, see + # the code for more info. Keeping the handling on the framework in such + # a case as we have no work to do in such a case here. events_data = fetch.load_data_asset(api.current_actor(), pes_json_filename, asset_fulltext_name='PES events file', @@ -83,22 +86,27 @@ def get_pes_events(pes_json_directory, pes_json_filename): events_matching_arch = [e for e in all_events if not e.architectures or arch in e.architectures] return events_matching_arch except (ValueError, KeyError): - rpmname = 'leapp-upgrade-el{}toel{}'.format(get_source_major_version(), get_target_major_version()) - title = 'Missing/Invalid PES data file ({}/{})'.format(pes_json_directory, pes_json_filename) + local_path = os.path.join(pes_json_directory, pes_json_filename) + title = 'Missing/Invalid PES data file ({})'.format(local_path) summary = ( 'All official data files are nowadays part of the installed rpms.' ' This issue is usually encountered when the data files are incorrectly customized, replaced, or removed' ' (e.g. by custom scripts).' - ' In case you want to recover the original file, remove it (if still exists)' - ' and reinstall the {} rpm.' - .format(rpmname) + ) + hint = ( + ' In case you want to recover the original {lp} file, remove it (if it still exists)' + ' and reinstall the following rpms: {rpms}.' + .format( + lp=local_path, + rpms=', '.join(get_leapp_packages(component=LeappComponents.REPOSITORY)) + ) ) reporting.create_report([ reporting.Title(title), reporting.Summary(summary), + reporting.Remediation(hint=hint), reporting.Severity(reporting.Severity.HIGH), - reporting.Groups([reporting.Groups.SANITY]), - reporting.Groups([reporting.Groups.INHIBITOR]), + reporting.Groups([reporting.Groups.SANITY, reporting.Groups.INHIBITOR]), reporting.RelatedResource('file', os.path.join(pes_json_directory, pes_json_filename)) ]) raise StopActorExecution() diff --git a/repos/system_upgrade/common/actors/repositoriesmapping/libraries/repositoriesmapping.py b/repos/system_upgrade/common/actors/repositoriesmapping/libraries/repositoriesmapping.py index 6f2b2e0f..8045634e 100644 --- a/repos/system_upgrade/common/actors/repositoriesmapping/libraries/repositoriesmapping.py +++ b/repos/system_upgrade/common/actors/repositoriesmapping/libraries/repositoriesmapping.py @@ -4,6 +4,7 @@ from collections import defaultdict from leapp.exceptions import StopActorExecutionError from leapp.libraries.common.config.version import get_source_major_version, get_target_major_version from leapp.libraries.common.fetch import load_data_asset +from leapp.libraries.common.rpms import get_leapp_packages, LeappComponents from leapp.libraries.stdlib import api from leapp.models import PESIDRepositoryEntry, RepoMapEntry, RepositoriesMapping from leapp.models.fields import ModelViolationError @@ -130,29 +131,31 @@ class RepoMapData(object): def _inhibit_upgrade(msg): - rpmname = 'leapp-upgrade-el{}toel{}'.format(get_source_major_version(), get_target_major_version()) + local_path = os.path.join('/etc/leapp/file', REPOMAP_FILE) hint = ( 'All official data files are nowadays part of the installed rpms.' ' This issue is usually encountered when the data files are incorrectly customized, replaced, or removed' ' (e.g. by custom scripts).' - ' In case you want to recover the original file, remove it (if still exists)' - ' and reinstall the {} rpm.' - .format(rpmname) + ' In case you want to recover the original {lp} file, remove the current one (if it still exists)' + ' and reinstall the following packages: {rpms}.' + .format( + lp=local_path, + rpms=', '.join(get_leapp_packages(component=LeappComponents.REPOSITORY)) + ) ) raise StopActorExecutionError(msg, details={'hint': hint}) def _read_repofile(repofile): - # NOTE: what about catch StopActorExecution error when the file cannot be - # obtained -> then check whether old_repomap file exists and in such a case - # inform user they have to provide the new repomap.json file (we have the - # warning now only which could be potentially overlooked) + # NOTE(pstodulk): load_data_assert raises StopActorExecutionError, see + # the code for more info. Keeping the handling on the framework in such + # a case as we have no work to do in such a case here. repofile_data = load_data_asset(api.current_actor(), repofile, asset_fulltext_name='Repositories mapping', docs_url='', docs_title='') - return repofile_data # If the file does not contain a valid json then load_asset will do a stop actor execution + return repofile_data def scan_repositories(read_repofile_func=_read_repofile): diff --git a/repos/system_upgrade/common/libraries/fetch.py b/repos/system_upgrade/common/libraries/fetch.py index 42fcb74c..82bf4ff3 100644 --- a/repos/system_upgrade/common/libraries/fetch.py +++ b/repos/system_upgrade/common/libraries/fetch.py @@ -7,7 +7,7 @@ import requests from leapp import models from leapp.exceptions import StopActorExecutionError from leapp.libraries.common.config import get_consumed_data_stream_id, get_env -from leapp.libraries.common.config.version import get_source_major_version, get_target_major_version +from leapp.libraries.common.rpms import get_leapp_packages, LeappComponents from leapp.libraries.stdlib import api SERVICE_HOST_DEFAULT = "https://cert.cloud.redhat.com" @@ -16,16 +16,18 @@ MAX_ATTEMPTS = 3 ASSET_PROVIDED_DATA_STREAMS_FIELD = 'provided_data_streams' -def _get_hint(): - rpmname = 'leapp-upgrade-el{}toel{}'.format(get_source_major_version(), get_target_major_version()) +def _get_hint(local_path): hint = ( - 'All official data files are nowadays part of the installed rpms.' - ' That is the only official resource of actual official data files for in-place upgrades.' + 'All official data files are part of the installed rpms these days.' + ' The rpm is the only official source of the official data files for in-place upgrades.' ' This issue is usually encountered when the data files are incorrectly customized, replaced, or removed' ' (e.g. by custom scripts).' - ' In case you want to recover the original file, remove it (if still exists)' - ' and reinstall the {} rpm.' - .format(rpmname) + ' In case you want to recover the original {lp} file, remove the current one (if it still exists)' + ' and reinstall the following packages: {rpms}.' + .format( + lp=local_path, + rpms=', '.join(get_leapp_packages(component=LeappComponents.REPOSITORY)) + ) ) return hint @@ -34,10 +36,9 @@ def _raise_error(local_path, details): """ If the file acquisition fails in any way, throw an informative error to stop the actor. """ - rpmname = 'leapp-upgrade-el{}toel{}'.format(get_source_major_version(), get_target_major_version()) summary = 'Data file {lp} is missing or invalid.'.format(lp=local_path) - raise StopActorExecutionError(summary, details={'details': details, 'hint': _get_hint()}) + raise StopActorExecutionError(summary, details={'details': details, 'hint': _get_hint(local_path)}) def _request_data(service_path, cert, proxies, timeout=REQUEST_TIMEOUT): @@ -148,6 +149,7 @@ def load_data_asset(actor_requesting_asset, docs_title): """ Load the content of the data asset with given asset_filename + and produce :class:`leapp.model.ConsumedDataAsset` message. :param Actor actor_requesting_asset: The actor instance requesting the asset file. It is necessary for the actor to be able to produce ConsumedDataAsset message in order for leapp to be able @@ -157,6 +159,10 @@ def load_data_asset(actor_requesting_asset, :param str docs_url: Docs url to provide if an asset is malformed or outdated. :param str docs_title: Title of the documentation to where `docs_url` points to. :returns: A dict with asset contents (a parsed JSON), or None if the asset was outdated. + :raises StopActorExecutionError: In following cases: + * ConsumedDataAsset is not specified in the produces tuple of the actor_requesting_asset actor + * The content of the required data file is not valid JSON format + * The required data cannot be obtained (e.g. due to missing file) """ # Check that the actor that is attempting to obtain the asset meets the contract to call this function @@ -167,7 +173,7 @@ def load_data_asset(actor_requesting_asset, error_hint = {'hint': ('Read documentation at the following link for more information about how to retrieve ' 'the valid file: {0}'.format(docs_url))} else: - error_hint = {'hint': _get_hint()} + error_hint = {'hint': _get_hint(os.path.join('/etc/leapp/files', asset_filename))} data_stream_id = get_consumed_data_stream_id() data_stream_major = data_stream_id.split('.', 1)[0] -- 2.43.0