add jsonpath to kubectl #12202

daizuozhuo · 2015-08-04T13:32:01Z

@brendandburns I add jsonpath as an output option for kubectl. And I think I should make a page to tell people the jsonpath syntax. Where should I put it?

k8s-bot · 2015-08-04T13:33:05Z

Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist")

If this message is too spammy, please complain to ixdy.

brendandburns · 2015-08-05T20:50:52Z

pkg/kubectl/resource_printer.go

+
+func NewJSONPathPrinter(tmpl []byte) (*JSONPathPrinter, error) {
+	j := jsonpath.New("out")
+	err := j.Parse(string(tmpl))


if err := j.Parse(string(tmpl)); err != nil { return nil, err }

brendandburns · 2015-08-05T20:55:15Z

Small comments, basically LGTM.

You need to run ./hack/run-gendoc.sh to regenerate kubectl docs.

brendandburns · 2015-08-05T20:55:57Z

I would add the jsonpath documentation to docs/user-guide/... and add a link in the kubectl help docs.

derekwaynecarr · 2015-08-05T21:11:17Z

Can something exercise this in hack/test-cmd.sh?

daizuozhuo · 2015-08-06T00:59:01Z

@derekwaynecarr Yes, I will add some tests to hack/test.cmd.sh.

thockin · 2015-08-06T03:33:48Z

docs/user-guide/kubectl/kubectl_expose.md

@@ -69,10 +69,11 @@ $ kubectl expose rc streamer --port=4100 --protocol=udp --name=video-stream
      --dry-run=false: If true, only print the object that would be sent, without creating it.
      --generator="service/v2": The name of the API generator to use. There are 2 generators: 'service/v1' and 'service/v2'. The only difference between them is that service port in v1 is named 'default', while it is left unnamed in v2. Default is 'service/v2'.
  -h, --help=false: help for expose
+  -j, --jsonpath="": JSONPath template string to use when -o=jsonpath. The jsonpath template is composed of jsonpath expressions enclosed by {}


Is it too late to argue for -o jsonpath="template string" and -o template="template string".

Not too late. See:
https://github.com/GoogleCloudPlatform/kubernetes/pull/12220/files#r36381964

that's fine with me too.

@daizuozhuo do you want to do a separate PR to do that?

If you want to do that as a separate PR, then in this one we should:

a) Keep the jsonpath output format
b) Drop the new jsonpath flag and reuse the existing template flag, just interpreting it based on the requested output format.

Yes, I would do this in a separate RP.

@bgrant0607 Good Suggestions!

bgrant0607 · 2015-08-07T03:07:51Z

The command-line args LGTM. Please address comments from @brendandburns and @derekwaynecarr.

brendandburns · 2015-08-07T22:00:55Z

LGTM. Needs rebase due to the grand rename.

daizuozhuo · 2015-08-08T00:08:14Z

@brendandburns Rebased

daizuozhuo · 2015-08-10T03:29:45Z

@derekwaynecarr How to make hack/test-cmd.sh run? It reports error like:

/Users/daizuozhuo/goproj/src/github.com/GoogleCloudPlatform/kubernetes/_output/local/bin/darwin/amd64/kubelet --really-crash-for-testing=true --root-dir=/tmp/kubelet.31768 --cert-dir=/var/folders/81/n6gk7mk1279_b4x2vqdcp1680000gn/T/ --docker-endpoint=fake:// --hostname-override=127.0.0.1 --address=127.0.0.1 --port=10250 --healthz-port=10248
I0810 11:25:08.464685   31811 plugins.go:70] No cloud provider specified.
W0810 11:25:08.464920   31811 server.go:407] setting OOM scores is unsupported in this build
W0810 11:25:08.465017   31811 server.go:639] No api server defined - no events will be sent to API server.
failed to create kubelet: cAdvisor is unsupported in this build
!!! [0810 11:25:20] Timed out waiting for kubelet(masterless) to answer at http://127.0.0.1:10248/healthz; tried 25 waiting 0.5 between each
!!! Error in ./test-cmd.sh:121
  'return 1' exited with status 1
Call stack:
  1: ./test-cmd.sh:121 main(...)

daizuozhuo · 2015-08-11T10:27:58Z

Comments are addressed. By the way, I noticed that hack/test-cmd.sh is not working in Mac OS, So I tested it in Linux.

daizuozhuo · 2015-08-11T16:53:06Z

I will change it to -o go-template=... , -o go-template-file=... and -o jsonpath=... in another PR.

daizuozhuo · 2015-08-13T09:15:40Z

@brendandburns I added jsonpath.md. Please comment if it has any problem.

daizuozhuo · 2015-08-17T08:08:54Z

@brendandburns @bgrant0607 When will it be merged?

bgrant0607 · 2015-08-17T19:38:46Z

pkg/kubectl/resource_printer_test.go

@@ -164,6 +164,23 @@ func TestPrintTemplate(t *testing.T) {
 	}
 }

+func TestPrintJSONPath(t *testing.T) {
+	buf := bytes.NewBuffer([]byte{})
+	printer, found, err := GetPrinter("jsonpath", "{.metadata.name}")


I understand that this is patterned after TestPrintTemplate, but what I'd like to see is a table-driven test that exercises more cases than just a single field.

An example of a table-driven test:
https://github.com/kubernetes/kubernetes/blob/master/pkg/kubectl/generate_test.go#L52

The test could be expanded in a subsequent PR, though.

Thanks. I changed it and moved several other test function into the table.

brendandburns · 2015-08-18T16:08:42Z

I still see one comment that hasn't been addressed, can you take a look at it?

daizuozhuo · 2015-08-18T16:12:32Z

@brendandburns I use rawTemplate to print debug message now.

daizuozhuo · 2015-08-18T16:13:36Z

pkg/kubectl/resource_printer.go

+	}
+	if err = j.JSONPath.Execute(w, out); err != nil {
+		fmt.Fprintf(w, "Error executing template: %v\n", err)
+		fmt.Fprintf(w, "template was:\n\t%v\n", j.rawTemplate)


@brendandburns print rawTemplate here

bgrant0607 · 2015-08-19T04:24:24Z

cc @kubernetes/kubectl

brendandburns · 2015-08-19T18:50:10Z

LGTM

saad-ali · 2015-08-19T23:58:49Z

Rebase needed, removing LGTM for now

daizuozhuo · 2015-08-20T01:20:15Z

@saad-ali Rebased, thanks.

add jsonpath to kubectl

smarterclayton · 2015-08-21T04:52:45Z

Tests are flaking:

ok k8s.io/kubernetes/pkg/util/flushwriter   1.041s  coverage: 100.0% of statements
--- FAIL: TestKubenates (0.02 seconds)
jsonpath_test.go:47: in recursive name, expect to get "127.0.0.1 127.0.0.2 myself e2e", got "myself e2e 127.0.0.1 127.0.0.2"
FAIL
coverage: 90.2% of statements
FAIL k8s.io/kubernetes/pkg/util/jsonpath 0.129s
ok k8s.io/kubernetes/pkg/util/mount 1.033s coverage: 25.5% of statements
ok k8s.io/kubernetes/pkg/util/config 1.069s coverage: 92.9% of statements
ok k8s.io/kubernetes/pkg/storage    1.284s  coverage: 89.0% of statements

daizuozhuo · 2015-08-21T05:13:52Z

@smarterclayton Since the JSON input is a map, JSONPath may iterate it in unknown order. Sorry, I will change the test method in next PR.

googlebot added the cla: yes label Aug 4, 2015

alex-mohr assigned brendandburns Aug 4, 2015

brendandburns reviewed Aug 5, 2015
View reviewed changes

daizuozhuo force-pushed the kubectl branch 2 times, most recently from 579246e to daa36df Compare August 6, 2015 00:56

thockin reviewed Aug 6, 2015
View reviewed changes

daizuozhuo force-pushed the kubectl branch from daa36df to 2e39e4f Compare August 7, 2015 02:42

daizuozhuo force-pushed the kubectl branch from 2e39e4f to 6bba2c5 Compare August 8, 2015 00:00

daizuozhuo force-pushed the kubectl branch from 6bba2c5 to 87c8da3 Compare August 11, 2015 10:24

daizuozhuo force-pushed the kubectl branch from 87c8da3 to 09b766c Compare August 11, 2015 16:50

daizuozhuo unassigned brendandburns Aug 12, 2015

mikedanese assigned brendandburns Aug 12, 2015

daizuozhuo force-pushed the kubectl branch from 09b766c to 09c3f7a Compare August 13, 2015 09:12

daizuozhuo force-pushed the kubectl branch 2 times, most recently from 54183c2 to b786fc9 Compare August 16, 2015 13:49

bgrant0607 mentioned this pull request Aug 17, 2015

Add a printer that knows how to print user-defined columns #12705

Merged

bgrant0607 reviewed Aug 17, 2015
View reviewed changes

daizuozhuo force-pushed the kubectl branch from b786fc9 to 6f462e1 Compare August 18, 2015 14:26

daizuozhuo reviewed Aug 18, 2015
View reviewed changes

bgrant0607 added the area/kubectl label Aug 19, 2015

brendandburns added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 19, 2015

saad-ali removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 19, 2015

daizuozhuo added 2 commits August 20, 2015 08:57

add jsonpath to kubectl

b61a905

jsonpath user guide docs

85972c4

daizuozhuo force-pushed the kubectl branch from 6f462e1 to 85972c4 Compare August 20, 2015 01:17

saad-ali added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 20, 2015

saad-ali added a commit that referenced this pull request Aug 20, 2015

Merge pull request #12202 from daizuozhuo/kubectl

489e75e

add jsonpath to kubectl

saad-ali merged commit 489e75e into kubernetes:master Aug 20, 2015

daizuozhuo deleted the kubectl branch August 21, 2015 05:17

janetkuo mentioned this pull request Aug 21, 2015

Deprecated -t for --template to make it --tty #12813

Merged

jszczepkowski mentioned this pull request Aug 21, 2015

jsonpath_test.go flaky #13024

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add jsonpath to kubectl #12202

add jsonpath to kubectl #12202

daizuozhuo commented Aug 4, 2015

k8s-bot commented Aug 4, 2015

brendandburns Aug 5, 2015

brendandburns commented Aug 5, 2015

brendandburns commented Aug 5, 2015

derekwaynecarr commented Aug 5, 2015

daizuozhuo commented Aug 6, 2015

thockin Aug 6, 2015

bgrant0607 Aug 6, 2015

brendandburns Aug 6, 2015

bgrant0607 Aug 7, 2015

daizuozhuo Aug 7, 2015

daizuozhuo Aug 7, 2015

bgrant0607 commented Aug 7, 2015

brendandburns commented Aug 7, 2015

daizuozhuo commented Aug 8, 2015

daizuozhuo commented Aug 10, 2015

daizuozhuo commented Aug 11, 2015

daizuozhuo commented Aug 11, 2015

daizuozhuo commented Aug 13, 2015

daizuozhuo commented Aug 17, 2015

bgrant0607 Aug 17, 2015

daizuozhuo Aug 18, 2015

brendandburns commented Aug 18, 2015

daizuozhuo commented Aug 18, 2015

daizuozhuo Aug 18, 2015

bgrant0607 commented Aug 19, 2015

brendandburns commented Aug 19, 2015

saad-ali commented Aug 19, 2015

daizuozhuo commented Aug 20, 2015

smarterclayton commented Aug 21, 2015

daizuozhuo commented Aug 21, 2015

add jsonpath to kubectl #12202

add jsonpath to kubectl #12202

Conversation

daizuozhuo commented Aug 4, 2015

k8s-bot commented Aug 4, 2015

Choose a reason for hiding this comment

brendandburns commented Aug 5, 2015

brendandburns commented Aug 5, 2015

derekwaynecarr commented Aug 5, 2015

daizuozhuo commented Aug 6, 2015

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

bgrant0607 commented Aug 7, 2015

brendandburns commented Aug 7, 2015

daizuozhuo commented Aug 8, 2015

daizuozhuo commented Aug 10, 2015

daizuozhuo commented Aug 11, 2015

daizuozhuo commented Aug 11, 2015

daizuozhuo commented Aug 13, 2015

daizuozhuo commented Aug 17, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brendandburns commented Aug 18, 2015

daizuozhuo commented Aug 18, 2015

Choose a reason for hiding this comment

bgrant0607 commented Aug 19, 2015

brendandburns commented Aug 19, 2015

saad-ali commented Aug 19, 2015

daizuozhuo commented Aug 20, 2015

smarterclayton commented Aug 21, 2015

daizuozhuo commented Aug 21, 2015