exit with 1 if failed

convert2rhel/backup/__init__.py

@@ -48,6 +48,7 @@ class BackupController:
 
     def __init__(self):
         self._restorables = []
+        self._rollback_failures = []
 
     def push(self, restorable):
         """
@@ -86,18 +87,18 @@ def pop_all(self):
         """
         Restores all RestorableChanges known to the Controller and then returns them.
 
-        :returns: List of RestorableChange objects that were known to the Controller.
+        :returns list[RestorableChange]: List of RestorableChange objects that were processed by the Controller.
         :raises IndexError: If there are no RestorableChanges currently known to the Controller.
 
         After running, the Controller object will not know about any RestorableChanges.
         """
         # Only raise IndexError if there are no restorables registered.
-        # Partitions are ignored for this check as they aren't really Changes.
         if not self._restorables:
             raise IndexError("No backups to restore")
 
-        # Restore the Changes in the reverse order the changes were enabled.
         processed_restorables = []
+
+        # Restore the Changes in the reverse order the changes were enabled.
         while True:
             try:
                 restorable = self._restorables.pop()
@@ -110,12 +111,34 @@ def pop_all(self):
             # logger.critical in some places.
             except (Exception, SystemExit) as e:
                 # Don't let a failure in one restore influence the others
-                loggerinst.warning("Error while rolling back a %s: %s" % (restorable.__class__.__name__, str(e)))
+                message = "Error while rolling back a %s: %s" % (restorable.__class__.__name__, str(e))
+                loggerinst.warning(message)
+                # Add the rollback failures to the list
+                self._rollback_failures.append(message)
 
             processed_restorables.append(restorable)
 
         return processed_restorables
 
+    @property
+    def rollback_failed(self):
+        """
+        Return True when one or more restorables were unsuccessful.
+
+        :returns: Boolean value if the rollback failed.
+        """
+
+        return any(self._rollback_failures)
+
+    @property
+    def rollback_failures(self):
+        """
+        Errors from failed rollback. Used to provide more information to the user.
+
+        :returns: List of strings containing errors captured during rollback.
+        """
+        return self._rollback_failures
+
 
 @six.add_metaclass(abc.ABCMeta)
 class RestorableChange:

convert2rhel/backup/certs.py

@@ -187,7 +187,8 @@ def _restore(self):
                 # Resolves RHSM error when removing certs, as the system might not have installed any certs yet
                 loggerinst.info("No certificates found to be removed.")
             else:
-                loggerinst.error("OSError({0}): {1}".format(err.errno, err.strerror))
+                # Will be handled in BackupController
+                raise err
 
 
 def _get_cert(cert_dir):

convert2rhel/backup/files.py

@@ -118,15 +118,9 @@ def restore(self, rollback=True):
             loggerinst.info("%s hasn't been backed up." % self.filepath)
             return
 
-        try:
-            shutil.copy2(self._backup_path, self.filepath)
-            os.remove(self._backup_path)
-        except (OSError, IOError) as err:
-            # Do not call 'critical' which would halt the program. We are in
-            # a rollback phase now and we want to rollback as much as possible.
-            # IOError for py2 and OSError for py3
-            loggerinst.critical_no_exit("Error(%s): %s" % (err.errno, err.strerror))
-            return
+        # Possible exceptions will be handled in the BackupController
+        shutil.copy2(self._backup_path, self.filepath)
+        os.remove(self._backup_path)
 
         if rollback:
             loggerinst.info("File %s restored." % self.filepath)
@@ -179,13 +173,8 @@ def restore(self):
         if not os.path.isfile(self.filepath):
             loggerinst.info("File {filepath} wasn't created during conversion".format(filepath=self.filepath))
         else:
-            try:
-                os.remove(self.filepath)
-                loggerinst.info("File {filepath} removed".format(filepath=self.filepath))
-            except OSError as err:
-                # Do not call 'critical' which would halt the program. We are in
-                # a rollback phase now and we want to rollback as much as possible.
-                loggerinst.critical_no_exit("Error(%s): %s" % (err.errno, err.strerror))
-                return
+            # Possible exceptions will be handled in the BackupController
+            os.remove(self.filepath)
+            loggerinst.info("File {filepath} removed".format(filepath=self.filepath))
 
-        super(MissingFile, self).restore()
+            super(MissingFile, self).restore()

convert2rhel/main.py

@@ -167,12 +167,11 @@ def main_locked():
             subscription.update_rhsm_custom_facts()
 
         rollback_changes()
+        provide_status_after_rollback(pre_conversion_results, True)
+
+        if backup.backup_control.rollback_failed:
+            return 1
 
-        report.summary(
-            pre_conversion_results,
-            include_all_reports=True,
-            disable_colors=logger_module.should_disable_color_output(),
-        )
         return 0
 
     except exceptions.CriticalError as err:
@@ -185,7 +184,7 @@ def main_locked():
     finally:
         # Write the assessment to a file as json data so that other tools can
         # parse and act upon it.
-        if pre_conversion_results:
+        if pre_conversion_results and not backup.backup_control.rollback_failed:
             actions.report.summary_as_json(pre_conversion_results)
             actions.report.summary_as_txt(pre_conversion_results)
 
@@ -222,14 +221,7 @@ def _handle_main_exceptions(process_phase, pre_conversion_results=None):
             subscription.update_rhsm_custom_facts()
 
         rollback_changes()
-        if pre_conversion_results is None:
-            loggerinst.info("\nConversion interrupted before analysis of system completed. Report not generated.\n")
-        else:
-            report.summary(
-                pre_conversion_results,
-                include_all_reports=(toolopts.tool_opts.activity == "analysis"),
-                disable_colors=logger_module.should_disable_color_output(),
-            )
+        provide_status_after_rollback(pre_conversion_results, (toolopts.tool_opts.activity == "analysis"))
     elif process_phase == ConversionPhase.POST_PONR_CHANGES:
         # After the process of subscription is done and the mass update of
         # packages is started convert2rhel will not be able to guarantee a
@@ -363,3 +355,31 @@ def rollback_changes():
             loggerinst.info("During rollback there were no backups to restore")
         else:
             raise
+
+    return
+
+
+def provide_status_after_rollback(pre_conversion_results, include_all_reports):
+    """Print after-rollback messages. Also determine if there is a report to print or
+    report should't be printed after failure in rollback."""
+    if backup.backup_control.rollback_failed:
+        loggerinst.critical_no_exit(
+            "Rollback of system wasn't completed successfully.\n"
+            "The system is left in an undetermined state that Convert2RHEL cannot fix.\n"
+            "It is strongly recommended to store the Convert2RHEL logs for later investigation, and restore"
+            " the system from a backup.\n"
+            "Following errors were captured during rollback:\n"
+            "%s" % "\n".join(backup.backup_control.rollback_failures)
+        )
+
+        return
+
+    elif not pre_conversion_results:
+        loggerinst.info("\nConversion interrupted before analysis of system completed. Report not generated.\n")
+
+    else:
+        report.summary(
+            results=pre_conversion_results,
+            include_all_reports=include_all_reports,
+            disable_colors=logger_module.should_disable_color_output(),
+        )

