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 k8s libraries to 0.19.2 #1305
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
Hi @ivan-valkov. Thanks for your PR. I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
@@ -8,7 +8,7 @@ import ( | |||
"log" | |||
"time" | |||
|
|||
pb "helloworld/proto" | |||
pb "github.com/kubeflow/kfserving/docs/samples/v1alpha2/custom/grpc-server/proto" |
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.
had to update this to make go mod happy
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.
Hey @ivan-valkov , changes look good! 👍 Do you foresee any backwards compatibility issues running in older clusters?
Good question @adriangonz , I do not expect there to be any issues, but let me test some basic workflows on a k8s 1.15 cluster and I will get back to you. |
I tested on a kind cluster and creation and deletion work fine. I think there won't be backwards compatibility issues with updating these libs @adriangonz |
@ivan-valkov can you help sign the cla? |
k8s.io/kubelet => k8s.io/kubelet v0.19.2 | ||
k8s.io/legacy-cloud-providers => k8s.io/legacy-cloud-providers v0.19.2 | ||
k8s.io/metrics => k8s.io/metrics v0.19.2 | ||
k8s.io/sample-apiserver => k8s.io/sample-apiserver v0.19.2 | ||
k8s.io/test-infra => k8s.io/test-infra v0.0.0-20200803112140-d8aa4e063646 | ||
k8s.io/utils => k8s.io/utils v0.0.0-20200324210504-a9aa75ae1b89 | ||
// based on https://github.com/openshift/cluster-ingress-operator/pull/429/files#diff-33ef32bf6c23acb95f5902d7097b7a1d5128ca061167ec0716715b0b9eeaa5f6R34 |
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.
Can you help remove the following controller-runtime
pin and bump up to 0.7.0? I think we no longer need it once move to 1.19.2.
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.
I gave it a try but it turns out we use a bunch of deprecated packages from controller-runtime
that are deleted in 0.7.0
. These are pkg/runtime/log
and pkg/runtime/signals
. The issue mentioned in this comment (kubernetes-sigs/controller-runtime#1033) is fixed only from 0.7.0 onwards. I created #1307 to track the migration away from the deprecated packages and the update of this dependency.
@googlebot I signed it! |
/ok-to-test |
/test kubeflow-kfserving-presubmit Test to see if E2E test works fine without |
6c0f1c0
to
7b34a42
Compare
Hey, hope you had a good weekend. I resolved a conflict with master and tests seem to be passing. Is there anything else we need to get this one merged @yuzisun @PatrickXYS |
/test kubeflow-kfserving-presubmit |
|
/test kubeflow-kfserving-presubmit |
/test kubeflow-kfserving-presubmit Testing to see if the 401 occurs again. |
/retest |
1 similar comment
/retest |
/test ? |
@PatrickXYS: The following commands are available to trigger jobs:
Use In response to this:
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. |
/test kubeflow-kfserving-presubmit |
Please rebase master branch to make sure it updates test code |
/retest |
Still |
7b34a42
to
7a3f011
Compare
just rebased on master. Let's retest this :) |
/retest |
|
/lgtm |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ivan-valkov, yuzisun 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 |
* parent a0a52f8 author Manasvi Tickoo <MTICKOO@bloomberg.net> 1611275174 -0500 committer Manasvi Tickoo <MTICKOO@bloomberg.net> 1612370131 -0500 1159 - cloudevent support ofr kfserving PR comments Fix clpudevent headers Add unit test for cloud event messages Add avro unit test Add avro to kfserving requirements.txt PR comments kfserving check ce-contenttyp before unmarshalling Update k8s libraries to 0.19.2 (#1305) * go mod tidy * Update k8s libs to 0.19.2, fix issues, and run make tests * Handle ce to_binary return type wile building resp
What this PR does / why we need it:
This PR updates our k8s dependencies to 0.19. It will provide compatibility with 0.19 k8s versions and allow projects using kfserving internally to have an easier migration to 0.19.
Only a small modification was needed to be able to use 0.19 - updating the copy of podSpec we have.
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 #1293
Special notes for your reviewer:
I have tested this change by running
make test
. All the formatting changes were made from runningmake test
.Release note: