teuthology/suite: pass the base config to scripts #2008

batrick · 2024-10-17T20:01:34Z

This PR is to support: ceph/ceph#60386

Also, this PR fixes a bug: teuthology/tasks/kernel: default branch should be "distro". When the kernel dictionary is empty, the kernel task should assume distro.

Normally, the fs suite overrides the kernel branch whenever the kernel client is used, according to the matrix of configs. This prevents easily testing a new kernel with the -k option to teuthology-suite. So, using the base config passed to the script, detect if an alternate testing kernel is desired and use that instead. The scheduler still needs to do some work: $ teuthology-suite ... -k master --filter k-testing ... 2024-10-17 19:57:52,051.051 DEBUG:teuthology.suite.merge:base kernel {'branch': 'master', 'kdb': 1, 'sha1': 'ccda9910d8490f4fb067131598e4b2e986faa5a0', 'flavor': 'default'} 2024-10-17 19:57:52,051.051 DEBUG:teuthology.suite.merge:overriding testing kernel with {'branch': 'master', 'kdb': 1, 'sha1': 'ccda9910d8490f4fb067131598e4b2e986faa5a0', 'flavor': 'default'} vs. $ teuthology-suite ... --filter k-testing ... 2024-10-17 20:04:52,265.265 DEBUG:teuthology.suite.merge:base kernel {'branch': 'distro', 'kdb': 1, 'sha1': 'distro'} Which will only select jobs using the "testing" kernel. See-also: ceph/teuthology#2008 Fixes: https://tracker.ceph.com/issues/68603 Signed-off-by: Patrick Donnelly <[email protected]>

Normally, the fs suite overrides the kernel branch whenever the kernel client is used, according to the matrix of configs. This prevents easily testing a new kernel with the -k option to teuthology-suite. So, using the base config passed to the script, detect if an alternate testing kernel is desired and use that instead. The scheduler still needs to do some work: $ teuthology-suite ... --verbose -k master --filter k-testing ... 2024-10-17 19:57:52,051.051 DEBUG:teuthology.suite.merge:base kernel {'branch': 'master', 'kdb': 1, 'sha1': 'ccda9910d8490f4fb067131598e4b2e986faa5a0', 'flavor': 'default'} 2024-10-17 19:57:52,051.051 DEBUG:teuthology.suite.merge:overriding testing kernel with {'branch': 'master', 'kdb': 1, 'sha1': 'ccda9910d8490f4fb067131598e4b2e986faa5a0', 'flavor': 'default'} vs. $ teuthology-suite ... --verbose --filter k-testing ... 2024-10-17 20:04:52,265.265 DEBUG:teuthology.suite.merge:base kernel {'branch': 'distro', 'kdb': 1, 'sha1': 'distro'} Which will only select jobs using the "testing" kernel. See-also: ceph/teuthology#2008 Fixes: https://tracker.ceph.com/issues/68603 Signed-off-by: Patrick Donnelly <[email protected]>

Normally, the fs suite overrides the kernel branch whenever the kernel client is used, according to the matrix of configs. This prevents easily testing a new kernel with the -k option to teuthology-suite. So, using the base config passed to the script, detect if an alternate testing kernel is desired and use that instead. The scheduler still needs to do some work: $ teuthology-suite ... --verbose -k wip-pdonnell-i66704 --filter k-testing ... 2024-10-18 00:29:43,875.875 DEBUG:teuthology.suite.merge:base kernel {'branch': 'wip-pdonnell-i66704', 'kdb': 1, 'sha1': '745cacd8f31e50d7f3b6039bbd8c9a8dfc07bf03', 'flavor': 'default'} 2024-10-18 00:29:43,875.875 DEBUG:teuthology.suite.merge:overriding testing kernel with {'branch': 'wip-pdonnell-i66704', 'kdb': 1, 'sha1': '745cacd8f31e50d7f3b6039bbd8c9a8dfc07bf03', 'flavor': 'default'} vs. $ teuthology-suite ... --verbose --filter k-testing ... 2024-10-17 20:04:52,265.265 DEBUG:teuthology.suite.merge:base kernel {'branch': 'distro', 'kdb': 1, 'sha1': 'distro'} Which will only select jobs using the "testing" kernel. See-also: ceph/teuthology#2008 Fixes: https://tracker.ceph.com/issues/68603 Signed-off-by: Patrick Donnelly <[email protected]>

