teuthology/suite: pass the base config to scripts by batrick · Pull Request #2008 · ceph/teuthology

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 <pdonnell@ibm.com>

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 <pdonnell@ibm.com>

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 <pdonnell@ibm.com>

So it's easier to see its value before any configs are generated. Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

For inspection by scripts. Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

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 <pdonnell@ibm.com>

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 <pdonnell@ibm.com>

zmc

Nice! I just have one question

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

teuthology/suite/merge.py

    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.

teuthology/suite/merge.py

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 <pdonnell@redhat.com>

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 <pdonnell@redhat.com>

batrick · 2024-10-23T02:35:01Z

(override k-testing.yaml:) https://pulpito.ceph.com/pdonnell-2024-10-23_02:34:42-fs-wip-pdonnell-testing-20241019.005706-debug-wip-pdonnell-i66704-default-smithi

and

(override default from -k) https://pulpito.ceph.com/pdonnell-2024-10-23_02:34:50-fs-wip-pdonnell-testing-20241019.005706-debug-wip-pdonnell-i66704-default-smithi

batrick · 2024-10-24T00:51:36Z

@zmc good to go?

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 <pdonnell@ibm.com>

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 <pdonnell@ibm.com> (cherry picked from commit 4b084c6)

batrick mentioned this pull request Oct 17, 2024

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

Merged

14 tasks

batrick requested review from lxbsz and vshankar 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 <pdonnell@redhat.com>

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

5bac476

For inspection by scripts. Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

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

teuthology/suite/merge.py Outdated Show resolved Hide resolved

batrick added 2 commits October 22, 2024 21:18

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

65f6bfa

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 <pdonnell@redhat.com>

batrick force-pushed the fragment-merge-base-config branch from ab123ee to 65f6bfa Compare October 23, 2024 01:18

zmc approved these changes Oct 25, 2024

View reviewed changes

zmc merged commit d61fe8c into ceph:main Oct 25, 2024

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
zmc merged 4 commits intoceph:mainfrom
batrick:fragment-merge-base-config

batrick commented Oct 17, 2024 •

edited

Loading

Uh oh!

batrick commented Oct 21, 2024 •

edited

Loading

Uh oh!

zmc left a comment

Uh oh!

zmc Oct 22, 2024

Uh oh!

batrick Oct 22, 2024

Uh oh!

batrick Oct 22, 2024

Uh oh!

zmc Oct 22, 2024

Uh oh!

batrick Oct 22, 2024

Uh oh!

batrick Oct 22, 2024

Uh oh!

Uh oh!

batrick commented Oct 23, 2024 •

edited

Loading

Uh oh!

batrick commented Oct 24, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

batrick commented Oct 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

batrick commented Oct 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zmc left a comment

Choose a reason for hiding this comment

Uh oh!

zmc Oct 22, 2024

Choose a reason for hiding this comment

Uh oh!

batrick Oct 22, 2024

Choose a reason for hiding this comment

Uh oh!

batrick Oct 22, 2024

Choose a reason for hiding this comment

Uh oh!

zmc Oct 22, 2024

Choose a reason for hiding this comment

Uh oh!

batrick Oct 22, 2024

Choose a reason for hiding this comment

Uh oh!

batrick Oct 22, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

batrick commented Oct 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

batrick commented Oct 24, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

batrick commented Oct 17, 2024 •

edited

Loading

batrick commented Oct 21, 2024 •

edited

Loading

batrick commented Oct 23, 2024 •

edited

Loading