[RHELC-1226] Update CONVERT2RHEL_CONFIGURE_HOST_METERING env var beha…

…vior

convert2rhel/hostmetering.py

@@ -25,6 +25,7 @@
 import logging
 import os
 
+from convert2rhel import systeminfo
 from convert2rhel.pkghandler import call_yum_cmd
 from convert2rhel.subscription import get_rhsm_facts
 from convert2rhel.systeminfo import system_info
@@ -34,16 +35,15 @@
 logger = logging.getLogger(__name__)
 
 
-def is_running_on_hyperscaller(rhsm_facts):
+def is_running_on_hyperscaler(rhsm_facts):
     """
     Check if the system is running on hyperscaller. Currently supported
     hyperscallers are aws, azure and gcp.
 
-    Args:
-        rhsm_facts (dict): Facts about the system from RHSM.
-
-    Returns:
-        bool: True if the system is running on hyperscaller, False otherwise.
+    :param rhsm_facts: Facts about the system from RHSM.
+    :type rhsm_facts: dict
+    :return: True if the system is running on hyperscaller, False otherwise.
+    :rtype: bool
     """
     is_aws = rhsm_facts.get("aws_instance_id")
     is_azure = rhsm_facts.get("azure_instance_id")
@@ -53,58 +53,81 @@ def is_running_on_hyperscaller(rhsm_facts):
 
 def configure_host_metering():
     """
-    Install, enable and start host-metering on the system when it is running
-    on a hyperscaller and is RHEL 7.
+    Decide whether to install, enable and start host-metering on the system based on the
+    CONVERT2RHEL_CONFIGURE_HOST_METERING environment variable.
 
     Behavior can be controlled CONVERT2RHEL_CONFIGURE_HOST_METERING environment variable:
-    - unset: host-metering will be configured based on the above conditions
-    - "no": host-metering will not be configured
+    - "auto": host-metering will be configured based on the above conditions
+    - empty: host-metering will not be configured
     - "force": forces configuration of host-metering (e.g., even if not running on a hyperscaller)
-    - any other value: behaves as unset
+    - any other value: behaves as empty
 
-    Returns:
-        bool: True if host-metering is configured successfully, False otherwise.
+    :return: True if host-metering is configured successfully, False otherwise.
+    :rtype: bool
     """
+    env_var = os.environ.get("CONVERT2RHEL_CONFIGURE_HOST_METERING", "empty")
     if "CONVERT2RHEL_CONFIGURE_HOST_METERING" not in os.environ:
-        # TODO(r0x0d): Do we want to silently return here?
-        logger.info("")
-        return
+        logger.debug("")
+        return False
 
-    if system_info.version.major > 7:
+    if system_info.version.major > 7 and env_var != "force":
         logger.info("Skipping host metering configuration. Only supported for RHEL 7.")
-        return
+        return False
+
+    if env_var == "force":
+        should_configure_metering = True
+        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. "
+            " For generic usage please use the 'auto' option."
+        )
+    elif env_var == "auto":
+        should_configure_metering = True
+    else:
+        should_configure_metering = False
 
     rhsm_facts = get_rhsm_facts()
-    conditions_met = is_running_on_hyperscaller(rhsm_facts)
 
-    if not conditions_met:
+    if (not is_running_on_hyperscaler(rhsm_facts) or not should_configure_metering) and env_var != "force":
         logger.info("Skipping host-metering configuration.")
         return False
 
-    logger.info("Installing host-metering rpms.")
+    logger.info("Installing host-metering packages.")
     output, ret_install = call_yum_cmd("install", ["host-metering"])
     logger.debug("Output of yum call: %s" % output)
     if ret_install:
         logger.warning("Failed to install host-metering rpms.")
         return False
 
-    _enable_host_metering_service()
+    if not _enable_host_metering_service():
+        return False
 
-    return system_info.is_systemd_managed_service_running("host-metering.service")
+    return systeminfo.is_systemd_managed_service_running("host-metering.service")
 
 
 def _enable_host_metering_service():
+    """
+    Enables and starts the host metering service.
+
+    :return: True if host-metering is enabled and started successfully, False otherwise.
+    :rtype: bool
+    """
+
     logger.info("Enabling host-metering service.")
     output, ret_enable = run_subprocess(["systemctl", "enable", "host-metering.service"])
     logger.debug("Output of systemctl call: %s" % output)
     if ret_enable:
         logger.warning("Failed to enable host-metering service.")
+        return False
 
     logger.info("Starting host-metering service.")
     output, ret_start = run_subprocess(["systemctl", "start", "host-metering.service"])
     logger.debug("Output of systemctl call: %s" % output)
     if ret_start:
         logger.warning("Failed to start host-metering service.")
