Skip to content
This repository has been archived by the owner on Jul 6, 2023. It is now read-only.

Bump kubernetes dependencies to client-go 1.10/k8s 1.13 #1542

Closed
wants to merge 3 commits into from

Conversation

munnerz
Copy link

@munnerz munnerz commented Mar 2, 2019

What does this PR achieve? Why do we need it?

This pull-request follows up on #1272 and bumps client-go to 1.10 (i.e. the latest).

From my understanding of the previous pull request, this should help unblock work on getting kubernetes/kubernetes to use go modules eventually 馃槃

The changes/concerns expressed in @sigma's comment: #1272 (comment) are not issues, as the Kubernetes API types as serialised to JSON are still identical, despite the structure names changing.

/cc @sigma @phlogistonjohn @obnoxxx

sigma and others added 2 commits March 2, 2019 00:17
Also, it removes dependency to kubernetes/kubernetes, which is not meant to be
vendored, in favor of additional dependencies to kubernetes/api and
kubernetes/apimachinery, which are.
Incidentally, this reduces the overall dependency graph by a fair amount.
@openshift-ci-robot
Copy link
Collaborator

Hi @munnerz. Thanks for your PR.

I'm waiting for a heketi member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

@centos-ci
Copy link
Collaborator

Can one of the admins verify this patch?

@munnerz munnerz mentioned this pull request Mar 2, 2019
@munnerz
Copy link
Author

munnerz commented Mar 2, 2019

This seems to be passing on Travis 馃槃

As @sigma described, Kubernetes is using Go features that were only introduced in 1.10 (strings.Builder). I've therefore disable travis tests using Go 1.9 as part of this PR, and added Go 1.12 tests.

@munnerz
Copy link
Author

munnerz commented Mar 2, 2019

I am able to build the pkg/volume/glusterfs package using this ref of heketi/heketi 馃帀 https://github.com/kubernetes/kubernetes/compare/master...munnerz:bump-heketi?expand=1

@raghavendra-talur
Copy link
Member

/ok-to-test

@liggitt
Copy link
Contributor

liggitt commented Mar 19, 2019

eyes on this would be appreciated. I'm looking to move forward with go modules in k8s 1.15 and getting this ironed out would be helpful

@munnerz
Copy link
Author

munnerz commented Mar 19, 2019

/retest

@phlogistonjohn
Copy link
Contributor

retest this please

@phlogistonjohn
Copy link
Contributor

(centos ci uses a different magic incantation ;-))

@phlogistonjohn
Copy link
Contributor

phlogistonjohn commented Mar 19, 2019

One thing we need to sort out with the PR is the spate of unrelated changes that occurs in glide. This is perpetually an issue with heketi (at least how we use glide). I'm not against updating dependencies but we should not be updating a bunch of stuff unrelated to the work happening for kube specific stuff in this PR.
If someone wanted to accelerate this I'd appreciate it someone could look into possibly making a PR that first does a general update and/or locks down more of our depencies. Then, we can base new PRs that need to update dependencies upon that work.

@obnoxxx
Copy link
Contributor

obnoxxx commented Mar 22, 2019

I fully agree, the patch looks good. But we need to untangle the glide.lock changes.

@obnoxxx
Copy link
Contributor

obnoxxx commented Mar 23, 2019

@munnerz I (and I guess others) are more than happy to help with the reworking of the glide.lock patch, if you don't have the time or motivation to do it. :-)

PS: we really have to get rid of glide...

@obnoxxx
Copy link
Contributor

obnoxxx commented Mar 23, 2019

The commit Bump to client-go 1.10 should read v10.0.0 instead 1.10 (and so should the PR title, or what am I missing.

Also, we could squash the two patches for moving to v8.0.0 and then to v10.0.0 into one unless there's a good reason to keep the two steps separate. (If you wanted to keep the authorship of the original comment, I could live with that.)

@liggitt
Copy link
Contributor

liggitt commented Sep 9, 2019

@munnerz any chance of reviving this (maybe on v1.15 or v1.16)? looks like this blocks getting k8s.io/kubernetes to go1.13 (xref kubernetes/kubernetes#82506)

@liggitt
Copy link
Contributor

liggitt commented Sep 12, 2019

updated to 1.15.3 and rebased in #1649

@phlogistonjohn
Copy link
Contributor

PR is obsoleted by #1649 which was merged. Closing this PR as the goals of this PR are now met. Thank you for your contribution.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants