[RHELC-1397] Always save conversion facts file (oamg#1102)

* Always save conversion facts. * Remove unit tests related to disabling telemetry. * Remove disabled telemetry checks from integration tests. * Apply suggestions from code review Co-authored-by: Michal Bocek <[email protected]> --------- Co-authored-by: Michal Bocek <[email protected]>
jochapma · Mar 11, 2024 · 7909dd6 · 7909dd6
1 parent 5b8d2b0
commit 7909dd6
Show file tree

Hide file tree

Showing 13 changed files with 33 additions and 169 deletions.
diff --git a/convert2rhel/breadcrumbs.py b/convert2rhel/breadcrumbs.py
@@ -72,8 +72,6 @@ def __init__(self):
         self.run_id = "null"
         # The convert2rhel package object from the yum/dnf python API for further information extraction.
         self._pkg_object = None
-        # Flag to inform the user about DISABLE_TELEMETRY. If not informed, we shouldn't save rhsm_facts.
-        self._inform_telemetry = False
 
     def collect_early_data(self):
         """Set data which is accessible before the conversion"""
@@ -101,8 +99,7 @@ def finish_collection(self, success=False):
         self._set_ended()
 
         self._save_migration_results()
-        if self._inform_telemetry and "CONVERT2RHEL_DISABLE_TELEMETRY" not in os.environ:
-            self._save_rhsm_facts()
+        self._save_rhsm_facts()
 
     def _set_activity(self):
         """Set the activity that convert2rhel is going to perform"""
@@ -213,26 +210,21 @@ def _save_rhsm_facts(self):
         utils.write_json_object_to_file(path=RHSM_CUSTOM_FACTS_FILE, data=data)
 
     def print_data_collection(self):
-        """Print information about telemetry and ask for acknowledge."""
-        if "CONVERT2RHEL_DISABLE_TELEMETRY" not in os.environ:
-            loggerinst.info(
-                "The convert2rhel utility uploads the following data about the system conversion"
-                " to Red Hat servers for the purpose of the utility usage analysis:\n"
-                "- The Convert2RHEL command as executed\n"
-                "- The Convert2RHEL RPM version and GPG signature\n"
-                "- Success or failure status of the conversion\n"
-                "- Conversion start and end timestamps\n"
-                "- Source OS vendor and version\n"
-                "- Target RHEL version\n"
-                "- Convert2RHEL related environment variables\n\n"
-                "To disable the data collection, use the 'CONVERT2RHEL_DISABLE_TELEMETRY=1' environment variable."
-            )
-
-            utils.ask_to_continue()
-            # User informed about CONVERT2RHEL_DISABLE_TELEMETRY environment variable
-            self._inform_telemetry = True
-        else:
-            loggerinst.info("Skipping, telemetry disabled.")
+        """Print information about data collection and ask for acknowledgement."""
+        loggerinst.info(
+            "The convert2rhel utility generates a /etc/rhsm/facts/convert2rhel.fact file that contains the below data"
+            " about the system conversion. The subscription-manager then uploads the data to the server the system is"
+            " registered to.\n"
+            "- The Convert2RHEL command as executed\n"
+            "- The Convert2RHEL RPM version and GPG signature\n"
+            "- Success or failure status of the conversion\n"
+            "- Conversion start and end timestamps\n"
+            "- Source OS vendor and version\n"
+            "- Target RHEL version\n"
+            "- Convert2RHEL related environment variables\n\n"
+        )
+
+        utils.ask_to_continue()
 
 
 def _write_obj_to_array_json(path, new_object, key):

diff --git a/convert2rhel/main.py b/convert2rhel/main.py
@@ -226,8 +226,7 @@ def perform_boilerplate():
     loggerinst.task("Prepare: Show Red Hat software EULA")
     show_eula()
 
-    # Telemetry opt-out
-    loggerinst.task("Prepare: Inform about telemetry")
+    loggerinst.task("Prepare: Inform about data collection")
     breadcrumbs.breadcrumbs.print_data_collection()
 
 

diff --git a/convert2rhel/subscription.py b/convert2rhel/subscription.py
@@ -882,10 +882,6 @@ def update_rhsm_custom_facts():
     the conversion with the candlepin server, thus, propagating the
     "breadcrumbs" from convert2rhel as RHSM facts.
     """
-    if "CONVERT2RHEL_DISABLE_TELEMETRY" in os.environ:
-        loggerinst.info("Telemetry disabled, skipping RHSM facts upload.")
-        return
-
     if not tool_opts.no_rhsm:
         loggerinst.info("Updating RHSM custom facts collected during the conversion.")
         cmd = ["subscription-manager", "facts", "--update"]

diff --git a/convert2rhel/unit_tests/breadcrumbs_test.py b/convert2rhel/unit_tests/breadcrumbs_test.py
@@ -101,8 +101,6 @@ def test_finish_collection_success(
         breadcrumbs_instance, "_save_migration_results", finish_collection_mocks["save_migration_results"]
     )
     monkeypatch.setattr(breadcrumbs_instance, "_save_rhsm_facts", finish_collection_mocks["save_rhsm_facts"])
-    # Set to true, pretend that user was informed about collecting data
-    monkeypatch.setattr(breadcrumbs_instance, "_inform_telemetry", True)
 
     global_tool_opts.activity = activity
     breadcrumbs_instance.collect_early_data()
@@ -136,8 +134,6 @@ def test_finish_collection_failure(
         breadcrumbs_instance, "_save_migration_results", finish_collection_mocks["save_migration_results"]
     )
     monkeypatch.setattr(breadcrumbs_instance, "_save_rhsm_facts", finish_collection_mocks["save_rhsm_facts"])
-    # Set to true, pretend that user was informed about collecting data
-    monkeypatch.setattr(breadcrumbs_instance, "_inform_telemetry", True)
 
     global_tool_opts.activity = activity
     breadcrumbs_instance.collect_early_data()
@@ -342,51 +338,3 @@ def test_set_target_os(pretend_os):
         "name": "CentOS Linux",
         "version": "7.9",
     } == breadcrumbs.breadcrumbs.target_os
-
-
-@pytest.mark.parametrize(("telemetry_disabled", "telemetry_called"), [(True, 0), (False, 1)])
-def test_disable_telemetry(telemetry_disabled, telemetry_called, monkeypatch):
-    if telemetry_disabled:
-        monkeypatch.setenv("CONVERT2RHEL_DISABLE_TELEMETRY", "1")
-
-    _save_migration_results = mock.Mock()
-    _save_rhsm_facts = mock.Mock()
-
-    monkeypatch.setattr(breadcrumbs.breadcrumbs, "_save_migration_results", _save_migration_results)
-    monkeypatch.setattr(breadcrumbs.breadcrumbs, "_save_rhsm_facts", _save_rhsm_facts)
-    # Set to true, pretend that user was informed about collecting data
-    monkeypatch.setattr(breadcrumbs.breadcrumbs, "_inform_telemetry", True)
-
-    breadcrumbs.breadcrumbs.finish_collection()
-
-    assert _save_migration_results.call_count == 1
-    assert _save_rhsm_facts.call_count == telemetry_called
-
-
-def test_user_not_informed_about_telemetry(monkeypatch):
-    _save_migration_results = mock.Mock()
-    _save_rhsm_facts = mock.Mock()
-
-    monkeypatch.setattr(breadcrumbs.breadcrumbs, "_save_migration_results", _save_migration_results)
-    monkeypatch.setattr(breadcrumbs.breadcrumbs, "_save_rhsm_facts", _save_rhsm_facts)
-
-    breadcrumbs.breadcrumbs.finish_collection()
-
-    _save_migration_results.assert_called_once()
-    _save_rhsm_facts.assert_not_called()
-
-
-@pytest.mark.parametrize(("telemetry_disabled", "call_count"), [(True, 0), (False, 1)])
-def test_print_data_collection(telemetry_disabled, call_count, monkeypatch, caplog):
-    if telemetry_disabled:
-        monkeypatch.setenv("CONVERT2RHEL_DISABLE_TELEMETRY", "1")
-
-    ask_to_continue = mock.Mock()
-    monkeypatch.setattr(utils, "ask_to_continue", ask_to_continue)
-
-    breadcrumbs.breadcrumbs.print_data_collection()
-
-    ask_to_continue.call_count == call_count
-
-    if telemetry_disabled:
-        assert "Skipping, telemetry disabled." in caplog.records[-1].message
diff --git a/convert2rhel/unit_tests/subscription_test.py b/convert2rhel/unit_tests/subscription_test.py
@@ -1257,12 +1257,3 @@ def test_update_rhsm_custom_facts_no_rhsm(global_tool_opts, caplog, monkeypatch)
 
     subscription.update_rhsm_custom_facts()
     assert "Skipping updating RHSM custom facts." in caplog.records[-1].message
-
-
-def test_update_rhsm_custom_facts_disable_telemetry(monkeypatch, caplog):
-    message = "Telemetry disabled, skipping RHSM facts upload."
-    monkeypatch.setenv("CONVERT2RHEL_DISABLE_TELEMETRY", "1")
-
-    subscription.update_rhsm_custom_facts()
-
-    assert message in caplog.records[-1].message
diff --git a/plans/tier0.fmf b/plans/tier0.fmf
@@ -23,8 +23,6 @@ description+: |
         /basic_conversion_methods:
 
             /activation_key_conversion:
-                environment+:
-                        CONVERT2RHEL_DISABLE_TELEMETRY: 1
                 discover+:
                     test+<:
                         - conversion-method/activation_key_conversion

diff --git a/tests/integration/common/checks-after-conversion/main.fmf b/tests/integration/common/checks-after-conversion/main.fmf
@@ -38,9 +38,7 @@ order: 52
     summary+: |
         Data collection
     description: |
-        Verify, that when the data collection is disabled with CONVERT2RHEL_DISABLE_TELEMETRY,
-        the convert2rhel.facts file is not present on the system after the conversion.
-        When data collection is enabled, verify, the convert2rhel.facts file exists.
+        Verify that after conversion the convert2rhel.facts file exists.
     test: pytest -svv -m test_check_data_collection
 
 

diff --git a/tests/integration/common/checks-after-conversion/test_check_data_collection.py b/tests/integration/common/checks-after-conversion/test_check_data_collection.py
@@ -6,11 +6,7 @@
 @pytest.mark.test_check_data_collection
 def test_check_data_collection():
     """
-    Verify, that convert2rhel.facts data are not collected, when CONVERT2RHEL_DISABLE_TELEMETRY envar is set.
-    Also verify, the file is present, when the environment variable is not set
+    Verify that after conversion the convert2rhel.facts file is present.
     """
     convert2rhel_facts_file = "/etc/rhsm/facts/convert2rhel.facts"
-    if os.getenv("CONVERT2RHEL_DISABLE_TELEMETRY") and os.environ["CONVERT2RHEL_DISABLE_TELEMETRY"] == "1":
-        assert not os.path.exists(convert2rhel_facts_file)
-    else:
-        assert os.path.exists(convert2rhel_facts_file)
+    assert os.path.exists(convert2rhel_facts_file)
diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py
@@ -519,22 +519,6 @@ def pre_registered(shell):
         del os.environ["C2R_TESTS_SUBMAN_CLEANUP"]
 
 
-@pytest.fixture
-def disabled_telemetry(shell):
-    """
-    Fixture exporting CONVERT2RHEL_DISABLE_TELEMETRY envar to disable data collection.
-    Removes after the test.
-    Used in scenarios where we do not care about the data collection and want to bypass
-    the data collection acknowledgement prompt.
-    """
-    os.environ["CONVERT2RHEL_DISABLE_TELEMETRY"] = "1"
-
-    yield
-
-    if os.environ["CONVERT2RHEL_DISABLE_TELEMETRY"]:
-        del os.environ["CONVERT2RHEL_DISABLE_TELEMETRY"]
-
-
 @pytest.fixture()
 def hybrid_rocky_image(shell, system_release):
     """

diff --git a/tests/integration/tier0/non-destructive/basic-sanity-checks/main.fmf b/tests/integration/tier0/non-destructive/basic-sanity-checks/main.fmf
@@ -146,20 +146,6 @@ tag+:
         test: |
           pytest -svv -m test_data_collection_acknowledgement
 
-    /disable_data_collection:
-        summary+: |
-            Disabled data collection.
-        description+: |
-            Verify that disabling the data collection inhibits the conversion.
-            The convert2rhel.facts file is not created.
-        adjust+:
-            - environment+:
-                CONVERT2RHEL_DISABLE_TELEMETRY: 1
-        tag+:
-            - disable-data-collection
-        test: |
-          pytest -svv -m test_disable_data_collection
-
 /incomplete_rollback_in_analyze:
     summary+: |
         Incomplete rollback envar not honored in analyze

diff --git a/tests/integration/tier0/non-destructive/basic-sanity-checks/test_basic_sanity_checks.py b/tests/integration/tier0/non-destructive/basic-sanity-checks/test_basic_sanity_checks.py
@@ -278,12 +278,9 @@ def test_data_collection_acknowledgement(shell, convert2rhel):
     """
     # Remove facts from previous runs.
     shell(f"rm -f {CONVERT2RHEL_FACTS_FILE}")
-    # Remove envar disabling telemetry just in case.
-    if os.getenv("CONVERT2RHEL_DISABLE_TELEMETRY"):
-        del os.environ["CONVERT2RHEL_DISABLE_TELEMETRY"]
 
     with convert2rhel("--debug") as c2r:
-        assert c2r.expect("Prepare: Inform about telemetry", timeout=300) == 0
+        assert c2r.expect("Prepare: Inform about data collection", timeout=300) == 0
         assert (
             c2r.expect("The convert2rhel utility uploads the following data about the system conversion", timeout=300)
             == 0
@@ -297,31 +294,6 @@ def test_data_collection_acknowledgement(shell, convert2rhel):
     assert c2r.exitstatus != 0
 
 
-@pytest.mark.test_disable_data_collection
-def test_disable_data_collection(shell, convert2rhel):
-    """
-    This test verifies functionality of CONVERT2RHEL_DISABLE_TELEMETRY envar.
-    The data collection should be disabled, therefore convert2rhel.facts file should not get created.
-    The environment variable is set by tmt test metadata.
-    """
-    # Remove facts from previous runs.
-    shell(f"rm -f {CONVERT2RHEL_FACTS_FILE}")
-
-    with convert2rhel("--debug") as c2r:
-        assert c2r.expect("Prepare: Inform about telemetry", timeout=300) == 0
-        assert c2r.expect("Skipping, telemetry disabled.", timeout=300) == 0
-
-        c2r.sendcontrol("c")
-
-        # Verify the file is not created if CONVERT2RHEL_DISABLE_TELEMETRY is set.
-        assert not os.path.exists(CONVERT2RHEL_FACTS_FILE)
-
-    assert c2r.exitstatus != 0
-
-    # Remove envar disabling telemetry.
-    del os.environ["CONVERT2RHEL_DISABLE_TELEMETRY"]
-
-
 @pytest.fixture
 def analyze_incomplete_rollback_envar():
     os.environ["CONVERT2RHEL_UNSUPPORTED_INCOMPLETE_ROLLBACK"] = "1"

diff --git a/...n-destructive/single-yum-transaction-validation/test_single_yum_transaction_validation.py b/...n-destructive/single-yum-transaction-validation/test_single_yum_transaction_validation.py
@@ -152,8 +152,6 @@ def packages_with_period(shell):
     E.g. python3.11-3.11.2-2.el8.x86_64 java-1.8.0-openjdk-headless-1.8.0.372.b07-4.el8.x86_64
     """
     problematic_packages = ["python3.11", "java-1.8.0-openjdk-headless"]
-    # We don't care for the telemetry, disable the collection to skip over the acknowledgement
-    os.environ["CONVERT2RHEL_DISABLE_TELEMETRY"] = "1"
 
     # Install packages with in name period
     for package in problematic_packages:
@@ -165,9 +163,6 @@ def packages_with_period(shell):
     for package in problematic_packages:
         shell(f"yum remove -y {package}")
 
-    # Remove the envar
-    del os.environ["CONVERT2RHEL_DISABLE_TELEMETRY"]
-
 
 @pytest.mark.test_validation_packages_with_in_name_period
 def test_validation_packages_with_in_name_period(shell, convert2rhel, packages_with_period):
@@ -189,6 +184,15 @@ def test_validation_packages_with_in_name_period(shell, convert2rhel, packages_w
             env.str("RHSM_POOL"),
         )
     ) as c2r:
