[RHELC-1740] Use toolopts instead of env var (oamg#1387)

* 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 <[email protected]>

---------

Co-authored-by: Adam Hošek <[email protected]>

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

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

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"

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(

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"

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",

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,15 +44,15 @@ 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 "
             "software support policy:\n{1}\n".format(module_names, LINK_KMODS_RH_POLICY)
         )
 
         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",

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)

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,14 +374,34 @@ 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)
     assert action_message.issubset(hostmetering_instance.messages)
     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"),
     (