Informer framework #4

sttts · 2016-09-10T12:37:16Z

To write third-party controllers having ~~object stores and~~ the informer framework would be very handy.

sttts · 2016-09-10T12:39:02Z

Object stores are included: https://github.com/kubernetes/client-go/tree/b7b3ea539440e6d0efc6ef4dab644108016788c0/1.4/tools/cache

timothysc · 2016-09-12T13:16:54Z

Many folks would like to refactor or eliminate the current informer entirely.

deads2k · 2016-09-12T13:31:51Z

Many folks would like to refactor or eliminate the current informer entirely.

Can you elaborate on this? I know that some people have suggested doing things differently, but what I've seen has only differed in implementation, nothing significant in the API interfaces. I also haven't seen much action on it.

wojtek-t · 2016-09-12T13:38:28Z

Yeah - I'm also completely not aware of people that want to remove informer (though we obviously can improve it)

timothysc · 2016-09-12T13:39:59Z

@hongchaodeng and @xiang90 and proposed a refactoring to the informer in the 1.4 cycle that was pushed out. I'd have to dig up the issue number.

s/Eliminate/Choose not to use - Some folks that have designed their own controllers, simply do not use the informer.

wongma7 · 2016-09-14T14:59:57Z

Is this the proposal in question? kubernetes/kubernetes#27519

+1 to this anyway, I would rather use the same thing upstream controllers use, whatever flaws it has, than attempt to roll my own

deads2k · 2016-09-14T15:21:49Z

Is this the proposal in question? kubernetes/kubernetes#27519

+1 to this anyway, I would rather use the same thing upstream controllers use, whatever flaws it has, than attempt to roll my own

That proposal isn't what kube uses. Kube uses the SharedInformers as they exist today. The proposal didn't interoperate well with the existing DeltaFifo structure that has done a reasonable job so far.

lavalamp · 2016-09-14T22:24:41Z

This was an oversight, we intended to include this library, too.

lavalamp · 2016-09-14T22:25:28Z

It looks like @mikedanese is moving it anyway, so it should show up soon.

jimmidyson · 2016-09-21T09:59:10Z

Any news on this? Chomping at the bit to migrate over to this & really would like the informer framework in place to simplify migration.

mikedanese · 2016-09-21T15:06:47Z

kubernetes/kubernetes#32718 Merged. This should hit the 1.5 client once it's synced.

jimmidyson · 2016-09-21T15:28:58Z

So this client gets automatically synced from kube repo then? How often does that happen?

lavalamp · 2016-09-21T16:51:44Z

~ daily. But I need to approve a PR before the 1.5 client will show up here
:)

On Wed, Sep 21, 2016 at 8:28 AM, Jimmi Dyson notifications@github.com
wrote:

So this client gets automatically synced from kube repo then? How often
does that happen?

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#4 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAnglguclN5PZIetrQUnIovIZdRFiLAIks5qsU06gaJpZM4J5tgT
.

seh · 2016-09-25T20:31:38Z

If I'm looking to confine my Kubernetes dependencies to this repository, what's a good replacement for workqueue.Interface from "k8s.io/kubernetes/pkg/util/workqueue”?

Type cache.FIFO from “k8s.io/client-go/1.4/tools/cache” is similar, but I don’t see a way to unblock a caller waiting on FIFO.Pop, nor a way to shut the FIFO down. Is there a better choice for replacing workqueue.Interface?

fabxc · 2016-09-26T11:00:40Z

@lavalamp The 1.5/ sub directory is in but it doesn't include Informers. They are neither in the staging path in kubernetes/kubernetes.

Anything we could do to speed this up? It's blocking/complicating some high priority stuff for us.

caesarxuchao · 2016-09-26T16:50:00Z

The informer will show up after kubernetes/kubernetes#33334 gets merged.

We need to manually run staging/src/k8s.io/client-go/copy.sh to populate changes to the staging area, then they will be published to client-go.

lavalamp · 2016-09-26T16:51:16Z

@caesarxuchao can we include the workqueue, as well?

seh · 2016-09-26T16:54:44Z

I've been working today on porting my code that relies on workqueue to use cache.FIFO, with some awkwardness, so if workqueue could come across I'd stick with it.

lavalamp · 2016-09-26T16:56:04Z

Yeah, workqueue and cache.FIFO are somewhat different animals-- we should
include workqueue.

On Mon, Sep 26, 2016 at 9:54 AM, Steven E. Harris notifications@github.com
wrote:

I've been working today on porting my code that relies on workqueue to
use cache.FIFO, with some awkwardness, so if workqueue could come across
I'd stick with it.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#4 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAnglhDqHp2HQfWKLYhRmwu_M9mbETIYks5qt_jVgaJpZM4J5tgT
.

seh · 2016-09-27T20:00:45Z

I see that kubernetes/kubernetes#33334 merged today. Is it the case that the next run of copy.sh will put the informer stuff into this repository?

caesarxuchao · 2016-09-27T20:59:41Z

#33334 ran the copy.sh. The next run (every 12 hours) of the bot will push the change to client-go. workqueue is still missing though.

seh · 2016-09-28T00:38:57Z

Thar she blows! Commit 5974905 includes cache.NewInformer—though only in version 1.5, as hinted by @mikedanese earlier.

Should we expect it to arrive in the version 1.4 subtree soon, or is that out of scope?

caesarxuchao · 2016-09-28T02:41:32Z

Should we expect it to arrive in the version 1.4 subtree soon, or is that out of scope?

Depending on how bad people want it. It requires cherrypicking kubernetes/kubernetes#32718 and kubernetes/kubernetes#33334, i.e., it requires some labor.

mqliang · 2016-09-28T03:21:24Z

@caesarxuchao files under 1.5/tools/cache/ are in package testing, instead of cache, is it intended?

see https://github.com/kubernetes/client-go/blob/master/1.5/tools/cache/controller.go#L17

caesarxuchao · 2016-09-28T03:42:37Z

No, it's a bug. I'll take a look.

seh · 2016-09-28T13:06:22Z

@caesarxuchao, I'm not sure when it's safe for me to start using the 1.5 subtree. Is it guaranteed to work against a Kubernetes version 1.3 cluster? That hesitation is what had me using the 1.4 subtree.

lavalamp · 2016-09-28T17:21:40Z

The 1.5 subtree is backwards compatible. However, the go interface will not
be frozen until 1.5 is released, i.e., it's mutable--tracking head--until
then.

So if you use the 1.5 client, be aware that we may make interface changes
that require you to change your code. We haven't done this yet, but we
probably will.

I'm totally open to cherrypicking client changes into the 1.4 client
branch-- now that 1.4 is released we should be able to do that without
getting the evil eye from the release czar. Maybe after Chao fixes the
package name. :)

The thing we absolutely cannot do is change the interface in the 1.4
client--no changes there that will make folks who import it change their
code. We only make changes like that over version boundaries.

On Wed, Sep 28, 2016 at 6:06 AM, Steven E. Harris notifications@github.com
wrote:

@caesarxuchao https://github.com/caesarxuchao, I'm not sure when it's
safe for me to start using the 1.5 subtree. Is it guaranteed to work
against a Kubernetes version 1.3 cluster? That hesitation is what had me
using the 1.4 subtree.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#4 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAngljd_txC5GQEN3W3lXAeJ0kOnDRjDks5qumZPgaJpZM4J5tgT
.

seh · 2016-09-28T17:33:02Z

Thank you for the explanation. I would appreciate a back-port, should one arrive soon, but in the meantime I'll try using the 1.5 client and see how it goes.

jimmidyson · 2016-09-28T17:35:51Z

no changes there that will make folks who import it change their
code. We only make changes like that over version boundaries.

As you're versioning this in line with kubernetes itself, this obviously goes against semver. Not that semver is a panacea but at least it gives a feel for API stability.

lavalamp · 2016-09-28T17:55:06Z

Indeed, that's a great point. Maybe we should version the go client
independently. I've put this on my list of things to think about.

On Wed, Sep 28, 2016 at 10:35 AM, Jimmi Dyson notifications@github.com
wrote:

no changes there that will make folks who import it change their
code. We only make changes like that over version boundaries.

As you're versioning this in line with kubernetes itself, this is
obviously goes against semver. Not that semver is a panacea but at least it
gives a feel for API stability.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#4 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAnglhk5jZsGORqbnoopeI3JKj2TtYq7ks5quqV3gaJpZM4J5tgT
.

fabxc · 2016-09-28T18:07:29Z

As it's pulled together from the kubernetes repo, independent versioning would ultimately leek into that. Even if intentionally developing on the most recent version, having the the sanitized package layout is valuable.

Maybe it's simplest to stick with freezing together with kubernetes but calling the non-frozen version "nightly" or "head"?

lavalamp · 2016-09-28T19:49:21Z

We talked a fair amount about this at the API Machinery SIG just now. I
think we definitely need to make it more clear what we're doing here.

Note that we haven't actually broken compatibility between 1.4 and 1.5 yet
so technically we're still following semver :)

On Wed, Sep 28, 2016 at 11:07 AM, Fabian Reinartz notifications@github.com
wrote:

As it's pulled together from the kubernetes repo, independent versioning
would ultimately leek into that. Even if intentionally developing on the
most recent version, having the the sanitized package layout is valuable.

Maybe it's simplest to stick with freezing together with kubernetes but
calling the non-frozen version "nightly" or "head"?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#4 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAngltdkhzYiuugtc1ChQYeyX1Cq8m94ks5quqzigaJpZM4J5tgT
.

caesarxuchao · 2016-09-29T01:59:22Z

I'll keep it open until we move workqueue.

lavalamp · 2016-09-29T16:38:53Z

I'm not quite sure why github closed this?

…heus Automatic merge from submit-queue decouple workqueue metrics from prometheus  **What this PR does / why we need it**: We want to include the workqueue in client-go, but do not want to having to import Prometheus. This PR decouples the workqueue from prometheus. **Which issue this PR fixes** *(optional, in `fixes #<issue number>(, #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes # Partially address #33497 User requested for `workqueue` in client-go: kubernetes/client-go#4 (comment) **Special notes for your reviewer**: **Release note**:  ```release-note The implicit registration of Prometheus metrics for workqueue has been removed, and a plug-able interface was added. If you were using workqueue in your own binaries and want these metrics, add the following to your imports in the main package: "k8s.io/pkg/util/workqueue/prometheus". ```

caesarxuchao · 2017-01-09T10:02:34Z

informers and workqueue are in the master branch now. Closing.

…heus Automatic merge from submit-queue decouple workqueue metrics from prometheus  **What this PR does / why we need it**: We want to include the workqueue in client-go, but do not want to having to import Prometheus. This PR decouples the workqueue from prometheus. **Which issue this PR fixes** *(optional, in `fixes #<issue number>(, #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes # Partially address kubernetes/kubernetes#33497 User requested for `workqueue` in client-go: kubernetes/client-go#4 (comment) **Special notes for your reviewer**: **Release note**:  ```release-note The implicit registration of Prometheus metrics for workqueue has been removed, and a plug-able interface was added. If you were using workqueue in your own binaries and want these metrics, add the following to your imports in the main package: "k8s.io/pkg/util/workqueue/prometheus". ```

sttts added the enhancement label Sep 10, 2016

sttts changed the title ~~Strores and informer~~ Informer framework Sep 10, 2016

MrHohn mentioned this issue Sep 12, 2016

Kube-dns should use versioned client-go kubernetes/kubernetes#31929

Closed

aledbf mentioned this issue Sep 22, 2016

middleware/kubernetes: -ENOCOMPILE coredns/coredns#277

Closed

caesarxuchao mentioned this issue Sep 26, 2016

Include workqueue in client-go kubernetes/kubernetes#33497

Closed

fabxc mentioned this issue Sep 27, 2016

use kubernetes informer framework kubernetes/kube-state-metrics#30

Merged

caesarxuchao mentioned this issue Sep 28, 2016

Fix package name bug kubernetes/kubernetes#33632

Merged

k8s-github-robot closed this as completed in kubernetes/kubernetes#33632 Sep 29, 2016

caesarxuchao reopened this Sep 29, 2016

soltysh closed this as completed in soltysh/kubernetes@1e5e4fd Sep 29, 2016

lavalamp reopened this Sep 29, 2016

caesarxuchao mentioned this issue Sep 29, 2016

decouple workqueue metrics from prometheus kubernetes/kubernetes#33792

Merged

caesarxuchao closed this as completed Jan 9, 2017

Informer framework #4

Informer framework #4

Comments

sttts commented Sep 10, 2016 • edited

sttts commented Sep 10, 2016

timothysc commented Sep 12, 2016

deads2k commented Sep 12, 2016

wojtek-t commented Sep 12, 2016

timothysc commented Sep 12, 2016 • edited

wongma7 commented Sep 14, 2016

deads2k commented Sep 14, 2016

lavalamp commented Sep 14, 2016

lavalamp commented Sep 14, 2016

jimmidyson commented Sep 21, 2016

mikedanese commented Sep 21, 2016

jimmidyson commented Sep 21, 2016

lavalamp commented Sep 21, 2016

seh commented Sep 25, 2016

fabxc commented Sep 26, 2016

caesarxuchao commented Sep 26, 2016 • edited

lavalamp commented Sep 26, 2016

seh commented Sep 26, 2016

lavalamp commented Sep 26, 2016

seh commented Sep 27, 2016 • edited

caesarxuchao commented Sep 27, 2016

seh commented Sep 28, 2016 • edited

caesarxuchao commented Sep 28, 2016

mqliang commented Sep 28, 2016 • edited

caesarxuchao commented Sep 28, 2016

seh commented Sep 28, 2016

lavalamp commented Sep 28, 2016

seh commented Sep 28, 2016

jimmidyson commented Sep 28, 2016 • edited

lavalamp commented Sep 28, 2016

fabxc commented Sep 28, 2016

lavalamp commented Sep 28, 2016

caesarxuchao commented Sep 29, 2016

lavalamp commented Sep 29, 2016

caesarxuchao commented Jan 9, 2017

sttts commented Sep 10, 2016 •

edited

timothysc commented Sep 12, 2016 •

edited

caesarxuchao commented Sep 26, 2016 •

edited

seh commented Sep 27, 2016 •

edited

seh commented Sep 28, 2016 •

edited

mqliang commented Sep 28, 2016 •

edited

jimmidyson commented Sep 28, 2016 •

edited