From cf5e4ca3d7de4f420a2b975fa8bea80c12bca847 Mon Sep 17 00:00:00 2001 From: Alexander Graul Date: Wed, 21 Aug 2024 17:19:24 +0200 Subject: [PATCH 1/7] rhnLog: Also configure logging's root logger Since all loggers in the logging library inherit the config from their parents up to the root logger, we can use the root logger similarly to rhnLog's LOG object. The root logger singleton is changed to direct the log messages to the same log file with a similar log level as LOG. --- python/spacewalk/common/rhnLog.py | 40 +++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/python/spacewalk/common/rhnLog.py b/python/spacewalk/common/rhnLog.py index f66d71c824c7..f7efcc9b5a25 100644 --- a/python/spacewalk/common/rhnLog.py +++ b/python/spacewalk/common/rhnLog.py @@ -39,6 +39,7 @@ import atexit from spacewalk.common.rhnConfig import cfg_component +import logging from uyuni.common.fileutils import getUidGid @@ -91,6 +92,7 @@ def initLOG(log_file="stderr", level=0, component=""): if log_file is None or LOG.file == log_file: # Keep the same logging object, change only the log level LOG.level = level + align_root_logger() return # We need a different type, so destroy the old one LOG = None @@ -134,6 +136,7 @@ def initLOG(log_file="stderr", level=0, component=""): # At this point, LOG is None and log_file is not None # Get a new LOG LOG = rhnLog(log_file, level, component) + align_root_logger() return 0 @@ -306,6 +309,43 @@ def _exit(): atexit.register(_exit) +def log_level_to_logging_constant(rhnLog_log_level: int): + mapping = { + 0: logging.ERROR, + 1: logging.WARNING, + 2: logging.INFO, + 3: logging.DEBUG, + } + # 4+: logging.DEBUG + return mapping.get(rhnLog_log_level, logging.DEBUG) + + +def align_root_logger(): + """Align the root logger with LOG. + + Makes sure the root_logger has a single handler with the same destination as + LOG.file and a log level that corresponds to LOG.level + """ + # initLOG() not called or didn't finish correctly + if LOG is None or LOG.file is None or LOG.level is None: + return + + if LOG.file == "stderr": + handler = logging.StreamHandler(sys.stderr) + elif LOG.file == "stdout": + handler = logging.StreamHandler(stream=sys.stdout) + else: + handler = logging.FileHandler(filename=LOG.file) + + formatter = logging.Formatter( + fmt="%(asctime)s - %(name)s - %(message)s", datefmt="%Y/%m/%d %H:%M:%S" + ) + handler.setFormatter(formatter) + root_logger = logging.getLogger(None) + root_logger.handlers = [handler] + root_logger.setLevel(log_level_to_logging_constant(LOG.level)) + + # ------------------------------------------------------------------------------ if __name__ == "__main__": print("You can not run this module by itself") From b7a8e7f7a10da7dedd5234f347efadd2f7396393 Mon Sep 17 00:00:00 2001 From: Alexander Graul Date: Wed, 21 Aug 2024 17:32:43 +0200 Subject: [PATCH 2/7] reposync: Add more logging for debian repositories --- python/spacewalk/common/repo.py | 148 +++++++----------- .../satellite_tools/repo_plugins/deb_src.py | 14 +- 2 files changed, 68 insertions(+), 94 deletions(-) diff --git a/python/spacewalk/common/repo.py b/python/spacewalk/common/repo.py index 8fe2035663b7..fcac0e3e8bbe 100644 --- a/python/spacewalk/common/repo.py +++ b/python/spacewalk/common/repo.py @@ -17,11 +17,15 @@ import requests +from spacewalk.common import suseLib + # pylint:disable=W0612,W0212,C0301 SPACEWALK_LIB = "/var/lib/spacewalk" SPACEWALK_GPG_HOMEDIR = os.path.join(SPACEWALK_LIB, "gpgdir") +logger = logging.getLogger(__name__) + class GeneralRepoException(Exception): """ @@ -39,6 +43,7 @@ class DpkgRepo: PKG_GZ = "Packages.gz" PKG_XZ = "Packages.xz" PKG_RW = "Packages" + GPG_VERIFICATION_FAILED = "GPG verification failed" class ReleaseEntry: # pylint: disable=W0612,R0903 """ @@ -103,6 +108,13 @@ def __init__( self.gpg_verify = gpg_verify self.timeout = timeout + def __repr__(self): + return ( + f"DpkgRepo(url={suseLib.URL(self._url).getURL(stripPw=True)}, " + f"proxies={self.proxies}, gpg_verify={self.gpg_verify}, " + f"timemout={self.timeout})" + ) + def append_index_file(self, index_file: str) -> str: """ Append an index file, such as Packages.gz or Packagex.xz etc @@ -115,7 +127,7 @@ def append_index_file(self, index_file: str) -> str: path = p_url.path if not path.endswith(index_file): if index_file in path: - logging.error( + logger.error( # pylint: disable-next=logging-format-interpolation,consider-using-f-string "URL has already {} mentioned in it. Raising \ GeneralRepoException!".format( @@ -150,16 +162,15 @@ def get_pkg_index_raw(self) -> typing.Tuple[str, bytes]: for cnt_fname in [DpkgRepo.PKG_GZ, DpkgRepo.PKG_XZ, DpkgRepo.PKG_RW]: packages_url = self.append_index_file(cnt_fname) if packages_url.startswith("file://"): + local_path = packages_url.replace("file://", "") try: - with open(packages_url.replace("file://", ""), "rb") as f: + with open(local_path, "rb") as f: self._pkg_index = cnt_fname, f.read() break except FileNotFoundError as ex: - logging.debug( - # pylint: disable-next=logging-format-interpolation,consider-using-f-string - "File not found: {}".format( - packages_url.replace("file://", "") - ), + logger.debug( + "File not found: '%s'", + local_path, exc_info=True, ) else: @@ -189,22 +200,14 @@ def decompress_pkg_index(self) -> str: elif fname == DpkgRepo.PKG_XZ: cnt_data = lzma.decompress(cnt_data) except (zlib.error, lzma.LZMAError) as exc: - logging.exception( - "Exception during decompression of pkg index", exc_info=True - ) - # pylint: disable-next=raise-missing-from - raise GeneralRepoException(exc) + logger.exception("Could not decompress pkg index '%s'", fname) + raise GeneralRepoException from exc except Exception as exc: - logging.exception( - "Unknown exception during decompression of \ - pkg index. Raising GeneralRepoException", - exc_info=True, + logger.exception( + "Unhandled exception during decompression of pkg index '%s'.", fname ) raise GeneralRepoException( - # pylint: disable-next=consider-using-f-string - "Unhandled exception occurred while decompressing {}: {}".format( - fname, exc - ) + f"Unhandled exception during decompressing of pkg index '{fname}': {exc}" ) from exc return cnt_data.decode("utf-8") @@ -301,12 +304,9 @@ def _has_valid_gpg_signature(self, uri: str, response=None) -> bool: ) out = process.wait(timeout=90) else: - logging.error( - # pylint: disable-next=logging-format-interpolation,consider-using-f-string - "Signature file for GPG check could not be accessed: \ - '{}. Raising GeneralRepoException.".format( - release_signature_file - ) + logger.error( + "Could not access sinature file '%s' for GPG check.", + release_signature_file, ) raise GeneralRepoException( # pylint: disable-next=consider-using-f-string @@ -315,11 +315,8 @@ def _has_valid_gpg_signature(self, uri: str, response=None) -> bool: ) ) else: - logging.error( - # pylint: disable-next=logging-format-interpolation,consider-using-f-string - "No release file found: '{}'. Raising GeneralRepoException.".format( - uri - ) + logger.error( + "No release file found: '%s'. Raising GeneralRepoException.", uri ) # pylint: disable-next=consider-using-f-string raise GeneralRepoException("No release file found: {}".format(uri)) @@ -363,14 +360,11 @@ def _has_valid_gpg_signature(self, uri: str, response=None) -> bool: out = process.wait(timeout=90) if process.returncode == 0: - logging.debug("GPG signature is valid") + logger.debug("GPG signature is valid") return True else: - logging.debug( - # pylint: disable-next=logging-format-interpolation,consider-using-f-string - "GPG signature is invalid. gpg return code: {}".format( - process.returncode - ) + logger.debug( + "GPG signature is invalid. gpg return code: %s", process.returncode ) return False @@ -392,13 +386,8 @@ def get_release_index(self) -> typing.Dict[str, "DpkgRepo.ReleaseEntry"]: def _get_release_index_from_file(self) -> typing.Dict[str, "DpkgRepo.ReleaseEntry"]: # InRelease files take precedence per uyuni-rfc 00057-deb-repo-sync-gpg-check - logging.debug( - # pylint: disable-next=logging-format-interpolation,consider-using-f-string - "Fetching release file from local filesystem: {}".format( - self._url.replace("file://", "") - ) - ) local_path = self._url.replace("file://", "") + logger.debug("Reading release file from local filesystem: '%s'", local_path) release_file = None if os.access(self._get_parent_url(local_path, 2, "InRelease"), os.R_OK): release_file = self._get_parent_url(local_path, 2, "InRelease") @@ -415,24 +404,18 @@ def _get_release_index_from_file(self) -> typing.Dict[str, "DpkgRepo.ReleaseEntr # Repo format is not flat if not self.is_flat(): if self.gpg_verify and not self._has_valid_gpg_signature(local_path): - # pylint: disable-next=logging-format-interpolation,consider-using-f-string - logging.error("GPG verification failed: {}".format(release_file)) - logging.error("Raising GeneralRepoException!") + logger.error("%s: %s", DpkgRepo.GPG_VERIFICATION_FAILED, release_file) + logger.error("Raising GeneralRepoException!") raise GeneralRepoException( - # pylint: disable-next=consider-using-f-string - "GPG verification failed: {}".format(release_file) + f"{DpkgRepo.GPG_VERIFICATION_FAILED}: {release_file}" ) try: with open(release_file, "rb") as f: self._release = self._parse_release_index(f.read().decode("utf-8")) except IOError as ex: - logging.exception( - # pylint: disable-next=logging-format-interpolation,consider-using-f-string - "IOError while accessing file: '{}'. Raising \ - GeneralRepoException!".format( - release_file - ), - exc_info=True, + logger.exception( + "IOError while accessing file: '%s'. Raising GeneralRepoException!", + release_file, ) raise GeneralRepoException( # pylint: disable-next=consider-using-f-string @@ -446,12 +429,9 @@ def _get_release_index_from_file(self) -> typing.Dict[str, "DpkgRepo.ReleaseEntr elif os.access(self._get_parent_url(local_path, 0, "Release"), os.R_OK): release_file = self._get_parent_url(local_path, 0, "Release") else: - logging.error( - # pylint: disable-next=logging-format-interpolation,consider-using-f-string - "No release file found in '{}'. Raising \ - GeneralRepoException.".format( - self._get_parent_url(local_path, 0) - ) + logger.error( + "No release file found in '%s'. Raising GeneralRepoException.", + self._get_parent_url(local_path, 0), ) raise GeneralRepoException( # pylint: disable-next=consider-using-f-string @@ -466,26 +446,19 @@ def _get_release_index_from_file(self) -> typing.Dict[str, "DpkgRepo.ReleaseEntr if self.gpg_verify and not self._has_valid_gpg_signature( local_path ): - logging.error( - # pylint: disable-next=logging-format-interpolation,consider-using-f-string - "GPG verification failed: '{}'. \ - Raising GeneralRepoException.".format( - release_file - ) + logger.error( + "%s: %s. Raising GeneralRepoException.", + DpkgRepo.GPG_VERIFICATION_FAILED, + release_file, ) raise GeneralRepoException( - # pylint: disable-next=consider-using-f-string - "GPG verification failed: {}".format(release_file) + f"{DpkgRepo.GPG_VERIFICATION_FAILED}: {release_file}" ) self._release = self._parse_release_index(release_file_content) except IOError as ex: - logging.exception( - # pylint: disable-next=logging-format-interpolation,consider-using-f-string - "IOError while accessing file: '{}'. Raising \ - GeneralRepoException.".format( - release_file - ), - exc_info=True, + logger.exception( + "IOError while accessing file: '%s'. Raising GeneralRepoException.", + release_file, ) raise GeneralRepoException( # pylint: disable-next=consider-using-f-string @@ -496,8 +469,7 @@ def _get_release_index_from_file(self) -> typing.Dict[str, "DpkgRepo.ReleaseEntr def _get_release_index_from_http(self) -> typing.Dict[str, "DpkgRepo.ReleaseEntry"]: # InRelease files take precedence per uyuni-rfc 00057-deb-repo-sync-gpg-check - # pylint: disable-next=logging-format-interpolation,consider-using-f-string - logging.debug("Fetching release file from local http: {}".format(self._url)) + logger.debug("Fetching release file from local http: %s", self._url) resp = requests.get( self._get_parent_url(self._url, 2, "InRelease"), proxies=self.proxies, @@ -516,12 +488,10 @@ def _get_release_index_from_http(self) -> typing.Dict[str, "DpkgRepo.ReleaseEntr http.HTTPStatus.OK, http.HTTPStatus.FORBIDDEN, ]: - logging.error( - # pylint: disable-next=logging-format-interpolation,consider-using-f-string - "Fetching release index failed with http status \ - '{}'. Raising GeneralRepoException.".format( - resp.status_code - ) + logger.error( + "Fetching release index failed with http status '%s'. Raising " + "GeneralRepoException.", + resp.status_code, ) raise GeneralRepoException( # pylint: disable-next=consider-using-f-string @@ -541,12 +511,11 @@ def _get_release_index_from_http(self) -> typing.Dict[str, "DpkgRepo.ReleaseEntr and self.gpg_verify and not self._has_valid_gpg_signature(resp.url, resp) ): - logging.error( + logger.error( "Repo has no valid GPG signature. Raising GeneralRepoException." ) raise GeneralRepoException( - # pylint: disable-next=consider-using-f-string - "GPG verification failed: {}".format(resp.url) + f"{DpkgRepo.GPG_VERIFICATION_FAILED}: {resp.url}" ) self._release = self._parse_release_index(resp.content.decode("utf-8")) @@ -568,12 +537,11 @@ def _get_release_index_from_http(self) -> typing.Dict[str, "DpkgRepo.ReleaseEntr if self.gpg_verify and not self._has_valid_gpg_signature( resp.url, resp ): - logging.error( + logger.error( "Repo has no valid GPG signature. GeneralRepoException will be raised!" ) raise GeneralRepoException( - # pylint: disable-next=consider-using-f-string - "GPG verification failed: {}".format(resp.url) + f"{DpkgRepo.GPG_VERIFICATION_FAILED}: {resp.url}" ) self._release = self._parse_release_index( resp.content.decode("utf-8") diff --git a/python/spacewalk/satellite_tools/repo_plugins/deb_src.py b/python/spacewalk/satellite_tools/repo_plugins/deb_src.py index a2c50f439210..451e1f732318 100644 --- a/python/spacewalk/satellite_tools/repo_plugins/deb_src.py +++ b/python/spacewalk/satellite_tools/repo_plugins/deb_src.py @@ -217,9 +217,11 @@ def verify(self): :return: """ - if not repo.DpkgRepo( + dpkg_repo = repo.DpkgRepo( self.url, self._get_proxies(), self.gpg_verify, self.timeout - ).verify_packages_index(): + ) + log.debug("DebRepo.verify() dpkg_repo=%s", dpkg_repo) + if not dpkg_repo.verify_packages_index(): raise repo.GeneralRepoException("Package index checksum failure") def _get_proxies(self): @@ -469,6 +471,7 @@ def get_mediaproducts(self): def list_packages(self, filters, latest): """list packages""" + log.debug("ContentSource.list_packages(filters=%s, latest=%s)", filters, latest) pkglist = self.repo.get_package_list() self.num_packages = len(pkglist) if latest: @@ -505,8 +508,11 @@ def list_packages(self, filters, latest): pack.name, pack.version, pack.release, pack.epoch, pack.arch ) except ValueError as e: - log(0, "WARNING: package contains incorrect metadata. SKIPPING!") - log(0, e) + log.error( + "Skipping package %s. Package contains incorrect metadata.\n%s", + new_pack, + e, + ) continue new_pack.unique_id = pack new_pack.checksum_type = pack.checksum_type From 63d86c8df4aadb52eedf2f60b2b2da7ac44c2a56 Mon Sep 17 00:00:00 2001 From: Alexander Graul Date: Thu, 22 Aug 2024 13:34:11 +0200 Subject: [PATCH 3/7] reposync: Use log_level fallback from rhn.conf --- python/spacewalk/satellite_tools/spacewalk-repo-sync | 11 +++++++++-- ...spacewalk-backend.changes.meaksh.master-bsc1227859 | 1 + 2 files changed, 10 insertions(+), 2 deletions(-) create mode 100644 python/spacewalk/spacewalk-backend.changes.meaksh.master-bsc1227859 diff --git a/python/spacewalk/satellite_tools/spacewalk-repo-sync b/python/spacewalk/satellite_tools/spacewalk-repo-sync index 7ad9f8dd5e0d..43b30df165bb 100755 --- a/python/spacewalk/satellite_tools/spacewalk-repo-sync +++ b/python/spacewalk/satellite_tools/spacewalk-repo-sync @@ -43,6 +43,7 @@ def systemExit(code, msg=None): try: from rhn import rhnLockfile + from uyuni.common.context_managers import cfg_component from spacewalk.common import rhnLog from spacewalk.common.rhnConfig import CFG, initCFG from spacewalk.satellite_tools import reposync @@ -134,11 +135,17 @@ def main(): log_level = options.verbose if log_level is None: - log_level = 0 + # if no -v flag is passed, use "debug" setting from rhn.conf + with cfg_component(None) as cfg: + log_level = cfg.get("DEBUG", 0) + # Apparently we need to call initCFG and have CFG available + # to prevent some errors accessing CFG later on during package + # import. initCFG('server.satellite') CFG.set('DEBUG', log_level) CFG.set("TRACEBACK_MAIL", options.traceback_mail or CFG.TRACEBACK_MAIL) + if options.email: initEMAIL_LOG() rhnLog.initLOG(log_path, log_level) @@ -265,7 +272,7 @@ def main(): no_packages=options.no_packages, sync_kickstart=options.sync_kickstart, latest=options.latest, - log_level=options.verbose, + log_level=log_level, force_all_errata=options.force_all_errata, show_packages_only=options.show_packages) if options.batch_size: sync.set_import_batch_size(options.batch_size) diff --git a/python/spacewalk/spacewalk-backend.changes.meaksh.master-bsc1227859 b/python/spacewalk/spacewalk-backend.changes.meaksh.master-bsc1227859 new file mode 100644 index 000000000000..2a67aaf8c38f --- /dev/null +++ b/python/spacewalk/spacewalk-backend.changes.meaksh.master-bsc1227859 @@ -0,0 +1 @@ +- Improve debian reposync logging (bsc#1227859) From 98aa5188e8d4ed4d9078ca0becf42f766287b9b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Su=C3=A1rez=20Hern=C3=A1ndez?= Date: Fri, 14 Feb 2025 12:04:51 +0100 Subject: [PATCH 4/7] Partially format spacewalk-repo-sync --- python/linting/lint.sh | 12 +- .../satellite_tools/spacewalk-repo-sync | 379 ++++++++++++------ 2 files changed, 272 insertions(+), 119 deletions(-) diff --git a/python/linting/lint.sh b/python/linting/lint.sh index 24a87ebe6a2d..495b3a388abf 100755 --- a/python/linting/lint.sh +++ b/python/linting/lint.sh @@ -56,14 +56,22 @@ function execute_lint() { } function get_all_py_files() { - # Filter added, copied, modified, and renamed files + # Filter added, copied, modified, and renamed Python files echo "$(git diff --name-only --diff-filter=ACMR HEAD)" | grep '\.py' | tr '\n' ' ' } +function get_all_files_with_python_shebang() { + # Filter added, copied, modified, and renamed files containing Python shebang on it + LIST_OF_FILES=$(git diff --name-only --diff-filter=ACMR HEAD) + if [ ! -z "$LIST_OF_FILES" ]; then + egrep '^#!/usr/bin/python|^#!/usr/bin/env python' $LIST_OF_FILES | cut -d":" -f1 | sort -u | tr '\n' ' ' + fi +} + function main() { ensure_latest_container_image if [[ "${CHECK_ALL_FILES}" == "true" ]]; then - files="$(get_all_py_files)" + files="$(get_all_py_files)$(get_all_files_with_python_shebang)" echo "Linting and formatting: $files" execute_black $files execute_lint $files diff --git a/python/spacewalk/satellite_tools/spacewalk-repo-sync b/python/spacewalk/satellite_tools/spacewalk-repo-sync index 43b30df165bb..0dd74af2fca7 100755 --- a/python/spacewalk/satellite_tools/spacewalk-repo-sync +++ b/python/spacewalk/satellite_tools/spacewalk-repo-sync @@ -1,4 +1,5 @@ #!/usr/bin/python3 -u +# pylint: disable=missing-module-docstring,invalid-name # # Copyright (c) 2008--2017 Red Hat, Inc. # Copyright (c) 2011 SUSE LLC @@ -16,16 +17,13 @@ # import re -try: - # python2 - import StringIO as StringIO -except ImportError: - # python3 - import io as StringIO + +import io as StringIO import json -import shutil import sys import os + +# pylint: disable-next=deprecated-module from optparse import OptionParser import datetime @@ -33,17 +31,20 @@ from spacewalk.satellite_tools.syncLib import initEMAIL_LOG LOCK = None -log_path = '/var/log/rhn/reposync.log' +log_path = "/var/log/rhn/reposync.log" def systemExit(code, msg=None): "Exit with a code and optional message(s). Saved a few lines of code." - sys.stderr.write(str(msg)+'\n') + sys.stderr.write(str(msg) + "\n") sys.exit(code) + try: from rhn import rhnLockfile from uyuni.common.context_managers import cfg_component + + # pylint: disable-next=ungrouped-imports from spacewalk.common import rhnLog from spacewalk.common.rhnConfig import CFG, initCFG from spacewalk.satellite_tools import reposync @@ -51,8 +52,7 @@ try: except KeyboardInterrupt: systemExit(0, "\nUser interrupted process.") except ImportError: - systemExit(1, "Unable to find code tree.\n" - "Path not correct? " + str(sys.path)) + systemExit(1, "Unable to find code tree.\n" "Path not correct? " + str(sys.path)) def releaseLOCK(): @@ -66,72 +66,189 @@ def main(): # quick check to see if you are a super-user. if os.getuid() != 0: - systemExit(8, 'ERROR: must be root to execute.') - + systemExit(8, "ERROR: must be root to execute.") parser = OptionParser() - parser.add_option('-l', '--list', action='store_true', dest='list', - help='List the custom channels with the associated repositories.') - parser.add_option('-s', '--show-packages', action='store_true', dest='show_packages', - help='List all packages in a specified channel.') - parser.add_option('-u', '--url', action='append', dest='url', - default=[], help='The url of the repository. Can be used multiple times.') - parser.add_option('-c', '--channel', action='append', - dest='channel_label', - help='The label of the channel to sync packages to. Can be used multiple times.') - parser.add_option('-p', '--parent-channel', action='append', - dest='parent_label', default=[], - help='Synchronize the parent channel and all its child channels.') - parser.add_option('-d', '--dry-run', action='store_true', - dest='dry_run', - help='Test run. No sync takes place.') - parser.add_option('--latest', action='store_true', - dest='latest', - help='Sync latest packages only. Use carefully - you might need to fix some dependencies on your own.') - parser.add_option('-g', '--config', action='store', dest='config', - help='Configuration file') - parser.add_option('-t', '--type', action='store', dest='repo_type', - help='Force type of repository ("yum", "uln" and "deb" are supported)') - parser.add_option('-f', '--fail', action='store_true', dest='fail', - default=False, - help="If a package import fails, fail the entire operation") - parser.add_option('-n', '--non-interactive', action='store_true', - dest='noninteractive', default=False, - help="Do not ask anything, use default answers") - parser.add_option('-i', '--include', action='callback', - callback=reposync.set_filter_opt, type='str', nargs=1, - dest='filters', default=[], - help="Comma or space separated list of included packages or package groups.") - parser.add_option('-e', '--exclude', action='callback', - callback=reposync.set_filter_opt, - type='str', nargs=1, dest='filters', default=[], - help="Comma or space separated list of excluded packages or package groups.") - parser.add_option('', '--no-strict', action='store_true',help="do not unlink packages when deleted from repository", dest='no_strict') - parser.add_option('', '--email', action="store_true", help="e-mail a report of what was synced/imported") - parser.add_option('', '--traceback-mail', action="store", - help="alternative email address(es) for sync output (--email option)") - parser.add_option('', '--no-errata', action='store_true', dest='no_errata', - default=False, help="Do not sync errata") - parser.add_option('', '--no-packages', action='store_true', dest='no_packages', - default=False, help="Do not sync packages") - parser.add_option('', '--sync-kickstart', action='store_true', dest='sync_kickstart', - default=False, help="Sync kickstartable tree") - parser.add_option('', '--force-all-errata', action='store_true', dest='force_all_errata', - default=False, help="Process metadata of all errata, not only missing.") - parser.add_option('', '--batch-size', action='store', help="max. batch size for package import (debug only)") - parser.add_option('-Y', '--deep-verify', action='store_true', - dest='deep_verify', default=False, - help='Do not use cached package checksums') - parser.add_option('-v', '--verbose', action='count', - help="Verbose output. Possible to accumulate: -vvv") - (options, args) = parser.parse_args() + parser.add_option( + "-l", + "--list", + action="store_true", + dest="list", + help="List the custom channels with the associated repositories.", + ) + parser.add_option( + "-s", + "--show-packages", + action="store_true", + dest="show_packages", + help="List all packages in a specified channel.", + ) + parser.add_option( + "-u", + "--url", + action="append", + dest="url", + default=[], + help="The url of the repository. Can be used multiple times.", + ) + parser.add_option( + "-c", + "--channel", + action="append", + dest="channel_label", + help="The label of the channel to sync packages to. Can be used multiple times.", + ) + parser.add_option( + "-p", + "--parent-channel", + action="append", + dest="parent_label", + default=[], + help="Synchronize the parent channel and all its child channels.", + ) + parser.add_option( + "-d", + "--dry-run", + action="store_true", + dest="dry_run", + help="Test run. No sync takes place.", + ) + parser.add_option( + "--latest", + action="store_true", + dest="latest", + help="Sync latest packages only. Use carefully - you might need to fix some dependencies on your own.", + ) + parser.add_option( + "-g", "--config", action="store", dest="config", help="Configuration file" + ) + parser.add_option( + "-t", + "--type", + action="store", + dest="repo_type", + help='Force type of repository ("yum", "uln" and "deb" are supported)', + ) + parser.add_option( + "-f", + "--fail", + action="store_true", + dest="fail", + default=False, + help="If a package import fails, fail the entire operation", + ) + parser.add_option( + "-n", + "--non-interactive", + action="store_true", + dest="noninteractive", + default=False, + help="Do not ask anything, use default answers", + ) + parser.add_option( + "-i", + "--include", + action="callback", + callback=reposync.set_filter_opt, + type="str", + nargs=1, + dest="filters", + default=[], + help="Comma or space separated list of included packages or package groups.", + ) + parser.add_option( + "-e", + "--exclude", + action="callback", + callback=reposync.set_filter_opt, + type="str", + nargs=1, + dest="filters", + default=[], + help="Comma or space separated list of excluded packages or package groups.", + ) + parser.add_option( + "", + "--no-strict", + action="store_true", + help="do not unlink packages when deleted from repository", + dest="no_strict", + ) + parser.add_option( + "", + "--email", + action="store_true", + help="e-mail a report of what was synced/imported", + ) + parser.add_option( + "", + "--traceback-mail", + action="store", + help="alternative email address(es) for sync output (--email option)", + ) + parser.add_option( + "", + "--no-errata", + action="store_true", + dest="no_errata", + default=False, + help="Do not sync errata", + ) + parser.add_option( + "", + "--no-packages", + action="store_true", + dest="no_packages", + default=False, + help="Do not sync packages", + ) + parser.add_option( + "", + "--sync-kickstart", + action="store_true", + dest="sync_kickstart", + default=False, + help="Sync kickstartable tree", + ) + parser.add_option( + "", + "--force-all-errata", + action="store_true", + dest="force_all_errata", + default=False, + help="Process metadata of all errata, not only missing.", + ) + parser.add_option( + "", + "--batch-size", + action="store", + help="max. batch size for package import (debug only)", + ) + parser.add_option( + "-Y", + "--deep-verify", + action="store_true", + dest="deep_verify", + default=False, + help="Do not use cached package checksums", + ) + parser.add_option( + "-v", + "--verbose", + action="count", + help="Verbose output. Possible to accumulate: -vvv", + ) + (options, _) = parser.parse_args() global LOCK try: - LOCK = rhnLockfile.Lockfile('/run/spacewalk-repo-sync.pid') + LOCK = rhnLockfile.Lockfile("/run/spacewalk-repo-sync.pid") except rhnLockfile.LockfileLockedException: - systemExit(1, "ERROR: attempting to run more than one instance of " - "spacewalk-repo-sync Exiting.") + systemExit( + 1, + "ERROR: attempting to run more than one instance of " + "spacewalk-repo-sync Exiting.", + ) log_level = options.verbose if log_level is None: @@ -142,21 +259,22 @@ def main(): # Apparently we need to call initCFG and have CFG available # to prevent some errors accessing CFG later on during package # import. - initCFG('server.satellite') - CFG.set('DEBUG', log_level) + initCFG("server.satellite") + CFG.set("DEBUG", log_level) CFG.set("TRACEBACK_MAIL", options.traceback_mail or CFG.TRACEBACK_MAIL) if options.email: initEMAIL_LOG() rhnLog.initLOG(log_path, log_level) + # pylint: disable-next=consider-using-f-string log2disk(0, "Command: %s" % str(sys.argv)) - l_params=["no_errata", "sync_kickstart", "fail", "no-strict"] - d_chan_repo=reposync.getChannelRepo() - l_ch_custom=reposync.getCustomChannels() - d_parent_child=reposync.getParentsChilds() - d_ch_repo_sync={} - l_no_ch_repo_sync=[] + l_params = ["no_errata", "sync_kickstart", "fail", "no-strict"] + d_chan_repo = reposync.getChannelRepo() + l_ch_custom = reposync.getCustomChannels() + d_parent_child = reposync.getParentsChilds() + d_ch_repo_sync = {} + l_no_ch_repo_sync = [] if options.list: log(0, "======================================") @@ -164,8 +282,10 @@ def main(): log(0, "======================================") for ch in list(set(l_ch_custom) & set(d_chan_repo)): for repo in d_chan_repo[ch]: - log(0, "%s | %s" %(ch,repo)) - for ch in list(set(l_ch_custom)-set(d_chan_repo)): + # pylint: disable-next=consider-using-f-string + log(0, "%s | %s" % (ch, repo)) + for ch in list(set(l_ch_custom) - set(d_chan_repo)): + # pylint: disable-next=consider-using-f-string log(0, "%s | No repository set" % ch) return 0 @@ -174,49 +294,60 @@ def main(): if options.config: try: + # pylint: disable-next=unspecified-encoding config_file = open(options.config).read() # strip all whitespace - config_file = re.sub(r'\s', '', config_file) + config_file = re.sub(r"\s", "", config_file) config = json.load(StringIO.StringIO(config_file)) + # pylint: disable-next=broad-exception-caught except Exception as e: - systemExit(1, "Configuration file is invalid, please check syntax. Error [%s]" % e ) + systemExit( + 1, + # pylint: disable-next=consider-using-f-string + "Configuration file is invalid, please check syntax. Error [%s]" % e, + ) for key in l_params: if key in config and not getattr(options, key): setattr(options, key, config[key]) # Channels - if 'channel' in config: - for ch,repo in config['channel'].items(): - if not isinstance(repo, list): + if "channel" in config: + for ch, repo in config["channel"].items(): + if not isinstance(repo, list): systemExit( 1, + # pylint: disable-next=consider-using-f-string "Configuration file is invalid, " - "{0}'s value needs to be a list.".format(ch) + "{0}'s value needs to be a list.".format(ch), ) - d_ch_repo_sync[ch]=repo + d_ch_repo_sync[ch] = repo - if 'parent_channel' in config: - options.parent_label+=config['parent_channel'] + if "parent_channel" in config: + options.parent_label += config["parent_channel"] - if options.channel_label and len(options.channel_label)>0: + if options.channel_label and len(options.channel_label) > 0: for channel in options.channel_label: - d_ch_repo_sync[channel]=options.url + d_ch_repo_sync[channel] = options.url if options.parent_label: for pch in options.parent_label: if pch in d_parent_child: - for ch in [pch]+d_parent_child[pch]: + for ch in [pch] + d_parent_child[pch]: if ch in l_ch_custom and ch not in d_ch_repo_sync: - d_ch_repo_sync[ch]=[] + d_ch_repo_sync[ch] = [] else: + # pylint: disable-next=consider-using-f-string systemExit(1, "Channel %s is not custom base channel." % pch) + # pylint: disable-next=consider-using-dict-items for ch in d_ch_repo_sync: if ch not in l_ch_custom: + # pylint: disable-next=consider-using-f-string systemExit(1, "Channel %s is not custom or does not exist." % ch) if not d_ch_repo_sync[ch] and not ch in d_chan_repo: + # pylint: disable-next=consider-using-f-string log(0, "Channel %s Channel has no URL associated, skipping sync" % ch) l_no_ch_repo_sync.append(ch) @@ -228,17 +359,20 @@ def main(): log(0, "| Channel Label | Repository |") log(0, "======================================") - for ch,repo in list(d_ch_repo_sync.items()): + for ch, repo in list(d_ch_repo_sync.items()): if repo: - log(0, " %s : %s" % (ch,", ".join(repo))) + # pylint: disable-next=consider-using-f-string + log(0, " %s : %s" % (ch, ", ".join(repo))) else: - log(0, " %s : %s" % (ch,", ".join(d_chan_repo[ch]))) + # pylint: disable-next=consider-using-f-string + log(0, " %s : %s" % (ch, ", ".join(d_chan_repo[ch]))) log(0, "======================================") log(0, "| Parameters |") log(0, "======================================") - for key in l_params: - log(0, " %s: %s" % (key,str(getattr(options, key)))) + for key in l_params: + # pylint: disable-next=consider-using-f-string + log(0, " %s: %s" % (key, str(getattr(options, key)))) return 0 if options.batch_size: @@ -247,33 +381,43 @@ def main(): if batch_size <= 0: raise ValueError() except ValueError: + # pylint: disable-next=consider-using-f-string systemExit(1, "Invalid batch size: %s" % options.batch_size) reposync.clear_ssl_cache() total_time = datetime.timedelta() ret_code = 0 - for ch,repo in list(d_ch_repo_sync.items()): + for ch, repo in list(d_ch_repo_sync.items()): log(0, "======================================") + # pylint: disable-next=consider-using-f-string log(0, "| Channel: %s" % ch) log(0, "======================================") log(0, "Sync of channel started.") - log2disk(0, "Please check 'reposync/%s.log' for sync log of this channel." % ch, notimeYN=True) - sync = reposync.RepoSync(channel_label=ch, - repo_type=options.repo_type, - url=repo, - fail=options.fail, - strict= not options.no_strict, - noninteractive=options.noninteractive, - filters=options.filters, - deep_verify=options.deep_verify, - no_errata=options.no_errata, - no_packages=options.no_packages, - sync_kickstart=options.sync_kickstart, - latest=options.latest, - log_level=log_level, - force_all_errata=options.force_all_errata, show_packages_only=options.show_packages) + log2disk( + 0, + # pylint: disable-next=consider-using-f-string + "Please check 'reposync/%s.log' for sync log of this channel." % ch, + notimeYN=True, + ) + sync = reposync.RepoSync( + channel_label=ch, + repo_type=options.repo_type, + url=repo, + fail=options.fail, + strict=not options.no_strict, + noninteractive=options.noninteractive, + filters=options.filters, + deep_verify=options.deep_verify, + no_errata=options.no_errata, + no_packages=options.no_packages, + sync_kickstart=options.sync_kickstart, + latest=options.latest, + log_level=log_level, + force_all_errata=options.force_all_errata, + show_packages_only=options.show_packages, + ) if options.batch_size: sync.set_import_batch_size(options.batch_size) elapsed_time, channel_ret_code = sync.sync() @@ -284,14 +428,15 @@ def main(): rhnLog.initLOG(log_path, log_level) log2disk(0, "Sync of channel completed.") - log(0, "Total time: %s" % str(total_time).split('.')[0]) + # pylint: disable-next=consider-using-f-string,use-maxsplit-arg + log(0, "Total time: %s" % str(total_time).split(".")[0]) if options.email: reposync.send_mail() releaseLOCK() return ret_code -if __name__ == '__main__': +if __name__ == "__main__": try: sys.exit(abs(main() or 0)) except KeyboardInterrupt: From 2234c66e9ee99f5be8d11d33667f40cd4e9d0216 Mon Sep 17 00:00:00 2001 From: Alexander Graul Date: Tue, 25 Feb 2025 16:30:56 +0100 Subject: [PATCH 5/7] fixup! reposync: Add more logging for debian repositories --- python/test/unit/spacewalk/common/test_repo.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/python/test/unit/spacewalk/common/test_repo.py b/python/test/unit/spacewalk/common/test_repo.py index a979c9109470..0661fcab0ac8 100644 --- a/python/test/unit/spacewalk/common/test_repo.py +++ b/python/test/unit/spacewalk/common/test_repo.py @@ -502,7 +502,7 @@ def test_decompress_pkg_index_gz_general_failure(self): err = str(exc.value) assert err.startswith( - "Unhandled exception occurred while decompressing Packages.gz:" + "Unhandled exception during decompressing of pkg index 'Packages.gz':" ) assert "symlinks" in err @@ -534,7 +534,7 @@ def test_decompress_pkg_index_xz_general_failure(self): err = str(exc.value) assert err.startswith( - "Unhandled exception occurred while decompressing Packages.xz:" + "Unhandled exception during decompressing of pkg index 'Packages.xz':" ) assert "Software" in err @@ -563,7 +563,11 @@ def test_decompress_pkg_index_xz_failure(self): assert not zdcmp.called assert xdcmp.called - assert "/dev/null" in str(exc.value) + # exc.value is the exception object. The code uses + # raise GeneralRepoException from exc + # this `from exc` sets the original exception as the + # __cause__ of the GeneralRepoException + assert "/dev/null" in str(exc.value.__cause__) @patch( "spacewalk.common.repo.DpkgRepo.get_pkg_index_raw", @@ -588,7 +592,11 @@ def test_decompress_pkg_index_gz_failure(self): assert not xdcmp.called assert zdcmp.called - assert "hot" in str(exc.value) + # exc.value is the exception object. The code uses + # raise GeneralRepoException from exc + # this `from exc` sets the original exception as the + # __cause__ of the GeneralRepoException + assert "hot" in str(exc.value.__cause__) def test_append_index_file_to_url(self): """ From 14467671bf966889be79430378a1775f249c8747 Mon Sep 17 00:00:00 2001 From: Alexander Graul Date: Tue, 25 Feb 2025 16:36:30 +0100 Subject: [PATCH 6/7] fixup! reposync: Add more logging for debian repositories --- python/spacewalk/common/repo.py | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/python/spacewalk/common/repo.py b/python/spacewalk/common/repo.py index fcac0e3e8bbe..b30a6f47a8a0 100644 --- a/python/spacewalk/common/repo.py +++ b/python/spacewalk/common/repo.py @@ -315,9 +315,7 @@ def _has_valid_gpg_signature(self, uri: str, response=None) -> bool: ) ) else: - logger.error( - "No release file found: '%s'. Raising GeneralRepoException.", uri - ) + logger.error("No release file found: '%s'.", uri) # pylint: disable-next=consider-using-f-string raise GeneralRepoException("No release file found: {}".format(uri)) else: @@ -405,7 +403,6 @@ def _get_release_index_from_file(self) -> typing.Dict[str, "DpkgRepo.ReleaseEntr if not self.is_flat(): if self.gpg_verify and not self._has_valid_gpg_signature(local_path): logger.error("%s: %s", DpkgRepo.GPG_VERIFICATION_FAILED, release_file) - logger.error("Raising GeneralRepoException!") raise GeneralRepoException( f"{DpkgRepo.GPG_VERIFICATION_FAILED}: {release_file}" ) @@ -414,7 +411,7 @@ def _get_release_index_from_file(self) -> typing.Dict[str, "DpkgRepo.ReleaseEntr self._release = self._parse_release_index(f.read().decode("utf-8")) except IOError as ex: logger.exception( - "IOError while accessing file: '%s'. Raising GeneralRepoException!", + "IOError while accessing file: '%s'.", release_file, ) raise GeneralRepoException( @@ -430,7 +427,7 @@ def _get_release_index_from_file(self) -> typing.Dict[str, "DpkgRepo.ReleaseEntr release_file = self._get_parent_url(local_path, 0, "Release") else: logger.error( - "No release file found in '%s'. Raising GeneralRepoException.", + "No release file found in '%s'.", self._get_parent_url(local_path, 0), ) raise GeneralRepoException( @@ -447,7 +444,7 @@ def _get_release_index_from_file(self) -> typing.Dict[str, "DpkgRepo.ReleaseEntr local_path ): logger.error( - "%s: %s. Raising GeneralRepoException.", + "%s: '%s'.", DpkgRepo.GPG_VERIFICATION_FAILED, release_file, ) @@ -457,7 +454,7 @@ def _get_release_index_from_file(self) -> typing.Dict[str, "DpkgRepo.ReleaseEntr self._release = self._parse_release_index(release_file_content) except IOError as ex: logger.exception( - "IOError while accessing file: '%s'. Raising GeneralRepoException.", + "IOError while accessing file: '%s'.", release_file, ) raise GeneralRepoException( @@ -489,8 +486,7 @@ def _get_release_index_from_http(self) -> typing.Dict[str, "DpkgRepo.ReleaseEntr http.HTTPStatus.FORBIDDEN, ]: logger.error( - "Fetching release index failed with http status '%s'. Raising " - "GeneralRepoException.", + "Fetching release index failed with http status '%s'.", resp.status_code, ) raise GeneralRepoException( @@ -511,9 +507,7 @@ def _get_release_index_from_http(self) -> typing.Dict[str, "DpkgRepo.ReleaseEntr and self.gpg_verify and not self._has_valid_gpg_signature(resp.url, resp) ): - logger.error( - "Repo has no valid GPG signature. Raising GeneralRepoException." - ) + logger.error("Repo has no valid GPG signature.") raise GeneralRepoException( f"{DpkgRepo.GPG_VERIFICATION_FAILED}: {resp.url}" ) From 36aebab6a446ea164233c82760b3b7ce44ce8270 Mon Sep 17 00:00:00 2001 From: Alexander Graul Date: Tue, 25 Feb 2025 16:36:45 +0100 Subject: [PATCH 7/7] fixup! Partially format spacewalk-repo-sync --- python/linting/lint.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/linting/lint.sh b/python/linting/lint.sh index 495b3a388abf..45d8490edb5c 100755 --- a/python/linting/lint.sh +++ b/python/linting/lint.sh @@ -71,7 +71,7 @@ function get_all_files_with_python_shebang() { function main() { ensure_latest_container_image if [[ "${CHECK_ALL_FILES}" == "true" ]]; then - files="$(get_all_py_files)$(get_all_files_with_python_shebang)" + files="$(get_all_py_files) $(get_all_files_with_python_shebang)" echo "Linting and formatting: $files" execute_black $files execute_lint $files