-
Notifications
You must be signed in to change notification settings - Fork 4.8k
[wip] test: experiment with size labels for resource-aware parallelism #30604
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Add Ginkgo Size labels to enable variable parallelism based on test resource intensity. This is a wild experiment to help reduce resource contention. Only impacts tests in origin currently, but I suspect that's where our "heaviest" tests are. If this proves worthwhile we should be able to apply the same things to OTE and Kube. Labels were applied with Claude. I asked Claude to use the criteria below. I have no idea if it is accurate, but the tests I expected to be "L" are indeed tagged "L". Size label criteria: - Size:S - Low CPU, quick tests, simple API calls, no pod creation, basic CLI commands. - Size:M - Moderate complexity, 1-2 pods, basic services, simple deployments. DEFAULT for unlabeled tests. - Size:L - High resource usage, many pods, image builds, complex workflows, stress tests. Examples: must-gather, multi-replica deployments, complex builds. Run at 0.5x parallelism (e.g., 15).
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: stbenjam The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/payload-aggregate periodic-ci-openshift-release-master-nightly-4.21-e2e-aws-ovn-upgrade-fips 5 |
|
@stbenjam: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/647a44e0-d6d8-11f0-8d37-c1425c275bcc-0 |
|
/payload-aggregate-with-prs periodic-ci-openshift-release-master-nightly-4.21-e2e-aws-ovn-upgrade-fips 5 openshift/cluster-api#254 Seems to still be picking up the older payload |
|
@neisw: An error was encountered. No known errors were detected, please see the full error message for details. Full error message.
unable to get additional pr info from string: still: string: still doesn't match expected format: org/repo#number
Please contact an administrator to resolve this issue. |
|
/payload-aggregate-with-prs periodic-ci-openshift-release-master-nightly-4.21-e2e-aws-ovn-upgrade-fips 5 openshift/cluster-api#254 |
|
@neisw: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/df1743d0-d6f9-11f0-82e3-520759d1a342-0 |
|
Scheduling required tests: Scheduling tests matching the |
pkg/test/ginkgo/cmd_runsuite.go
Outdated
| // Run openshift tests by size with appropriate parallelism | ||
| openshiftTestsSCopy := copyTests(openshiftTestsS) | ||
| q.Execute(testCtx, openshiftTestsSCopy, max(1, parallelism*2), testOutputConfig, abortFn) // Size:S tests run at 2x parallelism | ||
| tests = append(tests, openshiftTestsSCopy...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks mega. How would you feel about throwing in some intervals with a new Source so we could clearly see what group we're running when we hit HighCPU etc. Maybe Source = TestGroup, Locator.group = OpenShiftTests-M or something like that.
|
Scheduling required tests: Scheduling tests matching the |
|
/test e2e-aws-ovn-fips |
|
@stbenjam: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/test e2e-aws-ovn-fips |
|
/payload-aggregate periodic-ci-openshift-release-master-nightly-4.21-e2e-aws-ovn-upgrade-fips 5 |
|
@stbenjam: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/5edbda30-d7be-11f0-91fc-b3ba3575a46b-0 |
|
/test e2e-aws-ovn-fips |
|
/payload-aggregate periodic-ci-openshift-release-master-nightly-4.22-e2e-aws-ovn-upgrade-fips 10 |
|
@bertinatto: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/c1044980-d99a-11f0-8f0a-f7e303a21127-0 |
|
/payload-aggregate periodic-ci-openshift-release-master-nightly-4.22-e2e-aws-ovn-upgrade-fips 10 |
|
@neisw: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/665b8cb0-da75-11f0-8b16-2875970a0e40-0 |
|
@neisw @bertinatto Any particular reason you're trying to test this? This WIP PR is for me to iterate on it, I can handle testing it when I need to. The current code has multiple issues I need to resolve, and the end result of this will probably be a couple of separate PR's as there's a few different concepts here. I understood the issues with the upgrade job were resolved and there's less urgency for something like this? |
|
Let Stephen know we are still seeing some issues in 4.22 and are curious how changing the test grouping based on estimated resource usage impacts that signal. |
Add Ginkgo Size labels to enable variable parallelism based on test resource intensity. This is a wild experiment to help reduce resource contention. Only impacts tests in origin currently, but I suspect that's where our "heaviest" tests are. If this proves worthwhile we should be able to apply the same things to OTE and Kube.
Labels were applied with Claude. I asked Claude to use the criteria below. I have no idea if it is accurate, but the tests I expected to be "L" are indeed tagged "L".
Size label criteria: