Cleanup c2r pkg correctly

scripts/conversion_script.py

@@ -222,28 +222,41 @@ def run_subprocess(cmd, print_cmd=True, env=None):
     return output, process.returncode
 
 
+def _check_if_package_installed(pkg_name):
+    _, return_code = run_subprocess(["/usr/bin/rpm", "-q", pkg_name])
+    return return_code == 0
+
+
 def install_convert2rhel():
-    """Install the convert2rhel tool to the system."""
+    """
+    Install the convert2rhel tool to the system.
+    Returns True if the yum transaction should be undone during cleanup.
+    """
     print("Installing & updating Convert2RHEL package.")
-    output, returncode = run_subprocess(
-        ["/usr/bin/yum", "install", "convert2rhel", "-y"],
-    )
-    if returncode:
-        raise ProcessError(
-            message="Failed to install convert2rhel RPM.",
-            report="Installing convert2rhel with yum exited with code '%s' and output: %s."
-            % (returncode, output.rstrip("\n")),
+
+    c2r_pkg_name = "convert2rhel"
+    c2r_installed = _check_if_package_installed(c2r_pkg_name)
+
+    if not c2r_installed:
+        output, returncode = run_subprocess(
+            ["/usr/bin/yum", "install", c2r_pkg_name, "-y"],
         )
+        if returncode:
+            raise ProcessError(
+                message="Failed to install convert2rhel RPM.",
+                report="Installing convert2rhel with yum exited with code '%s' and output: %s."
+                % (returncode, output.rstrip("\n")),
+            )
+        return True
 
-    output, returncode = run_subprocess(
-        ["/usr/bin/yum", "update", "convert2rhel", "-y"]
-    )
+    output, returncode = run_subprocess(["/usr/bin/yum", "update", c2r_pkg_name, "-y"])
     if returncode:
         raise ProcessError(
             message="Failed to update convert2rhel RPM.",
             report="Updating convert2rhel with yum exited with code '%s' and output: %s."
             % (returncode, output.rstrip("\n")),
         )
+    return False
 
 
 def run_convert2rhel():
@@ -270,7 +283,7 @@ def run_convert2rhel():
         )
 
 
-def cleanup(required_files):
+def cleanup(required_files, undo_last_yum_transaction=True):
     """
     Cleanup the downloaded files downloaded in previous steps in this script.
 
@@ -289,6 +302,16 @@ def cleanup(required_files):
             os.remove(required_file.path)
         _create_or_restore_backup_file(required_file)
 
+    if undo_last_yum_transaction:
+        output, returncode = run_subprocess(
+            ["/usr/bin/yum", "history", "undo", "last"],
+        )
+        if returncode:
+            print(
+                "Undo of last yum transaction failed with exist status '%s' and output '%s'"
+                % (returncode, output)
+            )
+
 
 def _create_or_restore_backup_file(required_file):
     """
@@ -415,6 +438,7 @@ def update_insights_inventory():
 
 def main():
     """Main entrypoint for the script."""
+    c2r_installed_undo = True
     output = OutputCollector()
     gpg_key_file = RequiredFile(
         path="/etc/pki/rpm-gpg/RPM-GPG-KEY-redhat-release",
@@ -432,7 +456,7 @@ def main():
     try:
         # Setup Convert2RHEL to be executed.
         setup_convert2rhel(required_files)
-        install_convert2rhel()
+        c2r_installed_undo = install_convert2rhel()
         run_convert2rhel()
 
         # Gather JSON & Textual report
@@ -450,6 +474,12 @@ def main():
         output.message, output.alert = generate_report_message(
             highest_level, gpg_key_file
         )
+
+        if "successfully" in output.message:
+            # NOTE: When c2r statistics on insights are not reliant on rpm being installed
+            # remove this condition (=decide only based on install_convert2rhel() result)
+            c2r_installed_undo = False
+
         output.entries = transform_raw_data(data)
         update_insights_inventory()
         print("Conversion script finish successfully!")
@@ -473,7 +503,7 @@ def main():
         )
     finally:
         print("Cleaning up modifications to the system.")
-        cleanup(required_files)
+        cleanup(required_files, undo_last_yum_transaction=c2r_installed_undo)
 
         print("### JSON START ###")
         print(json.dumps(output.to_dict(), indent=4))

scripts/preconversion_assessment_script.py

@@ -217,28 +217,41 @@ def run_subprocess(cmd, print_cmd=True, env=None):
     return output, process.returncode
 
 
+def _check_if_package_installed(pkg_name):
+    _, return_code = run_subprocess(["/usr/bin/rpm", "-q", pkg_name])
+    return return_code == 0
+
+
 def install_convert2rhel():
-    """Install the convert2rhel tool to the system."""
+    """
+    Install the convert2rhel tool to the system.
+    Returns True if the yum transaction should be undone during cleanup.
+    """
     print("Installing & updating Convert2RHEL package.")
