kubeadm icon indicating copy to clipboard operation
kubeadm copied to clipboard

Machine readable output from kubeadm commands

Open tamalsaha opened this issue 8 years ago • 57 comments

(original FR by tamalsaha)

[Feature Request]

We would like to compose the output from kubeadm upgrade plan command to our GO binary. It will be very helpful if kubeadm upgrade could return the output in machine readable form (json/yaml).


(edits by: neolit123)

KEP: https://github.com/kubernetes/enhancements/blob/master/keps/sig-cluster-lifecycle/kubeadm/20190506-kubeadm-machine-output.md

status / supported commands:

  • [x] kubeadm config images list
  • [x] kubeadm token list
  • [x] kubeadm upgade plan https://github.com/kubernetes/kubernetes/pull/108447
  • [ ] can we make kubeadm init with machine readable output print the join command + token only? do other commands need machine readable output?

tamalsaha avatar Oct 13 '17 16:10 tamalsaha

Thanks! I'm open to reviewing a short design proposal for this if you can help making that possible. i.e. What exactly would you like to see, what's the use case, etc.

luxas avatar Oct 20 '17 09:10 luxas

Moving out of the v1.9 milestone, as it won't fit into the release anymore at this point @tamalsaha can you provide a short status update on where we are with this feature?

luxas avatar Nov 18 '17 14:11 luxas

I'd help if you could suggest an example of a kind of JSON/YAML object you'd like to see instead of plain log lines kubeadm currently offers!

errordeveloper avatar Jan 24 '18 17:01 errordeveloper

Is this something that can be achieved by vendoring kubeadm into your go binary directly? An example of the desired output could also be helpful.

stealthybox avatar Jan 24 '18 17:01 stealthybox

/remove-help I don't think this meets the criteria for help wanted. Please add back via /help if I'm incorrect

spiffxp avatar Apr 18 '18 22:04 spiffxp

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle stale

fejta-bot avatar Oct 01 '18 20:10 fejta-bot

/lifecycle frozen

neolit123 avatar Oct 01 '18 21:10 neolit123

/assign @detiber

This will likely dovetail with cluster-api work Ideally we will start to refactor the code to be more consumable from an import perspective. This is very similar to other work we had done on sonobuoy.

/cc @chuckha @vincepri

timothysc avatar Oct 06 '18 13:10 timothysc

IMO machine readable output (ideally for all the kubeadm commands) and provide better support for kubeadm vendoring are two separated problems. The latter relates to https://github.com/kubernetes/kubeadm/issues/1205, so IMO this issue should focus only on the first

fabriziopandini avatar Jan 07 '19 19:01 fabriziopandini

@neolit123 @fabriziopandini I can work on this after https://github.com/kubernetes/kubernetes/pull/75633 is merged.

@tamalsaha Is it still needed for you?

bart0sh avatar Mar 27 '19 17:03 bart0sh

Thanks for asking @bart0sh ! This will be still helpful.

tamalsaha avatar Mar 27 '19 17:03 tamalsaha

i think we need to draw the boundary of what is machine readable though. so let's have some discussion before jumping into PRs...

the whole output of init, might be out of scope for now, because even if we put it in fields in a structure, the will just end up as lines of text. so this type of wrapping in a machine readable object is not needed.

however at the end of kubeadm init there is the join command which is useful to have machine readable.

i see this in the lines of

$ kubeadm init:
....
{
  kubeadmLog: <big log here>
  errors: <optionally if there is an error>
  joinCommand: .....
}
$ kubeadm ... --print-join-command --output=json
{
  joinCommand: ....
}
$ kubeadm upgrade plan
{
  plan: {
    ...
  }
}

but we should probably go one command at a time, starting with the smaller ones. also using some sort of a mechanic to marshal any type of object.

@bart0sh next to token list, do you have any other commands in mind? i think we should leave plan, init join for last.... @kubernetes/sig-cluster-lifecycle

neolit123 avatar Mar 27 '19 17:03 neolit123

@neolit123 no, I didn't have anything in particular, but I like the overall idea to have parseable output from kubeadm commands. Why not to do 'upgrade plan' if there is a clear interest for it?

