Add an Informer example in client-go #44446

wallrj · 2017-04-13T14:30:29Z

This PR adds an example demonstrating how to use the client-go library to set up an Informer to print out Pod events and to maintain a local store containing Pod configuration synchronized with the Kubernetes API server.

I've copied the work started in kubernetes/client-go#30 and addressed most of the outstanding code review comments:
kubernetes/client-go#30

k8s-ci-robot · 2017-04-13T14:30:34Z

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://github.com/kubernetes/kubernetes/wiki/CLA-FAQ to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.

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. I understand the commands that are listed here.

k8s-reviewable · 2017-04-13T14:30:37Z

This change is

k8s-ci-robot · 2017-04-13T14:30:37Z

Hi @wallrj. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with @k8s-bot 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.

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. I understand the commands that are listed here.

k8s-github-robot · 2017-04-13T14:30:50Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: wallrj
We suggest the following additional approver: @smarterclayton

Needs approval from an approver in each of these OWNERS Files:

staging/src/k8s.io/client-go/OWNERS

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

wallrj · 2017-04-13T16:00:31Z

I'm struggling to sign the CLA as an employee of jetstack.io.
The branch originally contained commits with my generic email github project email address wallrj@users.noreply.github.com but I've force pushed a new single commit using my jetstack.io address.
(I'm told that jetstack.io have completed the corporate CLA process for CNCF.)

grodrigues3 · 2017-04-13T18:14:02Z

@k8s-bot ok to test

caesarxuchao · 2017-04-13T21:37:58Z

FWIW, #44320 adds an example showing how to use workqueue together with informer. Its emphasis is on the use of the workqueue.

caesarxuchao · 2017-04-13T21:30:09Z

staging/src/k8s.io/client-go/examples/informer/main.go

+	"k8s.io/client-go/tools/clientcmd"
+
+	// Only required to authenticate against GKE clusters
+	_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"


Can we keep this line but comment it out? This line recursively introduces a huge dependency.

caesarxuchao · 2017-04-13T21:43:18Z

staging/src/k8s.io/client-go/examples/informer/main.go

+	s := <-signals
+	fmt.Printf("received signal %#v, exiting...\n", s)
+	close(stop)
+	os.Exit(0)


Curious why os.Exit(0) is needed here?

I think the intent was to exit with status 0 after handling SIGINT.
Default behaviour is to exit with 1.
I agree though, it distracts from the Informer code.
Removed.

ash2k · 2017-04-14T01:07:44Z

staging/src/k8s.io/client-go/examples/informer/main.go

+	source := cache.NewListWatchFromClient(
+		clientset.Core().RESTClient(),
+		"pods",
+		api.NamespaceAll,


Should it be a versioned package like meta/v1 or api/v1? They also have this constant.

ash2k · 2017-04-14T01:09:18Z

staging/src/k8s.io/client-go/examples/informer/main.go

+		panic(err)
+	}
+
+	stop := make(chan struct{}, 1)


Doesn't have to be buffered?

Agreed. Done.

rmohr · 2017-04-21T14:09:21Z

staging/src/k8s.io/client-go/examples/informer/main.go

+	return fmt.Sprintf("%s/%s", pod.Namespace, pod.Name)
+}
+
+func buildConfig(kubeconfig string) (*rest.Config, error) {


This helper method is not needed, BuildConfigFromFlags, checks if a masterUrl or a kubeconfigPath is set. If both are unset, it assumes that it is running from inside the cluster.

rmohr · 2017-04-21T14:10:57Z

staging/src/k8s.io/client-go/examples/informer/main.go

+	// Create the client config. Use kubeconfig if given, otherwise assume in-cluster.
+	config, err := buildConfig(*kubeconfig)
+	if err != nil {
+		panic(err)


You can use glog.Fatal(err)

rmohr · 2017-04-21T14:11:06Z

staging/src/k8s.io/client-go/examples/informer/main.go

+
+	clientset, err := kubernetes.NewForConfig(config)
+	if err != nil {
+		panic(err)


You can use glog.Fatal(err)

0xmichalis · 2017-04-21T14:23:04Z

@rmohr already added an example here

caesarxuchao · 2017-04-21T17:46:10Z

@Kargakis the one you linked to emphasis more on the workqueue and is more complex. I think it doesn't hurt to have a simpler example of how to use informer alone. WDYT?

caesarxuchao · 2017-04-21T17:48:51Z

@wallrj a few commands you need to run to pass the verification script:

W0418 01:42:06.769] 1 BUILD files not up-to-date.
I0418 01:42:06.870] 
I0418 01:42:06.870] Run ./hack/update-bazel.sh