-    output, returncode = run_subprocess(
-        ["/usr/bin/yum", "install", "convert2rhel", "-y"],
-    )
-    if returncode:
-        raise ProcessError(
-            message="Failed to install convert2rhel RPM.",
-            report="Installing convert2rhel with yum exited with code '%s' and output: %s."
-            % (returncode, output.rstrip("\n")),
+
+    c2r_pkg_name = "convert2rhel"
+    c2r_installed = _check_if_package_installed(c2r_pkg_name)
+
+    if not c2r_installed:
+        output, returncode = run_subprocess(
+            ["/usr/bin/yum", "install", c2r_pkg_name, "-y"],
         )
+        if returncode:
+            raise ProcessError(
+                message="Failed to install convert2rhel RPM.",
+                report="Installing convert2rhel with yum exited with code '%s' and output: %s."
+                % (returncode, output.rstrip("\n")),
+            )
+        return True
 
-    output, returncode = run_subprocess(
-        ["/usr/bin/yum", "update", "convert2rhel", "-y"]
-    )
+    output, returncode = run_subprocess(["/usr/bin/yum", "update", c2r_pkg_name, "-y"])
     if returncode:
         raise ProcessError(
             message="Failed to update convert2rhel RPM.",
             report="Updating convert2rhel with yum exited with code '%s' and output: %s."
             % (returncode, output.rstrip("\n")),
         )
+    return False
 
 
 def run_convert2rhel():
@@ -267,7 +280,7 @@ def run_convert2rhel():
         )
 
 
-def cleanup(required_files):
+def cleanup(required_files, undo_last_yum_transaction=True):
     """
     Cleanup the downloaded files downloaded in previous steps in this script.
 
@@ -284,6 +297,16 @@ def cleanup(required_files):
             os.remove(required_file.path)
         _create_or_restore_backup_file(required_file)
 
+    if undo_last_yum_transaction:
+        output, returncode = run_subprocess(
+            ["/usr/bin/yum", "history", "undo", "last"],
+        )
+        if returncode:
+            print(
+                "Undo of last yum transaction failed with exist status '%s' and output '%s'"
+                % (returncode, output)
+            )
+
 
 def _create_or_restore_backup_file(required_file):
     """
@@ -395,6 +418,7 @@ def transform_raw_data(raw_data):
 
 def main():
     """Main entrypoint for the script."""
+    c2r_installed_undo = True
     output = OutputCollector()
     required_files = [
         RequiredFile(
@@ -410,7 +434,7 @@ def main():
     try:
         # Setup Convert2RHEL to be executed.
         setup_convert2rhel(required_files)
-        install_convert2rhel()
+        c2r_installed_undo = install_convert2rhel()
         run_convert2rhel()
 
         # Gather JSON & Textual report
@@ -448,7 +472,7 @@ def main():
         )
     finally:
         print("Cleaning up modifications to the system.")
-        cleanup(required_files)
+        cleanup(required_files, undo_last_yum_transaction=c2r_installed_undo)
 
         print("### JSON START ###")
         print(json.dumps(output.to_dict(), indent=4))

tests/conversion_script/test_check_pkg_installed.py

@@ -0,0 +1,13 @@
+from mock import patch
+
+from scripts.conversion_script import _check_if_package_installed
+
+
+@patch("scripts.conversion_script.run_subprocess")
+def test_check_if_package_installed(mock_run_subprocess):
+    pkg_name = "example-package"
+    mock_run_subprocess.return_value = ("", 0)
+    result = _check_if_package_installed(pkg_name)
+    expected_command = ["/usr/bin/rpm", "-q", pkg_name]
+    mock_run_subprocess.assert_called_once_with(expected_command)
+    assert result

tests/conversion_script/test_cleanup.py

@@ -3,10 +3,13 @@
 from scripts.conversion_script import cleanup, RequiredFile
 
 
+@patch("scripts.conversion_script.run_subprocess", return_value=("", 0))
 @patch("os.path.exists", side_effect=Mock())
 @patch("os.remove", side_effect=Mock())
 @patch("scripts.conversion_script._create_or_restore_backup_file")
-def test_cleanup_with_file_to_remove(mock_restore, mock_remove, mock_exists):
+def test_cleanup_with_file_to_remove(
+    mock_restore, mock_remove, mock_exists, mock_yum_undo
+):
     """Only downloaded files are removed."""
 
     present_file = RequiredFile("/already/present")
@@ -18,20 +21,44 @@ def test_cleanup_with_file_to_remove(mock_restore, mock_remove, mock_exists):
     assert mock_exists.call_count == 1
     assert mock_remove.call_count == 1
     assert mock_restore.call_count == 1
+    assert mock_yum_undo.call_count == 1
 
 
+@patch("scripts.conversion_script.run_subprocess", return_value=("", 1))
 @patch("os.path.exists", side_effect=Mock())
 @patch("os.remove", side_effect=Mock())
 @patch("scripts.conversion_script._create_or_restore_backup_file")
-def test_cleanup_with_file_to_keep(mock_restore, mock_remove, mock_exists):
+def test_cleanup_with_file_to_keep(
+    mock_restore, mock_remove, mock_exists, mock_yum_undo
+):
     """Only downloaded files are removed."""
 
     keep_downloaded_file = RequiredFile("/download/keep", keep=True)
     required_files = [keep_downloaded_file]
 
-    cleanup(required_files)
+    cleanup(required_files, undo_last_yum_transaction=False)
 
     # For removal of file, then two checks in backup function
     assert mock_exists.call_count == 0
     assert mock_remove.call_count == 0
     assert mock_restore.call_count == 0
+    assert mock_yum_undo.call_count == 0
+
+
+@patch("scripts.conversion_script.run_subprocess", return_value=("", 1))
+@patch("os.path.exists", side_effect=Mock())
+@patch("os.remove", side_effect=Mock())
+@patch("scripts.conversion_script._create_or_restore_backup_file")
+def test_cleanup_with_undo_yum(mock_restore, mock_remove, mock_exists, mock_yum_undo):
+    """Only downloaded files are removed."""
+
+    present_file = RequiredFile("/already/present")
+    required_files = [present_file]
+
+    cleanup(required_files, undo_last_yum_transaction=True)
+
+    # For removal of file, then two checks in backup function
+    assert mock_exists.call_count == 1
+    assert mock_remove.call_count == 1
+    assert mock_restore.call_count == 1
+    assert mock_yum_undo.call_count == 1

tests/conversion_script/test_install.py

@@ -7,52 +7,67 @@
 )
 
 