@tamalsaha Can you show us what you'd like to see in 'upgrade plan' JSON or YAML output?

bart0sh avatar Mar 27 '19 17:03 bart0sh

plan is broken ATM, so i need to look at what the problem is. but if we are doing machine readable we should support both YAML and JSON always because kubeadm version already does that -o, --output string Output format; available options are 'yaml', 'json'

neolit123 avatar Mar 27 '19 17:03 neolit123

@bart0sh could you please start with kubeadm config images list ? so that we can define how are we going to do the utility that can marshal any object for any command..

neolit123 avatar Mar 27 '19 17:03 neolit123

yes, the general idea is to support -o, --output yaml, json. This is what I did for 'token list'.

I'll do 'kubeadm config images list' then.

bart0sh avatar Mar 27 '19 17:03 bart0sh

@neolit123 done, please review PR 75878

bart0sh avatar Mar 29 '19 09:03 bart0sh

posted some ideas in this comment on how we may handle this as a common utility: https://github.com/kubernetes/kubernetes/pull/75878#discussion_r270383820

neolit123 avatar Mar 29 '19 14:03 neolit123

I'll answer to your comment here.

we need to define a unified way that handles all possible output format cases in a utility function. we don't want to handle if format == "text" in a lot of places.

Agree with this.

we want to define all outputFormats as constants. OutputFormatText = "text" OutputFormatJSON = "json" OutputFormatYAML = "yaml"

not sure we need this. It's better to keep this logic only in one module.

possibly the location of this should be cmd/kubeadm/app/util/machinereadable.go

If we want to handle all output formats then the name should be different in my opinion. I'd propose cmd/kubeadm/app/util/output.go