+        return False
 
-    if not system_info.is_systemd_managed_service_running("host-metering.service"):
+    if not systeminfo.is_systemd_managed_service_running("host-metering.service"):
         logger.critical_no_exit("host-metering unit is not active.")
+        return False
+    return True

convert2rhel/subscription.py

@@ -68,6 +68,16 @@ class StopRhsmError(Exception):
     """Raised with problems stopping the rhsm daemon."""
 
 
+class FileNotFoundError(OSError):
+    """Raised when the requested file cannot be found."""
+
+    pass
+
+
+class JSONDecodeError(ValueError):
+    pass
+
+
 class RestorableSystemSubscription(backup.RestorableChange):
     """
     Register with RHSM in a fashion that can be reverted.
@@ -912,7 +922,7 @@ def get_rhsm_facts():
     :returns dict: The RHSM facts.
     """
     rhsm_facts = {}
-    if not tool_opts.no_rhsm:
+    if tool_opts.no_rhsm:
         loggerinst.info("Ignoring RHSM facts collection. --no-rhsm is used.")
         return rhsm_facts
 

convert2rhel/unit_tests/hostmetering_test.py

@@ -20,9 +20,9 @@
 import pytest
 import six
 
-from convert2rhel import hostmetering, utils
+from convert2rhel import hostmetering, systeminfo
 from convert2rhel.systeminfo import Version, system_info
-from convert2rhel.unit_tests import RunSubprocessMocked
+from convert2rhel.unit_tests import RunSubprocessMocked, run_subprocess_side_effect
 
 
 six.add_move(six.MovedModule("mock", "mock", "unittest.mock"))
@@ -36,45 +36,44 @@
             {},
             Version(7, 9),
             False,  # not on hyperscaller
-            None,
+            "auto",
         ),
         (
             {"aws_instance_id": "i-1234567890abcdef0"},
             Version(7, 9),
             True,
-            None,
+            "auto",
         ),
         (
             {"azure_instance_id": "012345678-abcde-efgh-1234-abcdefgh1234"},
             Version(7, 9),
             True,
-            None,
+            "auto",
         ),
         (
             {"gcp_instance_id": "12345-6789-abcd-efgh-0123456789ab"},
             Version(7, 9),
             True,
-            None,
+            "auto",
         ),
         (
             {"aws_instance_id": "i-1234567890abcdef0"},
             Version(8, 8),
             False,  # not on RHEL 7
-            None,
+            "auto",
         ),
         (
             {"azure_instance_id": "012345678-abcde-efgh-1234-abcdefgh1234"},
             Version(8, 8),
             False,  # not on RHEL 7
-            None,
+            "auto",
         ),
         (
             {"gcp_instance_id": "12345-6789-abcd-efgh-0123456789ab"},
             Version(8, 8),
             False,  # not on RHEL 7
-            None,
+            "auto",
         ),
-        # env var set behavior
         (
             {},
             Version(7, 9),
@@ -91,33 +90,32 @@
             {"aws_instance_id": "i-1234567890abcdef0"},
             Version(7, 9),
             False,
-            "no",  # disabled
+            "arbitrary",  # unknown option
         ),
         (
             {"aws_instance_id": "i-1234567890abcdef0"},
             Version(7, 9),
-            True,
-            "arbitrary",  # condition met
-        ),
-        (
-            {"aws_instance_id": "i-1234567890abcdef0"},
-            Version(7, 9),
-            True,
-            "",  # condition met
+            False,
+            "",  # option left empty
         ),
     ),
 )
-def test_hostmetering(monkeypatch, rhsm_facts, os_version, should_configure_metering, envvar):
-    if envvar is not None:
+def test_configure_host_metering(monkeypatch, rhsm_facts, os_version, should_configure_metering, envvar):
+    if envvar:
         monkeypatch.setenv("CONVERT2RHEL_CONFIGURE_HOST_METERING", envvar)
 
     monkeypatch.setattr(system_info, "version", os_version)
-    monkeypatch.setattr(system_info, "releasever", "")  # reset as other test set it
+    # monkeypatch.setattr(system_info, "releasever", "")  # reset as other test set it
     monkeypatch.setattr(hostmetering, "get_rhsm_facts", mock.Mock(return_value=rhsm_facts))
     yum_mock = mock.Mock(return_value=(0, ""))
     monkeypatch.setattr(hostmetering, "call_yum_cmd", yum_mock)
     subprocess_mock = RunSubprocessMocked(return_string="mock")
     monkeypatch.setattr(hostmetering, "run_subprocess", subprocess_mock)
