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
scheduler_perf: add DRA structured parameters test with shared claims #124548
base: master
Are you sure you want to change the base?
scheduler_perf: add DRA structured parameters test with shared claims #124548
Conversation
Several pods sharing the same claim is not common, but can be useful and thus should get tested. Before, createPods and createAny operations were not able to do this because each generated object was the same. What we need are different, predictable names of the claims (from createAny) and different references to those in the pods (from createPods). Now text/template processing with the index number of the pod respectively claim as input is used to inject these varying fields. A "div" function is needed to use the same claim in several different pods. While at it, some existing test cases get cleaned up a bit (removal of incorrect comments, adding comments for testing with queuing hints).
This issue is currently awaiting triage. If a SIG or subproject determines this is a relevant issue, they will accept it by applying the The Instructions 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/test-infra repository. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pohly The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@@ -1493,7 +1508,24 @@ func NewCustomCreatePodStrategy(podTemplate *v1.Pod) TestPodCreateStrategy { | |||
// volumeFactory creates an unique PersistentVolume for given integer. | |||
type volumeFactory func(uniqueID int) *v1.PersistentVolume | |||
|
|||
func NewCreatePodWithPersistentVolumeStrategy(claimTemplate *v1.PersistentVolumeClaim, factory volumeFactory, podTemplate *v1.Pod) TestPodCreateStrategy { | |||
// PodTemplate is responsible for creating a v1.Pod. |
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.
Note that this is conceptually similar to volumeFactory
above, I just decided to use a template instead of a function. That interface then has two different implementations:
- transforming a YAML file with text/template (new)
- returning a fixed pod instance (what was done before)
If you like the interface approach, then a follow-up PR could use the same concept also for volumes. Not sure whether it's worth it, though.
} | ||
fm := template.FuncMap{"div": func(a, b int) int { | ||
return a / b | ||
}} |
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.
It's a pity that text/template
hasn't standardized some common functions like this 😢
@pohly: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions 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/test-infra repository. I understand the commands that are listed here. |
No thanks, wrapping errors doesn't make sense here. |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Several pods sharing the same claim is not common, but can be useful and thus should get tested.
Special notes for your reviewer:
Before, createPods and createAny operations were not able to do this because each generated object was the same. What we need are different, predictable names of the claims (from createAny) and different references to those in the pods (from createPods). Now text/template processing with the index number of the pod respectively claim as input is used to inject these varying fields. A "div" function is needed to use the same claim in several different pods.
While at it, some existing test cases get cleaned up a bit (removal of incorrect comments, adding comments for testing with queuing hints).
Does this PR introduce a user-facing change?
/assign @kerthcet