From 8c5f42e509d8cbed408adff6e9ffc88bbe314083 Mon Sep 17 00:00:00 2001 From: Andrea Waltlova Date: Mon, 18 Mar 2024 15:03:45 +0100 Subject: [PATCH 1/5] Replace some prints by log messages Signed-off-by: Andrea Waltlova --- scripts/c2r_script.py | 104 ++++++++++++++++++++++-------------- tests/test_main_analysis.py | 10 ++-- tests/test_main_common.py | 3 +- 3 files changed, 73 insertions(+), 44 deletions(-) diff --git a/scripts/c2r_script.py b/scripts/c2r_script.py index 2bed488..d574f30 100644 --- a/scripts/c2r_script.py +++ b/scripts/c2r_script.py @@ -1,4 +1,5 @@ import json +import logging import os import re import shutil @@ -8,6 +9,11 @@ from urllib2 import urlopen, URLError +# NOTE: What are all possible levels we can get from rhc? +log_level = os.getenv("LOG_LEVEL", "INFO").upper() +log_level = getattr(logging, log_level, logging.INFO) +logging.basicConfig(level=log_level, format="%(asctime)s - %(levelname)s - %(message)s") + # SCRIPT_TYPE is either 'CONVERSION' or 'ANALYSIS' # Value is set in signed yaml envelope in content_vars (SCRIPT_MODE) SCRIPT_TYPE = os.getenv("RHC_WORKER_SCRIPT_MODE", "").upper() @@ -68,46 +74,52 @@ def _create(self, data): try: directory = os.path.dirname(self.path) if not os.path.exists(directory): - print("Creating directory at '%s'" % directory) + logging.info("Creating directory at '%s'", directory) os.makedirs(directory, mode=0o755) - print("Writing file to destination: '%s'" % self.path) + logging.info("Writing file to destination: '%s'", self.path) with open(self.path, mode="w") as handler: handler.write(data) os.chmod(self.path, 0o644) except OSError as err: - print(err) + logging.warning(err) return False return True def delete(self): try: - print("Removing the file '%s' as it was previously downloaded." % self.path) + logging.info( + "Removing the file '%s' as it was previously downloaded.", self.path + ) os.remove(self.path) except OSError as err: - print(err) + logging.warning(err) return False return True def restore(self): """Restores file backup (rename). Returns True if restored, otherwise False.""" try: - print("Trying to restore %s" % (self.path + self.backup_suffix)) + logging.info("Restoring backed up file %s.", (self.path + self.backup_suffix)) os.rename(self.path + self.backup_suffix, self.path) print("File restored (%s)." % (self.path)) except OSError as err: - print("Failed to restore %s (%s)" % (self.path + self.backup_suffix, err)) + logging.warning("Failed to restore %s (%s)" % (self.path + self.backup_suffix, err)) return False return True def backup(self): """Creates backup file (rename). Returns True if backed up, otherwise False.""" try: - print("Trying to create back up of %s" % (self.path)) + logging.info( + "File %s already present on system, backing up to %s.", + self.path, + self.backup_suffix, + ) os.rename(self.path, self.path + self.backup_suffix) print("Back up created (%s)." % (self.path + self.backup_suffix)) except OSError as err: - print("Failed to create back up of %s (%s)" % (self.path, err)) + logging.warning("Failed to create back up of %s (%s)" % (self.path, err)) return False return True @@ -164,7 +176,9 @@ def to_dict(self): def check_for_inhibitors_in_rollback(): """Returns lines with errors in rollback section of c2r log file, or empty string.""" - print("Checking content of '%s' for possible rollback problems ..." % C2R_LOG_FILE) + logging.info( + "Checking content of '%s' for possible rollback problems ...", C2R_LOG_FILE + ) matches = "" start_of_rollback_section = "WARNING - Abnormal exit! Performing rollback ..." try: @@ -181,12 +195,13 @@ def check_for_inhibitors_in_rollback(): matches = list(filter(DETECT_ERROR_IN_ROLLBACK_PATTERN.match, actual_data)) matches = "\n".join(matches) except ValueError: - print( - "Failed to find rollback section ('%s') in '%s' file." - % (start_of_rollback_section, C2R_LOG_FILE) + logging.info( + "Failed to find rollback section ('%s') in '%s' file.", + start_of_rollback_section, + C2R_LOG_FILE, ) except IOError: - print("Failed to read '%s' file.") + logging.warning("Failed to read '%s' file.", C2R_LOG_FILE) return matches @@ -220,9 +235,10 @@ def check_convert2rhel_inhibitors_before_run(): """ default_ini_path = "/etc/convert2rhel.ini" custom_ini_path = os.path.expanduser("~/.convert2rhel.ini") - print( - "Checking that '%s' wasn't modified and '%s' doesn't exist ..." - % (default_ini_path, custom_ini_path) + logging.info( + "Checking that '%s' wasn't modified and '%s' doesn't exist ...", + default_ini_path, + custom_ini_path, ) if os.path.exists(custom_ini_path): @@ -250,7 +266,7 @@ def check_convert2rhel_inhibitors_before_run(): def get_system_distro_version(): """Currently we execute the task only for RHEL 7 or 8""" - print("Checking OS distribution and version ID ...") + logging.info("Checking OS distribution and version ID ...") try: distribution_id = None version_id = None @@ -266,9 +282,13 @@ def get_system_distro_version(): if match: version_id = "%s.%s" % (match.group(1), match.group(2)) except IOError: - print("Couldn't read /etc/system-release") + logging.warning("Couldn't read /etc/system-release") - print("Detected distribution='%s' in version='%s'" % (distribution_id, version_id)) + logging.info( + "Detected distribution='%s' in version='%s'", + distribution_id, + version_id, + ) return distribution_id, version_id @@ -302,7 +322,7 @@ def archive_analysis_report(file): def gather_json_report(): """Collect the json report generated by convert2rhel.""" - print("Collecting JSON report.") + logging.info("Collecting JSON report.") if not os.path.exists(C2R_REPORT_FILE): return {} @@ -330,7 +350,7 @@ def gather_textual_report(): It's fine if the textual report does not exist, but the JSON one is required. """ - print("Collecting TXT report.") + logging.info("Collecting TXT report.") data = "" if os.path.exists(C2R_REPORT_TXT_FILE): with open(C2R_REPORT_TXT_FILE, mode="r") as handler: @@ -375,7 +395,7 @@ def generate_report_message(highest_status): def setup_convert2rhel(required_files): """Setup convert2rhel tool by downloading the required files.""" - print("Downloading required files.") + logging.info("Downloading required files.") try: for required_file in required_files: required_file.backup() @@ -411,7 +431,7 @@ def run_subprocess(cmd, print_cmd=True, env=None): raise TypeError("cmd should be a list, not a str") if print_cmd: - print("Calling command '%s'" % " ".join(cmd)) + logging.info("Calling command '%s'", " ".join(cmd)) process = subprocess.Popen( cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, bufsize=1, env=env @@ -432,11 +452,13 @@ def _get_last_yum_transaction_id(pkg_name): output, return_code = run_subprocess(["/usr/bin/yum", "history", "list", pkg_name]) if return_code: # NOTE: There is only print because list will exit with 1 when no such transaction exist - print( + logging.warning( "Listing yum transaction history for '%s' failed with exit status '%s' and output '%s'" - % (pkg_name, return_code, output), - "\nThis may cause clean up function to not remove '%s' after Task run." - % pkg_name, + "\nThis may cause clean up function to not remove '%s' after Task run.", + pkg_name, + return_code, + output, + pkg_name, ) return None @@ -454,7 +476,7 @@ def install_or_update_convert2rhel(required_files): Install the convert2rhel tool to the system. Returns True and transaction ID if the c2r pkg was installed, otherwise False, None. """ - print("Installing & updating Convert2RHEL package.") + logging.info("Installing & updating Convert2RHEL package.") c2r_pkg_name = "convert2rhel" c2r_installed = _check_if_package_installed(c2r_pkg_name) @@ -488,7 +510,7 @@ def run_convert2rhel(): """ Run the convert2rhel tool assigning the correct environment variables. """ - print("Running Convert2RHEL %s" % SCRIPT_TYPE.title()) + logging.info("Running Convert2RHEL %s" % SCRIPT_TYPE.title()) env = os.environ for key in env: @@ -524,9 +546,11 @@ def cleanup(required_files): ["/usr/bin/yum", "history", "undo", "-y", transaction_id], ) if returncode: - print( - "Undo of yum transaction with ID %s failed with exit status '%s' and output:\n%s" - % (transaction_id, returncode, output) + logging.warning( + "Undo of yum transaction with ID %s failed with exit status '%s' and output:\n%s", + transaction_id, + returncode, + output, ) @@ -625,7 +649,7 @@ def update_insights_inventory(): """ Call insights-client to update insights inventory. """ - print("Updating system status in Red Hat Insights.") + logging.info("Updating system status in Red Hat Insights.") output, returncode = run_subprocess(cmd=["/usr/bin/insights-client"]) if returncode: @@ -635,7 +659,7 @@ def update_insights_inventory(): % (returncode, output.rstrip("\n")), ) - print("System registered with insights-client successfully.") + logging.info("System registered with insights-client successfully.") # pylint: disable=too-many-branches @@ -744,9 +768,11 @@ def main(): if IS_CONVERSION: update_insights_inventory() - print("Convert2RHEL %s script finished successfully!" % SCRIPT_TYPE.title()) + logging.info( + "Convert2RHEL %s script finished successfully!", SCRIPT_TYPE.title() + ) except ProcessError as exception: - print(exception.report) + logging.error(exception.report) output = OutputCollector( status="ERROR", alert=True, @@ -755,7 +781,7 @@ def main(): report=exception.report, ) except Exception as exception: - print(str(exception)) + logging.critical(str(exception)) output = OutputCollector( status="ERROR", alert=True, @@ -803,7 +829,7 @@ def main(): output.entries = transform_raw_data(data) if do_cleanup: - print("Cleaning up modifications to the system.") + logging.info("Cleaning up modifications to the system.") cleanup(required_files) print("### JSON START ###") diff --git a/tests/test_main_analysis.py b/tests/test_main_analysis.py index 6e2007c..a311b42 100644 --- a/tests/test_main_analysis.py +++ b/tests/test_main_analysis.py @@ -39,12 +39,13 @@ def test_main_success_c2r_installed( mock_install_or_update_convert2rhel, mock_gather_json_report, capsys, # to check for rollback info in stdout + caplog, ): main() output = capsys.readouterr().out - assert "rollback" not in output - assert "Convert2RHEL Analysis script finished successfully!" in output + assert "rollback" not in caplog.text + assert "Convert2RHEL Analysis script finished successfully!" in caplog.text assert '"alert": false' in output assert mock_rollback_inhibitor_check.call_count == 1 assert mock_update_insights_inventory.call_count == 0 @@ -95,12 +96,13 @@ def test_main_success_c2r_updated( mock_install_or_update_convert2rhel, mock_gather_json_report, capsys, # to check for rollback info in stdout + caplog, ): main() output = capsys.readouterr().out - assert "rollback" not in output - assert "Convert2RHEL Analysis script finished successfully!" in output + assert "rollback" not in caplog.text + assert "Convert2RHEL Analysis script finished successfully!" in caplog.text assert '"alert": false' in output assert mock_rollback_inhibitor_check.call_count == 1 diff --git a/tests/test_main_common.py b/tests/test_main_common.py index 1ab46f2..cd20025 100644 --- a/tests/test_main_common.py +++ b/tests/test_main_common.py @@ -12,13 +12,14 @@ def test_main_invalid_script_value( mock_cleanup, mock_archive, capsys, + caplog, ): mock_output_collector.return_value = OutputCollector(entries=["non-empty"]) main() output = capsys.readouterr().out - assert "Exiting because RHC_WORKER_SCRIPT_MODE" in output + assert "Exiting because RHC_WORKER_SCRIPT_MODE" in caplog.text assert '"alert": false' in output mock_output_collector.assert_called() From 955989abee4c19bd18d137cee0b7d4326862e1b2 Mon Sep 17 00:00:00 2001 From: Rodolfo Olivieri Date: Wed, 3 Apr 2024 10:16:16 -0300 Subject: [PATCH 2/5] Add sosreport and log to a file support Now the logs will be written to the log file and also have a link to the sos report, so when it will be collected, we will have all the logs that were executed as well. --- scripts/c2r_script.py | 180 +++++++++++++++++++++++++++------- tests/test_logging.py | 71 ++++++++++++++ tests/test_main_analysis.py | 77 ++++++++++++++- tests/test_main_common.py | 20 ++++ tests/test_main_conversion.py | 74 ++++++++++++++ 5 files changed, 386 insertions(+), 36 deletions(-) create mode 100644 tests/test_logging.py diff --git a/scripts/c2r_script.py b/scripts/c2r_script.py index d574f30..3ab3f2f 100644 --- a/scripts/c2r_script.py +++ b/scripts/c2r_script.py @@ -5,15 +5,11 @@ import shutil import subprocess import copy +import sys from time import gmtime, strftime from urllib2 import urlopen, URLError -# NOTE: What are all possible levels we can get from rhc? -log_level = os.getenv("LOG_LEVEL", "INFO").upper() -log_level = getattr(logging, log_level, logging.INFO) -logging.basicConfig(level=log_level, format="%(asctime)s - %(levelname)s - %(message)s") - # SCRIPT_TYPE is either 'CONVERSION' or 'ANALYSIS' # Value is set in signed yaml envelope in content_vars (SCRIPT_MODE) SCRIPT_TYPE = os.getenv("RHC_WORKER_SCRIPT_MODE", "").upper() @@ -54,6 +50,23 @@ # Detect the last transaction id in yum. LATEST_YUM_TRANSACTION_PATTERN = re.compile(r"^(\s+)?(\d+)", re.MULTILINE) +# Path to store the script logs +LOG_DIR = "/var/log/convert2rhel-worker-scripts" +# Log filename for the script. It will be created based on the script type of +# execution. +LOG_FILENAME = "convert2rhel-worker-script-%s.log" % ( + "conversion" if IS_CONVERSION else "analysis" +) + +# Path to the sos extras folder +SOS_REPORT_FOLDER = "/etc/sos.extras.d" +# Name of the file based on the conversion type for sos report +SOS_REPORT_FILE = "convert2rhel-worker-scripts-%s-logs" % ( + "conversion" if IS_CONVERSION else "analysis" +) + +logger = logging.getLogger(__name__) + class RequiredFile(object): """Holds data about files needed to download convert2rhel""" @@ -74,44 +87,44 @@ def _create(self, data): try: directory = os.path.dirname(self.path) if not os.path.exists(directory): - logging.info("Creating directory at '%s'", directory) + logger.info("Creating directory at '%s'", directory) os.makedirs(directory, mode=0o755) - logging.info("Writing file to destination: '%s'", self.path) + logger.info("Writing file to destination: '%s'", self.path) with open(self.path, mode="w") as handler: handler.write(data) os.chmod(self.path, 0o644) except OSError as err: - logging.warning(err) + logger.warning(err) return False return True def delete(self): try: - logging.info( + logger.info( "Removing the file '%s' as it was previously downloaded.", self.path ) os.remove(self.path) except OSError as err: - logging.warning(err) + logger.warning(err) return False return True def restore(self): """Restores file backup (rename). Returns True if restored, otherwise False.""" try: - logging.info("Restoring backed up file %s.", (self.path + self.backup_suffix)) + logger.info("Restoring backed up file %s.", (self.path + self.backup_suffix)) os.rename(self.path + self.backup_suffix, self.path) print("File restored (%s)." % (self.path)) except OSError as err: - logging.warning("Failed to restore %s (%s)" % (self.path + self.backup_suffix, err)) + logger.warning("Failed to restore %s (%s)" % (self.path + self.backup_suffix, err)) return False return True def backup(self): """Creates backup file (rename). Returns True if backed up, otherwise False.""" try: - logging.info( + logger.info( "File %s already present on system, backing up to %s.", self.path, self.backup_suffix, @@ -119,7 +132,7 @@ def backup(self): os.rename(self.path, self.path + self.backup_suffix) print("Back up created (%s)." % (self.path + self.backup_suffix)) except OSError as err: - logging.warning("Failed to create back up of %s (%s)" % (self.path, err)) + logger.warning("Failed to create back up of %s (%s)" % (self.path, err)) return False return True @@ -174,9 +187,102 @@ def to_dict(self): } +def setup_sos_report(): + """Setup sos report log collection.""" + if not os.path.exists(SOS_REPORT_FOLDER): + os.makedirs(SOS_REPORT_FOLDER) + + script_log_file = os.path.join(LOG_DIR, LOG_FILENAME) + sosreport_link_file = os.path.join(SOS_REPORT_FOLDER, SOS_REPORT_FILE) + # In case the file for sos report does not exist, lets create one and add + # the log file path to it. + if not os.path.exists(sosreport_link_file): + with open(sosreport_link_file, mode="w") as handler: + handler.write(":%s\n" % script_log_file) + + +def setup_logger_handler(): + """ + Setup custom logging levels, handlers, and so on. Call this method from + your application's main start point. + """ + # Receive the log level from the worker and try to parse it. If the log + # level is not compatible with what the logging library expects, set the + # log level to INFO automatically. + log_level = os.getenv("RHC_WORKER_LOG_LEVEL", "INFO").upper() + log_level = logging.getLevelName(log_level) + if isinstance(log_level, str): + log_level = logger.INFO + + # enable raising exceptions + logging.raiseExceptions = True + logger.setLevel(log_level) + + # create sys.stdout handler for info/debug + stdout_handler = logging.StreamHandler(sys.stdout) + formatter = logging.Formatter("%(asctime)s - %(levelname)s - %(message)s") + stdout_handler.setFormatter(formatter) + + # Create the directory if it don't exist + if not os.path.exists(LOG_DIR): + os.makedirs(LOG_DIR) + + log_filepath = os.path.join(LOG_DIR, LOG_FILENAME) + file_handler = logging.FileHandler(log_filepath) + file_handler.setFormatter(formatter) + + # can flush logs to the file that were logged before initializing the file handler + logger.addHandler(stdout_handler) + logger.addHandler(file_handler) + + +def archive_old_logger_files(): + """ + Archive the old log files to not mess with multiple runs outputs. Every + time a new run begins, this method will be called to archive the previous + logs if there is a `convert2rhel.log` file there, it will be archived using + the same name for the log file, but having an appended timestamp to it. + + For example: + /var/log/convert2rhel-worker-scripts/archive/convert2rhel-worker-scripts-1635162445070567607.log + /var/log/convert2rhel-worker-scripts/archive/convert2rhel-worker-scripts-1635162478219820043.log + + This way, the user can track the logs for each run individually based on + the timestamp. + """ + + current_log_file = os.path.join(LOG_DIR, LOG_FILENAME) + archive_log_dir = os.path.join(LOG_DIR, "archive") + + # No log file found, that means it's a first run or it was manually deleted + if not os.path.exists(current_log_file): + return + + stat = os.stat(current_log_file) + + # Get the last modified time in UTC + last_modified_at = gmtime(stat.st_mtime) + + # Format time to a human-readable format + formatted_time = strftime("%Y%m%dT%H%M%SZ", last_modified_at) + + # Create the directory if it don't exist + if not os.path.exists(archive_log_dir): + os.makedirs(archive_log_dir) + + file_name, suffix = tuple(LOG_FILENAME.rsplit(".", 1)) + archive_log_file = "%s/%s-%s.%s" % ( + archive_log_dir, + file_name, + formatted_time, + suffix, + ) + shutil.move(current_log_file, archive_log_file) + + def check_for_inhibitors_in_rollback(): """Returns lines with errors in rollback section of c2r log file, or empty string.""" - logging.info( + logger.info( "Checking content of '%s' for possible rollback problems ...", C2R_LOG_FILE ) matches = "" @@ -195,13 +301,13 @@ def check_for_inhibitors_in_rollback(): matches = list(filter(DETECT_ERROR_IN_ROLLBACK_PATTERN.match, actual_data)) matches = "\n".join(matches) except ValueError: - logging.info( + logger.info( "Failed to find rollback section ('%s') in '%s' file.", start_of_rollback_section, C2R_LOG_FILE, ) except IOError: - logging.warning("Failed to read '%s' file.", C2R_LOG_FILE) + logger.warning("Failed to read '%s' file.", C2R_LOG_FILE) return matches @@ -235,7 +341,7 @@ def check_convert2rhel_inhibitors_before_run(): """ default_ini_path = "/etc/convert2rhel.ini" custom_ini_path = os.path.expanduser("~/.convert2rhel.ini") - logging.info( + logger.info( "Checking that '%s' wasn't modified and '%s' doesn't exist ...", default_ini_path, custom_ini_path, @@ -266,7 +372,7 @@ def check_convert2rhel_inhibitors_before_run(): def get_system_distro_version(): """Currently we execute the task only for RHEL 7 or 8""" - logging.info("Checking OS distribution and version ID ...") + logger.info("Checking OS distribution and version ID ...") try: distribution_id = None version_id = None @@ -282,9 +388,9 @@ def get_system_distro_version(): if match: version_id = "%s.%s" % (match.group(1), match.group(2)) except IOError: - logging.warning("Couldn't read /etc/system-release") + logger.warning("Couldn't read /etc/system-release") - logging.info( + logger.info( "Detected distribution='%s' in version='%s'", distribution_id, version_id, @@ -322,7 +428,7 @@ def archive_analysis_report(file): def gather_json_report(): """Collect the json report generated by convert2rhel.""" - logging.info("Collecting JSON report.") + logger.info("Collecting JSON report.") if not os.path.exists(C2R_REPORT_FILE): return {} @@ -350,7 +456,7 @@ def gather_textual_report(): It's fine if the textual report does not exist, but the JSON one is required. """ - logging.info("Collecting TXT report.") + logger.info("Collecting TXT report.") data = "" if os.path.exists(C2R_REPORT_TXT_FILE): with open(C2R_REPORT_TXT_FILE, mode="r") as handler: @@ -395,7 +501,7 @@ def generate_report_message(highest_status): def setup_convert2rhel(required_files): """Setup convert2rhel tool by downloading the required files.""" - logging.info("Downloading required files.") + logger.info("Downloading required files.") try: for required_file in required_files: required_file.backup() @@ -431,7 +537,7 @@ def run_subprocess(cmd, print_cmd=True, env=None): raise TypeError("cmd should be a list, not a str") if print_cmd: - logging.info("Calling command '%s'", " ".join(cmd)) + logger.info("Calling command '%s'", " ".join(cmd)) process = subprocess.Popen( cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, bufsize=1, env=env @@ -452,7 +558,7 @@ def _get_last_yum_transaction_id(pkg_name): output, return_code = run_subprocess(["/usr/bin/yum", "history", "list", pkg_name]) if return_code: # NOTE: There is only print because list will exit with 1 when no such transaction exist - logging.warning( + logger.warning( "Listing yum transaction history for '%s' failed with exit status '%s' and output '%s'" "\nThis may cause clean up function to not remove '%s' after Task run.", pkg_name, @@ -476,7 +582,7 @@ def install_or_update_convert2rhel(required_files): Install the convert2rhel tool to the system. Returns True and transaction ID if the c2r pkg was installed, otherwise False, None. """ - logging.info("Installing & updating Convert2RHEL package.") + logger.info("Installing & updating Convert2RHEL package.") c2r_pkg_name = "convert2rhel" c2r_installed = _check_if_package_installed(c2r_pkg_name) @@ -510,7 +616,7 @@ def run_convert2rhel(): """ Run the convert2rhel tool assigning the correct environment variables. """ - logging.info("Running Convert2RHEL %s" % SCRIPT_TYPE.title()) + logger.info("Running Convert2RHEL %s" % SCRIPT_TYPE.title()) env = os.environ for key in env: @@ -546,7 +652,7 @@ def cleanup(required_files): ["/usr/bin/yum", "history", "undo", "-y", transaction_id], ) if returncode: - logging.warning( + logger.warning( "Undo of yum transaction with ID %s failed with exit status '%s' and output:\n%s", transaction_id, returncode, @@ -649,7 +755,7 @@ def update_insights_inventory(): """ Call insights-client to update insights inventory. """ - logging.info("Updating system status in Red Hat Insights.") + logger.info("Updating system status in Red Hat Insights.") output, returncode = run_subprocess(cmd=["/usr/bin/insights-client"]) if returncode: @@ -659,7 +765,7 @@ def update_insights_inventory(): % (returncode, output.rstrip("\n")), ) - logging.info("System registered with insights-client successfully.") + logger.info("System registered with insights-client successfully.") # pylint: disable=too-many-branches @@ -690,6 +796,10 @@ def main(): # Switched to True only after setup is called do_cleanup = False + setup_sos_report() + archive_old_logger_files() + setup_logger_handler() + try: # Exit if invalid value for SCRIPT_TYPE if SCRIPT_TYPE not in ["CONVERSION", "ANALYSIS"]: @@ -768,11 +878,11 @@ def main(): if IS_CONVERSION: update_insights_inventory() - logging.info( + logger.info( "Convert2RHEL %s script finished successfully!", SCRIPT_TYPE.title() ) except ProcessError as exception: - logging.error(exception.report) + logger.error(exception.report) output = OutputCollector( status="ERROR", alert=True, @@ -781,7 +891,7 @@ def main(): report=exception.report, ) except Exception as exception: - logging.critical(str(exception)) + logger.critical(str(exception)) output = OutputCollector( status="ERROR", alert=True, @@ -829,7 +939,7 @@ def main(): output.entries = transform_raw_data(data) if do_cleanup: - logging.info("Cleaning up modifications to the system.") + logger.info("Cleaning up modifications to the system.") cleanup(required_files) print("### JSON START ###") diff --git a/tests/test_logging.py b/tests/test_logging.py new file mode 100644 index 0000000..f42a50f --- /dev/null +++ b/tests/test_logging.py @@ -0,0 +1,71 @@ +import os +import logging +import pytest +import scripts + +from scripts.c2r_script import ( + setup_sos_report, + setup_logger_handler, + archive_old_logger_files, +) + + +def test_setup_sos_report(monkeypatch, tmpdir): + sos_report_folder = str(tmpdir) + monkeypatch.setattr(scripts.c2r_script, "SOS_REPORT_FOLDER", sos_report_folder) + + setup_sos_report() + + sos_report_file = os.path.join( + sos_report_folder, "convert2rhel-worker-scripts-analysis-logs" + ) + assert os.path.exists(sos_report_folder) + assert os.path.exists(sos_report_file) + + with open(sos_report_file) as handler: + assert ( + ":/var/log/convert2rhel-worker-scripts/convert2rhel-worker-script-analysis.log" + == handler.read().strip() + ) + + +@pytest.mark.noautofixtures +def test_setup_logger_handler(monkeypatch, tmpdir): + log_dir = str(tmpdir) + monkeypatch.setattr(scripts.c2r_script, "LOG_DIR", log_dir) + monkeypatch.setattr(scripts.c2r_script, "LOG_FILENAME", "filelog.log") + + logger = logging.getLogger(__name__) + setup_logger_handler() + + # emitting some log entries + logger.info("Test info: %s", "data") + logger.debug("Test debug: %s", "other data") + + assert os.path.exists(log_dir) + + +def test_archive_old_logger_files(monkeypatch, tmpdir): + log_dir = str(tmpdir) + archive_dir = os.path.join(log_dir, "archive") + monkeypatch.setattr(scripts.c2r_script, "LOG_DIR", log_dir) + monkeypatch.setattr(scripts.c2r_script, "LOG_FILENAME", "test.log") + + original_log_file = tmpdir.join("test.log") + original_log_file.write("test") + + archive_old_logger_files() + + assert os.path.exists(log_dir) + assert os.path.exists(archive_dir) + assert len(os.listdir(archive_dir)) == 1 + + +def test_archive_old_logger_files_no_log_file(monkeypatch, tmpdir): + log_dir = str(tmpdir.join("something-else")) + monkeypatch.setattr(scripts.c2r_script, "LOG_DIR", log_dir) + monkeypatch.setattr(scripts.c2r_script, "LOG_FILENAME", "test.log") + + archive_old_logger_files() + + assert not os.path.exists(log_dir) diff --git a/tests/test_main_analysis.py b/tests/test_main_analysis.py index a311b42..441ee92 100644 --- a/tests/test_main_analysis.py +++ b/tests/test_main_analysis.py @@ -22,9 +22,15 @@ @patch("scripts.c2r_script.archive_analysis_report", side_effect=Mock()) @patch("scripts.c2r_script.check_for_inhibitors_in_rollback", return_value="") @patch("scripts.c2r_script.update_insights_inventory", side_effect=Mock()) +@patch("scripts.c2r_script.setup_sos_report", side_effect=Mock()) +@patch("scripts.c2r_script.archive_old_logger_files", side_effect=Mock()) +@patch("scripts.c2r_script.setup_logger_handler", side_effect=Mock()) # fmt: on # pylint: disable=too-many-locals def test_main_success_c2r_installed( + mock_setup_logger_handler, + mock_setup_sos_report, + mock_archive_old_logger_files, mock_update_insights_inventory, mock_rollback_inhibitor_check, mock_archive_analysis_report, @@ -47,6 +53,10 @@ def test_main_success_c2r_installed( assert "rollback" not in caplog.text assert "Convert2RHEL Analysis script finished successfully!" in caplog.text assert '"alert": false' in output + + assert mock_setup_logger_handler.call_count == 1 + assert mock_setup_sos_report.call_count == 1 + assert mock_archive_old_logger_files.call_count == 1 assert mock_rollback_inhibitor_check.call_count == 1 assert mock_update_insights_inventory.call_count == 0 assert mock_install_or_update_convert2rhel.call_count == 1 @@ -79,9 +89,15 @@ def test_main_success_c2r_installed( @patch("scripts.c2r_script.archive_analysis_report", side_effect=Mock()) @patch("scripts.c2r_script.check_for_inhibitors_in_rollback", return_value="") @patch("scripts.c2r_script.update_insights_inventory", side_effect=Mock()) +@patch("scripts.c2r_script.setup_sos_report", side_effect=Mock()) +@patch("scripts.c2r_script.archive_old_logger_files", side_effect=Mock()) +@patch("scripts.c2r_script.setup_logger_handler", side_effect=Mock()) # fmt: on # pylint: disable=too-many-locals def test_main_success_c2r_updated( + mock_setup_logger_handler, + mock_setup_sos_report, + mock_archive_old_logger_files, mock_update_insights_inventory, mock_rollback_inhibitor_check, mock_archive_analysis_report, @@ -104,8 +120,11 @@ def test_main_success_c2r_updated( assert "rollback" not in caplog.text assert "Convert2RHEL Analysis script finished successfully!" in caplog.text assert '"alert": false' in output - assert mock_rollback_inhibitor_check.call_count == 1 + assert mock_rollback_inhibitor_check.call_count == 1 + assert mock_setup_logger_handler.call_count == 1 + assert mock_setup_sos_report.call_count == 1 + assert mock_archive_old_logger_files.call_count == 1 assert mock_update_insights_inventory.call_count == 0 assert mock_install_or_update_convert2rhel.call_count == 1 assert mock_inhibitor_check.call_count == 1 @@ -136,8 +155,15 @@ def test_main_success_c2r_updated( @patch("scripts.c2r_script.is_eligible_releases", return_value=True) @patch("scripts.c2r_script.archive_analysis_report", side_effect=Mock()) @patch("scripts.c2r_script.update_insights_inventory", side_effect=Mock()) +@patch("scripts.c2r_script.setup_sos_report", side_effect=Mock()) +@patch("scripts.c2r_script.archive_old_logger_files", side_effect=Mock()) +@patch("scripts.c2r_script.setup_logger_handler", side_effect=Mock()) # fmt: on +# pylint: disable=too-many-locals def test_main_process_error( + mock_setup_logger_handler, + mock_setup_sos_report, + mock_archive_old_logger_files, mock_update_insights_inventory, mock_archive_analysis_report, mock_is_eligible_releases, @@ -159,6 +185,9 @@ def test_main_process_error( assert "Process error" in output assert '"alert": true' in output + assert mock_setup_logger_handler.call_count == 1 + assert mock_setup_sos_report.call_count == 1 + assert mock_archive_old_logger_files.call_count == 1 assert mock_update_insights_inventory.call_count == 0 assert mock_install_or_update_convert2rhel.call_count == 1 assert mock_inhibitor_check.call_count == 1 @@ -187,8 +216,14 @@ def test_main_process_error( @patch("scripts.c2r_script.is_eligible_releases", return_value=True) @patch("scripts.c2r_script.archive_analysis_report", side_effect=Mock()) @patch("scripts.c2r_script.update_insights_inventory", side_effect=Mock()) +@patch("scripts.c2r_script.setup_sos_report", side_effect=Mock()) +@patch("scripts.c2r_script.archive_old_logger_files", side_effect=Mock()) +@patch("scripts.c2r_script.setup_logger_handler", side_effect=Mock()) # fmt: on def test_main_general_exception( + mock_setup_logger_handler, + mock_setup_sos_report, + mock_archive_old_logger_files, mock_update_insights_inventory, mock_archive_analysis_report, mock_is_eligible_releases, @@ -208,6 +243,9 @@ def test_main_general_exception( assert "'Mock' object is not iterable" in output assert '"alert": true' in output + assert mock_setup_logger_handler.call_count == 1 + assert mock_setup_sos_report.call_count == 1 + assert mock_archive_old_logger_files.call_count == 1 assert mock_update_insights_inventory.call_count == 0 assert mock_install_or_update_convert2rhel.call_count == 1 assert mock_inhibitor_check.call_count == 1 @@ -235,8 +273,15 @@ def test_main_general_exception( @patch("scripts.c2r_script.is_eligible_releases", return_value=True) @patch("scripts.c2r_script.archive_analysis_report", side_effect=Mock()) @patch("scripts.c2r_script.update_insights_inventory", side_effect=Mock()) +@patch("scripts.c2r_script.setup_sos_report", side_effect=Mock()) +@patch("scripts.c2r_script.archive_old_logger_files", side_effect=Mock()) +@patch("scripts.c2r_script.setup_logger_handler", side_effect=Mock()) # fmt: on +# pylint: disable=too-many-locals def test_main_inhibited_ini_modified( + mock_setup_logger_handler, + mock_setup_sos_report, + mock_archive_old_logger_files, mock_update_insights_inventory, mock_archive_analysis_report, mock_is_eligible_releases, @@ -256,6 +301,9 @@ def test_main_inhibited_ini_modified( assert "/etc/convert2rhel.ini was modified" in output assert '"alert": true' in output + assert mock_setup_logger_handler.call_count == 1 + assert mock_setup_sos_report.call_count == 1 + assert mock_archive_old_logger_files.call_count == 1 assert mock_update_insights_inventory.call_count == 0 assert mock_archive_analysis_report.call_count == 0 assert mock_get_system_distro_version.call_count == 1 @@ -284,8 +332,14 @@ def test_main_inhibited_ini_modified( @patch("scripts.c2r_script.is_eligible_releases", return_value=True) @patch("scripts.c2r_script.archive_analysis_report", side_effect=Mock()) @patch("scripts.c2r_script.update_insights_inventory", side_effect=Mock()) +@patch("scripts.c2r_script.setup_sos_report", side_effect=Mock()) +@patch("scripts.c2r_script.archive_old_logger_files", side_effect=Mock()) +@patch("scripts.c2r_script.setup_logger_handler", side_effect=Mock()) # fmt: on def test_main_inhibited_custom_ini( + mock_setup_logger_handler, + mock_setup_sos_report, + mock_archive_old_logger_files, mock_update_insights_inventory, mock_archive_analysis_report, mock_is_eligible_releases, @@ -304,6 +358,9 @@ def test_main_inhibited_custom_ini( assert ".convert2rhel.ini was found" in output assert '"alert": true' in output + assert mock_setup_logger_handler.call_count == 1 + assert mock_setup_sos_report.call_count == 1 + assert mock_archive_old_logger_files.call_count == 1 assert mock_update_insights_inventory.call_count == 0 assert mock_archive_analysis_report.call_count == 2 assert mock_get_system_distro_version.call_count == 1 @@ -332,9 +389,15 @@ def test_main_inhibited_custom_ini( @patch("scripts.c2r_script.archive_analysis_report", side_effect=Mock()) @patch("scripts.c2r_script.check_for_inhibitors_in_rollback", return_value="") @patch("scripts.c2r_script.update_insights_inventory", side_effect=Mock()) +@patch("scripts.c2r_script.setup_sos_report", side_effect=Mock()) +@patch("scripts.c2r_script.archive_old_logger_files", side_effect=Mock()) +@patch("scripts.c2r_script.setup_logger_handler", side_effect=Mock()) # fmt: on # pylint: disable=too-many-locals def test_main_inhibited_c2r_installed_no_rollback_err( + mock_setup_logger_handler, + mock_setup_sos_report, + mock_archive_old_logger_files, mock_update_insights_inventory, mock_rollback_inhibitor_check, mock_archive_analysis_report, @@ -367,6 +430,9 @@ def test_main_inhibited_c2r_installed_no_rollback_err( assert mock_get_system_distro_version.call_count == 1 assert mock_is_eligible_releases.call_count == 1 assert mock_archive_analysis_report.call_count == 0 + assert mock_setup_logger_handler.call_count == 1 + assert mock_setup_sos_report.call_count == 1 + assert mock_archive_old_logger_files.call_count == 1 # fmt: off @@ -391,9 +457,15 @@ def test_main_inhibited_c2r_installed_no_rollback_err( @patch("scripts.c2r_script.archive_analysis_report", side_effect=Mock()) @patch("scripts.c2r_script.check_for_inhibitors_in_rollback", return_value="rollback error") @patch("scripts.c2r_script.update_insights_inventory", side_effect=Mock()) +@patch("scripts.c2r_script.setup_sos_report", side_effect=Mock()) +@patch("scripts.c2r_script.archive_old_logger_files", side_effect=Mock()) +@patch("scripts.c2r_script.setup_logger_handler", side_effect=Mock()) # fmt: on # pylint: disable=too-many-locals def test_main_inhibited_c2r_installed_rollback_errors( + mock_setup_logger_handler, + mock_setup_sos_report, + mock_archive_old_logger_files, mock_update_insights_inventory, mock_rollback_inhibitor_check, mock_archive_analysis_report, @@ -421,6 +493,9 @@ def test_main_inhibited_c2r_installed_rollback_errors( assert "A rollback of changes performed by convert2rhel failed" in output assert '"alert": true' in output + assert mock_setup_logger_handler.call_count == 1 + assert mock_setup_sos_report.call_count == 1 + assert mock_archive_old_logger_files.call_count == 1 assert mock_update_insights_inventory.call_count == 0 assert mock_install_or_update_convert2rhel.call_count == 1 assert mock_inhibitor_check.call_count == 1 diff --git a/tests/test_main_common.py b/tests/test_main_common.py index cd20025..e292dbe 100644 --- a/tests/test_main_common.py +++ b/tests/test_main_common.py @@ -7,7 +7,14 @@ @patch("scripts.c2r_script.archive_analysis_report", side_effect=Mock()) @patch("scripts.c2r_script.cleanup") @patch("scripts.c2r_script.OutputCollector") +@patch("scripts.c2r_script.setup_sos_report", side_effect=Mock()) +@patch("scripts.c2r_script.archive_old_logger_files", side_effect=Mock()) +@patch("scripts.c2r_script.setup_logger_handler", side_effect=Mock()) +# pylint: disable=too-many-arguments def test_main_invalid_script_value( + mock_setup_logger_handler, + mock_setup_sos_report, + mock_archive_old_logger_files, mock_output_collector, mock_cleanup, mock_archive, @@ -25,6 +32,9 @@ def test_main_invalid_script_value( mock_output_collector.assert_called() mock_cleanup.assert_not_called() mock_archive.assert_not_called() + assert mock_setup_logger_handler.call_count == 1 + assert mock_setup_sos_report.call_count == 1 + assert mock_archive_old_logger_files.call_count == 1 @pytest.mark.parametrize(("script_type"), [("ANALYSIS"), ("CONVERSION")]) @@ -35,7 +45,14 @@ def test_main_invalid_script_value( ) @patch("scripts.c2r_script.cleanup") @patch("scripts.c2r_script.OutputCollector") +@patch("scripts.c2r_script.setup_sos_report", side_effect=Mock()) +@patch("scripts.c2r_script.archive_old_logger_files", side_effect=Mock()) +@patch("scripts.c2r_script.setup_logger_handler", side_effect=Mock()) +# pylint: disable=too-many-arguments def test_main_non_eligible_release( + mock_setup_logger_handler, + mock_setup_sos_report, + mock_archive_old_logger_files, mock_output_collector, mock_cleanup, mock_get_system_distro_version, @@ -51,3 +68,6 @@ def test_main_non_eligible_release( mock_output_collector.assert_called() mock_cleanup.assert_not_called() mock_archive.assert_not_called() + assert mock_setup_logger_handler.call_count == 1 + assert mock_setup_sos_report.call_count == 1 + assert mock_archive_old_logger_files.call_count == 1 diff --git a/tests/test_main_conversion.py b/tests/test_main_conversion.py index 9d99055..6421380 100644 --- a/tests/test_main_conversion.py +++ b/tests/test_main_conversion.py @@ -21,9 +21,15 @@ @patch("scripts.c2r_script.get_system_distro_version", return_value=("centos", "7.9")) @patch("scripts.c2r_script.is_eligible_releases", return_value=True) @patch("scripts.c2r_script.archive_analysis_report", side_effect=Mock()) +@patch("scripts.c2r_script.setup_sos_report", side_effect=Mock()) +@patch("scripts.c2r_script.archive_old_logger_files", side_effect=Mock()) +@patch("scripts.c2r_script.setup_logger_handler", side_effect=Mock()) # fmt: on # pylint: disable=too-many-locals def test_main_success_c2r_installed( + mock_setup_logger_handler, + mock_setup_sos_report, + mock_archive_old_logger_files, mock_archive_analysis_report, mock_is_eligible_releases, mock_get_system_distro_version, @@ -44,6 +50,9 @@ def test_main_success_c2r_installed( assert "No problems found. The system was converted successfully." in output assert '"alert": false' in output + assert mock_setup_logger_handler.call_count == 1 + assert mock_setup_sos_report.call_count == 1 + assert mock_archive_old_logger_files.call_count == 1 assert mock_archive_analysis_report.call_count == 0 assert mock_get_system_distro_version.call_count == 1 assert mock_is_eligible_releases.call_count == 1 @@ -76,9 +85,15 @@ def test_main_success_c2r_installed( # These patches are calls made in cleanup @patch("os.path.exists", return_value=False) @patch("scripts.c2r_script.run_subprocess", return_value=("", 1)) +@patch("scripts.c2r_script.setup_sos_report", side_effect=Mock()) +@patch("scripts.c2r_script.archive_old_logger_files", side_effect=Mock()) +@patch("scripts.c2r_script.setup_logger_handler", side_effect=Mock()) # fmt: on # pylint: disable=too-many-locals def test_main_success_c2r_updated( + mock_setup_logger_handler, + mock_setup_sos_report, + mock_archive_old_logger_files, mock_cleanup_pkg_call, mock_os_exists, mock_transform_raw_data, @@ -99,6 +114,9 @@ def test_main_success_c2r_updated( assert "No problems found. The system was converted successfully." in output assert '"alert": false' in output + assert mock_setup_logger_handler.call_count == 1 + assert mock_setup_sos_report.call_count == 1 + assert mock_archive_old_logger_files.call_count == 1 assert mock_archive_analysis_report.call_count == 0 assert mock_get_system_distro_version.call_count == 1 assert mock_is_eligible_releases.call_count == 1 @@ -128,8 +146,14 @@ def test_main_success_c2r_updated( @patch("scripts.c2r_script.is_eligible_releases", return_value=True) @patch("scripts.c2r_script.archive_analysis_report", side_effect=Mock()) @patch("scripts.c2r_script.update_insights_inventory", side_effect=Mock()) +@patch("scripts.c2r_script.setup_sos_report", side_effect=Mock()) +@patch("scripts.c2r_script.archive_old_logger_files", side_effect=Mock()) +@patch("scripts.c2r_script.setup_logger_handler", side_effect=Mock()) # fmt: on def test_main_process_error_no_report( + mock_setup_logger_handler, + mock_setup_sos_report, + mock_archive_old_logger_files, mock_update_insights_inventory, mock_archive_analysis_report, mock_is_eligible_releases, @@ -148,6 +172,9 @@ def test_main_process_error_no_report( assert "An error occurred during the conversion" in output assert '"alert": true' in output + assert mock_setup_logger_handler.call_count == 1 + assert mock_setup_sos_report.call_count == 1 + assert mock_archive_old_logger_files.call_count == 1 # Zero because os.path.exists is not mocked and reports do not exist assert mock_archive_analysis_report.call_count == 0 assert mock_get_system_distro_version.call_count == 1 @@ -174,8 +201,14 @@ def test_main_process_error_no_report( @patch("scripts.c2r_script.is_eligible_releases", return_value=True) @patch("scripts.c2r_script.archive_analysis_report", side_effect=Mock()) @patch("scripts.c2r_script.update_insights_inventory", side_effect=Mock()) +@patch("scripts.c2r_script.setup_sos_report", side_effect=Mock()) +@patch("scripts.c2r_script.archive_old_logger_files", side_effect=Mock()) +@patch("scripts.c2r_script.setup_logger_handler", side_effect=Mock()) # fmt: on def test_main_general_exception( + mock_setup_logger_handler, + mock_setup_sos_report, + mock_archive_old_logger_files, mock_update_insights_inventory, mock_archive_analysis_report, mock_is_eligible_releases, @@ -194,6 +227,9 @@ def test_main_general_exception( assert "An unexpected error occurred" in output assert '"alert": true' in output + assert mock_setup_logger_handler.call_count == 1 + assert mock_setup_sos_report.call_count == 1 + assert mock_archive_old_logger_files.call_count == 1 # Zero because os.path.exists is not mocked and reports do not exist assert mock_archive_analysis_report.call_count == 0 assert mock_get_system_distro_version.call_count == 1 @@ -221,8 +257,15 @@ def test_main_general_exception( @patch("scripts.c2r_script.is_eligible_releases", return_value=True) @patch("scripts.c2r_script.archive_analysis_report", side_effect=Mock()) @patch("scripts.c2r_script.update_insights_inventory", side_effect=Mock()) +@patch("scripts.c2r_script.setup_sos_report", side_effect=Mock()) +@patch("scripts.c2r_script.archive_old_logger_files", side_effect=Mock()) +@patch("scripts.c2r_script.setup_logger_handler", side_effect=Mock()) # fmt: on +# pylint: disable=too-many-locals def test_main_inhibited_ini_modified( + mock_setup_logger_handler, + mock_setup_sos_report, + mock_archive_old_logger_files, mock_update_insights_inventory, mock_archive_analysis_report, mock_is_eligible_releases, @@ -242,6 +285,9 @@ def test_main_inhibited_ini_modified( assert "/etc/convert2rhel.ini was modified" in output assert '"alert": true' in output + assert mock_setup_logger_handler.call_count == 1 + assert mock_setup_sos_report.call_count == 1 + assert mock_archive_old_logger_files.call_count == 1 assert mock_archive_analysis_report.call_count == 0 assert mock_get_system_distro_version.call_count == 1 assert mock_is_eligible_releases.call_count == 1 @@ -270,8 +316,15 @@ def test_main_inhibited_ini_modified( @patch("scripts.c2r_script.is_eligible_releases", return_value=True) @patch("scripts.c2r_script.archive_analysis_report", side_effect=Mock()) @patch("scripts.c2r_script.update_insights_inventory", side_effect=Mock()) +@patch("scripts.c2r_script.setup_sos_report", side_effect=Mock()) +@patch("scripts.c2r_script.archive_old_logger_files", side_effect=Mock()) +@patch("scripts.c2r_script.setup_logger_handler", side_effect=Mock()) # fmt: on +# pylint: disable=too-many-locals def test_main_inhibited_custom_ini( + mock_setup_logger_handler, + mock_setup_sos_report, + mock_archive_old_logger_files, mock_update_insights_inventory, mock_archive_analysis_report, mock_is_eligible_releases, @@ -291,6 +344,9 @@ def test_main_inhibited_custom_ini( assert ".convert2rhel.ini was found" in output assert '"alert": true' in output + assert mock_setup_logger_handler.call_count == 1 + assert mock_setup_sos_report.call_count == 1 + assert mock_archive_old_logger_files.call_count == 1 assert mock_archive_analysis_report.call_count == 2 assert mock_get_system_distro_version.call_count == 1 assert mock_is_eligible_releases.call_count == 1 @@ -321,9 +377,15 @@ def test_main_inhibited_custom_ini( @patch("scripts.c2r_script.get_system_distro_version", return_value=("centos", "7.9")) @patch("scripts.c2r_script.is_eligible_releases", return_value=True) @patch("scripts.c2r_script.check_for_inhibitors_in_rollback", return_value="") +@patch("scripts.c2r_script.setup_sos_report", side_effect=Mock()) +@patch("scripts.c2r_script.archive_old_logger_files", side_effect=Mock()) +@patch("scripts.c2r_script.setup_logger_handler", side_effect=Mock()) # fmt: on # pylint: disable=too-many-locals def test_main_inhibited_c2r_installed_no_rollback_err( + mock_setup_logger_handler, + mock_setup_sos_report, + mock_archive_old_logger_files, mock_rollback_inhibitor_check, mock_is_eligible_releases, mock_get_system_distro_version, @@ -346,6 +408,9 @@ def test_main_inhibited_c2r_installed_no_rollback_err( mock_rollback_inhibitor_check.assert_called_once() + assert mock_setup_logger_handler.call_count == 1 + assert mock_setup_sos_report.call_count == 1 + assert mock_archive_old_logger_files.call_count == 1 assert mock_get_system_distro_version.call_count == 1 assert mock_is_eligible_releases.call_count == 1 assert mock_inhibitor_check.call_count == 1 @@ -377,9 +442,15 @@ def test_main_inhibited_c2r_installed_no_rollback_err( @patch("scripts.c2r_script.get_system_distro_version", return_value=("centos", "7.9")) @patch("scripts.c2r_script.is_eligible_releases", return_value=True) @patch("scripts.c2r_script.check_for_inhibitors_in_rollback", return_value="rollback error") +@patch("scripts.c2r_script.setup_sos_report", side_effect=Mock()) +@patch("scripts.c2r_script.archive_old_logger_files", side_effect=Mock()) +@patch("scripts.c2r_script.setup_logger_handler", side_effect=Mock()) # fmt: on # pylint: disable=too-many-locals def test_main_inhibited_c2r_installed_rollback_errors( + mock_setup_logger_handler, + mock_setup_sos_report, + mock_archive_old_logger_files, mock_rollback_inhibitor_check, mock_is_eligible_releases, mock_get_system_distro_version, @@ -401,6 +472,9 @@ def test_main_inhibited_c2r_installed_rollback_errors( assert "A rollback of changes performed by convert2rhel failed" in output assert '"alert": true' in output + assert mock_setup_logger_handler.call_count == 1 + assert mock_setup_sos_report.call_count == 1 + assert mock_archive_old_logger_files.call_count == 1 mock_rollback_inhibitor_check.assert_called_once() assert mock_get_system_distro_version.call_count == 1 assert mock_is_eligible_releases.call_count == 1 From 0b2112b9831ec8e9e054646b800071df4f6c91f6 Mon Sep 17 00:00:00 2001 From: Rodolfo Olivieri Date: Fri, 12 Apr 2024 09:54:07 -0300 Subject: [PATCH 3/5] Fix couple of logs --- scripts/c2r_script.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/scripts/c2r_script.py b/scripts/c2r_script.py index 3ab3f2f..9833499 100644 --- a/scripts/c2r_script.py +++ b/scripts/c2r_script.py @@ -112,12 +112,13 @@ def delete(self): def restore(self): """Restores file backup (rename). Returns True if restored, otherwise False.""" + file_path = os.path.join(self.path, self.backup_suffix) try: - logger.info("Restoring backed up file %s.", (self.path + self.backup_suffix)) - os.rename(self.path + self.backup_suffix, self.path) - print("File restored (%s)." % (self.path)) + logger.info("Restoring backed up file %s.", file_path) + os.rename(file_path, self.path) + logger.info("File restored (%s).", self.path) except OSError as err: - logger.warning("Failed to restore %s (%s)" % (self.path + self.backup_suffix, err)) + logger.warning("Failed to restore %s:\n %s", file_path, err) return False return True @@ -132,7 +133,7 @@ def backup(self): os.rename(self.path, self.path + self.backup_suffix) print("Back up created (%s)." % (self.path + self.backup_suffix)) except OSError as err: - logger.warning("Failed to create back up of %s (%s)" % (self.path, err)) + logger.warning("Failed to create back up of %s (%s)", self.path, err) return False return True @@ -616,7 +617,7 @@ def run_convert2rhel(): """ Run the convert2rhel tool assigning the correct environment variables. """ - logger.info("Running Convert2RHEL %s" % SCRIPT_TYPE.title()) + logger.info("Running Convert2RHEL %s", (SCRIPT_TYPE.title())) env = os.environ for key in env: From c74d419775ce240baf341c967550546730dd7c27 Mon Sep 17 00:00:00 2001 From: Andrea Waltlova Date: Tue, 16 Apr 2024 14:58:42 +0200 Subject: [PATCH 4/5] Fix restore function to use suffix instead of join Signed-off-by: Andrea Waltlova --- scripts/c2r_script.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/c2r_script.py b/scripts/c2r_script.py index 9833499..22df2b6 100644 --- a/scripts/c2r_script.py +++ b/scripts/c2r_script.py @@ -112,7 +112,7 @@ def delete(self): def restore(self): """Restores file backup (rename). Returns True if restored, otherwise False.""" - file_path = os.path.join(self.path, self.backup_suffix) + file_path = self.path + self.backup_suffix try: logger.info("Restoring backed up file %s.", file_path) os.rename(file_path, self.path) From 0b411e1a74d3a5c82cdbc5a37ab12fc8c93d3f6d Mon Sep 17 00:00:00 2001 From: Andrea Waltlova Date: Tue, 16 Apr 2024 16:39:44 +0200 Subject: [PATCH 5/5] Update test coverage apart from new logging functions Signed-off-by: Andrea Waltlova --- scripts/c2r_script.py | 2 +- tests/test_archive_report.py | 9 +++++++++ tests/test_cleanup.py | 2 +- tests/test_convert2rhel_inhibitors.py | 10 ++++++++++ tests/test_get_system_distro_version.py | 8 ++++++++ tests/test_logging.py | 16 ++++++++++++++++ tests/test_required_file.py | 12 ++++++++++-- 7 files changed, 55 insertions(+), 4 deletions(-) create mode 100644 tests/test_convert2rhel_inhibitors.py diff --git a/scripts/c2r_script.py b/scripts/c2r_script.py index 22df2b6..37832fa 100644 --- a/scripts/c2r_script.py +++ b/scripts/c2r_script.py @@ -379,7 +379,7 @@ def get_system_distro_version(): version_id = None with open("/etc/system-release", "r") as system_release_file: data = system_release_file.readline() - match = re.search(r"(.+?)\s?(?:release\s?)?\d", data) + match = re.search(r"(.+?)\s?(?:release\s?)", data) if match: # Split and get the first position, which will contain the system # name. diff --git a/tests/test_archive_report.py b/tests/test_archive_report.py index eb265bb..3704296 100644 --- a/tests/test_archive_report.py +++ b/tests/test_archive_report.py @@ -22,3 +22,12 @@ def test_archive_old_report(create_json_report_mock, tmpdir): archive_analysis_report(create_json_report_mock) assert len(os.listdir(tmp_archive_dir)) == 1 + + +@patch("scripts.c2r_script.os.path.exists", return_value=True) +@patch("scripts.c2r_script.shutil.move") +def test_archive_old_report_no_dir(mock_dir_exists, mock_move, create_json_report_mock): + archive_analysis_report(create_json_report_mock) + + mock_dir_exists.assert_called_once() + mock_move.assert_called_once() diff --git a/tests/test_cleanup.py b/tests/test_cleanup.py index 1c68ed4..e386e53 100644 --- a/tests/test_cleanup.py +++ b/tests/test_cleanup.py @@ -30,7 +30,7 @@ def test_cleanup_with_file_to_keep(mock_yum_undo): @patch("scripts.c2r_script.YUM_TRANSACTIONS_TO_UNDO", new=set([1])) -@patch("scripts.c2r_script.run_subprocess", return_value=("", 1)) +@patch("scripts.c2r_script.run_subprocess", return_value=("", 0)) def test_cleanup_with_undo_yum(mock_yum_undo): """Only downloaded files are removed.""" diff --git a/tests/test_convert2rhel_inhibitors.py b/tests/test_convert2rhel_inhibitors.py new file mode 100644 index 0000000..c95c9d9 --- /dev/null +++ b/tests/test_convert2rhel_inhibitors.py @@ -0,0 +1,10 @@ +from mock import patch +from scripts.c2r_script import check_convert2rhel_inhibitors_before_run + + +@patch("scripts.c2r_script.os.path.exists", return_value=False) +@patch("scripts.c2r_script._check_ini_file_modified", return_value=False) +def test_no_inhibitors(mock_ini_file_modified, mock_custom_ini): + check_convert2rhel_inhibitors_before_run() + mock_ini_file_modified.assert_called_once() + mock_custom_ini.assert_called_once() diff --git a/tests/test_get_system_distro_version.py b/tests/test_get_system_distro_version.py index ff3dcda..ab16bca 100644 --- a/tests/test_get_system_distro_version.py +++ b/tests/test_get_system_distro_version.py @@ -10,6 +10,14 @@ def test_get_system_distro_version_existing_file_centos(): assert version_id == "7.9" +@patch("__builtin__.open", mock_open(read_data="foo 7.9.0 (Core)\n")) +def test_get_version_no_matching_system_distro(): + distribution_id, version_id = get_system_distro_version() + + assert distribution_id is None + assert version_id == "7.9" + + @patch( "__builtin__.open", mock_open(read_data="Red Hat Enterprise Linux Server release 7.9 (Maipo)\n"), diff --git a/tests/test_logging.py b/tests/test_logging.py index f42a50f..b201e29 100644 --- a/tests/test_logging.py +++ b/tests/test_logging.py @@ -1,5 +1,6 @@ import os import logging +from mock import patch import pytest import scripts @@ -29,6 +30,21 @@ def test_setup_sos_report(monkeypatch, tmpdir): ) +@patch("scripts.c2r_script.os.makedirs") +@patch("scripts.c2r_script.os.path.exists", side_effect=[False, True]) +def test_setup_sos_report_no_sos_report_folder( + patch_exists, patch_makedirs, monkeypatch, tmpdir +): + sos_report_folder = str(tmpdir) + monkeypatch.setattr(scripts.c2r_script, "SOS_REPORT_FOLDER", sos_report_folder) + + setup_sos_report() + + # Folder created + assert patch_exists.call_count == 2 + patch_makedirs.assert_called_once_with(sos_report_folder) + + @pytest.mark.noautofixtures def test_setup_logger_handler(monkeypatch, tmpdir): log_dir = str(tmpdir) diff --git a/tests/test_required_file.py b/tests/test_required_file.py index 3ed4528..529f681 100644 --- a/tests/test_required_file.py +++ b/tests/test_required_file.py @@ -28,14 +28,22 @@ def test_create_host(required_file_instance): def test_create_data(required_file_instance): with patch("scripts.c2r_script.urlopen") as mock_urlopen, patch( + "scripts.c2r_script.os.path.dirname", return_value="/test" + ) as mock_dirname, patch( + "scripts.c2r_script.os.path.exists", return_value=True + ) as mock_exists, patch( "scripts.c2r_script.os.makedirs" - ) as mock_makedirs, patch("scripts.c2r_script.open") as mock_open, patch( + ) as mock_makedirs, patch( + "scripts.c2r_script.open" + ) as mock_open, patch( "scripts.c2r_script.os.chmod" ) as mock_chmod: required_file_instance.create_from_data(b"Mocked data") mock_urlopen.assert_not_called() - mock_makedirs.assert_called_once_with("/test", mode=0o755) + mock_dirname.assert_called_once() + mock_exists.assert_called_once() + mock_makedirs.assert_not_called() mock_open.assert_called_once_with("/test/path", mode="w") mock_chmod.assert_called_once_with("/test/path", 0o644)