From d603f0421eabdd24e12f38f7df13c4f902bb9e94 Mon Sep 17 00:00:00 2001 From: Rodolfo Olivieri Date: Fri, 18 Oct 2024 08:53:20 -0300 Subject: [PATCH] [RHELC-1740] Use toolopts instead of env var (#1387) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add deprecation notice about environment variables With the most recent work to migrate all environment variables to the configuration file, we need now to warn the user that the variables will be deprecated in order to use configuration file as our main place to track configuarations. This patch is introducing a very lightweight change to make sure that we will be able modify the code in the future to remove all environment variables. * Use toolopts instead of environment variable Following the strategy of moving all the environment variables to be inside the configuration file, this patch introduces the ability of converting the env_var to their toolopts counterpart. This will help in the deprecation later down the road when all users are comfortable enough using the config file. * Apply suggestions from code review Co-authored-by: Adam Hošek --------- Co-authored-by: Adam Hošek --- .../actions/post_conversion/hostmetering.py | 23 +++++----- .../actions/pre_ponr_changes/backup_system.py | 4 +- .../pre_ponr_changes/kernel_modules.py | 4 +- .../system_checks/convert2rhel_latest.py | 4 +- .../system_checks/is_loaded_kernel_latest.py | 4 +- .../actions/system_checks/package_updates.py | 5 ++- .../actions/system_checks/tainted_kmods.py | 6 +-- convert2rhel/toolopts/__init__.py | 16 ++++++- .../post_conversion/hostmetering_test.py | 44 +++++++++++++++---- .../pre_ponr_changes/backup_system_test.py | 42 +++++++++--------- .../pre_ponr_changes/kernel_modules_test.py | 7 ++- .../system_checks/convert2rhel_latest_test.py | 11 +++-- .../is_loaded_kernel_latest_test.py | 35 ++++++++++++--- .../system_checks/package_updates_test.py | 8 +++- .../system_checks/tainted_kmods_test.py | 6 +-- convert2rhel/unit_tests/conftest.py | 23 +++++++++- convert2rhel/unit_tests/utils/utils_test.py | 39 +++++++++++++++- convert2rhel/utils/__init__.py | 18 ++++++-- 18 files changed, 226 insertions(+), 73 deletions(-) diff --git a/convert2rhel/actions/post_conversion/hostmetering.py b/convert2rhel/actions/post_conversion/hostmetering.py index 7a77b49f30..4a9d958226 100644 --- a/convert2rhel/actions/post_conversion/hostmetering.py +++ b/convert2rhel/actions/post_conversion/hostmetering.py @@ -15,12 +15,12 @@ __metaclass__ = type - from convert2rhel import actions, systeminfo from convert2rhel.logger import root_logger from convert2rhel.pkgmanager import call_yum_cmd from convert2rhel.subscription import get_rhsm_facts from convert2rhel.systeminfo import system_info +from convert2rhel.toolopts import tool_opts from convert2rhel.utils import run_subprocess, warn_deprecated_env @@ -35,7 +35,6 @@ class ConfigureHostMetering(actions.Action): """ id = "CONFIGURE_HOST_METERING_IF_NEEDED" - env_var = None # type: str|None def run(self): """ @@ -53,11 +52,11 @@ def run(self): super(ConfigureHostMetering, self).run() - self.env_var = warn_deprecated_env("CONVERT2RHEL_CONFIGURE_HOST_METERING") + warn_deprecated_env("CONVERT2RHEL_CONFIGURE_HOST_METERING") if not self._check_env_var(): return False - if system_info.version.major != 7 and self.env_var == "auto": + if system_info.version.major != 7 and tool_opts.configure_host_metering == "auto": logger.info("Did not perform host metering configuration. Only supported for RHEL 7.") self.add_message( level="INFO", @@ -69,7 +68,7 @@ def run(self): is_hyperscaler = self.is_running_on_hyperscaler() - if not is_hyperscaler and self.env_var == "auto": + if not is_hyperscaler and tool_opts.configure_host_metering == "auto": logger.info("Did not perform host-metering configuration.") self.add_message( level="INFO", @@ -142,7 +141,7 @@ def _check_env_var(self): :return: Return True if the value is equal to auto|force, otherwise False :rtype: bool """ - if self.env_var is None: + if tool_opts.configure_host_metering is None: logger.debug("CONVERT2RHEL_CONFIGURE_HOST_METERING was not set. Skipping it.") self.add_message( level="INFO", @@ -152,18 +151,20 @@ def _check_env_var(self): ) return False - if self.env_var not in ("force", "auto"): - logger.debug("Value for environment variable not recognized: {}".format(self.env_var)) + if tool_opts.configure_host_metering not in ("force", "auto"): + logger.debug("Value for environment variable not recognized: {}".format(tool_opts.configure_host_metering)) self.add_message( level="WARNING", id="UNRECOGNIZED_OPTION_CONFIGURE_HOST_METERING", title="Unrecognized option in CONVERT2RHEL_CONFIGURE_HOST_METERING environment variable.", - description="Environment variable {env_var} not recognized.".format(env_var=self.env_var), + description="Environment variable {env_var} not recognized.".format( + env_var=tool_opts.configure_host_metering + ), remediations="Set the option to `auto` value if you want to configure host metering.", ) return False - if self.env_var == "force": + if tool_opts.configure_host_metering == "force": logger.warning( "The `force' option has been used for the CONVERT2RHEL_CONFIGURE_HOST_METERING environment variable." " Please note that this option is mainly used for testing and will configure host-metering unconditionally. " @@ -177,7 +178,7 @@ def _check_env_var(self): " will configure host-metering unconditionally." " For generic usage please use the 'auto' option.", ) - elif self.env_var == "auto": + elif tool_opts.configure_host_metering == "auto": logger.debug("Automatic detection of host hyperscaler and configuration.") return True diff --git a/convert2rhel/actions/pre_ponr_changes/backup_system.py b/convert2rhel/actions/pre_ponr_changes/backup_system.py index a0429c826e..2bd8919399 100644 --- a/convert2rhel/actions/pre_ponr_changes/backup_system.py +++ b/convert2rhel/actions/pre_ponr_changes/backup_system.py @@ -26,6 +26,7 @@ from convert2rhel.redhatrelease import os_release_file, system_release_file from convert2rhel.repo import DEFAULT_DNF_VARS_DIR, DEFAULT_YUM_REPOFILE_DIR, DEFAULT_YUM_VARS_DIR from convert2rhel.systeminfo import system_info +from convert2rhel.toolopts import tool_opts from convert2rhel.utils import warn_deprecated_env from convert2rhel.utils.rpm import PRE_RPM_VA_LOG_FILENAME @@ -189,7 +190,8 @@ def _get_changed_package_files(self): output = f.read() # Catch the IOError due Python 2 compatibility except IOError as err: - if warn_deprecated_env("CONVERT2RHEL_INCOMPLETE_ROLLBACK"): + warn_deprecated_env("CONVERT2RHEL_INCOMPLETE_ROLLBACK") + if tool_opts.incomplete_rollback: logger.debug("Skipping backup of the package files. CONVERT2RHEL_INCOMPLETE_ROLLBACK detected.") # Return empty list results in no backup of the files return data diff --git a/convert2rhel/actions/pre_ponr_changes/kernel_modules.py b/convert2rhel/actions/pre_ponr_changes/kernel_modules.py index 640bbbfdd6..6cb0fba536 100644 --- a/convert2rhel/actions/pre_ponr_changes/kernel_modules.py +++ b/convert2rhel/actions/pre_ponr_changes/kernel_modules.py @@ -23,6 +23,7 @@ from convert2rhel import actions, pkghandler from convert2rhel.logger import root_logger from convert2rhel.systeminfo import system_info +from convert2rhel.toolopts import tool_opts from convert2rhel.utils import run_subprocess, warn_deprecated_env @@ -248,7 +249,8 @@ def run(self): # Check if we have the environment variable set, if we do, send a # warning and return. - if warn_deprecated_env("CONVERT2RHEL_ALLOW_UNAVAILABLE_KMODS"): + warn_deprecated_env("CONVERT2RHEL_ALLOW_UNAVAILABLE_KMODS") + if tool_opts.allow_unavailable_kmods: logger.warning( "Detected 'CONVERT2RHEL_ALLOW_UNAVAILABLE_KMODS' environment variable." " We will continue the conversion with the following kernel modules unavailable in RHEL:\n" diff --git a/convert2rhel/actions/system_checks/convert2rhel_latest.py b/convert2rhel/actions/system_checks/convert2rhel_latest.py index 6373f7cf7a..33705c648b 100644 --- a/convert2rhel/actions/system_checks/convert2rhel_latest.py +++ b/convert2rhel/actions/system_checks/convert2rhel_latest.py @@ -26,6 +26,7 @@ from convert2rhel.logger import root_logger from convert2rhel.pkghandler import parse_pkg_string from convert2rhel.systeminfo import system_info +from convert2rhel.toolopts import tool_opts from convert2rhel.utils import warn_deprecated_env @@ -174,7 +175,8 @@ def run(self): formatted_available_version = _format_EVR(*precise_available_version) if ver_compare < 0: - if warn_deprecated_env("CONVERT2RHEL_ALLOW_OLDER_VERSION"): + warn_deprecated_env("CONVERT2RHEL_ALLOW_OLDER_VERSION") + if tool_opts.allow_older_version: diagnosis = ( "You are currently running {} and the latest version of convert2rhel is {}.\n" "'CONVERT2RHEL_ALLOW_OLDER_VERSION' environment variable detected, continuing conversion".format( diff --git a/convert2rhel/actions/system_checks/is_loaded_kernel_latest.py b/convert2rhel/actions/system_checks/is_loaded_kernel_latest.py index e092a63fd9..98249281a9 100644 --- a/convert2rhel/actions/system_checks/is_loaded_kernel_latest.py +++ b/convert2rhel/actions/system_checks/is_loaded_kernel_latest.py @@ -20,6 +20,7 @@ from convert2rhel.logger import root_logger from convert2rhel.pkghandler import compare_package_versions from convert2rhel.systeminfo import system_info +from convert2rhel.toolopts import tool_opts from convert2rhel.utils import run_subprocess, warn_deprecated_env @@ -65,7 +66,8 @@ def run(self): # Repoquery failed to detected any kernel or kernel-core packages in it's repositories # we allow the user to provide a environment variable to override the functionality and proceed # with the conversion, otherwise, we just throw a critical logging to them. - if warn_deprecated_env("CONVERT2RHEL_SKIP_KERNEL_CURRENCY_CHECK"): + warn_deprecated_env("CONVERT2RHEL_SKIP_KERNEL_CURRENCY_CHECK") + if tool_opts.skip_kernel_currency_check: logger.warning( "Detected 'CONVERT2RHEL_SKIP_KERNEL_CURRENCY_CHECK' environment variable, we will skip " "the {} comparison.\n" diff --git a/convert2rhel/actions/system_checks/package_updates.py b/convert2rhel/actions/system_checks/package_updates.py index 67bcda3d5a..60c77de3dd 100644 --- a/convert2rhel/actions/system_checks/package_updates.py +++ b/convert2rhel/actions/system_checks/package_updates.py @@ -20,6 +20,7 @@ from convert2rhel.logger import root_logger from convert2rhel.pkghandler import get_total_packages_to_update from convert2rhel.systeminfo import system_info +from convert2rhel.toolopts import tool_opts from convert2rhel.utils import warn_deprecated_env @@ -78,7 +79,7 @@ def run(self): return if len(packages_to_update) > 0: - package_not_up_to_date_skip = warn_deprecated_env("CONVERT2RHEL_OUTDATED_PACKAGE_CHECK_SKIP") + warn_deprecated_env("CONVERT2RHEL_OUTDATED_PACKAGE_CHECK_SKIP") package_not_up_to_date_error_message = ( "The system has {} package(s) not updated based on repositories defined in the system repositories.\n" "List of packages to update: {}.\n\n" @@ -87,7 +88,7 @@ def run(self): len(packages_to_update), " ".join(packages_to_update) ) ) - if not package_not_up_to_date_skip: + if not tool_opts.outdated_package_check_skip: logger.warning(package_not_up_to_date_error_message) self.set_result( level="OVERRIDABLE", diff --git a/convert2rhel/actions/system_checks/tainted_kmods.py b/convert2rhel/actions/system_checks/tainted_kmods.py index 21147ac5e7..7543fd7dce 100644 --- a/convert2rhel/actions/system_checks/tainted_kmods.py +++ b/convert2rhel/actions/system_checks/tainted_kmods.py @@ -15,9 +15,9 @@ __metaclass__ = type - from convert2rhel import actions from convert2rhel.logger import root_logger +from convert2rhel.toolopts import tool_opts from convert2rhel.utils import run_subprocess, warn_deprecated_env @@ -44,7 +44,7 @@ def run(self): logger.task("Prepare: Check if loaded kernel modules are not tainted") unsigned_modules, _ = run_subprocess(["grep", "(", "/proc/modules"]) module_names = "\n ".join([mod.split(" ")[0] for mod in unsigned_modules.splitlines()]) - tainted_kmods_skip = warn_deprecated_env("CONVERT2RHEL_TAINTED_KERNEL_MODULE_CHECK_SKIP") + warn_deprecated_env("CONVERT2RHEL_TAINTED_KERNEL_MODULE_CHECK_SKIP") diagnosis = ( "Tainted kernel modules detected:\n {0}\n" "Third-party components are not supported per our " @@ -52,7 +52,7 @@ def run(self): ) if unsigned_modules: - if not tainted_kmods_skip: + if not tool_opts.tainted_kernel_module_check_skip: self.set_result( level="OVERRIDABLE", id="TAINTED_KMODS_DETECTED", diff --git a/convert2rhel/toolopts/__init__.py b/convert2rhel/toolopts/__init__.py index deb7e83665..14c6854677 100644 --- a/convert2rhel/toolopts/__init__.py +++ b/convert2rhel/toolopts/__init__.py @@ -26,7 +26,10 @@ class ToolOpts: def _handle_config_conflict(self, config_sources): - file_config = next((config for config in config_sources if config.SOURCE == "configuration file"), None) + file_config = next( + (config for config in config_sources if config.SOURCE == "configuration file"), + None, + ) # No file config detected, let's just bail out. if not file_config: return @@ -117,6 +120,17 @@ def set_opts(self, key, value): if value and not current_attribute_value: setattr(self, key, value) + def update_opts(self, key, value): + """Update a given option in toolopts. + + :param key: + :type key: str + :param value: + :type value: str + """ + if key and value: + setattr(self, key, value) + def _handle_rhsm_parts(self): # Sending itself as the ToolOpts class contains all the attribute references. rhsm_parts = setup_rhsm_parts(self) diff --git a/convert2rhel/unit_tests/actions/post_conversion/hostmetering_test.py b/convert2rhel/unit_tests/actions/post_conversion/hostmetering_test.py index 38cdb088a7..6befe3ff23 100644 --- a/convert2rhel/unit_tests/actions/post_conversion/hostmetering_test.py +++ b/convert2rhel/unit_tests/actions/post_conversion/hostmetering_test.py @@ -20,7 +20,7 @@ import pytest import six -from convert2rhel import actions, systeminfo +from convert2rhel import actions, systeminfo, toolopts from convert2rhel.actions.post_conversion import hostmetering from convert2rhel.systeminfo import Version, system_info from convert2rhel.unit_tests import RunSubprocessMocked, assert_actions_result, run_subprocess_side_effect @@ -41,7 +41,7 @@ def hostmetering_instance(): ( {}, Version(7, 9), - False, # not on hyperscaler + False, # not on hyperscalershould_configure_metering "auto", False, ), @@ -125,11 +125,17 @@ def hostmetering_instance(): ), ) def test_configure_host_metering( - monkeypatch, rhsm_facts, os_version, should_configure_metering, envvar, hostmetering_instance, managed_service + monkeypatch, + rhsm_facts, + os_version, + should_configure_metering, + envvar, + hostmetering_instance, + managed_service, + global_tool_opts, ): - if envvar: - monkeypatch.setenv("CONVERT2RHEL_CONFIGURE_HOST_METERING", envvar) - + monkeypatch.setattr(toolopts, "tool_opts", global_tool_opts) + monkeypatch.setenv("CONVERT2RHEL_CONFIGURE_HOST_METERING", envvar) monkeypatch.setattr(system_info, "version", os_version) monkeypatch.setattr(hostmetering, "get_rhsm_facts", mock.Mock(return_value=rhsm_facts)) yum_mock = mock.Mock(return_value=(0, "")) @@ -152,7 +158,7 @@ def test_configure_host_metering( subprocess_mock.assert_any_call(["systemctl", "enable", "host-metering.service"]) subprocess_mock.assert_any_call(["systemctl", "start", "host-metering.service"]) else: - assert ret is False, "Should not configure host-metering." + assert not ret, "Should not configure host-metering." assert yum_mock.call_count == 0, "Should not install anything." assert subprocess_mock.call_count == 0, "Should not configure anything." @@ -352,11 +358,11 @@ def test_configure_host_metering_messages_and_results( service_running, action_message, action_result, + global_tool_opts, ): """Test outputted report/message in each part of the action.""" if env_var: monkeypatch.setenv("CONVERT2RHEL_CONFIGURE_HOST_METERING", env_var) - monkeypatch.setattr(system_info, "version", os_version) # The facts aren't used during the test run monkeypatch.setattr(hostmetering, "get_rhsm_facts", mock.Mock(return_value=None)) @@ -368,7 +374,7 @@ def test_configure_host_metering_messages_and_results( hostmetering_instance, "_enable_host_metering_service", mock.Mock(return_value=enable_host_metering_service) ) monkeypatch.setattr(systeminfo, "is_systemd_managed_service_running", mock.Mock(return_value=service_running)) - + monkeypatch.setattr(toolopts, "tool_opts", global_tool_opts) hostmetering_instance.run() assert action_message.issuperset(hostmetering_instance.messages) @@ -376,6 +382,26 @@ def test_configure_host_metering_messages_and_results( assert action_result == hostmetering_instance.result +def test_configure_host_metering_no_env_var(monkeypatch, hostmetering_instance, global_tool_opts): + expected = set( + ( + actions.ActionMessage( + level="INFO", + id="CONFIGURE_HOST_METERING_SKIP", + title="Did not perform host metering configuration.", + description="CONVERT2RHEL_CONFIGURE_HOST_METERING was not set.", + ), + ), + ) + monkeypatch.setattr(hostmetering, "tool_opts", global_tool_opts) + + hostmetering_instance.run() + + assert expected.issuperset(hostmetering_instance.messages) + assert expected.issubset(hostmetering_instance.messages) + assert actions.ActionResult(level="SUCCESS", id="SUCCESS") == hostmetering_instance.result + + @pytest.mark.parametrize( ("rhsm_facts", "expected"), ( diff --git a/convert2rhel/unit_tests/actions/pre_ponr_changes/backup_system_test.py b/convert2rhel/unit_tests/actions/pre_ponr_changes/backup_system_test.py index e9c337c66b..adadd7ee6a 100644 --- a/convert2rhel/unit_tests/actions/pre_ponr_changes/backup_system_test.py +++ b/convert2rhel/unit_tests/actions/pre_ponr_changes/backup_system_test.py @@ -22,7 +22,7 @@ import pytest import six -from convert2rhel import unit_tests +from convert2rhel import toolopts, unit_tests from convert2rhel.actions.pre_ponr_changes import backup_system from convert2rhel.backup import files from convert2rhel.backup.files import RestorableFile @@ -220,31 +220,29 @@ def test_get_changed_package_files(self, rpm_va_output, expected, message, caplo else: assert "" == caplog.text - @pytest.mark.parametrize( - ("env_var", "message"), - ( - (True, "Skipping backup of the package files. CONVERT2RHEL_INCOMPLETE_ROLLBACK detected."), - (False, "Missing file {rpm_va_output} in it's location"), - ), - ) - def test_get_changed_package_files_missing(self, caplog, message, env_var, tmpdir, monkeypatch): + def test_get_changed_package_files_missing( + self, caplog, tmpdir, monkeypatch, backup_package_files_action, global_tool_opts + ): + message = "Skipping backup of the package files. CONVERT2RHEL_INCOMPLETE_ROLLBACK detected." monkeypatch.setattr(backup_system, "LOG_DIR", str(tmpdir)) + monkeypatch.setenv("CONVERT2RHEL_INCOMPLETE_ROLLBACK", "1") + monkeypatch.setattr(toolopts, "tool_opts", global_tool_opts) - if env_var: - os.environ["CONVERT2RHEL_INCOMPLETE_ROLLBACK"] = "1" - else: - # Unset the variable - os.environ.pop("CONVERT2RHEL_INCOMPLETE_ROLLBACK", None) + backup_package_files_action._get_changed_package_files() + assert caplog.records[-1].message == message - backup_package_file = backup_system.BackupPackageFiles() + def test_get_changed_package_file_system_exit( + self, monkeypatch, tmpdir, backup_package_files_action, global_tool_opts + ): + tmp_path = str(tmpdir) + monkeypatch.setattr(backup_system, "tool_opts", global_tool_opts) + monkeypatch.setattr(backup_system, "LOG_DIR", str(tmp_path)) - try: - backup_package_file._get_changed_package_files() - except SystemExit: - path = os.path.join(str(tmpdir), PRE_RPM_VA_LOG_FILENAME) - assert caplog.records[-1].message == message.format(rpm_va_output=path) - else: - assert caplog.records[-1].message == message + rpm_va_path = os.path.join(tmp_path, PRE_RPM_VA_LOG_FILENAME) + message = "Missing file {} in it's location".format(rpm_va_path) + + with pytest.raises(SystemExit, match=message): + backup_package_files_action._get_changed_package_files() @pytest.mark.parametrize( ("rpm_va_output", "message"), diff --git a/convert2rhel/unit_tests/actions/pre_ponr_changes/kernel_modules_test.py b/convert2rhel/unit_tests/actions/pre_ponr_changes/kernel_modules_test.py index 9298d1bac8..fe5a7289a1 100644 --- a/convert2rhel/unit_tests/actions/pre_ponr_changes/kernel_modules_test.py +++ b/convert2rhel/unit_tests/actions/pre_ponr_changes/kernel_modules_test.py @@ -104,6 +104,7 @@ def test_ensure_compatibility_of_kmods( caplog, host_kmods, should_be_in_logs, + global_tool_opts, ): monkeypatch.setattr( ensure_kernel_modules_compatibility_instance, "_get_loaded_kmods", mock.Mock(return_value=host_kmods) @@ -121,6 +122,7 @@ def test_ensure_compatibility_of_kmods( "run_subprocess", value=run_subprocess_mock, ) + monkeypatch.setattr(kernel_modules, "tool_opts", global_tool_opts) ensure_kernel_modules_compatibility_instance.run() assert_actions_result(ensure_kernel_modules_compatibility_instance, level="SUCCESS") @@ -172,12 +174,12 @@ def test_ensure_compatibility_of_kmods_failure( ensure_kernel_modules_compatibility_instance, monkeypatch, pretend_os, - caplog, host_kmods, repo_kmod_pkgs, makecache_output, error_id, level, + global_tool_opts, ): monkeypatch.setattr( ensure_kernel_modules_compatibility_instance, "_get_loaded_kmods", mock.Mock(return_value=host_kmods) @@ -195,6 +197,7 @@ def test_ensure_compatibility_of_kmods_failure( "run_subprocess", value=run_subprocess_mock, ) + monkeypatch.setattr(kernel_modules, "tool_opts", global_tool_opts) ensure_kernel_modules_compatibility_instance.run() assert_actions_result(ensure_kernel_modules_compatibility_instance, level=level, id=error_id) @@ -273,6 +276,7 @@ def test_ensure_compatibility_of_kmods_excluded( msg_in_logs, msg_not_in_logs, exception, + global_tool_opts, ): monkeypatch.setattr( ensure_kernel_modules_compatibility_instance, @@ -300,6 +304,7 @@ def test_ensure_compatibility_of_kmods_excluded( "_get_unsupported_kmods", value=get_unsupported_kmods_mocked, ) + monkeypatch.setattr(kernel_modules, "tool_opts", global_tool_opts) if exception: ensure_kernel_modules_compatibility_instance.run() diff --git a/convert2rhel/unit_tests/actions/system_checks/convert2rhel_latest_test.py b/convert2rhel/unit_tests/actions/system_checks/convert2rhel_latest_test.py index 149f9496dd..8686df3524 100644 --- a/convert2rhel/unit_tests/actions/system_checks/convert2rhel_latest_test.py +++ b/convert2rhel/unit_tests/actions/system_checks/convert2rhel_latest_test.py @@ -144,9 +144,10 @@ class TestCheckConvert2rhelLatest: indirect=True, ) def test_convert2rhel_latest_outdated_version_inhibitor( - self, convert2rhel_latest_action_instance, prepare_convert2rhel_latest_action + self, convert2rhel_latest_action_instance, prepare_convert2rhel_latest_action, monkeypatch, global_tool_opts ): """When runnnig on a supported major version, we issue an error = inhibitor.""" + monkeypatch.setattr(convert2rhel_latest, "tool_opts", global_tool_opts) convert2rhel_latest_action_instance.run() running_version, latest_version = prepare_convert2rhel_latest_action @@ -231,9 +232,10 @@ def test_convert2rhel_latest_outdated_version_inhibitor( indirect=True, ) def test_convert2rhel_latest_outdated_version_warning( - self, convert2rhel_latest_action_instance, prepare_convert2rhel_latest_action + self, convert2rhel_latest_action_instance, prepare_convert2rhel_latest_action, monkeypatch, global_tool_opts ): """When runnnig on an older unsupported major version, we issue just a warning instead of an inhibitor.""" + monkeypatch.setattr(convert2rhel_latest, "tool_opts", global_tool_opts) convert2rhel_latest_action_instance.run() running_version, latest_version = prepare_convert2rhel_latest_action @@ -738,10 +740,12 @@ def mock_run_subprocess(cmd, print_output=False): ), indirect=True, ) - def test_convert2rhel_latest_bad_NEVRA_to_parse_pkg_string( + def test_convert2rhel_latest_bad_nevra_to_parse_pkg_string( self, + monkeypatch, convert2rhel_latest_action_instance, prepare_convert2rhel_latest_action, + global_tool_opts, ): generator = extract_convert2rhel_versions_generator() @@ -750,6 +754,7 @@ def test_convert2rhel_latest_bad_NEVRA_to_parse_pkg_string( "convert2rhel_latest._extract_convert2rhel_versions", side_effect=generator, ) + monkeypatch.setattr(convert2rhel_latest, "tool_opts", global_tool_opts) convert2rhel_latest_action_instance.run() running_version, latest_version = prepare_convert2rhel_latest_action diff --git a/convert2rhel/unit_tests/actions/system_checks/is_loaded_kernel_latest_test.py b/convert2rhel/unit_tests/actions/system_checks/is_loaded_kernel_latest_test.py index d5221b35a7..10da0b684a 100644 --- a/convert2rhel/unit_tests/actions/system_checks/is_loaded_kernel_latest_test.py +++ b/convert2rhel/unit_tests/actions/system_checks/is_loaded_kernel_latest_test.py @@ -83,6 +83,7 @@ def test_is_loaded_kernel_latest_eus_system_invalid_kernel_version( package_name, monkeypatch, is_loaded_kernel_latest_action, + global_tool_opts, ): run_subprocess_mocked = mock.Mock( spec=run_subprocess, @@ -110,7 +111,7 @@ def test_is_loaded_kernel_latest_eus_system_invalid_kernel_version( "run_subprocess", value=run_subprocess_mocked, ) - + monkeypatch.setattr(is_loaded_kernel_latest, "tool_opts", global_tool_opts) is_loaded_kernel_latest_action.run() unit_tests.assert_actions_result( @@ -172,6 +173,7 @@ def test_is_loaded_kernel_latest_invalid_kernel_package_dnf( remediations, monkeypatch, is_loaded_kernel_latest_action, + global_tool_opts, ): run_subprocess_mocked = mock.Mock( spec=run_subprocess, @@ -200,6 +202,7 @@ def test_is_loaded_kernel_latest_invalid_kernel_package_dnf( value=run_subprocess_mocked, ) + monkeypatch.setattr(is_loaded_kernel_latest, "tool_opts", global_tool_opts) is_loaded_kernel_latest_action.run() expected = set( ( @@ -266,6 +269,7 @@ def test_is_loaded_kernel_latest_invalid_kernel_package_yum( remediations, monkeypatch, is_loaded_kernel_latest_action, + global_tool_opts, ): run_subprocess_mocked = mock.Mock( spec=run_subprocess, @@ -293,7 +297,7 @@ def test_is_loaded_kernel_latest_invalid_kernel_package_yum( "run_subprocess", value=run_subprocess_mocked, ) - + monkeypatch.setattr(is_loaded_kernel_latest, "tool_opts", global_tool_opts) is_loaded_kernel_latest_action.run() expected = set( @@ -313,7 +317,9 @@ def test_is_loaded_kernel_latest_invalid_kernel_package_yum( assert expected.issubset(is_loaded_kernel_latest_action.messages) @centos8 - def test_is_loaded_kernel_latest_eus_system(self, pretend_os, monkeypatch, caplog, is_loaded_kernel_latest_action): + def test_is_loaded_kernel_latest_eus_system( + self, pretend_os, monkeypatch, caplog, is_loaded_kernel_latest_action, global_tool_opts + ): run_subprocess_mocked = mock.Mock( spec=run_subprocess, side_effect=run_subprocess_side_effect( @@ -341,6 +347,7 @@ def test_is_loaded_kernel_latest_eus_system(self, pretend_os, monkeypatch, caplo value=run_subprocess_mocked, ) + monkeypatch.setattr(is_loaded_kernel_latest, "tool_opts", global_tool_opts) is_loaded_kernel_latest_action.run() assert "The currently loaded kernel is at the latest version." in caplog.records[-1].message @@ -481,6 +488,7 @@ def test_is_loaded_kernel_latest_unable_to_fetch_kernels( monkeypatch, caplog, is_loaded_kernel_latest_action, + global_tool_opts, ): run_subprocess_mocked = mock.Mock( spec=run_subprocess, @@ -507,6 +515,7 @@ def test_is_loaded_kernel_latest_unable_to_fetch_kernels( "run_subprocess", value=run_subprocess_mocked, ) + monkeypatch.setattr(is_loaded_kernel_latest, "tool_opts", global_tool_opts) expected_set = set( ( @@ -565,6 +574,7 @@ def test_is_loaded_kernel_latest_unsupported_skip_error( diagnosis, monkeypatch, is_loaded_kernel_latest_action, + global_tool_opts, ): run_subprocess_mocked = mock.Mock( spec=run_subprocess, @@ -591,6 +601,7 @@ def test_is_loaded_kernel_latest_unsupported_skip_error( "run_subprocess", value=run_subprocess_mocked, ) + monkeypatch.setattr(is_loaded_kernel_latest, "tool_opts", global_tool_opts) is_loaded_kernel_latest_action.run() diagnosis = diagnosis.format(package_name) unit_tests.assert_actions_result( @@ -671,6 +682,7 @@ def test_is_loaded_kernel_latest( monkeypatch, caplog, is_loaded_kernel_latest_action, + global_tool_opts, ): # Using the minor version as 99, so the tests should never fail because of a # constraint in the code, since we don't mind the minor version number (for @@ -705,6 +717,7 @@ def test_is_loaded_kernel_latest( (("uname", "-r"), (uname_version, return_code)), ), ) + monkeypatch.setattr(is_loaded_kernel_latest, "tool_opts", global_tool_opts) monkeypatch.setattr( is_loaded_kernel_latest, "run_subprocess", @@ -714,7 +727,7 @@ def test_is_loaded_kernel_latest( is_loaded_kernel_latest_action.run() assert expected_message in caplog.records[-1].message - def test_is_loaded_kernel_latest_system_exit(self, monkeypatch, is_loaded_kernel_latest_action, tmpdir): + def test_is_loaded_kernel_latest_system_exit(self, monkeypatch, is_loaded_kernel_latest_action, global_tool_opts): repoquery_version = "C2R\t1634146676\t3.10.0-1160.45.1.el7\tbaseos" uname_version = "3.10.0-1160.42.2.el7.x86_64" @@ -756,6 +769,7 @@ def test_is_loaded_kernel_latest_system_exit(self, monkeypatch, is_loaded_kernel "run_subprocess", value=run_subprocess_mocked, ) + monkeypatch.setattr(is_loaded_kernel_latest, "tool_opts", global_tool_opts) is_loaded_kernel_latest_action.run() unit_tests.assert_actions_result( @@ -799,7 +813,13 @@ def test_is_loaded_kernel_latest_system_exit(self, monkeypatch, is_loaded_kernel ), ) def test_is_loaded_kernel_latest_disable_repos( - self, monkeypatch, enablerepos, expected_cmd, is_loaded_kernel_latest_action, pretend_os + self, + monkeypatch, + enablerepos, + expected_cmd, + is_loaded_kernel_latest_action, + pretend_os, + global_tool_opts, ): """Test if the --disablerepo part of the command is built propertly.""" run_subprocess = mock.Mock(return_value=[None, None]) @@ -808,8 +828,9 @@ def test_is_loaded_kernel_latest_disable_repos( "run_subprocess", run_subprocess, ) - - monkeypatch.setattr(repo.tool_opts, "enablerepo", enablerepos) + global_tool_opts.enablerepos = enablerepos + monkeypatch.setattr(is_loaded_kernel_latest, "tool_opts", global_tool_opts) + monkeypatch.setattr(repo, "tool_opts", global_tool_opts) is_loaded_kernel_latest_action.run() diff --git a/convert2rhel/unit_tests/actions/system_checks/package_updates_test.py b/convert2rhel/unit_tests/actions/system_checks/package_updates_test.py index bcfbc6a986..ac9aed1e4e 100644 --- a/convert2rhel/unit_tests/actions/system_checks/package_updates_test.py +++ b/convert2rhel/unit_tests/actions/system_checks/package_updates_test.py @@ -64,7 +64,8 @@ def test_check_package_updates_skip_on_not_latest_ol(pretend_os, caplog, package @centos8 -def test_check_package_updates(pretend_os, monkeypatch, caplog, package_updates_action): +def test_check_package_updates(pretend_os, monkeypatch, caplog, package_updates_action, global_tool_opts): + monkeypatch.setattr(package_updates, "tool_opts", global_tool_opts) monkeypatch.setattr(package_updates, "get_total_packages_to_update", value=lambda: []) package_updates_action.run() @@ -72,7 +73,10 @@ def test_check_package_updates(pretend_os, monkeypatch, caplog, package_updates_ @centos8 -def test_check_package_updates_not_up_to_date(pretend_os, monkeypatch, package_updates_action, caplog): +def test_check_package_updates_not_up_to_date( + pretend_os, monkeypatch, package_updates_action, caplog, global_tool_opts +): + monkeypatch.setattr(package_updates, "tool_opts", global_tool_opts) packages = ["package-2", "package-1"] diagnosis = ( "The system has 2 package(s) not updated based on repositories defined in the system repositories.\n" diff --git a/convert2rhel/unit_tests/actions/system_checks/tainted_kmods_test.py b/convert2rhel/unit_tests/actions/system_checks/tainted_kmods_test.py index 1ae9cb30db..adacfddc6e 100644 --- a/convert2rhel/unit_tests/actions/system_checks/tainted_kmods_test.py +++ b/convert2rhel/unit_tests/actions/system_checks/tainted_kmods_test.py @@ -54,13 +54,15 @@ def tainted_kmods_action(): ), ), ) -def test_check_tainted_kmods(monkeypatch, command_return, is_error, tainted_kmods_action): +def test_check_tainted_kmods(monkeypatch, command_return, is_error, tainted_kmods_action, global_tool_opts): run_subprocess_mock = mock.Mock(return_value=command_return) monkeypatch.setattr( tainted_kmods, "run_subprocess", value=run_subprocess_mock, ) + monkeypatch.setattr(tainted_kmods, "tool_opts", global_tool_opts) + tainted_kmods_action.run() if is_error: @@ -151,7 +153,5 @@ def test_check_tainted_kmods_skip(monkeypatch, command_return, is_error, tainted ), ) ) - print(expected) - print(tainted_kmods_action.messages) assert expected.issuperset(tainted_kmods_action.messages) assert expected.issubset(tainted_kmods_action.messages) diff --git a/convert2rhel/unit_tests/conftest.py b/convert2rhel/unit_tests/conftest.py index f1315e0090..daf0cc9161 100644 --- a/convert2rhel/unit_tests/conftest.py +++ b/convert2rhel/unit_tests/conftest.py @@ -138,10 +138,31 @@ def run(self): pass +class FileConfigMock: + SOURCE = "configuration file" + + def __init__(self): + self.configure_host_metering = None + self.incomplete_rollback = None + self.tainted_kernel_module_check_skip = None + self.outdated_package_check_skip = None + self.allow_older_version = None + self.allow_unavailable_kmods = None + self.skip_kernel_currency_check = None + + self.username = None + self.password = None + self.org = None + self.activation_key = None + + def run(self): + pass + + @pytest.fixture def global_tool_opts(monkeypatch): local_tool_opts = toolopts.ToolOpts() - local_tool_opts.initialize(config_sources=[CliConfigMock()]) + local_tool_opts.initialize(config_sources=[CliConfigMock(), FileConfigMock()]) monkeypatch.setattr(toolopts, "tool_opts", local_tool_opts) return local_tool_opts diff --git a/convert2rhel/unit_tests/utils/utils_test.py b/convert2rhel/unit_tests/utils/utils_test.py index 471ba7f415..cff9c219dd 100644 --- a/convert2rhel/unit_tests/utils/utils_test.py +++ b/convert2rhel/unit_tests/utils/utils_test.py @@ -40,7 +40,7 @@ from convert2rhel import exceptions, systeminfo, toolopts, unit_tests, utils # Imports unit_tests/__init__.py from convert2rhel.systeminfo import system_info -from convert2rhel.unit_tests import RunCmdInPtyMocked, RunSubprocessMocked, is_rpm_based_os +from convert2rhel.unit_tests import RunCmdInPtyMocked, RunSubprocessMocked, conftest, is_rpm_based_os DOWNLOADED_RPM_NVRA = "kernel-4.18.0-193.28.1.el8_2.x86_64" @@ -1134,3 +1134,40 @@ def test_remove_pkgs_with_empty_list(self, caplog): ) def test_remove_epoch_from_yum_nevra_notation(pkg_nevra, nvra_without_epoch): assert utils._remove_epoch_from_yum_nevra_notation(pkg_nevra) == nvra_without_epoch + + +@pytest.mark.parametrize( + ("env_name", "env_value", "tool_opts_name", "message"), + ( + ( + "CONVERT2RHEL_SKIP_KERNEL_CURRENCY_CHECK", + True, + "skip_kernel_currency_check", + "The environment variable CONVERT2RHEL_SKIP_KERNEL_CURRENCY_CHECK is deprecated and is set to be removed on Convert2RHEL 2.4.0.\n" + "Please, use the configuration file instead.", + ), + ), +) +def test_warn_deprecated_env(global_tool_opts, monkeypatch, env_name, env_value, tool_opts_name, message, caplog): + """Test setting the value based on env variable and it's logging.""" + monkeypatch.setattr(utils, "tool_opts", global_tool_opts) + monkeypatch.setenv(env_name, env_value) + + utils.warn_deprecated_env(env_name) + assert getattr(global_tool_opts, tool_opts_name) == str(env_value) + assert caplog.records[-1].message == message + + +def test_warn_deprecated_env_wrong_name(global_tool_opts, monkeypatch, caplog): + """Test when unsupported env variable is used, nothing is set.""" + monkeypatch.setattr(utils, "tool_opts", global_tool_opts) + + utils.warn_deprecated_env("UNSUPPORTED_ENV_VAR") + + # Get tool_opts with default values + default_tool_opts = toolopts.ToolOpts() + default_tool_opts.initialize(config_sources=[conftest.CliConfigMock(), conftest.FileConfigMock()]) + + for item, value in global_tool_opts.__dict__.items(): + assert default_tool_opts.__dict__[item] == value + assert not caplog.text diff --git a/convert2rhel/utils/__init__.py b/convert2rhel/utils/__init__.py index 9975d8dd2f..b0542bfb4a 100644 --- a/convert2rhel/utils/__init__.py +++ b/convert2rhel/utils/__init__.py @@ -1129,12 +1129,24 @@ def warn_deprecated_env(env_name): :param env_name: The name of the environment variable that is deprecated. :type env_name: str """ + env_var_to_toolopts_map = { + "CONVERT2RHEL_OUTDATED_PACKAGE_CHECK_SKIP": "outdated_package_check_skip", + "CONVERT2RHEL_SKIP_KERNEL_CURRENCY_CHECK": "skip_kernel_currency_check", + "CONVERT2RHEL_INCOMPLETE_ROLLBACK": "incomplete_rollback", + "CONVERT2RHEL_ALLOW_UNAVAILABLE_KMODS": "allow_unavailable_kmods", + "CONVERT2RHEL_ALLOW_OLDER_VERSION": "allow_older_version", + "CONVERT2RHEL_CONFIGURE_HOST_METERING": "configure_host_metering", + "CONVERT2RHEL_TAINTED_KERNEL_MODULE_CHECK_SKIP": "tainted_kernel_module_check_skip", + } + if env_name not in os.environ: # Nothing to do here. return None root_logger.warning( - "The environment variable {} is deprecated in favor of using a configuration file and will be removed in version Convert2RHEL 2.4.0." + "The environment variable {} is deprecated and is set to be removed on Convert2RHEL 2.4.0.\n" + "Please, use the configuration file instead.".format(env_name) ) - - return os.getenv(env_name, None) + key = env_var_to_toolopts_map[env_name] + value = os.getenv(env_name, None) + tool_opts.update_opts(key, value)