[RHELC-1393] Move logger initialization to initialize module (oamg#1095)

* Move logger initialization to initialize module Initializing the logger module earlier makes the tool work correctly when trying to find the /etc/system-release file, and if it is not there, throw a critical error to the user before anything else happens. If we don't move the module to this point, the first /etc/system-releaes check will silently fail and only the next call will throw the exception, because at this point, we already have the logger initialized. To prevent further executions in the system, it is more reliable to initialize the logger module earlier. Signed-off-by: Rodolfo Olivieri <[email protected]> * Revert initialize_file_logger function Signed-off-by: Rodolfo Olivieri <[email protected]> --------- Signed-off-by: Rodolfo Olivieri <[email protected]>
Andrew-ang9 · Feb 20, 2024 · 790db4b · 790db4b
1 parent bbae477
commit 790db4b
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 53 deletions.
diff --git a/convert2rhel/backup/files.py b/convert2rhel/backup/files.py
@@ -33,7 +33,6 @@
 class RestorableFile(RestorableChange):
     def __init__(self, filepath):
         super(RestorableFile, self).__init__()
-
         # The filepath we want to back up needs to start with at least a `/`,
         # otherwise, let's error out and warn the developer/user that the
         # filepath is not what we expect. This is mostly intended to be an

diff --git a/convert2rhel/initialize.py b/convert2rhel/initialize.py
@@ -21,6 +21,10 @@
 import os
 
 from convert2rhel import i18n
+from convert2rhel import logger as logger_module
+
+
+loggerinst = logging.getLogger(__name__)
 
 
 def disable_root_logger():
@@ -36,6 +40,15 @@ def disable_root_logger():
     logging.getLogger().addHandler(logging.NullHandler())
 
 
+def initialize_logger():
+    """
+    Entrypoint function that aggregates other calls for initialization logic
+    and setup for logger handlers that do not require root.
+    """
+
+    return logger_module.setup_logger_handler()
+
+
 def set_locale():
     """
     Set the C locale, also known as the POSIX locale, for the main process as well as the child processes.
@@ -77,6 +90,9 @@ def run():
     # Initialize logging to stop duplicate messages.
     disable_root_logger()
 
+    # initialize logging
+    initialize_logger()
+
     from convert2rhel import main
 
     return main.main()
diff --git a/convert2rhel/main.py b/convert2rhel/main.py
@@ -19,7 +19,6 @@
 
 import logging
 import os
-import sys
 
 from convert2rhel import actions, applock, backup, breadcrumbs, checks, exceptions, grub
 from convert2rhel import logger as logger_module
@@ -44,15 +43,6 @@ class ConversionPhase:
     POST_PONR_CHANGES = 4
 
 
-def initialize_logger():
-    """
-    Entrypoint function that aggregates other calls for initialization logic
-    and setup for logger handlers that do not require root.
-    """
-
-    return logger_module.setup_logger_handler()
-
-
 def initialize_file_logging(log_name, log_dir):
     """
     Archive existing file logs and setup all logging handlers that require
@@ -87,9 +77,6 @@ def main():
     the application lock, to do the conversion process.
     """
 
-    # initialize logging
-    initialize_logger()
-
     # handle command line arguments
     toolopts.CLI()
 

diff --git a/convert2rhel/unit_tests/__init__.py b/convert2rhel/unit_tests/__init__.py
@@ -30,6 +30,7 @@
     breadcrumbs,
     exceptions,
     grub,
+    initialize,
     main,
     pkghandler,
     subscription,
@@ -355,7 +356,7 @@ class PrintDataCollectionMocked(MockFunctionObject):
 
 
 class InitializeLoggerMocked(MockFunctionObject):
-    spec = main.initialize_logger
+    spec = initialize.initialize_logger
 
 
 class InitializeFileLoggingMocked(MockFunctionObject):

diff --git a/convert2rhel/unit_tests/initialize_test.py b/convert2rhel/unit_tests/initialize_test.py
@@ -17,11 +17,16 @@
 
 __metaclass__ = type
 
-import os
-
 import pytest
+import six
+
+from convert2rhel import applock, initialize
+from convert2rhel import logger as logger_module
+from convert2rhel import main
 
-from convert2rhel import applock, initialize, main
+
+six.add_move(six.MovedModule("mock", "mock", "unittest.mock"))
+from six.moves import mock
 
 
 @pytest.mark.parametrize(
@@ -32,6 +37,20 @@
     ),
 )
 def test_run(monkeypatch, exit_code, tmp_path):
+    monkeypatch.setattr(logger_module, "LOG_DIR", str(tmp_path))
     monkeypatch.setattr(main, "main", value=lambda: exit_code)
     monkeypatch.setattr(applock, "_DEFAULT_LOCK_DIR", str(tmp_path))
     assert initialize.run() == exit_code
+
+
+def test_initialize_logger(monkeypatch):
+    setup_logger_handler_mock = mock.Mock()
+
+    monkeypatch.setattr(
+        logger_module,
+        "setup_logger_handler",
+        value=setup_logger_handler_mock,
+    )
+
+    initialize.initialize_logger()
+    setup_logger_handler_mock.assert_called_once()
diff --git a/convert2rhel/unit_tests/main_test.py b/convert2rhel/unit_tests/main_test.py
@@ -41,7 +41,6 @@
     CollectEarlyDataMocked,
     FinishCollectionMocked,
     InitializeFileLoggingMocked,
-    InitializeLoggerMocked,
     MainLockedMocked,
     PrintDataCollectionMocked,
     PrintSystemInformationMocked,
@@ -121,19 +120,6 @@ def test_backup_control_unknown_exception(self, monkeypatch, global_backup_contr
             main.rollback_changes()
 
 
-def test_initialize_logger(monkeypatch):
-    setup_logger_handler_mock = mock.Mock()
-
-    monkeypatch.setattr(
-        logger_module,
-        "setup_logger_handler",
-        value=setup_logger_handler_mock,
-    )
-
-    main.initialize_logger()
-    setup_logger_handler_mock.assert_called_once()
-
-
 @pytest.mark.parametrize(("exception_type", "exception"), ((IOError, True), (OSError, True), (None, False)))
 def test_initialize_file_logging(exception_type, exception, monkeypatch, caplog):
     add_file_handler_mock = mock.Mock()
@@ -216,7 +202,6 @@ def test_help_exit(monkeypatch, tmp_path):
     monkeypatch.setattr(applock, "_DEFAULT_LOCK_DIR", str(tmp_path))
     monkeypatch.setattr(sys, "argv", ["convert2rhel", "--help"])
     monkeypatch.setattr(utils, "require_root", RequireRootMocked())
-    monkeypatch.setattr(main, "initialize_logger", InitializeLoggerMocked())
     monkeypatch.setattr(main, "initialize_file_logging", InitializeFileLoggingMocked())
     monkeypatch.setattr(main, "main_locked", MainLockedMocked())
 
@@ -228,7 +213,6 @@ def test_help_exit(monkeypatch, tmp_path):
 
 def test_main(monkeypatch, tmp_path):
     require_root_mock = mock.Mock()
-    initialize_logger_mock = mock.Mock()
     initialize_file_logging_mock = mock.Mock()
     toolopts_cli_mock = mock.Mock()
     show_eula_mock = mock.Mock()
@@ -255,7 +239,6 @@ def test_main(monkeypatch, tmp_path):
 
     monkeypatch.setattr(applock, "_DEFAULT_LOCK_DIR", str(tmp_path))
     monkeypatch.setattr(utils, "require_root", require_root_mock)
-    monkeypatch.setattr(main, "initialize_logger", initialize_logger_mock)
     monkeypatch.setattr(main, "initialize_file_logging", initialize_file_logging_mock)
     monkeypatch.setattr(toolopts, "CLI", toolopts_cli_mock)
     monkeypatch.setattr(main, "show_eula", show_eula_mock)
@@ -282,7 +265,7 @@ def test_main(monkeypatch, tmp_path):
 
     assert main.main() == 0
     assert require_root_mock.call_count == 1
-    assert initialize_logger_mock.call_count == 1
+    assert initialize_file_logging_mock.call_count == 1
     assert toolopts_cli_mock.call_count == 1
     assert show_eula_mock.call_count == 1
     assert print_data_collection_mock.call_count == 1
@@ -308,7 +291,6 @@ def test_main(monkeypatch, tmp_path):
 class TestRollbackFromMain:
     def test_main_rollback_post_cli_phase(self, monkeypatch, caplog, tmp_path):
         require_root_mock = mock.Mock()
-        initialize_logger_mock = mock.Mock()
         initialize_file_logging_mock = mock.Mock()
         toolopts_cli_mock = mock.Mock()
         show_eula_mock = mock.Mock(side_effect=Exception)
@@ -318,15 +300,14 @@ def test_main_rollback_post_cli_phase(self, monkeypatch, caplog, tmp_path):
 
         monkeypatch.setattr(applock, "_DEFAULT_LOCK_DIR", str(tmp_path))
         monkeypatch.setattr(utils, "require_root", require_root_mock)
-        monkeypatch.setattr(main, "initialize_logger", initialize_logger_mock)
         monkeypatch.setattr(main, "initialize_file_logging", initialize_file_logging_mock)
         monkeypatch.setattr(toolopts, "CLI", toolopts_cli_mock)
         monkeypatch.setattr(main, "show_eula", show_eula_mock)
         monkeypatch.setattr(breadcrumbs, "finish_collection", finish_collection_mock)
 
         assert main.main() == 1
         assert require_root_mock.call_count == 1
-        assert initialize_logger_mock.call_count == 1
+        assert initialize_file_logging_mock.call_count == 1
         assert toolopts_cli_mock.call_count == 1
         assert show_eula_mock.call_count == 1
         assert finish_collection_mock.call_count == 1
@@ -335,7 +316,6 @@ def test_main_rollback_post_cli_phase(self, monkeypatch, caplog, tmp_path):
     def test_main_traceback_in_clear_versionlock(self, caplog, monkeypatch, tmp_path):
         monkeypatch.setattr(applock, "_DEFAULT_LOCK_DIR", str(tmp_path))
         monkeypatch.setattr(utils, "require_root", RequireRootMocked())
-        monkeypatch.setattr(main, "initialize_logger", InitializeLoggerMocked())
         monkeypatch.setattr(main, "initialize_file_logging", InitializeFileLoggingMocked())
         monkeypatch.setattr(toolopts, "CLI", CLIMocked())
         monkeypatch.setattr(main, "show_eula", ShowEulaMocked())
@@ -360,7 +340,6 @@ def test_main_traceback_in_clear_versionlock(self, caplog, monkeypatch, tmp_path
 
         assert main.main() == 1
         assert utils.require_root.call_count == 1
-        assert main.initialize_logger.call_count == 1
         assert toolopts.CLI.call_count == 1
         assert main.show_eula.call_count == 1
         assert breadcrumbs.print_data_collection.call_count == 1
@@ -377,7 +356,6 @@ def test_main_traceback_in_clear_versionlock(self, caplog, monkeypatch, tmp_path
 
     def test_main_traceback_before_action_completion(self, monkeypatch, caplog, tmp_path):
         require_root_mock = mock.Mock()
-        initialize_logger_mock = mock.Mock()
         initialize_file_logging_mock = mock.Mock()
         toolopts_cli_mock = mock.Mock()
         show_eula_mock = mock.Mock()
@@ -397,7 +375,6 @@ def test_main_traceback_before_action_completion(self, monkeypatch, caplog, tmp_
 
         monkeypatch.setattr(applock, "_DEFAULT_LOCK_DIR", str(tmp_path))
         monkeypatch.setattr(utils, "require_root", require_root_mock)
-        monkeypatch.setattr(main, "initialize_logger", initialize_logger_mock)
         monkeypatch.setattr(main, "initialize_file_logging", initialize_file_logging_mock)
         monkeypatch.setattr(toolopts, "CLI", toolopts_cli_mock)
         monkeypatch.setattr(main, "show_eula", show_eula_mock)
@@ -414,7 +391,6 @@ def test_main_traceback_before_action_completion(self, monkeypatch, caplog, tmp_
         monkeypatch.setattr(report, "summary_as_txt", summary_as_txt_mock)
 
         assert main.main() == 1
-        assert initialize_logger_mock.call_count == 1
         assert require_root_mock.call_count == 1
         assert initialize_file_logging_mock.call_count == 1
         assert toolopts_cli_mock.call_count == 1
@@ -437,7 +413,6 @@ def test_main_traceback_before_action_completion(self, monkeypatch, caplog, tmp_
 
     def test_main_rollback_pre_ponr_changes_phase(self, monkeypatch, caplog, tmp_path):
         require_root_mock = mock.Mock()
-        initialize_logger_mock = mock.Mock()
         initialize_file_logging_mock = mock.Mock()
         toolopts_cli_mock = mock.Mock()
         show_eula_mock = mock.Mock()
@@ -459,7 +434,6 @@ def test_main_rollback_pre_ponr_changes_phase(self, monkeypatch, caplog, tmp_pat
 
         monkeypatch.setattr(applock, "_DEFAULT_LOCK_DIR", str(tmp_path))
         monkeypatch.setattr(utils, "require_root", require_root_mock)
-        monkeypatch.setattr(main, "initialize_logger", initialize_logger_mock)
         monkeypatch.setattr(main, "initialize_file_logging", initialize_file_logging_mock)
         monkeypatch.setattr(toolopts, "CLI", toolopts_cli_mock)
         monkeypatch.setattr(main, "show_eula", show_eula_mock)
@@ -478,7 +452,6 @@ def test_main_rollback_pre_ponr_changes_phase(self, monkeypatch, caplog, tmp_pat
         monkeypatch.setattr(report, "summary_as_txt", summary_as_txt_mock)
 
         assert main.main() == 1
-        assert initialize_logger_mock.call_count == 1
         assert require_root_mock.call_count == 1
         assert initialize_file_logging_mock.call_count == 1
         assert toolopts_cli_mock.call_count == 1
@@ -500,7 +473,6 @@ def test_main_rollback_pre_ponr_changes_phase(self, monkeypatch, caplog, tmp_pat
 
     def test_main_rollback_analyze_exit_phase(self, global_tool_opts, monkeypatch, tmp_path):
         require_root_mock = mock.Mock()
-        initialize_logger_mock = mock.Mock()
         initialize_file_logging_mock = mock.Mock()
         toolopts_cli_mock = mock.Mock()
         show_eula_mock = mock.Mock()
@@ -521,7 +493,6 @@ def test_main_rollback_analyze_exit_phase(self, global_tool_opts, monkeypatch, t
 
         monkeypatch.setattr(applock, "_DEFAULT_LOCK_DIR", str(tmp_path))
         monkeypatch.setattr(utils, "require_root", require_root_mock)
-        monkeypatch.setattr(main, "initialize_logger", initialize_logger_mock)
         monkeypatch.setattr(main, "initialize_file_logging", initialize_file_logging_mock)
         monkeypatch.setattr(toolopts, "CLI", toolopts_cli_mock)
         monkeypatch.setattr(main, "show_eula", show_eula_mock)
@@ -542,7 +513,6 @@ def test_main_rollback_analyze_exit_phase(self, global_tool_opts, monkeypatch, t
 
         assert main.main() == 0
         assert require_root_mock.call_count == 1
-        assert initialize_logger_mock.call_count == 1
         assert initialize_file_logging_mock.call_count == 1
         assert toolopts_cli_mock.call_count == 1
         assert show_eula_mock.call_count == 1
@@ -560,7 +530,6 @@ def test_main_rollback_analyze_exit_phase(self, global_tool_opts, monkeypatch, t
 
     def test_main_rollback_post_ponr_changes_phase(self, monkeypatch, caplog, tmp_path):
         require_root_mock = mock.Mock()
-        initialize_logger_mock = mock.Mock()
         initialize_file_logging_mock = mock.Mock()
         toolopts_cli_mock = mock.Mock()
         show_eula_mock = mock.Mock()
@@ -584,7 +553,6 @@ def test_main_rollback_post_ponr_changes_phase(self, monkeypatch, caplog, tmp_pa
 
         monkeypatch.setattr(applock, "_DEFAULT_LOCK_DIR", str(tmp_path))
         monkeypatch.setattr(utils, "require_root", require_root_mock)
-        monkeypatch.setattr(main, "initialize_logger", initialize_logger_mock)
         monkeypatch.setattr(main, "initialize_file_logging", initialize_file_logging_mock)
         monkeypatch.setattr(toolopts, "CLI", toolopts_cli_mock)
         monkeypatch.setattr(main, "show_eula", show_eula_mock)
@@ -606,7 +574,6 @@ def test_main_rollback_post_ponr_changes_phase(self, monkeypatch, caplog, tmp_pa
 
         assert main.main() == 1
         assert require_root_mock.call_count == 1
-        assert initialize_logger_mock.call_count == 1
         assert initialize_file_logging_mock.call_count == 1
         assert toolopts_cli_mock.call_count == 1
         assert show_eula_mock.call_count == 1