-
Notifications
You must be signed in to change notification settings - Fork 108
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 Kubernetes dependencies to 1.18.3 #318
Conversation
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.
Couple questions around versions, but I'm likely just missing context.
k8s.io/kubelet => k8s.io/kubelet v0.17.5 | ||
k8s.io/legacy-cloud-providers => k8s.io/legacy-cloud-providers v0.17.5 | ||
k8s.io/metrics => k8s.io/metrics v0.17.5 | ||
k8s.io/api => k8s.io/api v0.18.3 |
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.
Is this replace
necessary, given that k8s.io/api v0.18.3
is in the require
above?
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.
It should not be, and a few other direct dependencies of ours probably don't need the replacement either. I think it's a consequence of how we upgrade dependencies through our script, which we need because we have to vendor k8s.io/kubernetes for a resizing library, and that requires replacing all of the monorepo's dependencies in the fashion we do.
We could exclude the direct dependencies from the script, but it'd involve a bit effort for I think relatively little gain. AFAIK, upstream is planning on moving the resize logic into a separate repository as well; at that point, we can simplify our dependency management process by referencing direct dependencies only and letting gomod pull in just the transitive dependencies we need.
go.mod
Outdated
k8s.io/metrics => k8s.io/metrics v0.17.5 | ||
k8s.io/api => k8s.io/api v0.18.3 | ||
k8s.io/apiextensions-apiserver => k8s.io/apiextensions-apiserver v0.18.3 | ||
k8s.io/apimachinery => k8s.io/apimachinery v0.18.4-rc.0 |
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.
Is this intentionally not 0.18.3
?
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.
This one's a bit odd: our upgrade script puts in a replacement rule to v0.18.3 properly. It's only when go mod tidy
runs that k8s.io/apimachinery is replaced for v0.18.4-rc.0.
I played around with go mod why
and go mod graph
but couldn't find any indicators for why the release candidate would be referenced. I wonder if this is a bug. 🤔
On the bright side, changing the replacement to v0.18.3 and running go mod tidy
afterwards seems to work in the sense that the rc version is not replaced again. That'd work for me for now unless you may have another idea on what we can or should do here.
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 think I understand now: k8s.io/[email protected] and k8s.io/[email protected] point at the same commit. It seems similar with a few other dependencies.
I forced all of them into v0.18.3; makes sense to me to be consistent.
c6b5e87
to
e7d8bf0
Compare
No description provided.