[RHELC-1099] Move application lock into main. (#904)

* Move application lock into main.

* Remove an unused import.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Prevent backtrace when non-root user runs --help.

* Update convert2rhel/main.py

Add a comment explaining why we don't log a message.

Co-authored-by: Toshio Kuratomi <a.badger@gmail.com>

* Update convert2rhel/main.py

Clarify comment on main().

Co-authored-by: Toshio Kuratomi <a.badger@gmail.com>

* Update convert2rhel/unit_tests/initialize_test.py

Fix imports.

Co-authored-by: Toshio Kuratomi <a.badger@gmail.com>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Apply changes suggested in review.

* Fix an import problem.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Toshio Kuratomi <a.badger@gmail.com>

Author: 3 people

convert2rhel/initialize.py

@@ -17,9 +17,8 @@
 
 import logging
 import os
-import sys
 
-from convert2rhel import applock, i18n, utils
+from convert2rhel import i18n
 
 
 def disable_root_logger():
@@ -66,26 +65,16 @@ def set_locale():
 def run():
     """Wrapper around the main function.
 
-    This function is intended to initialize all early code and function calls
-    before any other main imports.
+    This function is needed to initialize early code and function calls
+    that have to be done before certain modules (which are imported by
+    main) are imported.
     """
     # prepare environment
     set_locale()
 
     # Initialize logging to stop duplicate messages.
     disable_root_logger()
 
-    # Make sure we're being run by root
-    utils.require_root()
-
     from convert2rhel import main
 
-    retval = 0
-    try:
-        with applock.ApplicationLock("convert2rhel"):
-            retval = main.main()
-    except applock.ApplicationLockedError:
-        retval = 1
-        sys.stderr.write("Another copy of convert2rhel is running.\n")
-        sys.stderr.write("\nNo changes were made to the system.\n")
-    sys.exit(retval)
+    return main.main()

convert2rhel/main.py

@@ -17,8 +17,9 @@
 
 import logging
 import os
+import sys
 
-from convert2rhel import actions, backup, breadcrumbs, checks, grub
+from convert2rhel import actions, applock, backup, breadcrumbs, checks, grub
 from convert2rhel import logger as logger_module
 from convert2rhel import pkghandler, pkgmanager, redhatrelease, repo, subscription, systeminfo, toolopts, utils
 from convert2rhel.actions import level_for_raw_action_data, report
@@ -63,16 +64,39 @@ def initialize_logger(log_name, log_dir):
 
 
 def main():
-    """Perform all steps for the entire conversion process."""
+    """
+    Wrapper around the main entrypoint.
+
+    Performs everything necessary to set up before starting the actual
+    conversion process itself, then calls main_locked(), protected by
+    the application lock, to do the conversion process.
+    """
 
     process_phase = ConversionPhase.INIT
 
+    # Make sure we're being run by root
+    utils.require_root()
+
     # initialize logging
     initialize_logger("convert2rhel.log", logger_module.LOG_DIR)
 
     # handle command line arguments
     toolopts.CLI()
 
+    try:
+        with applock.ApplicationLock("convert2rhel"):
+            return main_locked(process_phase)
+    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")
+        return 1
+
+
+def main_locked(process_phase):
+    """Perform all steps for the entire conversion process."""
+
     pre_conversion_results = None
     try:
         process_phase = ConversionPhase.POST_CLI

convert2rhel/unit_tests/initialize_test.py

@@ -18,13 +18,8 @@
 import os
 
 import pytest
-import six
 
-from convert2rhel import applock, initialize, main, utils
-
-
-six.add_move(six.MovedModule("mock", "mock", "unittest.mock"))
-from six.moves import mock
+from convert2rhel import applock, initialize, main
 
 
 @pytest.mark.parametrize(
@@ -35,25 +30,6 @@
     ),
 )
 def test_run(monkeypatch, exit_code, tmp_path):
-    require_root_mock = mock.Mock()
-    monkeypatch.setattr(utils, "require_root", require_root_mock)
     monkeypatch.setattr(main, "main", value=lambda: exit_code)
     monkeypatch.setattr(applock, "_DEFAULT_LOCK_DIR", str(tmp_path))
-    with pytest.raises(SystemExit):
-        initialize.run()
-    assert require_root_mock.call_count == 1
-
-
-def test_locked(monkeypatch, tmp_path, capsys):
-    require_root_mock = mock.Mock()
-    monkeypatch.setattr(utils, "require_root", require_root_mock)
-    monkeypatch.setattr(applock, "_DEFAULT_LOCK_DIR", str(tmp_path))
-    pidfile = os.path.join(str(tmp_path), "convert2rhel.pid")
-    with open(pidfile, "w") as f:
-        f.write(str(os.getpid()) + "\n")
-    with pytest.raises(SystemExit):
-        initialize.run()
-    captured = capsys.readouterr()
-    assert "Another copy of convert2rhel" in captured.err
-    os.unlink(pidfile)
-    assert require_root_mock.call_count == 1
+    assert initialize.run() == exit_code

convert2rhel/unit_tests/main_test.py

@@ -27,7 +27,7 @@
 six.add_move(six.MovedModule("mock", "mock", "unittest.mock"))
 from six.moves import mock
 
-from convert2rhel import actions, backup, cert, checks, grub
+from convert2rhel import actions, applock, backup, cert, checks, grub
 from convert2rhel import logger as logger_module
 from convert2rhel import main, pkghandler, pkgmanager, redhatrelease, repo, subscription, toolopts, unit_tests, utils
 from convert2rhel.actions import report
@@ -201,7 +201,8 @@ def test_post_ponr_conversion(monkeypatch):
     assert lock_releasever_in_rhel_repositories_mock.call_count == 1
 
 
-def test_main(monkeypatch):
+def test_main(monkeypatch, tmp_path):
+    require_root_mock = mock.Mock()
     initialize_logger_mock = mock.Mock()
     toolopts_cli_mock = mock.Mock()
     show_eula_mock = mock.Mock()
@@ -225,6 +226,8 @@ def test_main(monkeypatch):
     update_rhsm_custom_facts_mock = mock.Mock()
     summary_as_json_mock = mock.Mock()
 
+    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(toolopts, "CLI", toolopts_cli_mock)
     monkeypatch.setattr(main, "show_eula", show_eula_mock)
@@ -249,6 +252,7 @@ def test_main(monkeypatch):
     monkeypatch.setattr(report, "summary_as_json", summary_as_json_mock)
 
     assert main.main() == 0
+    assert require_root_mock.call_count == 1
     assert initialize_logger_mock.call_count == 1
     assert toolopts_cli_mock.call_count == 1
     assert show_eula_mock.call_count == 1
@@ -271,28 +275,33 @@ def test_main(monkeypatch):
     assert summary_as_json_mock.call_count == 1
 
 
-def test_main_rollback_post_cli_phase(monkeypatch, caplog):
+def test_main_rollback_post_cli_phase(monkeypatch, caplog, tmp_path):
+    require_root_mock = mock.Mock()
     initialize_logger_mock = mock.Mock()
     toolopts_cli_mock = mock.Mock()
     show_eula_mock = mock.Mock(side_effect=Exception)
 
     # Mock the rollback calls
     finish_collection_mock = mock.Mock()
 
