Structured output for 'kubeadm token list' #78764

bart0sh · 2019-06-06T14:04:26Z

What type of PR is this?
/kind feature

What this PR does / why we need it:

This is an initial implementation of kubeadm structured output described in this KEP

It adds new api group output.kubeadm.k8s.io to define versioned structured output and implements
output in TEXT, JSON, YAML, Go template and JSONPATH for kubeadm token list command using cli-runtime printer APIs.

Example output:

$ kubeadm token list
TOKEN                     TTL         EXPIRES                     USAGES                   DESCRIPTION                                                EXTRA GROUPS
7vg8cr.pks5g06s84aisb27   <invalid>   2019-06-05T17:13:55+03:00   authentication,signing   The default bootstrap token generated by 'kubeadm init'.   system:bootstrappers:kubeadm:default-node-token
vkmbz7.z52etl221i9lcrvt   2h          2019-06-06T17:22:48+03:00   authentication,signing   <none>                                                     system:bootstrappers:kubeadm:default-node-token

$ kubeadm token list -o json
{
    "kind": "BootstrapToken",
    "apiVersion": "output.kubeadm.k8s.io/v1alpha1",
    "creationTimestamp": null,
    "token": "7vg8cr.pks5g06s84aisb27",
    "description": "The default bootstrap token generated by 'kubeadm init'.",
    "expires": "2019-06-05T14:13:55Z",
    "usages": [
        "authentication",
        "signing"
    ],
    "groups": [
        "system:bootstrappers:kubeadm:default-node-token"
    ]
}
{
    "kind": "BootstrapToken",
    "apiVersion": "output.kubeadm.k8s.io/v1alpha1",
    "creationTimestamp": null,
    "token": "vkmbz7.z52etl221i9lcrvt",
    "expires": "2019-06-06T14:22:48Z",
    "usages": [
        "authentication",
        "signing"
    ],
    "groups": [
        "system:bootstrappers:kubeadm:default-node-token"
    ]
}


$ kubeadm token list -o yaml
apiVersion: output.kubeadm.k8s.io/v1alpha1
creationTimestamp: null
description: The default bootstrap token generated by 'kubeadm init'.
expires: "2019-06-05T14:13:55Z"
groups:
- system:bootstrappers:kubeadm:default-node-token
kind: BootstrapToken
token: 7vg8cr.pks5g06s84aisb27
usages:
- authentication
- signing
---
apiVersion: output.kubeadm.k8s.io/v1alpha1
creationTimestamp: null
expires: "2019-06-06T14:22:48Z"
groups:
- system:bootstrappers:kubeadm:default-node-token
kind: BootstrapToken
token: vkmbz7.z52etl221i9lcrvt
usages:
- authentication
- signing
---

$ kubeadm token list -o "go-template={{println .token .description .groups}}"
7vg8cr.pks5g06s84aisb27 The default bootstrap token generated by 'kubeadm init'. [system:bootstrappers:kubeadm:default-node-token]
vkmbz7.z52etl221i9lcrvt <nil> [system:bootstrappers:kubeadm:default-node-token]

$ kubeadm token list -o "jsonpath={.token} {.description}{'\n'}}"
vg8cr.pks5g06s84aisb27 The default bootstrap token generated by 'kubeadm init'.
vkmbz7.z52etl221i9lcrvt

Does this PR introduce a user-facing change?:

kubeadm: implemented structured output of 'kubeadm token list' in JSON, YAML, Go template and JsonPath formats

bart0sh · 2019-06-06T14:08:57Z

/cc @akutz

bart0sh · 2019-06-06T14:10:00Z

/cc @kubernetes/sig-cluster-lifecycle-pr-reviews

bart0sh · 2019-06-06T14:13:52Z

/cc @rosti @timothysc @neolit123

bart0sh · 2019-06-06T14:18:44Z

/test pull-kubernetes-bazel-test

bart0sh · 2019-06-06T14:47:07Z

/test pull-kubernetes-e2e-gce

kad · 2019-06-11T11:07:42Z

lgtm from my side, however question: should the output of bootstrap token in parsable form to be compatible with something that can be used directly in joinconfig?

rosti

Thanks @bart0sh !
Looks good for a start. I propose to reuse the corresponding types from the config (when possible).

cmd/kubeadm/app/apis/output/types.go

cmd/kubeadm/app/apis/output/v1alpha1/types.go

cmd/kubeadm/app/cmd/options/generic.go

cmd/kubeadm/app/cmd/token.go

neolit123 · 2019-10-01T13:47:33Z

cmd/kubeadm/app/util/output/output.go

