Bump kubernetes/client-go #2848

yue9944882 · 2018-02-12T19:20:17Z

What does this PR do?

Briefly, this PR does the following things:

Edit Gopkg.toml: Constraints client-go version to v6 and kubernetes/apixxx to release-1.9 branch.
Align those moved / renamed packages in kubernetes repo.
Strict kubernetes package naming: v1 -> corev1, v1beta1 -> extensionsv1beta1
Fixes removed/deprecated functions since cilent-go 2.0.0.

Motivation

Fixes #2840.

More

Added/updated tests

Additional Notes

ldez · 2018-02-12T19:45:43Z

@yue9944882 we don't really like WIP PR because:

each time you commit, you take 2 executors in our CI (we have only 4)
can block some other changes
we are notifying of each commit

Now, you have done this PR, could you work in another branch, run make test and make validate on your computer and when you are ready, push on this branch.

And could you rebase your PR on master.

ldez · 2018-02-12T19:47:35Z

@yue9944882 please rebase on master and use make dep-ensure

https://github.com/containous/traefik/blob/master/CONTRIBUTING.md#setting-up-dependency-management

yue9944882 · 2018-02-12T19:58:13Z

@ldez Okay, I will only push next time when this PR is completely ready. (Facepalm

timoreimann · 2018-02-12T20:15:12Z

@yue9944882 Each client-go version should be completely backwards compatible, so I suggest we skip version 5 and go straight for version 6.

timoreimann · 2018-02-12T20:17:10Z

In case you missed it: The official blog post on client-go v6 has some details on how to configure dep for v6 properly.

yue9944882 · 2018-02-12T20:49:43Z

@timoreimann @ldez Even though this PR looks extremely large. But actually the changes is small. I will point out these changes in Code Review.

yue9944882

Other unmentioned changes is all about pkg moving or remaning.

yue9944882 · 2018-02-12T20:53:55Z

provider/kubernetes/client.go

-			Namespace(namespace).
-			Resource(resource).
-			VersionedParams(&options, api.ParameterCodec).
-			FieldsSelectorParam(fieldSelector).


This functions FieldsSelectorParam and LabelsSelectorParam is removed. And the selectors are moved into ListOptions

yue9944882 · 2018-02-12T20:54:58Z

provider/kubernetes/client.go

-		panic(err)
-	}
-
+	informer.AddEventHandler(newResourceEventHandler(watchCh))


AddEventHandler has no return value now.

yue9944882 · 2018-02-12T20:56:41Z

provider/kubernetes/client.go

-		fields.Everything(),
-		labelSelector)
-
-	informer := loadInformer(listWatch, &v1beta1.Ingress{}, watchCh)


ListerWatcher is moved from L301 to here.

ldez · 2018-02-12T21:08:22Z

@yue9944882 Could you revert all the changes on the vendor and leave the lock and the toml file and Traefik code, as first step, if possible.

ldez · 2018-02-12T21:09:13Z

If not possible please use make dep-ensure before push.

ldez · 2018-02-12T21:15:22Z

For the continuation, make commits like that.:

one commit for Gopkg.toml and Gopkg.lock
one commit for the Træfik code
one commit for the vendored files (after the review)

Please respect this order, in a first time please don't commit vendored files.

yue9944882 · 2018-02-12T21:21:15Z

@ldez I am sure I've make dep-ensure before pushing the last commit.

Could you revert all the changes on the vendor and leave the lock and the toml file and Traefik code, as first step, if possible.

Actually reverting vendor will fail this PR. When I typed dep ensure -update k8s.io/client-go, dep didn't retrieve the latest dependency (e.g. github.com/juju/ratelimit) correctly which fails compiling of go binary. I think this can a potential BUG of dep. So I had a walk-around using dep ensure -update github.com/juju/ratelimit locally before pushing this PR, then it works again now.

ldez · 2018-02-12T21:23:28Z

Actually reverting vendor will fail this PR.

I know, but for now it's not important. Please revert.

git reset 7d3dd5a0e43b701a8231a8eff8d8024fcc7a5dba
git add Gopkg.*
git commit -m "chore: update dep configuration."
git add provider/ integration/
git commit -m "feat: update Traefik code."

# Do not do that for now 
# git add vendor/
# git commit -m "chore: update vendor."

You need to use dep 0.4.1 minimum, please check you version: dep version.

ldez · 2018-02-12T21:24:42Z

I think this can a potential BUG of dep.