+    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(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 toolopts_cli_mock.call_count == 1
     assert show_eula_mock.call_count == 1
     assert finish_collection_mock.call_count == 1
-    assert "No changes were made to the system." in caplog.records[-1].message
+    assert "No changes were made to the system." in caplog.records[-2].message
 
 
-def test_main_traceback_before_action_completion(monkeypatch, caplog):
+def test_main_traceback_before_action_completion(monkeypatch, caplog, tmp_path):
+    require_root_mock = mock.Mock()
     initialize_logger_mock = mock.Mock()
     toolopts_cli_mock = mock.Mock()
     show_eula_mock = mock.Mock()
@@ -309,6 +318,8 @@ def test_main_traceback_before_action_completion(monkeypatch, caplog):
     rollback_changes_mock = mock.Mock()
     summary_as_json_mock = mock.Mock()
 
+    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(toolopts, "CLI", toolopts_cli_mock)
     monkeypatch.setattr(main, "show_eula", show_eula_mock)
@@ -324,6 +335,7 @@ def test_main_traceback_before_action_completion(monkeypatch, caplog):
     monkeypatch.setattr(report, "summary_as_json", summary_as_json_mock)
 
     assert main.main() == 1
+    assert require_root_mock.call_count == 1
     assert initialize_logger_mock.call_count == 1
     assert toolopts_cli_mock.call_count == 1
     assert show_eula_mock.call_count == 1
@@ -338,13 +350,14 @@ def test_main_traceback_before_action_completion(monkeypatch, caplog):
     assert summary_as_json_mock.call_count == 0
     print(caplog.records)
     assert (
-        caplog.records[-1].message.strip()
+        caplog.records[-2].message.strip()
         == "Conversion interrupted before analysis of system completed. Report not generated."
     )
-    assert "Action Framework Crashed" in caplog.records[-2].message
+    assert "Action Framework Crashed" in caplog.records[-3].message
 
 
-def test_main_rollback_pre_ponr_changes_phase(monkeypatch, caplog):
+def test_main_rollback_pre_ponr_changes_phase(monkeypatch, caplog, tmp_path):
+    require_root_mock = mock.Mock()
     initialize_logger_mock = mock.Mock()
     toolopts_cli_mock = mock.Mock()
     show_eula_mock = mock.Mock()
@@ -363,6 +376,8 @@ def test_main_rollback_pre_ponr_changes_phase(monkeypatch, caplog):
     rollback_changes_mock = mock.Mock()
     summary_as_json_mock = mock.Mock()
 
+    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(toolopts, "CLI", toolopts_cli_mock)
     monkeypatch.setattr(main, "show_eula", show_eula_mock)
@@ -380,6 +395,7 @@ def test_main_rollback_pre_ponr_changes_phase(monkeypatch, caplog):
     monkeypatch.setattr(report, "summary_as_json", summary_as_json_mock)
 
     assert main.main() == 1
+    assert require_root_mock.call_count == 1
     assert initialize_logger_mock.call_count == 1
     assert toolopts_cli_mock.call_count == 1
     assert show_eula_mock.call_count == 1
@@ -394,11 +410,12 @@ def test_main_rollback_pre_ponr_changes_phase(monkeypatch, caplog):
     assert finish_collection_mock.call_count == 1
     assert rollback_changes_mock.call_count == 1
     assert summary_as_json_mock.call_count == 1
-    assert caplog.records[-2].message == "Conversion failed."
-    assert caplog.records[-2].levelname == "CRITICAL"
+    assert caplog.records[-3].message == "Conversion failed."
+    assert caplog.records[-3].levelname == "CRITICAL"
 
 
-def test_main_rollback_analyze_exit_phase(global_tool_opts, monkeypatch):
+def test_main_rollback_analyze_exit_phase(global_tool_opts, monkeypatch, tmp_path):
+    require_root_mock = mock.Mock()
     initialize_logger_mock = mock.Mock()
     toolopts_cli_mock = mock.Mock()
     show_eula_mock = mock.Mock()
@@ -416,6 +433,8 @@ def test_main_rollback_analyze_exit_phase(global_tool_opts, monkeypatch):
     finish_collection_mock = mock.Mock()
     rollback_changes_mock = mock.Mock()
 
+    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(toolopts, "CLI", toolopts_cli_mock)
     monkeypatch.setattr(main, "show_eula", show_eula_mock)
@@ -434,6 +453,7 @@ def test_main_rollback_analyze_exit_phase(global_tool_opts, monkeypatch):
     global_tool_opts.activity = "analysis"
 
     assert main.main() == 0
+    assert require_root_mock.call_count == 1
     assert initialize_logger_mock.call_count == 1
     assert toolopts_cli_mock.call_count == 1
     assert show_eula_mock.call_count == 1
@@ -449,7 +469,8 @@ def test_main_rollback_analyze_exit_phase(global_tool_opts, monkeypatch):
     assert summary_as_json_mock.call_count == 1
 
 
-def test_main_rollback_post_ponr_changes_phase(monkeypatch, caplog):
+def test_main_rollback_post_ponr_changes_phase(monkeypatch, caplog, tmp_path):
+    require_root_mock = mock.Mock()
     initialize_logger_mock = mock.Mock()
     toolopts_cli_mock = mock.Mock()
     show_eula_mock = mock.Mock()
@@ -470,6 +491,8 @@ def test_main_rollback_post_ponr_changes_phase(monkeypatch, caplog):
     finish_collection_mock = mock.Mock()
     update_rhsm_custom_facts_mock = mock.Mock()
 
+    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(toolopts, "CLI", toolopts_cli_mock)
     monkeypatch.setattr(main, "show_eula", show_eula_mock)
@@ -489,6 +512,7 @@ def test_main_rollback_post_ponr_changes_phase(monkeypatch, caplog):
     monkeypatch.setattr(report, "summary_as_json", summary_as_json_mock)
 
     assert main.main() == 1
+    assert require_root_mock.call_count == 1
     assert initialize_logger_mock.call_count == 1
     assert toolopts_cli_mock.call_count == 1
     assert show_eula_mock.call_count == 1
@@ -504,5 +528,5 @@ def test_main_rollback_post_ponr_changes_phase(monkeypatch, caplog):
     assert post_ponr_conversion_mock.call_count == 1
     assert finish_collection_mock.call_count == 1
     assert summary_as_json_mock.call_count == 1
-    assert "The system is left in an undetermined state that Convert2RHEL cannot fix." in caplog.records[-1].message
+    assert "The system is left in an undetermined state that Convert2RHEL cannot fix." in caplog.records[-2].message
     assert update_rhsm_custom_facts_mock.call_count == 1