-@pytest.mark.parametrize("subprocess_mock", [(b"output", 0), (b"output", 0)])
-def test_install_convert2rhel(subprocess_mock):
+@pytest.mark.parametrize(
+    ("subprocess_mock", "pkg_installed_mock", "should_undo_transaction"),
+    (
+        ((b"output", 0), False, True),
+        ((b"output", 0), True, False),
+    ),
+)
+def test_install_convert2rhel(
+    subprocess_mock, pkg_installed_mock, should_undo_transaction
+):
     with patch(
         "scripts.conversion_script.run_subprocess",
         return_value=subprocess_mock,
     ) as mock_run_subprocess:
-        install_convert2rhel()
+        with patch(
+            "scripts.conversion_script._check_if_package_installed",
+            return_value=pkg_installed_mock,
+        ) as mock_run_pkg_check:
+            return_status = install_convert2rhel()
 
-    expected_calls = [
-        ["/usr/bin/yum", "install", "convert2rhel", "-y"],
-        ["/usr/bin/yum", "update", "convert2rhel", "-y"],
-    ]
+    assert return_status is should_undo_transaction
+    mock_run_pkg_check.assert_called_once()
+    mock_run_subprocess.assert_called_once()
+
+    if pkg_installed_mock:
+        expected_calls = [
+            ["/usr/bin/yum", "update", "convert2rhel", "-y"],
+        ]
+    else:
+        expected_calls = [["/usr/bin/yum", "install", "convert2rhel", "-y"]]
 
     assert mock_run_subprocess.call_args_list == [call(args) for args in expected_calls]
 
 
-def test_install_convert2rhel_raise_exception():
-    with patch(
-        "scripts.conversion_script.run_subprocess",
-        return_value=(b"failed", 1),
-    ) as mock_run_subprocess:
-        with pytest.raises(
-            ProcessError,
-            match="Installing convert2rhel with yum exited with code '1' and output: failed.",
-        ):
-            install_convert2rhel()
+@patch("scripts.conversion_script._check_if_package_installed", return_value=False)
+@patch("scripts.conversion_script.run_subprocess", return_value=(b"failed", 1))
+def test_install_convert2rhel_raise_exception(mock_run_subprocess, mock_pkg_check):
+    with pytest.raises(
+        ProcessError,
+        match="Installing convert2rhel with yum exited with code '1' and output: failed.",
+    ):
+        install_convert2rhel()
 
     expected_calls = [["/usr/bin/yum", "install", "convert2rhel", "-y"]]
 
+    mock_pkg_check.assert_called_once()
     assert mock_run_subprocess.call_args_list == [call(args) for args in expected_calls]
 
 
-def test_update_convert2rhel_raise_exception():
-    with patch(
-        "scripts.conversion_script.run_subprocess",
-        side_effect=[(b"output", 0), (b"failed", 1)],
-    ) as mock_run_subprocess:
-        with pytest.raises(
-            ProcessError,
-            match="Updating convert2rhel with yum exited with code '1' and output: failed.",
-        ):
-            install_convert2rhel()
+@patch("scripts.conversion_script._check_if_package_installed", return_value=True)
+@patch("scripts.conversion_script.run_subprocess", return_value=(b"failed", 1))
+def test_update_convert2rhel_raise_exception(mock_run_subprocess, mock_pkg_check):
+    with pytest.raises(
+        ProcessError,
+        match="Updating convert2rhel with yum exited with code '1' and output: failed.",
+    ):
+        install_convert2rhel()
 
     expected_calls = [
-        ["/usr/bin/yum", "install", "convert2rhel", "-y"],
         ["/usr/bin/yum", "update", "convert2rhel", "-y"],
     ]
 
+    mock_pkg_check.assert_called_once()
     assert mock_run_subprocess.call_args_list == [call(args) for args in expected_calls]