And it's not a bug, it's the behavior of dep for transitive when a dependency don't use dep.

yue9944882 · 2018-02-12T21:42:25Z

git reset 7d3dd5a
git add Gopkg.*
git commit -m "chore: update dep configuration."
git add provider/ integration/
git commit -m "feat: update Traefik code."

@ldez Oops, I didn't notice these edition of the message you leave. How about editing these commit messages altogether after review? Btw I didn't modify files under integration/

yue9944882 · 2018-02-12T21:45:17Z

@ldez @timoreimann PTAL. The tests failing tho because missing upgrades of vendor.

ldez · 2018-02-12T21:45:21Z

Not needed 😉

ldez · 2018-02-12T21:46:04Z

Please wait, we need time to review.
For us, it's the night 😉

ldez

Good job 👍

ldez · 2018-02-12T23:27:55Z

Gopkg.toml

+  name = "k8s.io/apimachinery"
+  version = "kubernetes-1.9.0"
+
+[[constraint]]


Warning: the following project(s) have [[constraint]] stanzas in Gopkg.toml: ✗ k8s.io/apiextensions-apiserver However, these projects are not direct dependencies of the current project: they are not imported in any .go files, nor are they in the 'required' list in Gopkg.toml. Dep only applies [[constraint]] rules to direct dependencies, so these rules will have no effect. Either import/require packages from these projects so that they become direct dependencies, or convert each [[constraint]] to an [[override]] to enforce rules on these projects, if they happen to be transitive dependencies,

Could you convert this constraint to override

It's more likely that we don't need it: k8s.io/apiextensions-apiserver is only required when dealing with things like CRDs, which we don't to the best of my knowledge.

If not used directly, I'd recommend to remove the stanza.

yue9944882 · 2018-02-13T09:38:40Z

@timoreimann I just find that k8s.io/kube-openapi is a transitive dependency by k8s.io/apimachinery. Adding this to constraints.

ldez · 2018-02-13T10:17:35Z

@yue9944882 it's good to me, now you can add the last commit with the vendor.

ldez

LGTM

timoreimann

I gave the PR a quick test drive, validating that the core functionality (including cluster-wide and namespace-specific watches) is working as expected.

PR LGTM -- great job @yue9944882 👏

(Next stop: See if we can use some of the newer client-go features, such as the throttling backoff queue.)

mmatur

LGTM
Nice job @yue9944882 👏

yue9944882 requested a review from a team as a code owner February 12, 2018 19:20

traefiker added area/provider/k8s/ingress size/L status/0-needs-triage labels Feb 12, 2018

yue9944882 changed the title ~~feat(kubernetes): bump kubernetes/client-go to v5.0.1 (1.8 branch)~~ [WIP]feat(kubernetes): bump kubernetes/client-go to v5.0.1 (1.8 branch) Feb 12, 2018

yue9944882 force-pushed the feature/bump-client-go branch from 41545a9 to cd776fc Compare February 12, 2018 19:33

ldez added WIP kind/enhancement a new or improved feature. and removed status/0-needs-triage labels Feb 12, 2018

ldez changed the title ~~[WIP]feat(kubernetes): bump kubernetes/client-go to v5.0.1 (1.8 branch)~~ Bump kubernetes/client-go to v5.0.1 (1.8 branch) Feb 12, 2018

ldez added contributor/waiting-for-corrections and removed WIP labels Feb 12, 2018

ldez changed the title ~~Bump kubernetes/client-go to v5.0.1 (1.8 branch)~~ Bump kubernetes/client-go Feb 12, 2018

yue9944882 force-pushed the feature/bump-client-go branch from 6d1240e to ac8289d Compare February 12, 2018 20:47

yue9944882 commented Feb 12, 2018

View reviewed changes

traefik deleted a comment from yue9944882 Feb 12, 2018

yue9944882 force-pushed the feature/bump-client-go branch from ac8289d to 3939206 Compare February 12, 2018 21:39

ldez requested changes Feb 13, 2018

View reviewed changes

ldez removed the contributor/waiting-for-corrections label Feb 13, 2018

ldez added this to the 1.6 milestone Feb 13, 2018

yue9944882 force-pushed the feature/bump-client-go branch from 328bbdd to d9ecf75 Compare February 13, 2018 09:36

yue9944882 force-pushed the feature/bump-client-go branch from d9ecf75 to 810bc45 Compare February 13, 2018 09:40

ldez approved these changes Feb 13, 2018

View reviewed changes