+    monkeypatch.setattr(
+        hostmetering.systeminfo,
+        "is_systemd_managed_service_running",
+        lambda name: True,
+    )
 
     ret = hostmetering.configure_host_metering()
 
@@ -128,5 +126,56 @@ def test_hostmetering(monkeypatch, rhsm_facts, os_version, should_configure_mete
         subprocess_mock.assert_any_call(["systemctl", "start", "host-metering.service"])
     else:
         assert ret is False, "Should not configure host-metering."
-        assert yum_mock.call_count == 0, "Should not install anythibg."
+        assert yum_mock.call_count == 0, "Should not install anything."
         assert subprocess_mock.call_count == 0, "Should not configure anything."
+
+
+@pytest.mark.parametrize(
+    ("rhsm_facts", "expected"),
+    (
+        ({"aws_instance_id": "23143", "azure_instance_id": "12134", "gcp_instance_id": "34213"}, True),
+        ({"aws_instance_id": "23143"}, True),
+        ({"azure_instance_id": "12134"}, True),
+        ({"gcp_instance_id": "34213"}, True),
+        ({"invalid_instance_id": "00001"}, False),
+    ),
+)
+def test_is_running_on_hyperscaler(rhsm_facts, expected):
+    running_on_hyperscaler = hostmetering.is_running_on_hyperscaler(rhsm_facts)
+    assert running_on_hyperscaler == expected
+
+
+@pytest.mark.parametrize(
+    ("enable_output", "enable_ret_code", "start_output", "start_ret_code", "managed_service", "expected"),
+    (
+        ("", 0, "", 0, True, True),
+        ("", 0, "", 0, False, False),
+        ("", 1, "", 0, True, False),
+        ("", 0, "", 1, True, False),
+        ("", 1, "", 1, True, False),
+    ),
+)
+def test_enable_host_metering_service(
+    enable_output, enable_ret_code, start_output, start_ret_code, managed_service, expected, monkeypatch
+):
+    systemctl_enable = ("systemctl", "enable", "host-metering.service")
+    systemctl_start = ("systemctl", "start", "host-metering.service")
+
+    # Mock rpm command
+    run_subprocess_mock = RunSubprocessMocked(
+        side_effect=run_subprocess_side_effect(
+            (
+                systemctl_enable,
+                (
+                    enable_output,
+                    enable_ret_code,
+                ),
+            ),
+            (systemctl_start, (start_output, start_ret_code)),
+        ),
+    )
+    monkeypatch.setattr(hostmetering, "run_subprocess", value=run_subprocess_mock)
+    monkeypatch.setattr(systeminfo, "is_systemd_managed_service_running", mock.Mock(return_value=managed_service))
+
+    enable = hostmetering._enable_host_metering_service()
+    assert enable == expected

convert2rhel/unit_tests/subscription_test.py

@@ -18,6 +18,7 @@
 __metaclass__ = type
 
 import errno
+import json
 
 from collections import namedtuple
 
@@ -1268,22 +1269,22 @@ def test_update_rhsm_custom_facts_disable_telemetry(monkeypatch, caplog):
     assert message in caplog.records[-1].message
 
 
-def test_get_rhsm_facts(monkeypatch):
-    facts_string = """\
-invalid value
-cpu.cpu(s): 8
-cpu.cpu_socket(s): 3
-"""
-    monkeypatch.setattr(
-        utils,
-        "run_subprocess",
-        RunSubprocessMocked(return_string=facts_string),
-    )
+def test_get_rhsm_facts(monkeypatch, global_tool_opts, tmp_path):
+    facts_string = {
+        u"cpu.cpu(s)": "8",
+        u"cpu.cpu_socket(s)": "3",
+    }
+    facts_json = json.dumps(facts_string, encoding="utf-8", ensure_ascii=False)
+    path = tmp_path / "facts.json"
+    path.write_text(facts_json)
+    monkeypatch.setattr(subscription, "tool_opts", global_tool_opts)
+    monkeypatch.setattr(subscription, "RHSM_FACTS_FILE", path)
+    global_tool_opts.no_rhsm = False
     facts = subscription.get_rhsm_facts()
     assert facts == {"cpu.cpu(s)": "8", "cpu.cpu_socket(s)": "3"}
 
 
-def test_get_rhsm_facts_no_rhsm(global_tool_opts, monkeypatch):
+def test_get_rhsm_facts_no_rhsm(monkeypatch, global_tool_opts):
     run_mock = RunSubprocessMocked(return_string="")
     monkeypatch.setattr(subscription, "tool_opts", global_tool_opts)
     global_tool_opts.no_rhsm = True