From 794c21b489b5d8eba58ce9ca7c9b467f09461979 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Reh=C3=A1k?= Date: Sat, 28 Mar 2020 08:36:10 +0100 Subject: [PATCH 01/13] Substitute RHSM skip variable & bump leapp-framework rq The development variable LEAPP_DEVEL_SKIP_RHSM is being deprecated and replaced with LEAPP_NO_RHSM, which is going to be officially supported. At the framework level, setting LEAPP_DEVEL_SKIP_RHSM to 1 also sets LEAPP_NO_RHSM to 1. This is intentional not to break existing tests and scripts. Therefore, it makes sense to only check for the latter one in tests. Bump leapp-framework requirement to 1.2 regarding the change in the leapp. --- packaging/leapp-repository.spec | 2 +- .../el7toel8/actors/checkrhsmsku/tests/test_rhsmsku.py | 6 +++--- .../actors/enablerhsmreposonrhel8/libraries/library.py | 6 +++--- .../tests/test_enablerhsmreposonrhel8.py | 4 ++-- .../actors/targetuserspacecreator/tests/unit_test.py | 2 +- repos/system_upgrade/el7toel8/libraries/rhsm.py | 2 +- 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/packaging/leapp-repository.spec b/packaging/leapp-repository.spec index 138b4df8e0..3dc19767ea 100644 --- a/packaging/leapp-repository.spec +++ b/packaging/leapp-repository.spec @@ -30,7 +30,7 @@ Requires: leapp-repository-dependencies = 5 # IMPORTANT: this is capability provided by the leapp framework rpm. # Check that 'version' this instead of the real framework rpm version. -Requires: leapp-framework >= 1.1, leapp-framework < 2 +Requires: leapp-framework >= 1.2, leapp-framework < 2 Requires: python2-leapp # That's temporary to ensure the obsoleted subpackage is not installed diff --git a/repos/system_upgrade/el7toel8/actors/checkrhsmsku/tests/test_rhsmsku.py b/repos/system_upgrade/el7toel8/actors/checkrhsmsku/tests/test_rhsmsku.py index d6233e0718..ba68e8dde8 100644 --- a/repos/system_upgrade/el7toel8/actors/checkrhsmsku/tests/test_rhsmsku.py +++ b/repos/system_upgrade/el7toel8/actors/checkrhsmsku/tests/test_rhsmsku.py @@ -4,7 +4,7 @@ def test_sku_report_skipped(monkeypatch, current_actor_context): with monkeypatch.context() as context: - context.setenv('LEAPP_DEVEL_SKIP_RHSM', '1') + context.setenv('LEAPP_NO_RHSM', '1') current_actor_context.feed(RHSMInfo(attached_skus=[])) current_actor_context.run() assert not list(current_actor_context.consume(Report)) @@ -12,7 +12,7 @@ def test_sku_report_skipped(monkeypatch, current_actor_context): def test_sku_report_has_skus(monkeypatch, current_actor_context): with monkeypatch.context() as context: - context.setenv('LEAPP_DEVEL_SKIP_RHSM', '0') + context.setenv('LEAPP_NO_RHSM', '0') current_actor_context.feed(RHSMInfo(attached_skus=['testing-sku'])) current_actor_context.run() assert not list(current_actor_context.consume(Report)) @@ -20,7 +20,7 @@ def test_sku_report_has_skus(monkeypatch, current_actor_context): def test_sku_report_has_no_skus(monkeypatch, current_actor_context): with monkeypatch.context() as context: - context.setenv('LEAPP_DEVEL_SKIP_RHSM', '0') + context.setenv('LEAPP_NO_RHSM', '0') current_actor_context.feed(RHSMInfo(attached_skus=[])) current_actor_context.run() reports = list(current_actor_context.consume(Report)) diff --git a/repos/system_upgrade/el7toel8/actors/enablerhsmreposonrhel8/libraries/library.py b/repos/system_upgrade/el7toel8/actors/enablerhsmreposonrhel8/libraries/library.py index e51d8384a9..8380ed6456 100644 --- a/repos/system_upgrade/el7toel8/actors/enablerhsmreposonrhel8/libraries/library.py +++ b/repos/system_upgrade/el7toel8/actors/enablerhsmreposonrhel8/libraries/library.py @@ -6,7 +6,7 @@ def set_rhsm_release(): """Set the RHSM release to the target RHEL 8 minor version.""" if rhsm.skip_rhsm(): - api.current_logger().debug('Skipping setting the RHSM release due to the use of LEAPP_DEVEL_SKIP_RHSM.') + api.current_logger().debug('Skipping setting the RHSM release due to --no-rhsm or environment variables.') return if config.get_product_type('target') != 'ga': @@ -29,8 +29,8 @@ def enable_rhsm_repos(): the known repositories. """ if rhsm.skip_rhsm(): - api.current_logger().debug('Skipping enabling repositories through subscription-manager due to the use of' - ' LEAPP_DEVEL_SKIP_RHSM.') + api.current_logger().debug('Skipping enabling repositories through subscription-manager due to --no-rhsm' + ' or environment variables.') return try: run(get_submgr_cmd(get_repos_to_enable())) diff --git a/repos/system_upgrade/el7toel8/actors/enablerhsmreposonrhel8/tests/test_enablerhsmreposonrhel8.py b/repos/system_upgrade/el7toel8/actors/enablerhsmreposonrhel8/tests/test_enablerhsmreposonrhel8.py index dfc02b7e3d..56db418ca4 100644 --- a/repos/system_upgrade/el7toel8/actors/enablerhsmreposonrhel8/tests/test_enablerhsmreposonrhel8.py +++ b/repos/system_upgrade/el7toel8/actors/enablerhsmreposonrhel8/tests/test_enablerhsmreposonrhel8.py @@ -93,7 +93,7 @@ def test_setrelease_submgr_throwing_error(monkeypatch): @pytest.mark.parametrize('product', ['beta', 'htb']) def test_setrelease_skip_rhsm(monkeypatch, product): commands_called, _ = not_isolated_actions() - monkeypatch.setenv('LEAPP_DEVEL_SKIP_RHSM', '1') + monkeypatch.setenv('LEAPP_NO_RHSM', '1') monkeypatch.setattr(config, 'get_product_type', lambda dummy: product) # To make this work we need to re-apply the decorator, so it respects the environment variable monkeypatch.setattr(rhsm, 'set_release', rhsm.with_rhsm(rhsm.set_release)) @@ -134,7 +134,7 @@ def test_running_submgr_fail(monkeypatch): def test_enable_repos_skip_rhsm(monkeypatch): - monkeypatch.setenv('LEAPP_DEVEL_SKIP_RHSM', '1') + monkeypatch.setenv('LEAPP_NO_RHSM', '1') monkeypatch.setattr(library, 'run', run_mocked()) monkeypatch.setattr(api, 'current_logger', logger_mocked()) library.enable_rhsm_repos() diff --git a/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/tests/unit_test.py b/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/tests/unit_test.py index 2674c56800..4e221ca2fc 100644 --- a/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/tests/unit_test.py +++ b/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/tests/unit_test.py @@ -150,7 +150,7 @@ def test_consume_data(monkeypatch, raised, pkg_msgs, rhsm_info, xfs, storage): mocked_consume = MockedConsume(pkg_msgs, rhsm_info, xfs, storage) monkeypatch.setattr(api, 'consume', mocked_consume) monkeypatch.setattr(api, 'current_logger', mocked_logger()) - monkeypatch.setenv('LEAPP_DEVEL_SKIP_RHSM', '0') + monkeypatch.setenv('LEAPP_NO_RHSM', '0') if not xfs: xfs = models.XFSPresence() if not raised: diff --git a/repos/system_upgrade/el7toel8/libraries/rhsm.py b/repos/system_upgrade/el7toel8/libraries/rhsm.py index b02a04f84b..fc8b41ff4e 100644 --- a/repos/system_upgrade/el7toel8/libraries/rhsm.py +++ b/repos/system_upgrade/el7toel8/libraries/rhsm.py @@ -81,7 +81,7 @@ def _handle_rhsm_exceptions(hint=None): def skip_rhsm(): """Check whether we should skip RHSM related code.""" - return os.getenv('LEAPP_DEVEL_SKIP_RHSM', '0') == '1' + return os.getenv('LEAPP_NO_RHSM', '0') == '1' def with_rhsm(f): From 1b59298d66f3bf52e995aedd3723dea8c6c6e8e4 Mon Sep 17 00:00:00 2001 From: Petr Stodulka Date: Thu, 2 Apr 2020 03:41:38 +0200 Subject: [PATCH 02/13] rhsm: always set rhsm container mode inside container In case the system is subscribed but user wants to use own repositories for the IPU, rhsm on the host could be affected when run any yum/dnf commands inside the container. For that reason, set the container mode for rhsm everytime, despite the skip_rhsm(). --- repos/system_upgrade/el7toel8/libraries/rhsm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/repos/system_upgrade/el7toel8/libraries/rhsm.py b/repos/system_upgrade/el7toel8/libraries/rhsm.py index fc8b41ff4e..3e10cf777e 100644 --- a/repos/system_upgrade/el7toel8/libraries/rhsm.py +++ b/repos/system_upgrade/el7toel8/libraries/rhsm.py @@ -269,7 +269,7 @@ def get_existing_product_certificates(context): return certs -@with_rhsm +# DO NOT SET the with_rhsm decorator for this function def set_container_mode(context): """ Put RHSM into the container mode. From 718bc9271e47cc44f45b83bb71343d099002a146 Mon Sep 17 00:00:00 2001 From: Petr Stodulka Date: Tue, 31 Mar 2020 18:02:18 +0200 Subject: [PATCH 03/13] [RFE custom repos 1/9] add actor to scan the default custom repo file The actor scan the custom repo file that user could use to define own repositories that should be used during the IPU as well. The expected path of the file is: /etc/leapp/files/leapp_upgrade_repositories.repo These repositories will be used automatically for the IPU despite the enable/disable settings. It's required the file and defined repositories has to be valid. If the file doesn't exist, nothing happens. --- .../actors/scancustomrepofile/actor.py | 24 ++++++ .../libraries/scancustomrepofile.py | 32 ++++++++ .../tests/test_scancustomrepofile.py | 75 +++++++++++++++++++ 3 files changed, 131 insertions(+) create mode 100644 repos/system_upgrade/el7toel8/actors/scancustomrepofile/actor.py create mode 100644 repos/system_upgrade/el7toel8/actors/scancustomrepofile/libraries/scancustomrepofile.py create mode 100644 repos/system_upgrade/el7toel8/actors/scancustomrepofile/tests/test_scancustomrepofile.py diff --git a/repos/system_upgrade/el7toel8/actors/scancustomrepofile/actor.py b/repos/system_upgrade/el7toel8/actors/scancustomrepofile/actor.py new file mode 100644 index 0000000000..cb6e55f915 --- /dev/null +++ b/repos/system_upgrade/el7toel8/actors/scancustomrepofile/actor.py @@ -0,0 +1,24 @@ +from leapp.actors import Actor +from leapp.libraries.actor import scancustomrepofile +from leapp.models import CustomTargetRepository +from leapp.tags import FactsPhaseTag, IPUWorkflowTag + + +class ScanCustomRepofile(Actor): + """ + Scan the custom /etc/leapp/files/leapp_upgrade_repositories.repo repo file. + + This is the official path where to put the YUM/DNF repository file with + custom repositories for the target system. These repositories will be used + automatically for the in-place upgrade despite the enable/disable settings. + + If the file doesn't exist, nothing happens. + """ + + name = 'scan_custom_repofile' + consumes = () + produces = (CustomTargetRepository,) + tags = (FactsPhaseTag, IPUWorkflowTag) + + def process(self): + scancustomrepofile.process() diff --git a/repos/system_upgrade/el7toel8/actors/scancustomrepofile/libraries/scancustomrepofile.py b/repos/system_upgrade/el7toel8/actors/scancustomrepofile/libraries/scancustomrepofile.py new file mode 100644 index 0000000000..3f6ba83d24 --- /dev/null +++ b/repos/system_upgrade/el7toel8/actors/scancustomrepofile/libraries/scancustomrepofile.py @@ -0,0 +1,32 @@ +import os + +from leapp.libraries.common import repofileutils +from leapp.libraries.stdlib import api +from leapp.models import CustomTargetRepository + + +CUSTOM_REPO_PATH = "/etc/leapp/files/leapp_upgrade_repositories.repo" + + +def process(): + """ + Produce CustomTargetRepository msgs for the custom repo file if the file + exists. + + The CustomTargetRepository msg is produced for every repository inside + the file. + """ + if not os.path.isfile(CUSTOM_REPO_PATH): + api.current_logger().debug( + "The {} file doesn't exist. Nothing to do." + .format(CUSTOM_REPO_PATH)) + return + api.current_logger().info("The {} file exists.".format(CUSTOM_REPO_PATH)) + repofile = repofileutils.parse_repofile(CUSTOM_REPO_PATH) + for repo in repofile.data: + api.produce(CustomTargetRepository( + repoid=repo.repoid, + name=repo.name, + baseurl=repo.baseurl, + enabled=repo.enabled, + )) diff --git a/repos/system_upgrade/el7toel8/actors/scancustomrepofile/tests/test_scancustomrepofile.py b/repos/system_upgrade/el7toel8/actors/scancustomrepofile/tests/test_scancustomrepofile.py new file mode 100644 index 0000000000..5f1893b33e --- /dev/null +++ b/repos/system_upgrade/el7toel8/actors/scancustomrepofile/tests/test_scancustomrepofile.py @@ -0,0 +1,75 @@ +import os + +from leapp.libraries.actor import scancustomrepofile +from leapp.libraries.common import repofileutils +from leapp.libraries.common.testutils import produce_mocked +from leapp.libraries.stdlib import api +from leapp.models import CustomTargetRepository, RepositoryData, RepositoryFile + + +_REPODATA = [ + RepositoryData(repoid="repo1", name="repo1name", baseurl="repo1url", enabled=True), + RepositoryData(repoid="repo2", name="repo2name", baseurl="repo2url", enabled=False), + RepositoryData(repoid="repo3", name="repo3name", enabled=True), + RepositoryData(repoid="repo4", name="repo4name", mirrorlist="mirror4list", enabled=True), +] + +_CUSTOM_REPOS = [ + CustomTargetRepository(repoid="repo1", name="repo1name", baseurl="repo1url", enabled=True), + CustomTargetRepository(repoid="repo2", name="repo2name", baseurl="repo2url", enabled=False), + CustomTargetRepository(repoid="repo3", name="repo3name", baseurl=None, enabled=True), + CustomTargetRepository(repoid="repo4", name="repo4name", baseurl=None, enabled=True), +] + + +class LoggerMocked(object): + def __init__(self): + self.infomsg = None + self.debugmsg = None + + def info(self, msg): + self.infomsg = msg + + def debug(self, msg): + self.debugmsg = msg + + def __call__(self): + return self + + +def test_no_repofile(monkeypatch): + monkeypatch.setattr(os.path, 'isfile', lambda dummy: False) + monkeypatch.setattr(api, 'produce', produce_mocked()) + monkeypatch.setattr(api, 'current_logger', LoggerMocked()) + scancustomrepofile.process() + msg = "The {} file doesn't exist. Nothing to do.".format(scancustomrepofile.CUSTOM_REPO_PATH) + assert api.current_logger.debugmsg == msg + assert not api.produce.called + + +def test_valid_repofile_exists(monkeypatch): + def _mocked_parse_repofile(fpath): + return RepositoryFile(file=fpath, data=_REPODATA) + monkeypatch.setattr(os.path, 'isfile', lambda dummy: True) + monkeypatch.setattr(api, 'produce', produce_mocked()) + monkeypatch.setattr(repofileutils, 'parse_repofile', _mocked_parse_repofile) + monkeypatch.setattr(api, 'current_logger', LoggerMocked()) + scancustomrepofile.process() + msg = "The {} file exists.".format(scancustomrepofile.CUSTOM_REPO_PATH) + assert api.current_logger.infomsg == msg + assert api.produce.called == len(_CUSTOM_REPOS) + for crepo in _CUSTOM_REPOS: + assert crepo in api.produce.model_instances + + +def test_empty_repofile_exists(monkeypatch): + def _mocked_parse_repofile(fpath): + return RepositoryFile(file=fpath, data=[]) + monkeypatch.setattr(os.path, 'isfile', lambda dummy: True) + monkeypatch.setattr(api, 'produce', produce_mocked()) + monkeypatch.setattr(repofileutils, 'parse_repofile', _mocked_parse_repofile) + monkeypatch.setattr(api, 'current_logger', LoggerMocked()) + scancustomrepofile.process() + msg = "The {} file exists.".format(scancustomrepofile.CUSTOM_REPO_PATH) + assert api.current_logger.infomsg == msg + assert not api.produce.called From 4047f2016b3170ec39fc3baefb0d1ac656f4f5d6 Mon Sep 17 00:00:00 2001 From: Petr Stodulka Date: Tue, 31 Mar 2020 21:34:39 +0200 Subject: [PATCH 04/13] [RFE custom repos 2/9] Add the CustomTargetRepositoryFile model The model is used to share information about custom repo files that should be extra handled during the IPU. Basically we have just one file that we want to use for these purposes now, but I haven't found good library for the constant and I haven't wanted to duplicate the string several times. As well, maybe someone will want to handle own repofile. For that reasons, I rather created the new model. Note: Feel free to drop it and hardcode the path. --- .../el7toel8/actors/scancustomrepofile/actor.py | 7 +++++-- .../scancustomrepofile/libraries/scancustomrepofile.py | 5 ++++- .../scancustomrepofile/tests/test_scancustomrepofile.py | 7 +++++-- repos/system_upgrade/el7toel8/models/targetrepositories.py | 5 +++++ 4 files changed, 19 insertions(+), 5 deletions(-) diff --git a/repos/system_upgrade/el7toel8/actors/scancustomrepofile/actor.py b/repos/system_upgrade/el7toel8/actors/scancustomrepofile/actor.py index cb6e55f915..d46018fab3 100644 --- a/repos/system_upgrade/el7toel8/actors/scancustomrepofile/actor.py +++ b/repos/system_upgrade/el7toel8/actors/scancustomrepofile/actor.py @@ -1,6 +1,6 @@ from leapp.actors import Actor from leapp.libraries.actor import scancustomrepofile -from leapp.models import CustomTargetRepository +from leapp.models import CustomTargetRepository, CustomTargetRepositoryFile from leapp.tags import FactsPhaseTag, IPUWorkflowTag @@ -12,12 +12,15 @@ class ScanCustomRepofile(Actor): custom repositories for the target system. These repositories will be used automatically for the in-place upgrade despite the enable/disable settings. + Additionally the CustomTargetRepositoryFile message is produced if the file + exists to let the other actors know they should handle the file as well. + If the file doesn't exist, nothing happens. """ name = 'scan_custom_repofile' consumes = () - produces = (CustomTargetRepository,) + produces = (CustomTargetRepository, CustomTargetRepositoryFile) tags = (FactsPhaseTag, IPUWorkflowTag) def process(self): diff --git a/repos/system_upgrade/el7toel8/actors/scancustomrepofile/libraries/scancustomrepofile.py b/repos/system_upgrade/el7toel8/actors/scancustomrepofile/libraries/scancustomrepofile.py index 3f6ba83d24..c294193dca 100644 --- a/repos/system_upgrade/el7toel8/actors/scancustomrepofile/libraries/scancustomrepofile.py +++ b/repos/system_upgrade/el7toel8/actors/scancustomrepofile/libraries/scancustomrepofile.py @@ -2,7 +2,7 @@ from leapp.libraries.common import repofileutils from leapp.libraries.stdlib import api -from leapp.models import CustomTargetRepository +from leapp.models import CustomTargetRepository, CustomTargetRepositoryFile CUSTOM_REPO_PATH = "/etc/leapp/files/leapp_upgrade_repositories.repo" @@ -23,6 +23,9 @@ def process(): return api.current_logger().info("The {} file exists.".format(CUSTOM_REPO_PATH)) repofile = repofileutils.parse_repofile(CUSTOM_REPO_PATH) + if not repofile.data: + return + api.produce(CustomTargetRepositoryFile(file=CUSTOM_REPO_PATH)) for repo in repofile.data: api.produce(CustomTargetRepository( repoid=repo.repoid, diff --git a/repos/system_upgrade/el7toel8/actors/scancustomrepofile/tests/test_scancustomrepofile.py b/repos/system_upgrade/el7toel8/actors/scancustomrepofile/tests/test_scancustomrepofile.py index 5f1893b33e..4ea33a26f1 100644 --- a/repos/system_upgrade/el7toel8/actors/scancustomrepofile/tests/test_scancustomrepofile.py +++ b/repos/system_upgrade/el7toel8/actors/scancustomrepofile/tests/test_scancustomrepofile.py @@ -4,7 +4,7 @@ from leapp.libraries.common import repofileutils from leapp.libraries.common.testutils import produce_mocked from leapp.libraries.stdlib import api -from leapp.models import CustomTargetRepository, RepositoryData, RepositoryFile +from leapp.models import CustomTargetRepository, CustomTargetRepositoryFile, RepositoryData, RepositoryFile _REPODATA = [ @@ -21,6 +21,8 @@ CustomTargetRepository(repoid="repo4", name="repo4name", baseurl=None, enabled=True), ] +_CUSTOM_REPO_FILE_MSG = CustomTargetRepositoryFile(file=scancustomrepofile.CUSTOM_REPO_PATH) + class LoggerMocked(object): def __init__(self): @@ -57,7 +59,8 @@ def _mocked_parse_repofile(fpath): scancustomrepofile.process() msg = "The {} file exists.".format(scancustomrepofile.CUSTOM_REPO_PATH) assert api.current_logger.infomsg == msg - assert api.produce.called == len(_CUSTOM_REPOS) + assert api.produce.called == len(_CUSTOM_REPOS) + 1 + assert _CUSTOM_REPO_FILE_MSG in api.produce.model_instances for crepo in _CUSTOM_REPOS: assert crepo in api.produce.model_instances diff --git a/repos/system_upgrade/el7toel8/models/targetrepositories.py b/repos/system_upgrade/el7toel8/models/targetrepositories.py index 5487ac4344..3604772c35 100644 --- a/repos/system_upgrade/el7toel8/models/targetrepositories.py +++ b/repos/system_upgrade/el7toel8/models/targetrepositories.py @@ -30,3 +30,8 @@ class TargetRepositories(Model): class UsedTargetRepositories(Model): topic = TransactionTopic repos = fields.List(fields.Model(UsedTargetRepository)) + + +class CustomTargetRepositoryFile(Model): + topic = TransactionTopic + file = fields.String() From cc773288d95f8b181e497eb25b48cc899ab3c5ca Mon Sep 17 00:00:00 2001 From: Petr Stodulka Date: Wed, 1 Apr 2020 08:09:27 +0200 Subject: [PATCH 05/13] [RFE custom repos 3/9] update rhsm.skip_rhsm: read envar from configuration Direct reading of LEAPP & LEAPP_DEVEL evars in actors should happen from the IPUConfig using the leapp.libraries.common.config library. Update according to guildelines. As well, I had to do some refactoring because of the change inside checkrhsmku and enablerhsmreposonrhel8 actors, including fix of tests. Co-authored-by: Vinzenz 'evilissimo' Feenstra --- .../el7toel8/actors/checkrhsmsku/actor.py | 21 +------- .../actors/checkrhsmsku/libraries/library.py | 24 +++++++++ .../actors/checkrhsmsku/tests/test_rhsmsku.py | 50 +++++++++--------- .../tests/test_enablerhsmreposonrhel8.py | 51 +++++++++++++++---- .../targetuserspacecreator/tests/unit_test.py | 2 +- .../system_upgrade/el7toel8/libraries/rhsm.py | 15 +++--- 6 files changed, 103 insertions(+), 60 deletions(-) create mode 100644 repos/system_upgrade/el7toel8/actors/checkrhsmsku/libraries/library.py diff --git a/repos/system_upgrade/el7toel8/actors/checkrhsmsku/actor.py b/repos/system_upgrade/el7toel8/actors/checkrhsmsku/actor.py index 2708400fd0..1f812f5de3 100644 --- a/repos/system_upgrade/el7toel8/actors/checkrhsmsku/actor.py +++ b/repos/system_upgrade/el7toel8/actors/checkrhsmsku/actor.py @@ -1,8 +1,6 @@ from leapp.actors import Actor -from leapp.libraries.common import rhsm +from leapp.libraries.actor import library from leapp.models import Report, RHSMInfo -from leapp.reporting import create_report -from leapp import reporting from leapp.tags import IPUWorkflowTag, ChecksPhaseTag @@ -20,19 +18,4 @@ class CheckRedHatSubscriptionManagerSKU(Actor): tags = (IPUWorkflowTag, ChecksPhaseTag) def process(self): - if not rhsm.skip_rhsm(): - for info in self.consume(RHSMInfo): - if not info.attached_skus: - create_report([ - reporting.Title('The system is not registered or subscribed.'), - reporting.Summary( - 'The system has to be registered and subscribed to be able to proceed the upgrade.' - ), - reporting.Severity(reporting.Severity.HIGH), - reporting.Tags([reporting.Tags.SANITY]), - reporting.Flags([reporting.Flags.INHIBITOR]), - reporting.Remediation( - hint='Register your system with the subscription-manager tool and attach it to proper SKUs' - ' to be able to proceed the upgrade.'), - reporting.RelatedResource('package', 'subscription-manager') - ]) + library.process() diff --git a/repos/system_upgrade/el7toel8/actors/checkrhsmsku/libraries/library.py b/repos/system_upgrade/el7toel8/actors/checkrhsmsku/libraries/library.py new file mode 100644 index 0000000000..d3377117e5 --- /dev/null +++ b/repos/system_upgrade/el7toel8/actors/checkrhsmsku/libraries/library.py @@ -0,0 +1,24 @@ +from leapp import reporting +from leapp.libraries.common import rhsm +from leapp.libraries.stdlib import api +from leapp.models import RHSMInfo +from leapp.reporting import create_report + + +def process(): + if not rhsm.skip_rhsm(): + for info in api.consume(RHSMInfo): + if not info.attached_skus: + create_report([ + reporting.Title('The system is not registered or subscribed.'), + reporting.Summary( + 'The system has to be registered and subscribed to be able to proceed the upgrade.' + ), + reporting.Severity(reporting.Severity.HIGH), + reporting.Tags([reporting.Tags.SANITY]), + reporting.Flags([reporting.Flags.INHIBITOR]), + reporting.Remediation( + hint='Register your system with the subscription-manager tool and attach it to proper SKUs' + ' to be able to proceed the upgrade.'), + reporting.RelatedResource('package', 'subscription-manager') + ]) diff --git a/repos/system_upgrade/el7toel8/actors/checkrhsmsku/tests/test_rhsmsku.py b/repos/system_upgrade/el7toel8/actors/checkrhsmsku/tests/test_rhsmsku.py index ba68e8dde8..cd837a43b5 100644 --- a/repos/system_upgrade/el7toel8/actors/checkrhsmsku/tests/test_rhsmsku.py +++ b/repos/system_upgrade/el7toel8/actors/checkrhsmsku/tests/test_rhsmsku.py @@ -1,30 +1,34 @@ + +from leapp import reporting +from leapp.libraries.actor import library from leapp.libraries.common import rhsm -from leapp.models import Report, RHSMInfo +from leapp.libraries.common.testutils import create_report_mocked +from leapp.libraries.stdlib import api +from leapp.models import RHSMInfo -def test_sku_report_skipped(monkeypatch, current_actor_context): - with monkeypatch.context() as context: - context.setenv('LEAPP_NO_RHSM', '1') - current_actor_context.feed(RHSMInfo(attached_skus=[])) - current_actor_context.run() - assert not list(current_actor_context.consume(Report)) +def test_sku_report_skipped(monkeypatch): + monkeypatch.setattr(rhsm, 'skip_rhsm', lambda: True) + monkeypatch.setattr(api, 'consume', lambda x: (RHSMInfo(attached_skus=[]),)) + monkeypatch.setattr(library, 'create_report', create_report_mocked()) + library.process() + assert not library.create_report.called -def test_sku_report_has_skus(monkeypatch, current_actor_context): - with monkeypatch.context() as context: - context.setenv('LEAPP_NO_RHSM', '0') - current_actor_context.feed(RHSMInfo(attached_skus=['testing-sku'])) - current_actor_context.run() - assert not list(current_actor_context.consume(Report)) +def test_sku_report_has_skus(monkeypatch): + monkeypatch.setattr(rhsm, 'skip_rhsm', lambda: False) + monkeypatch.setattr(api, 'consume', lambda x: (RHSMInfo(attached_skus=['testing-sku']),)) + monkeypatch.setattr(library, 'create_report', create_report_mocked()) + library.process() + assert not library.create_report.called -def test_sku_report_has_no_skus(monkeypatch, current_actor_context): - with monkeypatch.context() as context: - context.setenv('LEAPP_NO_RHSM', '0') - current_actor_context.feed(RHSMInfo(attached_skus=[])) - current_actor_context.run() - reports = list(current_actor_context.consume(Report)) - assert reports and len(reports) == 1 - report_fields = reports[0].report - assert report_fields['severity'] == 'high' - assert report_fields['title'] == 'The system is not registered or subscribed.' +def test_sku_report_has_no_skus(monkeypatch): + monkeypatch.setattr(rhsm, 'skip_rhsm', lambda: False) + monkeypatch.setattr(api, 'consume', lambda x: (RHSMInfo(attached_skus=[]),)) + monkeypatch.setattr(library, 'create_report', create_report_mocked()) + library.process() + assert library.create_report.called == 1 + assert library.create_report.report_fields['title'] == 'The system is not registered or subscribed.' + assert library.create_report.report_fields['severity'] == 'high' + assert 'inhibitor' in library.create_report.report_fields['flags'] diff --git a/repos/system_upgrade/el7toel8/actors/enablerhsmreposonrhel8/tests/test_enablerhsmreposonrhel8.py b/repos/system_upgrade/el7toel8/actors/enablerhsmreposonrhel8/tests/test_enablerhsmreposonrhel8.py index 56db418ca4..867ccb01bf 100644 --- a/repos/system_upgrade/el7toel8/actors/enablerhsmreposonrhel8/tests/test_enablerhsmreposonrhel8.py +++ b/repos/system_upgrade/el7toel8/actors/enablerhsmreposonrhel8/tests/test_enablerhsmreposonrhel8.py @@ -1,13 +1,15 @@ -import sys from collections import namedtuple +import os +import sys import pytest from leapp.exceptions import StopActorExecutionError from leapp.libraries.actor import library from leapp.libraries.common import config, mounting, rhsm +from leapp.libraries.common.config import architecture from leapp.libraries.stdlib import CalledProcessError, api -from leapp.models import UsedTargetRepositories, UsedTargetRepository, Version +from leapp.models import UsedTargetRepositories, UsedTargetRepository, EnvVar def not_isolated_actions(raise_err=False): @@ -50,6 +52,10 @@ class logger_mocked(object): def __init__(self): self.warnmsg = None self.dbgmsg = None + self.infomsg = None + + def info(self, *args): + self.infomsg = args def warning(self, *args): self.warnmsg = args @@ -61,15 +67,37 @@ def __call__(self): return self +# TODO: this is kinda overpowered mock, but we plan to provide something like +# that in the testutils library in future, so using as it is. class CurrentActorMocked(object): - configuration = namedtuple('configuration', ['version'])( - Version(source='7.6', target='8.0')) + def __init__(self, kernel='3.10.0-957.43.1.el7.x86_64', release_id='rhel', + src_ver='7.6', dst_ver='8.1', arch=architecture.ARCH_X86_64, + envars=None): + + if envars: + envarsList = [EnvVar(name=key, value=value) for key, value in envars.items()] + else: + envarsList = [] + + version = namedtuple('Version', ['source', 'target'])(src_ver, dst_ver) + os_release = namedtuple('OS_release', ['release_id', 'version_id'])(release_id, src_ver) + args = (version, kernel, os_release, arch, envarsList) + conf_fields = ['version', 'kernel', 'os_release', 'architecture', 'leapp_env_vars'] + self.configuration = namedtuple('configuration', conf_fields)(*args) + self._common_folder = '../../files' + self.log = logger_mocked() + + def __call__(self): + return self + + def get_common_folder_path(self, folder): + return os.path.join(self._common_folder, folder) def test_setrelease(monkeypatch): commands_called, klass = not_isolated_actions() monkeypatch.setattr(mounting, 'NotIsolatedActions', klass) - monkeypatch.setattr(api, 'current_actor', CurrentActorMocked) + monkeypatch.setattr(api, 'current_actor', CurrentActorMocked(dst_ver='8.0')) monkeypatch.setattr(config, 'get_product_type', lambda dummy: 'ga') library.set_rhsm_release() assert commands_called and len(commands_called) == 1 @@ -79,13 +107,14 @@ def test_setrelease(monkeypatch): def test_setrelease_submgr_throwing_error(monkeypatch): _, klass = not_isolated_actions(raise_err=True) monkeypatch.setattr(mounting, 'NotIsolatedActions', klass) - monkeypatch.setattr(api, 'current_actor', CurrentActorMocked) + monkeypatch.setattr(api, 'current_actor', CurrentActorMocked(dst_ver='8.0', envars={'LEAPP_NO_RHSM': '0'})) monkeypatch.setattr(config, 'get_product_type', lambda dummy: 'ga') # free the set_release funtion from the @_rhsm_retry decorator which would otherwise cause 25 sec delay of the test if sys.version_info.major < 3: - monkeypatch.setattr(rhsm, 'set_release', rhsm.set_release.func_closure[0].cell_contents) + monkeypatch.setattr(rhsm, 'set_release', + rhsm.set_release.func_closure[0].cell_contents.func_closure[0].cell_contents) else: - monkeypatch.setattr(rhsm, 'set_release', rhsm.set_release.__wrapped__) + monkeypatch.setattr(rhsm, 'set_release', rhsm.set_release.__wrapped__.__wrapped__) with pytest.raises(StopActorExecutionError): library.set_rhsm_release() @@ -93,7 +122,7 @@ def test_setrelease_submgr_throwing_error(monkeypatch): @pytest.mark.parametrize('product', ['beta', 'htb']) def test_setrelease_skip_rhsm(monkeypatch, product): commands_called, _ = not_isolated_actions() - monkeypatch.setenv('LEAPP_NO_RHSM', '1') + monkeypatch.setattr(api, 'current_actor', CurrentActorMocked(envars={'LEAPP_NO_RHSM': '1'})) monkeypatch.setattr(config, 'get_product_type', lambda dummy: product) # To make this work we need to re-apply the decorator, so it respects the environment variable monkeypatch.setattr(rhsm, 'set_release', rhsm.with_rhsm(rhsm.set_release)) @@ -117,6 +146,7 @@ def test_get_submgr_cmd(): def test_running_submgr_ok(monkeypatch): + monkeypatch.setattr(api, 'current_actor', CurrentActorMocked(dst_ver='8.0', envars={'LEAPP_NO_RHSM': '0'})) monkeypatch.setattr(library, 'get_repos_to_enable', lambda: {'some-repo'}) monkeypatch.setattr(library, 'run', run_mocked()) library.enable_rhsm_repos() @@ -125,6 +155,7 @@ def test_running_submgr_ok(monkeypatch): def test_running_submgr_fail(monkeypatch): + monkeypatch.setattr(api, 'current_actor', CurrentActorMocked(dst_ver='8.0', envars={'LEAPP_NO_RHSM': '0'})) monkeypatch.setattr(library, 'get_repos_to_enable', lambda: {'some-repo'}) monkeypatch.setattr(library, 'run', run_mocked(raise_err=True)) monkeypatch.setattr(api, 'current_logger', logger_mocked()) @@ -134,7 +165,7 @@ def test_running_submgr_fail(monkeypatch): def test_enable_repos_skip_rhsm(monkeypatch): - monkeypatch.setenv('LEAPP_NO_RHSM', '1') + monkeypatch.setattr(api, 'current_actor', CurrentActorMocked(envars={'LEAPP_NO_RHSM': '1'})) monkeypatch.setattr(library, 'run', run_mocked()) monkeypatch.setattr(api, 'current_logger', logger_mocked()) library.enable_rhsm_repos() diff --git a/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/tests/unit_test.py b/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/tests/unit_test.py index 4e221ca2fc..78b51159d2 100644 --- a/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/tests/unit_test.py +++ b/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/tests/unit_test.py @@ -150,7 +150,7 @@ def test_consume_data(monkeypatch, raised, pkg_msgs, rhsm_info, xfs, storage): mocked_consume = MockedConsume(pkg_msgs, rhsm_info, xfs, storage) monkeypatch.setattr(api, 'consume', mocked_consume) monkeypatch.setattr(api, 'current_logger', mocked_logger()) - monkeypatch.setenv('LEAPP_NO_RHSM', '0') + monkeypatch.setattr(api, 'current_actor', CurrentActorMocked(envars={'LEAPP_NO_RHSM': '0'})) if not xfs: xfs = models.XFSPresence() if not raised: diff --git a/repos/system_upgrade/el7toel8/libraries/rhsm.py b/repos/system_upgrade/el7toel8/libraries/rhsm.py index 3e10cf777e..7968346880 100644 --- a/repos/system_upgrade/el7toel8/libraries/rhsm.py +++ b/repos/system_upgrade/el7toel8/libraries/rhsm.py @@ -7,6 +7,7 @@ from leapp import reporting from leapp.exceptions import StopActorExecutionError from leapp.libraries.common import repofileutils +from leapp.libraries.common.config import get_env from leapp.libraries.stdlib import CalledProcessError, api from leapp.models import RHSMInfo @@ -81,17 +82,17 @@ def _handle_rhsm_exceptions(hint=None): def skip_rhsm(): """Check whether we should skip RHSM related code.""" - return os.getenv('LEAPP_NO_RHSM', '0') == '1' + return get_env('LEAPP_NO_RHSM', '0') == '1' def with_rhsm(f): """Decorator to allow skipping RHSM functions by executing a no-op.""" - if skip_rhsm(): - @functools.wraps(f) - def _no_op(*args, **kwargs): - return - return _no_op - return f + @functools.wraps(f) + def wrapper(*args, **kwargs): + if not skip_rhsm(): + return f(*args, **kwargs) + return None + return wrapper @with_rhsm From 04c62211f5890e7edbb9ba82286b326df761315d Mon Sep 17 00:00:00 2001 From: Petr Stodulka Date: Thu, 2 Apr 2020 00:29:09 +0200 Subject: [PATCH 06/13] [RFE custom repos 4/9] Add new actor: inhibit if target repositories are missing This will simplify checks in other actors and it will provide more clear information to users what actually happens. The actor takes care around scenarious we know the IPU the most probably fails. Actors in the following phases do not have to handle these situations properly (can keep the current handling, raising the StopActorExecutionError). See the table below about the behaviour of the actor. Case studdies ============= RHSM | CTR | CTRF || result -----+-----+------++------- Yes | --- | ---- || - -----+-----+------++------- No | No | No || inhibit -----+-----+------++------- No | No | Yes || inhibit -----+-----+------++------- No | Yes | No || report info -----+-----+------++------- No | Yes | Yes || - CTR - CustomTargetRepositories found CTRF - CustomTargetRepositoryFile found RHSM - RHSM is used (it is not skipped) TODO: unit-tests are missing --- .../el7toel8/actors/checktargetrepos/actor.py | 39 ++++++++ .../checktargetrepos/libraries/library.py | 91 +++++++++++++++++++ .../tests/test_checktargetrepos.py | 5 + 3 files changed, 135 insertions(+) create mode 100644 repos/system_upgrade/el7toel8/actors/checktargetrepos/actor.py create mode 100644 repos/system_upgrade/el7toel8/actors/checktargetrepos/libraries/library.py create mode 100644 repos/system_upgrade/el7toel8/actors/checktargetrepos/tests/test_checktargetrepos.py diff --git a/repos/system_upgrade/el7toel8/actors/checktargetrepos/actor.py b/repos/system_upgrade/el7toel8/actors/checktargetrepos/actor.py new file mode 100644 index 0000000000..08e766a954 --- /dev/null +++ b/repos/system_upgrade/el7toel8/actors/checktargetrepos/actor.py @@ -0,0 +1,39 @@ +from leapp.actors import Actor +from leapp.libraries.actor import library +from leapp.models import CustomTargetRepositoryFile, Report, TargetRepositories +from leapp.tags import IPUWorkflowTag, ChecksPhaseTag + + +class Checktargetrepos(Actor): + """ + Check whether target yum repositories are specified. + + RHSM | CTR | CTRF || result + -----+-----+------++------- + Yes | --- | ---- || - + -----+-----+------++------- + No | No | No || inhibit + -----+-----+------++------- + No | No | Yes || inhibit + -----+-----+------++------- + No | Yes | No || warn/report info + -----+-----+------++------- + No | Yes | Yes || - + + CTR - CustomTargetRepositories found + CTRF - the expected CustomTargetRepositoryFile found + RHSM - RHSM is used (it is not skipped) + + This is not 100 % reliable check. This cover just the most obvious cases + that are expected to fail. Reporting of such issues in this way, here, + will be probably much more clear, without additional errors that could + be raised. + """ + + name = 'checktargetrepos' + consumes = (CustomTargetRepositoryFile, TargetRepositories) + produces = (Report) + tags = (IPUWorkflowTag, ChecksPhaseTag) + + def process(self): + library.process() diff --git a/repos/system_upgrade/el7toel8/actors/checktargetrepos/libraries/library.py b/repos/system_upgrade/el7toel8/actors/checktargetrepos/libraries/library.py new file mode 100644 index 0000000000..7ef599965e --- /dev/null +++ b/repos/system_upgrade/el7toel8/actors/checktargetrepos/libraries/library.py @@ -0,0 +1,91 @@ +from leapp.models import CustomTargetRepositoryFile, TargetRepositories +from leapp.libraries.stdlib import api +from leapp import reporting +from leapp.libraries.common import rhsm + + +_IPU_DOC_URL = ('https://access.redhat.com/documentation/en-us/' + 'red_hat_enterprise_linux/8/html-single/upgrading_to_rhel_8/index') +# TODO: we need to provide this path in a shared library +CUSTOM_REPO_PATH = '/etc/leapp/files/leapp_upgrade_repositories.repo' + + +def _any_custom_repo_defined(): + for tr in api.consume(TargetRepositories): + if tr.custom_repos: + return True + return False + + +def _the_custom_repofile_defined(): + for ctrf in api.consume(CustomTargetRepositoryFile): + if ctrf and ctrf.file == CUSTOM_REPO_PATH: + return True + return False + + +def process(): + if not rhsm.skip_rhsm(): + # getting RH repositories through RHSM; resolved by seatbelts + # implemented in other actors + return + + # rhsm skipped; take your seatbelts please + is_ctr = _any_custom_repo_defined() + is_ctrf = _the_custom_repofile_defined() + if not is_ctr: + # no rhsm, no custom repositories.. this will really not work :) + # TODO: add link to the RH article about use of custom repositories!! + # NOTE: we can put here now the link to the main document, as this + # will be described there or at least the link to the right document + # will be delivered here. + if is_ctrf: + summary_ctrf = 'The custom repository file has been detected. Maybe it is empty?' + else: + summary_ctrf = 'The custom repository file has not been detected.' + reporting.create_report([ + reporting.Title('Using RHSM has been skipped but no custom repositories have been delivered.'), + reporting.Summary( + 'Leapp is run in the mode when the Red Hat Subscription Manager' + ' is not used (the --no-rhsm option or the LEAPP_NO_RHSM=1' + ' environment variable has been set) so leapp is not able to' + ' obtain YUM/DNF repositories with the content for the target' + ' system in the standard way and has to be delivered by user' + ' manually.' + ), + reporting.Remediation(hint=( + 'Create the repository file according to instructions in the' + ' referred document on the following path with all' + ' repositories that should be used during the upgrade: "{}".' + '\n\n{}' + .format(CUSTOM_REPO_PATH, summary_ctrf) + )), + reporting.Severity(reporting.Severity.HIGH), + reporting.Tags([reporting.Tags.SANITY]), + reporting.Flags([reporting.Flags.INHIBITOR]), + reporting.ExternalLink(url=_IPU_DOC_URL, title='UPGRADING TO RHEL 8'), + reporting.RelatedResource('file', CUSTOM_REPO_PATH), + ]) + elif not is_ctrf: + # Some custom repositories have been discovered, but the custom repo + # file not. Inform about the official recommended way. + reporting.create_report([ + reporting.Title('CustomTargetRepositories discovered, but the CustomTargetRepositoryFile is missing.'), + reporting.Summary( + 'Red Hat provides now official way how to use custom' + ' repositories during the in-place upgrade through' + ' the referred custom repository file. The CustomTargetRepositories' + ' have been detected (from custom actors?) but the repository' + ' file not.' + ), + reporting.Remediation(hint=( + 'Follow the new simple way to enable custom repositories' + ' during the upgrade (see the referred document) or create' + ' the empty custom repository file to acknowledge this report' + ' message.' + + )), + reporting.Severity(reporting.Severity.INFO), + reporting.ExternalLink(url=_IPU_DOC_URL, title='UPGRADING TO RHEL 8'), + reporting.RelatedResource('file', CUSTOM_REPO_PATH), + ]) diff --git a/repos/system_upgrade/el7toel8/actors/checktargetrepos/tests/test_checktargetrepos.py b/repos/system_upgrade/el7toel8/actors/checktargetrepos/tests/test_checktargetrepos.py new file mode 100644 index 0000000000..69626e4215 --- /dev/null +++ b/repos/system_upgrade/el7toel8/actors/checktargetrepos/tests/test_checktargetrepos.py @@ -0,0 +1,5 @@ + + +# TODO: unit tests are required (@drehak?) +def test_sth(): + pass From 39f9f6f55352c8bd946beb769a4a472bee772f79 Mon Sep 17 00:00:00 2001 From: Petr Stodulka Date: Thu, 2 Apr 2020 03:25:14 +0200 Subject: [PATCH 07/13] [RFE custom repos 5/9] targetuserspacecreator: Process the custom refile and enable IPU withou RHSM - handle CustomTargetRepositoryFile messages - all those files has to be copied into the container and part - of the target (rhel8userspace) container as well - extended processing of input data and related tests - covered skipping of rhsm as well - added warning about the deprecation of LEAPP_DEVEL_SKIP_RHSM if the envar is detected and == '1' - RHSM repositories are scanned only if rhsm_skip() == False - small refactoring to reduce number of arguments for various functions TODO: tests should be extended TODO: gathering of target repos should be extended by other checks for custom repositories (check availability and duplicity of repositories) --- .../actors/targetuserspacecreator/actor.py | 4 +- .../libraries/userspacegen.py | 122 +++++++++++++----- .../targetuserspacecreator/tests/unit_test.py | 113 ++++++++++++---- 3 files changed, 181 insertions(+), 58 deletions(-) diff --git a/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/actor.py b/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/actor.py index 503493d1e1..4831ad7c9b 100644 --- a/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/actor.py +++ b/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/actor.py @@ -1,7 +1,7 @@ from leapp.actors import Actor from leapp.libraries.actor import userspacegen from leapp.libraries.common.config import get_env, version -from leapp.models import (RepositoriesMap, RequiredTargetUserspacePackages, +from leapp.models import (CustomTargetRepositoryFile, RepositoriesMap, RequiredTargetUserspacePackages, RHSMInfo, StorageInfo, TargetRepositories, TargetUserSpaceInfo, UsedTargetRepositories, XFSPresence) @@ -20,7 +20,7 @@ class TargetUserspaceCreator(Actor): """ name = 'target_userspace_creator' - consumes = (RepositoriesMap, RequiredTargetUserspacePackages, + consumes = (CustomTargetRepositoryFile, RepositoriesMap, RequiredTargetUserspacePackages, StorageInfo, RHSMInfo, TargetRepositories, XFSPresence) produces = (TargetUserSpaceInfo, UsedTargetRepositories) tags = (IPUWorkflowTag, TargetTransactionFactsPhaseTag) diff --git a/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/libraries/userspacegen.py b/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/libraries/userspacegen.py index f2cd25dbbb..24b7a48e58 100644 --- a/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/libraries/userspacegen.py +++ b/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/libraries/userspacegen.py @@ -4,9 +4,9 @@ from leapp.exceptions import StopActorExecution, StopActorExecutionError from leapp.libraries.actor import constants from leapp.libraries.common import dnfplugin, mounting, overlaygen, rhsm, utils -from leapp.libraries.common.config import get_product_type +from leapp.libraries.common.config import get_product_type, get_env from leapp.libraries.stdlib import CalledProcessError, api, config, run -from leapp.models import (RequiredTargetUserspacePackages, RHSMInfo, +from leapp.models import (CustomTargetRepositoryFile, RequiredTargetUserspacePackages, RHSMInfo, StorageInfo, TargetRepositories, TargetUserSpaceInfo, UsedTargetRepositories, UsedTargetRepository, XFSPresence) @@ -14,6 +14,54 @@ PROD_CERTS_FOLDER = 'prod-certs' +def _check_deprecated_rhsm_skip(): + # we do not plan to cover this case by tests as it is purely + # devel/testing stuff, that becomes deprecated now + # just log the warning now (better than nothing?); deprecation process will + # be specified in close future + if get_env('LEAPP_DEVEL_SKIP_RHSM', '0') == '1': + api.current_logger().warn( + 'The LEAPP_DEVEL_SKIP_RHSM has been deprecated. Use' + ' LEAPP_NO_RHSM istead or use the --no-rhsm option for' + ' leapp. as well custom repofile has not been defined.' + ' Please read documentation about new "skip rhsm" solution.' + ) + + +class _InputData(object): + def __init__(self): + self._consume_data() + + def _consume_data(self): + """ + Wrapper function to consume majority input data. + + It doesn't consume TargetRepositories, which are consumed in the + own function. + """ + self.packages = {'dnf'} + for message in api.consume(RequiredTargetUserspacePackages): + self.packages.update(message.packages) + + # Get the RHSM information (available repos, attached SKUs, etc.) of the source (RHEL 7) system + self.rhsm_info = next(api.consume(RHSMInfo), None) + if not self.rhsm_info and not rhsm.skip_rhsm(): + api.current_logger().warn('Could not receive RHSM information - Is this system registered?') + raise StopActorExecution() + if rhsm.skip_rhsm() and self.rhsm_info: + # this should not happen. if so, raise an error as something in + # other actors is wrong really + raise StopActorExecutionError("RHSM is not handled but the RHSMInfo message has been produced.") + + # list comprehension needed on Py2 + + self.custom_repofiles = list(api.consume(CustomTargetRepositoryFile)) + self.xfs_info = next(api.consume(XFSPresence), XFSPresence()) + self.storage_info = next(api.consume(StorageInfo), None) + if not self.storage_info: + raise StopActorExecutionError('No storage info available cannot proceed.') + + def prepare_target_userspace(context, userspace_dir, enabled_repos, packages): """ Implement the creation of the target userspace. @@ -142,17 +190,18 @@ def gather_target_repositories(context): :return: List of target system repoids :rtype: List(string) """ - # Get the RHSM repos available in the RHEL 8 container - # FIXME: this cannot happen with custom --no-rhsm setup - available_repos = rhsm.get_available_repo_ids(context) - # FIXME: check that required repo IDs (baseos, appstream) # + or check that all required RHEL repo IDs are available. if not rhsm.skip_rhsm(): + # Get the RHSM repos available in the RHEL 8 container + # TODO: very similar thing should happens for all other repofiles in container + # + available_repos = rhsm.get_available_repo_ids(context) if not available_repos or len(available_repos) < 2: raise StopActorExecutionError( message='Cannot find required basic RHEL 8 repositories.', details={ + # FIXME: update the text - mention the possibility of custom repos 'hint': ('It is required to have RHEL repositories on the system' ' provided by the subscription-manager. Possibly you' ' are missing a valid SKU for the target system or network' @@ -160,6 +209,8 @@ def gather_target_repositories(context): ' to a valid SKU providing RHEL 8 repositories.') } ) + else: + available_repos = [] target_repoids = [] for target_repo in api.consume(TargetRepositories): @@ -171,9 +222,10 @@ def gather_target_repositories(context): # The StopActorExecutionError called above might be moved here. pass for custom_repo in target_repo.custom_repos: + # FIXME: this have to be done for the PR !! + # # now it works well when the custom repofile is used, but + # # we should require that all those repositories really exists # TODO: complete processing of custom repositories - # HINT: now it will work only for custom repos that exist - # + already on the system in a repo file # TODO: should check available_target_repoids + additional custom repos # + outside of rhsm.. # #if custom_repo.repoid in available_target_repoids: @@ -191,35 +243,41 @@ def gather_target_repositories(context): return target_repoids -def _consume_data(): - """Wrapper function to consume all input data.""" - packages = {'dnf'} - for message in api.consume(RequiredTargetUserspacePackages): - packages.update(message.packages) +def _install_custom_repofiles(context, custom_repofiles): + """ + Install the required custom repository files into the container. - # Get the RHSM information (available repos, attached SKUs, etc.) of the source (RHEL 7) system - rhsm_info = next(api.consume(RHSMInfo), None) - if not rhsm_info and not rhsm.skip_rhsm(): - api.current_logger().warn('Could not receive RHSM information - Is this system registered?') - raise StopActorExecution() + The repostory files are copied from the host into the /etc/yum.repos.d + directory into the container. - xfs_info = next(api.consume(XFSPresence), XFSPresence()) - storage_info = next(api.consume(StorageInfo), None) - if not storage_info: - raise StopActorExecutionError('No storage info available cannot proceed.') - return packages, rhsm_info, xfs_info, storage_info + :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 _gather_target_repositories(context, rhsm_info, prod_cert_path): +def _gather_target_repositories(context, indata, prod_cert_path): """ This is wrapper function to gather the target repoids. 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. + + :param context: the container where the repofiles should be copied + :type context: mounting.IsolatedActions class + :param indata: majority of input data for the actor + :type indata: class _InputData + :param prod_cert_path: path where is stored the target product cert + :type prod_cert_path: string """ rhsm.set_container_mode(context) - rhsm.switch_certificate(context, rhsm_info, prod_cert_path) + rhsm.switch_certificate(context, indata.rhsm_info, prod_cert_path) + _install_custom_repofiles(context, indata.custom_repofiles) return gather_target_repositories(context) @@ -234,16 +292,20 @@ def _create_target_userspace(context, packages, target_repoids): def perform(): - packages, rhsm_info, xfs_info, storage_info = _consume_data() + # NOTE: this one action is out of unit-tests completely; we do not use + # in unit tests the LEAPP_DEVEL_SKIP_RHSM envar anymore + _check_deprecated_rhsm_skip() + + indata = _InputData() prod_cert_path = _get_product_certificate_path() with overlaygen.create_source_overlay( mounts_dir=constants.MOUNTS_DIR, scratch_dir=constants.SCRATCH_DIR, - storage_info=storage_info, - xfs_info=xfs_info) as overlay: + storage_info=indata.storage_info, + xfs_info=indata.xfs_info) as overlay: with overlay.nspawn() as context: - target_repoids = _gather_target_repositories(context, rhsm_info, prod_cert_path) - _create_target_userspace(context, packages, target_repoids) + target_repoids = _gather_target_repositories(context, indata, prod_cert_path) + _create_target_userspace(context, indata.packages, target_repoids) api.produce(UsedTargetRepositories( repos=[UsedTargetRepository(repoid=repo) for repo in target_repoids])) api.produce(TargetUserSpaceInfo( diff --git a/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/tests/unit_test.py b/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/tests/unit_test.py index 78b51159d2..850ced0dca 100644 --- a/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/tests/unit_test.py +++ b/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/tests/unit_test.py @@ -106,6 +106,12 @@ def __call__(self): _RHSMINFO_MSG = models.RHSMInfo(attached_skus=['testing-sku']) _XFS_MSG = models.XFSPresence() _STORAGEINFO_MSG = models.StorageInfo() +_CTRF_MSGS = [ + models.CustomTargetRepositoryFile(file='rfileA'), + models.CustomTargetRepositoryFile(file='rfileB'), +] +_SAEE = StopActorExecutionError +_SAE = StopActorExecution class MockedConsume(object): @@ -123,43 +129,92 @@ def __call__(self, model): return iter([msg for msg in self._msgs if isinstance(msg, model)]) -# FIXME: the results should be probably different with supported disconnected -# systems (see rhsm) -# undefined: (????, _PACKAGES_MSGS, None, _XFS_MSG, None), -@pytest.mark.parametrize('raised,pkg_msgs,rhsm_info,xfs,storage', [ - (None, _PACKAGES_MSGS, _RHSMINFO_MSG, _XFS_MSG, _STORAGEINFO_MSG), - (None, _PACKAGES_MSGS[0], _RHSMINFO_MSG, _XFS_MSG, _STORAGEINFO_MSG), - (None, [], _RHSMINFO_MSG, _XFS_MSG, _STORAGEINFO_MSG), - (None, _PACKAGES_MSGS, _RHSMINFO_MSG, None, _STORAGEINFO_MSG), - ((StopActorExecution, 'RHSM information'), _PACKAGES_MSGS, None, _XFS_MSG, _STORAGEINFO_MSG), - ((StopActorExecution, 'RHSM information'), _PACKAGES_MSGS, None, None, _STORAGEINFO_MSG), - ((StopActorExecution, 'RHSM information'), [], None, _XFS_MSG, _STORAGEINFO_MSG), - ((StopActorExecution, 'RHSM information'), [], None, None, _STORAGEINFO_MSG), - ((StopActorExecutionError, 'No storage'), _PACKAGES_MSGS, _RHSMINFO_MSG, _XFS_MSG, None), - ((StopActorExecutionError, 'No storage'), _PACKAGES_MSGS, _RHSMINFO_MSG, None, None), - ((StopActorExecutionError, 'No storage'), [], _RHSMINFO_MSG, _XFS_MSG, None), - ((StopActorExecutionError, 'No storage'), [], _RHSMINFO_MSG, None, None), +testInData = namedtuple('TestInData', ['pkg_msgs', 'rhsm_info', 'xfs', 'storage', 'custom_repofiles']) + + +@pytest.mark.parametrize('raised,no_rhsm,testdata', [ + # valid cases with RHSM + (None, '0', testInData(_PACKAGES_MSGS, _RHSMINFO_MSG, _XFS_MSG, _STORAGEINFO_MSG, None)), + (None, '0', testInData(_PACKAGES_MSGS[0], _RHSMINFO_MSG, _XFS_MSG, _STORAGEINFO_MSG, None)), + (None, '0', testInData([], _RHSMINFO_MSG, _XFS_MSG, _STORAGEINFO_MSG, None)), + (None, '0', testInData(_PACKAGES_MSGS, _RHSMINFO_MSG, None, _STORAGEINFO_MSG, None)), + (None, '0', testInData(_PACKAGES_MSGS, _RHSMINFO_MSG, _XFS_MSG, _STORAGEINFO_MSG, _CTRF_MSGS)), + (None, '0', testInData(_PACKAGES_MSGS[0], _RHSMINFO_MSG, _XFS_MSG, _STORAGEINFO_MSG, _CTRF_MSGS)), + (None, '0', testInData([], _RHSMINFO_MSG, _XFS_MSG, _STORAGEINFO_MSG, _CTRF_MSGS)), + (None, '0', testInData(_PACKAGES_MSGS, _RHSMINFO_MSG, None, _STORAGEINFO_MSG, _CTRF_MSGS)), + + # valid cases without RHSM (== skip_rhsm) + (None, '1', testInData(_PACKAGES_MSGS, None, _XFS_MSG, _STORAGEINFO_MSG, None)), + (None, '1', testInData(_PACKAGES_MSGS, None, None, _STORAGEINFO_MSG, None)), + (None, '1', testInData([], None, _XFS_MSG, _STORAGEINFO_MSG, None)), + (None, '1', testInData([], None, None, _STORAGEINFO_MSG, None)), + (None, '1', testInData(_PACKAGES_MSGS, None, _XFS_MSG, _STORAGEINFO_MSG, _CTRF_MSGS)), + (None, '1', testInData(_PACKAGES_MSGS, None, None, _STORAGEINFO_MSG, _CTRF_MSGS)), + (None, '1', testInData([], None, _XFS_MSG, _STORAGEINFO_MSG, _CTRF_MSGS)), + (None, '1', testInData([], None, None, _STORAGEINFO_MSG, _CTRF_MSGS)), + + # no-rhsm but RHSMInfo defined (should be _RHSMINFO_MSG) + ((_SAEE, 'RHSM is not'), '1', testInData(_PACKAGES_MSGS, _RHSMINFO_MSG, _XFS_MSG, _STORAGEINFO_MSG, None)), + ((_SAEE, 'RHSM is not'), '1', testInData(_PACKAGES_MSGS[0], _RHSMINFO_MSG, _XFS_MSG, _STORAGEINFO_MSG, None)), + ((_SAEE, 'RHSM is not'), '1', testInData([], _RHSMINFO_MSG, _XFS_MSG, _STORAGEINFO_MSG, None)), + ((_SAEE, 'RHSM is not'), '1', testInData(_PACKAGES_MSGS, _RHSMINFO_MSG, None, _STORAGEINFO_MSG, None)), + ((_SAEE, 'RHSM is not'), '1', testInData(_PACKAGES_MSGS, _RHSMINFO_MSG, _XFS_MSG, _STORAGEINFO_MSG, _CTRF_MSGS)), + ((_SAEE, 'RHSM is not'), '1', testInData(_PACKAGES_MSGS[0], _RHSMINFO_MSG, _XFS_MSG, + _STORAGEINFO_MSG, _CTRF_MSGS)), + ((_SAEE, 'RHSM is not'), '1', testInData([], _RHSMINFO_MSG, _XFS_MSG, _STORAGEINFO_MSG, _CTRF_MSGS)), + ((_SAEE, 'RHSM is not'), '1', testInData(_PACKAGES_MSGS, _RHSMINFO_MSG, None, _STORAGEINFO_MSG, _CTRF_MSGS)), + + # missing RHSMInfo but it should exist + # NOTE: should be this Error?! + ((_SAE, 'RHSM information'), '0', testInData(_PACKAGES_MSGS, None, _XFS_MSG, _STORAGEINFO_MSG, None)), + ((_SAE, 'RHSM information'), '0', testInData(_PACKAGES_MSGS, None, None, _STORAGEINFO_MSG, None)), + ((_SAE, 'RHSM information'), '0', testInData([], None, _XFS_MSG, _STORAGEINFO_MSG, None)), + ((_SAE, 'RHSM information'), '0', testInData([], None, None, _STORAGEINFO_MSG, None)), + + # in the end, error when StorageInfo is missing + ((_SAEE, 'No storage'), '0', testInData(_PACKAGES_MSGS, _RHSMINFO_MSG, _XFS_MSG, None, None)), + ((_SAEE, 'No storage'), '0', testInData(_PACKAGES_MSGS, _RHSMINFO_MSG, None, None, None)), + ((_SAEE, 'No storage'), '0', testInData([], _RHSMINFO_MSG, _XFS_MSG, None, None)), + ((_SAEE, 'No storage'), '0', testInData([], _RHSMINFO_MSG, None, None, None)), + ((_SAEE, 'No storage'), '0', testInData(_PACKAGES_MSGS, _RHSMINFO_MSG, _XFS_MSG, None, _CTRF_MSGS)), + ((_SAEE, 'No storage'), '0', testInData(_PACKAGES_MSGS, _RHSMINFO_MSG, None, None, _CTRF_MSGS)), + ((_SAEE, 'No storage'), '0', testInData([], _RHSMINFO_MSG, _XFS_MSG, None, _CTRF_MSGS)), + ((_SAEE, 'No storage'), '0', testInData([], _RHSMINFO_MSG, None, None, _CTRF_MSGS)), ]) -def test_consume_data(monkeypatch, raised, pkg_msgs, rhsm_info, xfs, storage): +def test_consume_data(monkeypatch, raised, no_rhsm, testdata): + # do not write never into testdata inside the test !! + xfs = testdata.xfs + custom_repofiles = testdata.custom_repofiles _exp_pkgs = {'dnf'} - if isinstance(pkg_msgs, list): - for msg in pkg_msgs: + if isinstance(testdata.pkg_msgs, list): + for msg in testdata.pkg_msgs: _exp_pkgs.update(msg.packages) else: - _exp_pkgs.update(pkg_msgs.packages) - mocked_consume = MockedConsume(pkg_msgs, rhsm_info, xfs, storage) + _exp_pkgs.update(testdata.pkg_msgs.packages) + mocked_consume = MockedConsume(testdata.pkg_msgs, + testdata.rhsm_info, + xfs, + testdata.storage, + custom_repofiles) monkeypatch.setattr(api, 'consume', mocked_consume) monkeypatch.setattr(api, 'current_logger', mocked_logger()) - monkeypatch.setattr(api, 'current_actor', CurrentActorMocked(envars={'LEAPP_NO_RHSM': '0'})) + monkeypatch.setattr(api, 'current_actor', CurrentActorMocked(envars={'LEAPP_NO_RHSM': no_rhsm})) if not xfs: xfs = models.XFSPresence() + if not custom_repofiles: + custom_repofiles = [] if not raised: - assert userspacegen._consume_data() == (_exp_pkgs, rhsm_info, xfs, storage) + result = userspacegen._InputData() + assert result.packages == _exp_pkgs + assert result.rhsm_info == testdata.rhsm_info + assert result.xfs_info == xfs + assert result.storage_info == testdata.storage + assert result.custom_repofiles == custom_repofiles assert not api.current_logger.warnmsg assert not api.current_logger.errmsg else: with pytest.raises(raised[0]) as err: - userspacegen._consume_data() + userspacegen._InputData() if isinstance(err.value, StopActorExecutionError): assert raised[1] in err.value.message else: @@ -218,17 +273,23 @@ def mocked_consume_data(): rhsm_info = _RHSMINFO_MSG xfs_info = models.XFSPresence() storage_info = models.StorageInfo() - return packages, rhsm_info, xfs_info, storage_info + custom_repofiles = [] + fields = ['packages', 'rhsm_info', 'xfs_info', 'storage_info', 'custom_repofiles'] + + return namedtuple('TestInData', fields)( + packages, rhsm_info, xfs_info, storage_info, custom_repofiles + ) # TODO: come up with additional tests for the main function def test_perform_ok(monkeypatch): repoids = ['repoidX', 'repoidY'] - monkeypatch.setattr(userspacegen, '_consume_data', mocked_consume_data) + monkeypatch.setattr(userspacegen, '_InputData', mocked_consume_data) monkeypatch.setattr(userspacegen, '_get_product_certificate_path', lambda: _DEFAULT_CERT_PATH) monkeypatch.setattr(overlaygen, 'create_source_overlay', MockedMountingBase) monkeypatch.setattr(userspacegen, '_gather_target_repositories', lambda *x: repoids) monkeypatch.setattr(userspacegen, '_create_target_userspace', lambda *x: None) + monkeypatch.setattr(api, 'current_actor', CurrentActorMocked(envars={'LEAPP_NO_RHSM': '0'})) monkeypatch.setattr(api, 'produce', testutils.produce_mocked()) userspacegen.perform() msg_target_repos = models.UsedTargetRepositories( From 539d34850d9550757ee6320a43b02b885f5e0eb2 Mon Sep 17 00:00:00 2001 From: Petr Stodulka Date: Mon, 6 Apr 2020 11:51:31 +0200 Subject: [PATCH 08/13] [RFE custom repos 7/9] yum/dnf: disable subscription-manager plugin if rhsm skpped This is part of the scenario, when user wants to use only custom yum/dnf repositories (rhsm is skipped), but the system is subscribed in the same time. YUm by default uses the subscription-manager plugin; DNF uses it when configured. If the plugin is loaded, the redhat.repo file will be regenerated (e.g. it could lead into unexpected duplicated). As well, maybe it could happen that an error will be raised because of troubles to obtain expected data (e.g.). So we disable it). --- .../el7toel8/actors/removeleftoverpackages/actor.py | 4 ++++ .../targetuserspacecreator/libraries/userspacegen.py | 2 ++ repos/system_upgrade/el7toel8/libraries/dnfplugin.py | 7 ++++++- 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/repos/system_upgrade/el7toel8/actors/removeleftoverpackages/actor.py b/repos/system_upgrade/el7toel8/actors/removeleftoverpackages/actor.py index f38db2e86b..774d9474c7 100644 --- a/repos/system_upgrade/el7toel8/actors/removeleftoverpackages/actor.py +++ b/repos/system_upgrade/el7toel8/actors/removeleftoverpackages/actor.py @@ -1,5 +1,6 @@ from leapp.actors import Actor from leapp.libraries import stdlib +from leapp.libraries.common import rhsm from leapp.libraries.common.rpms import get_installed_rpms from leapp.models import LeftoverPackages, RemovedPackages, RPM from leapp.reporting import Report @@ -29,6 +30,9 @@ def process(self): to_remove = ['-'.join([pkg.name, pkg.version, pkg.release]) for pkg in leftover_packages.items] cmd = ['dnf', 'remove', '-y', '--noautoremove'] + to_remove + if rhsm.skip_rhsm(): + # ensure we don't use suscription-manager when it should be skipped + cmd += ['--disableplugin', 'subscription-manager'] try: stdlib.run(cmd) except stdlib.CalledProcessError: diff --git a/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/libraries/userspacegen.py b/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/libraries/userspacegen.py index 24b7a48e58..a59c149598 100644 --- a/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/libraries/userspacegen.py +++ b/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/libraries/userspacegen.py @@ -83,6 +83,8 @@ def prepare_target_userspace(context, userspace_dir, enabled_repos, packages): ] + repos_opt + packages if config.is_verbose(): cmd.append('-v') + if rhsm.skip_rhsm(): + cmd += ['--disableplugin', 'subscription-manager'] try: context.call(cmd, callback_raw=utils.logging_handler) except CalledProcessError as exc: diff --git a/repos/system_upgrade/el7toel8/libraries/dnfplugin.py b/repos/system_upgrade/el7toel8/libraries/dnfplugin.py index 0f2d312c3f..31d7b63c33 100644 --- a/repos/system_upgrade/el7toel8/libraries/dnfplugin.py +++ b/repos/system_upgrade/el7toel8/libraries/dnfplugin.py @@ -5,7 +5,7 @@ import shutil from leapp.exceptions import StopActorExecutionError -from leapp.libraries.common import guards, mounting, overlaygen, utils +from leapp.libraries.common import guards, mounting, overlaygen, rhsm, utils from leapp.libraries.stdlib import CalledProcessError, api, config DNF_PLUGIN_NAME = 'rhel_upgrade.py' @@ -97,6 +97,7 @@ def _transaction(context, stage, target_repoids, tasks, test=False, cmd_prefix=N create_config(context=context, target_repoids=target_repoids, debug=config.is_debug(), test=test, tasks=tasks) backup_config(context=context) + # FIXME: rhsm with guards.guarded_execution(guards.connection_guard(), guards.space_guard()): cmd = [ '/usr/bin/dnf', @@ -106,6 +107,8 @@ def _transaction(context, stage, target_repoids, tasks, test=False, cmd_prefix=N ] if config.is_verbose(): cmd.append('-v') + if rhsm.skip_rhsm(): + cmd += ['--disableplugin', 'subscription-manager'] if cmd_prefix: cmd = cmd_prefix + cmd try: @@ -159,6 +162,8 @@ def install_initramdisk_requirements(packages, target_userspace_info, used_repos ] + repos_opt + list(packages) if config.is_verbose(): cmd.append('-v') + if rhsm.skip_rhsm(): + cmd += ['--disableplugin', 'subscription-manager'] context.call(cmd) From 8774bd759cb980e8c47c6cf52da91d5ed859aeae Mon Sep 17 00:00:00 2001 From: Petr Stodulka Date: Mon, 6 Apr 2020 18:23:11 +0200 Subject: [PATCH 09/13] [RFE custom repos 8/9] update texts and reports regarding the --no-rhsm The content (target repositories) for the target system is supposed to be delivered using RHSM by default. However it is possible to skip it using the --no-rhsm option (or LEAPP_NO_RHSM=1). User should be notified that if problem is encountered around rhsm (e.g. system is not registered), there is possibility to skip RHSM and provide the RHEL 8 content via custom repositories. Modify all related texts and reports. --- .../el7toel8/actors/checkrhsmsku/actor.py | 3 +- .../actors/checkrhsmsku/libraries/library.py | 9 +++-- .../actors/reportsettargetrelease/actor.py | 7 +++- .../libraries/library.py | 36 +++++++++++++++++-- .../tests/test_targetreleasereport.py | 12 +++++++ .../libraries/userspacegen.py | 14 +++++--- .../system_upgrade/el7toel8/libraries/rhsm.py | 10 +++++- 7 files changed, 79 insertions(+), 12 deletions(-) diff --git a/repos/system_upgrade/el7toel8/actors/checkrhsmsku/actor.py b/repos/system_upgrade/el7toel8/actors/checkrhsmsku/actor.py index 1f812f5de3..c84ba8f662 100644 --- a/repos/system_upgrade/el7toel8/actors/checkrhsmsku/actor.py +++ b/repos/system_upgrade/el7toel8/actors/checkrhsmsku/actor.py @@ -9,7 +9,8 @@ class CheckRedHatSubscriptionManagerSKU(Actor): Ensure the system is subscribed to the subscription manager This actor verifies that the system is correctly subscribed to via the Red Hat Subscription Manager and - has attached SKUs. The actor will inhibit the upgrade if there are none. + has attached SKUs. The actor will inhibit the upgrade if there are none and RHSM is not supposed + to be skipped. """ name = 'check_rhsmsku' diff --git a/repos/system_upgrade/el7toel8/actors/checkrhsmsku/libraries/library.py b/repos/system_upgrade/el7toel8/actors/checkrhsmsku/libraries/library.py index d3377117e5..08434f400b 100644 --- a/repos/system_upgrade/el7toel8/actors/checkrhsmsku/libraries/library.py +++ b/repos/system_upgrade/el7toel8/actors/checkrhsmsku/libraries/library.py @@ -12,13 +12,16 @@ def process(): create_report([ reporting.Title('The system is not registered or subscribed.'), reporting.Summary( - 'The system has to be registered and subscribed to be able to proceed the upgrade.' + 'The system has to be registered and subscribed to be able to proceed' + ' with the upgrade, unless the --no-rhsm option is specified when' + ' executing leapp.' ), reporting.Severity(reporting.Severity.HIGH), reporting.Tags([reporting.Tags.SANITY]), reporting.Flags([reporting.Flags.INHIBITOR]), reporting.Remediation( - hint='Register your system with the subscription-manager tool and attach it to proper SKUs' - ' to be able to proceed the upgrade.'), + hint='Register your system with the subscription-manager tool and attach' + ' proper SKUs to be able to proceed the upgrade or use the --no-rhsm' + ' leapp option if you want to provide target repositories by yourself.'), reporting.RelatedResource('package', 'subscription-manager') ]) diff --git a/repos/system_upgrade/el7toel8/actors/reportsettargetrelease/actor.py b/repos/system_upgrade/el7toel8/actors/reportsettargetrelease/actor.py index 0abba98d5e..33c5104717 100644 --- a/repos/system_upgrade/el7toel8/actors/reportsettargetrelease/actor.py +++ b/repos/system_upgrade/el7toel8/actors/reportsettargetrelease/actor.py @@ -6,7 +6,12 @@ class ReportSetTargetRelease(Actor): """ - Reports that a release will be set in the subscription-manager after the upgrade. + Reports information related to the release set in the subscription-manager after the upgrade. + + When using Red Hat subscription-manager (RHSM), the release is set by default + to the target version release. In case of skip of the RHSM (--no-rhsm), the + release stay as it is on the RHEL 7 and user has to handle it manually after + the upgrade. """ name = 'report_set_target_release' diff --git a/repos/system_upgrade/el7toel8/actors/reportsettargetrelease/libraries/library.py b/repos/system_upgrade/el7toel8/actors/reportsettargetrelease/libraries/library.py index d32eddd74a..242fb9fc48 100644 --- a/repos/system_upgrade/el7toel8/actors/reportsettargetrelease/libraries/library.py +++ b/repos/system_upgrade/el7toel8/actors/reportsettargetrelease/libraries/library.py @@ -1,9 +1,9 @@ from leapp import reporting from leapp.libraries.stdlib import api +from leapp.libraries.common import rhsm -def process(): - # TODO: skip if users are not using rhsm at all (RHELLEAPP-201) +def _report_set_release(): target_version = api.current_actor().configuration.version.target reporting.create_report([ reporting.Title( @@ -20,3 +20,35 @@ def process(): reporting.Tags([reporting.Tags.UPGRADE_PROCESS]), reporting.RelatedResource('package', 'subscription-manager') ]) + + +def _report_unhandled_release(): + # TODO: set the POST group after it's created. + target_version = api.current_actor().configuration.version.target + commands = [ + ['subscription-manager', 'release', '--unset'], + ['subscription-manager', 'release', '--set', target_version], + ] + hint = 'Set the new release (or unset it) after the upgrade using subscription-manager.' + reporting.create_report([ + reporting.Title( + 'The subscription-manager release is going to be kept as it is during the upgrade'), + reporting.Summary( + 'The upgrade is executed with the --no-rhsm option (or with' + ' the LEAPP_NO_RHSM environment variable). In this case, the subscription-manager' + ' will not be configured during the upgrade. If the system is subscribed and release' + ' is set already, you could encounter issues to get RHEL 8 content using DNF/YUM' + ' after the upgrade.' + ), + reporting.Severity(reporting.Severity.LOW), + reporting.Remediation(commands=commands, hint=hint), + reporting.Tags([reporting.Tags.UPGRADE_PROCESS]), + reporting.RelatedResource('package', 'subscription-manager') + ]) + + +def process(): + if rhsm.skip_rhsm(): + _report_unhandled_release() + else: + _report_set_release() diff --git a/repos/system_upgrade/el7toel8/actors/reportsettargetrelease/tests/test_targetreleasereport.py b/repos/system_upgrade/el7toel8/actors/reportsettargetrelease/tests/test_targetreleasereport.py index 587e4d559c..98522a2b5f 100644 --- a/repos/system_upgrade/el7toel8/actors/reportsettargetrelease/tests/test_targetreleasereport.py +++ b/repos/system_upgrade/el7toel8/actors/reportsettargetrelease/tests/test_targetreleasereport.py @@ -4,6 +4,7 @@ from leapp import reporting from leapp.libraries.actor import library +from leapp.libraries.common import rhsm from leapp.libraries.common.testutils import create_report_mocked from leapp.libraries.stdlib import api @@ -21,8 +22,19 @@ def __call__(self): @pytest.mark.parametrize('version', ['8.{}'.format(i) for i in range(4)]) def test_report_target_version(monkeypatch, version): monkeypatch.setattr(api, 'current_actor', CurrentActorMocked(dst_ver=version)) + monkeypatch.setattr(rhsm, 'skip_rhsm', lambda: False) monkeypatch.setattr(reporting, 'create_report', create_report_mocked()) SUMMARY_FMT = 'will be set to {}.' library.process() assert reporting.create_report.called == 1 assert SUMMARY_FMT.format(version) in reporting.create_report.report_fields['summary'] + assert 'is going to be set' in reporting.create_report.report_fields['title'] + + +def test_report_unhandled_release(monkeypatch): + monkeypatch.setattr(api, 'current_actor', CurrentActorMocked(dst_ver='8.1')) + monkeypatch.setattr(rhsm, 'skip_rhsm', lambda: True) + monkeypatch.setattr(reporting, 'create_report', create_report_mocked()) + library.process() + assert reporting.create_report.called == 1 + assert 'is going to be kept' in reporting.create_report.report_fields['title'] diff --git a/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/libraries/userspacegen.py b/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/libraries/userspacegen.py index a59c149598..38045dcc65 100644 --- a/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/libraries/userspacegen.py +++ b/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/libraries/userspacegen.py @@ -205,10 +205,13 @@ def gather_target_repositories(context): details={ # FIXME: update the text - mention the possibility of custom repos 'hint': ('It is required to have RHEL repositories on the system' - ' provided by the subscription-manager. Possibly you' + ' provided by the subscription-manager unless the --no-rhsm' + ' options is specified. Possibly you' ' are missing a valid SKU for the target system or network' ' connection failed. Check whether your system is attached' - ' to a valid SKU providing RHEL 8 repositories.') + ' to a valid SKU providing RHEL 8 repositories.' + ' In case the Satellite is used, read the upgrade documentation' + ' to setup the satellite and the system properly.') } ) else: @@ -238,7 +241,10 @@ def gather_target_repositories(context): message='There are no enabled target repositories for the upgrade process to proceed.', details={'hint': ( 'Ensure your system is correctly registered with the subscription manager and that' - ' your current subscription is entitled to install the requested target version {version}' + ' your current subscription is entitled to install the requested target version {version}.' + ' In case the --no-rhsm option (or the LEAPP_NO_RHSM=1 environment variable is set)' + ' ensure the custom repository file is provided regarding the documentation with' + ' properly defined repositories.' ).format(version=api.current_actor().configuration.version.target) } ) @@ -274,7 +280,7 @@ def _gather_target_repositories(context, indata, prod_cert_path): :type context: mounting.IsolatedActions class :param indata: majority of input data for the actor :type indata: class _InputData - :param prod_cert_path: path where is stored the target product cert + :param prod_cert_path: path where the target product cert is stored :type prod_cert_path: string """ rhsm.set_container_mode(context) diff --git a/repos/system_upgrade/el7toel8/libraries/rhsm.py b/repos/system_upgrade/el7toel8/libraries/rhsm.py index 7968346880..2851fc4656 100644 --- a/repos/system_upgrade/el7toel8/libraries/rhsm.py +++ b/repos/system_upgrade/el7toel8/libraries/rhsm.py @@ -70,12 +70,20 @@ def _handle_rhsm_exceptions(hint=None): } ) except CalledProcessError as e: + _def_hint = ( + 'Please ensure you have a valid RHEL subscription and your network is up.' + ' If you are using proxy for Red Hat subscription-manager, please make sure' + ' it is specified inside the /etc/rhsm/rhsm.conf file.' + ' Or use the --no-rhsm option when running leapp, if you do not want to' + ' use subscription-manager for the in-place upgrade and you want to' + ' deliver all target repositories by yourself.' + ) raise StopActorExecutionError( message='A subscription-manager command failed to execute', details={ 'details': str(e), 'stderr': e.stderr, - 'hint': hint or 'Please ensure you have a valid RHEL subscription and your network is up.' + 'hint': hint or _def_hint } ) From 5e0a6936799063183c70b6632462513d250fc607 Mon Sep 17 00:00:00 2001 From: Petr Stodulka Date: Tue, 7 Apr 2020 00:24:17 +0200 Subject: [PATCH 10/13] [RFE custom repos 9/9] targetuserspacecreator: add checks for custom target repositories Previously when any custom target repository was not available inside the container (read, was not defined in a repo file), the fact was ignored. Currently, as user is able to proceed the upgrade without RHSM, completely, we should focus more on these values as well, otherwise it could happen we will not be able to create the target userspace (which in such case could end with ugly pretty-long output of errors immadiately after the fail of the bootstrap installation). As well, if rhsm is skipped, check whether there are not duplicate repository definitions inside the container. If so, inhibit the upgrade (as it happens in case when RHSM is used). Regarding the current state, the new issue has been created refactor and modify significantly the actor and related labrares. Issue: #486 --- .../actors/targetuserspacecreator/actor.py | 4 +- .../libraries/userspacegen.py | 182 ++++++++++++++---- .../system_upgrade/el7toel8/libraries/rhsm.py | 6 + 3 files changed, 150 insertions(+), 42 deletions(-) diff --git a/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/actor.py b/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/actor.py index 4831ad7c9b..67ff239a72 100644 --- a/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/actor.py +++ b/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/actor.py @@ -4,7 +4,7 @@ from leapp.models import (CustomTargetRepositoryFile, RepositoriesMap, RequiredTargetUserspacePackages, RHSMInfo, StorageInfo, TargetRepositories, TargetUserSpaceInfo, UsedTargetRepositories, - XFSPresence) + XFSPresence, Report) from leapp.tags import IPUWorkflowTag, TargetTransactionFactsPhaseTag @@ -22,7 +22,7 @@ class TargetUserspaceCreator(Actor): name = 'target_userspace_creator' consumes = (CustomTargetRepositoryFile, RepositoriesMap, RequiredTargetUserspacePackages, StorageInfo, RHSMInfo, TargetRepositories, XFSPresence) - produces = (TargetUserSpaceInfo, UsedTargetRepositories) + produces = (TargetUserSpaceInfo, UsedTargetRepositories, Report) tags = (IPUWorkflowTag, TargetTransactionFactsPhaseTag) def process(self): diff --git a/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/libraries/userspacegen.py b/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/libraries/userspacegen.py index 38045dcc65..0f588ff000 100644 --- a/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/libraries/userspacegen.py +++ b/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/libraries/userspacegen.py @@ -1,9 +1,10 @@ import itertools import os +from leapp import reporting from leapp.exceptions import StopActorExecution, StopActorExecutionError from leapp.libraries.actor import constants -from leapp.libraries.common import dnfplugin, mounting, overlaygen, rhsm, utils +from leapp.libraries.common import dnfplugin, mounting, overlaygen, repofileutils, rhsm, utils from leapp.libraries.common.config import get_product_type, get_env from leapp.libraries.stdlib import CalledProcessError, api, config, run from leapp.models import (CustomTargetRepositoryFile, RequiredTargetUserspacePackages, RHSMInfo, @@ -11,6 +12,30 @@ UsedTargetRepositories, UsedTargetRepository, XFSPresence) +# TODO: "refactor" (modify) the library significantly +# The current shape is really bad and ineffective (duplicit parsing +# of repofiles). The library is doing 3 (5) things: +# # (0.) consume process input data +# # 1. prepare the first container, to be able to obtain repositories for the +# # target system (this is extra neededwhen rhsm is used, but not reason to +# # do such thing only when rhsm is used. Be persistant here +# # 2. gather target repositories that should AND can be used +# # - basically here is the main thing that is PITA; I started +# # the refactoring but realized that it needs much more changes because +# # of RHSM... +# # 3. create the target userspace bootstrap +# # (4.) produce messages with the data +# +# Because of the lack of time, I am extending the current bad situation, +# but after the release, the related code should be really refactored. +# It would be probably ideal, if this and other actors in the current and the +# next phase are modified properly and we could create inhibitors in the check +# phase and keep everything on the report. But currently it seems it doesn't +# worth to invest so much energy into it. So let's just make this really +# readable (includes split of the functionality into several libraries) +# and do not mess. +# Issue: #486 + PROD_CERTS_FOLDER = 'prod-certs' @@ -53,8 +78,6 @@ def _consume_data(self): # other actors is wrong really raise StopActorExecutionError("RHSM is not handled but the RHSMInfo message has been produced.") - # list comprehension needed on Py2 - self.custom_repofiles = list(api.consume(CustomTargetRepositoryFile)) self.xfs_info = next(api.consume(XFSPresence), XFSPresence()) self.storage_info = next(api.consume(StorageInfo), None) @@ -182,59 +205,124 @@ def _create_target_userspace_directories(target_userspace): ) +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. + """ + # TODO: this is is duplicate of rhsm._inhibit_on_duplicate_repos + # Issue: #486 + duplicates = repofileutils.get_duplicate_repositories(repofiles).keys() + + if not duplicates: + return + list_separator_fmt = '\n - ' + api.current_logger().warn('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 inside the' + ' "upgrade" container:{0}{1}' + .format(list_separator_fmt, list_separator_fmt.join(duplicates)) + ), + reporting.Severity(reporting.Severity.MEDIUM), + reporting.Tags([reporting.Tags.REPOSITORY]), + reporting.Flags([reporting.Flags.INHIBITOR]), + reporting.Remediation(hint=( + 'Remove the duplicate repository definitions or change repoids of' + ' conflicting repositories on the system to prevent the' + ' conflict.' + ) + ) + ]) + + +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): + # FIXME: check that required repo IDs (baseos, appstream) + # + or check that all required RHEL repo IDs are available. + if rhsm.skip_rhsm(): + return [] + # Get the RHSM repos available in the RHEL 8 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: + raise StopActorExecutionError( + message='Cannot find required basic RHEL 8 repositories.', + details={ + 'hint': ('It is required to have RHEL repositories on the system' + ' provided by the subscription-manager unless the --no-rhsm' + ' options is specified. Possibly you' + ' are missing a valid SKU for the target system or network' + ' connection failed. Check whether your system is attached' + ' to a valid SKU providing RHEL 8 repositories.' + ' In case the Satellite is used, read the upgrade documentation' + ' to setup the satellite and the system properly.') + } + ) + return set(repoids) + + def gather_target_repositories(context): """ - Perform basic checks on requirements for RHSM repositories and return the list of target repository ids to use - during the upgrade. + 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) """ - # FIXME: check that required repo IDs (baseos, appstream) - # + or check that all required RHEL repo IDs are available. - if not rhsm.skip_rhsm(): - # Get the RHSM repos available in the RHEL 8 container - # TODO: very similar thing should happens for all other repofiles in container - # - available_repos = rhsm.get_available_repo_ids(context) - if not available_repos or len(available_repos) < 2: - raise StopActorExecutionError( - message='Cannot find required basic RHEL 8 repositories.', - details={ - # FIXME: update the text - mention the possibility of custom repos - 'hint': ('It is required to have RHEL repositories on the system' - ' provided by the subscription-manager unless the --no-rhsm' - ' options is specified. Possibly you' - ' are missing a valid SKU for the target system or network' - ' connection failed. Check whether your system is attached' - ' to a valid SKU providing RHEL 8 repositories.' - ' In case the Satellite is used, read the upgrade documentation' - ' to setup the satellite and the system properly.') - } - ) - else: - available_repos = [] + rhsm_available_repoids = _get_rhsm_available_repoids(context) + 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 available_repos: + if rhel_repo.repoid in rhsm_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. - # The StopActorExecutionError called above might be moved here. + # 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 everytime 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: - # FIXME: this have to be done for the PR !! - # # now it works well when the custom repofile is used, but - # # we should require that all those repositories really exists - # TODO: complete processing of custom repositories - # TODO: should check available_target_repoids + additional custom repos - # + outside of rhsm.. - # #if custom_repo.repoid in available_target_repoids: - target_repoids.append(custom_repo.repoid) + 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: raise StopActorExecutionError( @@ -248,6 +336,20 @@ def gather_target_repositories(context): ).format(version=api.current_actor().configuration.version.target) } ) + if missing_custom_repoids: + raise StopActorExecutionError( + message='Some required custom target repositories are not available.', + details={'hint': ( + ' The most probably you are using custom or third party actor' + ' that produces CustomTargetRepository message. However,' + ' inside the upgrade container, we are not able to find such' + ' repository inside any repository file. Consider use of the' + ' custom repository file regarding the official upgrade' + ' documentation.' + ) + } + ) + return target_repoids diff --git a/repos/system_upgrade/el7toel8/libraries/rhsm.py b/repos/system_upgrade/el7toel8/libraries/rhsm.py index 2851fc4656..e1a8746c4f 100644 --- a/repos/system_upgrade/el7toel8/libraries/rhsm.py +++ b/repos/system_upgrade/el7toel8/libraries/rhsm.py @@ -140,6 +140,12 @@ 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: From e166dd9836d837934f9dfa3c763e07179886edc1 Mon Sep 17 00:00:00 2001 From: Petr Stodulka Date: Wed, 8 Apr 2020 10:16:06 +0200 Subject: [PATCH 11/13] [RFE enablerepo 1/2] Add scanclienablerepo actor The actor produce CustomTargetRepository messages for repoids obtained from the cmdline when user use the --enablerepo option. The values are stored inside LEAPP_ENABLE_REPOS envar now, which is stored inside the configuration of the actor. --- .../actors/scanclienablerepo/actor.py | 18 +++++ .../scanclienablerepo/libraries/library.py | 11 +++ .../scanclienablerepo/tests/test_unit.py | 75 +++++++++++++++++++ 3 files changed, 104 insertions(+) create mode 100644 repos/system_upgrade/el7toel8/actors/scanclienablerepo/actor.py create mode 100644 repos/system_upgrade/el7toel8/actors/scanclienablerepo/libraries/library.py create mode 100644 repos/system_upgrade/el7toel8/actors/scanclienablerepo/tests/test_unit.py diff --git a/repos/system_upgrade/el7toel8/actors/scanclienablerepo/actor.py b/repos/system_upgrade/el7toel8/actors/scanclienablerepo/actor.py new file mode 100644 index 0000000000..7d3d6fb3ca --- /dev/null +++ b/repos/system_upgrade/el7toel8/actors/scanclienablerepo/actor.py @@ -0,0 +1,18 @@ +from leapp.actors import Actor +from leapp.libraries.actor import library +from leapp.models import CustomTargetRepository +from leapp.tags import FactsPhaseTag, IPUWorkflowTag + + +class ScanCLIenablrepo(Actor): + """ + Produce CustomTargetRepository based on the LEAPP_ENABLE_REPOS in config. + """ + + name = 'scanclienablerepo' + consumes = () + produces = (CustomTargetRepository) + tags = (FactsPhaseTag, IPUWorkflowTag) + + def process(self): + library.process() diff --git a/repos/system_upgrade/el7toel8/actors/scanclienablerepo/libraries/library.py b/repos/system_upgrade/el7toel8/actors/scanclienablerepo/libraries/library.py new file mode 100644 index 0000000000..841c17df47 --- /dev/null +++ b/repos/system_upgrade/el7toel8/actors/scanclienablerepo/libraries/library.py @@ -0,0 +1,11 @@ +from leapp.libraries.stdlib import api +from leapp.libraries.common import config +from leapp.models import CustomTargetRepository + + +def process(): + if not config.get_env('LEAPP_ENABLE_REPOS'): + return + api.current_logger().info('The --enablerepo option has been used,') + for repoid in config.get_env('LEAPP_ENABLE_REPOS').split(','): + api.produce(CustomTargetRepository(repoid=repoid)) diff --git a/repos/system_upgrade/el7toel8/actors/scanclienablerepo/tests/test_unit.py b/repos/system_upgrade/el7toel8/actors/scanclienablerepo/tests/test_unit.py new file mode 100644 index 0000000000..171bf6b302 --- /dev/null +++ b/repos/system_upgrade/el7toel8/actors/scanclienablerepo/tests/test_unit.py @@ -0,0 +1,75 @@ +from collections import namedtuple +import os + +import pytest + +from leapp.libraries.actor import library +from leapp.libraries.common.config import architecture +from leapp.libraries.common.testutils import produce_mocked +from leapp.libraries.stdlib import api +from leapp.models import CustomTargetRepository +from leapp import models + + +class CurrentActorMocked(object): + def __init__(self, kernel='3.10.0-957.43.1.el7.x86_64', release_id='rhel', + src_ver='7.6', dst_ver='8.1', arch=architecture.ARCH_X86_64, + envars=None): + + if envars: + envarsList = [models.EnvVar(name=key, value=value) for key, value in envars.items()] + else: + envarsList = [] + + version = namedtuple('Version', ['source', 'target'])(src_ver, dst_ver) + os_release = namedtuple('OS_release', ['release_id', 'version_id'])(release_id, src_ver) + args = (version, kernel, os_release, arch, envarsList) + conf_fields = ['version', 'kernel', 'os_release', 'architecture', 'leapp_env_vars'] + self.configuration = namedtuple('configuration', conf_fields)(*args) + self._common_folder = '../../files' + + def __call__(self): + return self + + def get_common_folder_path(self, folder): + return os.path.join(self._common_folder, folder) + + +class LoggerMocked(object): + def __init__(self): + self.infomsg = None + self.debugmsg = None + + def info(self, msg): + self.infomsg = msg + + def debug(self, msg): + self.debugmsg = msg + + def __call__(self): + return self + + +def test_no_enabledrepos(monkeypatch): + monkeypatch.setattr(api, 'produce', produce_mocked()) + monkeypatch.setattr(api, 'current_logger', LoggerMocked()) + monkeypatch.setattr(api, 'current_actor', CurrentActorMocked()) + library.process() + assert not api.current_logger.infomsg + assert not api.produce.called + + +@pytest.mark.parametrize('envars,result', [ + ({'LEAPP_ENABLE_REPOS': 'repo1'}, [CustomTargetRepository(repoid='repo1')]), + ({'LEAPP_ENABLE_REPOS': 'repo1,repo2'}, [CustomTargetRepository(repoid='repo1'), + CustomTargetRepository(repoid='repo2')]), +]) +def test_enabledrepos(monkeypatch, envars, result): + monkeypatch.setattr(api, 'produce', produce_mocked()) + monkeypatch.setattr(api, 'current_logger', LoggerMocked()) + monkeypatch.setattr(api, 'current_actor', CurrentActorMocked(envars=envars)) + library.process() + assert api.current_logger.infomsg + assert api.produce.called == len(result) + for i in result: + assert i in api.produce.model_instances From a3ba615e660b102e8af3b85d16ac541fdce6f300 Mon Sep 17 00:00:00 2001 From: Petr Stodulka Date: Wed, 8 Apr 2020 12:22:17 +0200 Subject: [PATCH 12/13] [RFE enablerepo 2/2] update texts and checktargetrepos Update texts to inform even about the second way how to enable custom target repositories using the --enablerepo option for leapp, and do not produce info report about those new way, when already used this or custom repofile one. --- .../el7toel8/actors/checktargetrepos/actor.py | 27 +++++++++++-------- .../checktargetrepos/libraries/library.py | 24 ++++++++++------- .../libraries/userspacegen.py | 13 ++++++--- 3 files changed, 40 insertions(+), 24 deletions(-) diff --git a/repos/system_upgrade/el7toel8/actors/checktargetrepos/actor.py b/repos/system_upgrade/el7toel8/actors/checktargetrepos/actor.py index 08e766a954..c128b91f55 100644 --- a/repos/system_upgrade/el7toel8/actors/checktargetrepos/actor.py +++ b/repos/system_upgrade/el7toel8/actors/checktargetrepos/actor.py @@ -8,18 +8,23 @@ class Checktargetrepos(Actor): """ Check whether target yum repositories are specified. - RHSM | CTR | CTRF || result - -----+-----+------++------- - Yes | --- | ---- || - - -----+-----+------++------- - No | No | No || inhibit - -----+-----+------++------- - No | No | Yes || inhibit - -----+-----+------++------- - No | Yes | No || warn/report info - -----+-----+------++------- - No | Yes | Yes || - + RHSM | ER | CTR | CTRF || result + -----+----+-----+------++------- + Yes | -- | --- | ---- || - + -----+----+-----+------++------- + No | -- | No | No || inhibit + -----+----+-----+------++------- + No | -- | No | Yes || inhibit + -----+----+-----+------++------- + No | No | Yes | No || warn/report info + -----+----+-----+------++------- + No | No | Yes | Yes || - + -----+----+-----+------++------- + No | Yes| Yes | No || - + -----+----+-----+------++------- + No | Yes| Yes | Yes || - + ER - config.get_env('LEAPP_ENABLE_REPOS') is non-empty CTR - CustomTargetRepositories found CTRF - the expected CustomTargetRepositoryFile found RHSM - RHSM is used (it is not skipped) diff --git a/repos/system_upgrade/el7toel8/actors/checktargetrepos/libraries/library.py b/repos/system_upgrade/el7toel8/actors/checktargetrepos/libraries/library.py index 7ef599965e..b91fd885e2 100644 --- a/repos/system_upgrade/el7toel8/actors/checktargetrepos/libraries/library.py +++ b/repos/system_upgrade/el7toel8/actors/checktargetrepos/libraries/library.py @@ -1,7 +1,7 @@ from leapp.models import CustomTargetRepositoryFile, TargetRepositories from leapp.libraries.stdlib import api from leapp import reporting -from leapp.libraries.common import rhsm +from leapp.libraries.common import config, rhsm _IPU_DOC_URL = ('https://access.redhat.com/documentation/en-us/' @@ -24,6 +24,10 @@ def _the_custom_repofile_defined(): return False +def _the_enablerepo_option_used(): + return config.get_env('LEAPP_ENABLE_REPOS', None) is not None + + def process(): if not rhsm.skip_rhsm(): # getting RH repositories through RHSM; resolved by seatbelts @@ -33,6 +37,7 @@ def process(): # rhsm skipped; take your seatbelts please is_ctr = _any_custom_repo_defined() is_ctrf = _the_custom_repofile_defined() + is_re = _the_enablerepo_option_used() if not is_ctr: # no rhsm, no custom repositories.. this will really not work :) # TODO: add link to the RH article about use of custom repositories!! @@ -40,9 +45,9 @@ def process(): # will be described there or at least the link to the right document # will be delivered here. if is_ctrf: - summary_ctrf = 'The custom repository file has been detected. Maybe it is empty?' + summary_ctrf = '\n\nThe custom repository file has been detected. Maybe it is empty?' else: - summary_ctrf = 'The custom repository file has not been detected.' + summary_ctrf = '' reporting.create_report([ reporting.Title('Using RHSM has been skipped but no custom repositories have been delivered.'), reporting.Summary( @@ -66,17 +71,18 @@ def process(): reporting.ExternalLink(url=_IPU_DOC_URL, title='UPGRADING TO RHEL 8'), reporting.RelatedResource('file', CUSTOM_REPO_PATH), ]) - elif not is_ctrf: + elif not (is_ctrf or is_re): # Some custom repositories have been discovered, but the custom repo - # file not. Inform about the official recommended way. + # file not - neither the --enablerepo option is usedd. Inform about + # the official recommended way. reporting.create_report([ - reporting.Title('CustomTargetRepositories discovered, but the CustomTargetRepositoryFile is missing.'), + reporting.Title('CustomTargetRepositories discovered, but no new provided mechanisms used.'), reporting.Summary( 'Red Hat provides now official way how to use custom' ' repositories during the in-place upgrade through' - ' the referred custom repository file. The CustomTargetRepositories' - ' have been detected (from custom actors?) but the repository' - ' file not.' + ' the referred custom repository file or through the' + ' --enablerepo option for leapp. The CustomTargetRepositories' + ' have been produced from custom (own) actors?' ), reporting.Remediation(hint=( 'Follow the new simple way to enable custom repositories' diff --git a/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/libraries/userspacegen.py b/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/libraries/userspacegen.py index 0f588ff000..d579e5dff7 100644 --- a/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/libraries/userspacegen.py +++ b/repos/system_upgrade/el7toel8/actors/targetuserspacecreator/libraries/userspacegen.py @@ -332,7 +332,9 @@ def gather_target_repositories(context): ' your current subscription is entitled to install the requested target version {version}.' ' In case the --no-rhsm option (or the LEAPP_NO_RHSM=1 environment variable is set)' ' ensure the custom repository file is provided regarding the documentation with' - ' properly defined repositories.' + ' properly defined repositories or in case repositories are already defined' + ' in any repofiles under /etc/yum.repos.d/ directory, use the --enablerepo option' + ' for leapp' ).format(version=api.current_actor().configuration.version.target) } ) @@ -341,11 +343,14 @@ def gather_target_repositories(context): message='Some required custom target repositories are not available.', details={'hint': ( ' The most probably you are using custom or third party actor' - ' that produces CustomTargetRepository message. However,' - ' inside the upgrade container, we are not able to find such' + ' that produces CustomTargetRepository message or you did a typo' + ' in one of repoids specified on command line for the leapp --enablerepo' + ' option.' + ' Inside the upgrade container, we are not able to find such' ' repository inside any repository file. Consider use of the' ' custom repository file regarding the official upgrade' - ' documentation.' + ' documentation or check whether you did not do a typo in any' + ' repoids you specified for the --enablerepo option of leapp.' ) } ) From 66b077e59c91618923093a55203c21952dcebc15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Reh=C3=A1k?= Date: Tue, 7 Apr 2020 17:31:52 +0100 Subject: [PATCH 13/13] Add tests for Checktargetrepos and reword report messages --- .../checktargetrepos/libraries/library.py | 11 +-- .../tests/test_checktargetrepos.py | 92 ++++++++++++++++++- 2 files changed, 94 insertions(+), 9 deletions(-) diff --git a/repos/system_upgrade/el7toel8/actors/checktargetrepos/libraries/library.py b/repos/system_upgrade/el7toel8/actors/checktargetrepos/libraries/library.py index b91fd885e2..1f012428b7 100644 --- a/repos/system_upgrade/el7toel8/actors/checktargetrepos/libraries/library.py +++ b/repos/system_upgrade/el7toel8/actors/checktargetrepos/libraries/library.py @@ -55,8 +55,8 @@ def process(): ' is not used (the --no-rhsm option or the LEAPP_NO_RHSM=1' ' environment variable has been set) so leapp is not able to' ' obtain YUM/DNF repositories with the content for the target' - ' system in the standard way and has to be delivered by user' - ' manually.' + ' system in the standard way. The content has to be delivered' + ' by the user manually.' ), reporting.Remediation(hint=( 'Create the repository file according to instructions in the' @@ -73,12 +73,12 @@ def process(): ]) elif not (is_ctrf or is_re): # Some custom repositories have been discovered, but the custom repo - # file not - neither the --enablerepo option is usedd. Inform about + # file not - neither the --enablerepo option is used. Inform about # the official recommended way. reporting.create_report([ - reporting.Title('CustomTargetRepositories discovered, but no new provided mechanisms used.'), + reporting.Title('Detected "CustomTargetRepositories" without using new provided mechanisms used.'), reporting.Summary( - 'Red Hat provides now official way how to use custom' + 'Red Hat now provides an official way for using custom' ' repositories during the in-place upgrade through' ' the referred custom repository file or through the' ' --enablerepo option for leapp. The CustomTargetRepositories' @@ -89,7 +89,6 @@ def process(): ' during the upgrade (see the referred document) or create' ' the empty custom repository file to acknowledge this report' ' message.' - )), reporting.Severity(reporting.Severity.INFO), reporting.ExternalLink(url=_IPU_DOC_URL, title='UPGRADING TO RHEL 8'), diff --git a/repos/system_upgrade/el7toel8/actors/checktargetrepos/tests/test_checktargetrepos.py b/repos/system_upgrade/el7toel8/actors/checktargetrepos/tests/test_checktargetrepos.py index 69626e4215..8db655250e 100644 --- a/repos/system_upgrade/el7toel8/actors/checktargetrepos/tests/test_checktargetrepos.py +++ b/repos/system_upgrade/el7toel8/actors/checktargetrepos/tests/test_checktargetrepos.py @@ -1,5 +1,91 @@ +from collections import namedtuple +import pytest -# TODO: unit tests are required (@drehak?) -def test_sth(): - pass +from leapp.models import (CustomTargetRepository, CustomTargetRepositoryFile, EnvVar, Report, + RepositoryData, RHELTargetRepository, TargetRepositories) +from leapp.libraries.actor import library +from leapp import reporting +from leapp.libraries.stdlib import api +from leapp.libraries.common import rhsm +from leapp.libraries.common.testutils import create_report_mocked + + +class MockedConsume(object): + def __init__(self, *args): + self._msgs = [] + for arg in args: + if not arg: + continue + if isinstance(arg, list): + self._msgs.extend(arg) + else: + self._msgs.append(arg) + + def __call__(self, model): + return iter([msg for msg in self._msgs if isinstance(msg, model)]) + + +class CurrentActorMocked(object): + def __init__(self, envars=None): + if envars: + envarsList = [EnvVar(name=key, value=value) for key, value in envars.items()] + else: + envarsList = [] + self.configuration = namedtuple('configuration', ['leapp_env_vars'])(envarsList) + + def __call__(self): + return self + + +_RHEL_REPOS = [ + RHELTargetRepository(repoid='repo1'), + RHELTargetRepository(repoid='repo2'), + RHELTargetRepository(repoid='repo3'), + RHELTargetRepository(repoid='repo4'), +] + +_CUSTOM_REPOS = [ + CustomTargetRepository(repoid='repo1', name='repo1name', baseurl='repo1url', enabled=True), + CustomTargetRepository(repoid='repo2', name='repo2name', baseurl='repo2url', enabled=False), + CustomTargetRepository(repoid='repo3', name='repo3name', baseurl=None, enabled=True), + CustomTargetRepository(repoid='repo4', name='repo4name', baseurl=None, enabled=True), +] + +_TARGET_REPOS_CUSTOM = TargetRepositories(rhel_repos=_RHEL_REPOS, custom_repos=_CUSTOM_REPOS) +_TARGET_REPOS_NO_CUSTOM = TargetRepositories(rhel_repos=_RHEL_REPOS) +_CUSTOM_TARGET_REPOFILE = CustomTargetRepositoryFile(file='/etc/leapp/files/leapp_upgrade_repositories.repo') + + +def test_checktargetrepos_rhsm(monkeypatch): + monkeypatch.setattr(reporting, 'create_report', create_report_mocked()) + monkeypatch.setattr(rhsm, 'skip_rhsm', lambda: False) + monkeypatch.setattr(api, 'consume', MockedConsume()) + library.process() + assert reporting.create_report.called == 0 + + +@pytest.mark.parametrize('enable_repos', [True, False]) +@pytest.mark.parametrize('custom_target_repos', [True, False]) +@pytest.mark.parametrize('custom_target_repofile', [True, False]) +def test_checktargetrepos_no_rhsm(monkeypatch, enable_repos, custom_target_repos, custom_target_repofile): + mocked_consume = MockedConsume(_TARGET_REPOS_CUSTOM if custom_target_repos else _TARGET_REPOS_NO_CUSTOM) + if custom_target_repofile: + mocked_consume._msgs.append(_CUSTOM_TARGET_REPOFILE) + envvars = {'LEAPP_ENABLE_REPOS': 'hill,spencer'} if enable_repos else {} + monkeypatch.setattr(api, 'current_actor', CurrentActorMocked(envvars)) + + monkeypatch.setattr(reporting, 'create_report', create_report_mocked()) + monkeypatch.setattr(rhsm, 'skip_rhsm', lambda: True) + monkeypatch.setattr(api, 'consume', mocked_consume) + + library.process() + + if not custom_target_repos: + assert reporting.create_report.called == 1 + assert 'inhibitor' in reporting.create_report.report_fields.get('flags', []) + elif not enable_repos and custom_target_repos and not custom_target_repofile: + assert reporting.create_report.called == 1 + assert 'inhibitor' not in reporting.create_report.report_fields.get('flags', []) + else: + assert reporting.create_report.called == 0