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 <zack@cerza.org>

diff --git a/teuthology/suite/fragment-merge.lua b/teuthology/suite/fragment-merge.lua
index 6be2e0b8794b2bed73a9f56359457a615e37dac3..a51667583e80661ec882109792a62abb93848ab5 100644 (file)
--- 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 0e109af02f6c31c81357922f3a897bf833502d29..2a9c2f355780522906ef60a5c35fdc23abc2b37c 100644 (file)
--- 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 f812fccac2d5e6845fe6da6935eb2ffe2291e4a4..e9be34d1eaeea666aaf680814d2b35e014fa4396 100644 (file)
--- 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 984231dfb1ab92874cc544d78111301beafd4acc..8e8698f52c2a88b02ba8c9b33e83da793877b836 100644 (file)
--- 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 a3e6d12b3492112361199bca51d06c153bf09a28..3b45974c1020b561a315a7a7ecc72b53752cac6a 100644 (file)
--- 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'