convert2rhel/unit_tests/__init__.py

@@ -379,6 +379,10 @@ class RollbackChangesMocked(MockFunctionObject):
     spec = main.rollback_changes
 
 
+class PrintInfoAfterRollbackMocked(MockFunctionObject):
+    spec = main.provide_status_after_rollback
+
+
 class ShowEulaMocked(MockFunctionObject):
     spec = main.show_eula
 

convert2rhel/unit_tests/backup/backup_test.py

@@ -50,6 +50,9 @@ def test_pop_multiple(self, backup_controller):
         assert restorable2.called["restore"] == 1
         assert restorable3.called["restore"] == 1
 
+        assert backup_controller.rollback_failed == False
+        assert backup_controller.rollback_failures == []
+
     def test_pop_when_empty(self, backup_controller):
         with pytest.raises(IndexError, match="No backups to restore"):
             backup_controller.pop()
@@ -74,6 +77,9 @@ def test_pop_all(self, backup_controller):
         assert restorable2.called["restore"] == 1
         assert restorable3.called["restore"] == 1
 
+        assert backup_controller.rollback_failed == False
+        assert backup_controller.rollback_failures == []
+
     def test_ready_to_push_after_pop_all(self, backup_controller):
         restorable1 = MinimalRestorable()
         restorable2 = MinimalRestorable()