+}
+
+// Info is a wrapper around klog.Info
+func (tp TextPrinter) Info(level klog.Level, args ...interface{}) {


i think we should either only have Infof or both Info and Infof.
also we need Warning(f).

this however raises another question.
users would expected structured output to go to STDOUT.
klog goes to STDERR, so do we need to wrap it with the structured output printer?

added Infof.

structured output is handled only by PrintObj. Info* calls are silenced in the current implementation. I'm ok to make them loud if other reviewers agree. It would create a bit messy console output though.

neolit123 · 2019-10-01T13:48:54Z

@bart0sh
thanks a lot for the work on this. i have added a number of comments in my first pass.
i think this PR is in a good shape but needs more review from others.
@kubernetes/sig-cluster-lifecycle-pr-reviews

i will do another pass myself at some point.

bart0sh · 2019-10-01T14:19:39Z

@neolit123 Thank you for the review!

bart0sh · 2019-10-01T16:24:02Z

/test pull-kubernetes-kubemark-e2e-gce-big

rosti

Thanks @bart0sh !
Looks good for a start. Mostly naming stuff that seems to need fixing and logging.

cmd/kubeadm/app/cmd/token.go

cmd/kubeadm/app/util/output/output.go

rosti · 2019-10-02T13:39:11Z

cmd/kubeadm/app/util/output/output.go

+// KubeadmOutputFlags composes common printer flag structs
+// used across kubeadm commands, and provides a method
+// of retrieving a known printer based on flag values provided.
+type KubeadmOutputFlags struct {


s/KubeadmOutputFlags/OutputContext ? Sounds better and shorted. WDYT?

*Flags suffix is used in the cli-runtime code. I'd rather use their naming convention.

P.S.: Would s/OutputFlags/OutputPrintFlags/ go? cli runtime uses PrintFlags suffix, so this way it would be even closer to their names.

but the structure seems to contain more than just flags?

Yes, it contains flags and additional fields to serve its second purpose that's mentioned in the comment - to retrive a printer based on the flag values.

cmd/kubeadm/app/util/output/output.go

rosti · 2019-10-02T13:52:53Z

cmd/kubeadm/app/util/output/output.go

+
+	if kof.JSONYamlPrintFlags != nil {
+		if p, err := kof.JSONYamlPrintFlags.ToPrinter(outputFormat); !genericclioptions.IsNoCompatiblePrinterError(err) {
+			return NewResourcePrinterWrapper(kof.TypeSetterPrinter.WrapToPrinter(p, err))


It is not clear at first sight, that the actual error, when ToPrinter fails, is correctly propagated and returned to the caller. The error plumbing here can be made more easy to see. Let's split this up into helpers with the more verbose (but clear) way of just returning the errors when they happen.

This approach was also taken from the cli-runtime code I used it this way just to not reinvent the wheel and be consistent with their implementation.

Let me know if you insist on using helpers - I'll modify the code.

cmd/kubeadm/app/util/output/output.go

This group contains APIs for handling kubeadm structured output.

bart0sh · 2019-10-03T10:21:13Z

/test pull-kubernetes-kubemark-e2e-gce-big

Added internal structures and APIs to handle kubeadm structured output.

Used cli-runtime API to print bootstrap tokens in 5 formats: - TEXT (identical to the current output) - YAML - JSON - JSONPATH - Go template

Tested JSON, YAML, Go Template and Text token output formats.

fabriziopandini · 2019-10-08T09:05:27Z

@bart0sh Thanks for your great job on this PR!
/approve
Leaving final LGTM to some other reviewer involved in this loop

k8s-ci-robot · 2019-10-08T09:06:14Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bart0sh, fabriziopandini

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~cmd/kubeadm/OWNERS~~ [fabriziopandini]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

bart0sh · 2019-10-08T10:20:14Z

@fabriziopandini Thank you very much for approving this PR! There will be more of this kind as soon as this one is merged.

neolit123 · 2019-10-09T15:33:20Z

voting on final merge on this PR during today's office hours.
seems good to go, we can amend later if needed.

neolit123 · 2019-10-09T16:23:05Z

/lgtm
we voted +1.

…tured-output-v2 Structured output for 'kubeadm token list'

bart0sh changed the title ~~Pr0070 kubeadm structured output v2~~ POC: Kubeadm structured output Jun 6, 2019

k8s-ci-robot added area/kubeadm sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jun 6, 2019

k8s-ci-robot requested review from fabriziopandini and kad June 6, 2019 14:06

k8s-ci-robot requested a review from akutz June 6, 2019 14:08

k8s-ci-robot requested review from neolit123, rosti and timothysc June 6, 2019 14:13

bart0sh force-pushed the PR0070-kubeadm-structured-output-v2 branch 2 times, most recently from 81e8f11 to 4b2eace Compare June 6, 2019 14:37

bart0sh force-pushed the PR0070-kubeadm-structured-output-v2 branch from 4b2eace to 27afe7e Compare June 7, 2019 09:00

bart0sh mentioned this pull request Jun 7, 2019

kubeadm: Implement --output/-o command line option #75894

Closed

bart0sh force-pushed the PR0070-kubeadm-structured-output-v2 branch 3 times, most recently from 3faf861 to eabc575 Compare June 7, 2019 13:18

rosti reviewed Jun 11, 2019

View reviewed changes

neolit123 reviewed Oct 1, 2019

View reviewed changes

cmd/kubeadm/app/cmd/token.go Outdated Show resolved Hide resolved

neolit123 reviewed Oct 1, 2019

View reviewed changes

bart0sh force-pushed the PR0070-kubeadm-structured-output-v2 branch from 6701279 to 64746be Compare October 1, 2019 14:08

rosti reviewed Oct 2, 2019

View reviewed changes

create new api group output.kubeadm.k8s.io

8cee92d

This group contains APIs for handling kubeadm structured output.

bart0sh force-pushed the PR0070-kubeadm-structured-output-v2 branch 2 times, most recently from 5793066 to b482930 Compare October 3, 2019 09:55

bart0sh added 3 commits October 3, 2019 14:13

Add internal kubeadm output API

3765f21

Added internal structures and APIs to handle kubeadm structured output.

kubeadm token list: implement structured output

766e2a4

Used cli-runtime API to print bootstrap tokens in 5 formats: - TEXT (identical to the current output) - YAML - JSON - JSONPATH - Go template

Test token output

ba0c84a

Tested JSON, YAML, Go Template and Text token output formats.

bart0sh force-pushed the PR0070-kubeadm-structured-output-v2 branch from b482930 to ba0c84a Compare October 3, 2019 11:14

timothysc unassigned akutz Oct 4, 2019

timothysc removed their request for review October 4, 2019 18:10

timothysc assigned neolit123 Oct 4, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 8, 2019

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 9, 2019

k8s-ci-robot merged commit a7e0b76 into kubernetes:master Oct 9, 2019

k8s-ci-robot added this to the v1.17 milestone Oct 9, 2019

ohsewon pushed a commit to ohsewon/kubernetes that referenced this pull request Oct 16, 2019

Merge pull request kubernetes#78764 from bart0sh/PR0070-kubeadm-struc…

30d0ae1

…tured-output-v2 Structured output for 'kubeadm token list'

bart0sh mentioned this pull request Jan 8, 2020

kubeadm config images list: implement structured output #86810

Merged

neolit123 mentioned this pull request Jan 10, 2022

kubeadm: machine readable output kubernetes/enhancements#2504

Open

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Structured output for 'kubeadm token list' #78764

Structured output for 'kubeadm token list' #78764

bart0sh commented Jun 6, 2019 •

edited

bart0sh commented Jun 6, 2019

bart0sh commented Jun 6, 2019

bart0sh commented Jun 6, 2019

bart0sh commented Jun 6, 2019

bart0sh commented Jun 6, 2019

kad commented Jun 11, 2019

rosti left a comment

neolit123 Oct 1, 2019

bart0sh Oct 1, 2019 •

edited

neolit123 commented Oct 1, 2019

bart0sh commented Oct 1, 2019

bart0sh commented Oct 1, 2019

rosti left a comment

rosti Oct 2, 2019

bart0sh Oct 3, 2019

neolit123 Oct 4, 2019

bart0sh Oct 4, 2019

rosti Oct 2, 2019

bart0sh Oct 3, 2019

bart0sh commented Oct 3, 2019

fabriziopandini commented Oct 8, 2019

k8s-ci-robot commented Oct 8, 2019

bart0sh commented Oct 8, 2019

neolit123 commented Oct 9, 2019

neolit123 commented Oct 9, 2019 •

edited

Structured output for 'kubeadm token list' #78764

Structured output for 'kubeadm token list' #78764

Conversation

bart0sh commented Jun 6, 2019 • edited

bart0sh commented Jun 6, 2019

bart0sh commented Jun 6, 2019

bart0sh commented Jun 6, 2019

bart0sh commented Jun 6, 2019

bart0sh commented Jun 6, 2019

kad commented Jun 11, 2019

rosti left a comment

Choose a reason for hiding this comment

neolit123 Oct 1, 2019

Choose a reason for hiding this comment

bart0sh Oct 1, 2019 • edited

Choose a reason for hiding this comment

neolit123 commented Oct 1, 2019

bart0sh commented Oct 1, 2019

bart0sh commented Oct 1, 2019

rosti left a comment

Choose a reason for hiding this comment

rosti Oct 2, 2019

Choose a reason for hiding this comment

bart0sh Oct 3, 2019

Choose a reason for hiding this comment

neolit123 Oct 4, 2019

Choose a reason for hiding this comment

bart0sh Oct 4, 2019

Choose a reason for hiding this comment

rosti Oct 2, 2019

Choose a reason for hiding this comment

bart0sh Oct 3, 2019

Choose a reason for hiding this comment

bart0sh commented Oct 3, 2019

fabriziopandini commented Oct 8, 2019

k8s-ci-robot commented Oct 8, 2019

bart0sh commented Oct 8, 2019

neolit123 commented Oct 9, 2019

neolit123 commented Oct 9, 2019 • edited

bart0sh commented Jun 6, 2019 •

edited

bart0sh Oct 1, 2019 •

edited

neolit123 commented Oct 9, 2019 •

edited