Bump AppWrapper version to 1.1.1 and Kueue to 0.11.0 #673
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
dgrove-oss
force-pushed
the
aw-1.1
branch
2 times, most recently
from
4155f44 to
79578c8
Compare
| sigs.k8s.io/yaml v1.4.0 | ||
| ) | ||
|
|
||
| replace k8s.io/client-go => k8s.io/client-go v0.31.4 | ||
|
|
||
| replace k8s.io/api => k8s.io/api v0.31.4 |
There was a problem hiding this comment.
There are more replace directives here than I'd like, but it was expedient in the short term. Someone who has a better understanding of how to coordinate Kubernetes/OpenShift API versions may be able to do something less heavy handed.
go.mod
Outdated
|
|
||
| replace sigs.k8s.io/controller-runtime v0.20.3 => sigs.k8s.io/controller-runtime v0.19.3 | ||
|
|
||
| replace sigs.k8s.io/kueue v0.11.0 => sigs.k8s.io/kueue v0.10.1 |
There was a problem hiding this comment.
This seems weird. We usually have to maintain compatibility between k8s, Kueue, AppWrapper. Downstreaming can get difficult with version mismatches.
cc: @sutaakar
There was a problem hiding this comment.
One thing to consider is that we use the KUEUE_VERSION in the Makefile for two things:
- Specifying which version of Kueue should be deployed in the CI testing.
- Specifying which version of Kueue's golang API the e2e test code depends on.
For this PR, it is important that the Kueue version for use case 1 is 0.11 (because AppWrapper 1.1 won't work with Kueue 0.10). However, for use case 2 it would have been fine to leave Kueue at 0.10.1. AppWrapper no longer imports Kueue at all, so the only reason we have a go dependency is for the APIs used in the e2e test code. And those are much less sensitive to exact Kueue version.
WDYT about have two variables for the two use cases and allowing them to diverge? I think it would make your downstreaming simpler.
There was a problem hiding this comment.
It is fine for me to separate the Kueue versions.
If I get it right it should be enough to remove
Line 163 in c03f1b5
There was a problem hiding this comment.
I pushed a change that removes line 163 as suggested. I think this is the right thing to do.
Decouple the Kueue version deployed on the cluster for CI testing from the golang dependency on Kueue APIs used in the e2e go test code.
This PR should not be merged until we are ready to upgrade Kueue to 0.11.x.
This PR bumps the AppWrapper version to 1.1.1, which is built to work with Kueue 0.11.
The key change is that in Kueue 0.11, AppWrapper is a built-in integration. This enables the removal of all Kueue-specific code from the AppWrapper controller, which eliminates the tight coupling between AppWrapper and Kueue versions.