-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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 go.mod for k8s 1.17 #8873
Update go.mod for k8s 1.17 #8873
Conversation
BTW here's a video of how to update these, except that I don't think we need to any more because it looks like upstream has (mostly?) fixed their go.mods, so now we can just use v0.17.4 instead of kubernetes-1.17.4, and that value isn't mangled by go.mod: https://asciinema.org/a/317782 |
cc @rifelpet on the video. The other steps are to run |
Thanks @justinsb! /lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: justinsb, mikesplain 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 |
go.mod
Outdated
k8s.io/cli-runtime v0.0.0 | ||
k8s.io/api v0.17.4 | ||
k8s.io/apimachinery v0.17.4 | ||
k8s.io/cli-runtime v0.17.4 | ||
k8s.io/client-go v11.0.1-0.20190409021438-1a26190bd76a+incompatible |
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.
thanks, the video was very helpful! can you explain why this version didn't (need to) change? It seems all of the k8s.io packages are covered by replace
s above, but this, kubectl, and legacy-cloud-providers below didn't change.
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.
Hmmm ... that's a good point and is just my mistake! Will hold the PR!
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.
Ah actually, I take it back...
The replace directives at the top take precedence, and we do have:
replace k8s.io/client-go => k8s.io/client-go v0.17.4
client-go started adopted a different versioning scheme though, and it looks like it isn't fully sorted out yet. That confuses go.mod which then puts some random version in the directives.
I was able to override the version in go.mod manually, so it now looks intuitive, even though that version wasn't used before.
We'll likely have to do this manually going forwards (but if we forget, it shouldn't really matter)
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.
ah ok. and does the same apply for kubectl and legacy-cloud-providers ?
/hold |
These are now tagged with v0.17.4, so we don't need the trick of the commented out imports any more!
It isn't meaningful as a kops flag.
d04bbcd
to
dfb75b8
Compare
/unhold |
seems like the answer to my last question is yes. /lgtm I propose we let this sit for a bit to let periodic e2e jobs run, then cherry-pick back to 1.17 |
These are now tagged with v0.17.4, so we don't need the trick of the
commented out imports any more!