implement TAS support in AppWrapper integration #4174
Conversation
k8s-ci-robot
added
the
release-note-none
k8s-ci-robot
added
cncf-cla: yes
✅ Deploy Preview for kubernetes-sigs-kueue ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
/retest Due to #4113 |
|
cc @PBundyra @mbobrovskyi could you give it a pass? |
| @@ -122,6 +122,9 @@ func (aw *AppWrapper) PodSets() ([]kueue.PodSet, error) { | |||
| ctrl.Log.Error(err, "Error returned from awutils.GetPodSets", "appwrapper", aw) | |||
| return nil, err | |||
| } | |||
| for idx := range podSets { | |||
| podSets[idx].TopologyRequest = jobframework.PodSetTopologyRequest(&podSets[idx].Template.ObjectMeta, nil, nil, nil) | |||
There was a problem hiding this comment.
Right, there is no rank-based ordering for now.
IIUC this requires checking the type of the wrapped job and preparing configuration depending on this.
We could probably register some "rank-based ordering" factory by framework, and you could do the lookup here to construct the input.
However, this is substantially more work so I suggest it as a follow up.
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: 1c4fb0c3033c03dcdca0779756054a79fa14a9c8
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dgrove-oss, mimowo 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
the
approved
* implement TAS support in AppWrapper integration * TAS e2e test for AppWrapper * update integration feature table * install appwrappers for TAS e2e tests
What type of PR is this?
/feature
What this PR does / why we need it:
Implements TAS support for the AppWrapper integration.
Which issue(s) this PR fixes:
Special notes for your reviewer:
The bump appwrappers from v1.0.0 to 1.0.3 is necessary to pick up fixes to appwrapper bugs found while implementing TAS support.
Does this PR introduce a user-facing change?