+        # Swallow the data collection warning
+        assert c2r.expect("Prepare: Inform about data collection", timeout=300) == 0
+        assert (
+            c2r.expect("The convert2rhel utility uploads the following data about the system conversion", timeout=300)
+            == 0
+        )
+        c2r.expect("Continue with the system conversion", timeout=300)
+        c2r.sendline("y")
+
         c2r.expect("VALIDATE_PACKAGE_MANAGER_TRANSACTION has succeeded")
         # Exit at PONR
         c2r.expect("Continue with the system conversion?")

diff --git a/tests/integration/tier0/non-destructive/subscription-manager/test_sub_man_pre_register.py b/tests/integration/tier0/non-destructive/subscription-manager/test_sub_man_pre_register.py
@@ -4,7 +4,7 @@
 
 
 @pytest.mark.test_pre_registered_wont_unregister
-def test_pre_registered_wont_unregister(shell, pre_registered, disabled_telemetry, convert2rhel):
+def test_pre_registered_wont_unregister(shell, pre_registered, convert2rhel):
     """
     This test verifies that running conversion on pre-registered system won't unregister the system.
     1. Install subscription-manager, download the SSL certificate
@@ -27,7 +27,7 @@ def test_pre_registered_wont_unregister(shell, pre_registered, disabled_telemetr
 
 
 @pytest.mark.test_pre_registered_re_register
-def test_pre_registered_re_register(shell, pre_registered, disabled_telemetry, convert2rhel):
+def test_pre_registered_re_register(shell, pre_registered, convert2rhel):
     """
     This test verifies that running conversion on pre-registered system and providing convert2rhel
     with credentials, will re-register the system.
@@ -53,7 +53,7 @@ def test_pre_registered_re_register(shell, pre_registered, disabled_telemetry, c
 
 
 @pytest.mark.test_unregistered_no_credentials
-def test_unregistered_no_credentials(shell, convert2rhel, disabled_telemetry):
+def test_unregistered_no_credentials(shell, convert2rhel):
     """
     This test verifies that conversion fails when the system is not pre-registered
     and credentials are not provided to the convert2rhel command.