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).
k8s-ci-robot
added
release-note-none
|
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. |
k8s-ci-robot
added
the
needs-priority
|
[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 |
k8s-ci-robot
added
approved
| @@ -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.
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.
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