[RHELC-1336] Port remove_tmp_dir to the Action framework. (#1283)

* Ported remove_tmp_dir to Action framework, initial code and unit test. * Warn, but don't mark the Action as SKIP if the directory remove fails. * Update convert2rhel/actions/post_conversion/remove_tmp_dir.py Co-authored-by: Rodolfo Olivieri <[email protected]> * Update convert2rhel/unit_tests/actions/post_conversion/remove_tmp_dir_test.py Co-authored-by: Rodolfo Olivieri <[email protected]> * Update convert2rhel/unit_tests/actions/post_conversion/remove_tmp_dir_test.py Co-authored-by: Rodolfo Olivieri <[email protected]> * Update convert2rhel/unit_tests/actions/post_conversion/remove_tmp_dir_test.py Co-authored-by: Rodolfo Olivieri <[email protected]> * Update convert2rhel/unit_tests/actions/post_conversion/remove_tmp_dir_test.py Co-authored-by: Rodolfo Olivieri <[email protected]> * Remove remove_tmp_dir from main. * Nits: use unit_tests non-existing dir and fix a permission complaint. * Update convert2rhel/unit_tests/actions/post_conversion/remove_tmp_dir_test.py Co-authored-by: Rodolfo Olivieri <[email protected]> * Add missing import in unit test. * Update convert2rhel/actions/post_conversion/remove_tmp_dir.py Co-authored-by: Preston Watson <[email protected]> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix unit tests * Improve SKIP message * Fix integration tests and add warning message - the tests were failing since this action was run before UPDATE_GRUB action, which creates backup of boot files. Add the UPDATE_GRUB as dependency - add warning message when TMP_DIR wasn't removed --------- Co-authored-by: Rodolfo Olivieri <[email protected]> Co-authored-by: Preston Watson <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Rodolfo Olivieri <[email protected]> Co-authored-by: Adam Hosek <[email protected]>
oamg · Aug 19, 2024 · 46d8c79 · 46d8c79
1 parent 8cbe69f
commit 46d8c79
Show file tree

Hide file tree

Showing 6 changed files with 154 additions and 8 deletions.
diff --git a/convert2rhel/actions/post_conversion/modified_rpm_files_diff.py b/convert2rhel/actions/post_conversion/modified_rpm_files_diff.py
@@ -49,7 +49,7 @@ def run(self):
                 level="INFO",
                 id="SKIPPED_MODIFIED_RPM_FILES_DIFF",
                 title="Skipped comparison of 'rpm -Va' output from before and after the conversion.",
-                description="Comparison of 'rpm -Va' output was skipped due missing output "
+                description="Comparison of 'rpm -Va' output was not performed due to missing output "
                 "of the 'rpm -Va' run before the conversion.",
                 diagnosis="This is caused mainly by using '--no-rpm-va' argument for convert2rhel.",
             )

diff --git a/convert2rhel/actions/post_conversion/remove_tmp_dir.py b/convert2rhel/actions/post_conversion/remove_tmp_dir.py
@@ -0,0 +1,66 @@
+# Copyright(C) 2024 Red Hat, Inc.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+__metaclass__ = type
+
+import errno
+import logging
+import shutil
+
+from convert2rhel import actions
+from convert2rhel.utils import TMP_DIR
+
+
+loggerinst = logging.getLogger(__name__)
+
+
+class RemoveTmpDir(actions.Action):
+    id = "REMOVE_TMP_DIR"
+    dependencies = ("UPDATE_GRUB",)
+    tmp_dir = TMP_DIR
+
+    def run(self):
+        """Remove the temporary directory (used for backups) and its
+        contents (if any) after the conversion is done. Warns if the
+        removal fails.
+
+        This function is idempotent and will do nothing if the
+        temporary directory does not exist.
+        """
+        super(RemoveTmpDir, self).run()
+
+        loggerinst.task("Final: Remove temporary folder %s" % TMP_DIR)
+
+        try:
+            shutil.rmtree(self.tmp_dir)
+            loggerinst.info("Temporary folder %s removed" % self.tmp_dir)
+        except OSError as exc:
+            # We want run() to be idempotent, so do nothing silently if
+            # the path doesn't exist.
+            # In Python 3 this could be changed to FileNotFoundError.
+            if exc.errno == errno.ENOENT:
+                return
+            warning_message = (
+                "The folder %s is left untouched. You may remove the folder manually"
+                " after you ensure there is no preserved data you would need." % self.tmp_dir
+            )
+            loggerinst.warning(warning_message)
+
+            self.add_message(
+                level="WARNING",
+                id="UNSUCCESSFUL_REMOVE_TMP_DIR",
+                title="Temporary folder {tmp_dir} wasn't removed.".format(tmp_dir=self.tmp_dir),
+                description=warning_message,
+            )
diff --git a/convert2rhel/main.py b/convert2rhel/main.py
@@ -373,9 +373,6 @@ def prepare_system():
 def post_ponr_changes():
     """Start the conversion itself"""
 
-    loggerinst.task("Final: Remove temporary folder %s" % utils.TMP_DIR)
-    utils.remove_tmp_dir()
-
     loggerinst.task("Final: Check kernel boot files")
     checks.check_kernel_boot_files()
 