So it's easier to see its value before any configs are generated. Signed-off-by: Patrick Donnelly <[email protected]>

For inspection by scripts. Signed-off-by: Patrick Donnelly <[email protected]>

Normally, the fs suite overrides the kernel branch whenever the kernel client is used, according to the matrix of configs. This prevents easily testing a new kernel with the -k option to teuthology-suite. So, using the base config passed to the script, detect if an alternate testing kernel is desired and use that instead. The scheduler still needs to do some work: $ teuthology-suite ... --verbose -k wip-pdonnell-i66704 --filter k-testing ... 2024-10-18 00:29:43,875.875 DEBUG:teuthology.suite.merge:base kernel {'branch': 'wip-pdonnell-i66704', 'kdb': 1, 'sha1': '745cacd8f31e50d7f3b6039bbd8c9a8dfc07bf03', 'flavor': 'default'} 2024-10-18 00:29:43,875.875 DEBUG:teuthology.suite.merge:overriding testing kernel with {'branch': 'wip-pdonnell-i66704', 'kdb': 1, 'sha1': '745cacd8f31e50d7f3b6039bbd8c9a8dfc07bf03', 'flavor': 'default'} vs. $ teuthology-suite ... --verbose --filter k-testing ... 2024-10-17 20:04:52,265.265 DEBUG:teuthology.suite.merge:base kernel {'branch': 'distro', 'kdb': 1, 'sha1': 'distro'} Which will only select jobs using the "testing" kernel. See-also: ceph/teuthology#2008 Fixes: https://tracker.ceph.com/issues/68603 Signed-off-by: Patrick Donnelly <[email protected]>

batrick · 2024-10-21T14:42:13Z

Also, this PR fixes a bug: teuthology/tasks/kernel: default branch should be "distro". When the kernel dictionary is empty, the kernel task should assume distro.

Examples:

Normally, the fs suite overrides the kernel branch whenever the kernel client is used, according to the matrix of configs. This prevents easily testing a new kernel with the -k option to teuthology-suite. So, using the base config passed to the script, detect if an alternate testing kernel is desired and use that instead. The scheduler still needs to do some work: $ teuthology-suite ... --verbose -k wip-pdonnell-i66704 --filter k-testing ... 2024-10-18 00:29:43,875.875 DEBUG:teuthology.suite.merge:base kernel {'branch': 'wip-pdonnell-i66704', 'kdb': 1, 'sha1': '745cacd8f31e50d7f3b6039bbd8c9a8dfc07bf03', 'flavor': 'default'} 2024-10-18 00:29:43,875.875 DEBUG:teuthology.suite.merge:overriding testing kernel with {'branch': 'wip-pdonnell-i66704', 'kdb': 1, 'sha1': '745cacd8f31e50d7f3b6039bbd8c9a8dfc07bf03', 'flavor': 'default'} vs. $ teuthology-suite ... --verbose --filter k-testing ... 2024-10-17 20:04:52,265.265 DEBUG:teuthology.suite.merge:base kernel {'branch': 'distro', 'kdb': 1, 'sha1': 'distro'} Which will only select jobs using the "testing" kernel. See-also: ceph/teuthology#2008 Fixes: https://tracker.ceph.com/issues/68603 Signed-off-by: Patrick Donnelly <[email protected]>

zmc

Nice! I just have one question

zmc · 2024-10-22T18:08:09Z

teuthology/suite/merge.py

    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))


It's not immediately obvious why we'd need to create a copy of base_config here and then again on 133 - could you explain the reasoning?

It's to basically enforce a read-only view of the base_config.

I tried to do copy.deepcopy(base_config) below but ran into inscrutable recursion errors that didn't seem worth debugging. So I'm just reloading it as a legit yaml dictionary here.

