Don't update RHSM facts without sub-man installed (#1435)

When an exception was raised early before we had a change to install sub-mand, we've
seen a traceback due to trying to call sub-man which hasn't been installed.

convert2rhel/main.py

@@ -181,8 +181,8 @@ def main_locked():
         utils.restart_system()
     except _AnalyzeExit:
         breadcrumbs.breadcrumbs.finish_collection(success=True)
-        # Update RHSM custom facts only when this returns False. Otherwise,
-        # sub-man get uninstalled and the data is removed from the RHSM server.
+        # Update RHSM custom facts only when the system does not need to be subscribed/registered. Otherwise,
+        # we unregister the system as part of the rollback and the data is removed from the RHSM server.
         if not subscription.should_subscribe():
             subscription.update_rhsm_custom_facts()
 
@@ -286,8 +286,8 @@ def _handle_main_exceptions(current_phase, results=None):  # type: (ConversionPh
         loggerinst.info(no_changes_msg)
         return ConversionExitCodes.FAILURE
     elif execution_phase == ConversionPhases.PRE_PONR_CHANGES:
-        # Update RHSM custom facts only when this returns False. Otherwise,
-        # sub-man get uninstalled and the data is removed from the RHSM server.
+        # Update RHSM custom facts only when the system does not need to be subscribed/registered. Otherwise,
+        # we unregister the system as part of the rollback and the data is removed from the RHSM server.
         if not subscription.should_subscribe():
             subscription.update_rhsm_custom_facts()
 

convert2rhel/pkghandler.py

@@ -130,7 +130,7 @@ def get_installed_pkg_information(pkg_name="*"):
 
     :param pkg_name: Full name of a package to check their signature.  If not given, information about all installed packages is returned.
     :type pkg_obj: str
-    :return: Return the package signature.
+    :return: Return a list of PackageInformation objects holding information about matching packages.
     :rtype: list[PackageInformation]
     """
     cmd = [

convert2rhel/subscription.py

@@ -894,22 +894,29 @@ def update_rhsm_custom_facts():
     the conversion with the candlepin server, thus, propagating the
     "breadcrumbs" from convert2rhel as RHSM facts.
     """
-    if not tool_opts.no_rhsm:
-        logger.info("Updating RHSM custom facts collected during the conversion.")
-        cmd = ["subscription-manager", "facts", "--update"]
-        output, ret_code = utils.run_subprocess(cmd, print_output=False)
+    if tool_opts.no_rhsm:
+        logger.info("Option to not use RHSM detected. Skipping updating RHSM custom facts.")
+        return None, None
 
-        if ret_code != 0:
-            logger.warning(
-                "Failed to update the RHSM custom facts with return code '%s' and output '%s'.",
-                ret_code,
-                output,
-            )
-            return ret_code, output
-        else:
-            logger.info("RHSM custom facts uploaded successfully.")
-    else:
-        logger.info("Skipping updating RHSM custom facts.")
+    # It may happen that this facts updating function is called before we install the sub-man packages (e.g. when an
+    # exception is raised early)
+    if not pkghandler.get_installed_pkg_information("subscription-manager"):
+        logger.info("The subscription-manager package is not installed. Skipping updating RHSM custom facts.")
+        return None, None
+
+    logger.info("Updating RHSM custom facts collected during the conversion.")
+    cmd = ["subscription-manager", "facts", "--update"]
+    output, ret_code = utils.run_subprocess(cmd, print_output=False)
+
+    if ret_code != 0:
+        logger.warning(
+            "Failed to update the RHSM custom facts with return code '%s' and output '%s'.",
+            ret_code,
+            output,
+        )
+        return ret_code, output
+
+    logger.info("RHSM custom facts uploaded successfully.")
     return None, None
 
 

convert2rhel/unit_tests/subscription_test.py

@@ -1098,6 +1098,8 @@ def test_enable_repos_toolopts_enablerepo(
 )
 @centos7
 def test_update_rhsm_custom_facts(subprocess, expected, pretend_os, monkeypatch, caplog):
+    monkeypatch.setattr(pkghandler, "get_installed_pkg_information", mock.Mock(return_value="whatever"))
+
     cmd = ["subscription-manager", "facts", "--update"]
     run_subprocess_mock = RunSubprocessMocked(
         side_effect=unit_tests.run_subprocess_side_effect(
@@ -1118,7 +1120,16 @@ def test_update_rhsm_custom_facts_no_rhsm(global_tool_opts, caplog, monkeypatch)
     global_tool_opts.no_rhsm = True
 
     subscription.update_rhsm_custom_facts()
-    assert "Skipping updating RHSM custom facts." in caplog.records[-1].message
+    assert "Option to not use RHSM detected. Skipping updating RHSM custom facts." in caplog.records[-1].message
+
+
+def test_update_rhsm_custom_facts_no_sub_man(global_tool_opts, caplog, monkeypatch):
+    monkeypatch.setattr(pkghandler, "get_installed_pkg_information", mock.Mock(return_value=[]))
+    subscription.update_rhsm_custom_facts()
+    assert (
+        "The subscription-manager package is not installed. Skipping updating RHSM custom facts."
+        in caplog.records[-1].message
+    )
 
 
 def test_get_rhsm_facts(monkeypatch, global_tool_opts, tmpdir):

tests/integration/tier0/non-destructive/basic-sanity-checks/test_basic_sanity_checks.py

@@ -286,9 +286,7 @@ def test_analyze_incomplete_rollback(remove_repositories, convert2rhel):
             == 0
         )
     # There should be an Error reported during the analysis,
-    # but the report should be printed out successfully
-    # TODO(danmyway) uncomment when https://issues.redhat.com/browse/RHELC-1728 gets resolved
-    # assert c2r.exitstatus == 0
+    assert c2r.exitstatus == 2
 
     with convert2rhel("--debug") as c2r:
         # We need to get past the data collection acknowledgement