diff --git a/convert2rhel/unit_tests/actions/post_conversion/modified_rpm_files_diff_test.py b/convert2rhel/unit_tests/actions/post_conversion/modified_rpm_files_diff_test.py
@@ -48,7 +48,7 @@ def test_modified_rpm_files_diff_with_no_rpm_va(monkeypatch, modified_rpm_files_
                 level="INFO",
                 id="SKIPPED_MODIFIED_RPM_FILES_DIFF",
                 title="Skipped comparison of 'rpm -Va' output from before and after the conversion.",
-                description="Comparison of 'rpm -Va' output was skipped due missing output "
+                description="Comparison of 'rpm -Va' output was not performed due to missing output "
                 "of the 'rpm -Va' run before the conversion.",
                 diagnosis="This is caused mainly by using '--no-rpm-va' argument for convert2rhel.",
             ),

diff --git a/convert2rhel/unit_tests/actions/post_conversion/remove_tmp_dir_test.py b/convert2rhel/unit_tests/actions/post_conversion/remove_tmp_dir_test.py
@@ -0,0 +1,86 @@
+# Copyright(C) 2024 Red Hat, Inc.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+__metaclass__ = type
+
+import logging
+import os
+
+import pytest
+
+from convert2rhel import actions, unit_tests
+from convert2rhel.actions.post_conversion import remove_tmp_dir
+
+
+@pytest.fixture
+def remove_tmp_dir_instance():
+    return remove_tmp_dir.RemoveTmpDir()
+
+
+def test_remove_tmp_dir(remove_tmp_dir_instance, monkeypatch, tmpdir, caplog):
+    caplog.set_level(logging.INFO)
+    path = str(tmpdir)
+    monkeypatch.setattr(remove_tmp_dir_instance, "tmp_dir", path)
+    assert os.path.isdir(path)
+    remove_tmp_dir_instance.run()
+    assert "Temporary folder %s removed" % path in caplog.text
+    assert not os.path.isdir(path)
+
+
+def test_remove_tmp_dir_non_existent(remove_tmp_dir_instance, monkeypatch, caplog):
+    caplog.set_level(logging.INFO)
+    path = unit_tests.NONEXISTING_DIR
+    monkeypatch.setattr(remove_tmp_dir_instance, "tmp_dir", path)
+    assert not os.path.isdir(path)
+    remove_tmp_dir_instance.run()
+    assert "Temporary folder %s removed" % path not in caplog.text
+
+
+def test_remove_tmp_dir_failure(remove_tmp_dir_instance, monkeypatch, tmpdir, caplog):
+    caplog.set_level(logging.INFO)
+    path = str(tmpdir)
+    monkeypatch.setattr(remove_tmp_dir_instance, "tmp_dir", path)
+    os.chmod(path, 0)
+    remove_tmp_dir_instance.run()
+    expected_message = (
+        "The folder %s is left untouched. You may remove the folder manually"
+        " after you ensure there is no preserved data you would need." % path
+    )
+    expected = set(
+        (
+            actions.ActionMessage(
+                id="UNSUCCESSFUL_REMOVE_TMP_DIR",
+                level="WARNING",
+                title="Temporary folder {tmp_dir} wasn't removed.".format(tmp_dir=path),
+                description=expected_message,
+            ),
+        ),
+    )
+    assert expected_message in caplog.text
+    assert expected.issubset(remove_tmp_dir_instance.messages)
+    assert expected.issuperset(remove_tmp_dir_instance.messages)
+    os.chmod(path, 0o700)
+
+
+def test_remove_tmp_dir_nonempty(remove_tmp_dir_instance, monkeypatch, tmpdir, caplog):
+    caplog.set_level(logging.INFO)
+    path = str(tmpdir)
+    monkeypatch.setattr(remove_tmp_dir_instance, "tmp_dir", path)
+    assert os.path.isdir(path)
+    with open(os.path.join(path, "remove_tmp_dir_test"), "w") as fp:
+        fp.write("This is a file in the temporary directory.\n")
+    remove_tmp_dir_instance.run()
+    assert "Temporary folder " + str(path) + " removed" in caplog.text
+    assert not os.path.isdir(path)
diff --git a/convert2rhel/unit_tests/main_test.py b/convert2rhel/unit_tests/main_test.py
@@ -241,7 +241,6 @@ def test_main(monkeypatch, tmp_path):
     run_post_actions_mock = mock.Mock()
     clear_versionlock_mock = mock.Mock()
     ask_to_continue_mock = mock.Mock()
-    remove_tmp_dir_mock = mock.Mock()
     restart_system_mock = mock.Mock()
     finish_collection_mock = mock.Mock()
     check_kernel_boot_files_mock = mock.Mock()
@@ -266,7 +265,6 @@ def test_main(monkeypatch, tmp_path):
     monkeypatch.setattr(main, "_raise_for_skipped_failures", raise_for_skipped_failures_mock)
     monkeypatch.setattr(report, "_summary", report_summary_mock)
     monkeypatch.setattr(utils, "ask_to_continue", ask_to_continue_mock)
-    monkeypatch.setattr(utils, "remove_tmp_dir", remove_tmp_dir_mock)
     monkeypatch.setattr(utils, "restart_system", restart_system_mock)
     monkeypatch.setattr(breadcrumbs, "finish_collection", finish_collection_mock)
     monkeypatch.setattr(checks, "check_kernel_boot_files", check_kernel_boot_files_mock)
@@ -290,7 +288,6 @@ def test_main(monkeypatch, tmp_path):
     assert report_summary_mock.call_count == 2
     assert clear_versionlock_mock.call_count == 1
     assert ask_to_continue_mock.call_count == 1
-    assert remove_tmp_dir_mock.call_count == 1
     assert restart_system_mock.call_count == 1
     assert finish_collection_mock.call_count == 1
     assert check_kernel_boot_files_mock.call_count == 1