I think I would suggest just doing a yaml_complete_obj = copy.deepcopy(base_config.to_dict()) on 133 and dropping this statement.

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]>

Generally, we don't want to install any kernel by default. There also is no "main" branch for the ceph-client.git project. Signed-off-by: Patrick Donnelly <[email protected]>

zmc · 2024-10-22T23:26:43Z

teuthology/suite/merge.py

@@ -115,6 +115,7 @@ 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', {})


Suggested change

base_config = kwargs.setdefault('base_config', {})

base_config = kwargs.setdefault('base_config', JobConfig())

And add from teuthology.config import JobConfig up top

batrick mentioned this pull request Oct 17, 2024

qa/cephfs: override testing kernel with -k option ceph/ceph#60386

Open

14 tasks

batrick requested review from vshankar and lxbsz October 18, 2024 12:35

batrick force-pushed the fragment-merge-base-config branch 2 times, most recently from 4f3fa41 to 44e3d73 Compare October 18, 2024 14:14

batrick added 2 commits October 18, 2024 10:19

teuthology/suite: print base config before config merge

5961d42

So it's easier to see its value before any configs are generated. Signed-off-by: Patrick Donnelly <[email protected]>

teuthology/suite: set the kernel branch in the kernel config

5bac476

For inspection by scripts. Signed-off-by: Patrick Donnelly <[email protected]>

batrick force-pushed the fragment-merge-base-config branch 4 times, most recently from 7b0d07a to ba3002e Compare October 18, 2024 17:21

batrick force-pushed the fragment-merge-base-config branch 6 times, most recently from af3ae9e to 60ca0e7 Compare October 18, 2024 19:27

batrick force-pushed the fragment-merge-base-config branch 13 times, most recently from b5be825 to 6325fc9 Compare October 19, 2024 00:31

batrick requested a review from zmc October 19, 2024 00:52

batrick added feature improvement needs-review labels Oct 19, 2024

batrick force-pushed the fragment-merge-base-config branch from fec9c14 to 9bdfb74 Compare October 21, 2024 14:09

zmc reviewed Oct 22, 2024

View reviewed changes

batrick added 2 commits October 22, 2024 17:04

teuthology/tasks/kernel: default branch should be "distro"

ab123ee

Generally, we don't want to install any kernel by default. There also is no "main" branch for the ceph-client.git project. Signed-off-by: Patrick Donnelly <[email protected]>

batrick force-pushed the fragment-merge-base-config branch from 9bdfb74 to ab123ee Compare October 22, 2024 21:04

zmc reviewed Oct 22, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

teuthology/suite: pass the base config to scripts #2008

teuthology/suite: pass the base config to scripts #2008

batrick commented Oct 17, 2024 •

edited

Loading

batrick commented Oct 21, 2024 •

edited

Loading

zmc left a comment

zmc Oct 22, 2024

batrick Oct 22, 2024

batrick Oct 22, 2024

zmc Oct 22, 2024

batrick Oct 22, 2024

batrick Oct 22, 2024

zmc Oct 22, 2024

	base_config = kwargs.setdefault('base_config', {})
	base_config = kwargs.setdefault('base_config', JobConfig())

teuthology/suite: pass the base config to scripts #2008

Are you sure you want to change the base?

teuthology/suite: pass the base config to scripts #2008

Conversation

batrick commented Oct 17, 2024 • edited Loading

batrick commented Oct 21, 2024 • edited Loading

zmc left a comment

Choose a reason for hiding this comment

zmc Oct 22, 2024

Choose a reason for hiding this comment

batrick Oct 22, 2024

Choose a reason for hiding this comment

batrick Oct 22, 2024

Choose a reason for hiding this comment

zmc Oct 22, 2024

Choose a reason for hiding this comment

batrick Oct 22, 2024

Choose a reason for hiding this comment

batrick Oct 22, 2024

Choose a reason for hiding this comment

zmc Oct 22, 2024

Choose a reason for hiding this comment

batrick commented Oct 17, 2024 •

edited

Loading

batrick commented Oct 21, 2024 •

edited

Loading