Skip to content

Commit

Permalink
teuthology/suite: merge base_config with other fragments
Browse files Browse the repository at this point in the history
Presently the code tries to merge the base_config when the worker starts
running. There's no need to construct it this way and it prevents sharing the
"defaults" with the fragment merging infrastructure. It also prevents
overriding defaults like:

    kernel
        branch: wip-pdonnell-i66704
        client:
            branch: wip-pdonnell-i66704
            flavor: default
            kdb: 1
            sha1: 745cacd8f31e50d7f3b6039bbd8c9a8dfc07bf03
        flavor: default
        kdb: 1
        sha1: 745cacd8f31e50d7f3b6039bbd8c9a8dfc07bf03

A YAML fragment can set kernel.client but it cannot delete the defaults for
kernel.(branch|flavor|kdb|sha1) because there's no way to remove YAML elements
via a deep merge.

Signed-off-by: Patrick Donnelly <[email protected]>
  • Loading branch information
batrick committed Oct 19, 2024
1 parent 5bac476 commit 6325fc9
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 22 deletions.
4 changes: 3 additions & 1 deletion teuthology/suite/merge.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,20 +115,22 @@ def config_merge(configs, suite_name=None, **kwargs):
the entire job (config) from the list.
"""
seed = kwargs.setdefault('seed', 1)
base_config = kwargs.setdefault('base_config', {})
if not isinstance(seed, int):
log.debug("no valid seed input: using 1")
seed = 1
log.debug("configuring Lua randomseed to %d", seed)
L.execute(f'local math = require"math"; math.randomseed({seed});')
new_script = L.eval('new_script')
base_config_yaml = yaml.safe_load(str(base_config))
yaml_cache = {}
for desc, paths in configs:
log.debug("merging config %s", desc)

if suite_name is not None:
desc = combine_path(suite_name, desc)

yaml_complete_obj = {}
yaml_complete_obj = copy.deepcopy(base_config_yaml)
deep_merge(yaml_complete_obj, dict(TEUTHOLOGY_TEMPLATE))
for path in paths:
if path not in yaml_cache:
Expand Down
15 changes: 1 addition & 14 deletions teuthology/suite/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

from humanfriendly import format_timespan

from tempfile import NamedTemporaryFile
from teuthology import repo_utils

from teuthology.config import config, JobConfig
Expand Down Expand Up @@ -639,17 +638,10 @@ def schedule_suite(self):
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))

# create, but do not write, the temp file here, so it can be
# added to the args in collect_jobs, but not filled until
# any backtracking is done
base_yaml_path = NamedTemporaryFile(
prefix='schedule_suite_', delete=False
).name
self.base_yaml_paths.insert(0, base_yaml_path)

# compute job limit in respect of --sleep-before-teardown
job_limit = self.args.limit or 0
sleep_before_teardown = int(self.args.sleep_before_teardown or 0)
Expand Down Expand Up @@ -714,9 +706,6 @@ def schedule_suite(self):
dry_run=self.args.dry_run,
)

with open(base_yaml_path, 'w+b') as base_yaml:
base_yaml.write(str(self.base_config).encode())

if jobs_to_schedule:
self.write_rerun_memo()

Expand All @@ -728,8 +717,6 @@ def schedule_suite(self):

self.schedule_jobs(jobs_missing_packages, jobs_to_schedule, name)

os.remove(base_yaml_path)

count = len(jobs_to_schedule)
missing_count = len(jobs_missing_packages)
total_count = count
Expand Down
44 changes: 37 additions & 7 deletions teuthology/suite/test/test_run_.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import logging
import os
import pytest
import requests
Expand All @@ -14,6 +15,7 @@
from teuthology.suite import run
from teuthology.util.time import TIMESTAMP_FMT

log = logging.getLogger(__name__)

class TestRun(object):
klass = run.Run
Expand Down Expand Up @@ -270,14 +272,35 @@ def test_successful_schedule(
[call('ceph_sha1', 'default', 'ubuntu', '14.04', 'machine_type')],
)
y = {
'teuthology': {
'fragments_dropped': [],
'meta': {},
'postmerge': []
},
'field1': 'val1',
'field2': 'val2'
}
teuthology_keys = [
'branch',
'machine_type',
'name',
'os_type',
'os_version',
'overrides',
'priority',
'repo',
'seed',
'sha1',
'sleep_before_teardown',
'suite',
'suite_branch',
'suite_relpath',
'suite_repo',
'suite_sha1',
'tasks',
'teuthology_branch',
'teuthology_sha1',
'timestamp',
'user',
'teuthology',
]
for t in teuthology_keys:
y[t] = ANY
expected_job = dict(
yaml=y,
sha1='ceph_sha1',
Expand All @@ -287,16 +310,23 @@ def test_successful_schedule(
'--description',
os.path.join(self.args.suite, build_matrix_desc),
'--',
ANY, # base config
'-'
],
stdin=yaml.dump(y),
stdin=ANY,
desc=os.path.join(self.args.suite, build_matrix_desc),
)

m_schedule_jobs.assert_has_calls(
[call([], [expected_job], runobj.name)],
)
args = m_schedule_jobs.call_args.args
log.debug("args =\n%s", args)
jobargs = args[1][0]
stdin_yaml = yaml.safe_load(jobargs['stdin'])
for k in y:
assert y[k] == stdin_yaml[k]
for k in teuthology_keys:
assert k in stdin_yaml
m_write_rerun_memo.assert_called_once_with()

@patch('teuthology.suite.util.find_git_parents')
Expand Down

0 comments on commit 6325fc9

Please sign in to comment.