I0418 01:53:26.662] FAILED   hack/make-rules/../../hack/verify-golint.sh	37s
I0418 01:53:26.662] Skipping hack/make-rules/../../hack/verify-govet.sh
I0418 01:53:26.662] Verifying hack/make-rules/../../hack/verify-import-boss.sh
W0418 01:53:26.763] Some packages passed golint but are not listed in hack/.linted_packages.
W0418 01:53:26.763] Please add them in alphabetical order:
W0418 01:53:26.763] 
W0418 01:53:26.764]   echo staging/src/k8s.io/client-go/examples/informer >> hack/.linted_packages

Also, the CLA check is also failing, have you signed it?

wallrj · 2017-05-02T08:55:03Z

Sorry for the delay. And thanks for the review @rmohr and @caesarxuchao.
I have now signed the CLA and I'll address your comments this week.

ncdc · 2017-05-31T13:30:15Z

staging/src/k8s.io/client-go/examples/informer/main.go

+}
+
+func (c *PodLoggingController) podDelete(obj interface{}) {
+	pod := obj.(*v1.Pod)


This might be a cache.DeletedFinalStateUnknown, so you'll need to check for that as well. If you grep in pkg/controller, you'll find examples.

ncdc · 2017-05-31T13:34:23Z

staging/src/k8s.io/client-go/examples/informer/main_test.go

+				clientset := fake.NewSimpleClientset(tc.initial...)
+				factory := informers.NewSharedInformerFactory(
+					clientset,
+					time.Hour*24,


You can also just do 0 for no resync

ncdc · 2017-05-31T13:44:12Z

staging/src/k8s.io/client-go/examples/informer/main_test.go

+					// Type conversion + type assertion? Is this the only way?
+					p := interface{}(podToAdd).(*v1.Pod)
+					// XXX: I expected this to trigger the
+					// cache.ResourceEventHandlerFuncs but it doesn't.


The informer does a list, followed by a watch. The list will only return what's initially known by the fake clientset (tc.initial). If you want to trigger the event handler functions after the initial list has completed, you'll have to use a fake watch, such as:

fakeWatch := watch.NewFake() clientset.PrependWatchReactor("pods", core.DefaultWatchReactor(fakeWatch, nil)) // core = "k8s.io/client-go/testing" fakeWatch.Add(&newPod) // confirm that the event handler fired

ncdc · 2017-05-31T13:45:17Z

@wallrj I added a line comment showing you how to use the fake watch to get the event handler functions to fire. Let me know if you need more help.

krmayankk · 2017-06-08T07:44:12Z

@ncdc @caesarxuchao i wouldn't block on a great example if this would help a lot of people using the informer. This is an example code. i am guessing @wallrj would loose motivation at some point :-)

caesarxuchao · 2017-06-13T22:32:15Z

Sorry i'm working on 1.7 release items. I'll try to take another look this week.

ahmetb · 2017-06-13T23:41:40Z

staging/src/k8s.io/client-go/examples/informer/README.md

+```sh
+go run main.go -kubeconfig=$HOME/.kube/config -logtostderr
+```
+


Can you explain what happens when someone runs this program? Example of this is at: https://github.com/kubernetes/client-go/tree/master/examples/create-update-delete-deployment We're trying to keep READMEs of all the client examples similar to each other.

ahmetb · 2017-06-13T23:42:46Z

staging/src/k8s.io/client-go/examples/informer/README.md

+* Building controllers that coordinate other resources.
+  Most controllers in [k8s.io/kubernetes/pkg/controller](https://godoc.org/k8s.io/kubernetes/pkg/controller) use informers.
+* Capturing resource events for logging to external systems
+  (e.g. monitor non-"Normal" events and publish metrics to a time series database)


Can you add a ## Cleanup section people can run to clean up the artifacts created by running this example? (For instance: https://github.com/kubernetes/client-go/tree/master/examples/create-update-delete-deployment or https://github.com/kubernetes/client-go/tree/master/examples/in-cluster has this section).

This will help us keep the README files for each client-go example similar.

k8s-ci-robot · 2017-08-01T13:03:24Z

@wallrj: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-kubernetes-bazel	`a1e2777`	link	`/test pull-kubernetes-bazel`
pull-kubernetes-unit	`a1e2777`	link	`/test pull-kubernetes-unit`
pull-kubernetes-verify	`a1e2777`	link	`/test pull-kubernetes-verify`
pull-kubernetes-e2e-gce-etcd3	`a1e2777`	link	`/test pull-kubernetes-e2e-gce-etcd3`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

k8s-github-robot · 2017-08-30T21:47:28Z

Adding do-not-merge/release-note-label-needed because the release note process has not been followed.
One of the following labels is required "release-note", "release-note-action-required", "release-note-experimental" or "release-note-none".
Please see: https://github.com/kubernetes/community/blob/master/contributors/devel/pull-requests.md#write-release-notes-if-needed.

k8s-github-robot · 2017-10-31T08:09:40Z

This PR hasn't been active in 90 days. Closing this PR. Please reopen if you would like to work towards merging this change, if/when the PR is ready for the next round of review.

cc @caesarxuchao @wallrj

You can add 'keep-open' label to prevent this from happening again, or add a comment to keep it open another 90 days

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Apr 13, 2017

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Apr 13, 2017

k8s-github-robot assigned derekwaynecarr and smarterclayton Apr 13, 2017

k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-label-needed labels Apr 13, 2017

wallrj mentioned this pull request Apr 13, 2017

Add Informer examples kubernetes/client-go#30

Closed

wallrj force-pushed the informer-example branch from 1d601f8 to 32faf96 Compare April 13, 2017 15:53

k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Apr 13, 2017

mbohlool assigned mbohlool and unassigned smarterclayton and derekwaynecarr Apr 13, 2017

caesarxuchao reviewed Apr 13, 2017

View reviewed changes

ash2k reviewed Apr 14, 2017

View reviewed changes

wallrj force-pushed the informer-example branch from 32faf96 to ddd9f5f Compare April 18, 2017 08:34

mbohlool assigned caesarxuchao and unassigned mbohlool Apr 18, 2017

rmohr reviewed Apr 21, 2017

View reviewed changes

ncdc reviewed May 31, 2017

View reviewed changes

caesarxuchao mentioned this pull request Jun 13, 2017

clientgo/examples: add ToC for examples #46883

Merged

ahmetb suggested changes Jun 13, 2017

View reviewed changes

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 21, 2017

wallrj added 11 commits July 31, 2017 10:02

Add an Informer example in client-go

dc1fd3d

Hopefully this version is clearer.

28a17cb

oops, I'd disabled the resync

6981572

update-bazel

e528f05

Use a longer resync period

c48e6af

Use factory.Start

298855e

Make the example more like a Kubernetes controller and add some tests.

bf27fc0

./hack/update-bazel.sh

8a6807f

Add missing boilerplate

e6f305a

Document public functions

2322b4a

simulate pods being added

de6bf81

wallrj force-pushed the informer-example branch from b2aca40 to de6bf81 Compare July 31, 2017 09:11

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 31, 2017

wallrj added 5 commits July 31, 2017 10:23

Use new k8s/io/api repo

d384e37

update-bazel.sh

ccd0ab3

Never resync

fa601c1

Experimenting with fuzz testing.

fa6852f

Restrict the names and namespaces and perform delete operations too

a1e2777

k8s-github-robot added the do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. label Aug 30, 2017

k8s-github-robot closed this Oct 31, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add an Informer example in client-go #44446

Add an Informer example in client-go #44446

wallrj commented Apr 13, 2017 •

edited by k8s-github-robot

k8s-ci-robot commented Apr 13, 2017

k8s-reviewable commented Apr 13, 2017

k8s-ci-robot commented Apr 13, 2017

k8s-github-robot commented Apr 13, 2017

wallrj commented Apr 13, 2017

grodrigues3 commented Apr 13, 2017

caesarxuchao commented Apr 13, 2017

caesarxuchao Apr 13, 2017

wallrj Apr 18, 2017

caesarxuchao Apr 13, 2017

wallrj Apr 18, 2017

ash2k Apr 14, 2017

wallrj Apr 18, 2017

ash2k Apr 14, 2017

wallrj Apr 18, 2017

rmohr Apr 21, 2017

rmohr Apr 21, 2017

rmohr Apr 21, 2017

0xmichalis commented Apr 21, 2017 •

edited

caesarxuchao commented Apr 21, 2017

caesarxuchao commented Apr 21, 2017

wallrj commented May 2, 2017

ncdc May 31, 2017

ncdc May 31, 2017

wallrj Jul 31, 2017

ncdc May 31, 2017

ncdc commented May 31, 2017

krmayankk commented Jun 8, 2017

caesarxuchao commented Jun 13, 2017

ahmetb Jun 13, 2017

ahmetb Jun 13, 2017

k8s-ci-robot commented Aug 1, 2017

k8s-github-robot commented Aug 30, 2017

k8s-github-robot commented Oct 31, 2017

Add an Informer example in client-go #44446

Add an Informer example in client-go #44446

Conversation

wallrj commented Apr 13, 2017 • edited by k8s-github-robot

k8s-ci-robot commented Apr 13, 2017

k8s-reviewable commented Apr 13, 2017

k8s-ci-robot commented Apr 13, 2017

k8s-github-robot commented Apr 13, 2017

wallrj commented Apr 13, 2017

grodrigues3 commented Apr 13, 2017

caesarxuchao commented Apr 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xmichalis commented Apr 21, 2017 • edited

caesarxuchao commented Apr 21, 2017

caesarxuchao commented Apr 21, 2017

wallrj commented May 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ncdc commented May 31, 2017

krmayankk commented Jun 8, 2017

caesarxuchao commented Jun 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-ci-robot commented Aug 1, 2017

k8s-github-robot commented Aug 30, 2017

k8s-github-robot commented Oct 31, 2017

wallrj commented Apr 13, 2017 •

edited by k8s-github-robot

0xmichalis commented Apr 21, 2017 •

edited