diff --git a/convert2rhel/logger.py b/convert2rhel/logger.py index 0b860497af..e6ee338633 100644 --- a/convert2rhel/logger.py +++ b/convert2rhel/logger.py @@ -35,11 +35,15 @@ import shutil import sys +from logging.handlers import BufferingHandler from time import gmtime, strftime LOG_DIR = "/var/log/convert2rhel" +# get root logger +logger = logging.getLogger("convert2rhel") + class LogLevelCriticalNoExit: level = 50 @@ -58,11 +62,65 @@ class LogLevelFile: label = "DEBUG" -def setup_logger_handler(log_name, log_dir): +class LogfileBufferHandler(BufferingHandler): + """ + FileHandler we use in Convert2RHEL requries root due to the location and the + tool itself checking for root user explicitly. Since we cannot obviously + use the logger if we aren't root in that case we simply add the FileHandler + after determining we're root. + + Caveat of that approach is that any logging prior to the initialization of + the FileHandler would be lost, to help with this we have this custom handler + which will keep a buffer of the logs and flush it to the FileHandler + """ + + name = "logfile_buffer_handler" + + def __init__(self, capacity, handler_name="file_handler"): + """ + Initialize the handler with the buffer size. + + :param int capacity: Buffer size for the handler + :param str handler_name: Handler to flush buffer to, defaults to "file_handler" + """ + super(LogfileBufferHandler, self).__init__(capacity) + # the FileLogger handler that we are logging to + self._handler_name = handler_name + + @property + def target(self): + """The computed Filehandler target that we are supposed to send to. + + This is mostly copied over from logging's MemoryHandler but instead of setting + the target manually we find it automatically given the name of the handler + + :return logging.Handler: Either the found FileHandler setup or temporary NullHandler + """ + for handler in logger.handlers: + if handler.name == self._handler_name: + return handler + return logging.NullHandler() + + def flush(self): + for record in self.buffer: + self.target.handle(record) + + def shouldFlush(self, record): + """ + We should never flush automatically, so we set this to always return false, that way we need to flush manually each time. Which is exactly what we want when it comes to keeping a buffer before we confirm we are + a root user. + + :param logging.LogRecord record: The record to log + :return bool: Always returns false + """ + if super(LogfileBufferHandler, self).shouldFlush(record): + self.buffer = self.buffer[1:] + return False + + +def setup_logger_handler(): """Setup custom logging levels, handlers, and so on. Call this method from your application's main start point. - log_name = the name for the log file - log_dir = path to the dir where log file will be presented """ # set custom labels logging.addLevelName(LogLevelTask.level, LogLevelTask.label) @@ -76,8 +134,6 @@ def setup_logger_handler(log_name, log_dir): # enable raising exceptions logging.raiseExceptions = True - # get root logger - logger = logging.getLogger("convert2rhel") # propagate logger.propagate = True # set default logging level @@ -91,22 +147,46 @@ def setup_logger_handler(log_name, log_dir): stdout_handler.setLevel(logging.DEBUG) logger.addHandler(stdout_handler) - # create file handler + # can flush logs to the file that were logged before initializing the file handler + logger.addHandler(LogfileBufferHandler(capacity=100)) + + +def add_file_handler(log_name, log_dir): + """Create a file handler for the logger instance + + :param str log_name: Name of the log file + :param str log_dir: Full path location + """ if not os.path.isdir(log_dir): os.makedirs(log_dir) # pragma: no cover - handler = logging.FileHandler(os.path.join(log_dir, log_name), "a") + filehandler = logging.FileHandler(os.path.join(log_dir, log_name), "a") + filehandler.name = "file_handler" formatter = CustomFormatter("%(message)s") + + # With a file we don't really need colors + # This might change in the future depending on customer requests + # or if we do something with UI work in the future that would be more + # helpful with colors formatter.disable_colors(True) - handler.setFormatter(formatter) - handler.setLevel(LogLevelFile.level) - logger.addHandler(handler) + filehandler.setFormatter(formatter) + filehandler.setLevel(LogLevelFile.level) + logger.addHandler(filehandler) + + # We now have a FileHandler added, but we still need the logs from before + # this point. Luckily we have the memory buffer that we can flush logs from + for handler in logger.handlers: + if handler.name == "logfile_buffer_handler": + handler.flush() + # after we've flushed to the file we don't need the handler anymore + logger.removeHandler(handler) + break def should_disable_color_output(): """ Return whether NO_COLOR exists in environment parameter and is true. - See http://no-color.org/ + See https://no-color.org/ """ if "NO_COLOR" in os.environ: NO_COLOR = os.environ["NO_COLOR"] diff --git a/convert2rhel/main.py b/convert2rhel/main.py index b4358ce9cf..7c07998c5f 100644 --- a/convert2rhel/main.py +++ b/convert2rhel/main.py @@ -44,24 +44,38 @@ class ConversionPhase: POST_PONR_CHANGES = 4 -def initialize_logger(log_name, log_dir): +def initialize_logger(): """ Entrypoint function that aggregates other calls for initialization logic - and setup for logger handlers. + 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 + root, like FileHandlers. + + This function should be called after + :func:`~convert2rhel.main.initialize_logger`. .. warning:: Setting log_dir underneath a world-writable directory (including letting it be user settable) is insecure. We will need to write some checks for all calls to `os.makedirs()` if we allow changing log_dir. - """ + :param str log_name: Name of the logfile to archive and log to + :param str log_dir: Directory where logfiles are stored + """ try: logger_module.archive_old_logger_files(log_name, log_dir) except (IOError, OSError) as e: - print("Warning: Unable to archive previous log: %s" % e) + loggerinst.warning("Unable to archive previous log: %s" % e) - logger_module.setup_logger_handler(log_name, log_dir) + logger_module.add_file_handler(log_name, log_dir) def main(): @@ -73,23 +87,21 @@ def main(): the application lock, to do the conversion process. """ - # Make sure we're being run by root - utils.require_root() - # initialize logging - initialize_logger("convert2rhel.log", logger_module.LOG_DIR) + initialize_logger() # handle command line arguments toolopts.CLI() + # Make sure we're being run by root + utils.require_root() + try: with applock.ApplicationLock("convert2rhel"): return main_locked() except applock.ApplicationLockedError: - # We have not rotated the log files at this point because main.initialize_logger() - # has not yet been called. So we use sys.stderr.write() instead of loggerinst.error() - sys.stderr.write("Another copy of convert2rhel is running.\n") - sys.stderr.write("\nNo changes were made to the system.\n") + loggerinst.warning("Another copy of convert2rhel is running.\n") + loggerinst.warning("\nNo changes were made to the system.\n") return 1 @@ -98,6 +110,11 @@ def main_locked(): pre_conversion_results = None process_phase = ConversionPhase.POST_CLI + + # since we now have root, we can add the FileLogging + # and also archive previous logs + initialize_file_logging("convert2rhel.log", logger_module.LOG_DIR) + try: perform_boilerplate() diff --git a/convert2rhel/unit_tests/__init__.py b/convert2rhel/unit_tests/__init__.py index 6fb303db1f..f908224bca 100644 --- a/convert2rhel/unit_tests/__init__.py +++ b/convert2rhel/unit_tests/__init__.py @@ -373,6 +373,10 @@ class InitializeLoggerMocked(MockFunctionObject): spec = main.initialize_logger +class InitializeFileLoggingMocked(MockFunctionObject): + spec = main.initialize_file_logging + + class MainLockedMocked(MockFunctionObject): spec = main.main_locked diff --git a/convert2rhel/unit_tests/conftest.py b/convert2rhel/unit_tests/conftest.py index 09ea41a8f2..cfd89189ee 100644 --- a/convert2rhel/unit_tests/conftest.py +++ b/convert2rhel/unit_tests/conftest.py @@ -8,7 +8,7 @@ import six from convert2rhel import backup, cert, pkgmanager, redhatrelease, systeminfo, toolopts, utils -from convert2rhel.logger import setup_logger_handler +from convert2rhel.logger import add_file_handler, setup_logger_handler from convert2rhel.systeminfo import system_info from convert2rhel.toolopts import tool_opts from convert2rhel.unit_tests import MinimalRestorable @@ -107,8 +107,12 @@ def pkg_root(): @pytest.fixture(autouse=True) -def setup_logger(tmpdir): - setup_logger_handler(log_name="convert2rhel", log_dir=str(tmpdir)) +def setup_logger(tmpdir, request): + # This makes it so we can skip this using @pytest.mark.noautofixtures + if "noautofixtures" in request.keywords: + return + setup_logger_handler() + add_file_handler(log_name="convert2rhel", log_dir=str(tmpdir)) @pytest.fixture diff --git a/convert2rhel/unit_tests/logger_test.py b/convert2rhel/unit_tests/logger_test.py index 9437d9cd85..a12fd457df 100644 --- a/convert2rhel/unit_tests/logger_test.py +++ b/convert2rhel/unit_tests/logger_test.py @@ -19,20 +19,22 @@ import logging import os +import sys import pytest from convert2rhel import logger as logger_module -def test_logger_handlers(monkeypatch, tmpdir, caplog, read_std, is_py2, global_tool_opts, clear_loggers): +def test_logger_handlers(monkeypatch, tmpdir, read_std, global_tool_opts): """Test if the logger handlers emits the events to the file and stdout.""" monkeypatch.setattr("convert2rhel.toolopts.tool_opts", global_tool_opts) # initializing the logger first log_fname = "convert2rhel.log" global_tool_opts.debug = True # debug entries > stdout if True - logger_module.setup_logger_handler(log_name=log_fname, log_dir=str(tmpdir)) + logger_module.setup_logger_handler() + logger_module.add_file_handler(log_name=log_fname, log_dir=str(tmpdir)) logger = logging.getLogger(__name__) # emitting some log entries @@ -50,10 +52,9 @@ def test_logger_handlers(monkeypatch, tmpdir, caplog, read_std, is_py2, global_t assert "Test debug: other data" in stdouterr_out -def test_tools_opts_debug(monkeypatch, tmpdir, read_std, is_py2, global_tool_opts, clear_loggers): +def test_tools_opts_debug(monkeypatch, read_std, is_py2, global_tool_opts): monkeypatch.setattr("convert2rhel.toolopts.tool_opts", global_tool_opts) - log_fname = "convert2rhel.log" - logger_module.setup_logger_handler(log_name=log_fname, log_dir=str(tmpdir)) + logger_module.setup_logger_handler() logger = logging.getLogger(__name__) global_tool_opts.debug = True logger.debug("debug entry 1: %s", "data") @@ -85,10 +86,9 @@ class TestCustomLogger: ("critical_no_exit", "CRITICAL"), ), ) - def test_logger_custom_logger(self, log_method_name, level_name, caplog, monkeypatch, tmpdir, clear_loggers): + def test_logger_custom_logger(self, log_method_name, level_name, caplog): """Test CustomLogger.""" - log_fname = "convert2rhel.log" - logger_module.setup_logger_handler(log_name=log_fname, log_dir=str(tmpdir)) + logger_module.setup_logger_handler() logger = logging.getLogger(__name__) log_method = getattr(logger, log_method_name) @@ -98,10 +98,9 @@ def test_logger_custom_logger(self, log_method_name, level_name, caplog, monkeyp assert "Some task: data" == caplog.records[-1].message assert caplog.records[-1].levelname == level_name - def test_logger_critical(self, caplog, tmpdir, clear_loggers): + def test_logger_critical(self, caplog): """Test CustomLogger.""" - log_fname = "convert2rhel.log" - logger_module.setup_logger_handler(log_name=log_fname, log_dir=str(tmpdir)) + logger_module.setup_logger_handler() logger = logging.getLogger(__name__) with pytest.raises(SystemExit): @@ -111,21 +110,18 @@ def test_logger_critical(self, caplog, tmpdir, clear_loggers): assert "Critical error: data\n" in caplog.text @pytest.mark.parametrize( - ("log_method_name", "level_name"), - ( - ("task", "TASK"), - ("file", "DEBUG"), - ("debug", "DEBUG"), - ("warning", "WARNING"), - ("critical_no_exit", "CRITICAL"), - ), + "log_method_name", + [ + "task", + "file", + "debug", + "warning", + "critical_no_exit", + ], ) - def test_logger_custom_logger_insufficient_level( - self, log_method_name, level_name, caplog, monkeypatch, tmpdir, clear_loggers - ): + def test_logger_custom_logger_insufficient_level(self, log_method_name, caplog): """Test CustomLogger.""" - log_fname = "convert2rhel.log" - logger_module.setup_logger_handler(log_name=log_fname, log_dir=str(tmpdir)) + logger_module.setup_logger_handler() logger = logging.getLogger(__name__) logger.setLevel(logging.CRITICAL + 40) log_method = getattr(logger, log_method_name) @@ -135,10 +131,9 @@ def test_logger_custom_logger_insufficient_level( assert "Some task: data" not in caplog.text assert not caplog.records - def test_logger_critical_insufficient_level(self, caplog, tmpdir, clear_loggers): + def test_logger_critical_insufficient_level(self, caplog): """Test CustomLogger.""" - log_fname = "convert2rhel.log" - logger_module.setup_logger_handler(log_name=log_fname, log_dir=str(tmpdir)) + logger_module.setup_logger_handler() logger = logging.getLogger(__name__) logger.setLevel(logging.CRITICAL + 40) @@ -155,7 +150,7 @@ def test_logger_critical_insufficient_level(self, caplog, tmpdir, clear_loggers) ("convert2rhel.log", False), ), ) -def test_archive_old_logger_files(log_name, path_exists, tmpdir, caplog): +def test_archive_old_logger_files(log_name, path_exists, tmpdir): tmpdir = str(tmpdir) archive_dir = os.path.join(tmpdir, "archive") log_file = os.path.join(tmpdir, log_name) @@ -184,3 +179,27 @@ def test_archive_old_logger_files(log_name, path_exists, tmpdir, caplog): def test_should_disable_color_output(monkeypatch, no_color_value, should_disable_color): monkeypatch.setattr(os, "environ", {"NO_COLOR": no_color_value}) assert logger_module.should_disable_color_output() == should_disable_color + + +@pytest.mark.noautofixtures +def test_logfile_buffer_handler(read_std): + logbuffer_handler = logger_module.LogfileBufferHandler(2, "custom_name") + logger = logging.getLogger("convert2rhel") + logger.addHandler(logbuffer_handler) + + logger.warning("message 1") + logger.warning("message 2") + + # flushing without other handlers should work, it will just go to NullHandlers + logbuffer_handler.flush() + + stdout_handler = logging.StreamHandler(sys.stdout) + stdout_handler.name = "custom_name" + logger.addHandler(stdout_handler) + + # flush to the streamhandler we just created + logbuffer_handler.flush() + + stdouterr_out, _ = read_std() + assert "message 1" not in stdouterr_out + assert "message 2" in stdouterr_out diff --git a/convert2rhel/unit_tests/main_test.py b/convert2rhel/unit_tests/main_test.py index 5feb27d332..1e86e6f649 100644 --- a/convert2rhel/unit_tests/main_test.py +++ b/convert2rhel/unit_tests/main_test.py @@ -40,6 +40,7 @@ CLIMocked, CollectEarlyDataMocked, FinishCollectionMocked, + InitializeFileLoggingMocked, InitializeLoggerMocked, MainLockedMocked, PrintDataCollectionMocked, @@ -122,9 +123,22 @@ def test_backup_control_unknown_exception(self, monkeypatch): main.rollback_changes() -@pytest.mark.parametrize(("exception_type", "exception"), ((IOError, True), (OSError, True), (None, False))) -def test_initialize_logger(exception_type, exception, monkeypatch, capsys): +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() archive_old_logger_files_mock = mock.Mock() if exception: @@ -132,8 +146,8 @@ def test_initialize_logger(exception_type, exception, monkeypatch, capsys): monkeypatch.setattr( logger_module, - "setup_logger_handler", - value=setup_logger_handler_mock, + "add_file_handler", + value=add_file_handler_mock, ) monkeypatch.setattr( logger_module, @@ -141,14 +155,14 @@ def test_initialize_logger(exception_type, exception, monkeypatch, capsys): value=archive_old_logger_files_mock, ) + main.initialize_file_logging("convert2rhel.log", "/tmp") + if exception: - main.initialize_logger("convert2rhel.log", "/tmp") - out, _ = capsys.readouterr() - assert "Warning: Unable to archive previous log:" in out - else: - main.initialize_logger("convert2rhel.log", "/tmp") - setup_logger_handler_mock.assert_called_once() - archive_old_logger_files_mock.assert_called_once() + assert caplog.records[-1].levelname == "WARNING" + assert "Unable to archive previous log:" in caplog.records[-1].message + + add_file_handler_mock.assert_called_once() + archive_old_logger_files_mock.assert_called_once() class TestShowEula: @@ -205,6 +219,7 @@ def test_help_exit(monkeypatch, 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()) with pytest.raises(SystemExit): @@ -216,6 +231,7 @@ 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() print_data_collection_mock = mock.Mock() @@ -242,6 +258,7 @@ 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) monkeypatch.setattr(breadcrumbs, "print_data_collection", print_data_collection_mock) @@ -294,6 +311,7 @@ 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) @@ -303,6 +321,7 @@ 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) @@ -319,6 +338,7 @@ 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()) monkeypatch.setattr(breadcrumbs, "print_data_collection", PrintDataCollectionMocked()) @@ -360,6 +380,7 @@ 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() print_data_collection_mock = mock.Mock() @@ -379,6 +400,7 @@ 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) monkeypatch.setattr(breadcrumbs, "print_data_collection", print_data_collection_mock) @@ -394,8 +416,9 @@ 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 require_root_mock.call_count == 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 assert show_eula_mock.call_count == 1 assert print_data_collection_mock.call_count == 1 @@ -417,6 +440,7 @@ 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() print_data_collection_mock = mock.Mock() @@ -438,6 +462,7 @@ 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) monkeypatch.setattr(breadcrumbs, "print_data_collection", print_data_collection_mock) @@ -455,8 +480,9 @@ 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 require_root_mock.call_count == 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 assert show_eula_mock.call_count == 1 assert print_data_collection_mock.call_count == 1 @@ -477,6 +503,7 @@ 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() print_data_collection_mock = mock.Mock() @@ -497,6 +524,7 @@ 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) monkeypatch.setattr(breadcrumbs, "print_data_collection", print_data_collection_mock) @@ -517,6 +545,7 @@ 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 assert print_data_collection_mock.call_count == 1 @@ -534,6 +563,7 @@ 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() print_data_collection_mock = mock.Mock() @@ -557,6 +587,7 @@ 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) monkeypatch.setattr(breadcrumbs, "print_data_collection", print_data_collection_mock) @@ -578,6 +609,7 @@ 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 assert print_data_collection_mock.call_count == 1 diff --git a/convert2rhel/unit_tests/utils_test.py b/convert2rhel/unit_tests/utils_test.py index 999c1fe1ad..d53015dd76 100644 --- a/convert2rhel/unit_tests/utils_test.py +++ b/convert2rhel/unit_tests/utils_test.py @@ -905,12 +905,12 @@ def test_run_subprocess_env(self, dummy_popen, monkeypatch): assert 0 == rc -def test_require_root_is_not_root(monkeypatch, capsys): +def test_require_root_is_not_root(monkeypatch, caplog): monkeypatch.setattr(os, "geteuid", GetEUIDMocked(1000)) with pytest.raises(SystemExit): utils.require_root() - assert "The tool needs to be run under the root user." in capsys.readouterr().out + assert "The tool needs to be run under the root user." in caplog.text def test_require_root_is_root(monkeypatch): diff --git a/convert2rhel/utils.py b/convert2rhel/utils.py index 63b6d3b51a..4bee774582 100644 --- a/convert2rhel/utils.py +++ b/convert2rhel/utils.py @@ -289,9 +289,7 @@ def get_executable_name(): def require_root(): if os.geteuid() != 0: - print("The tool needs to be run under the root user.") - print("\nNo changes were made to the system.") - sys.exit(1) + loggerinst.critical("The tool needs to be run under the root user.\nNo changes were made to the system.") def get_file_content(filename, as_list=False): 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 index 131614fc0c..a117983947 100644 --- 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 @@ -23,9 +23,7 @@ def test_check_user_privileges(shell): # Check the program exits as it is required to be run by root assert result.returncode != 0 # Check the program exits for the correct reason - assert ( - result.output == "The tool needs to be run under the root user.\n" "\n" "No changes were made to the system.\n" - ) + assert "The tool needs to be run under the root user.\nNo changes were made to the system." in result.output # Delete testuser (if present) assert shell(f"userdel -r '{user}'").returncode == 0