Added informer to third-party-resources example of client-go #45263

MikeSpreitzer · 2017-05-03T03:20:28Z

What this PR does / why we need it:

Demonstrates how to list and watch third party objects (i.e., objects
of a Kind defined by a ThirdPartyResource).

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged):

This makes a small contribution toward kubernetes/client-go#128

Special notes for your reviewer:

Release note:

NONE

k8s-reviewable · 2017-05-03T03:20:34Z

This change is

k8s-ci-robot · 2017-05-03T03:20:37Z

Hi @MikeSpreitzer. 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-05-03T03:20:37Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: MikeSpreitzer
We suggest the following additional approver: lavalamp

Assign the PR to them by writing /assign @lavalamp in a comment when ready.

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

MikeSpreitzer · 2017-05-03T04:13:50Z

This is propagating work by @yvespp ; @seh expressed interest on Slack.

feiskyer · 2017-05-03T06:02:27Z

@k8s-bot ok to test

MikeSpreitzer · 2017-05-03T06:53:36Z

It looks like something went wrong with dependencies, but I am mystified. go build in the third-part-resources directory works. I am not familiar with how dependencies are managed for the client-go examples. Can someone please help?

feiskyer · 2017-05-03T07:12:03Z

Run ./hack/update-bazel.sh?

k82cn · 2017-05-03T07:26:57Z

staging/src/k8s.io/client-go/examples/third-party-resources/main.go

+	"syscall"
+	"time"
+
+	"github.com/golang/glog"


no update on BUILD for new package?

Is this something that ./hack/update-bazel.sh would update?

As feiskyer suggested, run ./hack/update-bazel.sh; it will help to update staging/src/k8s.io/client-go/examples/third-party-resources/BUILD.

Thanks for the clue. I am not familiar with bazel and that script; where would I go to learn what they do?

I tried that command, and it failed as follows. What does it mean and what can I do about it?

mjs10:kubernetes mspreitz$ ./hack/update-bazel.sh F0503 13:04:03.982558 3351 gazel.go:60] err walking generated: openapi-gen path outside of kubernetes: github.com/MikeSpreitzer/kubernetes/cmd/libs/go2idl/client-gen/test_apis/testgroup/v1

timothysc · 2017-05-03T14:07:41Z

staging/src/k8s.io/client-go/examples/third-party-resources/main.go

@@ -165,3 +186,87 @@ func configureClient(config *rest.Config) {
 	metav1.AddToGroupVersion(scheme.Scheme, groupversion)
 	schemeBuilder.AddToScheme(scheme.Scheme)
 }
+
+func watch(client *rest.RESTClient) {


I kind of think this should be in it's own file, and exported in case folks crib.

MikeSpreitzer · 2017-05-04T03:09:39Z

Factored, hopefully as @timothysc suggested.

MikeSpreitzer · 2017-05-04T03:15:59Z

But the same two tests fail and the update-bazel script still fails. Can anybody help?

mjs10:kubernetes mspreitz$ ./hack/update-bazel.sh 
F0503 23:11:14.828093   17577 gazel.go:60] err walking generated: openapi-gen path outside of kubernetes: github.com/MikeSpreitzer/kubernetes/cmd/libs/go2idl/client-gen/test_apis/testgroup/v1

k82cn · 2017-05-04T03:21:13Z

@MikeSpreitzer, here's the output on my laptop; when running ./hack/update-bazel.sh , you need to set $GOPATH correctly, and the k8s source code should in $GOPATH/src/k8s.io/kubernetes.

diff --git a/staging/src/k8s.io/client-go/examples/third-party-resources/BUILD b/staging/src/k8s.io/client-go/examples/third-party-resources/BUILD
index a3b7023e02..9dc65d600a 100644
--- a/staging/src/k8s.io/client-go/examples/third-party-resources/BUILD
+++ b/staging/src/k8s.io/client-go/examples/third-party-resources/BUILD
@@ -34,6 +34,7 @@ go_library(
     ],
     tags = ["automanaged"],
     deps = [
+        "//vendor/github.com/golang/glog:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
diff --git a/staging/src/k8s.io/client-go/examples/third-party-resources/main.go b/staging/src/k8s.io/client-go/examples/third-party-resources/main.go
index 3ae3c525cc..78eb16bea9 100644
--- a/staging/src/k8s.io/client-go/examples/third-party-resources/main.go
+++ b/staging/src/k8s.io/client-go/examples/third-party-resources/main.go
@@ -21,6 +21,8 @@ import (
        "flag"
        "fmt"

+       "github.com/golang/glog"
+
        "k8s.io/apimachinery/pkg/api/errors"
        metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
        "k8s.io/apimachinery/pkg/runtime"

k82cn · 2017-05-04T03:22:01Z

@MikeSpreitzer , BTW, please do not use symbol link, update-bazel.sh does not work with symbol link, AFAIK.

MikeSpreitzer · 2017-05-04T13:34:05Z

@k82cn Thanks for trying to help, but I do not understand. The output you showed is not from running the update-bazel script, it looks more like git diff output. I have not introduced any symbolic links, I use only the ones that are normally there:

mjs10:kubernetes mspreitz$ find . -type l
./cluster/gce/cos
./cluster/gce/ubuntu
./cluster/juju/layers/kubernetes-master/actions/namespace-delete
./cluster/juju/layers/kubernetes-master/actions/namespace-list
./vendor/k8s.io/apimachinery
./vendor/k8s.io/apiserver
./vendor/k8s.io/client-go
./vendor/k8s.io/kube-aggregator
./vendor/k8s.io/metrics
./vendor/k8s.io/sample-apiserver

MikeSpreitzer · 2017-05-04T13:35:37Z

I wonder if my dependency problems are a consequence of the fact that I am developing under my own fork's location under $GOPATH rather than k8s.io/kubernetes:

mjs10:kubernetes mspreitz$ pwd
/Users/mspreitz/Documents/go2/src/github.com/MikeSpreitzer/kubernetes

mjs10:kubernetes mspreitz$ printenv | grep GO
GOROOT=/usr/local/go
GOPATH=/Users/mspreitz/Documents/go2

MikeSpreitzer · 2017-05-04T13:36:41Z

And now, for some reason, the update-bazel script is failing in a new way this morning:

mjs10:kubernetes mspreitz$ ./hack/update-bazel.sh 
extract err: cannot find package "k8s.io/kubernetes/pkg/api/helper" in any of:
	/Users/mspreitz/Documents/go2/src/github.com/MikeSpreitzer/kubernetes/vendor/k8s.io/kubernetes/pkg/api/helper (vendor tree)
	/usr/local/go/src/k8s.io/kubernetes/pkg/api/helper (from $GOROOT)
	/Users/mspreitz/Documents/go2/src/k8s.io/kubernetes/pkg/api/helper (from $GOPATH)
extract err: cannot find package "k8s.io/kubernetes/pkg/api/helper" in any of:
	/Users/mspreitz/Documents/go2/src/github.com/MikeSpreitzer/kubernetes/vendor/k8s.io/kubernetes/pkg/api/helper (vendor tree)
	/usr/local/go/src/k8s.io/kubernetes/pkg/api/helper (from $GOROOT)
	/Users/mspreitz/Documents/go2/src/k8s.io/kubernetes/pkg/api/helper (from $GOPATH)
extract err: cannot find package "k8s.io/kubernetes/pkg/api/helper" in any of:
	/Users/mspreitz/Documents/go2/src/github.com/MikeSpreitzer/kubernetes/vendor/k8s.io/kubernetes/pkg/api/helper (vendor tree)
	/usr/local/go/src/k8s.io/kubernetes/pkg/api/helper (from $GOROOT)
	/Users/mspreitz/Documents/go2/src/k8s.io/kubernetes/pkg/api/helper (from $GOPATH)
... (and many more) ...

k82cn · 2017-05-04T13:37:29Z

@MikeSpreitzer , here's it path in my laptop:

macpro:kubernetes klaus$ echo $GOPATH
/Users/klaus/Workspace/k8s_ws
macpro:kubernetes klaus$ pwd
/Users/klaus/Workspace/k8s_ws/src/k8s.io/kubernetes

MikeSpreitzer · 2017-05-04T13:53:18Z

Yes, switching the location on my laptop to $GOPATH/src/k8s.io/kubernetes makes the update-bazel script work.

On reviewing it, I see that https://github.com/kubernetes/community/blob/master/contributors/devel/development.md#workflow does specifically say to use that location. However, I used to be able to work in $GOPATH/src/github.com/${my github profile name}/kubernetes. It might be worth updating development.md#workflow to emphasize that the specific location it mentions is now required.

MikeSpreitzer · 2017-05-04T14:19:59Z

Why was the latest Jenkins verification NOT done against the latest commit?

What is going wrong in the latest Jenkins Bazel Build? It complains

W0504 13:49:18.131] ERROR: /workspace/k8s.io/kubernetes/vendor/k8s.io/client-go/examples/third-party-resources/BUILD:29:1: Target '//vendor/k8s.io/client-go/pkg/api:go_default_library' is not visible from target '//vendor/k8s.io/client-go/examples/third-party-resources:go_default_library'. Check the visibility declaration of the former target if you think the dependency is legitimate.

I see that this PR does indeed add //vendor/k8s.io/client-go/pkg/api:go_default_library as a dependency in BUILD but this was done by the hack/update-bazel.sh script. What is going wrong here?

MikeSpreitzer · 2017-05-04T14:32:15Z

/assign @lavalamp

MikeSpreitzer · 2017-05-04T15:46:39Z

Oh, it seems that the Jenkins Verification build was just slow to arrive. Now it has successfully run on the latest commit. The only outstanding mystery is the complaint from the Jenkins Bazel Build.

caesarxuchao · 2017-05-04T18:49:29Z

Judging by the title this might duplicate #43027. @nilebox @mbohlool could you help take a look?

Demonstrates how to list and watch third party objects (i.e., objects of a Kind defined by a ThirdPartyResource). This makes a tiny contribution towards client-go issue kubernetes#128

MikeSpreitzer · 2017-05-04T19:08:52Z

My latest change replaced use of client-go/pkg/api with use of client-go/pkg/api/v1 because @caesarxuchao just restricted the visibility of the former in anticipation of its deletion.

nilebox · 2017-05-08T01:27:33Z

@caesarxuchao Thanks for the heads up. Yes, it is a duplicate request, and I prefer mine, as it provides more details, as well as contains a code cleanup.

@MikeSpreitzer Do you mind taking a look at the PR #45463, which solves the same problem? What do you think?

nilebox

@MikeSpreitzer Please see the comments.
Also, please take a look at the #45463 as they solve the same problem.

nilebox · 2017-05-08T01:30:10Z

staging/src/k8s.io/client-go/examples/third-party-resources/main.go

@@ -69,6 +73,7 @@ func main() {
 			if err != nil {
 				panic(err)
 			}
+			kind_created_here = true


In my PR I have added a sleep instead of passing this flag: https://github.com/kubernetes/kubernetes/pull/45463/files#diff-eb875333b11b3ab1e7e6fdb35d1bc09dR75

While sleeping could look hacky, I think it's okay for the example (with the appropriate comment, of course), and it doesn't require to run the example twice.
In your case the first run of example will always fail.

nilebox · 2017-05-08T01:32:00Z

staging/src/k8s.io/client-go/examples/third-party-resources/main.go

+				if kind_created_here {
+					glog.Infoln("Probably because of delay issue noted in https://github.com/kubernetes/features/issues/95 ...")
+				}
+				glog.Fatalf("Unable to create example1 --- request=%#v, result=%#v, err=%#v", req, result, err)


I don't like that you are mixing fmt.Print() with glog.
The original example was only using fmt, so it would be better to stick with it, or replacing fmt with glog everywhere.

nilebox · 2017-05-08T01:34:09Z

staging/src/k8s.io/client-go/examples/third-party-resources/watch.go

+		// resyncPeriod
+		// Every resyncPeriod, all resources in the cache will retrigger events.
+		// Set to 0 to disable the resync.
+		time.Second*60,


As previously suggested by @caesarxuchao in #43027 (comment), resyncPeriod should be set to 0 instead.

MikeSpreitzer · 2017-05-08T05:41:01Z

@nilebox: I would be happy to see #45463 merged instead of this one. I would be unhappy to see neither merged.

nilebox · 2017-05-12T00:02:25Z

@MikeSpreitzer should this PR be closed as duplicate now? #45463 is merged into master.

k8s-github-robot · 2017-05-12T00:02:37Z

@MikeSpreitzer PR needs rebase

caesarxuchao · 2017-05-12T00:08:16Z

Closing. Thank you @nilebox @MikeSpreitzer.

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 3, 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 May 3, 2017

k8s-github-robot assigned timothysc May 3, 2017

k8s-github-robot assigned feiskyer May 3, 2017

k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-none Denotes a PR that doesn't merit a release note. labels May 3, 2017

MikeSpreitzer force-pushed the watch-tpo branch from 6b9d91c to 2ef8fe4 Compare May 3, 2017 03:24

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 May 3, 2017

k82cn reviewed May 3, 2017

View reviewed changes

timothysc reviewed May 3, 2017

View reviewed changes

MikeSpreitzer force-pushed the watch-tpo branch from 2ef8fe4 to 606781d Compare May 4, 2017 02:56

MikeSpreitzer force-pushed the watch-tpo branch from 606781d to 1c6e0db Compare May 4, 2017 13:48

MikeSpreitzer mentioned this pull request May 4, 2017

development.md#workflow should emphasize that local repo location is not optional kubernetes/community#597

Closed

k8s-ci-robot assigned lavalamp May 4, 2017

Added informer to third-party-resources example of client-go

f657e44

Demonstrates how to list and watch third party objects (i.e., objects of a Kind defined by a ThirdPartyResource). This makes a tiny contribution towards client-go issue kubernetes#128

MikeSpreitzer force-pushed the watch-tpo branch from 1c6e0db to f657e44 Compare May 4, 2017 19:05

nilebox mentioned this pull request May 8, 2017

ThirdPartyResource example: added watcher example, code cleanup #45463

Merged

nilebox reviewed May 8, 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 May 12, 2017

caesarxuchao closed this May 12, 2017

Added informer to third-party-resources example of client-go #45263

Added informer to third-party-resources example of client-go #45263

Conversation

MikeSpreitzer commented May 3, 2017

k8s-reviewable commented May 3, 2017

k8s-ci-robot commented May 3, 2017

k8s-github-robot commented May 3, 2017

MikeSpreitzer commented May 3, 2017

feiskyer commented May 3, 2017

MikeSpreitzer commented May 3, 2017

feiskyer commented May 3, 2017

k82cn May 3, 2017

Choose a reason for hiding this comment

MikeSpreitzer May 3, 2017

Choose a reason for hiding this comment

k82cn May 3, 2017 • edited Loading

Choose a reason for hiding this comment

MikeSpreitzer May 3, 2017

Choose a reason for hiding this comment

timothysc May 3, 2017

Choose a reason for hiding this comment

MikeSpreitzer commented May 4, 2017

MikeSpreitzer commented May 4, 2017

k82cn commented May 4, 2017

k82cn commented May 4, 2017 • edited Loading

MikeSpreitzer commented May 4, 2017

MikeSpreitzer commented May 4, 2017 • edited Loading

MikeSpreitzer commented May 4, 2017

k82cn commented May 4, 2017

MikeSpreitzer commented May 4, 2017

MikeSpreitzer commented May 4, 2017

MikeSpreitzer commented May 4, 2017

MikeSpreitzer commented May 4, 2017

caesarxuchao commented May 4, 2017

MikeSpreitzer commented May 4, 2017

nilebox commented May 8, 2017

nilebox left a comment

Choose a reason for hiding this comment

nilebox May 8, 2017

Choose a reason for hiding this comment

nilebox May 8, 2017

Choose a reason for hiding this comment

nilebox May 8, 2017

Choose a reason for hiding this comment

MikeSpreitzer commented May 8, 2017 • edited Loading

nilebox commented May 12, 2017

k8s-github-robot commented May 12, 2017

caesarxuchao commented May 12, 2017

k82cn May 3, 2017 •

edited

Loading

k82cn commented May 4, 2017 •

edited

Loading

MikeSpreitzer commented May 4, 2017 •

edited

Loading

MikeSpreitzer commented May 8, 2017 •

edited

Loading