From 42cc5695f05bc91a5a901ba73edd28fbc419fbb2 Mon Sep 17 00:00:00 2001 From: Zack Cerza Date: Tue, 21 Nov 2023 18:25:56 -0700 Subject: [PATCH] suite: Improve package query caching We had our own "system" for caching, but it had the unfortunate characteristic of being a big bowl of spaghetti. While eating said pasta I also noticed we had two competing "distro defaults" concepts - so that let me delete even more code. Yum! Signed-off-by: Zack Cerza --- teuthology/suite/run.py | 62 ++++++------ teuthology/suite/test/test_util.py | 25 ++--- teuthology/suite/util.py | 150 +++-------------------------- 3 files changed, 53 insertions(+), 184 deletions(-) diff --git a/teuthology/suite/run.py b/teuthology/suite/run.py index 02803e9ebf..509c8061cc 100644 --- a/teuthology/suite/run.py +++ b/teuthology/suite/run.py @@ -34,8 +34,7 @@ class Run(object): WAIT_PAUSE = 5 * 60 __slots__ = ( 'args', 'name', 'base_config', 'suite_repo_path', 'base_yaml_paths', - 'base_args', 'package_versions', 'kernel_dict', 'config_input', - 'timestamp', 'user', + 'base_args', 'kernel_dict', 'config_input', 'timestamp', 'user', 'os', ) def __init__(self, args): @@ -56,8 +55,6 @@ class Run(object): config.ceph_qa_suite_git_url = self.args.suite_repo self.base_config = self.create_initial_config() - # caches package versions to minimize requests to gbs - self.package_versions = dict() # Interpret any relative paths as being relative to ceph-qa-suite # (absolute paths are unchanged by this) @@ -90,6 +87,7 @@ class Run(object): :returns: A JobConfig object """ + self.os = self.choose_os() self.kernel_dict = self.choose_kernel() ceph_hash = self.choose_ceph_hash() # We don't store ceph_version because we don't use it yet outside of @@ -118,8 +116,8 @@ class Run(object): teuthology_branch=teuthology_branch, teuthology_sha1=teuthology_sha1, machine_type=self.args.machine_type, - distro=self.args.distro, - distro_version=self.args.distro_version, + distro=self.os.name, + distro_version=self.os.version, archive_upload=config.archive_upload, archive_upload_key=config.archive_upload_key, suite_repo=config.get_ceph_qa_suite_git_url(), @@ -128,6 +126,16 @@ class Run(object): ) return self.build_base_config() + def choose_os(self): + os_type = self.args.distro + os_version = self.args.distro_version + if not (os_type and os_version): + os_ = util.get_distro_defaults( + self.args.distro, self.args.machine_type)[2] + else: + os_ = OS(os_type, os_version) + return os_ + def choose_kernel(self): # Put together a stanza specifying the kernel hash if self.args.kernel_branch == 'distro': @@ -194,13 +202,14 @@ class Run(object): if config.suite_verify_ceph_hash and not self.args.newest: # don't bother if newest; we'll search for an older one # Get the ceph package version - try: - ceph_version = util.package_version_for_hash( - ceph_hash, self.args.flavor, self.args.distro, - self.args.distro_version, self.args.machine_type, - ) - except Exception as exc: - util.schedule_fail(str(exc), self.name, dry_run=self.args.dry_run) + ceph_version = util.package_version_for_hash( + ceph_hash, self.args.flavor, self.os.name, + self.os.version, self.args.machine_type, + ) + if not ceph_version: + msg = f"Packages for os_type '{self.os.name}', flavor " \ + f"{self.args.flavor} and ceph hash '{ceph_hash}' not found" + util.schedule_fail(msg, self.name, dry_run=self.args.dry_run) log.info("ceph version: {ver}".format(ver=ceph_version)) return ceph_version else: @@ -476,27 +485,14 @@ class Run(object): full_job_config = copy.deepcopy(self.base_config.to_dict()) deep_merge(full_job_config, parsed_yaml) flavor = util.get_install_task_flavor(full_job_config) - # Get package versions for this sha1, os_type and flavor. If - # we've already retrieved them in a previous loop, they'll be - # present in package_versions and gitbuilder will not be asked - # again for them. - try: - self.package_versions = util.get_package_versions( - sha1, - os_type, - os_version, - flavor, - self.package_versions - ) - except VersionNotFoundError: - pass - if not util.has_packages_for_distro( - sha1, os_type, os_version, flavor, self.package_versions - ): - m = "Packages for os_type '{os}', flavor {flavor} and " + \ - "ceph hash '{ver}' not found" - log.error(m.format(os=os_type, flavor=flavor, ver=sha1)) + version = util.package_version_for_hash(sha1, flavor, os_type, + os_version, self.args.machine_type) + if version: + log.debug(f"Found {version} for {os_type} {os_version} {flavor}") + else: jobs_missing_packages.append(job) + log.error(f"Packages for os_type '{os_type}', flavor {flavor} and " + f"ceph hash '{sha1}' not found") # optimization: one missing package causes backtrack in newest mode; # no point in continuing the search if newest: diff --git a/teuthology/suite/test/test_util.py b/teuthology/suite/test/test_util.py index 11d24ef365..e295010b85 100644 --- a/teuthology/suite/test/test_util.py +++ b/teuthology/suite/test/test_util.py @@ -349,32 +349,27 @@ class TestDistroDefaults(object): def setup_method(self): config.use_shaman = False - def test_distro_defaults_saya(self): - expected = ('armv7l', 'saucy', - OS(name='ubuntu', version='13.10', codename='saucy')) - assert util.get_distro_defaults('ubuntu', 'saya') == expected - def test_distro_defaults_plana(self): - expected = ('x86_64', 'xenial', - OS(name='ubuntu', version='16.04', codename='xenial')) + expected = ('x86_64', 'ubuntu/22.04', + OS(name='ubuntu', version='22.04', codename='jammy')) assert util.get_distro_defaults('ubuntu', 'plana') == expected def test_distro_defaults_debian(self): - expected = ('x86_64', 'wheezy', - OS(name='debian', version='7', codename='wheezy')) + expected = ('x86_64', 'debian/8.0', + OS(name='debian', version='8.0', codename='jessie')) assert util.get_distro_defaults('debian', 'magna') == expected def test_distro_defaults_centos(self): - expected = ('x86_64', 'centos7', - OS(name='centos', version='7', codename='core')) + expected = ('x86_64', 'centos/8', + OS(name='centos', version='8.stream', codename='core')) assert util.get_distro_defaults('centos', 'magna') == expected def test_distro_defaults_fedora(self): - expected = ('x86_64', 'fedora20', - OS(name='fedora', version='20', codename='heisenbug')) + expected = ('x86_64', 'fedora/25', + OS(name='fedora', version='25', codename='25')) assert util.get_distro_defaults('fedora', 'magna') == expected def test_distro_defaults_default(self): - expected = ('x86_64', 'centos7', - OS(name='centos', version='7', codename='core')) + expected = ('x86_64', 'centos/8', + OS(name='centos', version='8.stream', codename='core')) assert util.get_distro_defaults('rhel', 'magna') == expected diff --git a/teuthology/suite/util.py b/teuthology/suite/util.py index fabfa39472..8d0af8e90b 100644 --- a/teuthology/suite/util.py +++ b/teuthology/suite/util.py @@ -1,4 +1,5 @@ import copy +import functools import logging import os import requests @@ -17,8 +18,8 @@ from teuthology.config import config from teuthology.exceptions import BranchNotFoundError, ScheduleFailError from teuthology.misc import deep_merge from teuthology.repo_utils import fetch_qa_suite, fetch_teuthology -from teuthology.orchestra.opsys import OS -from teuthology.packaging import get_builder_project +from teuthology.orchestra.opsys import OS, DEFAULT_OS_VERSION +from teuthology.packaging import get_builder_project, VersionNotFoundError from teuthology.repo_utils import build_git_url from teuthology.task.install import get_flavor @@ -122,38 +123,15 @@ def get_distro_defaults(distro, machine_type): """ Given a distro (e.g. 'ubuntu') and machine type, return: (arch, release, pkg_type) - - This is used to default to: - ('x86_64', 'trusty', 'deb') when passed 'ubuntu' and 'plana' - ('armv7l', 'saucy', 'deb') when passed 'ubuntu' and 'saya' - ('x86_64', 'wheezy', 'deb') when passed 'debian' - ('x86_64', 'fedora20', 'rpm') when passed 'fedora' - And ('x86_64', 'centos7', 'rpm') when passed anything else """ arch = 'x86_64' - if distro in (None, 'None'): - os_type = 'centos' - os_version = '7' - elif distro in ('rhel', 'centos'): - os_type = 'centos' - os_version = '7' - elif distro == 'ubuntu': - os_type = distro - if machine_type == 'saya': - os_version = '13.10' - arch = 'armv7l' - else: - os_version = '16.04' - elif distro == 'debian': - os_type = distro - os_version = '7' - elif distro == 'fedora': - os_type = distro - os_version = '20' - elif distro == 'opensuse': + if distro in (None, 'None', 'rhel'): + distro = 'centos' + + try: + os_version = DEFAULT_OS_VERSION[distro] os_type = distro - os_version = '15.1' - else: + except IndexError: raise ValueError("Invalid distro value passed: %s", distro) _os = OS(name=os_type, version=os_version) release = get_builder_project()._get_distro( @@ -161,15 +139,6 @@ def get_distro_defaults(distro, machine_type): _os.version, _os.codename, ) - template = "Defaults for machine_type {mtype} distro {distro}: " \ - "arch={arch}, release={release}, pkg_type={pkg}" - log.debug(template.format( - mtype=machine_type, - distro=_os.name, - arch=arch, - release=release, - pkg=_os.package_type) - ) return ( arch, release, @@ -254,6 +223,7 @@ def get_branch_info(project, branch, project_owner='ceph'): return resp.json() +@functools.lru_cache() def package_version_for_hash(hash, flavor='default', distro='rhel', distro_version='8.0', machine_type='smithi'): """ @@ -281,7 +251,10 @@ def package_version_for_hash(hash, flavor='default', distro='rhel', log.info('build not complete') return None - return bp.version + try: + return bp.version + except VersionNotFoundError: + return None def get_arch(machine_type): @@ -333,101 +306,6 @@ def get_install_task_flavor(job_config): return get_flavor(first_install_config) -def get_package_versions(sha1, os_type, os_version, flavor, - package_versions=None): - """ - Will retrieve the package versions for the given sha1, os_type/version, - and flavor from gitbuilder. - - Optionally, a package_versions dict can be provided - from previous calls to this function to avoid calling gitbuilder for - information we've already retrieved. - - The package_versions dict will be in the following format:: - - { - "sha1": { - "ubuntu": { - "14.04": { - "basic": "version", - } - "15.04": { - "notcmalloc": "version", - } - } - "rhel": { - "basic": "version", - } - }, - "another-sha1": { - "ubuntu": { - "basic": "version", - } - } - } - - :param sha1: The sha1 hash of the ceph version. - :param os_type: The distro we want to get packages for, given - the ceph sha1. Ex. 'ubuntu', 'rhel', etc. - :param os_version: The distro's version, e.g. '14.04', '7.0' - :param flavor: Package flavor ('testing', 'notcmalloc', etc.) - :param package_versions: Use this optionally to use cached results of - previous calls to gitbuilder. - :returns: A dict of package versions. Will return versions - for all hashes/distros/vers, not just for the given - hash/distro/ver. - """ - if package_versions is None: - package_versions = dict() - - os_type = str(os_type) - - os_types = package_versions.get(sha1, dict()) - os_versions = os_types.get(os_type, dict()) - flavors = os_versions.get(os_version, dict()) - if flavor not in flavors: - package_version = package_version_for_hash( - sha1, - flavor, - distro=os_type, - distro_version=os_version, - ) - flavors[flavor] = package_version - os_versions[os_version] = flavors - os_types[os_type] = os_versions - package_versions[sha1] = os_types - - return package_versions - - -def has_packages_for_distro(sha1, os_type, os_version, flavor, - package_versions=None): - """ - Checks to see if gitbuilder has packages for the given sha1, os_type and - flavor. - - See above for package_versions description. - - :param sha1: The sha1 hash of the ceph version. - :param os_type: The distro we want to get packages for, given - the ceph sha1. Ex. 'ubuntu', 'rhel', etc. - :param flavor: The ceph packages shaman flavor - :param package_versions: Use this optionally to use cached results of - previous calls to gitbuilder. - :returns: True, if packages are found. False otherwise. - """ - os_type = str(os_type) - if package_versions is None: - package_versions = get_package_versions( - sha1, os_type, os_version, flavor) - - flavors = package_versions.get(sha1, dict()).get( - os_type, dict()).get( - os_version, dict()) - # we want to return a boolean here, not the actual package versions - return bool(flavors.get(flavor, None)) - - def teuthology_schedule(args, verbose, dry_run, log_prefix='', stdin=None): """ Run teuthology-schedule to schedule individual jobs. -- 2.39.5