Add json,yaml output format support to kubectl create, kubectl apply #38112

juanvallejo · 2016-12-05T15:27:43Z

Release note:

Added support for printing in all supported `--output` formats to `kubectl create ...` and `kubectl apply ...`

This patch adds the ability to specify an output format other than
"name" to kubectl create .... It can be used in conjunction with the
--dry-run option. Converts unstructured objects into known types in
order to support all --output values.

The patch prints *resource.Infos returned by the server. If a resource does not yet exist (and the --dry-run option is not set), the resource is created and printed in the specified format.

@kubernetes/cli-review @fabianofranz

k8s-ci-robot · 2016-12-05T15:27:46Z

Can a kubernetes member verify that this patch is reasonable to test? If so, please 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 will still work. Regular contributors should join the org to skip this step.

If you have questions or suggestions related to this bot's behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.

k8s-reviewable · 2016-12-05T15:27:53Z

This change is

deads2k · 2016-12-05T15:50:35Z

pkg/kubectl/cmd/create.go

+func decodeUnstructuredObject(obj runtime.Object) (runtime.Object, bool) {
+	switch obj.(type) {
+	case *runtime.UnstructuredList, *runtime.Unstructured, *runtime.Unknown:
+		if objBytes, err := runtime.Encode(api.Codecs.LegacyCodec(), obj); err == nil {


This looks very, very wrong. What are you trying to do?

@deads2k

This looks very, very wrong. What are you trying to do?

I'm attempting to convert to a known type, I used this implementation in filter.go as an example.

deads2k · 2016-12-05T15:51:06Z

Is the output what I sent or what I got back. I expect it to be what I got back.

deads2k · 2016-12-05T17:41:32Z

I'm attempting to convert to a known type, I used this implementation in filter.go as an example.

Why would I want to do that? The whole point of these commands is to be generic if possible.

juanvallejo · 2016-12-05T17:48:29Z

@deads2k

Why would I want to do that? The whole point of these commands is to be generic if possible.

--output wide returned an error such as:

error: unknown type &runtime.Unstructured{Object:map[string]interface {}{"status":map[string]interface {}{"lastVersion":1}, "kind":"BuildConfig", "apiVersion":"v1", "metadata":map[string]interface {}{"name":"gitauthtest", "namespace":"default", "selfLink":"/oapi/v1/namespaces/default/buildconfigs/gitauthtest", "uid":"39b2c5a1-b1b3-11e6-9a01-507b9dac96e1", "resourceVersion":"800167", "creationTimestamp":"2016-11-23T19:30:00Z", "labels":map[string]interface {}{"app":"gitauthtest", "template":"gitserver"}, "annotations":map[string]interface {}{"openshift.io/generated-by":"OpenShiftNewApp"}}, "spec":map[string]interface {}{"runPolicy":"Serial", "source":map[string]interface {}{"type":"Git", "git":map[string]interface {}{"uri":"http://gitserver-tokenauth.linux.xip.io/ruby-hello-world"}, "sourceSecret":map[string]interface {}{"name":"builder-token-nbme5"}}, "strategy":map[string]interface {}{"type":"Source", "sourceStrategy":map[string]interface {}{"from":map[string]interface {}{"kind":"ImageStreamTag", "namespace":"openshift", "name":"ruby:latest"}}}, "output":map[string]interface {}{}, "resources":map[string]interface {}{}, "postCommit":map[string]interface {}{}, "nodeSelector":interface {}(nil), "triggers":[]interface {}{}}}}

if I did not convert the object here, however another solution in the downstream PR was also to just output something like Output format not supported if a printer required object conversion.

deads2k · 2016-12-05T17:57:27Z

--output wide returned an error such as:

I'd rather see the conversion contained in something like the printer that knows what it cares about. As we move GET to the server, this should fall out of the flow

fabianofranz · 2016-12-05T19:16:57Z

@kubernetes/kubectl

juanvallejo · 2016-12-05T21:11:26Z

@deads2k

I'd rather see the conversion contained in something like the printer that knows what it cares about. As we move GET to the server, this should fall out of the flow

Thanks for the feedback. It looks like runtime.Unstructurted objects are already being converted to known types in the HumanReadablePrinter's printObj method. I went ahead and updated create.go so that a printer with an unversioned mapping is used, which allows me to send an unstructured info.Object to this method.

mengqiy · 2016-12-05T22:15:59Z

@juanvallejo Do you have another PR to fix the similar issue with kubectl apply. Ref: #34028 (comment)

juanvallejo · 2016-12-05T22:17:39Z

@ymqytw

Do you have another PR to fix the similar issue with kubectl apply

Sorry about that, I had forgotten to address this. Will add as part of this PR. Thanks!

juanvallejo · 2016-12-06T15:10:37Z

@ymqytw
Updated kubectl apply to support different output formats as well, PTAL c6658ea

Sample output

$ kubectl apply -f https://raw.githubusercontent.com/mdshuai/testfile-openshift/master/petset/hello-petset.yaml -o wide
NAME      CLUSTER-IP   EXTERNAL-IP   PORT(S)   AGE       SELECTOR
foo       None         <none>        80/TCP    9s        app=hello-pod
NAME           DESIRED   CURRENT   AGE
hello-petset   2         2         8s        hello-pod   docker.io/deshuai/hello-pod:latest   app=hello-pod

smarterclayton · 2016-12-06T15:56:36Z

pkg/kubectl/cmd/apply.go

@@ -251,6 +251,20 @@ func RunApply(f cmdutil.Factory, cmd *cobra.Command, out io.Writer, options *App
 			}

 			count++
+			if len(output) > 0 && !shortOutput {


Please extract this code into pkg/kubectl - appears to be identical across all three.

@smarterclayton done! extracted this out to kubectl/cmd/util/printing.go PTAL

mengqiy · 2016-12-07T00:10:13Z

pkg/kubectl/cmd/util/printing.go

+		if err != nil {
+			return err
+		}
+		return printer.PrintObj(info.Object, out)


I think we don't need this line.

This line is using the printer declared in line 142 although I realize that this entire part could read a bit better. I'll go ahead and rename the printer declared inside of the !generic || printer == nil block to something a bit more specific, like nonGenericPrinter

@ymqytw

This line is using the printer declared in line 142 although I realize that this entire part could read a bit better. I'll go ahead and rename the printer declared inside of the !generic || printer == nil block to something a bit more specific, like nonGenericPrinter

Sorry, misunderstood which line you were pointing out. I see what you mean, I'll go ahead and update. Please ignore the above comment.

mengqiy · 2016-12-07T00:11:28Z

This PR should have a release note, since it changes the output behavior.

juanvallejo · 2016-12-07T15:03:44Z

@ymqytw

This PR should have a release note, since it changes the output behavior.

Added a release note

juanvallejo · 2016-12-08T14:50:55Z

@fabianofranz or @ymqytw Is this patch okay to test? Thanks!

mengqiy · 2016-12-08T17:40:07Z

@k8s-bot ok to test

k8s-ci-robot · 2016-12-08T20:35:27Z

Jenkins GCE e2e failed for commit 29e65475f4460f9c362c79c689b73f28c3e1fa22. Full PR test history.

The magic incantation to run this job again is @k8s-bot cvm gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-ci-robot · 2016-12-08T20:36:19Z

Jenkins GCI GCE e2e failed for commit 29e65475f4460f9c362c79c689b73f28c3e1fa22. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-ci-robot · 2016-12-08T20:57:14Z

Jenkins verification failed for commit 29e65475f4460f9c362c79c689b73f28c3e1fa22. Full PR test history.

The magic incantation to run this job again is @k8s-bot verify test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

This patch adds the ability to specify an output format other than "name" to `kubectl create ...`. It can be used in conjunction with the `--dry-run` option. Converts unstructured objects into known types in order to support all `--output` values.

juanvallejo · 2016-12-13T14:43:07Z

@ymqytw or @deads2k are there any more comments on this patch? Thanks!

deads2k · 2016-12-14T15:07:35Z

The description still isn't clear to me. Is this showing me what was submitted or what was actually created (returned from the API). You definitely need tests.

Also, the printing API needs a significant overhaul. I'll let @fabianofranz decide what order things need to happen. I think this reduces overall readability.

juanvallejo · 2016-12-14T18:53:50Z

@deads2k

The description still isn't clear to me. Is this showing me what was submitted or what was actually created (returned from the API).

Thanks, went ahead and updated the description to clarify this. The printed objects are the ones returned from the server. If an info is returned and does not yet exist in the server (and the --dry-run option was not also provided with the --output flag), then it is first created and then it is printed in the format specified.

juanvallejo · 2016-12-14T19:44:29Z

@deads2k

Also, the printing API needs a significant overhaul. I'll let @fabianofranz decide what order things need to happen. I think this reduces overall readability.

@fabianofranz I agree that adding another helper to handle printing resources does not feel completely right. The reason why the new helper PrintResourceInfoForCommand was added in this PR was to handle cases where --output wide would cause a panic due to the way it is currently handled in kubectl.GetPrinter. Perhaps the best solution would be to update that line in resource_printer.go to return a NewHumanReadablePrinter, or maybe have it actually return an error if it is going to return a nil printer and handle returning a HumanReadablePrinter with the correct PrintOptions in cmdutil.PrinterForCommand.

Maybe this is something that could be added to the kubectl umbrella issue for printers?

fabianofranz · 2016-12-15T19:58:44Z

Added #38779 to the printers and describers umbrella. I agree it needs a significant overhaul which we are proposing to do after an analysis of the existing issues, but let's do that in a separate effort.

fabianofranz · 2016-12-21T14:03:29Z

Any other comments here?

fabianofranz · 2017-01-03T19:22:00Z

@k8s-bot bazel test this

k8s-github-robot · 2017-01-03T22:18:32Z

Automatic merge from submit-queue

This fixes kubernetes#38779. This allows us to avoid case in which printers.GetStandardPrinter returns nil for both printer and err removing any potential panics that may arise throughout kubectl commands. Please see kubernetes#38779 and kubernetes#38112 for complete context. Add comment explaining adding handlers to printers.HumanReadablePrinter also remove an unnecessary conversion of printers.HumanReadablePrinter to printers.ResourcePrinter.

Automatic merge from submit-queue (batch tested with PRs 41563, 45251, 46265, 46462, 46721) Denote if a printer is generic. This fixes #38779. This allows us to avoid case in which printers.GetStandardPrinter returns nil for both printer and err removing any potential panics that may arise throughout kubectl commands. Please see #38779 and #38112 for complete context.

This fixes kubernetes#38779. This allows us to avoid case in which printers.GetStandardPrinter returns nil for both printer and err removing any potential panics that may arise throughout kubectl commands. Please see kubernetes#38779 and kubernetes#38112 for complete context. Add comment explaining adding handlers to printers.HumanReadablePrinter also remove an unnecessary conversion of printers.HumanReadablePrinter to printers.ResourcePrinter.

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 5, 2016

juanvallejo mentioned this pull request Dec 5, 2016

UPSTREAM: 38112: Add json,yaml output format support to oc create, oc apply openshift/origin#12015

Closed

k8s-github-robot assigned deads2k Dec 5, 2016

k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note-label-needed labels Dec 5, 2016

deads2k reviewed Dec 5, 2016

View reviewed changes

k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 5, 2016

juanvallejo changed the title ~~Add json,yaml output format support kubectl create~~ Add json,yaml output format support to kubectl create, kubectl apply Dec 5, 2016

juanvallejo force-pushed the jvallejo/add-output-format-support-kubectl-create branch from 63c4d8b to 1112a57 Compare December 5, 2016 22:34

k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 6, 2016

smarterclayton reviewed Dec 6, 2016

View reviewed changes

mengqiy reviewed Dec 7, 2016

View reviewed changes

juanvallejo force-pushed the jvallejo/add-output-format-support-kubectl-create branch from 66f3121 to 3249527 Compare December 7, 2016 14:58

juanvallejo force-pushed the jvallejo/add-output-format-support-kubectl-create branch from 29e6547 to 3916587 Compare December 8, 2016 20:36

k8s-github-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed release-note-label-needed labels Dec 10, 2016

juanvallejo force-pushed the jvallejo/add-output-format-support-kubectl-create branch from 3916587 to cbe4790 Compare December 12, 2016 21:09

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

deads2k assigned fabianofranz and unassigned deads2k Dec 14, 2016

juanvallejo mentioned this pull request Dec 14, 2016

Update kubectl.GetPrinter to not return both nil for the printer and nil for the error on wide format #38779

Closed

fabianofranz added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 21, 2016

k8s-github-robot merged commit 3951ae4 into kubernetes:master Jan 3, 2017

juanvallejo deleted the jvallejo/add-output-format-support-kubectl-create branch January 9, 2017 15:11

waseem mentioned this pull request May 23, 2017

Denote if a printer is generic. #46265

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add json,yaml output format support to kubectl create, kubectl apply #38112

Add json,yaml output format support to kubectl create, kubectl apply #38112

juanvallejo commented Dec 5, 2016 •

edited

Loading

k8s-ci-robot commented Dec 5, 2016

k8s-reviewable commented Dec 5, 2016

deads2k Dec 5, 2016

juanvallejo Dec 5, 2016 •

edited

Loading

deads2k commented Dec 5, 2016

deads2k commented Dec 5, 2016

juanvallejo commented Dec 5, 2016

deads2k commented Dec 5, 2016

fabianofranz commented Dec 5, 2016

juanvallejo commented Dec 5, 2016 •

edited

Loading

mengqiy commented Dec 5, 2016

juanvallejo commented Dec 5, 2016

juanvallejo commented Dec 6, 2016 •

edited

Loading

smarterclayton Dec 6, 2016

juanvallejo Dec 6, 2016

mengqiy Dec 7, 2016

juanvallejo Dec 7, 2016

juanvallejo Dec 7, 2016

mengqiy commented Dec 7, 2016

juanvallejo commented Dec 7, 2016

juanvallejo commented Dec 8, 2016

mengqiy commented Dec 8, 2016

k8s-ci-robot commented Dec 8, 2016

k8s-ci-robot commented Dec 8, 2016

k8s-ci-robot commented Dec 8, 2016

juanvallejo commented Dec 13, 2016

deads2k commented Dec 14, 2016

juanvallejo commented Dec 14, 2016 •

edited

Loading

juanvallejo commented Dec 14, 2016

fabianofranz commented Dec 15, 2016

fabianofranz commented Dec 21, 2016

fabianofranz commented Jan 3, 2017

k8s-github-robot commented Jan 3, 2017

Add json,yaml output format support to kubectl create, kubectl apply #38112

Add json,yaml output format support to kubectl create, kubectl apply #38112

Conversation

juanvallejo commented Dec 5, 2016 • edited Loading

k8s-ci-robot commented Dec 5, 2016

k8s-reviewable commented Dec 5, 2016

deads2k Dec 5, 2016

Choose a reason for hiding this comment

juanvallejo Dec 5, 2016 • edited Loading

Choose a reason for hiding this comment

deads2k commented Dec 5, 2016

deads2k commented Dec 5, 2016

juanvallejo commented Dec 5, 2016

deads2k commented Dec 5, 2016

fabianofranz commented Dec 5, 2016

juanvallejo commented Dec 5, 2016 • edited Loading

mengqiy commented Dec 5, 2016

juanvallejo commented Dec 5, 2016

juanvallejo commented Dec 6, 2016 • edited Loading

smarterclayton Dec 6, 2016

Choose a reason for hiding this comment

juanvallejo Dec 6, 2016

Choose a reason for hiding this comment

mengqiy Dec 7, 2016

Choose a reason for hiding this comment

juanvallejo Dec 7, 2016

Choose a reason for hiding this comment

juanvallejo Dec 7, 2016

Choose a reason for hiding this comment

mengqiy commented Dec 7, 2016

juanvallejo commented Dec 7, 2016

juanvallejo commented Dec 8, 2016

mengqiy commented Dec 8, 2016

k8s-ci-robot commented Dec 8, 2016

k8s-ci-robot commented Dec 8, 2016

k8s-ci-robot commented Dec 8, 2016

juanvallejo commented Dec 13, 2016

deads2k commented Dec 14, 2016

juanvallejo commented Dec 14, 2016 • edited Loading

juanvallejo commented Dec 14, 2016

fabianofranz commented Dec 15, 2016

fabianofranz commented Dec 21, 2016

fabianofranz commented Jan 3, 2017

k8s-github-robot commented Jan 3, 2017

juanvallejo commented Dec 5, 2016 •

edited

Loading

juanvallejo Dec 5, 2016 •

edited

Loading

juanvallejo commented Dec 5, 2016 •

edited

Loading

juanvallejo commented Dec 6, 2016 •

edited

Loading

juanvallejo commented Dec 14, 2016 •

edited

Loading