@@ -86,11 +92,15 @@ def test_ready_to_push_after_pop_all(self, backup_controller):
         assert popped_restorables[0] == restorable1
         assert len(backup_controller._restorables) == 1
         assert backup_controller._restorables[0] is restorable2
+        assert backup_controller.rollback_failed == False
 
     def test_pop_all_when_empty(self, backup_controller):
         with pytest.raises(IndexError, match="No backups to restore"):
             backup_controller.pop_all()
 
+        assert backup_controller.rollback_failed == False
+        assert backup_controller.rollback_failures == []
+
     def test_pop_all_error_in_restore(self, backup_controller, caplog):
         restorable1 = MinimalRestorable()
         restorable2 = ErrorOnRestoreRestorable(exception=ValueError("Restorable2 failed"))
@@ -105,3 +115,7 @@ def test_pop_all_error_in_restore(self, backup_controller, caplog):
         assert len(popped_restorables) == 3
         assert popped_restorables == [restorable3, restorable2, restorable1]
         assert caplog.records[-1].message == "Error while rolling back a ErrorOnRestoreRestorable: Restorable2 failed"
+        assert backup_controller.rollback_failed == True
+        assert backup_controller.rollback_failures == [
+            "Error while rolling back a ErrorOnRestoreRestorable: Restorable2 failed"
+        ]

convert2rhel/unit_tests/backup/certs_test.py

@@ -245,10 +245,6 @@ def test_restore_cert_previously_installed(self, caplog, monkeypatch, system_cer
                 OSError(2, "No such file or directory"),
                 "No such file or directory",
             ),
-            (
-                OSError(13, "[Errno 13] Permission denied: '/tmpdir/certfile'"),
-                "OSError(13): Permission denied: '/tmpdir/certfile'",
-            ),
         ),
     )
     def test_restore_cert_error_conditions(

convert2rhel/unit_tests/backup/files_test.py

@@ -8,7 +8,7 @@
 import six
 
 from convert2rhel import exceptions
-from convert2rhel.backup import files
+from convert2rhel.backup import RestorableChange, files
 from convert2rhel.backup.files import MissingFile, RestorableFile
 
 
@@ -224,24 +224,6 @@ def test_restorable_file_restore(self, tmpdir, monkeypatch, caplog, filename, me
             assert shutil.copy2.call_count == 1
             assert os.remove.call_count == 1
 
-    def test_restorable_file_backup_oserror(self, tmpdir, caplog, monkeypatch):
-        backup_dir = tmpdir.mkdir("backup")
-        backup_file = backup_dir.join("filename")
-        backup_file.write("content")
-
-        monkeypatch.setattr(
-            shutil,
-            "copy2",
-            mock.Mock(side_effect=[None, OSError(2, "No such file or directory")]),
-        )
-        monkeypatch.setattr(os.path, "isfile", lambda file: True)
-        monkeypatch.setattr(files, "BACKUP_DIR", str(backup_dir))
-        file_backup = RestorableFile(str(backup_file))
-        file_backup.enable()
-        file_backup.restore()
-
-        assert "Error(2): No such file or directory" in caplog.records[-1].message
-
     @pytest.mark.parametrize(
         ("file", "filepath", "message"),
         (
@@ -340,19 +322,6 @@ def test_created_file_restore(self, tmpdir, exists, enabled, message, caplog):
         else:
             assert not caplog.records
 
-    def test_created_file_restore_oserror(self, monkeypatch, tmpdir, caplog):
-        path = tmpdir.join("filename")
-        path.write("content")
-
-        remove = mock.Mock(side_effect=OSError(2, "No such file or directory"))
-        monkeypatch.setattr(os, "remove", remove)
-
-        created_file = MissingFile(str(path))
-        created_file.enabled = True
-        created_file.restore()
-
-        assert "Error(2): No such file or directory" in caplog.records[-1].message
-
     @pytest.mark.parametrize(
         ("exists", "created", "message_push", "message_pop"),
         (