From: Zack Cerza Date: Thu, 20 Mar 2025 21:02:44 +0000 (-0600) Subject: suite: Filter YAML fragments for distro, version X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=55df6d53b0a4ac02da6dd80a79c4c6610fed904f;p=teuthology.git suite: Filter YAML fragments for distro, version The behavior of --distro{,-version} was never well-defined. With this change, they can independently be used to provide a default for jobs which don't otherwise specify a value; and more importantly, perform a post-merge filter looking at each job's values, rejecting those which don't match. This behavior should be more intuitive and more easy to understand. Signed-off-by: Zack Cerza --- diff --git a/teuthology/suite/fragment-merge.lua b/teuthology/suite/fragment-merge.lua index 6be2e0b87..a51667583 100644 --- a/teuthology/suite/fragment-merge.lua +++ b/teuthology/suite/fragment-merge.lua @@ -16,6 +16,7 @@ local lua_allowlist = { py_iterex = python.iterex, py_itemgetter = python.as_itemgetter, math = math, + string = string, } lua_allowlist.__index = lua_allowlist @@ -49,6 +50,18 @@ local function check_filters(_ENV) end end end + if os_type and yaml.os_type then + if os_type ~= yaml.os_type then + reject() + end + end + if os_version and yaml.os_version then + wanted_os_version = string.gsub(os_version, ".stream", "") + this_os_version = string.gsub(yaml.os_version, ".stream", "") + if wanted_os_version ~= this_os_version then + reject() + end + end if filter_in then local found, tried = false, false for i,f in py_enumerate(filter_in) do diff --git a/teuthology/suite/merge.py b/teuthology/suite/merge.py index 0e109af02..2a9c2f355 100644 --- a/teuthology/suite/merge.py +++ b/teuthology/suite/merge.py @@ -171,6 +171,10 @@ def config_merge(configs, suite_name=None, **kwargs): env['yaml'] = yaml_complete_obj for k,v in kwargs.items(): env[k] = v + if os_type := base_config.get('os_type'): + env['os_type'] = os_type + if os_version := base_config.get('os_version'): + env['os_version'] = os_version if not script(): log.debug("skipping config %s due to postmerge filter", desc) continue diff --git a/teuthology/suite/placeholder.py b/teuthology/suite/placeholder.py index f812fccac..e9be34d1e 100644 --- a/teuthology/suite/placeholder.py +++ b/teuthology/suite/placeholder.py @@ -6,8 +6,9 @@ class Placeholder(object): A placeholder for use with substitute_placeholders. Simply has a 'name' attribute. """ - def __init__(self, name): + def __init__(self, name, required=True): self.name = name + self.required = required def substitute_placeholders(input_dict, values_dict): @@ -31,12 +32,19 @@ def substitute_placeholders(input_dict, values_dict): if isinstance(value, dict): _substitute(value, values_dict) elif isinstance(value, Placeholder): - if values_dict[value.name] is None: + if values_dict.get(value.name) is None: del input_dict[key] continue # If there is a Placeholder without a corresponding entry in # values_dict, we will hit a KeyError - we want this. - input_dict[key] = values_dict[value.name] + try: + input_dict[key] = values_dict[value.name] + except KeyError: + print(f"KeyError {key}={input_dict[key]} ") + if value.required: + raise + else: + del input_dict[key] return input_dict return _substitute(input_dict, values_dict) @@ -52,8 +60,8 @@ dict_templ = { 'archive_upload': Placeholder('archive_upload'), 'archive_upload_key': Placeholder('archive_upload_key'), 'machine_type': Placeholder('machine_type'), - 'os_type': Placeholder('distro'), - 'os_version': Placeholder('distro_version'), + 'os_type': Placeholder('os_type', required=False), + 'os_version': Placeholder('os_version', required=False), 'overrides': { 'admin_socket': { 'branch': Placeholder('ceph_branch'), diff --git a/teuthology/suite/run.py b/teuthology/suite/run.py index 984231dfb..8e8698f52 100644 --- a/teuthology/suite/run.py +++ b/teuthology/suite/run.py @@ -95,12 +95,8 @@ class Run(object): dry_run=self.args.dry_run, ) - 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 - # logging. - self.choose_ceph_version(ceph_hash) suite_branch = self.choose_suite_branch() suite_hash = self.choose_suite_hash(suite_branch) if self.args.suite_dir: @@ -111,9 +107,12 @@ class Run(object): teuthology_branch, teuthology_sha1 = self.choose_teuthology_branch() - if self.args.distro_version: + if self.args.distro and self.args.distro_version: self.args.distro_version, _ = \ - OS.version_codename(self.args.distro, self.args.distro_version) + OS.version_codename( + self.args.distro, + self.args.distro_version, + ) self.config_input = dict( suite=self.args.suite, suite_branch=suite_branch, @@ -124,8 +123,6 @@ class Run(object): teuthology_branch=teuthology_branch, teuthology_sha1=teuthology_sha1, machine_type=self.args.machine_type, - 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(), @@ -133,6 +130,10 @@ class Run(object): flavor=self.args.flavor, expire=expires.strftime(TIMESTAMP_FMT) if expires else None, ) + if self.args.distro: + self.config_input['os_type'] = self.args.distro.lower() + if self.args.distro_version: + self.config_input['os_version'] = self.args.distro_version.lower() return self.build_base_config() def get_expiration(self, _base_time: datetime.datetime | None = None) -> datetime.datetime | None: @@ -234,23 +235,6 @@ class Run(object): log.info("ceph sha1: {hash}".format(hash=ceph_hash)) return ceph_hash - def choose_ceph_version(self, ceph_hash): - 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 - 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: - log.info('skipping ceph package verification') - def choose_teuthology_branch(self): """Select teuthology branch, check if it is present in repo and return tuple (branch, hash) where hash is commit sha1 corresponding @@ -640,14 +624,16 @@ Note: If you still want to go ahead, use --job-threshold 0''' seed=self.args.seed) generated = len(configs) log.info(f'Suite {suite_name} in {suite_path} generated {generated} jobs (not yet filtered or merged)') - configs = list(config_merge(configs, + config_merge_kwargs = dict( + base_config=self.base_config, filter_in=self.args.filter_in, filter_out=self.args.filter_out, filter_all=self.args.filter_all, filter_fragments=self.args.filter_fragments, - base_config=self.base_config, seed=self.args.seed, - suite_name=suite_name)) + suite_name=suite_name, + ) + configs = list(config_merge(configs, **config_merge_kwargs)) # compute job limit in respect of --sleep-before-teardown job_limit = self.args.limit or 0 diff --git a/teuthology/suite/test/test_run_.py b/teuthology/suite/test/test_run_.py index a3e6d12b3..3b45974c1 100644 --- a/teuthology/suite/test/test_run_.py +++ b/teuthology/suite/test/test_run_.py @@ -39,13 +39,11 @@ class TestRun(object): @patch('teuthology.suite.run.util.fetch_repos') @patch('teuthology.suite.run.util.git_ls_remote') - @patch('teuthology.suite.run.Run.choose_ceph_version') @patch('teuthology.suite.run.util.git_validate_sha1') - def test_email_addr(self, m_git_validate_sha1, m_choose_ceph_version, - m_git_ls_remote, m_fetch_repos): + def test_email_addr(self, m_git_validate_sha1, + m_git_ls_remote, _): # neuter choose_X_branch m_git_validate_sha1.return_value = self.args_dict['ceph_sha1'] - m_choose_ceph_version.return_value = self.args_dict['ceph_sha1'] self.args_dict['teuthology_branch'] = 'main' self.args_dict['suite_branch'] = 'main' m_git_ls_remote.return_value = 'suite_sha1'