we can do marshaling and printing in the same function: PrintMachineReadable(target, input *bytes.Buffer, data interface{}, outputFormat string) error { if outputFormat == "text" { if input == nil { return errors.Errorf("PrintMachineReadable input is nil for outputFormat: %s", outputFormat) } fmt.Fprintf(target, input.String()) return nil } // marshall cases here in a utility function that can be unit tested. }

// usage: var buf bytes.Buffer // write images to buf for format "text" PrintMachineReadable(os.Stdout, &buf, someStruct, format)

I'd propose to implement conversion from data structures to output text. The main API, let's call it ConvertToOutputFormat(data interface{}, outputFormat string) would return string representation of the output. Then caller can do whatever needed with this - parse, print, write to file, parse, etc.

it is still unclear to me how to handle commands like init, join, reset and upgrade

I'd propose to do it the same way - collect the output to the data structure and use the same API.

bart0sh avatar Mar 29 '19 15:03 bart0sh

we want to define all outputFormats as constants. OutputFormatText = "text" OutputFormatJSON = "json" OutputFormatYAML = "yaml"

not sure we need this. It's better to keep this logic only in one module.

if we define the constants in the same file that has the utility function(s) it would be ideal. these constants will be used in multiple files so we need them.

If we want to handle all output formats then the name should be different in my opinion. I'd propose cmd/kubeadm/app/util/output.go

it's OK, outputformat.go seems a bit better.

I'd propose to implement conversion from data structures to output text. The main API, let's call it ConvertToOutputFormat(data interface{}, outputFormat string) would return string representation of the output. Then caller can do whatever needed with this - parse, print, write to file, parse, etc.

ConvertToOutputFormat is OK for yaml and json. if "text" is the desired output format, how are we going to print to stdout or stderr?

it is still unclear to me how to handle commands like init, join, reset and upgrade

I'd propose to do it the same way - collect the output to the data structure and use the same API.

i still think we need some sort of wrapper utility: if "text" - > print to the standard output streams if "yaml/json" -> append to a buffer or a field in a struct.

i suggest that you open this in a google doc so that:

  • we can discuss on the API implementation
  • see examples of handling e.g. config images, token list, init... etc.
  • have a cleaner overview of the proposal

what do you think?

neolit123 avatar Mar 29 '19 16:03 neolit123

@bart0sh I see where you are going with PrintMachineReadable. It's a good idea to have such a function. However, I propose to start simple and if we need something along those lines, we can certainly retrofit it where it's needed.

Also, +1 for @neolit123 's proposition to start drafting ideas in a gdoc.

rosti avatar Mar 29 '19 16:03 rosti

if we define the constants in the same file that has the utility function(s) it would be ideal. these constants will be used in multiple files so we need them.

If they're going to be used in multiple files then we need them. However, I'm having hard time understanding why would we need them outside of outputformat.go

if "text" is the desired output format, how are we going to print to stdout or stderr?

We're going to call ConvertToOutputFormat(&data, "text"), get the output and print it to stdout or stderr.

i suggest that you open this in a google doc so that:

we can discuss on the API implementation see examples of handling e.g. config images, token list, init... etc. have a cleaner overview of the proposal what do you think?

I'm finishing a prototype code that uses this API for 'kubeadm version', 'kubeadm token list' and 'kubeadm config images list'. I'm going to create a doc after that as it's going to take longer than writing a code for me.

bart0sh avatar Mar 29 '19 16:03 bart0sh

@bart0sh I see where you are going with PrintMachineReadable. It's a good idea to have such a function. However, I propose to start simple and if we need something along those lines, we can certainly retrofit it where it's needed.

I already started simple (see https://github.com/kubernetes/kubernetes/pull/75633), but it didn't work out it seems.

Also, +1 for @neolit123 's proposition to start drafting ideas in a gdoc.

Yes, will do.

bart0sh avatar Mar 29 '19 16:03 bart0sh

If they're going to be used in multiple files then we need them. However, I'm having hard time understanding why would we need them outside of outputformat.go

possibly so that the CLI flags can know about the same formats and so that we can perform validation if case a format is unknown.

We're going to call ConvertToOutputFormat(&data, "text"), get the output and print it to stdout or stderr.

but, for "text" we need to print immediately to the standard streams after each line and not buffer the output in the "data" object.

I'm finishing a prototype code that uses this API for 'kubeadm version', 'kubeadm token list' and 'kubeadm config images list'. I'm going to create a doc after that as it's going to take longer than writing a code for me.

sounds good. thanks.

neolit123 avatar Mar 29 '19 16:03 neolit123

possibly so that the CLI flags can know about the same formats and so that we can perform validation if case a format is unknown.

We can also perform validation in the ConvertToOutputFormat and make the CLI code simply defining the flag as it does now.

but, for "text" we need to print immediately to the standard streams after each line and not buffer the output in the "data" object.

that's a valid point. So far I didn't think of this API as a way to handle output of long-running processes. For 'kubeadm version', 'kubeadm token list' and 'kubeadm config images list' the difference is not even noticeable.

bart0sh avatar Mar 29 '19 16:03 bart0sh

@neolit123 'kubeadm version' has 'short' output option. Should we consider having it handled by this API as well?

bart0sh avatar Mar 29 '19 16:03 bart0sh

@neolit123 'kubeadm version' has 'short' output option. Should we consider having it handled by this API as well?

possibly. we want the same code to be used in version.

neolit123 avatar Mar 29 '19 17:03 neolit123

@neolit123 > we want the same code to be used in version.

then we need to handle 'text', 'short', 'json' and 'yaml' This would require implementation for 'short' output in other places('token list', 'images list' etc) as far as I understand.

bart0sh avatar Mar 29 '19 17:03 bart0sh

@neolit123 > but, for "text" we need to print immediately to the standard streams after each line and not buffer the output in the "data" object.

If we really need this, current approach (collect data to some structure and then call output API) would not work. I'd not go that far right now.

P.S. It looks like it's time to create google doc

bart0sh avatar Mar 29 '19 17:03 bart0sh

then we need to handle 'text', 'short', 'json' and 'yaml'

This would require implementation for 'short' output in other places('token list', 'images list' etc) as far as I understand.

we don't want to break "version", by not supporting "short" anymore. we can possibly treat "short"=="text" in cases where it's not supported or throw an error.

neolit123 avatar Mar 29 '19 17:03 neolit123