timoreimann approved these changes Feb 13, 2018

View reviewed changes

mmatur approved these changes Feb 14, 2018

View reviewed changes

mmatur added the status/3-needs-merge label Feb 14, 2018

traefiker added the status/4-merge-in-progress label Feb 14, 2018

yue9944882 added 5 commits February 14, 2018 08:40

bump client-go to v6.0.0

129ce81

fixes broken functions after bumping

d43a3f1

add constraints for k8s.io/kube-openapi

1f246e0

remove k8s.io/kube-openapi: it has only one master branch

06a133a

make dep-ensure

475ff70

traefiker force-pushed the feature/bump-client-go branch from 8eddab1 to 475ff70 Compare February 14, 2018 08:40

traefiker removed the status/4-merge-in-progress label Feb 14, 2018

traefiker merged commit 83a9259 into traefik:master Feb 14, 2018

traefiker removed the status/3-needs-merge label Feb 14, 2018

yue9944882 mentioned this pull request Apr 7, 2018

Introducing kubernetes ratelimit queue for configuration reloading #3147

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump kubernetes/client-go #2848

Bump kubernetes/client-go #2848

yue9944882 commented Feb 12, 2018 •

edited by ldez

ldez commented Feb 12, 2018 •

edited

ldez commented Feb 12, 2018

yue9944882 commented Feb 12, 2018

timoreimann commented Feb 12, 2018

timoreimann commented Feb 12, 2018

yue9944882 commented Feb 12, 2018

yue9944882 left a comment

yue9944882 Feb 12, 2018 •

edited

yue9944882 Feb 12, 2018

yue9944882 Feb 12, 2018

ldez commented Feb 12, 2018 •

edited

ldez commented Feb 12, 2018

ldez commented Feb 12, 2018

yue9944882 commented Feb 12, 2018

ldez commented Feb 12, 2018 •

edited

ldez commented Feb 12, 2018 •

edited

yue9944882 commented Feb 12, 2018 •

edited

yue9944882 commented Feb 12, 2018

ldez commented Feb 12, 2018

ldez commented Feb 12, 2018

ldez left a comment

ldez Feb 12, 2018

timoreimann Feb 13, 2018 •

edited

yue9944882 commented Feb 13, 2018

ldez commented Feb 13, 2018

ldez left a comment

timoreimann left a comment

mmatur left a comment

Bump kubernetes/client-go #2848

Bump kubernetes/client-go #2848

Conversation

yue9944882 commented Feb 12, 2018 • edited by ldez

What does this PR do?

Motivation

More

Additional Notes

ldez commented Feb 12, 2018 • edited

ldez commented Feb 12, 2018

yue9944882 commented Feb 12, 2018

timoreimann commented Feb 12, 2018

timoreimann commented Feb 12, 2018

yue9944882 commented Feb 12, 2018

yue9944882 left a comment

Choose a reason for hiding this comment

yue9944882 Feb 12, 2018 • edited

Choose a reason for hiding this comment

yue9944882 Feb 12, 2018

Choose a reason for hiding this comment

yue9944882 Feb 12, 2018

Choose a reason for hiding this comment

ldez commented Feb 12, 2018 • edited

ldez commented Feb 12, 2018

ldez commented Feb 12, 2018

yue9944882 commented Feb 12, 2018

ldez commented Feb 12, 2018 • edited

ldez commented Feb 12, 2018 • edited

yue9944882 commented Feb 12, 2018 • edited

yue9944882 commented Feb 12, 2018

ldez commented Feb 12, 2018

ldez commented Feb 12, 2018

ldez left a comment

Choose a reason for hiding this comment

ldez Feb 12, 2018

Choose a reason for hiding this comment

timoreimann Feb 13, 2018 • edited

Choose a reason for hiding this comment

yue9944882 commented Feb 13, 2018

ldez commented Feb 13, 2018

ldez left a comment

Choose a reason for hiding this comment

timoreimann left a comment

Choose a reason for hiding this comment

mmatur left a comment

Choose a reason for hiding this comment

yue9944882 commented Feb 12, 2018 •

edited by ldez

ldez commented Feb 12, 2018 •

edited

yue9944882 Feb 12, 2018 •

edited

ldez commented Feb 12, 2018 •

edited

ldez commented Feb 12, 2018 •

edited

ldez commented Feb 12, 2018 •

edited

yue9944882 commented Feb 12, 2018 •

edited

timoreimann Feb 13, 2018 •

edited