-
Notifications
You must be signed in to change notification settings - Fork 272
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
Update to k8s 1.29 libs #3269
Update to k8s 1.29 libs #3269
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 |
Signed-off-by: Michael Henriksen <[email protected]>
Signed-off-by: Michael Henriksen <[email protected]>
Signed-off-by: Michael Henriksen <[email protected]>
Signed-off-by: Michael Henriksen <[email protected]>
Signed-off-by: Michael Henriksen <[email protected]>
Signed-off-by: Michael Henriksen <[email protected]>
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.
TBH looks very good, was expecting more logical changes but looks like it went smoothly (if we scratch over the VolumeResourceRequirements change lol)
@@ -31,5 +31,5 @@ for tag in ${docker_tag}; do | |||
--define container_prefix=${docker_prefix} \ | |||
--define container_tag=${tag} \ | |||
--host_force_python=PY3 \ | |||
//:test-container-images //cmd/cdi-operator:cdi-operator-image //cmd/cdi-controller:cdi-controller-image //cmd/cdi-apiserver:cdi-apiserver-image //cmd/cdi-cloner:cdi-cloner-image //cmd/cdi-importer:cdi-importer-image //cmd/cdi-uploadproxy:cdi-uploadproxy-image //cmd/cdi-uploadserver:cdi-uploadserver-image //cmd/ovirt-populator:ovirt-populator-image |
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.
how come this fails? are we really okay with removing?
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.
"make all" is broken without it. Remnant of forklift pr. There is a bogus target listed there
dv.Kind = "DataVolume" | ||
dv.APIVersion = "cdi.kubevirt.io/v1beta1" |
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.
what happens otherwise?
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.
Test fails seems like a change related to controller runtime. You actually added those last time we bumped libraries
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.
no I just copied a function that had those d3f39cb
But anyway I think this is the reasoning kubernetes-sigs/controller-runtime#2633
@awels I think FOSSA is lying, giving this a rerun |
If it fails again, I can investigate the issue with fossa. |
k8s.io/component-helpers v0.29.5 | ||
k8s.io/klog/v2 v2.110.1 | ||
k8s.io/kube-aggregator v0.29.5 | ||
k8s.io/kube-openapi v0.0.0-20231010175941-2dd684a91f00 | ||
k8s.io/utils v0.0.0-20230726121419-3b25d923346b | ||
kubevirt.io/containerized-data-importer-api v0.0.0 | ||
kubevirt.io/controller-lifecycle-operator-sdk v0.2.6 |
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.
Trying to think what we lose by not bumping this to newest k8s/ctrl runtime
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.
At the time I opened this PR, openshift 4.17 branches were still on 1.29 but they were just updated to 1.30 three days ago
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.
Sorry I am talking strictly about the operator sdk library kubevirt.io/controller-lifecycle-operator-sdk v0.2.6
@mhenriks: The following test failed, say
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-sigs/prow repository. I understand the commands that are listed here. |
/hold have to change client generation code for 1.30 |
PR needs rebase. 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-sigs/prow repository. |
closing in favor of #3336 |
What this PR does / why we need it:
Newer libs, not the latest (1.30) because OpenShift isn't using it yet
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #1097
Special notes for your reviewer:
Release note: